From 4823cce6223536f552fd7471ebb581d23660a111 Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Sat, 21 Dec 2024 17:58:55 -0800 Subject: [PATCH] Show conflicting shortcut in error message (#5287) --- .../src/settings/shortcuts_settings.cpp | 49 +++++++++++---- cockatrice/src/settings/shortcuts_settings.h | 26 ++++---- cockatrice/src/utility/sequence_edit.cpp | 59 +++++++++++++------ cockatrice/src/utility/sequence_edit.h | 1 + 4 files changed, 91 insertions(+), 44 deletions(-) diff --git a/cockatrice/src/settings/shortcuts_settings.cpp b/cockatrice/src/settings/shortcuts_settings.cpp index 7479b7bb4..bcfc3039a 100644 --- a/cockatrice/src/settings/shortcuts_settings.cpp +++ b/cockatrice/src/settings/shortcuts_settings.cpp @@ -114,6 +114,17 @@ QString ShortcutsSettings::getShortcutString(const QString &name) const return stringifySequence(getShortcut(name)); } +QString ShortcutsSettings::getShortcutFriendlyName(const QString &shortcutName) const +{ + for (auto it = defaultShortCuts.cbegin(); it != defaultShortCuts.cend(); ++it) { + if (shortcutName == it.key()) { + return it.value().getName(); + } + } + + return {}; +} + QString ShortcutsSettings::stringifySequence(const QList &Sequence) const { QStringList stringSequence; @@ -150,9 +161,9 @@ void ShortcutsSettings::setShortcuts(const QString &name, const QKeySequence &Se setShortcuts(name, QList{Sequence}); } -void ShortcutsSettings::setShortcuts(const QString &name, const QString &Sequences) +void ShortcutsSettings::setShortcuts(const QString &name, const QString &sequences) { - setShortcuts(name, parseSequenceString(Sequences)); + setShortcuts(name, parseSequenceString(sequences)); } void ShortcutsSettings::resetAllShortcuts() @@ -177,33 +188,45 @@ void ShortcutsSettings::clearAllShortcuts() emit shortCutChanged(); } -bool ShortcutsSettings::isKeyAllowed(const QString &name, const QString &Sequences) const +bool ShortcutsSettings::isKeyAllowed(const QString &name, const QString &sequences) const { // if the shortcut is not to be used in deck-editor then it doesn't matter if (name.startsWith("Player") || name.startsWith("Replays")) { return true; } - QString checkSequence = Sequences.split(sep).last(); + QString checkSequence = sequences.split(sep).last(); QStringList forbiddenKeys{"Del", "Backspace", "Down", "Up", "Left", "Right", "Return", "Enter", "Menu", "Ctrl+Alt+-", "Ctrl+Alt+=", "Ctrl+Alt+[", "Ctrl+Alt+]", "Tab", "Space", "Shift+S", "Shift+Left", "Shift+Right"}; return !forbiddenKeys.contains(checkSequence); } -bool ShortcutsSettings::isValid(const QString &name, const QString &Sequences) const +/** + * Checks that the shortcut doesn't overlap with an existing shortcut + * + * @param name The name of the shortcut + * @param sequences The shortcut key sequence + * @return Whether the shortcut is valid. + */ +bool ShortcutsSettings::isValid(const QString &name, const QString &sequences) const { - QString checkSequence = Sequences.split(sep).last(); + return findOverlaps(name, sequences).isEmpty(); +} + +QStringList ShortcutsSettings::findOverlaps(const QString &name, const QString &sequences) const +{ + QString checkSequence = sequences.split(sep).last(); QString checkKey = name.left(name.indexOf("/")); - QList allKeys = shortCuts.keys(); - for (const auto &key : allKeys) { + QStringList overlaps; + for (const auto &key : shortCuts.keys()) { if (key.startsWith(checkKey) || key.startsWith("MainWindow") || checkKey.startsWith("MainWindow")) { QString storedSequence = stringifySequence(shortCuts.value(key)); - QStringList stringSequences = storedSequence.split(sep); - if (stringSequences.contains(checkSequence)) { - return false; + if (storedSequence.split(sep).contains(checkSequence)) { + overlaps.append(getShortcutFriendlyName(key)); } } } - return true; -} + + return overlaps; +} \ No newline at end of file diff --git a/cockatrice/src/settings/shortcuts_settings.h b/cockatrice/src/settings/shortcuts_settings.h index 636870826..4b1a6e882 100644 --- a/cockatrice/src/settings/shortcuts_settings.h +++ b/cockatrice/src/settings/shortcuts_settings.h @@ -2,9 +2,7 @@ #define SHORTCUTSSETTINGS_H #include -#include #include -#include #include class ShortcutGroup @@ -80,18 +78,18 @@ public: class ShortcutKey : public QList { public: - ShortcutKey(const QString &_name = QString(), - QList _sequence = QList(), - ShortcutGroup::Groups _group = ShortcutGroup::Main_Window); - void setSequence(QList _sequence) + explicit ShortcutKey(const QString &_name = QString(), + QList _sequence = QList(), + ShortcutGroup::Groups _group = ShortcutGroup::Main_Window); + void setSequence(const QList &_sequence) { - QList::operator=(_sequence); + QList::operator=(_sequence); }; - const QString getName() const + QString getName() const { return QApplication::translate("shortcutsTab", name.toUtf8().data()); }; - const QString getGroupName() const + QString getGroupName() const { return ShortcutGroup::getGroupName(group); }; @@ -105,13 +103,14 @@ class ShortcutsSettings : public QObject { Q_OBJECT public: - ShortcutsSettings(const QString &settingsFilePath, QObject *parent = nullptr); + explicit ShortcutsSettings(const QString &settingsFilePath, QObject *parent = nullptr); ShortcutKey getDefaultShortcut(const QString &name) const; ShortcutKey getShortcut(const QString &name) const; QKeySequence getSingleShortcut(const QString &name) const; QString getDefaultShortcutString(const QString &name) const; QString getShortcutString(const QString &name) const; + QString getShortcutFriendlyName(const QString &shortcutName) const; QList getAllShortcutKeys() const { return shortCuts.keys(); @@ -119,10 +118,11 @@ public: void setShortcuts(const QString &name, const QList &Sequence); void setShortcuts(const QString &name, const QKeySequence &Sequence); - void setShortcuts(const QString &name, const QString &Sequences); + void setShortcuts(const QString &name, const QString &sequences); - bool isKeyAllowed(const QString &name, const QString &Sequences) const; - bool isValid(const QString &name, const QString &Sequences) const; + bool isKeyAllowed(const QString &name, const QString &sequences) const; + bool isValid(const QString &name, const QString &sequences) const; + QStringList findOverlaps(const QString &name, const QString &sequences) const; void resetAllShortcuts(); void clearAllShortcuts(); diff --git a/cockatrice/src/utility/sequence_edit.cpp b/cockatrice/src/utility/sequence_edit.cpp index 183e2096e..e436b3559 100644 --- a/cockatrice/src/utility/sequence_edit.cpp +++ b/cockatrice/src/utility/sequence_edit.cpp @@ -154,26 +154,49 @@ int SequenceEdit::translateModifiers(Qt::KeyboardModifiers state, const QString return result; } +/** + *Validates that shortcut is valid (is a valid shortcut key sequence and doesn't conflict with any other shortcuts). + *Displays warning messages if it's not valid. + * + * @param sequence The shortcut key sequence + * @return True if the sequence isn't already self-contained + */ +bool SequenceEdit::validateShortcut(const QKeySequence &sequence) +{ + if (sequence.isEmpty() || !valid) { + return true; + } + + const auto &shortcutsSettings = SettingsCache::instance().shortcuts(); + const QString sequenceString = sequence.toString(); + + if (!shortcutsSettings.isKeyAllowed(shortcutName, sequenceString)) { + QToolTip::showText(lineEdit->mapToGlobal(QPoint()), tr("Invalid key")); + return true; + } + + if (!shortcutsSettings.isValid(shortcutName, sequenceString)) { + auto overlaps = shortcutsSettings.findOverlaps(shortcutName, sequenceString); + QToolTip::showText(lineEdit->mapToGlobal(QPoint()), + tr("Shortcut already in use by:") + " " + overlaps.join(", ")); + return true; + } + + if (!lineEdit->text().isEmpty()) { + if (lineEdit->text().contains(sequenceString)) { + return false; + } + lineEdit->setText(lineEdit->text() + ";"); + } + lineEdit->setText(lineEdit->text() + sequenceString); + + return true; +} + void SequenceEdit::finishShortcut() { - QKeySequence sequence(keys); - if (!sequence.isEmpty() && valid) { - QString sequenceString = sequence.toString(); - if (SettingsCache::instance().shortcuts().isKeyAllowed(shortcutName, sequenceString)) { - if (SettingsCache::instance().shortcuts().isValid(shortcutName, sequenceString)) { - if (!lineEdit->text().isEmpty()) { - if (lineEdit->text().contains(sequenceString)) { - return; - } - lineEdit->setText(lineEdit->text() + ";"); - } - lineEdit->setText(lineEdit->text() + sequenceString); - } else { - QToolTip::showText(lineEdit->mapToGlobal(QPoint()), tr("Shortcut already in use")); - } - } else { - QToolTip::showText(lineEdit->mapToGlobal(QPoint()), tr("Invalid key")); - } + if (!validateShortcut(QKeySequence(keys))) { + return; } currentKey = 0; diff --git a/cockatrice/src/utility/sequence_edit.h b/cockatrice/src/utility/sequence_edit.h index 6bd490abd..ddb7847d6 100644 --- a/cockatrice/src/utility/sequence_edit.h +++ b/cockatrice/src/utility/sequence_edit.h @@ -36,6 +36,7 @@ private: void processKey(QKeyEvent *e); int translateModifiers(Qt::KeyboardModifiers state, const QString &text); + bool validateShortcut(const QKeySequence &sequence); void finishShortcut(); void updateSettings(); };