Codechange: make [Save|Load]Settings() behave more like other Save/Load code (#9335)

Prepare the full description and send it to SlObject. This does
require some code to be able to read to a SLE_VAR_NULL, like strings
etc, as there is no way to know their length beforehand.

3 files changed, 47 insertions, 29 deletions

diff --git a/src/saveload/saveload.cpp b/src/saveload/saveload.cpp
index 35a6437e1..29fcf89da 100644
--- a/src/saveload/saveload.cpp
+++ b/src/saveload/saveload.cpp
@@ -584,7 +584,7 @@ static inline uint SlGetArrayLength(size_t length)
  * @param conv VarType type of variable that is used for calculating the size
  * @return Return the size of this type in bytes
  */
-uint SlCalcConvMemLen(VarType conv)
+static inline uint SlCalcConvMemLen(VarType conv)
 {
 	static const byte conv_mem_size[] = {1, 1, 1, 2, 2, 4, 4, 8, 8, 0};
 	byte length = GB(conv, 4, 4);
@@ -943,6 +943,9 @@ static void SlString(void *ptr, size_t length, VarType conv)
 
 			switch (GetVarMemType(conv)) {
 				default: NOT_REACHED();
+				case SLE_VAR_NULL:
+					SlSkipBytes(len);
+					return;
 				case SLE_VAR_STRB:
 					if (len >= length) {
 						DEBUG(sl, 1, "String length in savegame is bigger than buffer, truncating");
@@ -1007,8 +1010,12 @@ static void SlStdString(void *ptr, VarType conv)
 		case SLA_LOAD_CHECK:
 		case SLA_LOAD: {
 			size_t len = SlReadArrayLength();
-			char *buf = AllocaM(char, len + 1);
+			if (GetVarMemType(conv) == SLE_VAR_NULL) {
+				SlSkipBytes(len);
+				return;
+			}
 
+			char *buf = AllocaM(char, len + 1);
 			SlCopyBytes(buf, len);
 			buf[len] = '\0'; // properly terminate the string
 
@@ -1469,6 +1476,8 @@ size_t SlCalcObjMemberLength(const void *object, const SaveLoad &sld)
  */
 [[maybe_unused]] static bool IsVariableSizeRight(const SaveLoad &sld)
 {
+	if (GetVarMemType(sld.conv) == SLE_VAR_NULL) return true;
+
 	switch (sld.cmd) {
 		case SL_VAR:
 			switch (GetVarMemType(sld.conv)) {
diff --git a/src/saveload/saveload.h b/src/saveload/saveload.h
index 94138eb74..53a81f7c2 100644
--- a/src/saveload/saveload.h
+++ b/src/saveload/saveload.h
@@ -893,7 +893,6 @@ void WriteValue(void *ptr, VarType conv, int64 val);
 void SlSetArrayIndex(uint index);
 int SlIterateArray();
 
-uint SlCalcConvMemLen(VarType conv);
 void SlAutolength(AutolengthProc *proc, void *arg);
 size_t SlGetFieldLength();
 void SlSetLength(size_t length);
diff --git a/src/settings.cpp b/src/settings.cpp
index c3afd5508..24039c564 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -2015,6 +2015,31 @@ void IConsoleListSettings(const char *prefilter)
 }
 
 /**
+ * Get the SaveLoad description for the SettingTable.
+ * @param settings SettingDesc struct containing all information.
+ * @param is_loading True iff the SaveLoad table is for loading.
+ * @return Vector with SaveLoad entries for the SettingTable.
+ */
+static std::vector<SaveLoad> GetSettingsDesc(const SettingTable &settings, bool is_loading)
+{
+	std::vector<SaveLoad> saveloads;
+	for (auto &sd : settings) {
+		if (sd->flags & SF_NOT_IN_SAVE) continue;
+
+		if (is_loading && (sd->flags & SF_NO_NETWORK_SYNC) && _networking && !_network_server) {
+			/* We don't want to read this setting, so we do need to skip over it. */
+			saveloads.push_back({sd->save.cmd, GetVarFileType(sd->save.conv) | SLE_VAR_NULL, sd->save.length, sd->save.version_from, sd->save.version_to, 0, nullptr, 0});
+			continue;
+		}
+
+		saveloads.push_back(sd->save);
+	}
+
+	return saveloads;
+}
+
+
+/**
  * Save and load handler for settings
  * @param settings SettingDesc struct containing all information
  * @param object can be either nullptr in which case we load global variables or
@@ -2022,20 +2047,18 @@ void IConsoleListSettings(const char *prefilter)
  */
 static void LoadSettings(const SettingTable &settings, void *object)
 {
-	for (auto &osd : settings) {
-		if (osd->flags & SF_NOT_IN_SAVE) continue;
+	const std::vector<SaveLoad> slt = GetSettingsDesc(settings, true);
 
-		SaveLoad sl = osd->save;
-		if ((osd->flags & SF_NO_NETWORK_SYNC) && _networking && !_network_server) {
-			/* We don't want to read this setting, so we do need to skip over it. */
-			sl = SLE_NULL(static_cast<uint16>(SlCalcConvMemLen(osd->save.conv) * osd->save.length));
-		}
+	SlObject(object, slt);
 
-		void *ptr = GetVariableAddress(object, sl);
-		if (!SlObjectMember(ptr, sl)) continue;
+	/* Ensure all IntSettings are valid (min/max could have changed between versions etc). */
+	for (auto &sd : settings) {
+		if (sd->flags & SF_NOT_IN_SAVE) continue;
+		if ((sd->flags & SF_NO_NETWORK_SYNC) && _networking && !_network_server) continue;
+		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
 
-		if (osd->IsIntSetting()) {
-			const IntSettingDesc *int_setting = osd->AsIntSetting();
+		if (sd->IsIntSetting()) {
+			const IntSettingDesc *int_setting = sd->AsIntSetting();
 			int_setting->MakeValueValidAndWrite(object, int_setting->Read(object));
 		}
 	}
@@ -2049,22 +2072,9 @@ static void LoadSettings(const SettingTable &settings, void *object)
  */
 static void SaveSettings(const SettingTable &settings, void *object)
 {
-	/* We need to write the CH_RIFF header, but unfortunately can't call
-	 * SlCalcLength() because we have a different format. So do this manually */
-	size_t length = 0;
-	for (auto &sd : settings) {
-		if (sd->flags & SF_NOT_IN_SAVE) continue;
-
-		length += SlCalcObjMemberLength(object, sd->save);
-	}
-	SlSetLength(length);
+	const std::vector<SaveLoad> slt = GetSettingsDesc(settings, false);
 
-	for (auto &sd : settings) {
-		if (sd->flags & SF_NOT_IN_SAVE) continue;
-
-		void *ptr = GetVariableAddress(object, sd->save);
-		SlObjectMember(ptr, sd->save);
-	}
+	SlObject(object, slt);
 }
 
 static void Load_OPTS()