Refactor accelerator / global shortcuts handling

* Removes wxAcceleratorEntryUnicode and assorted arrays in favor of a
  new class, `config::Shortcuts`, which handles UserInput assignment to
  commands and resolution at runtime. `config::Shortcuts` also handles the
  INI user configuration in a backwards-compatible way. Runtime
  resolution of UserInput to command is also now logarithmic rather than
  linear.
* The same shortcut can no longer be assigned to 2 different commands,
  which fixes #158.
* Moves the `AccelConfig` dialog to its own dedicated class.

src/wx/CMakeLists.txt

@@ -762,9 +762,12 @@ set(
     wxutil.cpp
     config/game-control.cpp
     config/internal/option-internal.cpp
+    config/internal/shortcuts-internal.cpp
     config/option-observer.cpp
     config/option.cpp
+    config/shortcuts.cpp
     config/user-input.cpp
+    dialogs/accel-config.cpp
     dialogs/directories-config.cpp
     dialogs/display-config.cpp
     dialogs/game-boy-config.cpp
@@ -808,11 +811,14 @@ set(
     wxutil.h
     config/game-control.h
     config/internal/option-internal.h
+    config/internal/shortcuts-internal.h
     config/option-id.h
     config/option-observer.h
     config/option-proxy.h
     config/option.h
+    config/shortcuts.h
     config/user-input.h
+    dialogs/accel-config.h
     dialogs/directories-config.h
     dialogs/display-config.h
     dialogs/game-boy-config.h

src/wx/cmdevents.cpp

@@ -2710,8 +2710,10 @@ EVT_HANDLER(Customize, "Customize UI...")
 
     if (!joy_timer) frame->StartJoyPollTimer();
 
-    if (ShowModal(dlg) == wxID_OK)
+    if (ShowModal(dlg) == wxID_OK) {
-        update_opts();
+        update_shortcut_opts();
+        ResetMenuAccelerators();
+    }
 
     if (!joy_timer) frame->StopJoyPollTimer();
 

src/wx/config/internal/shortcuts-internal.cpp

@@ -0,0 +1,107 @@
+#include "config/shortcuts.h"
+
+#include <wx/xrc/xmlres.h>
+
+#define VBAM_SHORTCUTS_INTERNAL_INCLUDE
+#include "config/internal/shortcuts-internal.h"
+#undef VBAM_SHORTCUTS_INTERNAL_INCLUDE
+
+namespace config {
+namespace internal {
+
+const std::unordered_map<int, UserInput>& DefaultShortcuts() {
+    static const std::unordered_map<int, UserInput> kDefaultShortcuts = {
+        {XRCID("CheatsList"), UserInput('C', wxMOD_CMD)},
+        {XRCID("NextFrame"), UserInput('N', wxMOD_CMD)},
+        // this was annoying people A LOT #334
+        //{wxID_EXIT, UserInput(WXK_ESCAPE, wxMOD_NONE)},
+        // this was annoying people #298
+        //{wxID_EXIT, UserInput('X', wxMOD_CMD)},
+
+        {wxID_EXIT, UserInput('Q', wxMOD_CMD)},
+        {wxID_CLOSE, UserInput('W', wxMOD_CMD)},
+        // load most recent is more commonly used than load state
+        // {XRCID("Load"), UserInput('L', wxMOD_CMD)},
+        {XRCID("LoadGameRecent"), UserInput('L', wxMOD_CMD)},
+        {XRCID("LoadGame01"), UserInput(WXK_F1, wxMOD_NONE)},
+        {XRCID("LoadGame02"), UserInput(WXK_F2, wxMOD_NONE)},
+        {XRCID("LoadGame03"), UserInput(WXK_F3, wxMOD_NONE)},
+        {XRCID("LoadGame04"), UserInput(WXK_F4, wxMOD_NONE)},
+        {XRCID("LoadGame05"), UserInput(WXK_F5, wxMOD_NONE)},
+        {XRCID("LoadGame06"), UserInput(WXK_F6, wxMOD_NONE)},
+        {XRCID("LoadGame07"), UserInput(WXK_F7, wxMOD_NONE)},
+        {XRCID("LoadGame08"), UserInput(WXK_F8, wxMOD_NONE)},
+        {XRCID("LoadGame09"), UserInput(WXK_F9, wxMOD_NONE)},
+        {XRCID("LoadGame10"), UserInput(WXK_F10, wxMOD_NONE)},
+        {XRCID("Pause"), UserInput(WXK_PAUSE, wxMOD_NONE)},
+        {XRCID("Pause"), UserInput('P', wxMOD_CMD)},
+        {XRCID("Reset"), UserInput('R', wxMOD_CMD)},
+        // add shortcuts for original size multiplier #415
+        {XRCID("SetSize1x"), UserInput('1', wxMOD_NONE)},
+        {XRCID("SetSize2x"), UserInput('2', wxMOD_NONE)},
+        {XRCID("SetSize3x"), UserInput('3', wxMOD_NONE)},
+        {XRCID("SetSize4x"), UserInput('4', wxMOD_NONE)},
+        {XRCID("SetSize5x"), UserInput('5', wxMOD_NONE)},
+        {XRCID("SetSize6x"), UserInput('6', wxMOD_NONE)},
+        // save oldest is more commonly used than save other
+        // {XRCID("Save"), UserInput('S', wxMOD_CMD)},
+        {XRCID("SaveGameOldest"), UserInput('S', wxMOD_CMD)},
+        {XRCID("SaveGame01"), UserInput(WXK_F1, wxMOD_SHIFT)},
+        {XRCID("SaveGame02"), UserInput(WXK_F2, wxMOD_SHIFT)},
+        {XRCID("SaveGame03"), UserInput(WXK_F3, wxMOD_SHIFT)},
+        {XRCID("SaveGame04"), UserInput(WXK_F4, wxMOD_SHIFT)},
+        {XRCID("SaveGame05"), UserInput(WXK_F5, wxMOD_SHIFT)},
+        {XRCID("SaveGame06"), UserInput(WXK_F6, wxMOD_SHIFT)},
+        {XRCID("SaveGame07"), UserInput(WXK_F7, wxMOD_SHIFT)},
+        {XRCID("SaveGame08"), UserInput(WXK_F8, wxMOD_SHIFT)},
+        {XRCID("SaveGame09"), UserInput(WXK_F9, wxMOD_SHIFT)},
+        {XRCID("SaveGame10"), UserInput(WXK_F10, wxMOD_SHIFT)},
+        // I prefer the SDL ESC key binding
+        // {XRCID("ToggleFullscreen"), UserInput(WXK_ESCAPE, wxMOD_NONE)},
+        // alt-enter is more standard anyway
+        {XRCID("ToggleFullscreen"), UserInput(WXK_RETURN, wxMOD_ALT)},
+        {XRCID("JoypadAutofireA"), UserInput('1', wxMOD_ALT)},
+        {XRCID("JoypadAutofireB"), UserInput('2', wxMOD_ALT)},
+        {XRCID("JoypadAutofireL"), UserInput('3', wxMOD_ALT)},
+        {XRCID("JoypadAutofireR"), UserInput('4', wxMOD_ALT)},
+        {XRCID("VideoLayersBG0"), UserInput('1', wxMOD_CMD)},
+        {XRCID("VideoLayersBG1"), UserInput('2', wxMOD_CMD)},
+        {XRCID("VideoLayersBG2"), UserInput('3', wxMOD_CMD)},
+        {XRCID("VideoLayersBG3"), UserInput('4', wxMOD_CMD)},
+        {XRCID("VideoLayersOBJ"), UserInput('5', wxMOD_CMD)},
+        {XRCID("VideoLayersWIN0"), UserInput('6', wxMOD_CMD)},
+        {XRCID("VideoLayersWIN1"), UserInput('7', wxMOD_CMD)},
+        {XRCID("VideoLayersOBJWIN"), UserInput('8', wxMOD_CMD)},
+        {XRCID("Rewind"), UserInput('B', wxMOD_CMD)},
+        // following are not in standard menus
+        // FILExx are filled in when recent menu is filled
+        {wxID_FILE1, UserInput(WXK_F1, wxMOD_CMD)},
+        {wxID_FILE2, UserInput(WXK_F2, wxMOD_CMD)},
+        {wxID_FILE3, UserInput(WXK_F3, wxMOD_CMD)},
+        {wxID_FILE4, UserInput(WXK_F4, wxMOD_CMD)},
+        {wxID_FILE5, UserInput(WXK_F5, wxMOD_CMD)},
+        {wxID_FILE6, UserInput(WXK_F6, wxMOD_CMD)},
+        {wxID_FILE7, UserInput(WXK_F7, wxMOD_CMD)},
+        {wxID_FILE8, UserInput(WXK_F8, wxMOD_CMD)},
+        {wxID_FILE9, UserInput(WXK_F9, wxMOD_CMD)},
+        {wxID_FILE10, UserInput(WXK_F10, wxMOD_CMD)},
+        {XRCID("VideoLayersReset"), UserInput('0', wxMOD_CMD)},
+        {XRCID("ChangeFilter"), UserInput('G', wxMOD_CMD)},
+        {XRCID("ChangeIFB"), UserInput('I', wxMOD_CMD)},
+        {XRCID("IncreaseVolume"), UserInput(WXK_NUMPAD_ADD, wxMOD_NONE)},
+        {XRCID("DecreaseVolume"), UserInput(WXK_NUMPAD_SUBTRACT, wxMOD_NONE)},
+        {XRCID("ToggleSound"), UserInput(WXK_NUMPAD_ENTER, wxMOD_NONE)},
+    };
+    return kDefaultShortcuts;
+}
+
+UserInput DefaultShortcutForCommand(int command) {
+    const auto& iter = DefaultShortcuts().find(command);
+    if (iter != DefaultShortcuts().end()) {
+        return iter->second;
+    }
+    return UserInput();
+}
+
+}  // namespace internal
+}  // namespace config

src/wx/config/internal/shortcuts-internal.h

@@ -0,0 +1,21 @@
+#ifndef VBAM_SHORTCUTS_INTERNAL_INCLUDE
+#error "Do not include "config/internal/shortcuts-internal.h" outside of the implementation."
+#endif
+
+#include <set>
+#include <unordered_map>
+
+#include "config/user-input.h"
+
+namespace config {
+namespace internal {
+
+// Returns the map of commands to their default shortcut.
+const std::unordered_map<int, UserInput>& DefaultShortcuts();
+
+// Returns the default shortcut for the given `command`.
+// Returns an Invalid UserInput if there is no default shortcut for `command`.
+UserInput DefaultShortcutForCommand(int command);
+
+}  // namespace internal
+}  // namespace config

src/wx/config/shortcuts.cpp

@@ -0,0 +1,183 @@
+#include "config/shortcuts.h"
+
+#include <wx/string.h>
+#include <wx/translation.h>
+#include <wx/xrc/xmlres.h>
+
+#include "config/user-input.h"
+
+#define VBAM_SHORTCUTS_INTERNAL_INCLUDE
+#include "config/internal/shortcuts-internal.h"
+#undef VBAM_SHORTCUTS_INTERNAL_INCLUDE
+
+namespace config {
+
+namespace {
+
+int NoopCommand() {
+    static const int noop = XRCID("NOOP");
+    return noop;
+}
+
+}  // namespace
+
+Shortcuts::Shortcuts() {
+    // Set up default shortcuts.
+    for (const auto& iter : internal::DefaultShortcuts()) {
+        AssignInputToCommand(iter.second, iter.first);
+    }
+}
+
+Shortcuts::Shortcuts(const std::unordered_map<int, std::set<UserInput>>& command_to_inputs,
+                     const std::map<UserInput, int>& input_to_command,
+                     const std::map<UserInput, int>& disabled_defaults)
+    : command_to_inputs_(command_to_inputs.begin(), command_to_inputs.end()),
+      input_to_command_(input_to_command.begin(), input_to_command.end()),
+      disabled_defaults_(disabled_defaults.begin(), disabled_defaults.end()) {}
+
+std::vector<std::pair<int, wxString>> Shortcuts::GetConfiguration() const {
+    std::vector<std::pair<int, wxString>> config;
+    config.reserve(command_to_inputs_.size() + 1);
+
+    if (!disabled_defaults_.empty()) {
+        std::set<UserInput> noop_inputs;
+        for (const auto& iter : disabled_defaults_) {
+            noop_inputs.insert(iter.first);
+        }
+        config.push_back(std::make_pair(NoopCommand(), UserInput::SpanToConfigString(noop_inputs)));
+    }
+
+    for (const auto& iter : command_to_inputs_) {
+        std::set<UserInput> inputs;
+        for (const auto& input : iter.second) {
+            if (internal::DefaultShortcutForCommand(iter.first) != input) {
+                // Not a default input.
+                inputs.insert(input);
+            }
+        }
+        if (!inputs.empty()) {
+            config.push_back(std::make_pair(iter.first, UserInput::SpanToConfigString(inputs)));
+        }
+    }
+
+    return config;
+}
+
+std::set<UserInput> Shortcuts::InputsForCommand(int command) const {
+    if (command == NoopCommand()) {
+        std::set<UserInput> noop_inputs;
+        for (const auto& iter : disabled_defaults_) {
+            noop_inputs.insert(iter.first);
+        }
+        return noop_inputs;
+    }
+
+    auto iter = command_to_inputs_.find(command);
+    if (iter == command_to_inputs_.end()) {
+        return {};
+    }
+    return iter->second;
+}
+
+int Shortcuts::CommandForInput(const UserInput& input) const {
+    const auto iter = input_to_command_.find(input);
+    if (iter == input_to_command_.end()) {
+        return 0;
+    }
+    return iter->second;
+}
+
+std::set<wxJoystick> Shortcuts::Joysticks() const {
+    std::set<wxJoystick> output;
+    for (const auto& iter : command_to_inputs_) {
+        for (const UserInput& user_input : iter.second) {
+            output.insert(user_input.joystick());
+        }
+    }
+    return output;
+}
+
+Shortcuts Shortcuts::Clone() const {
+    return Shortcuts(this->command_to_inputs_, this->input_to_command_, this->disabled_defaults_);
+}
+
+void Shortcuts::AssignInputToCommand(const UserInput& input, int command) {
+    if (command == NoopCommand()) {
+        // "Assigning to Noop" means unassinging the default binding.
+        UnassignDefaultBinding(input);
+        return;
+    }
+
+    // Remove the existing binding if it exists.
+    auto iter = input_to_command_.find(input);
+    if (iter != input_to_command_.end()) {
+        UnassignInput(input);
+    }
+
+    auto disabled_iter = disabled_defaults_.find(input);
+    if (disabled_iter != disabled_defaults_.end()) {
+        int original_command = disabled_iter->second;
+        if (original_command == command) {
+            // Restoring a disabled input. Remove from the disabled set.
+            disabled_defaults_.erase(disabled_iter);
+        }
+        // Then, just continue normally.
+    }
+
+    command_to_inputs_[command].emplace(input);
+    input_to_command_[input] = command;
+}
+
+void Shortcuts::UnassignInput(const UserInput& input) {
+    assert(input);
+
+    auto iter = input_to_command_.find(input);
+    if (iter == input_to_command_.end()) {
+        // Input not found, nothing to do.
+        return;
+    }
+
+    if (internal::DefaultShortcutForCommand(iter->second) == input) {
+        // Unassigning a default binding has some special handling.
+        UnassignDefaultBinding(input);
+        return;
+    }
+
+    // Otherwise, just remove it from the 2 maps.
+    auto command_iter = command_to_inputs_.find(iter->second);
+    assert(command_iter != command_to_inputs_.end());
+
+    command_iter->second.erase(input);
+    if (command_iter->second.empty()) {
+        // Remove empty set.
+        command_to_inputs_.erase(command_iter);
+    }
+    input_to_command_.erase(iter);
+}
+
+void Shortcuts::UnassignDefaultBinding(const UserInput& input) {
+    auto input_iter = input_to_command_.find(input);
+    if (input_iter == input_to_command_.end()) {
+        // This can happen if the INI file provided by the user has an invalid
+        // option. In this case, just silently ignore it.
+        return;
+    }
+
+    if (internal::DefaultShortcutForCommand(input_iter->second) != input) {
+        // As above, we have already removed the default binding, ignore it.
+        return;
+    }
+
+    auto command_iter = command_to_inputs_.find(input_iter->second);
+    assert(command_iter != command_to_inputs_.end());
+
+    command_iter->second.erase(input);
+    if (command_iter->second.empty()) {
+        command_to_inputs_.erase(command_iter);
+    }
+
+    disabled_defaults_[input] = input_iter->second;
+    input_to_command_.erase(input_iter);
+}
+
+}  // namespace config

src/wx/config/shortcuts.h

@@ -0,0 +1,86 @@
+#ifndef VBAM_WX_CONFIG_SHORTCUTS_H_
+#define VBAM_WX_CONFIG_SHORTCUTS_H_
+
+#include <map>
+#include <set>
+#include <vector>
+#include <unordered_map>
+#include <utility>
+
+#include "config/user-input.h"
+
+// by default, only 9 recent items
+#define wxID_FILE10 (wxID_FILE9 + 1)
+
+namespace config {
+
+// Represents a set of shortcuts from a user input (keyboard or joypad) to a
+// command. Internally, this class keeps track of all the necessary data for
+// resolving a user input to a command at runtime and for updating the INI file.
+class Shortcuts {
+public:
+    Shortcuts();
+    ~Shortcuts() = default;
+
+    Shortcuts(Shortcuts&&) noexcept = default;
+    Shortcuts& operator=(Shortcuts&&) noexcept = default;
+
+    // Disable copy and copy assignment operator.
+    // Clone() is provided only for the configuration window, this class
+    // should otherwise be treated as move-only.
+    Shortcuts(const Shortcuts&) = delete;
+    Shortcuts& operator=(const Shortcuts&) = delete;
+
+    // Returns the configuration for the INI file.
+    // Internally, there are global default system inputs that are immediately
+    // available on first run. For the configuration saved in the [Keyboard]
+    // section of the vbam.ini file, we only keep track of the following:
+    // - Disabled default input. These appear under [Keyboard/NOOP].
+    // - User-added custom bindings. These appear under [Keyboard/CommandName].
+    std::vector<std::pair<int, wxString>> GetConfiguration() const;
+
+    // Returns the list of input currently configured for `command`.
+    std::set<UserInput> InputsForCommand(int command) const;
+
+    // Returns the command currently assigned to `input` or nullptr if none.
+    int CommandForInput(const UserInput& input) const;
+
+    // Returns the set of joysticks used by shortcuts.
+    std::set<wxJoystick> Joysticks() const;
+
+    // Returns a copy of this object. This can be an expensive operation and
+    // should only be used to modify the currently active shortcuts
+    // configuration.
+    Shortcuts Clone() const;
+
+    // Assigns `input` to `command`. Silently unassigns `input` if it is already
+    // assigned to another command.
+    void AssignInputToCommand(const UserInput& input, int command);
+
+    // Removes `input` assignment. No-op if `input` is not assigned. `input`
+    // must be a valid UserInput.
+    void UnassignInput(const UserInput& input);
+
+private:
+    // Faster constructor for explitit copy.
+    Shortcuts(const std::unordered_map<int, std::set<UserInput>>& command_to_inputs,
+              const std::map<UserInput, int>& input_to_command,
+              const std::map<UserInput, int>& disabled_defaults);
+
+    // Helper method to unassign a binding used by the default configuration.
+    // This requires special handling since the INI configuration is a diff
+    // between the default bindings and the user configuration.
+    void UnassignDefaultBinding(const UserInput& input);
+
+    // Map of command to their associated input set.
+    std::unordered_map<int, std::set<UserInput>> command_to_inputs_;
+    // Reverse map of the above. An input can only map to a single command.
+    std::map<UserInput, int> input_to_command_;
+    // Disabled default inputs. This is used to easily retrieve the
+    // configuration to save in the INI file.
+    std::map<UserInput, int> disabled_defaults_;
+};
+
+}  // namespace config
+
+#endif  // VBAM_WX_CONFIG_SHORTCUTS_H_

src/wx/dialogs/accel-config.cpp

@@ -0,0 +1,364 @@
+#include "dialogs/accel-config.h"
+
+#include <wx/ctrlsub.h>
+#include <wx/event.h>
+#include <wx/listbox.h>
+#include <wx/menu.h>
+#include <wx/msgdlg.h>
+#include <wx/xrc/xmlres.h>
+
+#include "config/shortcuts.h"
+#include "config/user-input.h"
+#include "dialogs/validated-child.h"
+#include "opts.h"
+#include "widgets/user-input-ctrl.h"
+#include "wxvbam.h"
+
+namespace dialogs {
+
+namespace {
+
+// Client data holding a config::UserInput.
+class UserInputClientData : public wxClientData {
+public:
+    explicit UserInputClientData(const config::UserInput& user_input) : user_input_(user_input) {}
+    explicit UserInputClientData(config::UserInput&& user_input)
+        : user_input_(std::move(user_input)) {}
+    ~UserInputClientData() override = default;
+
+    const config::UserInput& user_input() const { return user_input_; }
+
+private:
+    const config::UserInput user_input_;
+};
+
+// Holds a Command reference and the corresponding string used for current
+// assignment reference for fast access at configuration time. Owned by
+// the corresponding wxTreeItem.
+class CommandTreeItemData : public wxTreeItemData {
+public:
+    CommandTreeItemData(int command, wxString assigned_string, wxString message_string)
+        : wxTreeItemData(),
+          command_(command),
+          assigned_string_(std::move(assigned_string)),
+          message_string_(std::move(message_string)) {}
+    ~CommandTreeItemData() override = default;
+
+    int command() const { return command_; }
+    const wxString& assigned_string() const { return assigned_string_; };
+    const wxString& message_string() const { return message_string_; };
+
+private:
+    const int command_;
+    const wxString assigned_string_;
+    const wxString message_string_;
+};
+
+wxString AppendString(const wxString& prefix, int level, const wxString& command_name) {
+    return prefix + wxString(' ', 2 * level) + command_name;
+}
+
+wxString AppendMenuItem(const wxString& prefix, int level, const wxMenuItem* menu_item) {
+    return AppendString(prefix, level,
+                        menu_item->GetItemLabelText() + (menu_item->IsSubMenu() ? "\n" : ""));
+}
+
+void AppendItemToTree(std::unordered_map<int, wxTreeItemId>* command_to_item_id,
+                      wxTreeCtrl* tree,
+                      const wxTreeItemId& parent,
+                      int command,
+                      const wxString& prefix,
+                      int level) {
+    int i = 0;
+    for (; i < ncmds; i++) {
+        if (command == cmdtab[i].cmd_id) {
+            break;
+        }
+    }
+    assert(i < ncmds);
+
+    const wxTreeItemId tree_item_id =
+        tree->AppendItem(parent,
+                         /*text=*/cmdtab[i].name,
+                         /*image=*/-1,
+                         /*selImage=*/-1,
+                         /*data=*/
+                         new CommandTreeItemData(
+                             command, AppendString(prefix, level, cmdtab[i].name), cmdtab[i].name));
+    command_to_item_id->emplace(command, tree_item_id);
+}
+
+// Built the initial tree control from the menu.
+void PopulateTreeWithMenu(std::unordered_map<int, wxTreeItemId>* command_to_item_id,
+                          wxTreeCtrl* tree,
+                          const wxTreeItemId& parent,
+                          wxMenu* menu,
+                          const wxMenu* recents,
+                          const wxString& prefix,
+                          int level = 1) {
+    for (auto menu_item : menu->GetMenuItems()) {
+        if (menu_item->IsSeparator()) {
+            tree->AppendItem(parent, "-----");
+        } else if (menu_item->IsSubMenu()) {
+            if (menu_item->GetSubMenu() == recents) {
+                // This has to be done manually because not all recents are always populated.
+                const wxTreeItemId recents_parent =
+                    tree->AppendItem(parent, menu_item->GetItemLabelText());
+                const wxString recents_prefix = AppendMenuItem(prefix, level, menu_item);
+                for (int i = wxID_FILE1; i <= wxID_FILE10; i++) {
+                    AppendItemToTree(command_to_item_id, tree, recents_parent, i, recents_prefix,
+                                     level + 1);
+                }
+            } else {
+                const wxTreeItemId sub_parent =
+                    tree->AppendItem(parent, menu_item->GetItemLabelText());
+                PopulateTreeWithMenu(command_to_item_id, tree, sub_parent, menu_item->GetSubMenu(),
+                                     recents, AppendMenuItem(prefix, level, menu_item), level + 1);
+            }
+        } else {
+            AppendItemToTree(command_to_item_id, tree, parent, menu_item->GetId(), prefix, level);
+        }
+    }
+}
+
+}  // namespace
+
+// static
+AccelConfig* AccelConfig::NewInstance(wxWindow* parent, wxMenuBar* menu, wxMenu* recents) {
+    assert(parent);
+    assert(menu);
+    assert(recents);
+    return new AccelConfig(parent, menu, recents);
+}
+
+AccelConfig::AccelConfig(wxWindow* parent, wxMenuBar* menu, wxMenu* recents)
+    : wxDialog(), keep_on_top_styler_(this) {
+    assert(parent);
+    assert(menu);
+
+    // Load the dialog XML.
+    const bool success = wxXmlResource::Get()->LoadDialog(this, parent, "AccelConfig");
+    assert(success);
+
+    // Loads the various dialog elements.
+    tree_ = GetValidatedChild<wxTreeCtrl>(this, "Commands");
+    current_keys_ = GetValidatedChild<wxListBox>(this, "Current");
+    assign_button_ = GetValidatedChild(this, "Assign");
+    remove_button_ = GetValidatedChild(this, "Remove");
+    key_input_ = GetValidatedChild<widgets::UserInputCtrl>(this, "Shortcut");
+    currently_assigned_label_ = GetValidatedChild<wxControl>(this, "AlreadyThere");
+
+    // Configure the key input.
+    key_input_->MoveBeforeInTabOrder(assign_button_);
+
+    // Populate the tree from the menu.
+    wxTreeItemId root_id = tree_->AddRoot("root");
+    wxTreeItemId menu_id = tree_->AppendItem(root_id, _("Menu commands"));
+    for (size_t i = 0; i < menu->GetMenuCount(); i++) {
+        wxTreeItemId id = tree_->AppendItem(menu_id, menu->GetMenuLabelText(i));
+        PopulateTreeWithMenu(&command_to_item_id_, tree_, id, menu->GetMenu(i), recents,
+                             menu->GetMenuLabelText(i) + '\n');
+    }
+    tree_->ExpandAll();
+    tree_->SelectItem(menu_id);
+
+    // Set the initial tree size.
+    wxSize size = tree_->GetBestSize();
+    size.SetHeight(std::min(200, size.GetHeight()));
+    tree_->SetSize(size);
+    size.SetWidth(-1);  // maybe allow it to become bigger
+    tree_->SetSizeHints(size, size);
+
+    int w, h;
+    current_keys_->GetTextExtent("CTRL-ALT-SHIFT-ENTER", &w, &h);
+    size.Set(w, h);
+    current_keys_->SetMinSize(size);
+
+    // Compute max size for currently_assigned_label_.
+    size.Set(0, 0);
+    for (const auto& iter : command_to_item_id_) {
+        const CommandTreeItemData* item_data =
+            static_cast<const CommandTreeItemData*>(tree_->GetItemData(iter.second));
+        assert(item_data);
+
+        currently_assigned_label_->GetTextExtent(item_data->assigned_string(), &w, &h);
+        size.SetWidth(std::max(w, size.GetWidth()));
+        size.SetHeight(std::max(h, size.GetHeight()));
+    }
+    currently_assigned_label_->SetMinSize(size);
+    currently_assigned_label_->SetSizeHints(size);
+
+    // Finally, bind the events.
+    Bind(wxEVT_SHOW, &AccelConfig::OnDialogShown, this, GetId());
+    Bind(wxEVT_TREE_SEL_CHANGING, &AccelConfig::OnCommandSelected, this, tree_->GetId());
+    Bind(wxEVT_TREE_SEL_CHANGED, &AccelConfig::OnCommandSelected, this, tree_->GetId());
+    Bind(wxEVT_LISTBOX, &AccelConfig::OnKeySelected, this, current_keys_->GetId());
+    Bind(wxEVT_BUTTON, &AccelConfig::OnValidate, this, wxID_OK);
+    Bind(wxEVT_BUTTON, &AccelConfig::OnAssignBinding, this, assign_button_->GetId());
+    Bind(wxEVT_BUTTON, &AccelConfig::OnRemoveBinding, this, remove_button_->GetId());
+    Bind(wxEVT_BUTTON, &AccelConfig::OnResetAll, this, XRCID("ResetAll"));
+    Bind(wxEVT_TEXT, &AccelConfig::OnKeyInput, this, key_input_->GetId());
+
+    // And fit everything nicely.
+    Fit();
+}
+
+void AccelConfig::OnDialogShown(wxShowEvent& ev) {
+    // Let the event propagate.
+    ev.Skip();
+
+    if (!ev.IsShown()) {
+        return;
+    }
+
+    // Reset the dialog.
+    current_keys_->Clear();
+    tree_->Unselect();
+    tree_->ExpandAll();
+    key_input_->Clear();
+    assign_button_->Enable(false);
+    remove_button_->Enable(false);
+    currently_assigned_label_->SetLabel("");
+
+    config_shortcuts_ = gopts.shortcuts.Clone();
+}
+
+void AccelConfig::OnValidate(wxCommandEvent& ev) {
+    gopts.shortcuts = std::move(config_shortcuts_);
+    ev.Skip();
+}
+
+void AccelConfig::OnCommandSelected(wxTreeEvent& ev) {
+    const CommandTreeItemData* command_tree_data =
+        static_cast<const CommandTreeItemData*>(tree_->GetItemData(ev.GetItem()));
+
+    if (!command_tree_data) {
+        selected_command_ = 0;
+        PopulateCurrentKeys();
+        ev.Veto();
+        return;
+    }
+
+    if (ev.GetEventType() == wxEVT_COMMAND_TREE_SEL_CHANGING) {
+        ev.Skip();
+        return;
+    }
+
+    selected_command_ = command_tree_data->command();
+    PopulateCurrentKeys();
+}
+
+void AccelConfig::OnKeySelected(wxCommandEvent&) {
+    remove_button_->Enable(current_keys_->GetSelection() != wxNOT_FOUND);
+}
+
+void AccelConfig::OnRemoveBinding(wxCommandEvent&) {
+    const int selection = current_keys_->GetSelection();
+    if (selection == wxNOT_FOUND) {
+        return;
+    }
+
+    config_shortcuts_.UnassignInput(
+        static_cast<UserInputClientData*>(current_keys_->GetClientObject(selection))->user_input());
+    PopulateCurrentKeys();
+}
+
+void AccelConfig::OnResetAll(wxCommandEvent&) {
+    const int confirmation = wxMessageBox(
+        _("This will clear all user-defined accelerators. Are you sure?"), _("Confirm"), wxYES_NO);
+    if (confirmation != wxYES) {
+        return;
+    }
+
+    config_shortcuts_ = config::Shortcuts();
+    tree_->Unselect();
+    key_input_->Clear();
+    PopulateCurrentKeys();
+}
+
+void AccelConfig::OnAssignBinding(wxCommandEvent&) {
+    const wxTreeItemId selected_id = tree_->GetSelection();
+    const config::UserInput user_input = key_input_->SingleInput();
+    if (!selected_id.IsOk() || !user_input) {
+        return;
+    }
+
+    const CommandTreeItemData* data =
+        static_cast<CommandTreeItemData*>(tree_->GetItemData(selected_id));
+    if (!data) {
+        return;
+    }
+
+    const int old_command = config_shortcuts_.CommandForInput(user_input);
+    if (old_command != 0) {
+        const auto iter = command_to_item_id_.find(old_command);
+        assert(iter != command_to_item_id_.end());
+        const CommandTreeItemData* old_command_item_data =
+            static_cast<const CommandTreeItemData*>(tree_->GetItemData(iter->second));
+        assert(old_command_item_data);
+
+        // Require user confirmation to override.
+        const int confirmation =
+            wxMessageBox(wxString::Format(_("This will unassign \"%s\" from \"%s\". Are you sure?"),
+                                          user_input.ToLocalizedString(),
+                                          old_command_item_data->message_string()),
+                         _("Confirm"), wxYES_NO);
+        if (confirmation != wxYES) {
+            return;
+        }
+    }
+
+    config_shortcuts_.AssignInputToCommand(user_input, data->command());
+    PopulateCurrentKeys();
+}
+
+void AccelConfig::OnKeyInput(wxCommandEvent&) {
+    const config::UserInput user_input = key_input_->SingleInput();
+    if (!user_input) {
+        currently_assigned_label_->SetLabel(wxEmptyString);
+        assign_button_->Enable(false);
+        return;
+    }
+
+    const int command = config_shortcuts_.CommandForInput(user_input);
+    if (command == 0) {
+        // No existing assignment.
+        currently_assigned_label_->SetLabel(wxEmptyString);
+    } else {
+        // Existing assignment, inform the user.
+        const auto iter = command_to_item_id_.find(command);
+        assert(iter != command_to_item_id_.end());
+        currently_assigned_label_->SetLabel(
+            static_cast<CommandTreeItemData*>(tree_->GetItemData(iter->second))->assigned_string());
+    }
+
+    assign_button_->Enable(true);
+}
+
+void AccelConfig::PopulateCurrentKeys() {
+    const int previous_selection = current_keys_->GetSelection();
+    current_keys_->Clear();
+
+    if (selected_command_ == 0) {
+        return;
+    }
+
+    // Populate `current_keys`.
+    int new_keys_count = 0;
+    for (const auto& user_input : config_shortcuts_.InputsForCommand(selected_command_)) {
+        current_keys_->Append(user_input.ToLocalizedString(), new UserInputClientData(user_input));
+        new_keys_count++;
+    }
+
+    // Reset the selection accordingly.
+    if (previous_selection == wxNOT_FOUND || new_keys_count == 0) {
+        current_keys_->SetSelection(wxNOT_FOUND);
+        remove_button_->Enable(false);
+    } else {
+        current_keys_->SetSelection(std::min(previous_selection, new_keys_count - 1));
+        remove_button_->Enable(true);
+    }
+
+}
+
+}  // namespace dialogs

src/wx/dialogs/accel-config.h

@@ -0,0 +1,83 @@
+#ifndef VBAM_WX_DIALOGS_ACCEL_CONFIG_H_
+#define VBAM_WX_DIALOGS_ACCEL_CONFIG_H_
+
+#include <unordered_map>
+
+#include <wx/dialog.h>
+#include <wx/treectrl.h>
+
+#include "config/shortcuts.h"
+#include "widgets/keep-on-top-styler.h"
+
+// Forward declarations.
+class wxControl;
+class wxListBox;
+class wxMenu;
+class wxMenuBar;
+class wxWindow;
+
+namespace widgets {
+class UserInputCtrl;
+}
+
+namespace dialogs {
+
+// Manages the shortcuts editor dialog.
+class AccelConfig : public wxDialog {
+public:
+    static AccelConfig* NewInstance(wxWindow* parent, wxMenuBar* menu_bar, wxMenu* recents);
+
+    ~AccelConfig() override = default;
+
+private:
+    // The constructor is private so initialization has to be done via the
+    // static method. This is because this class is destroyed when its
+    // owner, `parent` is destroyed. This prevents accidental deletion.
+    AccelConfig(wxWindow* parent, wxMenuBar* menu_bar, wxMenu* recents);
+
+    // Re-initializes the configuration.
+    void OnDialogShown(wxShowEvent& ev);
+
+    // On OK, saves the global shortcuts.
+    void OnValidate(wxCommandEvent& ev);
+
+    // Fills in the key list.
+    void OnCommandSelected(wxTreeEvent& ev);
+
+    // after selecting a key in key list, enable Remove button
+    void OnKeySelected(wxCommandEvent& ev);
+
+    // remove selected binding
+    void OnRemoveBinding(wxCommandEvent& ev);
+
+    // wipe out all user bindings
+    void OnResetAll(wxCommandEvent& ev);
+
+    // remove old key binding, add new key binding, and update GUI
+    void OnAssignBinding(wxCommandEvent& ev);
+
+    // update curas and maybe enable asb
+    // Called when the input text box is updated. Finds current binding.
+    void OnKeyInput(wxCommandEvent& ev);
+
+    // Helper method to populate `current_keys_` whenever the configuration or
+    // selection has changed.
+    void PopulateCurrentKeys();
+
+    wxTreeCtrl* tree_;
+    wxListBox* current_keys_;
+    wxWindow* assign_button_;
+    wxWindow* remove_button_;
+    widgets::UserInputCtrl* key_input_;
+    wxControl* currently_assigned_label_;
+    std::unordered_map<int, wxTreeItemId> command_to_item_id_;
+
+    config::Shortcuts config_shortcuts_;
+    int selected_command_ = 0;
+
+    const widgets::KeepOnTopStyler keep_on_top_styler_;
+};
+
+}  // namespace dialogs
+
+#endif  // VBAM_WX_DIALOGS_ACCEL_CONFIG_H_

src/wx/guiinit.cpp

@@ -38,6 +38,7 @@
 #include "config/option-proxy.h"
 #include "config/option.h"
 #include "config/user-input.h"
+#include "dialogs/accel-config.h"
 #include "dialogs/directories-config.h"
 #include "dialogs/display-config.h"
 #include "dialogs/game-boy-config.h"
@@ -1636,374 +1637,6 @@ public:
     }
 } JoyPadConfigHandler[4];
 
