Fix #13849: Settings in old saves could be overridden by defaults. (#13874)

Resolved by resetting settings to default values before the OPTS and PATS chunks are loaded.

src/saveload/saveload.cpp

@@ -45,6 +45,7 @@
 #include "../strings_type.h"
 #include "../newgrf_railtype.h"
 #include "../newgrf_roadtype.h"
+#include "../settings_internal.h"
 #include "saveload_internal.h"
 #include "saveload_filter.h"
 
@@ -2852,6 +2853,22 @@ void InitializeGame(uint size_x, uint size_y, bool reset_date, bool reset_settin
 extern bool AfterLoadGame();
 extern bool LoadOldSaveGame(const std::string &file);
 
+/**
+ * Reset all settings to their default, so any settings missing in the savegame
+ * are their default, and not "value of last game". AfterLoad might still fix
+ * up values to become non-default, depending on the saveload version.
+ */
+static void ResetSettings()
+{
+	for (auto &desc : GetSaveLoadSettingTable()) {
+		const SettingDesc *sd = GetSettingDesc(desc);
+		if (sd->flags.Test(SettingFlag::NotInSave)) continue;
+		if (sd->flags.Test(SettingFlag::NoNetworkSync) && _networking && !_network_server) continue;
+
+		sd->ResetToDefault(&_settings_game);
+	}
+}
+
 /**
  * Clear temporary data that is passed between various saveload phases.
  */
@@ -2861,6 +2878,7 @@ static void ResetSaveloadData()
 	ClearRailTypeLabelList();
 	ClearRoadTypeLabelList();
 	ResetOldWaypoints();
+	ResetSettings();
 }
 
 /**

src/saveload/settings_sl.cpp

@@ -14,6 +14,7 @@
 
 #include "../settings_type.h"
 #include "../settings_table.h"
+#include "../settings_internal.h"
 #include "../network/network.h"
 #include "../fios.h"
 
@@ -156,61 +157,19 @@ struct OPTSChunkHandler : ChunkHandler {
 struct PATSChunkHandler : ChunkHandler {
 	PATSChunkHandler() : ChunkHandler('PATS', CH_TABLE) {}
 
-	/**
-	 * Create a single table with all settings that should be stored/loaded
-	 * in the savegame.
-	 */
-	SettingTable GetSettingTable() const
-	{
-		static const SettingTable saveload_settings_tables[] = {
-			_difficulty_settings,
-			_economy_settings,
-			_game_settings,
-			_linkgraph_settings,
-			_locale_settings,
-			_pathfinding_settings,
-			_script_settings,
-			_world_settings,
-		};
-		static std::vector<SettingVariant> settings_table;
-
-		if (settings_table.empty()) {
-			for (auto &saveload_settings_table : saveload_settings_tables) {
-				for (auto &saveload_setting : saveload_settings_table) {
-					settings_table.push_back(saveload_setting);
-				}
-			}
-		}
-
-		return settings_table;
-	}
-
 	void Load() const override
 	{
-		const auto settings_table = this->GetSettingTable();
+		LoadSettings(GetSaveLoadSettingTable(), &_settings_game, _settings_sl_compat);
-
-		/* Reset all settings to their default, so any settings missing in the savegame
-		 * are their default, and not "value of last game". AfterLoad might still fix
-		 * up values to become non-default, depending on the saveload version. */
-		for (auto &desc : settings_table) {
-			const SettingDesc *sd = GetSettingDesc(desc);
-			if (sd->flags.Test(SettingFlag::NotInSave)) continue;
-			if (sd->flags.Test(SettingFlag::NoNetworkSync) && _networking && !_network_server) continue;
-
-			sd->ResetToDefault(&_settings_game);
-		}
-
-		LoadSettings(settings_table, &_settings_game, _settings_sl_compat);
 	}
 
 	void LoadCheck(size_t) const override
 	{
-		LoadSettings(this->GetSettingTable(), &_load_check_data.settings, _settings_sl_compat);
+		LoadSettings(GetSaveLoadSettingTable(), &_load_check_data.settings, _settings_sl_compat);
 	}
 
 	void Save() const override
 	{
-		SaveSettings(this->GetSettingTable(), &_settings_game);
+		SaveSettings(GetSaveLoadSettingTable(), &_settings_game);
 	}
 };
 

src/settings.cpp

@@ -1660,6 +1660,35 @@ void GetSaveLoadFromSettingTable(SettingTable settings, std::vector<SaveLoad> &s
 	}
 }
 
+/**
+ * Create a single table with all settings that should be stored/loaded
+ * in the savegame.
+ */
+SettingTable GetSaveLoadSettingTable()
+{
+	static const SettingTable saveload_settings_tables[] = {
+		_difficulty_settings,
+		_economy_settings,
+		_game_settings,
+		_linkgraph_settings,
+		_locale_settings,
+		_pathfinding_settings,
+		_script_settings,
+		_world_settings,
+	};
+	static std::vector<SettingVariant> settings_table;
+
+	if (settings_table.empty()) {
+		for (auto &saveload_settings_table : saveload_settings_tables) {
+			for (auto &saveload_setting : saveload_settings_table) {
+				settings_table.push_back(saveload_setting);
+			}
+		}
+	}
+
+	return settings_table;
+}
+
 /**
  * Given a name of setting, return a company setting description of it.
  * @param name  Name of the company setting to return a setting description of.

src/settings_internal.h

@@ -390,6 +390,7 @@ typedef std::span<const SettingVariant> SettingTable;
 
 const SettingDesc *GetSettingFromName(const std::string_view name);
 void GetSaveLoadFromSettingTable(SettingTable settings, std::vector<SaveLoad> &saveloads);
+SettingTable GetSaveLoadSettingTable();
 bool SetSettingValue(const IntSettingDesc *sd, int32_t value, bool force_newgame = false);
 bool SetSettingValue(const StringSettingDesc *sd, const std::string value, bool force_newgame = false);