From 9218e347cdeed73de7e74a8911c6efca694674c5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 01:08:13 -0400 Subject: [PATCH 1/7] sequence_dialog: Remove unnecessary horizontal specifier QDialogButtonBoxes are horizontal by default. --- src/yuzu/util/sequence_dialog/sequence_dialog.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/yuzu/util/sequence_dialog/sequence_dialog.cpp b/src/yuzu/util/sequence_dialog/sequence_dialog.cpp index d3edf6ec3d..16a5473ca1 100644 --- a/src/yuzu/util/sequence_dialog/sequence_dialog.cpp +++ b/src/yuzu/util/sequence_dialog/sequence_dialog.cpp @@ -12,8 +12,7 @@ SequenceDialog::SequenceDialog(QWidget* parent) : QDialog(parent) { auto* layout = new QVBoxLayout(this); key_sequence = new QKeySequenceEdit; layout->addWidget(key_sequence); - auto* buttons = - new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal); + auto* buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); buttons->setCenterButtons(true); layout->addWidget(buttons); connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); From 5d645c6dd9c32fe528d5e10538f1f0681ff8b332 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 01:11:01 -0400 Subject: [PATCH 2/7] sequence_dialog: Reorganize the constructor The previous code was all "smushed" together wasn't really grouped together that well. This spaces things out and separates them by relation to one another, making it easier to visually parse the individual sections of code that make up the constructor. --- src/yuzu/util/sequence_dialog/sequence_dialog.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/yuzu/util/sequence_dialog/sequence_dialog.cpp b/src/yuzu/util/sequence_dialog/sequence_dialog.cpp index 16a5473ca1..bb5f74ec4e 100644 --- a/src/yuzu/util/sequence_dialog/sequence_dialog.cpp +++ b/src/yuzu/util/sequence_dialog/sequence_dialog.cpp @@ -9,15 +9,19 @@ SequenceDialog::SequenceDialog(QWidget* parent) : QDialog(parent) { setWindowTitle(tr("Enter a hotkey")); - auto* layout = new QVBoxLayout(this); + setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint); + key_sequence = new QKeySequenceEdit; - layout->addWidget(key_sequence); - auto* buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); + + auto* const buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); buttons->setCenterButtons(true); + + auto* const layout = new QVBoxLayout(this); + layout->addWidget(key_sequence); layout->addWidget(buttons); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); - setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint); } SequenceDialog::~SequenceDialog() = default; From c03fb00ac142e27ba8d7e7cc1dde0539375b7d96 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 03:53:41 -0400 Subject: [PATCH 3/7] configure_hotkeys: Remove unused EmitHotkeysChanged() 1. This is something that should be solely emitted by the hotkey dialog itself 2. This is functionally unused, given there's nothing listening for the signal. --- src/yuzu/configuration/configure_dialog.cpp | 3 --- src/yuzu/configuration/configure_hotkeys.cpp | 5 ----- src/yuzu/configuration/configure_hotkeys.h | 5 ----- 3 files changed, 13 deletions(-) diff --git a/src/yuzu/configuration/configure_dialog.cpp b/src/yuzu/configuration/configure_dialog.cpp index 32c05b7976..8086f9d6bd 100644 --- a/src/yuzu/configuration/configure_dialog.cpp +++ b/src/yuzu/configuration/configure_dialog.cpp @@ -25,9 +25,6 @@ ConfigureDialog::ConfigureDialog(QWidget* parent, HotkeyRegistry& registry) adjustSize(); ui->selectorList->setCurrentRow(0); - - // Synchronise lists upon initialisation - ui->hotkeysTab->EmitHotkeysChanged(); } ConfigureDialog::~ConfigureDialog() = default; diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index a7a8752e59..9155da4e80 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -31,10 +31,6 @@ ConfigureHotkeys::ConfigureHotkeys(QWidget* parent) ConfigureHotkeys::~ConfigureHotkeys() = default; -void ConfigureHotkeys::EmitHotkeysChanged() { - emit HotkeysChanged(GetUsedKeyList()); -} - QList ConfigureHotkeys::GetUsedKeyList() const { QList list; for (int r = 0; r < model->rowCount(); r++) { @@ -87,7 +83,6 @@ void ConfigureHotkeys::Configure(QModelIndex index) { tr("You're using a key that's already bound.")); } else { model->setData(index, key_sequence.toString(QKeySequence::NativeText)); - EmitHotkeysChanged(); } } diff --git a/src/yuzu/configuration/configure_hotkeys.h b/src/yuzu/configuration/configure_hotkeys.h index 73fb8a175a..1bbe64114a 100644 --- a/src/yuzu/configuration/configure_hotkeys.h +++ b/src/yuzu/configuration/configure_hotkeys.h @@ -24,8 +24,6 @@ public: void applyConfiguration(HotkeyRegistry& registry); void retranslateUi(); - void EmitHotkeysChanged(); - /** * Populates the hotkey list widget using data from the provided registry. * Called everytime the Configure dialog is opened. @@ -33,9 +31,6 @@ public: */ void Populate(const HotkeyRegistry& registry); -signals: - void HotkeysChanged(QList new_key_list); - private: void Configure(QModelIndex index); bool IsUsedKey(QKeySequence key_sequence) const; From ef3c0f54d0cdfc3a4a24daac93ecc65a4359b0df Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 04:03:15 -0400 Subject: [PATCH 4/7] configure_hotkeys: Move conflict detection logic to IsUsedKey() We don't need to extract the entire set of hotkeys into a list and then iterate through it. We can traverse the list and early-exit if we're able to. --- src/yuzu/configuration/configure_hotkeys.cpp | 28 +++++++++++--------- src/yuzu/configuration/configure_hotkeys.h | 1 - 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 9155da4e80..02f2daabcf 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -31,18 +31,6 @@ ConfigureHotkeys::ConfigureHotkeys(QWidget* parent) ConfigureHotkeys::~ConfigureHotkeys() = default; -QList ConfigureHotkeys::GetUsedKeyList() const { - QList list; - for (int r = 0; r < model->rowCount(); r++) { - const QStandardItem* parent = model->item(r, 0); - for (int r2 = 0; r2 < parent->rowCount(); r2++) { - const QStandardItem* keyseq = parent->child(r2, 1); - list << QKeySequence::fromString(keyseq->text(), QKeySequence::NativeText); - } - } - return list; -} - void ConfigureHotkeys::Populate(const HotkeyRegistry& registry) { for (const auto& group : registry.hotkey_groups) { auto* parent_item = new QStandardItem(group.first); @@ -87,7 +75,21 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } bool ConfigureHotkeys::IsUsedKey(QKeySequence key_sequence) const { - return GetUsedKeyList().contains(key_sequence); + for (int r = 0; r < model->rowCount(); r++) { + const QStandardItem* const parent = model->item(r, 0); + + for (int r2 = 0; r2 < parent->rowCount(); r2++) { + const QStandardItem* const key_seq_item = parent->child(r2, 1); + const auto key_seq_str = key_seq_item->text(); + const auto key_seq = QKeySequence::fromString(key_seq_str, QKeySequence::NativeText); + + if (key_sequence == key_seq) { + return true; + } + } + } + + return false; } void ConfigureHotkeys::applyConfiguration(HotkeyRegistry& registry) { diff --git a/src/yuzu/configuration/configure_hotkeys.h b/src/yuzu/configuration/configure_hotkeys.h index 1bbe64114a..e77d73c35a 100644 --- a/src/yuzu/configuration/configure_hotkeys.h +++ b/src/yuzu/configuration/configure_hotkeys.h @@ -34,7 +34,6 @@ public: private: void Configure(QModelIndex index); bool IsUsedKey(QKeySequence key_sequence) const; - QList GetUsedKeyList() const; std::unique_ptr ui; From d61199721d4815841edd564ee2153cff11e66996 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 04:06:48 -0400 Subject: [PATCH 5/7] configure_hotkeys: Change critical error dialog into a warning dialog critical() is intended for critical/fatal errors that threaten the overall stability of an application. A user entering a conflicting key sequence is neither of those. --- src/yuzu/configuration/configure_hotkeys.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 02f2daabcf..583fd6a0e6 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -67,8 +67,8 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } if (IsUsedKey(key_sequence) && key_sequence != QKeySequence(previous_key.toString())) { - QMessageBox::critical(this, tr("Error in inputted key"), - tr("You're using a key that's already bound.")); + QMessageBox::warning(this, tr("Error in inputted key"), + tr("You're using a key that's already bound.")); } else { model->setData(index, key_sequence.toString(QKeySequence::NativeText)); } From 6640f631e2a97f376c42ef2e1e5a316ab61d9dc5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 04:19:51 -0400 Subject: [PATCH 6/7] configure_hotkeys: Tidy up key sequence conflict error string Avoids mentioning the user and formalizes the error itself. --- src/yuzu/configuration/configure_hotkeys.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index 583fd6a0e6..c486ac2ed8 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -67,8 +67,8 @@ void ConfigureHotkeys::Configure(QModelIndex index) { } if (IsUsedKey(key_sequence) && key_sequence != QKeySequence(previous_key.toString())) { - QMessageBox::warning(this, tr("Error in inputted key"), - tr("You're using a key that's already bound.")); + QMessageBox::warning(this, tr("Conflicting Key Sequence"), + tr("The entered key sequence is already assigned to another hotkey.")); } else { model->setData(index, key_sequence.toString(QKeySequence::NativeText)); } From 88cd5e888e6bcd611dbcd18648697ef2e6bcba04 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 May 2019 04:34:51 -0400 Subject: [PATCH 7/7] configure_hotkeys: Remove unnecessary Settings::Apply() call Nothing from the hotkeys dialog relies on this call occurring, and is already called from the dialog that calls applyConfiguration(). --- src/yuzu/configuration/configure_hotkeys.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/yuzu/configuration/configure_hotkeys.cpp b/src/yuzu/configuration/configure_hotkeys.cpp index c486ac2ed8..9fb970c212 100644 --- a/src/yuzu/configuration/configure_hotkeys.cpp +++ b/src/yuzu/configuration/configure_hotkeys.cpp @@ -111,7 +111,6 @@ void ConfigureHotkeys::applyConfiguration(HotkeyRegistry& registry) { } registry.SaveHotkeys(); - Settings::Apply(); } void ConfigureHotkeys::retranslateUi() {