Codechange: use setting name instead of index for CmdChange(Company)Setting (#9306)

This is mostly done as there are now constraints on settings.ini you might not
expected. For example, conditional settings always have to come last, as otherwise
they would influence the index.

8 files changed, 87 insertions, 135 deletions

diff --git a/src/autoreplace_gui.cpp b/src/autoreplace_gui.cpp
index a7bd071db..9f2deaf10 100644
--- a/src/autoreplace_gui.cpp
+++ b/src/autoreplace_gui.cpp
@@ -544,7 +544,7 @@ public:
 					DoCommandP(0, this->sel_group | (GroupFlags::GF_REPLACE_WAGON_REMOVAL << 16), (HasBit(g->flags, GroupFlags::GF_REPLACE_WAGON_REMOVAL) ? 0 : 1) | (_ctrl_pressed << 1), CMD_SET_GROUP_FLAG);
 				} else {
 					// toggle renew_keep_length
-					DoCommandP(0, GetCompanySettingIndex("company.renew_keep_length"), Company::Get(_local_company)->settings.renew_keep_length ? 0 : 1, CMD_CHANGE_COMPANY_SETTING);
+					DoCommandP(0, 0, Company::Get(_local_company)->settings.renew_keep_length ? 0 : 1, CMD_CHANGE_COMPANY_SETTING, nullptr, "company.renew_keep_length");
 				}
 				break;
 			}
diff --git a/src/script/api/script_company.cpp b/src/script/api/script_company.cpp
index 20f1b3f7a..9f5722c10 100644
--- a/src/script/api/script_company.cpp
+++ b/src/script/api/script_company.cpp
@@ -260,7 +260,7 @@
 
 /* static */ bool ScriptCompany::SetAutoRenewStatus(bool autorenew)
 {
-	return ScriptObject::DoCommand(0, ::GetCompanySettingIndex("company.engine_renew"), autorenew ? 1 : 0, CMD_CHANGE_COMPANY_SETTING);
+	return ScriptObject::DoCommand(0, 0, autorenew ? 1 : 0, CMD_CHANGE_COMPANY_SETTING, "company.engine_renew");
 }
 
 /* static */ bool ScriptCompany::GetAutoRenewStatus(CompanyID company)
@@ -273,7 +273,7 @@
 
 /* static */ bool ScriptCompany::SetAutoRenewMonths(int16 months)
 {
-	return ScriptObject::DoCommand(0, ::GetCompanySettingIndex("company.engine_renew_months"), months, CMD_CHANGE_COMPANY_SETTING);
+	return ScriptObject::DoCommand(0, 0, months, CMD_CHANGE_COMPANY_SETTING, "company.engine_renew_months");
 }
 
 /* static */ int16 ScriptCompany::GetAutoRenewMonths(CompanyID company)
@@ -288,7 +288,7 @@
 {
 	EnforcePrecondition(false, money >= 0);
 	EnforcePrecondition(false, (int64)money <= UINT32_MAX);
-	return ScriptObject::DoCommand(0, ::GetCompanySettingIndex("company.engine_renew_money"), money, CMD_CHANGE_COMPANY_SETTING);
+	return ScriptObject::DoCommand(0, 0, money, CMD_CHANGE_COMPANY_SETTING, "company.engine_renew_money");
 }
 
 /* static */ Money ScriptCompany::GetAutoRenewMoney(CompanyID company)
diff --git a/src/script/api/script_gamesettings.cpp b/src/script/api/script_gamesettings.cpp
index 8b0459367..0ade2d7cc 100644
--- a/src/script/api/script_gamesettings.cpp
+++ b/src/script/api/script_gamesettings.cpp
@@ -39,7 +39,7 @@
 
 	if ((sd->save.conv & SLF_NO_NETWORK_SYNC) != 0) return false;
 
-	return ScriptObject::DoCommand(0, GetSettingIndex(sd), value, CMD_CHANGE_SETTING);
+	return ScriptObject::DoCommand(0, 0, value, CMD_CHANGE_SETTING, sd->name);
 }
 
 /* static */ bool ScriptGameSettings::IsDisabledVehicleType(ScriptVehicle::VehicleType vehicle_type)
diff --git a/src/script/api/script_group.cpp b/src/script/api/script_group.cpp
index 9cf6c7c95..0295c67aa 100644
--- a/src/script/api/script_group.cpp
+++ b/src/script/api/script_group.cpp
@@ -118,7 +118,7 @@
 {
 	if (HasWagonRemoval() == enable_removal) return true;
 
-	return ScriptObject::DoCommand(0, ::GetCompanySettingIndex("company.renew_keep_length"), enable_removal ? 1 : 0, CMD_CHANGE_COMPANY_SETTING);
+	return ScriptObject::DoCommand(0, 0, enable_removal ? 1 : 0, CMD_CHANGE_COMPANY_SETTING, "company.renew_keep_length");
 }
 
 /* static */ bool ScriptGroup::HasWagonRemoval()
