Codechange: merge guiflags and flags for settings

It was rather confusing which one was for what, especially as some
SaveLoad flags were settings-only. Clean up this mess a bit by
having only Setting flags.

src/saveload/saveload.cpp

@@ -578,7 +578,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
  */
-static inline uint SlCalcConvMemLen(VarType conv)
+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);
@@ -1402,25 +1402,7 @@ static void SlDeque(void *deque, VarType conv)
 /** Are we going to save this object or not? */
 static inline bool SlIsObjectValidInSavegame(const SaveLoad &sld)
 {
-	if (_sl_version < sld.version_from || _sl_version >= sld.version_to) return false;
-	if (sld.conv & SLF_NOT_IN_SAVE) return false;
-
-	return true;
-}
-
-/**
- * Are we going to load this variable when loading a savegame or not?
- * @note If the variable is skipped it is skipped in the savegame
- * bytestream itself as well, so there is no need to skip it somewhere else
- */
-static inline bool SlSkipVariableOnLoad(const SaveLoad &sld)
-{
-	if ((sld.conv & SLF_NO_NETWORK_SYNC) && _sl.action != SLA_SAVE && _networking && !_network_server) {
-		SlSkipBytes(SlCalcConvMemLen(sld.conv) * sld.length);
-		return true;
-	}
-
-	return false;
+	return (_sl_version >= sld.version_from && _sl_version < sld.version_to);
 }
 
 /**
@@ -1535,7 +1517,6 @@ bool SlObjectMember(void *ptr, const SaveLoad &sld)
 		case SL_STDSTR:
 			/* CONDITIONAL saveload types depend on the savegame version */
 			if (!SlIsObjectValidInSavegame(sld)) return false;
