cpp/openttd-patchpack/source Changeset - r25566:1f8274202421

Changeset - r25566:1f8274202421

Parent rev.

Child rev.

[Not reviewed]

master

0 8 0

Patric Stout - 3 years ago 2021-05-29 21:27:01
truebrain@openttd.org

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 with 87 insertions and 135 deletions:

src/autoreplace_gui.cpp

src/script/api/script_company.cpp

src/script/api/script_gamesettings.cpp

src/script/api/script_group.cpp

src/settings.cpp

106

src/settings_func.h

src/settings_internal.h

src/table/settings/settings.ini

0 comments (0 inline, 0 general)

src/autoreplace_gui.cpp

➞

Show inline comments

@@ @@ -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;
+			}

src/script/api/script_company.cpp

➞

Show inline comments

@@ @@ -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)

src/script/api/script_gamesettings.cpp

➞

Show inline comments

@@ @@ -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)

src/script/api/script_group.cpp

➞

Show inline comments

@@ @@ -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()

src/settings.cpp

➞

Show inline comments

@@ @@ -104,17 +104,6 @@ const SettingDesc *GetSettingDescription @@
+}
 /**
  * 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 v @@
+}
 /**
  * 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 t @@
  * 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(Tile @@
+}
 /**
  * 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 IntSettingDes @@
 	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 IntSettingDes @@
 	/* 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(cons @@
 	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;

src/settings_func.h

➞

Show inline comments

@@ @@ -32,7 +32,6 @@ struct GRFConfig *LoadGRFPresetFromConfi @@
 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();

src/settings_internal.h

➞

Show inline comments

@@ @@ -302,6 +302,5 @@ typedef std::initializer_list<std::uniqu @@
 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 */

src/table/settings/settings.ini

➞

Show inline comments

@@ @@ -2533,6 +2533,21 @@ str = STR_CONFIG_SETTING_RIGHT_MOUS @@
 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_downl @@
 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

0 comments (0 inline, 0 general)