From 35281b4b3bdfd1568e40f6142d8d26aaa7af51ed 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/citra_qt/configuration/configure_hotkeys.cpp | 4 ++-- src/citra_qt/configuration/configure_hotkeys.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.cpp b/src/citra_qt/configuration/configure_hotkeys.cpp index aae212b5d..d8cfe52ef 100644 --- a/src/citra_qt/configuration/configure_hotkeys.cpp +++ b/src/citra_qt/configuration/configure_hotkeys.cpp @@ -35,7 +35,7 @@ void ConfigureHotkeys::EmitHotkeysChanged() { emit HotkeysChanged(GetUsedKeyList()); } -QList ConfigureHotkeys::GetUsedKeyList() { +QList ConfigureHotkeys::GetUsedKeyList() const { QList list; for (int r = 0; r < model->rowCount(); r++) { QStandardItem* parent = model->item(r, 0); @@ -94,7 +94,7 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } } -bool ConfigureHotkeys::IsUsedKey(QKeySequence key_sequence) { +bool ConfigureHotkeys::IsUsedKey(QKeySequence key_sequence) const { return input_keys_list.contains(key_sequence) || GetUsedKeyList().contains(key_sequence); } diff --git a/src/citra_qt/configuration/configure_hotkeys.h b/src/citra_qt/configuration/configure_hotkeys.h index bd2c1542c..6fc34207e 100644 --- a/src/citra_qt/configuration/configure_hotkeys.h +++ b/src/citra_qt/configuration/configure_hotkeys.h @@ -42,8 +42,8 @@ signals: private: void Configure(QModelIndex index); - bool IsUsedKey(QKeySequence key_sequence); - QList GetUsedKeyList(); + bool IsUsedKey(QKeySequence key_sequence) const; + QList GetUsedKeyList() const; std::unique_ptr ui; From 21cba77708065e1df9777529f38111d872a2a4c2 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/citra_qt/configuration/configure_hotkeys.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.h b/src/citra_qt/configuration/configure_hotkeys.h index 6fc34207e..30b6784ef 100644 --- a/src/citra_qt/configuration/configure_hotkeys.h +++ b/src/citra_qt/configuration/configure_hotkeys.h @@ -6,7 +6,6 @@ #include #include -#include "core/settings.h" namespace Ui { class ConfigureHotkeys; From c33c27d6462808af1efa29582fb879aea799ab46 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/citra_qt/configuration/configure_dialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/citra_qt/configuration/configure_dialog.cpp b/src/citra_qt/configuration/configure_dialog.cpp index 653f2a387..bbba5159c 100644 --- a/src/citra_qt/configuration/configure_dialog.cpp +++ b/src/citra_qt/configuration/configure_dialog.cpp @@ -11,7 +11,7 @@ #include "ui_configure.h" ConfigureDialog::ConfigureDialog(QWidget* parent, HotkeyRegistry& registry, bool enable_web_config) - : QDialog(parent), registry(registry), ui(new Ui::ConfigureDialog) { + : QDialog(parent), ui(new Ui::ConfigureDialog), registry(registry) { ui->setupUi(this); ui->hotkeysTab->Populate(registry); ui->webTab->SetWebServiceConfigEnabled(enable_web_config); From 47176e19be267d1232e4f885adf0de2fcde30da4 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/citra_qt/configuration/configure_hotkeys.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.cpp b/src/citra_qt/configuration/configure_hotkeys.cpp index d8cfe52ef..7f7d2d385 100644 --- a/src/citra_qt/configuration/configure_hotkeys.cpp +++ b/src/citra_qt/configuration/configure_hotkeys.cpp @@ -70,8 +70,9 @@ void ConfigureHotkeys::OnInputKeysChanged(QList new_key_list) { } 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 d7d1be509bee336e2397f1dcfd22211d6c7d5925 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/citra_qt/configuration/configure_hotkeys.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.cpp b/src/citra_qt/configuration/configure_hotkeys.cpp index 7f7d2d385..06447dc90 100644 --- a/src/citra_qt/configuration/configure_hotkeys.cpp +++ b/src/citra_qt/configuration/configure_hotkeys.cpp @@ -75,16 +75,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 562e0114eb2def50071b6f2a1fcacad681216fd8 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/citra_qt/configuration/configure_hotkeys.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.cpp b/src/citra_qt/configuration/configure_hotkeys.cpp index 06447dc90..621a27849 100644 --- a/src/citra_qt/configuration/configure_hotkeys.cpp +++ b/src/citra_qt/configuration/configure_hotkeys.cpp @@ -78,10 +78,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 c9cc37831242b7da94cb0eb7d7c553014c07333b 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/citra_qt/configuration/configure_hotkeys.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/citra_qt/configuration/configure_hotkeys.cpp b/src/citra_qt/configuration/configure_hotkeys.cpp index 621a27849..fb4b7455a 100644 --- a/src/citra_qt/configuration/configure_hotkeys.cpp +++ b/src/citra_qt/configuration/configure_hotkeys.cpp @@ -78,7 +78,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();