-			if (SlSkipVariableOnLoad(sld)) return false;
 
 			switch (sld.cmd) {
 				case SL_VAR: SlSaveLoadConv(ptr, conv); break;

src/saveload/saveload.h

@@ -483,11 +483,8 @@ enum VarTypes {
 
 	/* 8 bits allocated for a maximum of 8 flags
 	 * Flags directing saving/loading of a variable */
-	SLF_NOT_IN_SAVE     = 1 <<  8, ///< do not save with savegame, basically client-based
-	SLF_NOT_IN_CONFIG   = 1 <<  9, ///< do not save to config file
-	SLF_NO_NETWORK_SYNC = 1 << 10, ///< do not synchronize over network (but it is saved if SLF_NOT_IN_SAVE is not set)
-	SLF_ALLOW_CONTROL   = 1 << 11, ///< allow control codes in the strings
-	SLF_ALLOW_NEWLINE   = 1 << 12, ///< allow new lines in the strings
+	SLF_ALLOW_CONTROL   = 1 << 8, ///< Allow control codes in the strings.
+	SLF_ALLOW_NEWLINE   = 1 << 9, ///< Allow new lines in the strings.
 };
 
 typedef uint32 VarType;
@@ -671,7 +668,7 @@ using SaveLoadTable = span<const SaveLoad>;
  * @param from   First savegame version that has the empty space.
  * @param to     Last savegame version that has the empty space.
  */
-#define SLE_CONDNULL(length, from, to) {SL_ARR, SLE_FILE_U8 | SLE_VAR_NULL | SLF_NOT_IN_CONFIG, length, from, to, 0, nullptr, 0}
+#define SLE_CONDNULL(length, from, to) {SL_ARR, SLE_FILE_U8 | SLE_VAR_NULL, length, from, to, 0, nullptr, 0}
 
 /** Translate values ingame to different values in the savegame and vv. */
 #define SLE_WRITEBYTE(base, variable) SLE_GENERAL(SL_WRITEBYTE, base, variable, 0, 0, SL_MIN_VERSION, SL_MAX_VERSION, 0)
@@ -795,7 +792,7 @@ using SaveLoadTable = span<const SaveLoad>;
  * @param from   First savegame version that has the empty space.
  * @param to     Last savegame version that has the empty space.
  */
-#define SLEG_CONDNULL(length, from, to) {SL_ARR, SLE_FILE_U8 | SLE_VAR_NULL | SLF_NOT_IN_CONFIG, length, from, to, 0, nullptr, 0}
+#define SLEG_CONDNULL(length, from, to) {SL_ARR, SLE_FILE_U8 | SLE_VAR_NULL, length, from, to, 0, nullptr, 0}
 
 /**
  * Checks whether the savegame is below \a major.\a minor.
@@ -894,6 +891,7 @@ 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);

src/script/api/script_gamesettings.cpp

@@ -35,7 +35,7 @@
 
 	const SettingDesc *sd = GetSettingFromName(setting);
 
-	if ((sd->save.conv & SLF_NO_NETWORK_SYNC) != 0) return false;
+	if ((sd->flags & SF_NO_NETWORK_SYNC) != 0) return false;
 
 	return ScriptObject::DoCommand(0, 0, value, CMD_CHANGE_SETTING, sd->name);
 }

src/settings.cpp

@@ -591,7 +591,7 @@ static void IniSaveSettings(IniFile *ini, const SettingTable &settings_table, co
 		/* If the setting is not saved to the configuration
 		 * file, just continue with the next setting */
 		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
-		if (sd->save.conv & SLF_NOT_IN_CONFIG) continue;
+		if (sd->flags & SF_NOT_IN_CONFIG) continue;
 
 		/* XXX - wtf is this?? (group override?) */
 		std::string s{ sd->name };
@@ -741,7 +741,7 @@ void IniSaveWindowSettings(IniFile *ini, const char *grpname, void *desc)
  */
 bool SettingDesc::IsEditable(bool do_command) const
 {
-	if (!do_command && !(this->save.conv & SLF_NO_NETWORK_SYNC) && _networking && !_network_server && !(this->flags & SF_PER_COMPANY)) return false;
+	if (!do_command && !(this->flags & SF_NO_NETWORK_SYNC) && _networking && !_network_server && !(this->flags & SF_PER_COMPANY)) return false;
 	if ((this->flags & SF_NETWORK_ONLY) && !_networking && _game_mode != GM_MENU) return false;
 	if ((this->flags & SF_NO_NETWORK) && _networking) return false;
 	if ((this->flags & SF_NEWGAME_ONLY) &&
@@ -758,7 +758,7 @@ bool SettingDesc::IsEditable(bool do_command) const
 SettingType SettingDesc::GetType() const
 {
 	if (this->flags & SF_PER_COMPANY) return ST_COMPANY;
-	return (this->save.conv & SLF_NOT_IN_SAVE) ? ST_CLIENT : ST_GAME;
+	return (this->flags & SF_NOT_IN_SAVE) ? ST_CLIENT : ST_GAME;
 }
 
 /**
@@ -1844,7 +1844,7 @@ bool SetSettingValue(const IntSettingDesc *sd, int32 value, bool force_newgame)
 	 * (if any) to change. Also *hack*hack* we update the _newgame version
 	 * of settings because changing a company-based setting in a game also
 	 * changes its defaults. At least that is the convention we have chosen */
-	if (setting->save.conv & SLF_NO_NETWORK_SYNC) {
+	if (setting->flags & SF_NO_NETWORK_SYNC) {
 		if (_game_mode != GM_MENU) {
 			setting->ChangeValue(&_settings_newgame, value);
 		}
@@ -1899,7 +1899,7 @@ void SyncCompanySettings()
  */
 bool SetSettingValue(const StringSettingDesc *sd, std::string value, bool force_newgame)
 {
-	assert(sd->save.conv & SLF_NO_NETWORK_SYNC);
+	assert(sd->flags & SF_NO_NETWORK_SYNC);
 
 	if (GetVarMemType(sd->save.conv) == SLE_VAR_STRQ && value.compare("(null)") == 0) {
 		value.clear();
@@ -2023,9 +2023,17 @@ void IConsoleListSettings(const char *prefilter)
 static void LoadSettings(const SettingTable &settings, void *object)
 {
 	for (auto &osd : settings) {
-		void *ptr = GetVariableAddress(object, osd->save);
+		if (osd->flags & SF_NOT_IN_SAVE) continue;
+
+		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));
+		}
+
+		void *ptr = GetVariableAddress(object, sl);
+		if (!SlObjectMember(ptr, sl)) continue;
 
-		if (!SlObjectMember(ptr, osd->save)) continue;
 		if (osd->IsIntSetting()) {
 			const IntSettingDesc *int_setting = osd->AsIntSetting();
 			int_setting->MakeValueValidAndWrite(object, int_setting->Read(object));
@@ -2045,11 +2053,15 @@ static void SaveSettings(const SettingTable &settings, void *object)
 	 * 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);
 
 	for (auto &sd : settings) {
+		if (sd->flags & SF_NOT_IN_SAVE) continue;
+
 		void *ptr = GetVariableAddress(object, sd->save);
 		SlObjectMember(ptr, sd->save);
 	}

src/settings_internal.h

@@ -14,16 +14,19 @@
 
 enum SettingFlag : uint16 {
 	SF_NONE = 0,
-	SF_GUI_0_IS_SPECIAL        = 1 << 0, ///< A value of zero is possible and has a custom string (the one after "strval").
-	SF_GUI_NEGATIVE_IS_SPECIAL = 1 << 1, ///< A negative value has another string (the one after "strval").
-	SF_GUI_DROPDOWN            = 1 << 2, ///< The value represents a limited number of string-options (internally integer) presented as dropdown.
-	SF_GUI_CURRENCY            = 1 << 3, ///< The number represents money, so when reading value multiply by exchange rate.
-	SF_NETWORK_ONLY            = 1 << 4, ///< This setting only applies to network games.
-	SF_NO_NETWORK              = 1 << 5, ///< This setting does not apply to network games; it may not be changed during the game.
-	SF_NEWGAME_ONLY            = 1 << 6, ///< This setting cannot be changed in a game.
-	SF_SCENEDIT_TOO            = 1 << 7, ///< This setting can be changed in the scenario editor (only makes sense when SF_NEWGAME_ONLY is set).
-	SF_SCENEDIT_ONLY           = 1 << 8, ///< This setting can only be changed in the scenario editor.
-	SF_PER_COMPANY             = 1 << 9, ///< This setting can be different for each company (saved in company struct).
+	SF_GUI_0_IS_SPECIAL        = 1 <<  0, ///< A value of zero is possible and has a custom string (the one after "strval").
+	SF_GUI_NEGATIVE_IS_SPECIAL = 1 <<  1, ///< A negative value has another string (the one after "strval").
+	SF_GUI_DROPDOWN            = 1 <<  2, ///< The value represents a limited number of string-options (internally integer) presented as dropdown.
+	SF_GUI_CURRENCY            = 1 <<  3, ///< The number represents money, so when reading value multiply by exchange rate.
+	SF_NETWORK_ONLY            = 1 <<  4, ///< This setting only applies to network games.
+	SF_NO_NETWORK              = 1 <<  5, ///< This setting does not apply to network games; it may not be changed during the game.
+	SF_NEWGAME_ONLY            = 1 <<  6, ///< This setting cannot be changed in a game.
+	SF_SCENEDIT_TOO            = 1 <<  7, ///< This setting can be changed in the scenario editor (only makes sense when SF_NEWGAME_ONLY is set).
+	SF_SCENEDIT_ONLY           = 1 <<  8, ///< This setting can only be changed in the scenario editor.
+	SF_PER_COMPANY             = 1 <<  9, ///< This setting can be different for each company (saved in company struct).
+	SF_NOT_IN_SAVE             = 1 << 10, ///< Do not save with savegame, basically client-based.
+	SF_NOT_IN_CONFIG           = 1 << 11, ///< Do not save to config file.
+	SF_NO_NETWORK_SYNC         = 1 << 12, ///< Do not synchronize over network (but it is saved if SF_NOT_IN_SAVE is not set).
 };
 DECLARE_ENUM_AS_BIT_SET(SettingFlag)
 
@@ -288,7 +291,7 @@ struct ListSettingDesc : SettingDesc {
 /** Placeholder for settings that have been removed, but might still linger in the savegame. */
 struct NullSettingDesc : SettingDesc {
 	NullSettingDesc(SaveLoad save) :
-		SettingDesc(save, "", SF_NONE, false) {}
+		SettingDesc(save, "", SF_NOT_IN_CONFIG, false) {}
 	virtual ~NullSettingDesc() {}
 
 	void FormatValue(char *buf, const char *last, const void *object) const override { NOT_REACHED(); }