Codechange: split Write_ValidateSetting to get separate functions for making strings valid and writing strings

3 files changed, 40 insertions, 37 deletions

diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp
index 3c61cf77a..6df77cd08 100644
--- a/src/network/network_gui.cpp
+++ b/src/network/network_gui.cpp
@@ -2208,7 +2208,7 @@ public:
 				std::string client_name(str);
 				if (!NetworkValidateClientName(client_name)) break;
 
-				SetSettingValue(GetSettingFromName("network.client_name")->AsStringSetting(), client_name.c_str());
+				SetSettingValue(GetSettingFromName("network.client_name")->AsStringSetting(), client_name);
 				this->InvalidateData();
 				break;
 			}
diff --git a/src/settings.cpp b/src/settings.cpp
index af06d3bef..26460fbb1 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -460,34 +460,31 @@ int32 IntSettingDesc::Read(const void *object) const
 }
 
 /**
- * Set the string value of a setting.
- * @param object The object the setting is to be saved in.
- * @param str The string to save.
+ * Make the value valid given the limitations of this setting.
+ *
+ * In the case of string settings this is ensuring the string contains only accepted
+ * Utf8 characters and is at most the maximum length defined in this setting.
+ * @param str The string to make valid.
  */
-void StringSettingDesc::Write_ValidateSetting(const void *object, const char *str) const
+void StringSettingDesc::MakeValueValid(std::string &str) const
 {
-	std::string *dst = reinterpret_cast<std::string *>(GetVariableAddress(object, &this->save));
+	if (this->max_length == 0 || str.size() < this->max_length) return;
 
-	switch (GetVarMemType(this->save.conv)) {
-		case SLE_VAR_STR:
-		case SLE_VAR_STRQ:
-			if (str != nullptr) {
-				if (this->max_length != 0 && strlen(str) >= this->max_length) {
-					/* In case a maximum length is imposed by the setting, the length
-					 * includes the '\0' termination for network transfer purposes.
-					 * Also ensure the string is valid after chopping of some bytes. */
-					std::string stdstr(str, this->max_length - 1);
-					dst->assign(str_validate(stdstr, SVS_NONE));
-				} else {
-					dst->assign(str);
-				}
-			} else {
-				dst->clear();
-			}
-			break;
+	/* In case a maximum length is imposed by the setting, the length
+	 * includes the '\0' termination for network transfer purposes.
+	 * Also ensure the string is valid after chopping of some bytes. */
+	std::string stdstr(str, this->max_length - 1);
+	str.assign(str_validate(stdstr, SVS_NONE));
+}
 
-		default: NOT_REACHED();
-	}
+/**
+ * Write a string to the actual setting.
+ * @param object The object the setting is to be saved in.
+ * @param str The string to save.
+ */
+void StringSettingDesc::Write(const void *object, const std::string &str) const
+{
+	reinterpret_cast<std::string *>(GetVariableAddress(object, &this->save))->assign(str);
 }
 
 /**
@@ -553,8 +550,9 @@ void IntSettingDesc::ParseValue(const IniItem *item, void *object) const
 
 void StringSettingDesc::ParseValue(const IniItem *item, void *object) const
 {
-	const char *str = (item == nullptr) ? this->def : item->value.has_value() ? item->value->c_str() : nullptr;
-	this->Write_ValidateSetting(object, str);
+	std::string str = (item == nullptr) ? this->def : item->value.value_or("");
+	this->MakeValueValid(str);
+	this->Write(object, str);
 }
 
 void ListSettingDesc::ParseValue(const IniItem *item, void *object) const
@@ -2026,12 +2024,12 @@ uint GetCompanySettingIndex(const char *name)
  * @param force_newgame force the newgame settings
  * @note Strings WILL NOT be synced over the network
  */
-bool SetSettingValue(const StringSettingDesc *sd, const char *value, bool force_newgame)
+bool SetSettingValue(const StringSettingDesc *sd, std::string value, bool force_newgame)
 {
 	assert(sd->save.conv & SLF_NO_NETWORK_SYNC);
 
-	if (GetVarMemType(sd->save.conv) == SLE_VAR_STRQ && strcmp(value, "(null)") == 0) {
-		value = nullptr;
+	if (GetVarMemType(sd->save.conv) == SLE_VAR_STRQ && value.compare("(null)") == 0) {
+		value.clear();
 	}
 
 	const void *object = (_game_mode == GM_MENU || force_newgame) ? &_settings_newgame : &_settings_game;
@@ -2045,9 +2043,10 @@ bool SetSettingValue(const StringSettingDesc *sd, const char *value, bool force_
  * @param object The object the setting is in.
  * @param newval The new value for the setting.
  */
-void StringSettingDesc::ChangeValue(const void *object, const char *newval) const
+void StringSettingDesc::ChangeValue(const void *object, std::string &newval) const
 {
-	this->Write_ValidateSetting(object, newval);
+	this->MakeValueValid(newval);
+	this->Write(object, newval);
 	if (this->proc != nullptr) this->proc(0);
 
 	if (_save_config) SaveToConfig();
diff --git a/src/settings_internal.h b/src/settings_internal.h
index e7dd6e914..9e6358a6c 100644
--- a/src/settings_internal.h
+++ b/src/settings_internal.h
@@ -210,21 +210,25 @@ struct ManyOfManySettingDesc : OneOfManySettingDesc {
 struct StringSettingDesc : SettingDesc {
 	StringSettingDesc(SaveLoad save, const char *name, SettingGuiFlag flags, bool startup, const char *def,
 			uint32 max_length, OnChange proc) :
-		SettingDesc(save, name, flags, startup), def(def), max_length(max_length), proc(proc) {}
+		SettingDesc(save, name, flags, startup), def(def == nullptr ? "" : def), max_length(max_length),
+			proc(proc) {}
 	virtual ~StringSettingDesc() {}
 
-	const char *def;        ///< default value given when none is present
+	std::string def;        ///< default value given when none is present
 	uint32 max_length;      ///< maximum length of the string, 0 means no maximum length
 	OnChange *proc;         ///< callback procedure for when the value is changed
 
 	bool IsStringSetting() const override { return true; }
-	void ChangeValue(const void *object, const char *newval) const;
-	void Write_ValidateSetting(const void *object, const char *str) const;
+	void ChangeValue(const void *object, std::string &newval) const;
 
 	void FormatValue(char *buf, const char *last, const void *object) const override;
 	void ParseValue(const IniItem *item, void *object) const override;
 	bool IsSameValue(const IniItem *item, void *object) const override;
 	const std::string &Read(const void *object) const;
+
+private:
+	void MakeValueValid(std::string &str) const;
+	void Write(const void *object, const std::string &str) const;
 };
 
 /** List/array settings. */
@@ -255,7 +259,7 @@ 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 char *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 */