Refactor: CC_preset (#7809)

* CC_preset: Prevent post-construction type modification and handle pst duplication

* remove redundant function

* CC_preset: remove default constructor, require explicit type

* remove duplicate

code/controlconfig/controlsconfig.cpp

@@ -1322,7 +1322,7 @@ bool control_config_accept(bool API_Access)
 
 			// Check if a hardcoded preset with name already exists. If so, complain to user and force retry
 			auto it = std::find_if(Control_config_presets.begin(), Control_config_presets.end(), [str](CC_preset& p) {
-				return ((p.name == str) && ((p.type == Preset_t::tbl) || (p.type == Preset_t::hardcode)));
+				return ((p.name == str) && ((p.get_type() == Preset_t::tbl) || (p.get_type() == Preset_t::hardcode)));
 			});
 
 			if (it != Control_config_presets.end()) {
@@ -1353,15 +1353,14 @@ bool control_config_accept(bool API_Access)
 			}
 
 			// Pack the current bindings into a preset, then save the file
-			CC_preset preset;
-			preset.name = std::move(str);
+			CC_preset preset(std::move(str), Preset_t::pst);
 			std::copy(Control_config.begin(), Control_config.end(), std::back_inserter(preset.bindings));
 			Control_config_presets.push_back(preset);
 			save_preset_file(preset, true);
 
 			// Reload the presets from file. Do this instead of just pushing the preset to the vector direct to get
 			// consistant ordering
-			Control_config_presets.resize(1);
+			Control_config_presets.erase(Control_config_presets.begin() + 1, Control_config_presets.end());
 			load_preset_files();
 
 			// finally, save the new preset so that changes will get saved to this preset
@@ -1778,7 +1777,7 @@ bool control_config_create_new_preset(const SCP_string& newName, bool overwrite)
 
 	// Check if a hardcoded preset with name already exists. If so, complain to user and force retry
 	auto it = std::find_if(Control_config_presets.begin(), Control_config_presets.end(), [newName](CC_preset& p) {
-		return (p.name == newName) && ((p.type == Preset_t::tbl) || (p.type == Preset_t::hardcode));
+		return (p.name == newName) && ((p.get_type() == Preset_t::tbl) || (p.get_type() == Preset_t::hardcode));
 	});
 
 	if (it != Control_config_presets.end()) {
@@ -1791,9 +1790,7 @@ bool control_config_create_new_preset(const SCP_string& newName, bool overwrite)
 	}
 
 	// Pack the current bindings into a preset, then save the file
-	CC_preset preset;
-	preset.name = newName;
-	preset.type = Preset_t::pst;
+	CC_preset preset(newName, Preset_t::pst);
 	std::copy(Control_config.begin(), Control_config.end(), std::back_inserter(preset.bindings));
 
 	// Done with the file
@@ -1805,7 +1802,7 @@ bool control_config_create_new_preset(const SCP_string& newName, bool overwrite)
 		save_preset_file(preset, true);
 
 		// Reload the presets from file.
-		Control_config_presets.resize(1);
+		Control_config_presets.erase(Control_config_presets.begin() + 1, Control_config_presets.end());
 		load_preset_files(newName);
 
 		// use the newly created preset
@@ -1813,10 +1810,9 @@ bool control_config_create_new_preset(const SCP_string& newName, bool overwrite)
 
 		return true;
 
-	} else if ((clone->name != preset.name) || (clone->type != Preset_t::pst)) {
+	} else if ((clone->name != preset.name) || (clone->get_type() != Preset_t::pst)) {
 		// Complain and ignore if the preset names or the type differs
 		return false;
-
 	}
 
 	return false; //should be unreachable, but just in case
@@ -1826,7 +1822,7 @@ bool control_config_clone_preset(const CC_preset& preset, const SCP_string& newN
 
 	// Check if a hardcoded preset with name already exists. If so, complain to user and force retry
 	auto it = std::find_if(Control_config_presets.begin(), Control_config_presets.end(), [newName](CC_preset& p) {
-		return (p.name == newName) && ((p.type == Preset_t::tbl) || (p.type == Preset_t::hardcode));
+		return (p.name == newName) && ((p.get_type() == Preset_t::tbl) || (p.get_type() == Preset_t::hardcode));
 	});
 
 	if (it != Control_config_presets.end()) {
@@ -1838,13 +1834,13 @@ bool control_config_clone_preset(const CC_preset& preset, const SCP_string& newN
 		return false;
 	}
 
-	CC_preset newPreset = preset;
-	newPreset.name = newName;
+	CC_preset newPreset(newName, Preset_t::pst);
+	newPreset.bindings = preset.bindings;
 
 	bool success = save_preset_file(newPreset, false);
 
 	// Reload the presets from file.
-	Control_config_presets.resize(1);
+	Control_config_presets.erase(Control_config_presets.begin() + 1, Control_config_presets.end());
 	load_preset_files(newName);
 
 	// use the newly cloned preset

code/controlconfig/controlsconfig.h

@@ -572,20 +572,28 @@ public:
  * A preset, a collection of bindings for use in Control_config with an associated name
  */
 class CC_preset {
-public:
+  public:
 	SCP_vector<CCB> bindings;
 	SCP_string name;
-	Preset_t type = Preset_t::hardcode;
 
-public:
-	CC_preset() = default;
-	CC_preset(const CC_preset& A) = default;
+  private:
+	// No default value: the type is always set explicitly by the constructor
+	Preset_t type;
 
-	CC_preset& operator=(const CC_preset&);
-	/*!
-	 * Checks if the given preset is a duplicate of this one
-	 */
-	bool is_duplicate_of(CC_preset&);
+  public:
+	// No default constructor: every preset must be created with an explicit type
+	CC_preset(SCP_string preset_name, Preset_t preset_type) : name(std::move(preset_name)), type(preset_type) {}
+	CC_preset(const CC_preset&) = default;
+
+	// Preserve target's preset type during copy assignment
+	CC_preset& operator=(const CC_preset& other);
+
+	Preset_t get_type() const
+	{
+		return type;
+	}
+
+	bool is_duplicate_of(const CC_preset& other) const;
 };
 
 /*!

code/controlconfig/controlsconfigcommon.cpp

@@ -292,10 +292,8 @@ void control_config_common_init_bindings() {
 	// init default preset
 	Control_config_presets.clear();
 
-	CC_preset preset;
+	CC_preset preset("default", Preset_t::hardcode);
 	preset.bindings.reserve(Control_config.size());
-	preset.name = "default";
-	preset.type = Preset_t::hardcode;
 
 	for (auto &item : Control_config) {
 		preset.bindings.push_back(CCB(item));
@@ -1733,14 +1731,13 @@ size_t read_bind_1(CC_preset &preset) {
  */
 void control_config_common_read_section(int s, bool first_override) {
 	Assertion((s == 0) || (s == 1), "Expected value of s to be either 0 or 1, found %i instead.", s);
-	CC_preset new_preset;
+	CC_preset new_preset("", Preset_t::tbl);
 
 	// Set references to the default preset and bindings
 	auto& default_preset = Control_config_presets[0];
 	auto& default_bindings = default_preset.bindings;
 
 	new_preset.bindings.clear();
-	new_preset.type = Preset_t::tbl;
 
 	if (s == 0) {
 		// #ControlConfigOverride
@@ -3002,23 +2999,22 @@ CCI_builder& CCI_builder::operator()(IoActionId action_id, short primary, short
 	return *this;
 }
 
-CC_preset& CC_preset::operator=(const CC_preset& A) {
-	name = A.name;
-	type = A.type;
-
-	std::copy(A.bindings.begin(), A.bindings.end(), bindings.begin());
-
+CC_preset& CC_preset::operator=(const CC_preset& other)
+{
+	if (this != &other) {
+		bindings = other.bindings;
+		name = other.name;
+		// 'type' is intentionally preserved to maintain instance identity
+	}
 	return *this;
-};
+}
 
-bool CC_preset::is_duplicate_of(CC_preset& A) {
-	for (size_t i = 0; i < A.bindings.size(); ++i) {
-		if (bindings[i] != A.bindings[i]) {
-			// Found a binding that's different.  Thus, this preset is not a duplicate
+bool CC_preset::is_duplicate_of(const CC_preset& other) const
+{
+	for (size_t i = 0; i < other.bindings.size(); ++i) {
+		if (bindings[i] != other.bindings[i]) {
 			return false;
 		}
 	}
-
-	// Else, did not find any differences in the bindings.  Thus, this preset is a duplicate
 	return true;
 }

code/controlconfig/presets.cpp

@@ -74,9 +74,7 @@ void load_preset_files(SCP_string clone) {
 
 
 		// Start reading in data
-		CC_preset preset;
-		preset.name = file;
-		preset.type = Preset_t::pst;
+		CC_preset preset(file, Preset_t::pst);
 		preset.bindings.resize(Control_config.size());
 
 		size_t size;
@@ -134,7 +132,7 @@ void load_preset_files(SCP_string clone) {
 		if ((clone == preset.name) || (it == Control_config_presets.end())) {
 			Control_config_presets.push_back(preset);
 
-		} else if ((it->name != preset.name) || (it->type != Preset_t::pst)) {
+		} else if ((it->name != preset.name) || ((it->get_type() != Preset_t::pst))) {
 			if (gameseq_get_state() == GS_STATE_CONTROL_CONFIG) {
 				popup(PF_TITLE_WHITE | PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, "Preset '%s' is a duplicate of an existing preset, ignoring", preset.name.c_str());
 			} else {
@@ -173,7 +171,7 @@ bool delete_preset_file(CC_preset preset) {
 	cf_delete(filename.c_str(), CF_TYPE_PLAYER_BINDS, CF_LOCATION_ROOT_USER | CF_LOCATION_ROOT_GAME | CF_LOCATION_TYPE_ROOT);
 
 	// Reload the presets from file.
-	Control_config_presets.resize(1);
+	Control_config_presets.erase(Control_config_presets.begin() + 1, Control_config_presets.end());
 	load_preset_files();
 
 	return true;

code/pilotfile/csg.cpp

@@ -1369,12 +1369,13 @@ void pilotfile::csg_read_controls()
 		auto it = control_config_get_current_preset(true);
 		if (it == Control_config_presets.end()) {
 			// Not a preset, create one and its file
-			CC_preset preset;
-			preset.name = filename;
+			SCP_string preset_name = filename;
 
 			// strip off extension
-			auto n = preset.name.find_last_of('.');
-			preset.name.resize(n);
+			auto n = preset_name.find_last_of('.');
+			preset_name.resize(n);
+
+			CC_preset preset(std::move(preset_name), Preset_t::pst);
 
 			std::copy(Control_config.begin(), Control_config.end(), std::back_inserter(preset.bindings));
 			Control_config_presets.push_back(preset);

code/pilotfile/plr.cpp

@@ -738,13 +738,15 @@ void pilotfile::plr_read_controls()
 		int axi, inv;
 
 		// Set up preset name, we'll populate the rest of the preset's data later
-		CC_preset preset;
-		preset.name = filename;
+		SCP_string preset_name = filename;
 
 		// strip off extension
-		auto n = preset.name.find_last_of('.');
-		preset.name.resize(n);
-		
+		auto n = preset_name.find_last_of('.');
+		preset_name.resize(n);
+
+		// Old pilot files only carry bindings, so this is always a user preset
+		CC_preset preset(std::move(preset_name), Preset_t::pst);
+
 		// Load in the bindings to Control_config
 		// ...First the digital controls
 		auto list_size = handler->startArrayRead("controls", true);