Some improvements to LegoGameState (#1353)

* Beta match ReadVariable and WriteVariable

* Change param type for GetFileSavePath

* Remove copy constructor for LegoGameState::Username

LEGO1/lego/legoomni/include/legogamestate.h

@@ -135,7 +135,6 @@ public:
 	// SIZE 0x0e
 	struct Username {
 		Username();
-		Username(Username& p_other) { Set(p_other); }
 		void Set(Username& p_other) { memcpy(m_letters, p_other.m_letters, sizeof(m_letters)); }
 
 		MxResult Serialize(LegoStorage* p_storage);
@@ -193,7 +192,7 @@ public:
 	LegoState* GetState(const char* p_stateName);
 	LegoState* CreateState(const char* p_stateName);
 
-	void GetFileSavePath(MxString* p_outPath, MxU8 p_slotn);
+	void GetFileSavePath(MxString* p_outPath, MxS16 p_slotn);
 	void StopArea(Area p_area);
 	void SwitchArea(Area p_area);
 	void Init();

LEGO1/lego/legoomni/src/common/legogamestate.cpp

@@ -87,6 +87,8 @@ const char* g_historyGSI = "History.gsi";
 // TODO: make g_endOfVariables reference the actual end of the variable array.
 // GLOBAL: LEGO1 0x100f3e50
 // STRING: LEGO1 0x100f3e00
+// GLOBAL: BETA10 0x101ed5dc
+// STRING: BETA10 0x101ed768
 const char* g_endOfVariables = "END_OF_VARIABLES";
 
 // GLOBAL: LEGO1 0x100f3e58
@@ -250,6 +252,7 @@ void LegoGameState::ResetROI()
 }
 
 // FUNCTION: LEGO1 0x10039980
+// FUNCTION: BETA10 0x100840e4
 MxResult LegoGameState::Save(MxULong p_slot)
 {
 	InfocenterState* infocenterState = (InfocenterState*) GameState()->GetState("InfocenterState");
@@ -462,6 +465,7 @@ void LegoGameState::SetSavePath(char* p_savePath)
 }
 
 // FUNCTION: LEGO1 0x10039f70
+// FUNCTION: BETA10 0x1008483b
 MxResult LegoGameState::WriteVariable(LegoStorage* p_storage, MxVariableTable* p_from, const char* p_variableName)
 {
 	MxResult result = FAILURE;
@@ -469,20 +473,28 @@ MxResult LegoGameState::WriteVariable(LegoStorage* p_storage, MxVariableTable* p
 
 	if (variableValue) {
 		MxU8 length = strlen(p_variableName);
-		if (p_storage->Write(&length, sizeof(length)) == SUCCESS) {
+		if (p_storage->Write(&length, sizeof(length)) != SUCCESS) {
-			if (p_storage->Write(p_variableName, length) == SUCCESS) {
+			goto done;
-				length = strlen(variableValue);
-				if (p_storage->Write(&length, sizeof(length)) == SUCCESS) {
-					result = p_storage->Write(variableValue, length);
-				}
-			}
 		}
+
+		if (p_storage->Write(p_variableName, length) != SUCCESS) {
+			goto done;
+		}
+
+		length = strlen(variableValue);
+		if (p_storage->Write(&length, sizeof(length)) != SUCCESS) {
+			goto done;
+		}
+
+		result = p_storage->Write(variableValue, length);
 	}
 
+done:
 	return result;
 }
 
 // FUNCTION: LEGO1 0x1003a020
+// FUNCTION: BETA10 0x10084928
 MxResult LegoGameState::WriteEndOfVariables(LegoStorage* p_storage)
 {
 	MxU8 len = strlen(g_endOfVariables);
@@ -495,37 +507,52 @@ MxResult LegoGameState::WriteEndOfVariables(LegoStorage* p_storage)
 }
 
 // FUNCTION: LEGO1 0x1003a080
+// FUNCTION: BETA10 0x1008498b
 MxS32 LegoGameState::ReadVariable(LegoStorage* p_storage, MxVariableTable* p_to)
 {
 	MxS32 result = 1;
-	MxU8 length;
+	MxU8 len;
 
-	if (p_storage->Read(&length, sizeof(length)) == SUCCESS) {
+	if (p_storage->Read(&len, sizeof(len)) != SUCCESS) {
-		char nameBuffer[256];
+		goto done;
-		if (p_storage->Read(nameBuffer, length) == SUCCESS) {
-			nameBuffer[length] = '\0';
-			if (strcmp(nameBuffer, g_endOfVariables) == 0) {
-				// 2 -> "This was the last entry, done reading."
-				result = 2;
-			}
-			else {
-				if (p_storage->Read(&length, sizeof(length)) == SUCCESS) {
-					char valueBuffer[256];
-					if (p_storage->Read(valueBuffer, length) == SUCCESS) {
-						valueBuffer[length] = '\0';
-						p_to->SetVariable(nameBuffer, valueBuffer);
-						result = SUCCESS;
-					}
-				}
-			}
-		}
 	}
 
+	char varName[256];
+	assert(len < sizeof(varName));
+
+	if (p_storage->Read(varName, len) != SUCCESS) {
+		goto done;
+	}
+
+	varName[len] = '\0';
+	if (strcmp(varName, g_endOfVariables) == 0) {
+		// 2 -> "This was the last entry, done reading."
+		result = 2;
+		goto done;
+	}
+
+	if (p_storage->Read(&len, sizeof(len)) != SUCCESS) {
+		goto done;
+	}
+
+	char value[256];
+	assert(len < sizeof(value));
+
+	if (p_storage->Read(value, len) != SUCCESS) {
+		goto done;
+	}
+
+	value[len] = '\0';
+	p_to->SetVariable(varName, value);
+	result = SUCCESS;
+
+done:
 	return result;
 }
 
 // FUNCTION: LEGO1 0x1003a170
-void LegoGameState::GetFileSavePath(MxString* p_outPath, MxU8 p_slotn)
+// FUNCTION: BETA10 0x10084b45
+void LegoGameState::GetFileSavePath(MxString* p_outPath, MxS16 p_slotn)
 {
 	char baseForSlot[2] = "0";
 	char path[1024] = "";
@@ -597,6 +624,7 @@ MxResult LegoGameState::AddPlayer(Username& p_player)
 }
 
 // FUNCTION: LEGO1 0x1003a540
+// FUNCTION: BETA10 0x10084fc4
 void LegoGameState::SwitchPlayer(MxS16 p_playerId)
 {
 	if (p_playerId > 0) {