diff --git a/src/settings.cpp b/src/settings.cpp
index c26b3bb47..56c02e5e1 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -104,17 +104,6 @@ const SettingDesc *GetSettingDescription(uint index)
 }
 
 /**
- * Get the setting at the given index into the company settings table.
- * @param index The index to look for.
- * @return The setting at the given index, or nullptr when the index is invalid.
- */
-static const SettingDesc *GetCompanySettingDescription(uint index)
-{
-	if (index >= _company_settings.size()) return nullptr;
-	return _company_settings.begin()[index].get();
-}
-
-/**
  * Groups in openttd.cfg that are actually lists.
  */
 static const char * const _list_group_names[] = {
@@ -1710,19 +1699,74 @@ void IntSettingDesc::ChangeValue(const void *object, int32 newval) const
 }
 
 /**
+ * Given a name of setting, return a setting description from the table.
+ * @param name Name of the setting to return a setting description of.
+ * @param settings Table to look in for the setting.
+ * @return Pointer to the setting description of setting \a name if it can be found,
+ *         \c nullptr indicates failure to obtain the description.
+ */
+static const SettingDesc *GetSettingFromName(const char *name, const SettingTable &settings)
+{
+	/* First check all full names */
+	for (auto &sd : settings) {
+		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
+		if (strcmp(sd->name, name) == 0) return sd.get();
+	}
+
+	/* Then check the shortcut variant of the name. */
+	for (auto &sd : settings) {
+		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
+		const char *short_name = strchr(sd->name, '.');
+		if (short_name != nullptr) {
+			short_name++;
+			if (strcmp(short_name, name) == 0) return sd.get();
+		}
+	}
+
+	return nullptr;
+}
+
+/**
+ * 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.
+ * @return Pointer to the setting description of setting \a name if it can be found,
+ *         \c nullptr indicates failure to obtain the description.
+ */
+static const SettingDesc *GetCompanySettingFromName(const char *name)
+{
+	if (strncmp(name, "company.", 8) == 0) name += 8;
+	return GetSettingFromName(name, _company_settings);
+}
+
+/**
+ * Given a name of any setting, return any setting description of it.
+ * @param name  Name of the setting to return a setting description of.
+ * @return Pointer to the setting description of setting \a name if it can be found,
+ *         \c nullptr indicates failure to obtain the description.
+ */
+const SettingDesc *GetSettingFromName(const char *name)
+{
+	auto sd = GetSettingFromName(name, _settings);
+	if (sd != nullptr) return sd;
+
+	return GetCompanySettingFromName(name);
+}
+
+/**
  * Network-safe changing of settings (server-only).
  * @param tile unused
  * @param flags operation to perform
- * @param p1 the index of the setting in the SettingDesc array which identifies it
+ * @param p1 unused
  * @param p2 the new value for the setting
  * The new value is properly clamped to its minimum/maximum when setting
- * @param text unused
+ * @param text the name of the setting to change
  * @return the cost of this operation or an error
  * @see _settings
  */
 CommandCost CmdChangeSetting(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const std::string &text)
 {
-	const SettingDesc *sd = GetSettingDescription(p1);
+	if (text.empty()) return CMD_ERROR;
+	const SettingDesc *sd = GetSettingFromName(text.c_str());
 
 	if (sd == nullptr) return CMD_ERROR;
 	if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) return CMD_ERROR;
