summaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
author npwoods <npwoods@alumni.carnegiemellon.edu>2017-03-14 06:38:22 -0400
committer Olivier Galibert <galibert@pobox.com>2017-03-14 11:38:22 +0100
commit59de325644ab7e6ae12a7a57732484c2bddd59de (patch)
treeb782fb49c6dfb024a090487b09052577906b6a71
parent2110ca4c3e37ff8f0b5584349cbc5541a29fad0d (diff)
Preliminary attempt to fix regression caused by poor understanding of get_default_card_software() (#2122)
Tafoid brought to my attention a recent regression caused by my softlist changes. Specifically, this failed: mame odyssey2 -cart1 voice -cart2 bees The reason is that the recent refactoring caused some critical softlist logic to be bypassed. Essentially device_slot_interface::get_default_card_software() provides a technique for devices to inspect softlists and select slot options to be specified. In the case above, the selection of the "voice" software list item in the first cartridge slot requires get_default_card_software() to be called to specify the o2_voice slot device, which in turn adds the second cartridge slot. Skipping this caused '-cart2' to stop being recognized. This is an attempt to fix the problem. In some ways, this doesn't go far enough - the astute observer will note that mame_options::reevaluate_slot_options() looks a lot like mame_options::update_slot_options(); part of me wants to just rip the latter out. In the long run, device_slot_interface::get_default_card_software() should be reevaluated; having a hook is a good idea but it should be a pure function that doesn't alter state.
-rw-r--r--src/frontend/mame/mameopts.cpp100
-rw-r--r--src/frontend/mame/mameopts.h4
2 files changed, 83 insertions, 21 deletions
diff --git a/src/frontend/mame/mameopts.cpp b/src/frontend/mame/mameopts.cpp
index 2e09aef38ab..515e6829baa 100644
--- a/src/frontend/mame/mameopts.cpp
+++ b/src/frontend/mame/mameopts.cpp
@@ -126,12 +126,6 @@ void mame_options::add_device_options(emu_options &options, std::function<void(e
if (!image.user_loadable())
continue;
- // retrieve info about the device instance
- std::ostringstream option_name;
- util::stream_format(option_name, "%s;%s", image.instance_name(), image.brief_instance_name());
- if (strcmp(image.device_typename(image.image_type()), image.instance_name().c_str()) == 0)
- util::stream_format(option_name, ";%s1;%s1", image.instance_name(), image.brief_instance_name());
-
// add the option
if (!options.exists(image.instance_name().c_str()))
{
@@ -140,7 +134,8 @@ void mame_options::add_device_options(emu_options &options, std::function<void(e
options.add_entry(nullptr, "IMAGE DEVICES", OPTION_HEADER | OPTION_FLAG_DEVICE);
// add the option
- options.add_entry(option_name.str().c_str(), nullptr, OPTION_STRING | OPTION_FLAG_DEVICE, nullptr, true);
+ std::string option_name = get_full_option_name(image);
+ options.add_entry(option_name.c_str(), nullptr, OPTION_STRING | OPTION_FLAG_DEVICE, nullptr, true);
// allow opportunity to specify this value
if (value_specifier)
@@ -154,6 +149,20 @@ void mame_options::add_device_options(emu_options &options, std::function<void(e
// remove_device_options - remove device options
//-------------------------------------------------
+std::string mame_options::get_full_option_name(const device_image_interface &image)
+{
+ std::ostringstream option_name;
+ util::stream_format(option_name, "%s;%s", image.instance_name(), image.brief_instance_name());
+ if (strcmp(image.device_typename(image.image_type()), image.instance_name().c_str()) == 0)
+ util::stream_format(option_name, ";%s1;%s1", image.instance_name(), image.brief_instance_name());
+ return option_name.str();
+}
+
+
+//-------------------------------------------------
+// remove_device_options - remove device options
+//-------------------------------------------------
+
void mame_options::remove_device_options(emu_options &options)
{
// iterate through options and remove interesting ones
@@ -182,21 +191,73 @@ void mame_options::remove_device_options(emu_options &options)
// and update slot and image devices
//-------------------------------------------------
-bool mame_options::parse_slot_devices(emu_options &options, std::function<void(emu_options &options, const std::string &)> value_specifier)
+void mame_options::parse_slot_devices(emu_options &options, std::function<void(emu_options &options, const std::string &)> value_specifier)
{
- // keep adding slot options until we stop seeing new stuff
- while (add_slot_options(options, value_specifier)) { }
+ bool still_adding = true;
+ while (still_adding)
+ {
+ // keep adding slot options until we stop seeing new stuff
+ still_adding = false;
+ while (add_slot_options(options, value_specifier))
+ still_adding = true;
- // add device options
- add_device_options(options, value_specifier);
+ // add device options
+ add_device_options(options, value_specifier);
- int num;
- do {
- num = options.options_count();
- update_slot_options(options);
- } while (num != options.options_count());
+ if (reevaluate_slot_options(options))
+ still_adding = true;
+ }
+}
- return true;
+
+//-------------------------------------------------
+// reevaluate_slot_options - based on recent changes
+// in what images are mounted, give drivers a chance
+// to specify new default slot options
+//-------------------------------------------------
+
+bool mame_options::reevaluate_slot_options(emu_options &options)
+{
+ bool result = false;
+
+ // look up the system configured by name; if no match, do nothing
+ const game_driver *cursystem = system(options);
+ if (cursystem == nullptr)
+ return result;
+ machine_config config(*cursystem, options);
+
+ // iterate through all slot devices
+ for (device_slot_interface &slot : slot_interface_iterator(config.root_device()))
+ {
+ // retrieve info about the device instance
+ const char *name = slot.device().tag() + 1;
+ if (options.exists(name) && !slot.option_list().empty())
+ {
+ // device_slot_interface::get_default_card_software() is essentially a hook
+ // that lets devices provide a feedback loop to force a specified software
+ // list entry to be loaded
+ //
+ // In the repeated cycle of adding slots and slot devices, this gives a chance
+ // for devices to "plug in" default software list items. Of course, the fact
+ // that this is all shuffling options is brittle and roundabout, but such is
+ // the nature of software lists.
+ //
+ // In reality, having some sort of hook into the pipeline of slot/device evaluation
+ // makes sense, but the fact that it is joined at the hip to device_image_interface
+ // and device_slot_interface is unfortunate
+ std::string default_card_software = slot.get_default_card_software();
+ if (!default_card_software.empty())
+ {
+ std::string old_value = options.value(name);
+
+ options.set_default_value(name, default_card_software.c_str());
+
+ if (strcmp(old_value.c_str(), options.value(name)))
+ result = true;
+ }
+ }
+ }
+ return result;
}
@@ -239,8 +300,7 @@ bool mame_options::parse_command_line(emu_options &options, std::vector<std::str
};
// parse the slot devices
- if (!parse_slot_devices(options, value_specifier))
- return false;
+ parse_slot_devices(options, value_specifier);
// at this point, we should have handled all arguments; the only argument that shouldn't have
// been handled is the file name
diff --git a/src/frontend/mame/mameopts.h b/src/frontend/mame/mameopts.h
index 69e6e7ec931..8c1e2497327 100644
--- a/src/frontend/mame/mameopts.h
+++ b/src/frontend/mame/mameopts.h
@@ -67,7 +67,9 @@ private:
// device-specific option handling
static void add_device_options(emu_options &options, std::function<void(emu_options &options, const std::string &)> value_specifier = nullptr);
static void update_slot_options(emu_options &options, const software_part *swpart = nullptr);
- static bool parse_slot_devices(emu_options &options, std::function<void(emu_options &options, const std::string &)> value_specifier);
+ static void parse_slot_devices(emu_options &options, std::function<void(emu_options &options, const std::string &)> value_specifier);
+ static std::string get_full_option_name(const device_image_interface &image);
+ static bool reevaluate_slot_options(emu_options &options);
// INI parsing helper
static bool parse_one_ini(emu_options &options, const char *basename, int priority, std::string *error_string = nullptr);