Merge pull request #2238 from npwoods/another_option_refactoring

More options refactoring.

src/emu/diimage.h

@@ -250,6 +250,7 @@ public:
 	}
 
 	bool user_loadable() const { return m_user_loadable; }
+	const std::string &full_software_name() const { return m_full_software_name; }
 
 protected:
 	// interface-level overrides

src/emu/emuopts.cpp

@@ -272,23 +272,28 @@ void emu_options::value_changed(const std::string &name, const std::string &valu
 //	out image/slot options
 //-------------------------------------------------
 
-bool emu_options::override_get_value(const char *name, std::string &value) const
+core_options::override_get_value_result emu_options::override_get_value(const char *name, std::string &value) const
+{
+	if (name)
 	{
 		auto slotiter = m_slot_options.find(name);
-	if (slotiter != m_slot_options.end() && slotiter->second.is_selectable())
+		if (slotiter != m_slot_options.end())
 		{
-		value = slotiter->second.value();
+			value = slotiter->second.specified_value();
-		return true;
+			return slotiter->second.specified()
+				? override_get_value_result::OVERRIDE
+				: override_get_value_result::SKIP;
 		}
 
 		auto imageiter = m_image_options.find(name);
 		if (imageiter != m_image_options.end())
 		{
 			value = imageiter->second;
-		return true;
+			return override_get_value_result::OVERRIDE;
+		}
 	}
 
-	return false;
+	return override_get_value_result::NONE;
 }
 
 
@@ -303,7 +308,7 @@ 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);
+		slotiter->second.specify(std::string(value));
 		return true;
 	}
 
@@ -319,18 +324,97 @@ bool emu_options::override_set_value(const char *name, const std::string &value)
 
 
 //-------------------------------------------------
-//  parse_slot_option - parses a slot option (the
+//	slot_option ctor
-//	',bios=XYZ' syntax)
 //-------------------------------------------------
 
-slot_option emu_options::parse_slot_option(std::string &&text, bool selectable)
+slot_option::slot_option(const char *default_value)
+	: m_specified(false)
+	, m_default_value(default_value ? default_value : "")
 {
-	slot_option result;
+}
+
+
+//-------------------------------------------------
+//	slot_option::value
+//-------------------------------------------------
+
+const std::string &slot_option::value() const
+{
+	// There are a number of ways that the value can be determined; there
+	// is a specific order of precedence:
+	//
+	//	1.  Highest priority is whatever may have been specified by the user (whether it
+	//		was specified at the command line, an INI file, or in the UI).  We keep track
+	//		of whether these values were specified this way
+	//
+	//		Take note that slots have a notion of being "selectable".  Slots that are not
+	//		marked as selectable cannot be specified with this technique
+	//
+	//	2.  Next highest is what is returned from get_default_card_software()
+	//
+	//	3.	Last in priority is what was specified as the slot default.  This comes from
+	//		device setup
+	if (m_specified)
+		return m_specified_value;
+	else if (!m_default_card_software.empty())
+		return m_default_card_software;
+	else
+		return m_default_value;
+}
+
+
+//-------------------------------------------------
+//	slot_option::specified_value
+//-------------------------------------------------
+
+std::string slot_option::specified_value() const
+{
+	std::string result;
+	if (m_specified)
+	{
+		result = m_specified_bios.empty()
+			? m_specified_value
+			: util::string_format("%s,bios=%s", m_specified_value, m_specified_bios);
+	}
+	return result;
+}
+
+
+//-------------------------------------------------
+//	slot_option::specify
+//-------------------------------------------------
+
+void slot_option::specify(std::string &&text)
+{
+	// we need to do some elementary parsing here
 	const char *bios_arg = ",bios=";
 
 	size_t pos = text.find(bios_arg);
-	return pos != std::string::npos
+	if (pos != std::string::npos)
-		? slot_option(text.substr(0, pos), text.substr(pos + strlen(bios_arg)), selectable)
+	{
-		: slot_option(std::move(text), "", selectable);
+		m_specified = true;
+		m_specified_value = text.substr(0, pos);
+		m_specified_bios = text.substr(pos + strlen(bios_arg));
+	}
+	else
+	{
+		m_specified = true;
+		m_specified_value = std::move(text);
+		m_specified_bios = "";
+	}
 }
 
+
+//-------------------------------------------------
+//	slot_option::set_bios
+//-------------------------------------------------
+
+void slot_option::set_bios(std::string &&text)
+{
+	if (!m_specified)
+	{
+		m_specified = true;
+		m_specified_value = value();
+	}
+	m_specified_bios = std::move(text);
+}

src/emu/emuopts.h

@@ -198,44 +198,48 @@
 class slot_option
 {
 public:
-	slot_option()
+	slot_option(const char *default_value = nullptr);
-		: 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;
 	slot_option(slot_option &&that) = default;
 
 	const slot_option &operator=(const slot_option &that)
 	{
-		// I thought you got this implicitly by declaring a default copy constructor?
+		m_specified = that.m_specified;
-		m_value = that.m_value;
+		m_specified_value = that.m_specified_value;
+		m_specified_bios = that.m_specified_bios;
 		m_default_card_software = that.m_default_card_software;
-		m_bios = that.m_bios;
+		m_default_value = that.m_default_value;
-		return that;
+		return *this;
+	}
+
+	const slot_option &operator=(slot_option &&that)
+	{
+		m_specified = that.m_specified;
+		m_specified_value = std::move(that.m_specified_value);
+		m_specified_bios = std::move(that.m_specified_bios);
+		m_default_card_software = std::move(that.m_default_card_software);
+		m_default_value = std::move(that.m_default_value);
+		return *this;
 	}
 
 	// accessors
-	const std::string &value() const { return m_value.empty() ? m_default_card_software : m_value; }
+	const std::string &value() const;
-	const std::string &bios() const { return m_bios; }
+	std::string specified_value() const;
+	const std::string &bios() const { return m_specified_bios; }
 	const std::string &default_card_software() const { return m_default_card_software; }
-	bool is_default() const { return m_value.empty(); }
+	bool specified() const { return m_specified; }
-	bool is_selectable() const { return m_selectable; }
 
 	// seters
-	void set_bios(std::string &&s) { m_bios = std::move(s); }
+	void specify(std::string &&text);
+	void set_bios(std::string &&text);
 	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;
+	bool			m_specified;
-	std::string m_bios;
+	std::string		m_specified_value;
+	std::string		m_specified_bios;
 	std::string		m_default_card_software;
-	bool m_selectable;
+	std::string		m_default_value;
 };
 
 