-// this is the cmd table index for the accel tree ctrl
-// one of the "benefits" of using TreeItemData is that we have to
-// malloc them all, because treectrl destructor will free them all
-// that means we can't use e.g. a single static table of len ncmds
-class TreeInt : public wxTreeItemData {
-public:
-    TreeInt(int i)
-        : wxTreeItemData()
-    {
-        val = i;
-    }
-    int val;
-};
-
-// Convert a tree selection ID to a name
-// root
-//   parent
-//     item
-static bool treeid_to_name(int id, wxString& name, wxTreeCtrl* tc,
-    const wxTreeItemId& parent, int lev = 0)
-{
-    wxTreeItemIdValue cookie;
-
-    for (wxTreeItemId tid = tc->GetFirstChild(parent, cookie); tid.IsOk();
-         tid = tc->GetNextChild(parent, cookie)) {
-        const TreeInt* ti = static_cast<const TreeInt*>(tc->GetItemData(tid));
-
-        if (ti && ti->val == id) {
-            name = wxString(wxT(' '), 2 * lev) + tc->GetItemText(tid);
-            return true;
-        }
-
-        if (treeid_to_name(id, name, tc, tid, lev + 1)) {
-            name = wxString(wxT(' '), 2 * lev) + tc->GetItemText(tid) + wxT('\n') + name;
-            return true;
-        }
-    }
-
-    return false;
-}
-
-// for sorting accels by command ID
-static bool cmdid_lt(const wxAcceleratorEntryUnicode& a, const wxAcceleratorEntryUnicode& b)
-{
-    return a.GetCommand() < b.GetCommand();
-}
-
-class UserInputClientData : public wxClientData {
-public:
-    explicit UserInputClientData(const config::UserInput& user_input) : user_input_(user_input) {}
-    explicit UserInputClientData(config::UserInput&& user_input)
-        : user_input_(std::move(user_input)) {}
-    ~UserInputClientData() override = default;
-
-    const config::UserInput& user_input() const { return user_input_; }
-
-private:
-    const config::UserInput user_input_;
-};
-
-// manage the accel editor dialog
-static class AccelConfig_t : public wxEvtHandler {
-public:
-    wxTreeCtrl* tc;
-    wxControlWithItems* lb;
-    wxAcceleratorEntry_v user_accels, accels;
-    wxWindow *asb, *remb;
-    widgets::UserInputCtrl* key;
-    wxControl* curas;
-
-    // since this is not the actual dialog, derived from wxDialog, which is
-    // the normal way of doing things, do init on the show event instead of
-    // constructor
-    void Init(wxShowEvent& ev) {
-        ev.Skip();
-
-        if (!ev.IsShown())
-            return;
-
-        lb->Clear();
-        tc->Unselect();
-        tc->ExpandAll();
-        user_accels = gopts.accels;
-        asb->Enable(false);
-        remb->Enable(false);
-        curas->SetLabel("");
-        accels = wxGetApp().frame->get_accels(user_accels);
-    }
-
-    // on OK, save the accels in gopts
-    void Set(wxCommandEvent& ev)
-    {
-        // opts.cpp assumes that gopts.accels entries with same command ID
-        // are contiguous, so sort first
-        std::sort(gopts.accels.begin(), gopts.accels.end(), cmdid_lt);
-        gopts.accels = user_accels;
-        wxGetApp().frame->set_global_accels();
-        ev.Skip();
-    }
-
-    // After selecting item in command list, fill in key list
-    // and maybe enable asb
-    void CommandSel(wxTreeEvent& ev)
-    {
-        // wxTreeCtrl *tc = wxStaticCast(evt.GetEventObject(), wxTreeCtrl);
-        // can't use wxStaticCast; wxTreeItemData does not derive from wxObject
-        const TreeInt* id = static_cast<const TreeInt*>(tc->GetItemData(ev.GetItem()));
-
-        if (!id) {
-            ev.Veto();
-            return;
-        }
-
-        if (ev.GetEventType() == wxEVT_COMMAND_TREE_SEL_CHANGING) {
-            ev.Skip();
-            return;
-        }
-
-        lb->Clear();
-        remb->Enable(false);
-        asb->Enable(!key->inputs().empty());
-        int cmd = id->val;
-
-        for (size_t i = 0; i < accels.size(); ++i) {
-            if (accels[i].GetCommand() == cmdtab[cmd].cmd_id) {
-                config::UserInput input(accels[i].GetKeyCode(), accels[i].GetFlags(), accels[i].GetJoystick());
-                lb->Append(input.ToLocalizedString(), new UserInputClientData(std::move(input)));
-            }
-        }
-    }
-
-    // after selecting a key in key list, enable Remove button
-    void KeySel(wxCommandEvent& ev)
-    {
-        (void)ev; // unused params
-        remb->Enable(lb->GetSelection() != wxNOT_FOUND);
-    }
-
-    // remove selected binding
-    void Remove(wxCommandEvent& ev)
-    {
-        (void)ev; // unused params
-        int lsel = lb->GetSelection();
-
-        if (lsel == wxNOT_FOUND)
-            return;
-
-        wxString selstr = lb->GetString(lsel);
-        const config::UserInput& input =
-            static_cast<const UserInputClientData*>(lb->GetClientObject(lsel))->user_input();
-        int selmod = input.mod();
-        int selkey = input.key();
-        int seljoy = input.joy();
-
-        remb->Enable(false);
-
-        // if this key is currently in the shortcut field, clear out curas
-        if (selstr == key->GetValue())
-            curas->SetLabel("");
-
-        lb->Delete(lsel);
-
-        // first drop from user accels, if applicable
-        for (wxAcceleratorEntry_v::iterator i = user_accels.begin(); i < user_accels.end(); ++i)
-            if (i->GetFlags() == selmod && i->GetKeyCode() == selkey && i->GetJoystick() == seljoy) {
-                user_accels.erase(i);
-                break;
-            }
-
-        // if it's a system accel, disable by assigning to NOOP
-        wxAcceleratorEntry_v& sys_accels = wxGetApp().frame->sys_accels;
-
-        for (size_t i = 0; i < sys_accels.size(); i++)
-            if (sys_accels[i].GetFlags() == selmod && sys_accels[i].GetKeyCode() == selkey &&
-                sys_accels[i].GetJoystick() == seljoy) {
-                wxAcceleratorEntryUnicode ne(seljoy, selmod, selkey, XRCID("NOOP"));
-                user_accels.push_back(ne);
-            }
-
-        // finally, remove from accels instead of recomputing
-        for (wxAcceleratorEntry_v::iterator i = accels.begin(); i < accels.end(); ++i)
-            if (i->GetFlags() == selmod && i->GetKeyCode() == selkey &&
-                i->GetJoystick() == seljoy) {
-                accels.erase(i);
-                break;
-            }
-    }
-
-    // wipe out all user bindings
-    void ResetAll(wxCommandEvent& ev)
-    {
-        (void)ev; // unused params
-        if (user_accels.empty() ||
-            wxMessageBox(_("This will clear all user-defined accelerators. Are you sure?"),
-                         _("Confirm"), wxYES_NO) != wxYES)
-            return;
-
-        user_accels.clear();
-        accels = wxGetApp().frame->sys_accels;
-        tc->Unselect();
-        lb->Clear();
-        // rather than recomputing curas, just clear it
-        key->Clear();
-        curas->SetLabel("");
-    }
-
-    // remove old key binding, add new key binding, and update GUI
-    void Assign(wxCommandEvent& ev)
-    {
-        (void)ev; // unused params
-        wxTreeItemId csel = tc->GetSelection();
-        const std::set<config::UserInput>& inputs = key->inputs();
-
-        if (!csel.IsOk() || inputs.empty())
-            return;
-
-        assert(inputs.size() == 1);
-
-        const config::UserInput& input = *inputs.begin();
-        const int acmod = input.mod();
-        const int ackey = input.key();
-        const int acjoy = input.joy();
-
-        for (unsigned int i = 0; i < lb->GetCount(); i++) {
-            if (static_cast<UserInputClientData*>(lb->GetClientObject(i))->user_input() == input) {
-                // Ignore attempts to add twice.
-                return;
-            }
-        }
-
-        lb->Append(input.ToLocalizedString(), new UserInputClientData(input));
-
-        // first drop from user accels, if applicable
-        for (wxAcceleratorEntry_v::iterator i = user_accels.begin(); i < user_accels.end(); ++i)
-            if (i->GetFlags() == acmod && i->GetKeyCode() == ackey && i->GetJoystick() == acjoy) {
-                user_accels.erase(i);
-                break;
-            }
-
-        // then assign to this command
-        const TreeInt* id = static_cast<const TreeInt*>(tc->GetItemData(csel));
-        wxAcceleratorEntryUnicode ne(input, cmdtab[id->val].cmd_id);
-        user_accels.push_back(ne);
-
-        // now assigned to this cmd...
-        wxString lab;
-        treeid_to_name(id->val, lab, tc, tc->GetRootItem());
-        curas->SetLabel(lab);
-        // finally, instead of recomputing accels, just append new accel
-        accels.push_back(ne);
-    }
-
-    // update curas and maybe enable asb
-    void CheckKey(wxCommandEvent& ev)
-    {
-        (void)ev; // unused params
-        const auto& inputs = key->inputs();
-
-        if (inputs.empty()) {
-            curas->SetLabel("");
-            asb->Enable(false);
-            return;
-        }
-
-        assert(inputs.size() == 1);
-        const config::UserInput input = *inputs.begin();
-
-        const int ackey = input.key();
-        const int acmod = input.mod();
-        const int acjoy = input.joy();
-
-        asb->Enable(tc->GetSelection().IsOk());
-        int cmd = -1;
-
-        for (size_t i = 0; i < accels.size(); i++)
-            if (accels[i].GetFlags() == acmod && accels[i].GetKeyCode() == ackey &&
-                accels[i].GetJoystick() == acjoy) {
-                int cmdid = accels[i].GetCommand();
-                for (cmd = 0; cmd < ncmds; cmd++)
-                    if (cmdid == cmdtab[cmd].cmd_id)
-                        break;
-
-                break;
-            }
-
-        if (cmd < 0 || cmdtab[cmd].cmd_id == XRCID("NOOP")) {
-            curas->SetLabel("");
-            return;
-        }
-
-        wxString lab;
-        treeid_to_name(cmd, lab, tc, tc->GetRootItem());
-        curas->SetLabel(lab);
-    }
-} accel_config_handler;
-
-// build initial accel tree control from menu
-void MainFrame::add_menu_accels(wxTreeCtrl* tc, wxTreeItemId& parent, wxMenu* menu)
-{
-    wxMenuItemList mil = menu->GetMenuItems();
-
-    for (wxMenuItemList::iterator mi = mil.begin(); mi != mil.end(); ++mi) {
-        if ((*mi)->IsSeparator()) {
-            tc->AppendItem(parent, wxT("-----"));
-        } else if ((*mi)->IsSubMenu()) {
-            wxTreeItemId id = tc->AppendItem(parent, (*mi)->GetItemLabelText());
-            add_menu_accels(tc, id, (*mi)->GetSubMenu());
-
-            if ((*mi)->GetSubMenu() == recent) {
-                for (int i = wxID_FILE1; i <= wxID_FILE10; i++) {
-                    int cmdid;
-
-                    for (cmdid = 0; cmdid < ncmds; cmdid++)
-                        if (cmdtab[cmdid].cmd_id == i)
-                            break;
-
-                    TreeInt* val = new TreeInt(cmdid);
-                    tc->AppendItem(id, cmdtab[cmdid].name, -1, -1, val);
-                }
-            }
-        } else {
-            int mid = (*mi)->GetId();
-
-            if (mid >= wxID_FILE1 && mid <= wxID_FILE10)
-                continue;
-
-            int cmdid;
-
-            for (cmdid = 0; cmdid < ncmds; cmdid++)
-                if (cmdtab[cmdid].cmd_id == mid)
-                    break;
-
-            if (cmdid == ncmds)
-                continue; // bad menu item; should inform user really
-
-            TreeInt* val = new TreeInt(cmdid);
-            // ugh.  There has to be a better way...
-            // perhaps make XRCID ranges a requirement for load/save st?
-            // but then if the user overides main menu, that req. is broken..
-            wxString txt = (*mi)->GetItemLabelText();
-
-            // we could probably have a global hashmap:
-            // cmdtab[i].cmd -> cmdtab[i]
-            for (int i = 0; i < 10; i++) {
-                wxString slot;
-                if (*mi == loadst_mi[i]) {
-                    slot.Printf(wxT("LoadGame%02d"), wxAtoi(txt));
-                }
-                else if (*mi == savest_mi[i]) {
-                    slot.Printf(wxT("SaveGame%02d"), wxAtoi(txt));
-                }
-                else {
-                    continue;
-                }
-                for (int j = 0; j < ncmds; ++j) {
-                    if (cmdtab[j].cmd == slot) {
-                        txt = cmdtab[j].name;
-                        break;
-                    }
-                }
-                // no need to look further
-                break;
-            }
-            tc->AppendItem(parent, txt, -1, -1, val);
-        }
-    }
-}
-
 // manage throttle spinctrl/canned setting choice interaction
 static class ThrottleCtrl_t : public wxEvtHandler {
 public:
@@ -2228,113 +1861,6 @@ T* GetValidatedChild(wxWindow* parent, const char* name, V validator)
     return child;
 }
 
-wxAcceleratorEntry_v MainFrame::get_accels(wxAcceleratorEntry_v user_accels)
-{
-    // set global accelerators
-    // first system
-    wxAcceleratorEntry_v accels = sys_accels;
-
-    // then user overrides
-    // silently keep only last defined binding
-    // same horribly inefficent O(n*m) search for duplicates as above..
-    for (size_t i = 0; i < user_accels.size(); i++) {
-        const wxAcceleratorEntryUnicode& ae = user_accels[i];
-
-        for (wxAcceleratorEntry_v::iterator e = accels.begin(); e < accels.end(); ++e)
-            if (ae.GetFlags() == e->GetFlags() && ae.GetKeyCode() == e->GetKeyCode() &&
-                ae.GetJoystick() == e->GetJoystick()) {
-                accels.erase(e);
-                break;
-            }
-
-        accels.push_back(ae);
-    }
-
-    return accels;
-}
-
-void MainFrame::set_global_accels()
-{
-    wxAcceleratorEntry_v accels = get_accels(gopts.accels);
-    // this is needed on Wine/win32 to support accels for close & quit
-    wxGetApp().accels = accels;
-
-    // Update menus; this probably takes the longest
-    // as a side effect, any system-defined accels that weren't already in
-    // the menus will be added now
-
-    // first, zero out menu item on all accels
-    std::set<wxJoystick> needed_joysticks;
-    for (size_t i = 0; i < accels.size(); ++i) {
-        if (accels[i].GetJoystick() != 0) {
-            needed_joysticks.insert(
-                wxJoystick::FromLegacyPlayerIndex(accels[i].GetJoystick()));
-        }
-    }
-    joy.PollJoysticks(needed_joysticks);
-
-    // yet another O(n*m) loop.  I really ought to sort the accel arrays
-    for (int i = 0; i < ncmds; i++) {
-        wxMenuItem* mi = cmdtab[i].mi;
-
-        if (!mi)
-            continue;
-
-        // only *last* accelerator is made visible in menu (non-unicode)
-        // and is flagged as such by setting menu item in accel
-        // the last is chosen so menu overrides non-menu and user overrides
-        // system
-        int cmd = cmdtab[i].cmd_id;
-        if (cmd == XRCID("NOOP")) continue;
-        int last_accel = -1;
-
-        for (size_t j = 0; j < accels.size(); ++j)
-            if (cmd == accels[j].GetCommand())
-                last_accel = j;
-
-        if (last_accel >= 0) {
-            DoSetAccel(mi, &accels[last_accel]);
-            accels[last_accel].Set(accels[last_accel].GetJoystick(), accels[last_accel].GetFlags(),
-                                   accels[last_accel].GetKeyCode(), accels[last_accel].GetCommand(),
-                                   mi);
-        } else {
-            // clear out user-cleared menu items
-            DoSetAccel(mi, NULL);
-        }
-    }
-
-    // Finally, install a global accelerator table for any non-menu accels
-    int len = 0;
-
-    for (size_t i = 0; i < accels.size(); i++)
-        if (!accels[i].GetMenuItem())
-            len++;
-
-    if (len) {
-        wxAcceleratorEntry tab[1000];
-
-        for (size_t i = 0, j = 0; i < accels.size(); i++)
-            if (!accels[i].GetMenuItem())
-                tab[j++] = accels[i];
-
-        wxAcceleratorTable atab(len, tab);
-        // set the table on the panel, where focus usually is
-        // otherwise accelerators are lost sometimes
-        panel->SetAcceleratorTable(atab);
-    } else
-        panel->SetAcceleratorTable(wxNullAcceleratorTable);
-
-    // save recent accels
-    for (int i = 0; i < 10; i++)
-        recent_accel[i] = wxAcceleratorEntryUnicode();
-
-    for (size_t i = 0; i < accels.size(); i++)
-        if (accels[i].GetCommand() >= wxID_FILE1 && accels[i].GetCommand() <= wxID_FILE10)
-            recent_accel[accels[i].GetCommand() - wxID_FILE1] = accels[i];
-
-    SetRecentAccels();
-}
-
 void MainFrame::MenuOptionBool(const wxString& menuName, bool field)
 {
     int id = wxXmlResource::GetXRCID(menuName);
@@ -2456,10 +1982,6 @@ bool MainFrame::BindControls()
     // NOOP if no status area
     SetStatusText(wxT(""));
 
-    // Prepare system accel table
-    for (int i = 0; i < num_def_accels; i++)
-        sys_accels.push_back(default_accels[i]);
-
     wxMenuBar* menubar = GetMenuBar();
     ctx_menu = NULL;
 
@@ -2500,8 +2022,6 @@ bool MainFrame::BindControls()
             if (cmdtab[i].mask_flags & (CMDEN_SREC | CMDEN_NSREC | CMDEN_VREC | CMDEN_NVREC)) {
                 if (mi)
                     mi->GetMenu()->Remove(mi);
-
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
             }
@@ -2512,8 +2032,6 @@ bool MainFrame::BindControls()
             if (cmdtab[i].cmd_id == XRCID("Logging")) {
                 if (mi)
                     mi->GetMenu()->Remove(mi);
-
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
             }
@@ -2528,8 +2046,6 @@ bool MainFrame::BindControls()
                ) {
                 if (mi)
                     mi->GetMenu()->Remove(mi);
-
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
             }
@@ -2540,8 +2056,6 @@ bool MainFrame::BindControls()
             if (cmdtab[i].cmd_id == XRCID("LanLink") || cmdtab[i].cmd_id == XRCID("LinkType0Nothing") || cmdtab[i].cmd_id == XRCID("LinkType1Cable") || cmdtab[i].cmd_id == XRCID("LinkType2Wireless") || cmdtab[i].cmd_id == XRCID("LinkType3GameCube") || cmdtab[i].cmd_id == XRCID("LinkType4Gameboy") || cmdtab[i].cmd_id == XRCID("LinkAuto") || cmdtab[i].cmd_id == XRCID("SpeedOn") || cmdtab[i].cmd_id == XRCID("LinkProto") || cmdtab[i].cmd_id == XRCID("LinkConfigure")) {
                 if (mi)
                     mi->GetMenu()->Remove(mi);
-
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
             }
@@ -2552,8 +2066,6 @@ bool MainFrame::BindControls()
             if (cmdtab[i].cmd_id == XRCID("LinkType2Wireless")) {
                 if (mi)
                     mi->GetMenu()->Remove(mi);
-
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
             }
@@ -2561,27 +2073,25 @@ bool MainFrame::BindControls()
 #endif
 #ifdef NO_DEBUGGER
 
-	    if (cmdtab[i].cmd_id == XRCID("DebugGDBBreak") || cmdtab[i].cmd_id == XRCID("DebugGDBDisconnect") || cmdtab[i].cmd_id == XRCID("DebugGDBBreakOnLoad") || cmdtab[i].cmd_id == XRCID("DebugGDBPort"))
+            if (cmdtab[i].cmd_id == XRCID("DebugGDBBreak") || cmdtab[i].cmd_id == XRCID("DebugGDBDisconnect") || cmdtab[i].cmd_id == XRCID("DebugGDBBreakOnLoad") || cmdtab[i].cmd_id == XRCID("DebugGDBPort"))
-	    {
+            {
-		if (mi)
+                if (mi)
-		{
+                {
-		    mi->GetMenu()->Enable(mi->GetId(), false);
+                    mi->GetMenu()->Enable(mi->GetId(), false);
-		    //mi->GetMenu()->Remove(mi);
+                    //mi->GetMenu()->Remove(mi);
-		}
+                }
-		cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
-	    }
+            }
 #endif
 #if defined(NO_ONLINEUPDATES)
-	    if (cmdtab[i].cmd_id == XRCID("UpdateEmu"))
+            if (cmdtab[i].cmd_id == XRCID("UpdateEmu"))
-	    {
+            {
                 if (mi)
-		    mi->GetMenu()->Remove(mi);
+                    mi->GetMenu()->Remove(mi);
-                cmdtab[i].cmd_id = XRCID("NOOP");
                 cmdtab[i].mi = NULL;
                 continue;
-	    }
+            }
 #endif
 
             if (mi) {
@@ -2594,54 +2104,6 @@ bool MainFrame::BindControls()
                     mi->SetItemLabel(wxGetStockLabel(mi->GetId(),
                         wxSTOCK_WITH_MNEMONIC | wxSTOCK_WITH_ACCELERATOR));
 
-                // add accelerator to global accel table
-                wxAcceleratorEntry* a = mi->GetAccel();
-
-                if (a) {
-                    a->Set(a->GetFlags(), a->GetKeyCode(), cmdtab[i].cmd_id, mi);
-
-                    // only add it if not already there
-                    for (wxAcceleratorEntry_v::iterator e = sys_accels.begin();
-                         e < sys_accels.end(); ++e)
-                        if (a->GetFlags() == e->GetFlags() && a->GetKeyCode() == e->GetKeyCode()) {
-                            if (e->GetMenuItem()) {
-                                wxLogInfo(_("Duplicate menu accelerator: %s for %s and %s; keeping "
-                                            "first"),
-                                          config::UserInput(a->GetKeyCode(), a->GetFlags())
-                                              .ToLocalizedString(),
-                                          e->GetMenuItem()->GetItemLabelText().c_str(),
-                                          mi->GetItemLabelText().c_str());
-                                delete a;
-                                a = 0;
-                            } else {
-                                if (e->GetCommand() != a->GetCommand()) {
-                                    int cmd;
-
-                                    for (cmd = 0; cmd < ncmds; cmd++)
-                                        if (cmdtab[cmd].cmd_id == e->GetCommand())
-                                            break;
-
-                                    wxLogInfo(_("Menu accelerator %s for %s overrides default for "
-                                                "%s; keeping menu"),
-                                              config::UserInput(a->GetKeyCode(), a->GetFlags())
-                                                  .ToLocalizedString(),
-                                              mi->GetItemLabelText().c_str(),
-                                              cmdtab[cmd].cmd.c_str());
-                                }
-
-                                sys_accels.erase(e);
-                            }
-
-                            break;
-                        }
-
-                    if (a)
-                        sys_accels.push_back(wxAcceleratorEntryUnicode(a));
-                    else
-                        // strip from label so user isn't confused
-                        DoSetAccel(mi, NULL);
-                }
-
                 // store checkable items
                 if (mi->IsCheckable()) {
                     checkable_mi_t cmi = { cmdtab[i].cmd_id, mi, 0, 0 };
@@ -2760,15 +2222,16 @@ bool MainFrame::BindControls()
         MenuOptionIntRadioValue("LinkType4Gameboy", gopts.gba_link_type, 4);
     }
 
-    for (size_t i = 0; i < checkable_mi.size(); i++)
+    for (size_t i = 0; i < checkable_mi.size(); i++) {
         if (!checkable_mi[i].initialized) {
             wxLogError(_("Invalid menu item %s; removing"),
                 checkable_mi[i].mi->GetItemLabelText().c_str());
             checkable_mi[i].mi->GetMenu()->Remove(checkable_mi[i].mi);
             checkable_mi[i].mi = NULL;
         }
+    }
 
-    set_global_accels();
+    ResetMenuAccelerators();
 
     // preload and verify all resource dialogs
     // this will take init time and memory, but catches errors in xrc sooner
@@ -3385,111 +2848,7 @@ bool MainFrame::BindControls()
             d->Fit();
         }
 #endif
