Fix: store autosave settings under the new names

src/openttd.cpp

@@ -102,15 +102,6 @@ bool _save_config = false;
 bool _request_newgrf_scan = false;
 NewGRFScanCallback *_request_newgrf_scan_callback = nullptr;
 
-/** Available settings for autosave intervals. */
-static const std::chrono::milliseconds _autosave_ticks[] = {
-	std::chrono::minutes::zero(), ///< never
-	std::chrono::minutes(10),
-	std::chrono::minutes(30),
-	std::chrono::minutes(60),
-	std::chrono::minutes(120),
-};
-
 /**
  * Error handling for fatal user errors.
  * @param str the string to print.
@@ -1458,7 +1449,7 @@ static IntervalTimer<TimerGameRealtime> _autosave_interval({std::chrono::millise
  */
 void ChangeAutosaveFrequency(bool reset)
 {
-	_autosave_interval.SetInterval({_autosave_ticks[_settings_client.gui.autosave], TimerGameRealtime::AUTOSAVE}, reset);
+	_autosave_interval.SetInterval({_settings_client.gui.autosave_interval, TimerGameRealtime::AUTOSAVE}, reset);
 }
 
 /**

src/settings.cpp

@@ -161,6 +161,8 @@ enum IniFileVersion : uint32 {
 	IFV_LINKGRAPH_SECONDS,                                 ///< 3  PR#10610 Store linkgraph update intervals in seconds instead of days.
 	IFV_NETWORK_PRIVATE_SETTINGS,                          ///< 4  PR#10762 Move no_http_content_downloads / use_relay_service to private settings.
 
+	IFV_AUTOSAVE_RENAME,                                   ///< 5  PR#11143 Renamed values of autosave to be in minutes.
+
 	IFV_MAX_VERSION,       ///< Highest possible ini-file version.
 };
 
@@ -187,6 +189,20 @@ size_t OneOfManySettingDesc::ParseSingleValue(const char *str, size_t len, const
 	return (size_t)-1;
 }
 
+/**
+ * Find whether a string was a boolean true or a boolean false.
+ *
+ * @param str the current value of the setting for which a value needs found.
+ * @return Either true/false, or nullopt if no boolean value found.
+ */
+std::optional<bool> BoolSettingDesc::ParseSingleValue(const char *str)
+{
+	if (strcmp(str, "true") == 0 || strcmp(str, "on") == 0 || strcmp(str, "1") == 0) return true;
+	if (strcmp(str, "false") == 0 || strcmp(str, "off") == 0 || strcmp(str, "0") == 0) return false;
+
+	return std::nullopt;
+}
+
 /**
  * Find the set-integer value MANYofMANY type in a string
  * @param many full domain of values the MANYofMANY setting can have
@@ -422,8 +438,8 @@ size_t ManyOfManySettingDesc::ParseValue(const char *str) const
 
 size_t BoolSettingDesc::ParseValue(const char *str) const
 {
-	if (strcmp(str, "true") == 0 || strcmp(str, "on") == 0 || strcmp(str, "1") == 0) return true;
-	if (strcmp(str, "false") == 0 || strcmp(str, "off") == 0 || strcmp(str, "0") == 0) return false;
+	auto r = BoolSettingDesc::ParseSingleValue(str);
+	if (r.has_value()) return *r;
 
 	ErrorMessageData msg(STR_CONFIG_ERROR, STR_CONFIG_ERROR_INVALID_VALUE);
 	msg.SetDParamStr(0, str);
@@ -1196,6 +1212,53 @@ static void RemoveEntriesFromIni(IniFile &ini, const SettingTable &table)
 	}
 }
 
+/**
+ * Check whether a conversion should be done, and based on what old setting information.
+ *
+ * To prevent errors when switching back and forth between older and newer
+ * version of OpenTTD, the type of a setting is never changed. Instead, the
+ * setting is renamed, and this function is used to check whether a conversion
+ * between the old and new setting is required.
+ *
+ * This checks if the new setting doesn't exist, and if the old does.
+ *
+ * Doing it this way means that if you switch to an older client, the old
+ * setting is used, and only on the first time starting a new client, the
+ * old setting is converted to the new. After that, they are independent
+ * of each other. And you can safely, without errors on either, switch
+ * between old and new client.
+ *
+ * @param ini The ini-file to use.
+ * @param group The group the setting is in.
+ * @param old_var The old name of the setting.
+ * @param new_var The new name of the setting.
+ * @param[out] old_item The old item to base upgrading on.
+ * @return Whether upgrading should happen; if false, old_item is a nullptr.
+ */
+bool IsConversionNeeded(ConfigIniFile &ini, std::string group, std::string old_var, std::string new_var, IniItem **old_item)
+{
+	*old_item = nullptr;
+
+	IniGroup *igroup = ini.GetGroup(group, false);
+	/* If the group doesn't exist, there is nothing to convert. */
+	if (igroup == nullptr) return false;
+
+	IniItem *tmp_old_item = igroup->GetItem(old_var);
+	IniItem *new_item = igroup->GetItem(new_var);
+
+	/* If the old item doesn't exist, there is nothing to convert. */
+	if (tmp_old_item == nullptr) return false;
+
+	/* If the new item exists, it means conversion was already done. We only
+	 * do the conversion the first time, and after that these settings are
+	 * independent. This allows users to freely change between older and
+	 * newer clients without breaking anything. */
+	if (new_item != nullptr) return false;
+
+	*old_item = tmp_old_item;
+	return true;
+}
+
 /**
  * Load the values from the configuration files
  * @param startup Load the minimal amount of the configuration to "bootstrap" the blitter and such.
@@ -1219,24 +1282,12 @@ void LoadFromConfig(bool startup)
 
 	/* Load basic settings only during bootstrap, load other settings not during bootstrap */
 	if (!startup) {
-		/* Convert network.server_advertise to network.server_game_type, but only if network.server_game_type is set to default value. */
-		if (generic_version < IFV_GAME_TYPE) {
-			if (_settings_client.network.server_game_type == SERVER_GAME_TYPE_LOCAL) {
-				IniGroup *network = generic_ini.GetGroup("network", false);
-				if (network != nullptr) {
-					IniItem *server_advertise = network->GetItem("server_advertise");
-					if (server_advertise != nullptr && server_advertise->value == "true") {
-						_settings_client.network.server_game_type = SERVER_GAME_TYPE_PUBLIC;
-					}
-				}
-			}
-		}
-
 		if (generic_version < IFV_LINKGRAPH_SECONDS) {
 			_settings_newgame.linkgraph.recalc_interval *= SECONDS_PER_DAY;
 			_settings_newgame.linkgraph.recalc_time     *= SECONDS_PER_DAY;
 		}
 
+		/* Move no_http_content_downloads and use_relay_service from generic_ini to private_ini. */
 		if (generic_version < IFV_NETWORK_PRIVATE_SETTINGS) {
 			IniGroup *network = generic_ini.GetGroup("network", false);
 			if (network != nullptr) {
@@ -1262,6 +1313,27 @@ void LoadFromConfig(bool startup)
 			}
 		}
 
+		IniItem *old_item;
+
+		if (generic_version < IFV_GAME_TYPE && IsConversionNeeded(generic_ini, "network", "server_advertise", "server_game_type", &old_item)) {
+			auto old_value = BoolSettingDesc::ParseSingleValue(old_item->value->c_str());
+			_settings_client.network.server_game_type = old_value.value_or(false) ? SERVER_GAME_TYPE_PUBLIC : SERVER_GAME_TYPE_LOCAL;
+		}
+
+		if (generic_version < IFV_AUTOSAVE_RENAME && IsConversionNeeded(generic_ini, "gui", "autosave", "autosave_interval", &old_item)) {
+			static std::vector<std::string> _old_autosave_interval{"off", "monthly", "quarterly", "half year", "yearly"};
+			auto old_value = OneOfManySettingDesc::ParseSingleValue(old_item->value->c_str(), old_item->value->size(), _old_autosave_interval);
+
+			switch (old_value) {
+				case 0: _settings_client.gui.autosave_interval = std::chrono::minutes::zero(); break;
+				case 1: _settings_client.gui.autosave_interval = std::chrono::minutes(10); break;
+				case 2: _settings_client.gui.autosave_interval = std::chrono::minutes(30); break;
+				case 3: _settings_client.gui.autosave_interval = std::chrono::minutes(60); break;
+				case 4: _settings_client.gui.autosave_interval = std::chrono::minutes(120); break;
+				default: break;
+			}
+		}
+
 		_grfconfig_newgame = GRFLoadConfig(generic_ini, "newgrf", false);
 		_grfconfig_static  = GRFLoadConfig(generic_ini, "newgrf-static", true);
 		AILoadConfig(generic_ini, "ai_players");
@@ -1316,13 +1388,7 @@ void SaveToConfig()
 		}
 	}
 
