From 2ca7bacabb564488b7132f3a38f0fb19495bf6e3 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 ccb03bcd2f811507555f0cbe43b990eb20b99fee 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 4c08ff8b1c443c37baa6f71234d28484ec3f4225 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 a55ddfb175c3b2837381d4b7dc033881394a9117 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 5fd51f501d538613fe71f441fedd38ae600de373 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 0a2bf1dc809e680cd348cdaf1fb96f99445cc4f7 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 540b874cf35787b216cc927da99137b9ecf66b29 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();