-        d = LoadXRCDialog("AccelConfig");
+        dialogs::AccelConfig::NewInstance(this, menubar, recent);
-        {
-            wxTreeCtrl* tc;
-            tc = SafeXRCCTRL<wxTreeCtrl>(d, "Commands");
-            accel_config_handler.tc = tc;
-            wxControlWithItems* lb;
-            lb = SafeXRCCTRL<wxControlWithItems>(d, "Current");
-            accel_config_handler.lb = lb;
-            accel_config_handler.asb = SafeXRCCTRL<wxButton>(d, "Assign");
-            accel_config_handler.remb = SafeXRCCTRL<wxButton>(d, "Remove");
-            accel_config_handler.key = SafeXRCCTRL<widgets::UserInputCtrl>(d, "Shortcut");
-            accel_config_handler.curas = SafeXRCCTRL<wxControl>(d, "AlreadyThere");
-            accel_config_handler.key->MoveBeforeInTabOrder(accel_config_handler.asb);
-            wxTreeItemId rid = tc->AddRoot(wxT("root"));
-
-            if (menubar) {
-                wxTreeItemId mid = tc->AppendItem(rid, _("Menu commands"));
-
-                for (size_t i = 0; i < menubar->GetMenuCount(); i++) {
-#if wxCHECK_VERSION(2, 8, 8)
-                    wxTreeItemId id = tc->AppendItem(mid, menubar->GetMenuLabelText(i));
-#else
-                    // 2.8.4 has no equivalent for GetMenuLabelText()
-                    wxString txt = menubar->GetMenuLabel(i);
-                    txt.Replace(wxT("&"), wxT(""));
-                    wxTreeItemId id = tc->AppendItem(mid, txt);
-#endif
-                    add_menu_accels(tc, id, menubar->GetMenu(i));
-                }
-            }
-
-            wxTreeItemId oid;
-            int noop_id = XRCID("NOOP");
-
-            for (int i = 0; i < ncmds; i++) {
-                if (cmdtab[i].mi || (recent && cmdtab[i].cmd_id >= wxID_FILE1 && cmdtab[i].cmd_id <= wxID_FILE10) || cmdtab[i].cmd_id == noop_id)
-                    continue;
-
-                if (!oid.IsOk())
-                    oid = tc->AppendItem(rid, _("Other commands"));
-
-                TreeInt* val = new TreeInt(i);
-                tc->AppendItem(oid, cmdtab[i].name, -1, -1, val);
-            }
-
-            tc->ExpandAll();
-            // FIXME: make this actually show the entire line w/o scrolling
-            // BestSize cuts off on rhs; MaxSize is completely invalid
-            wxSize sz = tc->GetBestSize();
-
-            if (sz.GetHeight() > 200)
-                sz.SetHeight(200);
-
-            tc->SetSize(sz);
-            sz.SetWidth(-1); // maybe allow it to become bigger
-            tc->SetSizeHints(sz, sz);
-            int w, h;
-            lb->GetTextExtent(wxT("CTRL-ALT-SHIFT-ENTER"), &w, &h);
-            sz.Set(w, h);
-            lb->SetMinSize(sz);
-            sz.Set(0, 0);
-            wxControl* curas = accel_config_handler.curas;
-
-            for (int i = 0; i < ncmds; i++) {
-                wxString labs;
-                treeid_to_name(i, labs, tc, tc->GetRootItem());
-                curas->GetTextExtent(labs, &w, &h);
-
-                if (w > sz.GetWidth())
-                    sz.SetWidth(w);
-
-                if (h > sz.GetHeight())
-                    sz.SetHeight(h);
-            }
-
-            curas->SetSize(sz);
-            curas->SetSizeHints(sz);
-            tc->Connect(wxEVT_COMMAND_TREE_SEL_CHANGING,
-                wxTreeEventHandler(AccelConfig_t::CommandSel),
-                NULL, &accel_config_handler);
-            tc->Connect(wxEVT_COMMAND_TREE_SEL_CHANGED,
-                wxTreeEventHandler(AccelConfig_t::CommandSel),
-                NULL, &accel_config_handler);
-            d->Connect(wxEVT_SHOW, wxShowEventHandler(AccelConfig_t::Init),
-                NULL, &accel_config_handler);
-            d->Connect(wxID_OK, wxEVT_COMMAND_BUTTON_CLICKED,
-                wxCommandEventHandler(AccelConfig_t::Set),
-                NULL, &accel_config_handler);
-            d->Connect(XRCID("Assign"), wxEVT_COMMAND_BUTTON_CLICKED,
-                wxCommandEventHandler(AccelConfig_t::Assign),
-                NULL, &accel_config_handler);
-            d->Connect(XRCID("Remove"), wxEVT_COMMAND_BUTTON_CLICKED,
-                wxCommandEventHandler(AccelConfig_t::Remove),
-                NULL, &accel_config_handler);
-            d->Connect(XRCID("ResetAll"), wxEVT_COMMAND_BUTTON_CLICKED,
-                wxCommandEventHandler(AccelConfig_t::ResetAll),
-                NULL, &accel_config_handler);
-            lb->Connect(wxEVT_COMMAND_LISTBOX_SELECTED,
-                wxCommandEventHandler(AccelConfig_t::KeySel),
-                NULL, &accel_config_handler);
-            d->Connect(XRCID("Shortcut"), wxEVT_COMMAND_TEXT_UPDATED,
-                wxCommandEventHandler(AccelConfig_t::CheckKey),
-                NULL, &accel_config_handler);
-            d->Fit();
-        }
     } catch (std::exception& e) {
         wxLogError(wxString::FromUTF8(e.what()));
         return false;

src/wx/opts.cpp

@@ -110,98 +110,6 @@ uint32_t LoadUnsignedOption(wxConfigBase* cfg,
 
 opts_t gopts;
 
-// having the standard menu accels here means they will work even without menus
-const wxAcceleratorEntryUnicode default_accels[] = {
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'C', XRCID("CheatsList")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'N', XRCID("NextFrame")),
-    // some ports add ctrl-q anyway, so may as well make it official
-    // maybe make alt-f4 universal as well...
-    // FIXME: ctrl-Q does not work on wxMSW
-    // FIXME: esc does not work on wxMSW
-
-    // this was annoying people A LOT #334
-    //wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_ESCAPE, wxID_EXIT),
-
-    // this was annoying people #298
-    //wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'X', wxID_EXIT),
-
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'Q', wxID_EXIT),
-    // FIXME: ctrl-W does not work on wxMSW
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'W', wxID_CLOSE),
-    // load most recent is more commonly used than load other
-    //wxAcceleratorEntry(wxMOD_CMD, 'L', XRCID("Load")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'L', XRCID("LoadGameRecent")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F1, XRCID("LoadGame01")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F2, XRCID("LoadGame02")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F3, XRCID("LoadGame03")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F4, XRCID("LoadGame04")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F5, XRCID("LoadGame05")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F6, XRCID("LoadGame06")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F7, XRCID("LoadGame07")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F8, XRCID("LoadGame08")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F9, XRCID("LoadGame09")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_F10, XRCID("LoadGame10")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_PAUSE, XRCID("Pause")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'P', XRCID("Pause")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'R', XRCID("Reset")),
-    // add shortcuts for original size multiplier #415
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '1', XRCID("SetSize1x")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '2', XRCID("SetSize2x")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '3', XRCID("SetSize3x")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '4', XRCID("SetSize4x")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '5', XRCID("SetSize5x")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, '6', XRCID("SetSize6x")),
-    // save oldest is more commonly used than save other
-    //wxAcceleratorEntry(wxMOD_CMD, 'S', XRCID("Save")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'S', XRCID("SaveGameOldest")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F1, XRCID("SaveGame01")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F2, XRCID("SaveGame02")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F3, XRCID("SaveGame03")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F4, XRCID("SaveGame04")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F5, XRCID("SaveGame05")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F6, XRCID("SaveGame06")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F7, XRCID("SaveGame07")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F8, XRCID("SaveGame08")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F9, XRCID("SaveGame09")),
-    wxAcceleratorEntryUnicode(0, wxMOD_SHIFT, WXK_F10, XRCID("SaveGame10")),
-    // I prefer the SDL ESC key binding
-    //wxAcceleratorEntry(wxMOD_NONE, WXK_ESCAPE, XRCID("ToggleFullscreen"),
-    // alt-enter is more standard anyway
-    wxAcceleratorEntryUnicode(0, wxMOD_ALT, WXK_RETURN, XRCID("ToggleFullscreen")),
-    wxAcceleratorEntryUnicode(0, wxMOD_ALT, '1', XRCID("JoypadAutofireA")),
-    wxAcceleratorEntryUnicode(0, wxMOD_ALT, '2', XRCID("JoypadAutofireB")),
-    wxAcceleratorEntryUnicode(0, wxMOD_ALT, '3', XRCID("JoypadAutofireL")),
-    wxAcceleratorEntryUnicode(0, wxMOD_ALT, '4', XRCID("JoypadAutofireR")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '1', XRCID("VideoLayersBG0")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '2', XRCID("VideoLayersBG1")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '3', XRCID("VideoLayersBG2")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '4', XRCID("VideoLayersBG3")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '5', XRCID("VideoLayersOBJ")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '6', XRCID("VideoLayersWIN0")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '7', XRCID("VideoLayersWIN1")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '8', XRCID("VideoLayersOBJWIN")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'B', XRCID("Rewind")),
-    // following are not in standard menus
-    // FILExx are filled in when recent menu is filled
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F1, wxID_FILE1),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F2, wxID_FILE2),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F3, wxID_FILE3),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F4, wxID_FILE4),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F5, wxID_FILE5),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F6, wxID_FILE6),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F7, wxID_FILE7),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F8, wxID_FILE8),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F9, wxID_FILE9),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, WXK_F10, wxID_FILE10),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, '0', XRCID("VideoLayersReset")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'G', XRCID("ChangeFilter")),
-    wxAcceleratorEntryUnicode(0, wxMOD_CMD, 'I', XRCID("ChangeIFB")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_NUMPAD_ADD, XRCID("IncreaseVolume")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_NUMPAD_SUBTRACT, XRCID("DecreaseVolume")),
-    wxAcceleratorEntryUnicode(0, wxMOD_NONE, WXK_NUMPAD_ENTER, XRCID("ToggleSound"))
-};
-const int num_def_accels = sizeof(default_accels) / sizeof(default_accels[0]);
-
 const std::map<config::GameControl, std::set<config::UserInput>> kDefaultBindings = {
     { config::GameControl(0, config::GameKey::Up), {
         WJKB('W'),
@@ -336,8 +244,6 @@ const std::map<config::GameControl, std::set<config::UserInput>> kDefaultBinding
     { config::GameControl(3, config::GameKey::Gameshark), {}},
 };
 
-wxAcceleratorEntry_v sys_accels;
-
 // This constructor only works with globally allocated gopts.
 opts_t::opts_t()
 {
@@ -570,11 +476,9 @@ void load_opts(bool first_time_launch) {
             if (inputs.empty()) {
                 wxLogWarning(_("Invalid key binding %s for %s"), s.c_str(), kbopt.c_str());
             } else {
-                wxAcceleratorEntry_v val;
                 for (const auto& input : inputs) {
-                    val.push_back(wxAcceleratorEntryUnicode(input, cmdtab[i].cmd_id));
+                    gopts.shortcuts.AssignInputToCommand(input, cmdtab[i].cmd_id);
                 }
-                gopts.accels.insert(gopts.accels.end(), val.begin(), val.end());
             }
         }
     }
@@ -651,72 +555,31 @@ void update_opts() {
         config::GameControlState::Instance().OnGameBindingsChanged();
     }
 
-    // for keyboard, first remove any commands that aren't bound at all
+    cfg->SetPath("/");
-    if (cfg->HasGroup(wxT("/Keyboard"))) {
+    cfg->Flush();
-        cfg->SetPath(wxT("/Keyboard"));
+}
-        wxString s;
-        long entry_idx;
-        wxArrayString item_del;
 
-        for (bool cont = cfg->GetFirstEntry(s, entry_idx); cont;
+void update_shortcut_opts() {
-             cont = cfg->GetNextEntry(s, entry_idx)) {
+    wxConfigBase* cfg = wxConfigBase::Get();
-            const cmditem dummy = new_cmditem(s);
-            cmditem* cmd = std::lower_bound(&cmdtab[0], &cmdtab[ncmds], dummy, cmditem_lt);
-            size_t i;
-
-            for (i = 0; i < gopts.accels.size(); i++)
-                if (gopts.accels[i].GetCommand() == cmd->cmd_id)
-                    break;
-
-            if (i == gopts.accels.size())
-                item_del.push_back(s);
-        }
-
-        for (size_t i = 0; i < item_del.size(); i++)
-            cfg->DeleteEntry(item_del[i]);
-    }
-
-    // then, add/update the commands that are bound
-    // even if only ordering changed, a write will be triggered.
-    // nothing to worry about...
-    if (gopts.accels.size())
-        cfg->SetPath(wxT("/Keyboard"));
-
-    int cmd_id = -1;
-    for (wxAcceleratorEntry_v::iterator i = gopts.accels.begin();
-         i < gopts.accels.end(); ++i) {
-        if (cmd_id == i->GetCommand()) continue;
-        cmd_id = i->GetCommand();
-        int cmd;
 
+    // For shortcuts, it's easier to delete everything and start over.
+    cfg->DeleteGroup("/Keyboard");
+    cfg->SetPath("/Keyboard");
+    for (const auto& iter : gopts.shortcuts.GetConfiguration()) {
+        int cmd = 0;
         for (cmd = 0; cmd < ncmds; cmd++)
-            if (cmdtab[cmd].cmd_id == cmd_id)
+            if (cmdtab[cmd].cmd_id == iter.first)
                 break;
-
+        if (cmd == ncmds) {
-        // NOOP overwrittes commands removed by the user
+            // Command not found. This should never happen.
-        wxString command = cmdtab[cmd].cmd;
+            assert(false);
-        if (cmdtab[cmd].cmd_id == XRCID("NOOP"))
+            continue;
-            command = wxT("NOOP");
-
-        wxAcceleratorEntry_v::iterator j;
-
-        for (j = i + 1; j < gopts.accels.end(); ++j)
-            if (j->GetCommand() != cmd_id)
-                break;
-
-        wxAcceleratorEntry_v nv(i, j);
-        std::set<config::UserInput> user_inputs;
-        for (const auto& accel : nv) {
-            user_inputs.insert(config::UserInput(accel.GetKeyCode(), accel.GetFlags(), accel.GetJoystick()));
         }
-        wxString nvs = config::UserInput::SpanToConfigString(user_inputs);
 
-        if (nvs != cfg->Read(command))
+        cfg->Write(cmdtab[cmd].cmd, iter.second);
-            cfg->Write(command, nvs);
     }
 
-    cfg->SetPath(wxT("/"));
+    cfg->SetPath("/");
-    // recent items are updated separately
     cfg->Flush();
 }
 
@@ -793,28 +656,14 @@ void opt_set(const wxString& name, const wxString& val) {
             return;
         }
 
-        for (auto i = gopts.accels.begin(); i < gopts.accels.end(); ++i)
-            if (i->GetCommand() == cmd->cmd_id) {
-                auto j = i;
-                for (; j < gopts.accels.end(); ++j)
-                    if (j->GetCommand() != cmd->cmd_id)
-                        break;
-                gopts.accels.erase(i, j);
-                break;
-            }
-
         if (!val.empty()) {
-            auto inputs = config::UserInput::FromConfigString(val);
+            const auto inputs = config::UserInput::FromConfigString(val);
-            wxAcceleratorEntry_v aval;
+            if (inputs.empty()) {
-            for (const auto& input : inputs) {
-                aval.push_back(wxAcceleratorEntryUnicode(input, cmd->cmd_id));
-            }
-            if (!aval.size()) {
                 wxLogWarning(_("Invalid key binding %s for %s"), val.c_str(), name.c_str());
-                return;
             }
-
+            for (const auto& input : inputs) {
-            gopts.accels.insert(gopts.accels.end(), aval.begin(), aval.end());
+                gopts.shortcuts.AssignInputToCommand(input, cmd->cmd_id);
+            }
         }
 
         return;

src/wx/opts.h

@@ -7,6 +7,7 @@
 #include <wx/vidmode.h>
 
 #include "config/game-control.h"
+#include "config/shortcuts.h"
 #include "config/user-input.h"
 #include "wxutil.h"
 
@@ -44,7 +45,7 @@ extern struct opts_t {
     int default_stick = 1;
 
     /// Keyboard
-    wxAcceleratorEntry_v accels;
+    config::Shortcuts shortcuts;
 
     /// Core
     int gdb_port = 55555;
@@ -77,9 +78,6 @@ extern struct opts_t {
     // wxWidgets-generated options (opaque)
 } gopts;
 
-extern const wxAcceleratorEntryUnicode default_accels[];
-extern const int num_def_accels;
-
 // call to load config (once)
 // will write defaults for options not present and delete bad opts
 // will also initialize opts[] array translations
@@ -87,6 +85,8 @@ void load_opts(bool first_time_launch);
 // call whenever opt vars change
 // will detect changes and write config if necessary
 void update_opts();
+// Updates the shortcut options.
+void update_shortcut_opts();
 // returns true if option name correct; prints error if val invalid
 void opt_set(const wxString& name, const wxString& val);
 

src/wx/panel.cpp

@@ -191,7 +191,7 @@ void GameArea::LoadGame(const wxString& name)
 
         if (!OPTION(kGenFreezeRecent)) {
             gopts.recent->AddFileToHistory(name);
-            wxGetApp().frame->SetRecentAccels();
+            wxGetApp().frame->ResetRecentAccelerators();
             cfg->SetPath("/Recent");
             gopts.recent->Save(*cfg);
             cfg->SetPath("/");

src/wx/widgets/user-input-ctrl.cpp

@@ -67,6 +67,14 @@ void UserInputCtrl::SetInputs(const std::set<config::UserInput>& inputs) {
     UpdateText();
 }
 
+config::UserInput UserInputCtrl::SingleInput() const {
+    assert(!is_multikey_);
+    if (inputs_.empty()) {
+        return config::UserInput();
+    }
+    return *inputs_.begin();
+}
+
 void UserInputCtrl::Clear() {
     inputs_.clear();
     UpdateText();

src/wx/widgets/user-input-ctrl.h

@@ -47,6 +47,11 @@ public:
     // Sets this control inputs.
     void SetInputs(const std::set<config::UserInput>& inputs);
 
+    // Helper method to return the single input for no multikey UserInputCtrls.
+    // Asserts if `is_multikey_` is true.
+    // Returns an invalid UserInput if no input is currently set.
+    config::UserInput SingleInput() const;
+
     // Returns the inputs set in this control.
     const std::set<config::UserInput>& inputs() const { return inputs_; }
 

src/wx/wxhead.h

@@ -59,41 +59,6 @@ using std::int32_t;
 
 #include "wxutil.h"
 
-static inline void DoSetAccel(wxMenuItem* mi, wxAcceleratorEntryUnicode* acc)
-{
-    // cannot use SDL keybinding as text without wx assertion error
-    if (acc && acc->GetJoystick() != 0) return;
-
-    wxString lab = mi->GetItemLabel();
-    size_t tab = lab.find(wxT('\t'));
-
-    // following short circuit returns are to avoid UI update on no change
-    if (tab == wxString::npos && !acc)
-        return;
-
-    wxString accs;
-
-    if (acc) {
-        // actually, use keyedit's ToString(), as it is more reliable
-        // and doesn't generate wx assertions
-        // accs = acc->ToString();
-        accs = config::UserInput(acc->GetKeyCode(), acc->GetFlags()).ToLocalizedString();
-    }
-
-    if (tab != wxString::npos && accs == lab.substr(tab + 1))
-        return;
-
-    if (tab != wxString::npos)
-        lab.resize(tab);
-
-    if (acc) {
-        lab.append(wxT('\t'));
-        lab.append(accs);
-    }
-
-    mi->SetItemLabel(lab);
-}
-
 // wxrc helpers (for dynamic strings instead of constant)
 #define XRCID_D(str) wxXmlResource::GetXRCID(str)
 //#define XRCCTRL_D(win, id, type) (wxStaticCast((win).FindWindow(XRCID_D(id)), type))
@@ -118,7 +83,4 @@ static inline const wxCharBuffer UTF8(wxString str)
     return str.mb_str(wxConvUTF8);
 }
 
-// by default, only 9 recent items
-#define wxID_FILE10 (wxID_FILE9 + 1)
-
 #endif /* WX_WXHEAD_H */

src/wx/wxutil.cpp

@@ -1,5 +1,4 @@
 #include "wxutil.h"
-#include "../common/contains.h"
 
 int getKeyboardKeyCode(const wxKeyEvent& event) {
     int uc = event.GetUnicodeKey();
@@ -20,27 +19,3 @@ int getKeyboardKeyCode(const wxKeyEvent& event) {
         return event.GetKeyCode();
     }
 }
-
-wxAcceleratorEntryUnicode::wxAcceleratorEntryUnicode(wxAcceleratorEntry* accel)
-    : wxAcceleratorEntryUnicode(0,
-                                accel->GetFlags(),
-                                accel->GetKeyCode(),
-                                accel->GetCommand(),
-                                accel->GetMenuItem()) {}
-
-wxAcceleratorEntryUnicode::wxAcceleratorEntryUnicode(const config::UserInput& input,
-                                                     int cmd,
-                                                     wxMenuItem* item)
-    : wxAcceleratorEntryUnicode(input.joy(), input.mod(), input.key(), cmd, item) {}
-
-wxAcceleratorEntryUnicode::wxAcceleratorEntryUnicode(int joy,
-                                                     int flags,
-                                                     int keyCode,
-                                                     int cmd,
-                                                     wxMenuItem* item)
-    : wxAcceleratorEntry(flags, keyCode, cmd, item), joystick(joy) {}
-
-void wxAcceleratorEntryUnicode::Set(int joy, int flags, int keyCode, int cmd, wxMenuItem* item) {
-    joystick = joy;
-    wxAcceleratorEntry::Set(flags, keyCode, cmd, item);
-}

src/wx/wxutil.h

@@ -1,36 +1,8 @@
 #ifndef _WX_UTIL_H
 #define _WX_UTIL_H
 
-#include <vector>
-
-#include <wx/accel.h>
 #include <wx/event.h>
 
-#include "config/user-input.h"
-
 int getKeyboardKeyCode(const wxKeyEvent& event);
 
-class wxAcceleratorEntryUnicode : public wxAcceleratorEntry
-{
-  public:
-    wxAcceleratorEntryUnicode(wxAcceleratorEntry *accel);
-    wxAcceleratorEntryUnicode(const config::UserInput& input,
-                              int cmd = 0,
-                              wxMenuItem* item = nullptr);
-    wxAcceleratorEntryUnicode(int joy = 0,
-                              int flags = 0,
-                              int keyCode = 0,
-                              int cmd = 0,
-                              wxMenuItem* item = nullptr);
-
-    void Set(int joy, int flags, int keyCode, int cmd, wxMenuItem* item = nullptr);
-
-    int GetJoystick() const { return joystick; };
-
-private:
-    int joystick;
-};
-
-typedef std::vector<wxAcceleratorEntryUnicode> wxAcceleratorEntry_v;
-
 #endif

src/wx/wxvbam.cpp

@@ -48,9 +48,38 @@
 #include "wxhead.h"
 
 namespace {
+
+// Resets the accelerator text for `menu_item` to the first keyboard input.
+void ResetMenuItemAccelerator(wxMenuItem* menu_item) {
+    const wxString old_label = menu_item->GetItemLabel();
+    const size_t tab_index = old_label.find('\t');
+    wxString new_label;
+    new_label = old_label;
+    if (tab_index != wxString::npos) {
+        new_label.resize(tab_index);
+    }
+    std::set<config::UserInput> user_inputs =
+        gopts.shortcuts.InputsForCommand(menu_item->GetId());
+    for (const config::UserInput& user_input : user_inputs) {
+        if (user_input.device() != config::UserInput::Device::Keyboard) {
+            // Cannot use joystick keybinding as text without wx assertion error.
+            continue;
+        }
+
+        new_label.append('\t');
+        new_label.append(user_input.ToLocalizedString());
+        break;
+    }
+
+    if (old_label != new_label) {
+        menu_item->SetItemLabel(new_label);
+    }
+}
+
 static const wxString kOldConfigFileName("vbam.conf");
 static const wxString knewConfigFileName("vbam.ini");
 static const char kDotDir[] = "visualboyadvance-m";
+
 }  // namespace
 
 #ifndef NO_DEBUGGER
@@ -921,43 +950,31 @@ void MainFrame::OnSize(wxSizeEvent& event)
     OPTION(kGeomFullScreen) = IsFullScreen();
 }
 
-int MainFrame::FilterEvent(wxEvent& event)
+int MainFrame::FilterEvent(wxEvent& event) {
-{
+    if (menus_opened || dialog_opened) {
-    if (event.GetEventType() == wxEVT_KEY_DOWN && !menus_opened && !dialog_opened)
+        return wxEventFilter::Event_Skip;
-    {
+    }
-        wxKeyEvent& ke = (wxKeyEvent&)event;
+
-        int keyCode = getKeyboardKeyCode(ke);
+    int command = 0;
-        int keyMod = ke.GetModifiers();
+    if (event.GetEventType() == wxEVT_KEY_DOWN) {
-        wxAcceleratorEntry_v accels = wxGetApp().GetAccels();
+        const wxKeyEvent& key_event = static_cast<wxKeyEvent&>(event);
-        for (size_t i = 0; i < accels.size(); ++i)
+        command = gopts.shortcuts.CommandForInput(config::UserInput(key_event));
-            if (keyCode == accels[i].GetKeyCode() && keyMod == accels[i].GetFlags()
+    } else if (event.GetEventType() == wxEVT_JOY) {
-                && accels[i].GetCommand() != XRCID("NOOP"))
+        const wxJoyEvent& joy_event = static_cast<wxJoyEvent&>(event);
-            {
+        if (joy_event.pressed()) {
-                wxCommandEvent evh(wxEVT_COMMAND_MENU_SELECTED, accels[i].GetCommand());
+            // We ignore "button up" events here.
-                evh.SetEventObject(this);
+            command = gopts.shortcuts.CommandForInput(config::UserInput(joy_event));
-                GetEventHandler()->ProcessEvent(evh);
-                return wxEventFilter::Event_Processed;
         }
     }
-    else if (event.GetEventType() == wxEVT_JOY && !menus_opened && !dialog_opened)
+
-    {
+    if (command == 0) {
-        wxJoyEvent& je = (wxJoyEvent&)event;
+        return wxEventFilter::Event_Skip;
-        if (!je.pressed()) {
-            // joystick button UP
-            return -1;
-        }
-        wxAcceleratorEntry_v accels = wxGetApp().GetAccels();
-        for (size_t i = 0; i < accels.size(); ++i) {
-        if (accels[i].GetJoystick() == je.joystick().player_index() &&
-            accels[i].GetKeyCode() == je.control_index() && accels[i].GetFlags() == je.control()) {
-                wxCommandEvent evh(wxEVT_COMMAND_MENU_SELECTED, accels[i].GetCommand());
-                evh.SetEventObject(this);
-                GetEventHandler()->ProcessEvent(evh);
-                return wxEventFilter::Event_Processed;
-        }
-        }
     }
-    return wxEventFilter::Event_Skip;
+
+    wxCommandEvent command_event(wxEVT_COMMAND_MENU_SELECTED, command);
+    command_event.SetEventObject(this);
+    this->GetEventHandler()->ProcessEvent(command_event);
+    return wxEventFilter::Event_Processed;
 }
 
 wxString MainFrame::GetGamePath(wxString path)
@@ -989,12 +1006,10 @@ void MainFrame::SetJoystick()
      * destroying and creating the GameArea `panel`. */
     joy.StopPolling();
 
-    set_global_accels();
-
     if (!emulating)
         return;
 
-    std::set<wxJoystick> needed_joysticks;
+    std::set<wxJoystick> needed_joysticks = gopts.shortcuts.Joysticks();
     for (const auto& iter : gopts.game_control_bindings) {
         for (const auto& input_iter : iter.second) {
             needed_joysticks.emplace(input_iter.joystick());
@@ -1049,19 +1064,6 @@ void MainFrame::enable_menus()
                 loadst_mi[i]->Enable(state_ts[i].IsValid());
 }
 
-void MainFrame::SetRecentAccels()
-{
-    for (int i = 0; i < 10; i++) {
-        wxMenuItem* mi = recent->FindItem(i + wxID_FILE1);
-
-        if (!mi)
-            break;
-
-        // if command is 0, there is no accel
-        DoSetAccel(mi, recent_accel[i].GetCommand() ? &recent_accel[i] : NULL);
-    }
-}
-
 void MainFrame::update_state_ts(bool force)
 {
     bool any_states = false;
@@ -1171,6 +1173,26 @@ int MainFrame::newest_state_slot()
     return ns + 1;
 }
 
+void MainFrame::ResetRecentAccelerators() {
+    for (int i = wxID_FILE1; i <= wxID_FILE10; i++) {
+        wxMenuItem* menu_item = recent->FindItem(i);
+        if (!menu_item) {
+            break;
+        }
+        ResetMenuItemAccelerator(menu_item);
+    }
+}
+
+void MainFrame::ResetMenuAccelerators() {
+    for (int i = 0; i < ncmds; i++) {
+        if (!cmdtab[i].mi) {
+            continue;
+        }
+        ResetMenuItemAccelerator(cmdtab[i].mi);
+    }
+    ResetRecentAccelerators();
+}
+
 void MainFrame::MenuPopped(wxMenuEvent& evt)
 {
     // We consider the menu closed when the main menubar or system menu is closed, not any submenus.

src/wx/wxvbam.h

@@ -95,12 +95,6 @@ public:
 #endif
     // without this, global accels don't always work
     int FilterEvent(wxEvent&);
-    wxAcceleratorEntry_v accels;
-
-    wxAcceleratorEntry_v GetAccels()
-    {
-        return accels;
-    }
 
     // vba-over.ini
     wxFileConfig* overrides = nullptr;
@@ -262,15 +256,11 @@ public:
     int oldest_state_slot(); // or empty slot if available
     int newest_state_slot(); // or 0 if none
 
-    // system-defined accelerators
+    // Resets the Recent menu accelerators. Needs to be called every time the
-    wxAcceleratorEntry_v sys_accels;
+    // Recent menu is updated.
-    // adjust recent menu with accelerators
+    void ResetRecentAccelerators();
-    void SetRecentAccels();
+    // Resets all menu accelerators.
-    // merge sys accels with user-defined accels (user overrides sys)
+    void ResetMenuAccelerators();
-    wxAcceleratorEntry_v get_accels(wxAcceleratorEntry_v user_accels);
-    // update menu and global accelerator table with sys+user accel defs
-    // probably not the quickest way to add/remove individual accels
-    void set_global_accels();
 
     // 2.8 has no HasFocus(), and FindFocus() doesn't work right
     bool HasFocus() const
@@ -357,7 +347,6 @@ private:
     checkable_mi_array_t checkable_mi;
     // recent menu item accels
     wxMenu* recent;
-    wxAcceleratorEntryUnicode recent_accel[10];
     // joystick reader
     wxJoyPoller joy;
     JoystickPoller* jpoll = nullptr;

src/wx/xrc/AccelConfig.xrc

@@ -3,7 +3,7 @@
   <object class="wxDialog" name="AccelConfig">
     <title>Key Shortcuts</title>
     <object class="wxBoxSizer">
-      <orient>wxVERTICAL</orient>
+      <flag>wxEXPAND</flag>
       <object class="sizeritem">
         <object class="wxFlexGridSizer">
           <object class="sizeritem">