Fix region auto-detection when using native file dialogs.

hiro has two confusingly-named file-picker dialogs. "BrowserDialog" is the
custom dialog built entirely with hiro widgets, "BrowserWindow" is a wrapper
around the native file-picker. bsnes is built for BrowserDialog, but if you tick
"Use native file picker" in the Emulator options, when it needs a file it will
construct a BrowserDialog, copy the relevant config options across to a new
BrowserWindow and invoke that.

Unfortunately, BrowserDialog and BrowserWindow have different capabilities.
Specifically, BrowserDialog includes an "options" list which bsnes uses to let
the user override region detection when loading a game. BrowserWindow has no
such widget. Thus, when using a BrowserDialog the options list worked as intended,
but when using a BrowserWindow the options list was never initialised and no
option was ever chosen. As a result, when opening a game with the native file-
picker, bsnes always used NTSC emulation mode, instead of auto-detecting.

Previously, constructing a BrowserDialog and calling setOptions would leave the
BrowserDialog in an inconsistent state (with no option selected). This was OK if
you immediately displayed the dialog to the user (this would complete the
initialisation and choose a default), but bsnes also used BrowserDialog as an
*interface* typee, to represent the parameters and results of a file-picker
operation that was implemented elsewhere. For this use-case, the inconsistent
state caused problems.

Therefore, BrowserDialog has been changed:

  - setting the list of options always chooses a default, maintaining the
    invariant that `.options()` always returns one of the available options.
  - The internal BrowserDialogWindow class now takes a reference to a Response
    object, instead of constructing one from scratch and having to duplicate
    the "set .option to a reasonable value" code.

Fixes #44.
This commit is contained in:
Tim Allen 2020-10-26 21:52:42 +11:00 committed by Screwtapello
parent 090b79b3be
commit f09c45f3e4
1 changed files with 5 additions and 6 deletions

View File

@ -3,7 +3,7 @@
struct BrowserDialogWindow { struct BrowserDialogWindow {
Application::Namespace tr{"BrowserDialog"}; Application::Namespace tr{"BrowserDialog"};
BrowserDialogWindow(BrowserDialog::State& state) : state(state) {} BrowserDialogWindow(BrowserDialog::State& state, BrowserDialog::Response& response) : state(state), response(response) {}
auto accept() -> void; auto accept() -> void;
auto activate() -> void; auto activate() -> void;
auto change() -> void; auto change() -> void;
@ -41,7 +41,7 @@ private:
MenuCheckItem showHiddenOption{&contextMenu}; MenuCheckItem showHiddenOption{&contextMenu};
BrowserDialog::State& state; BrowserDialog::State& state;
BrowserDialog::Response response; BrowserDialog::Response& response;
vector<vector<string>> filters; vector<vector<string>> filters;
}; };
@ -193,8 +193,6 @@ auto BrowserDialogWindow::isMatch(const string& name) -> bool {
} }
auto BrowserDialogWindow::run() -> BrowserDialog::Response { auto BrowserDialogWindow::run() -> BrowserDialog::Response {
response = {};
auto document = BML::unserialize(file::read({Path::userSettings(), "hiro/browser-dialog.bml"})); auto document = BML::unserialize(file::read({Path::userSettings(), "hiro/browser-dialog.bml"}));
struct Settings { struct Settings {
bool showHidden = true; bool showHidden = true;
@ -226,7 +224,6 @@ auto BrowserDialogWindow::run() -> BrowserDialog::Response {
for(auto& option : state.options) { for(auto& option : state.options) {
optionList.append(ComboButtonItem().setText(option)); optionList.append(ComboButtonItem().setText(option));
} }
optionList.doChange(); //updates response.option to point to the default (first) option
image iconSearch{Icon::Action::Search}; image iconSearch{Icon::Action::Search};
iconSearch.scale(16_sx, 16_sy); iconSearch.scale(16_sx, 16_sy);
searchButton.setIcon(iconSearch).setBordered(false).onActivate([&] { setPath(state.path, fileName.text()); }); searchButton.setIcon(iconSearch).setBordered(false).onActivate([&] { setPath(state.path, fileName.text()); });
@ -503,6 +500,8 @@ auto BrowserDialog::setName(const string& name) -> type& {
auto BrowserDialog::setOptions(const vector<string>& options) -> type& { auto BrowserDialog::setOptions(const vector<string>& options) -> type& {
state.options = options; state.options = options;
// ensure response.option always contains a valid option
response.option = options ? options.first() : "";
return *this; return *this;
} }
@ -522,7 +521,7 @@ auto BrowserDialog::title() const -> string {
auto BrowserDialog::_run() -> vector<string> { auto BrowserDialog::_run() -> vector<string> {
if(!state.path) state.path = Path::user(); if(!state.path) state.path = Path::user();
response = BrowserDialogWindow(state).run(); BrowserDialogWindow(state, response).run();
return response.selected; return response.selected;
} }