-	/* Remove network.server_advertise. */
-	if (generic_version < IFV_GAME_TYPE) {
-		IniGroup *network = generic_ini.GetGroup("network", false);
-		if (network != nullptr) {
-			network->RemoveItem("server_advertise");
-		}
-	}
+	/* These variables are migrated from generic ini to private ini now. */
 	if (generic_version < IFV_NETWORK_PRIVATE_SETTINGS) {
 		IniGroup *network = generic_ini.GetGroup("network", false);
 		if (network != nullptr) {

src/settings_gui.cpp

@@ -58,6 +58,15 @@ static const StringID _autosave_dropdown[] = {
 	INVALID_STRING_ID,
 };
 
+/** Available settings for autosave intervals. */
+static const std::chrono::minutes _autosave_dropdown_to_minutes[] = {
+	std::chrono::minutes::zero(), ///< never
+	std::chrono::minutes(10),
+	std::chrono::minutes(30),
+	std::chrono::minutes(60),
+	std::chrono::minutes(120),
+};
+
 static Dimension _circle_size; ///< Dimension of the circle +/- icon. This is here as not all users are within the class of the settings window.
 
 static const void *ResolveObject(const GameSettings *settings_ptr, const IntSettingDesc *sd);
@@ -220,7 +229,13 @@ struct GameOptionsWindow : Window {
 			}
 
 			case WID_GO_AUTOSAVE_DROPDOWN: { // Setup autosave dropdown
-				*selected_index = _settings_client.gui.autosave;
+				int index = 0;
+				for (auto &minutes : _autosave_dropdown_to_minutes) {
+					index++;
+					if (_settings_client.gui.autosave_interval <= minutes) break;
+				}
+				*selected_index = index - 1;
+
 				const StringID *items = _autosave_dropdown;
 				for (uint i = 0; *items != INVALID_STRING_ID; items++, i++) {
 					list.emplace_back(new DropDownListStringItem(*items, i, false));
@@ -301,7 +316,15 @@ struct GameOptionsWindow : Window {
 				}
 				break;
 			}
-			case WID_GO_AUTOSAVE_DROPDOWN:     SetDParam(0, _autosave_dropdown[_settings_client.gui.autosave]); break;
+			case WID_GO_AUTOSAVE_DROPDOWN: {
+				int index = 0;
+				for (auto &minutes : _autosave_dropdown_to_minutes) {
+					index++;
+					if (_settings_client.gui.autosave_interval <= minutes) break;
+				}
+				SetDParam(0, _autosave_dropdown[index - 1]);
+				break;
+			}
 			case WID_GO_LANG_DROPDOWN:         SetDParamStr(0, _current_language->own_name); break;
 			case WID_GO_BASE_GRF_DROPDOWN:     SetDParamStr(0, BaseGraphics::GetUsedSet()->name); break;
 			case WID_GO_BASE_GRF_STATUS:       SetDParam(0, BaseGraphics::GetUsedSet()->GetNumInvalid()); break;
@@ -655,7 +678,7 @@ struct GameOptionsWindow : Window {
 				break;
 
 			case WID_GO_AUTOSAVE_DROPDOWN: // Autosave options
-				_settings_client.gui.autosave = index;
+				_settings_client.gui.autosave_interval = _autosave_dropdown_to_minutes[index];
 				ChangeAutosaveFrequency(false);
 				this->SetDirty();
 				break;
@@ -1791,7 +1814,7 @@ static SettingsContainer &GetSettingsTree()
 			}
 
 			interface->Add(new SettingEntry("gui.fast_forward_speed_limit"));
-			interface->Add(new SettingEntry("gui.autosave"));
+			interface->Add(new SettingEntry("gui.autosave_interval"));
 			interface->Add(new SettingEntry("gui.toolbar_pos"));
 			interface->Add(new SettingEntry("gui.statusbar_pos"));
 			interface->Add(new SettingEntry("gui.prefer_teamchat"));

src/settings_internal.h

@@ -196,6 +196,8 @@ struct BoolSettingDesc : IntSettingDesc {
 		IntSettingDesc(save, flags, startup, def, 0, 1, 0, str, str_help, str_val, cat,
 			pre_check, post_callback) {}
 
+	static std::optional<bool> ParseSingleValue(const char *str);
+
 	bool IsBoolSetting() const override { return true; }
 	size_t ParseValue(const char *str) const override;
 	std::string FormatValue(const void *object) const override;

src/settings_type.h

@@ -142,7 +142,7 @@ struct GUISettings {
 	ZoomLevel zoom_min;                      ///< minimum zoom out level
 	ZoomLevel zoom_max;                      ///< maximum zoom out level
 	ZoomLevel sprite_zoom_min;               ///< maximum zoom level at which higher-resolution alternative sprites will be used (if available) instead of scaling a lower resolution sprite
-	byte   autosave;                         ///< how often should we do autosaves?
+	std::chrono::minutes autosave_interval;  ///< how often should we do autosaves?
 	bool   threaded_saves;                   ///< should we do threaded saves?
 	bool   keep_all_autosave;                ///< name the autosave in a different way
 	bool   autosave_on_exit;                 ///< save an autosave when you quit the game, but do not ask "Do you really want to quit?"

src/table/settings/gui_settings.ini

@@ -16,7 +16,6 @@ static void InvalidateNewGRFChangeWindows(int32 new_value);
 static void ZoomMinMaxChanged(int32 new_value);
 static void SpriteZoomMinChanged(int32 new_value);
 
-static constexpr std::initializer_list<const char*> _autosave_interval{"off", "monthly", "quarterly", "half year", "yearly"};
 static constexpr std::initializer_list<const char*> _osk_activation{"disabled", "double", "single", "immediately"};
 static constexpr std::initializer_list<const char*> _savegame_date{"long", "short", "iso"};
 
@@ -48,16 +47,15 @@ extra    = 0
 startup  = false
 
 
-[SDTC_OMANY]
-var      = gui.autosave
-type     = SLE_UINT8
-flags    = SF_NOT_IN_SAVE | SF_NO_NETWORK_SYNC | SF_GUI_DROPDOWN
-def      = 1
-max      = 4
-full     = _autosave_interval
+[SDTC_VAR]
+var      = gui.autosave_interval
+type     = SLE_UINT32
+flags    = SF_NOT_IN_SAVE | SF_NO_NETWORK_SYNC
+def      = 10
+min      = 0
+max      = 1440
 str      = STR_CONFIG_SETTING_AUTOSAVE
 strhelp  = STR_CONFIG_SETTING_AUTOSAVE_HELPTEXT
-strval   = STR_GAME_OPTIONS_AUTOSAVE_DROPDOWN_OFF
 cat      = SC_BASIC
 
 [SDTC_BOOL]

src/table/settings/old_gameopt_settings.ini

@@ -142,15 +142,6 @@ min      = MIN_SNOWLINE_HEIGHT * TILE_HEIGHT
 max      = UINT8_MAX
 to       = SLV_22
 
-[SDTC_OMANY]
-var      = gui.autosave
-type     = SLE_UINT8
-flags    = SF_NOT_IN_SAVE | SF_NO_NETWORK_SYNC
-def      = 1
-max      = 4
-full     = _autosave_interval
-cat      = SC_BASIC
-
 [SDT_OMANY]
 var      = vehicle.road_side
 type     = SLE_UINT8