Codechange: refactor setting conversions and make sure it only happens once

By forcing it only happens once, the first time you upgrade to
a newer client, means you are free to jump between older and newer
versions after that. As they will not resync, the newer setting
can take on any of the (newer) values, without breaking the old
client. And when going to an old client and back, it doesn't
change it back to the converted value anymore.

src/settings.cpp

@@ -1210,6 +1210,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.
@@ -1233,25 +1280,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) {
-						auto old_value = BoolSettingDesc::ParseSingleValue(server_advertise->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_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) {
@@ -1277,6 +1311,13 @@ 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;
+		}
+
 		_grfconfig_newgame = GRFLoadConfig(generic_ini, "newgrf", false);
 		_grfconfig_static  = GRFLoadConfig(generic_ini, "newgrf-static", true);
 		AILoadConfig(generic_ini, "ai_players");
@@ -1331,13 +1372,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) {