@@ -435,11 +439,9 @@ 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 override_get_value_result 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:

src/emu/image.cpp

@@ -38,10 +38,11 @@ image_manager::image_manager(running_machine &machine)
 			continue;
 
 		// is an image specified for this image?
-		if (machine.options().image_options().count(image.instance_name()) > 0)
+		auto iter = machine.options().image_options().find(image.instance_name());
+		if (iter != machine.options().image_options().end() && !iter->second.empty())
 		{
 			// we do have a startup image specified - load it
-			const std::string &startup_image = machine.options().image_options()[image.instance_name()];
+			const std::string &startup_image(iter->second);
 			image_init_result result = image_init_result::FAIL;
 
 			// try as a softlist
@@ -174,24 +175,23 @@ int image_manager::write_config(emu_options &options, const char *filename, cons
 
 void image_manager::options_extract()
 {
-	/* only extract the device options if we've added them
-	   no need to assert in case they are missing */
-	{
-		int index = 0;
-
 	for (device_image_interface &image : image_interface_iterator(machine().root_device()))
 	{
-			const char *filename = image.filename();
+		// we have to assemble the image option differently for software lists and for normal images
-
+		std::string image_opt;
-			/* and set the option */
+		if (image.exists())
-			std::string error;
+		{
-			machine().options().set_value(image.instance_name().c_str(), filename ? filename : "", OPTION_PRIORITY_CMDLINE, error);
+			if (image.loaded_through_softlist())
-
+				image_opt = util::string_format("%s:%s:%s", image.software_list_name(), image.full_software_name(), image.brief_instance_name());
-			index++;
+			else
-		}
+				image_opt = image.filename();
 		}
 
-	/* write the config, if appropriate */
+		// and set the option
+		machine().options().image_options()[image.instance_name()] = std::move(image_opt);
+	}
+
+	// write the config, if appropriate
 	if (machine().options().write_config())
 		write_config(machine().options(), nullptr, &machine().system());
 }

src/emu/mconfig.cpp

@@ -44,6 +44,10 @@ machine_config::machine_config(const game_driver &gamedrv, emu_options &options)
 		bool is_default;
 		if (!has_option)
 		{
+			// Theoretically we should never get here; in the long run the expectation is that
+			// options.slot_options() should be fully qualified and all options should be
+			// present.  However, we're getting late in the MAME 0.185 development cycle and
+			// I don't want to rip this out (yet)
 			selval = slot.default_option();
 			is_default = true;
 		}
@@ -51,7 +55,7 @@ machine_config::machine_config(const game_driver &gamedrv, emu_options &options)
 		{
 			const slot_option &opt = options.slot_options()[slot_option_name];
 			selval = opt.value().c_str();
-			is_default = opt.is_default();
+			is_default = !opt.specified();
 		}
 
 		if (selval && *selval)

src/frontend/mame/mameopts.cpp

@@ -57,20 +57,14 @@ 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();
+			options.slot_options()[name] = slot_option(slot.default_option());
 
 			// allow opportunity to specify this value
 			if (value_specifier)
 			{
-				std::string value = value_specifier(name);
+				std::string specified_value = value_specifier(name);
-				if (value != value_specifier_invalid_value())
+				if (specified_value != value_specifier_invalid_value())
-				{
+					options.slot_options()[name].specify(std::move(specified_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());
-					}
-				}
 			}
 		}
 	}
@@ -307,13 +301,6 @@ 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;
 				}
 			}

src/lib/util/options.cpp

@@ -565,9 +565,21 @@ std::string core_options::output_ini(const core_options *diff) const
 	for (entry &curentry : m_entrylist)
 	{
 		const char *name = curentry.name();
-		const char *value = name && override_get_value(name, overridden_value)
+		const char *value;
-			? overridden_value.c_str()
+		switch (override_get_value(name, overridden_value))
-			: curentry.value();
+		{
+			case override_get_value_result::NONE:
+			default:
+				value = curentry.value();
+				break;
+
+			case override_get_value_result::SKIP:
+				continue;
+
+			case override_get_value_result::OVERRIDE:
+				value = overridden_value.c_str();
+				break;
+		}
 		bool is_unadorned = false;
 
 		// check if it's unadorned

src/lib/util/options.h

@@ -181,8 +181,17 @@ public:
 	int options_count() const { return m_entrylist.count(); }
 
 protected:
+	// This is a hook to allow option value retrieval to be overridden for various reasons; this is a crude
+	// extensibility mechanism that should really be replaced by something better
+	enum class override_get_value_result
+	{
+		NONE,
+		OVERRIDE,
+		SKIP
+	};
+
 	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 override_get_value_result override_get_value(const char *name, std::string &value) const { return override_get_value_result::NONE; }
 	virtual bool override_set_value(const char *name, const std::string &value) { return false; }
 
 private: