From 46c5609736afcae4b6548fc7f605b39e2000887e Mon Sep 17 00:00:00 2001 From: askepticaldreamer <106888785+askepticaldreamer@users.noreply.github.com> Date: Sun, 17 Mar 2024 04:46:58 -0700 Subject: [PATCH] feat: Warn for commands with duplicate triggers (#4322) Co-authored-by: pajlada --- CHANGELOG.md | 1 + src/common/SignalVectorModel.hpp | 10 +++ src/widgets/settingspages/CommandPage.cpp | 101 +++++++++++++++++++--- src/widgets/settingspages/CommandPage.hpp | 1 - 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a2930dd6..19dcbe7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ - Minor: 7TV emotes now have a 4x image rather than a 3x image. (#5209) - Minor: Add wrappers for Lua `io` library for experimental plugins feature. (#5231) - Minor: Add permissions to experimental plugins feature. (#5231) +- Minor: Added warning message if you have multiple commands with the same trigger. (#4322) - Minor: Add support to send /announce[color] commands. (#5250) - Minor: Added drop indicator line while dragging in tables. (#5252) - Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840) diff --git a/src/common/SignalVectorModel.hpp b/src/common/SignalVectorModel.hpp index bf31dbb00..620ca452d 100644 --- a/src/common/SignalVectorModel.hpp +++ b/src/common/SignalVectorModel.hpp @@ -165,12 +165,22 @@ public: else { int vecRow = this->getVectorIndexFromModelIndex(row); + // TODO: This is only a safety-thing for when we modify data that's being modified right now. + // It should not be necessary, but it would require some rethinking about this surrounding logic + if (vecRow >= this->vector_->readOnly()->size()) + { + return false; + } this->vector_->removeAt(vecRow, this); assert(this->rows_[row].original); TVectorItem item = this->getItemFromRow( this->rows_[row].items, this->rows_[row].original.value()); this->vector_->insert(item, vecRow, this); + + QVector roles = QVector(); + roles.append(role); + emit dataChanged(index, index, roles); } return true; diff --git a/src/widgets/settingspages/CommandPage.cpp b/src/widgets/settingspages/CommandPage.cpp index a1aba0575..9dc53b9bf 100644 --- a/src/widgets/settingspages/CommandPage.cpp +++ b/src/widgets/settingspages/CommandPage.cpp @@ -11,6 +11,7 @@ #include "util/StandardItemHelper.hpp" #include "widgets/helper/EditableModelView.hpp" +#include #include #include #include @@ -22,26 +23,73 @@ #define TEXT "{1} => first word     {1+} => first word and after     {{ => {     more info" // clang-format on -namespace chatterino { namespace { - QString c1settingsPath() + +using namespace chatterino; + +QString c1settingsPath() +{ + return combinePath(qgetenv("appdata"), "Chatterino\\Custom\\Commands.txt"); +} + +void checkCommandDuplicates(EditableModelView *view, QLabel *duplicateWarning) +{ + bool foundDuplicateTrigger = false; + + // Maps command triggers to model row indices + std::unordered_map> commands; + + for (int i = 0; i < view->getModel()->rowCount(); i++) { - return combinePath(qgetenv("appdata"), - "Chatterino\\Custom\\Commands.txt"); + QString commandTrigger = + view->getModel()->index(i, 0).data().toString(); + commands[commandTrigger].push_back(i); } + + for (const auto &[commandTrigger, rowIndices] : commands) + { + assert(!rowIndices.empty()); + + if (rowIndices.size() > 1) + { + foundDuplicateTrigger = true; + + for (const auto &rowIndex : rowIndices) + { + view->getModel()->setData(view->getModel()->index(rowIndex, 0), + QColor("yellow"), Qt::ForegroundRole); + } + } + else + { + view->getModel()->setData(view->getModel()->index(rowIndices[0], 0), + QColor("white"), Qt::ForegroundRole); + } + } + + if (foundDuplicateTrigger) + { + duplicateWarning->show(); + } + else + { + duplicateWarning->hide(); + } +} + } // namespace +namespace chatterino { + CommandPage::CommandPage() { - auto *app = getApp(); - LayoutCreator layoutCreator(this); auto layout = layoutCreator.setLayoutType(); - EditableModelView *view = layout - .emplace( - app->getCommands()->createModel(nullptr)) - .getElement(); + auto *view = layout + .emplace( + getIApp()->getCommands()->createModel(nullptr)) + .getElement(); view->setTitles({"Trigger", "Command", "Show In\nMessage Menu"}); view->getTableView()->horizontalHeader()->setSectionResizeMode( @@ -83,6 +131,39 @@ CommandPage::CommandPage() text->setStyleSheet("color: #bbb"); text->setOpenExternalLinks(true); + auto *duplicateWarning = + layout + .emplace("Multiple commands with the same trigger found. " + "Only one of the commands will work.") + .getElement(); + duplicateWarning->setStyleSheet("color: yellow"); + + // NOTE: These signals mean that the duplicate check happens in the middle of a row being moved, where he index can be wrong. + // This should be reconsidered, or potentially changed in the signalvectormodel. Or maybe we rely on a SignalVectorModel signal instead + QObject::connect(view->getModel(), &QAbstractItemModel::rowsInserted, this, + [view, duplicateWarning]() { + checkCommandDuplicates(view, duplicateWarning); + }); + + QObject::connect(view->getModel(), &QAbstractItemModel::rowsRemoved, this, + [view, duplicateWarning]() { + checkCommandDuplicates(view, duplicateWarning); + }); + + QObject::connect(view->getModel(), &QAbstractItemModel::dataChanged, this, + [view, duplicateWarning](const QModelIndex &topLeft, + const QModelIndex &bottomRight, + const QVector &roles) { + (void)topLeft; + (void)bottomRight; + if (roles.contains(Qt::EditRole)) + { + checkCommandDuplicates(view, duplicateWarning); + } + }); + + checkCommandDuplicates(view, duplicateWarning); + // ---- end of layout this->commandsEditTimer_.setSingleShot(true); } diff --git a/src/widgets/settingspages/CommandPage.hpp b/src/widgets/settingspages/CommandPage.hpp index ea97440bd..d88c00a61 100644 --- a/src/widgets/settingspages/CommandPage.hpp +++ b/src/widgets/settingspages/CommandPage.hpp @@ -2,7 +2,6 @@ #include "widgets/settingspages/SettingsPage.hpp" -#include #include namespace chatterino {