@@ -1741,15 +1785,17 @@ CommandCost CmdChangeSetting(TileIndex tile, DoCommandFlag flags, uint32 p1, uin
  * Change one of the per-company settings.
  * @param tile unused
  * @param flags operation to perform
- * @param p1 the index of the setting in the _company_settings array which identifies it
+ * @param p1 unused
  * @param p2 the new value for the setting
  * The new value is properly clamped to its minimum/maximum when setting
- * @param text unused
+ * @param text the name of the company setting to change
  * @return the cost of this operation or an error
  */
 CommandCost CmdChangeCompanySetting(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const std::string &text)
 {
-	const SettingDesc *sd = GetCompanySettingDescription(p1);
+	if (text.empty()) return CMD_ERROR;
+	const SettingDesc *sd = GetCompanySettingFromName(text.c_str());
+
 	if (sd == nullptr) return CMD_ERROR;
 	if (!sd->IsIntSetting()) return CMD_ERROR;
 
@@ -1761,44 +1807,6 @@ CommandCost CmdChangeCompanySetting(TileIndex tile, DoCommandFlag flags, uint32
 }
 
 /**
- * Get the index of the given setting in the setting table.
- * @param settings The settings to look through.
- * @param setting The setting to look for.
- * @return The index, or UINT32_MAX when it has not been found.
- */
-static uint GetSettingIndex(const SettingTable &settings, const SettingDesc *setting)
-{
-	uint index = 0;
-	for (auto &sd : settings) {
-		if (sd.get() == setting) return index;
-		index++;
-	}
-	return UINT32_MAX;
-}
-
-/**
- * Get the index of the setting with this description.
- * @param sd the setting to get the index for.
- * @return the index of the setting to be used for CMD_CHANGE_SETTING.
- */
-uint GetSettingIndex(const SettingDesc *sd)
-{
-	assert(sd != nullptr && (sd->flags & SGF_PER_COMPANY) == 0);
-	return GetSettingIndex(_settings, sd);
-}
-
-/**
- * Get the index of the company setting with this description.
- * @param sd the setting to get the index for.
- * @return the index of the setting to be used for CMD_CHANGE_COMPANY_SETTING.
- */
-static uint GetCompanySettingIndex(const SettingDesc *sd)
-{
-	assert(sd != nullptr && (sd->flags & SGF_PER_COMPANY) != 0);
-	return GetSettingIndex(_company_settings, sd);
-}
-
-/**
  * Top function to save the new value of an element of the Settings struct
  * @param index offset in the SettingDesc array of the Settings struct which
  * identifies the setting member we want to change
@@ -1810,7 +1818,7 @@ bool SetSettingValue(const IntSettingDesc *sd, int32 value, bool force_newgame)
 	const IntSettingDesc *setting = sd->AsIntSetting();
 	if ((setting->flags & SGF_PER_COMPANY) != 0) {
 		if (Company::IsValidID(_local_company) && _game_mode != GM_MENU) {
-			return DoCommandP(0, GetCompanySettingIndex(setting), value, CMD_CHANGE_COMPANY_SETTING);
+			return DoCommandP(0, 0, value, CMD_CHANGE_COMPANY_SETTING, nullptr, setting->name);
 		}
 
 		setting->ChangeValue(&_settings_client.company, value);
@@ -1836,7 +1844,7 @@ bool SetSettingValue(const IntSettingDesc *sd, int32 value, bool force_newgame)
 
 	/* send non-company-based settings over the network */
 	if (!_networking || (_networking && _network_server)) {
-		return DoCommandP(0, GetSettingIndex(setting), value, CMD_CHANGE_SETTING);
+		return DoCommandP(0, 0, value, CMD_CHANGE_SETTING, nullptr, setting->name);
 	}
 	return false;
 }
@@ -1860,26 +1868,14 @@ void SyncCompanySettings()
 {
 	const void *old_object = &Company::Get(_current_company)->settings;
 	const void *new_object = &_settings_client.company;
-	uint i = 0;
 	for (auto &sd : _company_settings) {
 		uint32 old_value = (uint32)sd->AsIntSetting()->Read(new_object);
 		uint32 new_value = (uint32)sd->AsIntSetting()->Read(old_object);
-		if (old_value != new_value) NetworkSendCommand(0, i, new_value, CMD_CHANGE_COMPANY_SETTING, nullptr, {}, _local_company);
-		i++;
+		if (old_value != new_value) NetworkSendCommand(0, 0, new_value, CMD_CHANGE_COMPANY_SETTING, nullptr, sd->name, _local_company);
 	}
 }
 
 /**
- * Get the index in the _company_settings array of a setting
- * @param name The name of the setting
- * @return The index in the _company_settings array
- */
-uint GetCompanySettingIndex(const char *name)
-{
-	return GetCompanySettingIndex(GetSettingFromName(name));
-}
-
-/**
  * Set a setting value with a string.
  * @param sd the setting to change.
  * @param value the value to write
@@ -1916,47 +1912,11 @@ void StringSettingDesc::ChangeValue(const void *object, std::string &newval) con
 	if (_save_config) SaveToConfig();
 }
 
-/**
- * Given a name of setting, return a setting description of it.
- * @param name  Name of the setting to return a setting description of
- * @param i     Pointer to an integer that will contain the index of the setting after the call, if it is successful.
- * @return Pointer to the setting description of setting \a name if it can be found,
- *         \c nullptr indicates failure to obtain the description
- */
-const SettingDesc *GetSettingFromName(const char *name)
-{
-	/* First check all full names */
-	for (auto &sd : _settings) {
-		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
-		if (strcmp(sd->name, name) == 0) return sd.get();
-	}
-
-	/* Then check the shortcut variant of the name. */
-	for (auto &sd : _settings) {
-		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
-		const char *short_name = strchr(sd->name, '.');
-		if (short_name != nullptr) {
-			short_name++;
-			if (strcmp(short_name, name) == 0) return sd.get();
-		}
-	}
-
-	if (strncmp(name, "company.", 8) == 0) name += 8;
-	/* And finally the company-based settings */
-	for (auto &sd : _company_settings) {
-		if (!SlIsObjectCurrentlyValid(sd->save.version_from, sd->save.version_to)) continue;
-		if (strcmp(sd->name, name) == 0) return sd.get();
-	}
-
-	return nullptr;
-}
-
 /* Those 2 functions need to be here, else we have to make some stuff non-static
  * and besides, it is also better to keep stuff like this at the same place */
 void IConsoleSetSetting(const char *name, const char *value, bool force_newgame)
 {
 	const SettingDesc *sd = GetSettingFromName(name);
-
 	if (sd == nullptr) {
 		IConsolePrintF(CC_WARNING, "'%s' is an unknown setting.", name);
 		return;
diff --git a/src/settings_func.h b/src/settings_func.h
index 4a55821cf..d1ecdbac1 100644
--- a/src/settings_func.h
+++ b/src/settings_func.h
@@ -32,7 +32,6 @@ struct GRFConfig *LoadGRFPresetFromConfig(const char *config_name);
 void SaveGRFPresetToConfig(const char *config_name, struct GRFConfig *config);
 void DeleteGRFPresetFromConfig(const char *config_name);
 
-uint GetCompanySettingIndex(const char *name);
 void SetDefaultCompanySettings(CompanyID cid);
 
 void SyncCompanySettings();
diff --git a/src/settings_internal.h b/src/settings_internal.h
index 4de017837..ba5331288 100644
--- a/src/settings_internal.h
+++ b/src/settings_internal.h
@@ -302,6 +302,5 @@ typedef std::initializer_list<std::unique_ptr<const SettingDesc>> SettingTable;
 const SettingDesc *GetSettingFromName(const char *name);
 bool SetSettingValue(const IntSettingDesc *sd, int32 value, bool force_newgame = false);
 bool SetSettingValue(const StringSettingDesc *sd, const std::string value, bool force_newgame = false);
-uint GetSettingIndex(const SettingDesc *sd);
 
 #endif /* SETTINGS_INTERNAL_H */
diff --git a/src/table/settings/settings.ini b/src/table/settings/settings.ini
index afa6db52f..4f9ea607a 100644
--- a/src/table/settings/settings.ini
+++ b/src/table/settings/settings.ini
@@ -2533,6 +2533,21 @@ str      = STR_CONFIG_SETTING_RIGHT_MOUSE_WND_CLOSE
 strhelp  = STR_CONFIG_SETTING_RIGHT_MOUSE_WND_CLOSE_HELPTEXT
 cat      = SC_BASIC
 
+; We might need to emulate a right mouse button on mac
+[SDTC_VAR]
+ifdef    = __APPLE__
+var      = gui.right_mouse_btn_emulation
+type     = SLE_UINT8
+flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
+guiflags = SGF_MULTISTRING
+def      = 0
+min      = 0
+max      = 2
+str      = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU
+strhelp  = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU_HELPTEXT
+strval   = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU_COMMAND
+cat      = SC_BASIC
+
 [SDTC_BOOL]
 var      = gui.measure_tooltip
 flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
@@ -3873,24 +3888,3 @@ var      = network.no_http_content_downloads
 flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
 def      = false
 cat      = SC_EXPERT
-
-; Since the network code (CmdChangeSetting and friends) use the index in this array to decide
-; which setting the server is talking about all conditional compilation of this array must be at the
-; end. This isn't really the best solution, the settings the server can tell the client about should
-; either use a separate array or some other form of identifier.
-
-;
-; We might need to emulate a right mouse button on mac
-[SDTC_VAR]
-ifdef    = __APPLE__
-var      = gui.right_mouse_btn_emulation
-type     = SLE_UINT8
-flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
-guiflags = SGF_MULTISTRING
-def      = 0
-min      = 0
-max      = 2
-str      = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU
-strhelp  = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU_HELPTEXT
-strval   = STR_CONFIG_SETTING_RIGHT_MOUSE_BTN_EMU_COMMAND
-cat      = SC_BASIC