Merge pull request #4137 from FearlessTobi/port-950

Port #950 from yuzu: "qt/hotkey: Get rid of global hotkey map instance"

src/citra_qt/configuration/configure_dialog.cpp

@@ -4,11 +4,14 @@
 
 #include "citra_qt/configuration/config.h"
 #include "citra_qt/configuration/configure_dialog.h"
+#include "citra_qt/hotkeys.h"
 #include "core/settings.h"
 #include "ui_configure.h"
 
-ConfigureDialog::ConfigureDialog(QWidget* parent) : QDialog(parent), ui(new Ui::ConfigureDialog) {
+ConfigureDialog::ConfigureDialog(QWidget* parent, const HotkeyRegistry& registry)
+    : QDialog(parent), ui(new Ui::ConfigureDialog) {
     ui->setupUi(this);
+    ui->generalTab->PopulateHotkeyList(registry);
     this->setConfiguration();
     connect(ui->generalTab, &ConfigureGeneral::languageChanged, this,
             &ConfigureDialog::onLanguageChanged);

src/citra_qt/configuration/configure_dialog.h

@@ -7,6 +7,8 @@
 #include <memory>
 #include <QDialog>
 
+class HotkeyRegistry;
+
 namespace Ui {
 class ConfigureDialog;
 }
@@ -15,7 +17,7 @@ class ConfigureDialog : public QDialog {
     Q_OBJECT
 
 public:
-    explicit ConfigureDialog(QWidget* parent);
+    explicit ConfigureDialog(QWidget* parent, const HotkeyRegistry& registry);
     ~ConfigureDialog();
 
     void applyConfiguration();

src/citra_qt/configuration/configure_general.cpp

@@ -56,6 +56,10 @@ void ConfigureGeneral::setConfiguration() {
         ui->language_combobox->findData(UISettings::values.language));
 }
 
+void ConfigureGeneral::PopulateHotkeyList(const HotkeyRegistry& registry) {
+    ui->hotkeysDialog->Populate(registry);
+}
+
 void ConfigureGeneral::applyConfiguration() {
     UISettings::values.confirm_before_closing = ui->toggle_check_exit->isChecked();
     UISettings::values.theme =

src/citra_qt/configuration/configure_general.h

@@ -7,6 +7,8 @@
 #include <memory>
 #include <QWidget>
 
+class HotkeyRegistry;
+
 namespace Ui {
 class ConfigureGeneral;
 }
@@ -18,6 +20,7 @@ public:
     explicit ConfigureGeneral(QWidget* parent = nullptr);
     ~ConfigureGeneral();
 
+    void PopulateHotkeyList(const HotkeyRegistry& registry);
     void applyConfiguration();
     void retranslateUi();
 
@@ -30,6 +33,5 @@ signals:
 private:
     void setConfiguration();
 
-private:
     std::unique_ptr<Ui::ConfigureGeneral> ui;
 };

src/citra_qt/hotkeys.cpp

@@ -9,58 +9,53 @@
 #include "citra_qt/hotkeys.h"
 #include "citra_qt/ui_settings.h"
 
-struct Hotkey {
+HotkeyRegistry::HotkeyRegistry() = default;
-    Hotkey() : shortcut(nullptr), context(Qt::WindowShortcut) {}
+HotkeyRegistry::~HotkeyRegistry() = default;
 
-    QKeySequence keyseq;
+void HotkeyRegistry::LoadHotkeys() {
-    QShortcut* shortcut;
-    Qt::ShortcutContext context;
-};
-
-typedef std::map<QString, Hotkey> HotkeyMap;
-typedef std::map<QString, HotkeyMap> HotkeyGroupMap;
-
-HotkeyGroupMap hotkey_groups;
-
-void SaveHotkeys() {
-    UISettings::values.shortcuts.clear();
-    for (auto group : hotkey_groups) {
-        for (auto hotkey : group.second) {
-            UISettings::values.shortcuts.emplace_back(
-                UISettings::Shortcut(group.first + "/" + hotkey.first,
-                                     UISettings::ContextualShortcut(hotkey.second.keyseq.toString(),
-                                                                    hotkey.second.context)));
-        }
-    }
-}
-
-void LoadHotkeys() {
     // Make sure NOT to use a reference here because it would become invalid once we call
     // beginGroup()
     for (auto shortcut : UISettings::values.shortcuts) {
-        QStringList cat = shortcut.first.split("/");
+        const QStringList cat = shortcut.first.split('/');
         Q_ASSERT(cat.size() >= 2);
 
         // RegisterHotkey assigns default keybindings, so use old values as default parameters
         Hotkey& hk = hotkey_groups[cat[0]][cat[1]];
         if (!shortcut.second.first.isEmpty()) {
             hk.keyseq = QKeySequence::fromString(shortcut.second.first);
-            hk.context = (Qt::ShortcutContext)shortcut.second.second;
+            hk.context = static_cast<Qt::ShortcutContext>(shortcut.second.second);
         }
         if (hk.shortcut)
             hk.shortcut->setKey(hk.keyseq);
     }
 }
 
-void RegisterHotkey(const QString& group, const QString& action, const QKeySequence& default_keyseq,
+void HotkeyRegistry::SaveHotkeys() {
-                    Qt::ShortcutContext default_context) {
+    UISettings::values.shortcuts.clear();
-    if (hotkey_groups[group].find(action) == hotkey_groups[group].end()) {
+    for (const auto& group : hotkey_groups) {
-        hotkey_groups[group][action].keyseq = default_keyseq;
+        for (const auto& hotkey : group.second) {
-        hotkey_groups[group][action].context = default_context;
+            UISettings::values.shortcuts.emplace_back(
+                UISettings::Shortcut(group.first + '/' + hotkey.first,
+                                     UISettings::ContextualShortcut(hotkey.second.keyseq.toString(),
+                                                                    hotkey.second.context)));
+        }
     }
 }
 
-QShortcut* GetHotkey(const QString& group, const QString& action, QWidget* widget) {
+void HotkeyRegistry::RegisterHotkey(const QString& group, const QString& action,
+                                    const QKeySequence& default_keyseq,
+                                    Qt::ShortcutContext default_context) {
+    auto& hotkey_group = hotkey_groups[group];
+    if (hotkey_group.find(action) != hotkey_group.end()) {
+        return;
+    }
+
+    auto& hotkey_action = hotkey_groups[group][action];
+    hotkey_action.keyseq = default_keyseq;
+    hotkey_action.context = default_context;
+}
+
+QShortcut* HotkeyRegistry::GetHotkey(const QString& group, const QString& action, QWidget* widget) {
     Hotkey& hk = hotkey_groups[group][action];
 
     if (!hk.shortcut)
@@ -71,10 +66,12 @@ QShortcut* GetHotkey(const QString& group, const QString& action, QWidget* widge
 
 GHotkeysDialog::GHotkeysDialog(QWidget* parent) : QWidget(parent) {
     ui.setupUi(this);
+}
 
-    for (auto group : hotkey_groups) {
+void GHotkeysDialog::Populate(const HotkeyRegistry& registry) {
+    for (const auto& group : registry.hotkey_groups) {
         QTreeWidgetItem* toplevel_item = new QTreeWidgetItem(QStringList(group.first));
-        for (auto hotkey : group.second) {
+        for (const auto& hotkey : group.second) {
             QStringList columns;
             columns << hotkey.first << hotkey.second.keyseq.toString();
             QTreeWidgetItem* item = new QTreeWidgetItem(columns);

src/citra_qt/hotkeys.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <map>
 #include "ui_hotkeys.h"
 
 class QDialog;
@@ -11,47 +12,69 @@ class QKeySequence;
 class QSettings;
 class QShortcut;
 
-/**
+class HotkeyRegistry final {
- * Register a hotkey.
+public:
- *
+    friend class GHotkeysDialog;
- * @param group General group this hotkey belongs to (e.g. "Main Window", "Debugger")
- * @param action Name of the action (e.g. "Start Emulation", "Load Image")
- * @param default_keyseq Default key sequence to assign if the hotkey wasn't present in the settings
- * file before
- * @param default_context Default context to assign if the hotkey wasn't present in the settings
- * file before
- * @warning Both the group and action strings will be displayed in the hotkey settings dialog
- */
-void RegisterHotkey(const QString& group, const QString& action,
-                    const QKeySequence& default_keyseq = QKeySequence(),
-                    Qt::ShortcutContext default_context = Qt::WindowShortcut);
 
-/**
+    explicit HotkeyRegistry();
- * Returns a QShortcut object whose activated() signal can be connected to other QObjects' slots.
+    ~HotkeyRegistry();
- *
- * @param group  General group this hotkey belongs to (e.g. "Main Window", "Debugger").
- * @param action Name of the action (e.g. "Start Emulation", "Load Image").
- * @param widget Parent widget of the returned QShortcut.
- * @warning If multiple QWidgets' call this function for the same action, the returned QShortcut
- * will be the same. Thus, you shouldn't rely on the caller really being the QShortcut's parent.
- */
-QShortcut* GetHotkey(const QString& group, const QString& action, QWidget* widget);
 
-/**
+    /**
- * Saves all registered hotkeys to the settings file.
- *
- * @note Each hotkey group will be stored a settings group; For each hotkey inside that group, a
- * settings group will be created to store the key sequence and the hotkey context.
- */
-void SaveHotkeys();
-
-/**
      * Loads hotkeys from the settings file.
      *
      * @note Yet unregistered hotkeys which are present in the settings will automatically be
      *       registered.
      */
-void LoadHotkeys();
+    void LoadHotkeys();
+
+    /**
+     * Saves all registered hotkeys to the settings file.
+     *
+     * @note Each hotkey group will be stored a settings group; For each hotkey inside that group, a
+     *       settings group will be created to store the key sequence and the hotkey context.
+     */
+    void SaveHotkeys();
+
+    /**
+     * Returns a QShortcut object whose activated() signal can be connected to other QObjects'
+     * slots.
+     *
+     * @param group  General group this hotkey belongs to (e.g. "Main Window", "Debugger").
+     * @param action Name of the action (e.g. "Start Emulation", "Load Image").
+     * @param widget Parent widget of the returned QShortcut.
+     * @warning If multiple QWidgets' call this function for the same action, the returned QShortcut
+     *          will be the same. Thus, you shouldn't rely on the caller really being the
+     *          QShortcut's parent.
+     */
+    QShortcut* GetHotkey(const QString& group, const QString& action, QWidget* widget);
+
+    /**
+     * Register a hotkey.
+     *
+     * @param group General group this hotkey belongs to (e.g. "Main Window", "Debugger")
+     * @param action Name of the action (e.g. "Start Emulation", "Load Image")
+     * @param default_keyseq Default key sequence to assign if the hotkey wasn't present in the
+     *                       settings file before
+     * @param default_context Default context to assign if the hotkey wasn't present in the settings
+     *                        file before
+     * @warning Both the group and action strings will be displayed in the hotkey settings dialog
+     */
+    void RegisterHotkey(const QString& group, const QString& action,
+                        const QKeySequence& default_keyseq = {},
+                        Qt::ShortcutContext default_context = Qt::WindowShortcut);
+
+private:
+    struct Hotkey {
+        QKeySequence keyseq;
+        QShortcut* shortcut = nullptr;
+        Qt::ShortcutContext context = Qt::WindowShortcut;
+    };
+
+    using HotkeyMap = std::map<QString, Hotkey>;
+    using HotkeyGroupMap = std::map<QString, HotkeyMap>;
+
+    HotkeyGroupMap hotkey_groups;
+};
 
 class GHotkeysDialog : public QWidget {
     Q_OBJECT
@@ -60,6 +83,8 @@ public:
     explicit GHotkeysDialog(QWidget* parent = nullptr);
     void retranslateUi();
 
+    void Populate(const HotkeyRegistry& registry);
+
 private:
     Ui::hotkeys ui;
 };

src/citra_qt/main.cpp

@@ -329,28 +329,29 @@ void GMainWindow::InitializeRecentFileMenuActions() {
 }
 
 void GMainWindow::InitializeHotkeys() {
-    RegisterHotkey("Main Window", "Load File", QKeySequence::Open);
+    hotkey_registry.RegisterHotkey("Main Window", "Load File", QKeySequence::Open);
-    RegisterHotkey("Main Window", "Start Emulation");
+    hotkey_registry.RegisterHotkey("Main Window", "Start Emulation");
-    RegisterHotkey("Main Window", "Continue/Pause", QKeySequence(Qt::Key_F4));
+    hotkey_registry.RegisterHotkey("Main Window", "Continue/Pause", QKeySequence(Qt::Key_F4));
-    RegisterHotkey("Main Window", "Restart", QKeySequence(Qt::Key_F5));
+    hotkey_registry.RegisterHotkey("Main Window", "Restart", QKeySequence(Qt::Key_F5));
-    RegisterHotkey("Main Window", "Swap Screens", QKeySequence(tr("F9")));
+    hotkey_registry.RegisterHotkey("Main Window", "Swap Screens", QKeySequence(tr("F9")));
-    RegisterHotkey("Main Window", "Toggle Screen Layout", QKeySequence(tr("F10")));
+    hotkey_registry.RegisterHotkey("Main Window", "Toggle Screen Layout", QKeySequence(tr("F10")));
-    RegisterHotkey("Main Window", "Fullscreen", QKeySequence::FullScreen);
+    hotkey_registry.RegisterHotkey("Main Window", "Fullscreen", QKeySequence::FullScreen);
-    RegisterHotkey("Main Window", "Exit Fullscreen", QKeySequence(Qt::Key_Escape),
+    hotkey_registry.RegisterHotkey("Main Window", "Exit Fullscreen", QKeySequence(Qt::Key_Escape),
                                    Qt::ApplicationShortcut);
-    RegisterHotkey("Main Window", "Toggle Speed Limit", QKeySequence("CTRL+Z"),
+    hotkey_registry.RegisterHotkey("Main Window", "Toggle Speed Limit", QKeySequence("CTRL+Z"),
                                    Qt::ApplicationShortcut);
-    RegisterHotkey("Main Window", "Increase Speed Limit", QKeySequence("+"),
+    hotkey_registry.RegisterHotkey("Main Window", "Increase Speed Limit", QKeySequence("+"),
                                    Qt::ApplicationShortcut);
-    RegisterHotkey("Main Window", "Decrease Speed Limit", QKeySequence("-"),
+    hotkey_registry.RegisterHotkey("Main Window", "Decrease Speed Limit", QKeySequence("-"),
                                    Qt::ApplicationShortcut);
-    LoadHotkeys();
+    hotkey_registry.LoadHotkeys();
 
-    connect(GetHotkey("Main Window", "Load File", this), &QShortcut::activated, this,
+    connect(hotkey_registry.GetHotkey("Main Window", "Load File", this), &QShortcut::activated,
-            &GMainWindow::OnMenuLoadFile);
+            this, &GMainWindow::OnMenuLoadFile);
-    connect(GetHotkey("Main Window", "Start Emulation", this), &QShortcut::activated, this,
+    connect(hotkey_registry.GetHotkey("Main Window", "Start Emulation", this),
-            &GMainWindow::OnStartGame);
+            &QShortcut::activated, this, &GMainWindow::OnStartGame);
-    connect(GetHotkey("Main Window", "Continue/Pause", this), &QShortcut::activated, this, [&] {
+    connect(hotkey_registry.GetHotkey("Main Window", "Continue/Pause", this), &QShortcut::activated,
+            this, [&] {
                 if (emulation_running) {
                     if (emu_thread->IsRunning()) {
                         OnPauseGame();
@@ -359,39 +360,42 @@ void GMainWindow::InitializeHotkeys() {
                     }
                 }
             });
-    connect(GetHotkey("Main Window", "Restart", this), &QShortcut::activated, this, [this] {
+    connect(hotkey_registry.GetHotkey("Main Window", "Restart", this), &QShortcut::activated, this,
+            [this] {
                 if (!Core::System::GetInstance().IsPoweredOn())
                     return;
                 BootGame(QString(game_path));
             });
-    connect(GetHotkey("Main Window", "Swap Screens", render_window), &QShortcut::activated,
+    connect(hotkey_registry.GetHotkey("Main Window", "Swap Screens", render_window),
-            ui.action_Screen_Layout_Swap_Screens, &QAction::trigger);
+            &QShortcut::activated, ui.action_Screen_Layout_Swap_Screens, &QAction::trigger);
-    connect(GetHotkey("Main Window", "Toggle Screen Layout", render_window), &QShortcut::activated,
+    connect(hotkey_registry.GetHotkey("Main Window", "Toggle Screen Layout", render_window),
-            this, &GMainWindow::ToggleScreenLayout);
+            &QShortcut::activated, this, &GMainWindow::ToggleScreenLayout);
-    connect(GetHotkey("Main Window", "Fullscreen", render_window), &QShortcut::activated,
+    connect(hotkey_registry.GetHotkey("Main Window", "Fullscreen", render_window),
-            ui.action_Fullscreen, &QAction::trigger);
+            &QShortcut::activated, ui.action_Fullscreen, &QAction::trigger);
-    connect(GetHotkey("Main Window", "Fullscreen", render_window), &QShortcut::activatedAmbiguously,
+    connect(hotkey_registry.GetHotkey("Main Window", "Fullscreen", render_window),
-            ui.action_Fullscreen, &QAction::trigger);
+            &QShortcut::activatedAmbiguously, ui.action_Fullscreen, &QAction::trigger);
-    connect(GetHotkey("Main Window", "Exit Fullscreen", this), &QShortcut::activated, this, [&] {
+    connect(hotkey_registry.GetHotkey("Main Window", "Exit Fullscreen", this),
+            &QShortcut::activated, this, [&] {
                 if (emulation_running) {
                     ui.action_Fullscreen->setChecked(false);
                     ToggleFullscreen();
                 }
             });
-    connect(GetHotkey("Main Window", "Toggle Speed Limit", this), &QShortcut::activated, this, [&] {
+    connect(hotkey_registry.GetHotkey("Main Window", "Toggle Speed Limit", this),
+            &QShortcut::activated, this, [&] {
                 Settings::values.use_frame_limit = !Settings::values.use_frame_limit;
                 UpdateStatusBar();
             });
     constexpr u16 SPEED_LIMIT_STEP = 5;
-    connect(GetHotkey("Main Window", "Increase Speed Limit", this), &QShortcut::activated, this,
+    connect(hotkey_registry.GetHotkey("Main Window", "Increase Speed Limit", this),
-            [&] {
+            &QShortcut::activated, this, [&] {
                 if (Settings::values.frame_limit < 9999 - SPEED_LIMIT_STEP) {
                     Settings::values.frame_limit += SPEED_LIMIT_STEP;
                     UpdateStatusBar();
                 }
             });
-    connect(GetHotkey("Main Window", "Decrease Speed Limit", this), &QShortcut::activated, this,
+    connect(hotkey_registry.GetHotkey("Main Window", "Decrease Speed Limit", this),
-            [&] {
+            &QShortcut::activated, this, [&] {
                 if (Settings::values.frame_limit > SPEED_LIMIT_STEP) {
                     Settings::values.frame_limit -= SPEED_LIMIT_STEP;
                     UpdateStatusBar();
@@ -506,9 +510,10 @@ void GMainWindow::ConnectMenuEvents() {
     connect(ui.action_Show_Room, &QAction::triggered, multiplayer_state,
             &MultiplayerState::OnOpenNetworkRoom);
 
-    ui.action_Fullscreen->setShortcut(GetHotkey("Main Window", "Fullscreen", this)->key());
+    ui.action_Fullscreen->setShortcut(
+        hotkey_registry.GetHotkey("Main Window", "Fullscreen", this)->key());
     ui.action_Screen_Layout_Swap_Screens->setShortcut(
-        GetHotkey("Main Window", "Swap Screens", this)->key());
+        hotkey_registry.GetHotkey("Main Window", "Swap Screens", this)->key());
     ui.action_Screen_Layout_Swap_Screens->setShortcutContext(Qt::WidgetWithChildrenShortcut);
     connect(ui.action_Fullscreen, &QAction::triggered, this, &GMainWindow::ToggleFullscreen);
     connect(ui.action_Screen_Layout_Default, &QAction::triggered, this,
@@ -1206,7 +1211,7 @@ void GMainWindow::OnSwapScreens() {
 }
 
 void GMainWindow::OnConfigure() {
-    ConfigureDialog configureDialog(this);
+    ConfigureDialog configureDialog(this, hotkey_registry);
     connect(&configureDialog, &ConfigureDialog::languageChanged, this,
             &GMainWindow::OnLanguageChanged);
     auto old_theme = UISettings::values.theme;
@@ -1364,7 +1369,7 @@ void GMainWindow::closeEvent(QCloseEvent* event) {
     UISettings::values.first_start = false;
 
     game_list->SaveInterfaceLayout();
-    SaveHotkeys();
+    hotkey_registry.SaveHotkeys();
 
     // Shutdown session if the emu thread is active...
     if (emu_thread != nullptr)

src/citra_qt/main.h

@@ -9,6 +9,7 @@
 #include <QMainWindow>
 #include <QTimer>
 #include <QTranslator>
+#include "citra_qt/hotkeys.h"
 #include "common/announce_multiplayer_room.h"
 #include "core/core.h"
 #include "core/hle/service/am/am.h"
@@ -237,6 +238,8 @@ private:
     // stores default icon theme search paths for the platform
     QStringList default_theme_paths;
 
+    HotkeyRegistry hotkey_registry;
+
 protected:
     void dropEvent(QDropEvent* event) override;
     void dragEnterEvent(QDragEnterEvent* event) override;