From 6b913ce59083167dde1356e3dd89ad21cf1eba62 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 82b996abf7883eef9d19ac613152f105afc5c053 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 c227927751cfee0e48d5afcd8b646d959fb0e10a 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 0989921fb9bb67458a8407a1f3d6f75c00b026ca 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 02687c55b49a598d020e65774c385e82e0268141 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 1e12e2f906621cec1e3fe50cb9bfabfd7e8d44ae 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 c0c69367c76220a662a5541ddf5b339794bdf999 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() {