From 8b4114d709a79fb7759935907e755faae5eb7d67 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sun, 23 Mar 2025 15:55:55 +0000 Subject: [PATCH] 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 | 18 +++++++++++++ src/saveload/settings_sl.cpp | 49 +++--------------------------------- src/settings.cpp | 29 +++++++++++++++++++++ src/settings_internal.h | 1 + 4 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/saveload/saveload.cpp b/src/saveload/saveload.cpp index 1dd460216f..c585851644 100644 --- a/src/saveload/saveload.cpp +++ b/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(); } /** diff --git a/src/saveload/settings_sl.cpp b/src/saveload/settings_sl.cpp index 96e50ecc9a..62a1a1c243 100644 --- a/src/saveload/settings_sl.cpp +++ b/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 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(); - - /* 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); + LoadSettings(GetSaveLoadSettingTable(), &_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); } }; diff --git a/src/settings.cpp b/src/settings.cpp index 6baf16c4f6..e405fcb6a7 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -1660,6 +1660,35 @@ void GetSaveLoadFromSettingTable(SettingTable settings, std::vector &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 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. diff --git a/src/settings_internal.h b/src/settings_internal.h index 09e2082866..8fcf2cd71b 100644 --- a/src/settings_internal.h +++ b/src/settings_internal.h @@ -390,6 +390,7 @@ typedef std::span SettingTable; const SettingDesc *GetSettingFromName(const std::string_view name); void GetSaveLoadFromSettingTable(SettingTable settings, std::vector &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);