From e28a5b0d18be203fc4fcb8688a13a74c24fcb9c2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:35:52 -0400 Subject: [PATCH 1/7] configure_hotkey: Make IsUsedKey() a const member function This doesn't actually modify instance state of the dialog, so this can be made const. --- src/yuzu/configuration/configure_hotkeys.cpp | 2 +- src/yuzu/configuration/configure_hotkeys.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index bfb5625353..55d23d3292 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -90,7 +90,7 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } } -bool ConfigureHotkeys::IsUsedKey(QKeySequence key_sequence) { +bool ConfigureHotkeys::IsUsedKey(QKeySequence key_sequence) const { return GetUsedKeyList().contains(key_sequence); } diff --git a/src/yuzu/configuration/configure_hotkeys.h b/src/yuzu/configuration/configure_hotkeys.h index cd203aad67..e3766df55c 100644 --- a/src/yuzu/configuration/configure_hotkeys.h +++ b/src/yuzu/configuration/configure_hotkeys.h @@ -39,7 +39,7 @@ signals: private: void Configure(QModelIndex index); - bool IsUsedKey(QKeySequence key_sequence); + bool IsUsedKey(QKeySequence key_sequence) const; QList GetUsedKeyList() const; std::unique_ptr ui; From 8c05dfaa61adfc8ac0acd691cd569e3f1f021a2a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:37:06 -0400 Subject: [PATCH 2/7] configure_hotkey: Remove unnecessary include Avoids dumping all of the core settings machinery into whatever files include this header. Nothing inside the header itself actually made use of anything in settings.h anyways. --- src/yuzu/configuration/configure_hotkeys.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/yuzu/configuration/configure_hotkeys.h b/src/yuzu/configuration/configure_hotkeys.h index e3766df55c..73fb8a175a 100644 --- a/src/yuzu/configuration/configure_hotkeys.h +++ b/src/yuzu/configuration/configure_hotkeys.h @@ -6,7 +6,6 @@ #include #include -#include "core/settings.h" namespace Ui { class ConfigureHotkeys; From c4ba717491ba4c002a3122551abc87920edea2e7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:39:41 -0400 Subject: [PATCH 3/7] configure_dialog: Amend constructor initializer list order Avoids a -Wreorder compiler warning. --- src/yuzu/configuration/configure_dialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yuzu/configuration/configure_dialog.cpp b/src/yuzu/configuration/configure_dialog.cpp index 51bd1f1211..a5218b0517 100644 --- a/src/yuzu/configuration/configure_dialog.cpp +++ b/src/yuzu/configuration/configure_dialog.cpp @@ -12,7 +12,7 @@ #include "yuzu/hotkeys.h" ConfigureDialog::ConfigureDialog(QWidget* parent, HotkeyRegistry& registry) - : QDialog(parent), registry(registry), ui(new Ui::ConfigureDialog) { + : QDialog(parent), ui(new Ui::ConfigureDialog), registry(registry) { ui->setupUi(this); ui->hotkeysTab->Populate(registry); this->setConfiguration(); From cf6cdd20f8a62179891476481c45abff751e0ead Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:47:18 -0400 Subject: [PATCH 4/7] configure_hotkeys: Make comparison check a little more self-documenting This is checking if an index is valid or not and returning early if it isn't. --- src/yuzu/configuration/configure_hotkeys.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 55d23d3292..e1ddf61eb8 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -66,8 +66,9 @@ void ConfigureHotkeys::Populate(const HotkeyRegistry& registry) { } void ConfigureHotkeys::Configure(QModelIndex index) { - if (index.parent() == QModelIndex()) + if (!index.parent().isValid()) { return; + } index = index.sibling(index.row(), 1); auto* model = ui->hotkey_list->model(); From dbf13f8169dc4ab84b97f27e26575375f3b51124 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:50:14 -0400 Subject: [PATCH 5/7] configure_hotkeys: Mark member variables as const where applicable in Configure() --- src/yuzu/configuration/configure_hotkeys.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index e1ddf61eb8..7a09b66b4e 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -71,16 +71,16 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } index = index.sibling(index.row(), 1); - auto* model = ui->hotkey_list->model(); - auto previous_key = model->data(index); + auto* const model = ui->hotkey_list->model(); + const auto previous_key = model->data(index); - auto* hotkey_dialog = new SequenceDialog; - int return_code = hotkey_dialog->exec(); + auto* const hotkey_dialog = new SequenceDialog; - auto key_sequence = hotkey_dialog->GetSequence(); - - if (return_code == QDialog::Rejected || key_sequence.isEmpty()) + const int return_code = hotkey_dialog->exec(); + const auto key_sequence = hotkey_dialog->GetSequence(); + if (return_code == QDialog::Rejected || key_sequence.isEmpty()) { return; + } if (IsUsedKey(key_sequence) && key_sequence != QKeySequence(previous_key.toString())) { QMessageBox::critical(this, tr("Error in inputted key"), From b47c0c8a801c3e60467ad2a5890a9de0dc33f280 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 19:50:59 -0400 Subject: [PATCH 6/7] configure_hotkeys: Avoid dialog memory leak within Configure() Without a parent, this dialog won't have its memory freed when it happens to get destroyed. --- src/yuzu/configuration/configure_hotkeys.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 7a09b66b4e..31347ce5a9 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -74,10 +74,10 @@ void ConfigureHotkeys::Configure(QModelIndex index) { auto* const model = ui->hotkey_list->model(); const auto previous_key = model->data(index); - auto* const hotkey_dialog = new SequenceDialog; + SequenceDialog hotkey_dialog; - const int return_code = hotkey_dialog->exec(); - const auto key_sequence = hotkey_dialog->GetSequence(); + const int return_code = hotkey_dialog.exec(); + const auto key_sequence = hotkey_dialog.GetSequence(); if (return_code == QDialog::Rejected || key_sequence.isEmpty()) { return; } From e1101d3e20ef976b4cbc78ae30c2565e8911fe73 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 9 Apr 2019 20:06:45 -0400 Subject: [PATCH 7/7] configure_hotkeys: Pass the dialog as a parent to SequenceDialog() Without passing in a parent, this can result in focus being stolen from the dialog in certain cases. Example: On Windows, if the logging window is left open, the logging Window will potentially get focus over the hotkey dialog itself, since it brings all open windows for the application into view. By specifying a parent, we only bring windows for the parent into view (of which there are none, aside from the hotkey dialog). --- src/yuzu/configuration/configure_hotkeys.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 31347ce5a9..a7a8752e59 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -74,7 +74,7 @@ void ConfigureHotkeys::Configure(QModelIndex index) { auto* const model = ui->hotkey_list->model(); const auto previous_key = model->data(index); - SequenceDialog hotkey_dialog; + SequenceDialog hotkey_dialog{this}; const int return_code = hotkey_dialog.exec(); const auto key_sequence = hotkey_dialog.GetSequence();