Fixes issues specifying image/slot options fron INI files (reported by Robbbert) (#2231)

This fix really doesn't go far enough.  I added hooks so that options specified at the command line can also be responded to when parsed from INI files, but in the long run much of the logic that is currently in mame_options should go into emu_options so that when an option is specified, all of the wacko logic around slot/image specification "just works" because it is encapsulated within emu_options.

We have a release 11 days away; I want to be in stabilization mode.

src/emu/emuopts.cpp

@@ -2,7 +2,7 @@
 // copyright-holders:Aaron Giles
 /***************************************************************************
 
-    emuopts.c
+    emuopts.cpp
 
     Options file and command line management.
 
@@ -11,6 +11,7 @@
 #include "emu.h"
 #include "emuopts.h"
 
+
 //**************************************************************************
 //  CORE EMULATOR OPTIONS
 //**************************************************************************
@@ -263,3 +264,73 @@ void emu_options::value_changed(const std::string &name, const std::string &valu
 			m_ui = UI_CABINET;
 	}
 }
+
+
+//-------------------------------------------------
+//	override_get_value - when saving to an INI, we
+//	need to hook into that process so we can write
+//	out image/slot options
+//-------------------------------------------------
+
+bool emu_options::override_get_value(const char *name, std::string &value) const
+{
+	auto slotiter = m_slot_options.find(name);
+	if (slotiter != m_slot_options.end() && slotiter->second.is_selectable())
+	{
+		value = slotiter->second.value();
+		return true;
+	}
+
+	auto imageiter = m_image_options.find(name);
+	if (imageiter != m_image_options.end())
+	{
+		value = imageiter->second;
+		return true;
+	}
+
+	return false;
+}
+
+
+//-------------------------------------------------
+//	override_set_value - when parsing an INI, we
+//	need to hook into into it so we can do the same
+//	crazy slot logic done in mameopt
+//-------------------------------------------------
+
+bool emu_options::override_set_value(const char *name, const std::string &value)
+{
+	auto slotiter = m_slot_options.find(name);
+	if (slotiter != m_slot_options.end())
+	{
+		slotiter->second = parse_slot_option(std::string(value), true);
+		return true;
+	}
+
+	auto imageiter = m_image_options.find(name);
+	if (imageiter != m_image_options.end())
+	{
+		imageiter->second = value;
+		return true;
+	}
+
+	return false;
+}
+
+
+//-------------------------------------------------
+//  parse_slot_option - parses a slot option (the
+//	',bios=XYZ' syntax)
+//-------------------------------------------------
+
+slot_option emu_options::parse_slot_option(std::string &&text, bool selectable)
+{
+	slot_option result;
+	const char *bios_arg = ",bios=";
+
+	size_t pos = text.find(bios_arg);
+	return pos != std::string::npos
+		? slot_option(text.substr(0, pos), text.substr(pos + strlen(bios_arg)), selectable)
+		: slot_option(std::move(text), "", selectable);
+}
+

src/emu/emuopts.h

@@ -198,8 +198,13 @@
 class slot_option
 {
 public:
-	slot_option(std::string &&value = "", std::string &&bios = "")
-		: m_value(std::move(value)), m_bios(std::move(bios))
+	slot_option()
+		: slot_option("", "", true)
+	{
+	}
+
+	slot_option(std::string &&value, std::string &&bios, bool selectable)
+		: m_value(std::move(value)), m_bios(std::move(bios)), m_selectable(selectable)
 	{
 	}
 	slot_option(const slot_option &that) = default;
@@ -219,15 +224,18 @@ public:
 	const std::string &bios() const { return m_bios; }
 	const std::string &default_card_software() const { return m_default_card_software; }
 	bool is_default() const { return m_value.empty(); }
+	bool is_selectable() const { return m_selectable; }
 
 	// seters
 	void set_bios(std::string &&s) { m_bios = std::move(s); }
 	void set_default_card_software(std::string &&s) { m_default_card_software = std::move(s); }
+	void set_is_selectable(bool selectable) { m_selectable = selectable; }
 
 private:
 	std::string m_value;
 	std::string m_bios;
 	std::string	m_default_card_software;
+	bool m_selectable;
 };
 
 
@@ -427,8 +435,12 @@ public:
 	std::map<std::string, std::string> &image_options() { return m_image_options; }
 	const std::map<std::string, std::string> &image_options() const { return m_image_options; }
 
+	static slot_option parse_slot_option(std::string &&text, bool selectable);
+
 protected:
 	virtual void value_changed(const std::string &name, const std::string &value) override;
+	virtual bool override_get_value(const char *name, std::string &value) const override;
+	virtual bool override_set_value(const char *name, const std::string &value) override;
 
 private:
 	static const options_entry s_option_entries[];

src/frontend/mame/mameopts.cpp

@@ -57,13 +57,20 @@ bool mame_options::add_slot_options(emu_options &options, value_specifier_func v
 
 			// add the option
 			options.add_entry(name, nullptr, OPTION_STRING | OPTION_FLAG_DEVICE, slot.default_option(), true);
+			options.slot_options()[name] = slot_option();
 
 			// allow opportunity to specify this value
 			if (value_specifier)
 			{
 				std::string value = value_specifier(name);
 				if (value != value_specifier_invalid_value())
-					options.slot_options()[name] = parse_slot_option(std::move(value));
+				{
+					const device_slot_option *option = slot.option(value.c_str());
+					if (option)
+					{
+						options.slot_options()[name] = emu_options::parse_slot_option(std::move(value), option->selectable());
+					}
+				}
 			}
 		}
 	}
@@ -179,6 +186,7 @@ void mame_options::add_device_options(emu_options &options, value_specifier_func
 			// add the option
 			std::string option_name = get_full_option_name(image);
 			options.add_entry(option_name.c_str(), nullptr, OPTION_STRING | OPTION_FLAG_DEVICE, nullptr, true);
+			options.image_options()[image.instance_name()] = "";
 
 			// allow opportunity to specify this value
 			if (value_specifier)
@@ -299,6 +307,13 @@ bool mame_options::reevaluate_slot_options(emu_options &options)
 				if (options.slot_options()[name].default_card_software() != default_card_software)
 				{
 					options.slot_options()[name].set_default_card_software(std::move(default_card_software));
+
+					// the value of this slot option has changed.  we may need to change is_selectable(); this
+					// should really be encapsulated within emu_options
+					const device_slot_option *option = slot.option(options.slot_options()[name].value().c_str());
+					if (option)
+						options.slot_options()[name].set_is_selectable(option->selectable());
+
 					result = true;
 				}
 			}
@@ -688,19 +703,3 @@ bool mame_options::parse_one_ini(emu_options &options, const char *basename, int
 	return result;
 }
 
-
-//-------------------------------------------------
-//  parse_slot_option - parses a slot option (the
-//	',bios=XYZ' syntax)
-//-------------------------------------------------
-
-slot_option mame_options::parse_slot_option(std::string &&text)
-{
-	slot_option result;
-	const char *bios_arg = ",bios=";
-
-	size_t pos = text.find(bios_arg);
-	return pos != std::string::npos
-		? slot_option(text.substr(0, pos), text.substr(pos + strlen(bios_arg)))
-		: slot_option(std::move(text));
-}

src/frontend/mame/mameopts.h

@@ -72,7 +72,6 @@ private:
 	static void update_slot_options(emu_options &options, const software_part *swpart = nullptr);
 	static void parse_slot_devices(emu_options &options, value_specifier_func value_specifier);
 	static std::string get_full_option_name(const device_image_interface &image);
-	static slot_option parse_slot_option(std::string &&text);
 	static std::string get_default_card_software(device_slot_interface &slot, const emu_options &options);
 
 	// INI parsing helper

src/lib/util/options.cpp

@@ -559,12 +559,15 @@ std::string core_options::output_ini(const core_options *diff) const
 	int num_valid_headers = 0;
 	int unadorned_index = 0;
 	const char *last_header = nullptr;
+	std::string overridden_value;
 
 	// loop over all items
 	for (entry &curentry : m_entrylist)
 	{
 		const char *name = curentry.name();
-		const char *value = curentry.value();
+		const char *value = name && override_get_value(name, overridden_value)
+			? overridden_value.c_str()
+			: curentry.value();
 		bool is_unadorned = false;
 
 		// check if it's unadorned
@@ -848,6 +851,10 @@ bool core_options::validate_and_set_data(core_options::entry &curentry, std::str
 		data.erase(data.length() - 1, 1);
 	}
 
+	// let derived classes override how we set this data
+	if (override_set_value(curentry.name(), data))
+		return true;
+
 	// validate the type of data and optionally the range
 	float fval;
 	int ival;

src/lib/util/options.h

@@ -182,6 +182,8 @@ public:
 
 protected:
 	virtual void value_changed(const std::string &name, const std::string &value) {}
+	virtual bool override_get_value(const char *name, std::string &value) const { return false; }
+	virtual bool override_set_value(const char *name, const std::string &value) { return false; }
 
 private:
 	// internal helpers