From 44d513818110343a61bbbd9e478502f0b9bcb69c Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:18:19 +0100 Subject: [PATCH 01/17] [VisualDatabase] Keep the card grid scrolling on macOS (#7764) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [VisualDatabase] Keep the card grid scrolling on macOS The view ignored the wheel whenever the vertical scroll bar was not visible. On macOS, scroll bars are transient: once the fade-out finishes, QScrollbarStyleAnimation hides the scroll bar widget, so isVisible() then reports false even though the grid has more rows to scroll. Scrolling was dead until something else brought the bar back. Gate on the scroll bar's range instead: it says whether there is anything to scroll, while visibility is only about presentation. An event with no range to scroll is still ignored, so an enclosing scroll area can take it. * Update card_database_visual_view.cpp --------- Co-authored-by: Lukas Brübach --- .../visual_database_display/card_database_visual_view.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp index 7b0d526f9..a8262c75a 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp @@ -130,8 +130,11 @@ void CardDatabaseVisualView::wheelEvent(QWheelEvent *event) return; } + // macOS uses transient (overlay) scroll bars that hide themselves + // once they fade out, so we need to use the range to determine if we ignore the wheel when there is nothing to + // scroll. This lets an enclosing scroll area take over. QScrollBar *bar = verticalScrollBar(); - if (!bar->isVisible()) { + if (bar->maximum() <= bar->minimum()) { event->ignore(); return; } From dd7ee3541c873beac3bf29df67835919c5657e0f Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:17:53 +0100 Subject: [PATCH 02/17] [PictureLoader] Keep the download log from going quadratic (#7766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [PictureLoader] Keep the download log from going quadratic A fast scrollbar sweep over the visual card database requests a picture for every cell it paints, so one gesture can ask for thousands of downloads. The status bar built a seven-widget log entry for each of them and answered every completion with findChildren() over the whole, unbounded log, rebuilding url.toString() per child: O(n) per completion, O(n^2) per burst. Profiling a sweep on master put 47% of all cycles on the GUI thread, with qt_qFindChildren_helper at 12.4% of the process (a quarter of that thread) and QWidgetPrivate::reparentFocusChildren at 4.1%. LagMonitor reported stalls of 3-18 s, growing with the backlog. Keep at most MAX_LOG_ENTRIES entries and index them by URL, so a completion and the 1 Hz cleanup only visit the matching entries. The progress bar follows the entries that remain: dropping a finished entry moves both counters back, dropping a pending one only its maximum. The log is a diagnostic view, not a queue - the downloads themselves are unaffected. * fixup! [PictureLoader] Keep the download log from going quadratic Make the URL index a QMultiHash and say why: a deferred retry re-queues the same URL while its first log entry is still pending, so one URL can back several live entries. --------- Co-authored-by: Lukas Brübach --- .../card_picture_loader_status_bar.cpp | 93 ++++++++++++++++--- .../card_picture_loader_status_bar.h | 30 ++++++ 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp index 3b26c8033..a731d067a 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp @@ -11,6 +11,7 @@ #include #include #include +#include class ExactCard; @@ -38,32 +39,94 @@ void CardPictureLoaderStatusBar::cleanOldEntries() if (!loadLog || !loadLog->popup) { return; } - for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : - loadLog->popup->findChildren()) { + + const QDateTime now = QDateTime::currentDateTime(); + // Removing an entry mutates the log, so walk a copy of it. + const QList entries = logEntries; + for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) { statusDisplayWidget->queryElapsedSeconds(); - if (statusDisplayWidget->getFinished() && - QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(QDateTime::currentDateTime()) > 10) { - loadLog->removeSettingsWidget(statusDisplayWidget); - progressBar->setMaximum(progressBar->maximum() - 1); - progressBar->setValue(progressBar->value() - 1); + if (!statusDisplayWidget->getFinished()) { + continue; + } + if (QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(now) > LOG_ENTRY_LIFETIME_SECS) { + removeLogEntry(statusDisplayWidget); } } } void CardPictureLoaderStatusBar::addQueuedImageLoad(const QUrl &url, const ExactCard &card, const QString &setName) { - loadLog->addSettingsWidget(new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName)); + if (logEntries.size() >= MAX_LOG_ENTRIES) { + trimLog(); + } + + auto *entry = new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName); + loadLog->addSettingsWidget(entry); + logEntries.append(entry); + entriesByUrl.insert(entry->getUrl(), entry); progressBar->setMaximum(progressBar->maximum() + 1); } void CardPictureLoaderStatusBar::addSuccessfulImageLoad(const QUrl &url) { - progressBar->setValue(progressBar->value() + 1); - for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : - loadLog->popup->findChildren()) { - if (statusDisplayWidget->getUrl() == url.toString()) { - statusDisplayWidget->queryElapsedSeconds(); - statusDisplayWidget->setFinished(); + const QList entries = entriesByUrl.values(url.toString()); + if (entries.isEmpty()) { + // The entry may already have been dropped from the log; the download + // itself still succeeded, there is just nothing left to mark. + return; + } + + int finishedEntries = 0; + for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) { + if (statusDisplayWidget->getFinished()) { + continue; + } + statusDisplayWidget->queryElapsedSeconds(); + statusDisplayWidget->setFinished(); + ++finishedEntries; + } + + // The bar counts the entries currently in the log, so only entries that are + // still listed move it. + progressBar->setValue(progressBar->value() + finishedEntries); +} + +void CardPictureLoaderStatusBar::trimLog() +{ + if (logEntries.isEmpty()) { + return; + } + + // Prefer evicting an entry that has already finished; only fall back to an + // in-flight request when the whole log is still pending. + CardPictureLoaderRequestStatusDisplayWidget *toRemove = nullptr; + for (CardPictureLoaderRequestStatusDisplayWidget *entry : logEntries) { + if (entry->getFinished()) { + toRemove = entry; + break; } } -} \ No newline at end of file + + removeLogEntry(toRemove != nullptr ? toRemove : logEntries.constFirst()); +} + +void CardPictureLoaderStatusBar::removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry) +{ + if (entry == nullptr) { + return; + } + + logEntries.removeOne(entry); + entriesByUrl.remove(entry->getUrl(), entry); + + // The progress bar's maximum counts the entries currently in the log and + // its value the finished ones, so dropping an entry moves both counters back. + // Move the value first: a maximum of 0 turns the bar into a busy indicator and + // resets its value. + if (entry->getFinished()) { + progressBar->setValue(qMax(0, progressBar->value() - 1)); + } + progressBar->setMaximum(qMax(0, progressBar->maximum() - 1)); + + loadLog->removeSettingsWidget(entry); +} diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h index 2ba386c42..7f614e28f 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h @@ -1,10 +1,13 @@ #ifndef PICTURE_LOADER_STATUS_BAR_H #define PICTURE_LOADER_STATUS_BAR_H +#include +#include #include #include #include +class CardPictureLoaderRequestStatusDisplayWidget; class ExactCard; class QHBoxLayout; class QProgressBar; @@ -21,11 +24,20 @@ class SettingsButtonWidget; * - Shows overall progress of image downloads. * - Maintains a log of individual requests via a popup (SettingsButtonWidget). * - Cleans up finished request entries automatically after a delay. + * + * The log is a diagnostic view, not a queue: it keeps at most MAX_LOG_ENTRIES + * entries and indexes them by URL, so a burst of requests stays O(1) per event + * instead of scanning every entry the popup holds. */ class CardPictureLoaderStatusBar : public QWidget { Q_OBJECT public: + /** Entries kept in the log; older ones are evicted as new requests arrive. */ + static constexpr int MAX_LOG_ENTRIES = 50; + /** Seconds a finished entry stays in the log before it is cleaned up. */ + static constexpr int LOG_ENTRY_LIFETIME_SECS = 10; + /** * @brief Constructs a status bar with progress bar and log button. * @param parent Parent widget @@ -55,10 +67,28 @@ public slots: void cleanOldEntries(); private: + /** + * @brief Evicts the oldest finished entry (or the oldest entry if none finished) + * until the log fits within MAX_LOG_ENTRIES. + */ + void trimLog(); + + /** + * @brief Removes one entry from the log, its URL index and the popup, and keeps + * the progress bar counters in step with the entries that remain. + * @param entry Entry to remove + */ + void removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry); + QHBoxLayout *layout; ///< Horizontal layout containing progress bar and log button QProgressBar *progressBar; ///< Progress bar showing overall download progress SettingsButtonWidget *loadLog; ///< Popup log showing individual request statuses QTimer *cleaner; ///< Timer for periodically cleaning old log entries + QList logEntries; ///< Logged requests, oldest first + // A URL can back several live entries: a request parked for a deferred retry is re-queued while + // its first entry is still pending, so a completion must finish every entry for that URL. + QMultiHash + entriesByUrl; ///< Log entries indexed by their URL for O(1) completion lookup }; #endif // PICTURE_LOADER_STATUS_BAR_H From c5b776ca46792b4d9fbcc1c17362958e4e604705 Mon Sep 17 00:00:00 2001 From: "transifex-integration[bot]" <43880903+transifex-integration[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:31:12 +0100 Subject: [PATCH 03/17] Translate oracle/oracle_en@source.ts in de (#7767) 100% translated source file: 'oracle/oracle_en@source.ts' on 'de'. Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com> --- oracle/translations/oracle_de.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/oracle/translations/oracle_de.ts b/oracle/translations/oracle_de.ts index faf08d5ce..50a5f8e20 100644 --- a/oracle/translations/oracle_de.ts +++ b/oracle/translations/oracle_de.ts @@ -137,22 +137,22 @@ Downloading (0 MB) - + Herunterladen (0 MB) Downloading (%1 MB) - + Herunterladen (%1 MB) Parsing file (%1%) - + Datei wird verarbeitet (%1%) Extracting file... - + Extrahiere Datei... @@ -168,7 +168,7 @@ Parsing file (100%) - + Datei wird verarbeitet (100%) @@ -348,7 +348,7 @@ A Cockatrice card database file of %1 MB has been downloaded. - + Eine Cockatrice-Kartendatenbankdatei der Größe %1 MB wurde heruntergeladen. @@ -434,7 +434,7 @@ Downloading (0 MB) Downloading (0MB) - + Herunterladen (0 MB) From 381dd21e8c5441794a2df3f272037ff96641ffb8 Mon Sep 17 00:00:00 2001 From: PILIX123 <89544445+PILIX123@users.noreply.github.com> Date: Tue, 3 Nov 2026 04:33:21 -0500 Subject: [PATCH 04/17] [Client] Add option to load decks from ManaBox (#7642) * [Client] Add option to load decks from ManaBox * [Client] Fix PR comments * [Client] Fix UI and transaltion requests * [Client] Fix for UI and parser not doing what expected when loading maybe board * Add ManaBox support to docs As pointed out in review comments * [Client] Fix final PR comments --------- Co-authored-by: tooomm --- .../parsers/deck_link_to_api_transformer.cpp | 15 ++- .../parsers/deck_link_to_api_transformer.h | 1 + .../parsers/interface_json_deck_parser.h | 106 +++++++++++++++++- .../dialogs/dlg_load_deck_from_website.cpp | 16 ++- .../deck_management/importing_decks.md | 2 +- 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp index 7127b0a29..2b307735d 100644 --- a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp +++ b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp @@ -15,14 +15,18 @@ static const QString ARCHIDEKT_SUFFIX = "/?format=json"; static const QString MOXFIELD_BASE = "https://api.moxfield.com/v2/decks/all/"; static const QString MOXFIELD_SUFFIX = "/"; +static const QString DECKSTATS_BASE = "https://deckstats.net/decks/"; static const QString DECKSTATS_SUFFIX = "?include_comments=1&export_mtgarena=1"; +static const QString MANABOX_BASE = "https://cloud.manabox.app/decks/"; + bool parseDeckUrl(const QString &url, ParsedDeckInfo &outInfo) { static QRegularExpression rxTappedOut("tappedout\\.net/(?:mtg-decks/)?([^/?#]+)"); static QRegularExpression rxArchidekt("archidekt\\.com/decks/(\\d+)"); static QRegularExpression rxMoxfield("moxfield\\.com/decks/([a-zA-Z0-9_-]+)"); static QRegularExpression rxDeckstats("deckstats\\.net/decks/(\\d+/[a-zA-Z0-9_-]+)"); + static QRegularExpression rxManabox("manabox\\.app/decks/([a-zA-Z0-9_-]+)"); QRegularExpressionMatch match; @@ -49,11 +53,18 @@ bool parseDeckUrl(const QString &url, ParsedDeckInfo &outInfo) return true; } else if ((match = rxDeckstats.match(url)).hasMatch()) { QString deckPath = match.captured(1); - outInfo = ParsedDeckInfo{.baseUrl = "https://deckstats.net/decks/", + outInfo = ParsedDeckInfo{.baseUrl = DECKSTATS_BASE, .deckID = deckPath, - .fullUrl = "https://deckstats.net/decks/" + deckPath + DECKSTATS_SUFFIX, + .fullUrl = DECKSTATS_BASE + deckPath + DECKSTATS_SUFFIX, .provider = DeckProvider::Deckstats}; return true; + } else if ((match = rxManabox.match(url)).hasMatch()) { + QString deckPath = match.captured(1); + outInfo = ParsedDeckInfo{.baseUrl = MANABOX_BASE, + .deckID = deckPath, + .fullUrl = MANABOX_BASE + deckPath, + .provider = DeckProvider::Manabox}; + return true; } return false; diff --git a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h index bbdd70c97..d3bff976f 100644 --- a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h +++ b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h @@ -15,6 +15,7 @@ enum class DeckProvider Archidekt, Moxfield, Deckstats, + Manabox, Unknown }; diff --git a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h index 87fde4d54..8f1defb7a 100644 --- a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h +++ b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h @@ -8,12 +8,19 @@ #define INTERFACE_JSON_DECK_PARSER_H #include "../../../interface/deck_loader/card_node_function.h" +#include "libcockatrice/deck_list/tree/inner_deck_list_node.h" #include #include +#include +#include +#include +#include #include #include +inline Q_LOGGING_CATEGORY(InterfaceJsonDeckParserLog, "interface_json_deck_parser"); + class IJsonDeckParser { public: @@ -32,9 +39,6 @@ public: QString deckName = obj.value("name").toString(); QString deckDescription = obj.value("description").toString(); - deckList.setName(deckName); - deckList.setComments(deckDescription); - QString outputText; QTextStream outStream(&outputText); @@ -52,6 +56,8 @@ public: deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + deckList.setName(deckName); + deckList.setComments(deckDescription); return deckList; } @@ -67,9 +73,6 @@ public: QString deckName = obj.value("name").toString(); QString deckDescription = obj.value("description").toString(); - deckList.setName(deckName); - deckList.setComments(deckDescription); - QString outputText; QTextStream outStream(&outputText); @@ -99,6 +102,8 @@ public: deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + deckList.setName(deckName); + deckList.setComments(deckDescription); QJsonObject commandersObj = obj.value("commanders").toObject(); if (!commandersObj.isEmpty()) { @@ -118,4 +123,93 @@ public: } }; +class ManaboxJsonParser : public IJsonDeckParser +{ + enum BoardType : quint32 + { + MainBoard = 0, + SideBoard = 1, + Commander = 2, + MaybeBoard = 3, + }; + +public: + DeckList parse(const QJsonObject &obj) override + { + DeckList deckList; + + QString deckName = obj.value("name").toString(); + + QString outputText; + QString mainboardText; + QString sideboardText; + QTextStream mainboardStream(&mainboardText); + QTextStream sideboardStream(&sideboardText); + QTextStream outStream(&outputText); + + auto commanderList = QList(); + auto maybeboardList = QList(); + for (auto entry : obj.value("cards").toArray()) { + auto card = entry.toObject(); + + auto collectorNumber = card.value("collectorNumber").toString(); + auto cardName = card.value("name").toString(); + auto boardType = static_cast(card.value("boardCategory").toInt()); + auto setName = card.value("setId").toString(); + auto quantity = card.value("quantity").toInt(); + auto providerId = card.value("scryfallId").toString(); + + switch (boardType) { + case MainBoard: + mainboardStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber + << '\n'; + break; + case SideBoard: + sideboardStream << "SB: " << quantity << ' ' << cardName << " (" << setName << ") " + << collectorNumber << '\n'; + break; + case MaybeBoard: + maybeboardList.append(card); + break; + case Commander: + commanderList.append(card); + break; + default: + qCWarning(InterfaceJsonDeckParserLog) + << "Unknown ManaBox Zone" << card.value("boardCategory").toInt() << "for" << cardName; + break; + } + } + + outStream << mainboardText << sideboardText; + + deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + + for (const auto &maybeboardCard : maybeboardList) { + QString cardName = maybeboardCard.value("name").toString(); + QString setName = maybeboardCard.value("setId").toString().toUpper(); + QString collectorNumber = maybeboardCard.value("collectorNumber").toString(); + QString providerId = maybeboardCard.value("scryfallId").toString(); + + for (int x = 0; x < maybeboardCard.value("quantity").toInt(); x++) { + deckList.addCard(cardName, DECK_ZONE_MAYBEBOARD, -1, setName, collectorNumber, providerId); + } + } + for (const auto &commanderCard : commanderList) { + + QString commanderName = commanderCard.value("name").toString(); + QString setName = commanderCard.value("setId").toString().toUpper(); + QString collectorNumber = commanderCard.value("collectorNumber").toString(); + QString providerId = commanderCard.value("scryfallId").toString(); + + deckList.setBannerCard({commanderName, providerId}); + deckList.addCard(commanderName, DECK_ZONE_MAIN, -1, setName, collectorNumber, providerId); + } + + return deckList; + } +}; + #endif // INTERFACE_JSON_DECK_PARSER_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp index 1c1ada302..04b0db2e3 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,7 @@ class QWidget; +static const QStringList SUPPORTED_PROVIDERS = {"Archidekt", "Deckstats", "ManaBox", "Moxfield", "TappedOut"}; DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent) { nam = NetworkManagerFactory::create(this); @@ -41,6 +43,7 @@ DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent setLayout(layout); instructionLabel = new QLabel(this); + instructionLabel->setWordWrap(true); layout->addWidget(instructionLabel); urlEdit = new QLineEdit(this); @@ -64,8 +67,8 @@ DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent void DlgLoadDeckFromWebsite::retranslateUi() { - instructionLabel->setText(tr("Paste a link to a decklist site here to import it.\n(Archidekt, Deckstats, Moxfield, " - "and TappedOut are supported.)")); + instructionLabel->setText(tr("Paste a link to a decklist site here to import it.") + "\n" + + tr("Supported services: %1").arg(SUPPORTED_PROVIDERS.join(", "))); } bool DlgLoadDeckFromWebsite::testValidUrl() @@ -84,8 +87,8 @@ void DlgLoadDeckFromWebsite::accept() if (!jsonParser && info.provider != DeckProvider::Deckstats && info.provider != DeckProvider::TappedOut) { qCWarning(DlgLoadDeckFromWebsiteLog) << "No parser found for provider"; QMessageBox::warning(this, tr("Load Deck from Website"), - tr("No parser available for this deck provider.\n (Archidekt, Deckstats, Moxfield, " - "and TappedOut are supported.)")); + tr("No parser available for this deck provider.") + "\n" + + tr("Supported services: %1").arg(SUPPORTED_PROVIDERS.join(", "))); QDialog::reject(); return; } @@ -152,6 +155,7 @@ void DlgLoadDeckFromWebsite::accept() "https://archidekt.com/decks/9999999\n" "https://deckstats.net/decks/99999/9999999-your-deck-name/en\n" "https://moxfield.com/decks/XYZxx-XYZ99Yyy-xyzXzzz\n" + "https://manabox.app/decks/XYZxxa-YZ99Yyyx_yzXzzz\n" "https://tappedout.net/mtg-decks/your-deck-name/")); QDialog::reject(); } @@ -164,7 +168,9 @@ QSharedPointer DlgLoadDeckFromWebsite::createParserForProvider( return QSharedPointer(new ArchidektJsonParser()); case DeckProvider::Moxfield: return QSharedPointer(new MoxfieldJsonParser()); + case DeckProvider::Manabox: + return QSharedPointer(new ManaboxJsonParser()); default: return QSharedPointer(nullptr); } -} \ No newline at end of file +} diff --git a/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md b/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md index 4aab7ad2c..fbfb8fe3c 100644 --- a/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md +++ b/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md @@ -70,4 +70,4 @@ confirm. The action will automatically import the deck from the online service without any other required user action. -Currently supported services are Archidekt, Deckstats, Moxfield, and TappedOut. \ No newline at end of file +Currently supported services are Archidekt, Deckstats, ManaBox, Moxfield, and TappedOut. From bfb53c5defeff356081789ff60bb103fd8fca75e Mon Sep 17 00:00:00 2001 From: Jurriaan Date: Tue, 3 Nov 2026 10:35:58 +0100 Subject: [PATCH 05/17] [Client] Visual Deck Editor: Added more mouse click modifiers to add/decrement cards to specific zones (#7589) * Make visual database mouse click modifiers properly add/decrement different card zones * Use event->modifiers(), refactor * Removed QApplication include * Added visual database display documentation page * Minor fixes * Added documentation for the mouse interaction of the table view --- .../tab_deck_editor_visual.cpp | 21 ++++++++++------ .../deck_management/editing_decks.md | 7 +++--- .../visual_database_display.md | 25 +++++++++++++++++++ 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp index bf4e15aa6..84bf026f7 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp @@ -20,7 +20,6 @@ #include "tab_deck_editor_visual_tab_widget.h" #include -#include #include #include #include @@ -261,17 +260,25 @@ void TabDeckEditorVisual::processMainboardCardClick(const QMouseEvent *event, } } +/** @brief Get a zone name corresponding to keyboard modifiers. */ +static QString zoneForClickModifiers(const Qt::KeyboardModifiers &modifiers) +{ + if (modifiers.testFlag(Qt::ControlModifier)) { + return DECK_ZONE_SIDE; + } else if (modifiers.testFlag(Qt::ShiftModifier)) { + return DECK_ZONE_MAYBEBOARD; + } else { + return DECK_ZONE_MAIN; + } +} + /** @brief Handles clicks on cards in the database display. */ void TabDeckEditorVisual::processDatabaseCardClick(const QMouseEvent *event, const ExactCard &card) { if (event->button() == Qt::LeftButton) { - if (QApplication::keyboardModifiers() & Qt::ControlModifier) { - addCard(card, DECK_ZONE_SIDE); - } else { - addCard(card, DECK_ZONE_MAIN); - } + addCard(card, zoneForClickModifiers(event->modifiers())); } else if (event->button() == Qt::RightButton) { - decrementCard(card, DECK_ZONE_MAIN); + decrementCard(card, zoneForClickModifiers(event->modifiers())); } else if (event->button() == Qt::MiddleButton) { deckDockWidget->actRemoveCard(); } diff --git a/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md b/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md index 37b613461..04833282d 100644 --- a/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md +++ b/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md @@ -1,6 +1,7 @@ @page editing_decks Editing Decks - - @subpage editing_decks_classic - - @subpage editing_decks_visual +- @subpage editing_decks_classic +- @subpage editing_decks_visual - - @subpage editing_decks_printings +- @subpage visual_database_display +- @subpage editing_decks_printings diff --git a/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md b/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md new file mode 100644 index 000000000..097848ab8 --- /dev/null +++ b/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md @@ -0,0 +1,25 @@ +@page visual_database_display Visual Database Display + +# Purpose + +The VisualDatabaseDisplay presents all the cards from the enabled sets in the CardDatabase, providing a visual way to find and add cards to your DeckList. + +# Using the Visual Database Display + +The VisualDatabaseDisplay can be opened by itself using the Tabs section in the menu bar at the top of the screen, or by selecting the "Visual Database Display" tab in the visual deck editor. When opened as a part of the visual deck editor, the VisualDatabaseDisplay can be used to add and remove cards from your deck. See @ref editing_decks_visual for more information about modifying your deck. + +By default the cards are displayed as a grid of pictures. However, a table view using card names can be toggled by pressing the Visual/Table button at the top right of the widget. + +# Filtering cards + +The top section of the VisualDatabaseDisplay can be used to filter cards based on various card properties, as well as specifying in which way they should be sorted. Filters can be saved and reused at a later time by using the Save/Load button at the right of this panel. + +# Modifying the Deck list + +When the VisualDatabaseDisplay is opened as part of the visual deck editor, clicking on cards adds or removes them to and from your deck. + +In the picture view, left-clicking on a card will add that card to the main board of your deck, while right-clicking will remove that printing of the card from the main board, if it exists. When clicking cards, modifier keys can be used to add (or remove) the card to zones other than the main board. Holding a Ctrl key on Windows and Linux or a Command key on MacOS adds/removes cards to the sideboard, while the Shift keys add/remove cards to the maybeboard. + +In the table view, double-clicking on a card adds that card to the main board of your deck, while right-click opens the card menu. Holding the Ctrl key on Windows and Linux or the Command key on MacOS when double-clicking adds the card to the sideboard instead. + +Tokens are always added to the token zone, regardless of which view is open or which modifier key is pressed. From bc09ec6afe31cd0fcf7e19a4afaf4ceadc6b1f2c Mon Sep 17 00:00:00 2001 From: Jurriaan Date: Tue, 3 Nov 2026 10:37:17 +0100 Subject: [PATCH 06/17] [Client] Make DeckStateManager::decrementCard remove tokens from the token zone if the token is not in the given zone (#7588) * Fixed decrementCard for tokens not using the token zone * Removed dead function actDecrementCard Updated documentation * Fixed no-op, added documentation * Removed stray blank line * Allow finding card nodes even if no database entry exists for the given card * Added DeckStateManager tests * Added tokens on mainboard test * Added if checks and sorted sources Added if checks * Library include fix * Renamed zone to targetZone * Revert "Allow finding card nodes even if no database entry exists for the given card" This reverts commit 88578cd0d954f50907a19ea539c824e7e83dfa3d. * Combined loader_local_matching_test and deck_state_manager_test settings sources * Added settings sandbox, refactor * Format * Renamed return variable * Flattened if statement * Style fix --- .../deck_editor_deck_dock_widget.cpp | 12 -- .../deck_editor_deck_dock_widget.h | 1 - .../deck_editor/deck_state_manager.cpp | 22 +- .../widgets/deck_editor/deck_state_manager.h | 5 +- tests/CMakeLists.txt | 60 ++++-- tests/deck_state_manager_test.cpp | 190 ++++++++++++++++++ 6 files changed, 253 insertions(+), 37 deletions(-) create mode 100644 tests/deck_state_manager_test.cpp diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp index 37dd57459..626a1586f 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp @@ -762,18 +762,6 @@ void DeckEditorDeckDockWidget::actSwapSelection() update(); } -void DeckEditorDeckDockWidget::actDecrementCard(const ExactCard &card, QString zoneName) -{ - if (!card) { - return; - } - if (card.getInfo().getIsToken()) { - zoneName = DECK_ZONE_TOKENS; - } - - deckStateManager->decrementCard(card, zoneName); -} - void DeckEditorDeckDockWidget::actDecrementSelection() { auto selectedRows = getSelectedCardNodeSourceIndices(); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h index 97ab8823e..fe03f02ae 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h @@ -70,7 +70,6 @@ public slots: void syncDisplayWidgetsToModel(); void actAddCard(const ExactCard &card, const QString &zoneName); void actIncrementSelection(); - void actDecrementCard(const ExactCard &card, QString zoneName); void actDecrementSelection(); void actSwapCard(const ExactCard &card, const QString &zoneName); void actSwapSelection(); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp index a7e231ecf..cbf9dd3c4 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -211,15 +211,15 @@ QModelIndex DeckStateManager::addCard(const ExactCard &card, const QString &zone return {}; } - QString zone = card.getInfo().getIsToken() ? DECK_ZONE_TOKENS : zoneName; + QString targetZone = card.getInfo().getIsToken() ? DECK_ZONE_TOKENS : zoneName; CardSetPtr set = card.getPrinting().getSet(); QString setName = set ? set->getCorrectedShortName() : ""; QString reason = - tr("Added (%1): %2 (%3) %4").arg(zone, card.getName(), setName, card.getPrinting().getProperty("num")); + tr("Added (%1): %2 (%3) %4").arg(targetZone, card.getName(), setName, card.getPrinting().getProperty("num")); - QModelIndex idx = modifyDeck(reason, [&card, &zone](auto model) { return model->addCard(card, zone); }); + QModelIndex idx = modifyDeck(reason, [&card, &targetZone](auto model) { return model->addCard(card, targetZone); }); if (idx.isValid()) { emit focusIndexChanged(idx, true); @@ -234,10 +234,22 @@ QModelIndex DeckStateManager::decrementCard(const ExactCard &card, const QString return {}; } + QString targetZone = zoneName; + QString providerId = card.getPrinting().getUuid(); QString collectorNumber = card.getPrinting().getProperty("num"); - QModelIndex idx = deckListModel->findCard(card.getName(), zoneName, providerId, collectorNumber); + QModelIndex idx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); + + // Tokens are usually added to the token zone, but it is possible for imported/hand-edited + // decks to contain them in the main or sideboards. This is why we only try to remove the + // token from the token zone if it does not exist in the given zone, allowing us to still + // remove it from there. + if (!idx.isValid() && card.getInfo().getIsToken() && targetZone != DECK_ZONE_TOKENS) { + targetZone = DECK_ZONE_TOKENS; + idx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); + } + if (!idx.isValid()) { return {}; } @@ -249,7 +261,7 @@ QModelIndex DeckStateManager::decrementCard(const ExactCard &card, const QString } // old index is no longer safe since rows could have been removed - QModelIndex newIdx = deckListModel->findCard(card.getName(), zoneName, providerId, collectorNumber); + QModelIndex newIdx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); if (newIdx.isValid()) { emit focusIndexChanged(newIdx, true); } diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h index 54c59eb79..0612147eb 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h @@ -188,17 +188,18 @@ public: ///@} /** - * @brief Adds the given card to the given zone. + * @brief Adds the given card to the given zone. Tokens always go to DECK_ZONE_TOKENS, ignoring the given zone. * Saves the operation to history if successful. * * @param card The card to add - * @param zoneName The zone to add the card to + * @param zoneName The zone to add the card to if the card is not a token * @return The index of the added card */ QModelIndex addCard(const ExactCard &card, const QString &zoneName); /** * @brief Removes 1 copy of the given card from the given zone. + * If the card is a token and is not in the given zone, then it is removed from DECK_ZONE_TOKENS instead. * Saves the operation to history if successful. * * @param card The card to remove diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0bb9654a1..cccff48c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -412,23 +412,6 @@ cockatrice_add_test( INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src ) -cockatrice_add_test( - NAME - loader_local_matching_test - SOURCES - ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp - ${VERSION_STRING_CPP} - loader_local_matching_test.cpp - LIBS - libcockatrice_network - libcockatrice_settings - INCLUDE_DIRS - ${CMAKE_SOURCE_DIR}/cockatrice/src -) cockatrice_add_test( NAME deck_hash_performance_test @@ -548,3 +531,46 @@ endif() if(WITH_ORACLE OR WITH_CLIENT) add_subdirectory(picture_loader_benchmark) endif() + +# loader_local_matching_test and deck_state_manager_test links libcockatrice_settings, +# which only exists when a client/UI-capable target is being built. +if(WITH_ORACLE OR WITH_CLIENT) + set(SETTINGS_SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp + ${VERSION_STRING_CPP} + ) + set(SETTINGS_LIBS libcockatrice_network libcockatrice_settings) + set(SETTINGS_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src) + + cockatrice_add_test( + NAME + loader_local_matching_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp + ${SETTINGS_SOURCES} + loader_local_matching_test.cpp + LIBS + ${SETTINGS_LIBS} + INCLUDE_DIRS + ${SETTINGS_INCLUDE_DIRS} + ) + cockatrice_add_test( + NAME + deck_state_manager_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp + ${SETTINGS_SOURCES} + deck_state_manager_test.cpp + LIBS + libcockatrice_card + libcockatrice_deck_list + libcockatrice_models + libcockatrice_utility + ${SETTINGS_LIBS} + INCLUDE_DIRS + ${SETTINGS_INCLUDE_DIRS} + ) +endif() diff --git a/tests/deck_state_manager_test.cpp b/tests/deck_state_manager_test.cpp new file mode 100644 index 000000000..ddd17912c --- /dev/null +++ b/tests/deck_state_manager_test.cpp @@ -0,0 +1,190 @@ +/** @file deck_state_manager_test.cpp + * @brief Tests for DeckStateManager operations. + * @ingroup Tests + */ + +#include "libcockatrice/card/database/card_database.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + +/** + * @brief Assert that the DeckStateManager's deck list contains only the given zone, + * and that that zone only contains the given number of cards, all with the given card name. + * + * @param manager the DeckStateManager + * @param restrictToZones In which zones to look for the zone and cards. Empty means use the entire deck + * @param zone The only zone that should exist in the deck + * @param cardName The name of the only card that should exist in the deck + * @param cardNumber The number of cards that should be in the deck + */ +static void containsExactly(const DeckStateManager &manager, + const QSet &restrictToZones, + QString zone, + QString cardName, + int cardNumber) +{ + QList zones = manager.getDeckList().getZoneNodes(restrictToZones); + ASSERT_EQ(zones.size(), 1); + ASSERT_EQ(zones[0]->getName(), zone); + ASSERT_EQ(zones[0]->size(), 1); + const DecklistCardNode *card = dynamic_cast((*zones[0])[0]); + ASSERT_NE(card, nullptr); + ASSERT_EQ(card->getName(), cardName); + ASSERT_EQ(card->getNumber(), cardNumber); +} + +/** @brief Return CardInfo of a token with the given name. */ +static CardInfoPtr makeToken(QString name) +{ + return CardInfo::newInstance(name, "", true, {}, {}, {}, {}, {}); +} + +// ===================================================================================================================== +// addCard +// ===================================================================================================================== + +TEST(DeckStateManager, AddCards) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 1); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); +} + +TEST(DeckStateManager, AddCardZones) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Grizzly Bears")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(CardInfo::newInstance("Swords to Plowshares")), DECK_ZONE_MAYBEBOARD); + + // Check if the cards got put into the correct zones. + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 3); + containsExactly(manager, {DECK_ZONE_MAIN}, DECK_ZONE_MAIN, "Grizzly Bears", 1); + containsExactly(manager, {DECK_ZONE_SIDE}, DECK_ZONE_SIDE, "Lightning Bolt", 1); + containsExactly(manager, {DECK_ZONE_MAYBEBOARD}, DECK_ZONE_MAYBEBOARD, "Swords to Plowshares", 1); +} + +TEST(DeckStateManager, AddTokens) +{ + DeckStateManager manager; + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAYBEBOARD); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + + // A token should always be put into the tokens zone. + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 4); +} + +// ===================================================================================================================== +// decrementCard +// ===================================================================================================================== + +TEST(DeckStateManager, DecrementCards) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); + + // Removing from the sideboard should do nothing, as there is nothing in the side board. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_SIDE); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); + + // Remove the first card. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 1); + + // Removing the second card should also remove the zone. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} + +TEST(DeckStateManager, DecrementTokens) +{ + DeckStateManager manager; + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 3); + + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 2); + + // Removing tokens from a given zone should remove them from the + // token zone if the given zone does not contain the token. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 1); + + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} + +TEST(DeckStateManager, DecrementTokenMainboard) +{ + DeckStateManager manager; + + // Add cards to the deck list directly so tokens are not + // automatically added to the token zone. + manager.getDeckListShared()->addCard("Beast", DECK_ZONE_MAIN); + manager.getDeckListShared()->addCard("Beast", DECK_ZONE_TOKENS); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 2); + containsExactly(manager, {DECK_ZONE_MAIN}, DECK_ZONE_MAIN, "Beast", 1); + containsExactly(manager, {DECK_ZONE_TOKENS}, DECK_ZONE_TOKENS, "Beast", 1); + + // decrementCard uses the model to find the card, so we need to rebuild it. + manager.getModel()->rebuildTree(); + + // Tokens should be removed from the main zone first. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes({DECK_ZONE_MAIN}).size(), 0); + containsExactly(manager, {DECK_ZONE_TOKENS}, DECK_ZONE_TOKENS, "Beast", 1); + + // If the main zone is empty they can be removed from the token zone. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} +} // namespace + +int main(int argc, char **argv) +{ + // Redirect SettingsCache reads/writes (app-data location) away from the real user profile. + QStandardPaths::setTestModeEnabled(true); + QCoreApplication app(argc, argv); + QCoreApplication::setOrganizationName(QStringLiteral("CockatriceTest")); + QCoreApplication::setApplicationName( + QStringLiteral("deck-state-manager-test-%1").arg(QCoreApplication::applicationPid())); + + ::testing::InitGoogleTest(&argc, argv); + + // Add cards to the card database + auto *db = CardDatabaseManager::getInstance(); + db->addCard(CardInfo::newInstance("Lightning Bolt")); + db->addCard(CardInfo::newInstance("Grizzly Bears")); + db->addCard(CardInfo::newInstance("Swords to Plowshares")); + db->addCard(makeToken("Beast")); + + int result = RUN_ALL_TESTS(); + + // Clean up the settings directory in ~/.qttest if the tests ran. + if (!::testing::GTEST_FLAG(list_tests)) { + QDir(SettingsCache::instance().getDataPath()).removeRecursively(); + QDir(SettingsCache::instance().getCachePath()).removeRecursively(); + } + + return result; +} From ada8f399f1a18152c1264943fd284c93ea605fac Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:38:54 -0500 Subject: [PATCH 07/17] Log server identification and connection lifecycle (#7731) * Log remote server identification and connection lifecycle [skip ci] * Update libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --------- Co-authored-by: BruebachL <44814898+BruebachL@users.noreply.github.com> Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --- .../network/client/remote/remote_client.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index faacdc9f7..df1413dd2 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -137,6 +137,7 @@ RemoteClient::~RemoteClient() void RemoteClient::slotSocketError(QAbstractSocket::SocketError /*error*/) { QString errorString = socket->errorString(); + qCWarning(RemoteClientLog) << "Server connection error:" << errorString; doDisconnectFromServer(); emit socketError(errorString); } @@ -146,6 +147,7 @@ void RemoteClient::slotWebSocketError(QAbstractSocket::SocketError /*error*/) QString errorString = websocket->errorString(); if (getStatus() != ClientStatus::StatusDisconnected) { + qCWarning(RemoteClientLog) << "Server connection error:" << errorString; doDisconnectFromServer(); emit socketError(errorString); } @@ -153,6 +155,7 @@ void RemoteClient::slotWebSocketError(QAbstractSocket::SocketError /*error*/) void RemoteClient::slotConnected() { + qCInfo(RemoteClientLog) << "Server transport connected"; timeRunning = lastDataReceived = 0; timer->start(); @@ -166,6 +169,12 @@ void RemoteClient::slotConnected() void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentification &event) { + // Identification arrives before login, including when protocol versions differ. + // Quote the server-supplied strings in the log and limit their size. + const QString serverName = QString::fromStdString(event.server_name()).left(256); + const QString serverVersion = QString::fromStdString(event.server_version()).left(256); + qCInfo(RemoteClientLog) << "Server identification:" << serverName + << "version" << serverVersion << "protocol version" << event.protocol_version(); if (event.protocol_version() != protocolVersion) { emit protocolVersionMismatch(protocolVersion, event.protocol_version()); setStatus(StatusDisconnecting); @@ -698,6 +707,8 @@ void RemoteClient::sendCommandContainer(const CommandContainer &cont) void RemoteClient::connectToHost(const QString &hostname, unsigned int port) { usingWebSocket = port == 443 || port == 80 || port == 4748 || port == 8080; + qCInfo(RemoteClientLog) << "Connecting to server" << hostname << "port" << port + << (usingWebSocket ? "using WebSocket" : "using TCP"); if (usingWebSocket) { QUrl url(QString("%1://%2:%3/servatrice").arg(port == 443 ? "wss" : "ws").arg(hostname).arg(port)); websocket->open(url); From dc4aa19d23fa24dbbd61a7522d9baaee4ce695dd Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:40:28 -0500 Subject: [PATCH 08/17] Import Moxfield signature spells into the deck [skip ci] (#7751) Co-authored-by: Codex --- .../parsers/interface_json_deck_parser.h | 26 ++++++ .../libcockatrice/deck_list/deck_list.cpp | 19 ++++- .../libcockatrice/deck_list/deck_list.h | 8 ++ tests/CMakeLists.txt | 13 +++ tests/moxfield_signature_spells_test.cpp | 84 +++++++++++++++++++ 5 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 tests/moxfield_signature_spells_test.cpp diff --git a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h index 8f1defb7a..7e15d472f 100644 --- a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h +++ b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h @@ -54,7 +54,11 @@ public: outStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber << '\n'; } + outStream.flush(); + outStream.seek(0); deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.setComments(deckDescription); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); deckList.setName(deckName); deckList.setComments(deckDescription); @@ -100,7 +104,11 @@ public: outStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber << '\n'; } + outStream.flush(); + outStream.seek(0); deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.setComments(deckDescription); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); deckList.setName(deckName); deckList.setComments(deckDescription); @@ -119,6 +127,24 @@ public: } } + // Moxfield stores Oathbreaker signature spells in their own board, + // separate from both mainboard and commanders. They are playable deck + // cards, but must not replace the Oathbreaker as the banner card. + const QJsonObject signatureSpells = obj.value("signatureSpells").toObject(); + for (auto it = signatureSpells.begin(); it != signatureSpells.end(); ++it) { + const QJsonObject entry = it.value().toObject(); + const QJsonObject card = entry.value("card").toObject(); + const QString name = card.value("name").toString(); + const int quantity = entry.value("quantity").toInt(); + if (name.isEmpty() || quantity <= 0) { + continue; + } + const QString set = card.value("set").toString().toUpper(); + const QString collectorNumber = card.value("cn").toString(); + const QString providerId = card.value("scryfall_id").toString(); + deckList.addCards(name, quantity, DECK_ZONE_MAIN, -1, set, collectorNumber, providerId); + } + return deckList; } }; diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 821b53dff..2cd5ef914 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -423,8 +423,23 @@ DecklistCardNode *DeckList::addCard(const QString &cardName, const QString &cardProviderId, bool formatLegal) { - auto node = - tree.addCard(cardName, 1, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, formatLegal); + return addCards(cardName, 1, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, formatLegal); +} + +DecklistCardNode *DeckList::addCards(const QString &cardName, + int amount, + const QString &zoneName, + int position, + const QString &cardSetName, + const QString &cardSetCollectorNumber, + const QString &cardProviderId, + bool formatLegal) +{ + if (amount <= 0) { + return nullptr; + } + auto node = tree.addCard(cardName, amount, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, + formatLegal); refreshDeckHash(); return node; } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index 6796e9c33..9d0f20586 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h @@ -284,6 +284,14 @@ public: const QString &cardSetCollectorNumber = QString(), const QString &cardProviderId = QString(), const bool formatLegal = true); + DecklistCardNode *addCards(const QString &cardName, + int amount, + const QString &zoneName, + int position = -1, + const QString &cardSetName = QString(), + const QString &cardSetCollectorNumber = QString(), + const QString &cardProviderId = QString(), + const bool formatLegal = true); ///@} /** @name Deck identity */ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cccff48c3..6acd225c2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -463,6 +463,19 @@ endif() # Guard must match the condition for src/single_instance_manager.cpp in cockatrice/CMakeLists.txt. if(WITH_CLIENT) + cockatrice_add_test( + NAME + moxfield_signature_spells_test + SOURCES + ${VERSION_STRING_CPP} + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_loader/card_node_function.cpp + moxfield_signature_spells_test.cpp + LIBS + libcockatrice_card + libcockatrice_deck_list + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src + ) cockatrice_add_test( NAME single_instance_manager_test diff --git a/tests/moxfield_signature_spells_test.cpp b/tests/moxfield_signature_spells_test.cpp new file mode 100644 index 000000000..54346a934 --- /dev/null +++ b/tests/moxfield_signature_spells_test.cpp @@ -0,0 +1,84 @@ +#include "client/network/parsers/interface_json_deck_parser.h" + +#include +#include +#include +#include + +TEST(MoxfieldImport, SignatureSpellIsAddedToMainWithoutReplacingCommanderBanner) +{ + // Moxfield v2 board objects map IDs to entries with quantity and card. + const auto input = QJsonDocument::fromJson(R"json({ + "name": "Oathbreaker example", "description": "Notes retained", + "mainboard": {"land": {"quantity": 1, "card": {"name": "Island", "set": "abc", "cn": "1"}}}, + "sideboard": {"side": {"quantity": 1, "card": {"name": "Mountain", "set": "abc", "cn": "2"}}}, + "commanders": {"walker": {"quantity": 1, "card": {"name": "Jace", "set": "abc", "cn": "3", "scryfall_id": "walker-id"}}}, + "signatureSpells": {"spell": {"quantity": 1, "card": {"name": "Lightning Bolt", "set": "xyz", "cn": "4", "scryfall_id": "spell-id"}}} + })json") + .object(); + MoxfieldJsonParser parser; + DeckList deck = parser.parse(input); + int mainSpellCount = 0; + int sideSpellCount = 0; + int mainLands = 0; + int sideLands = 0; + deck.forEachCard([&](InnerDecklistNode *zone, DecklistCardNode *card) { + if (card->getName() == "Island" && zone->getName() == DECK_ZONE_MAIN) { + mainLands += card->getNumber(); + } + if (card->getName() == "Mountain" && zone->getName() == DECK_ZONE_SIDE) { + sideLands += card->getNumber(); + } + if (card->getName() != "Lightning Bolt") { + return; + } + if (zone->getName() == DECK_ZONE_MAIN) { + mainSpellCount += card->getNumber(); + EXPECT_EQ(card->getCardSetShortName(), "XYZ"); + EXPECT_EQ(card->getCardCollectorNumber(), "4"); + EXPECT_EQ(card->getCardProviderId(), "spell-id"); + } else if (zone->getName() == DECK_ZONE_SIDE) { + sideSpellCount += card->getNumber(); + } + }); + EXPECT_EQ(mainSpellCount, 1); + EXPECT_EQ(sideSpellCount, 0); + EXPECT_EQ(mainLands, 1); + EXPECT_EQ(sideLands, 1); + EXPECT_EQ(deck.getName(), "Oathbreaker example"); + EXPECT_EQ(deck.getComments(), "Notes retained"); + EXPECT_EQ(deck.getBannerCard().name, "Jace"); + EXPECT_EQ(deck.getBannerCard().providerId, "walker-id"); +} + +TEST(MoxfieldImport, QuantityAwareDeckInsertionRetainsCountAndPrinting) +{ + DeckList deck; + auto *card = deck.addCards("Island", 2, DECK_ZONE_MAIN, -1, "ABC", "4", "id"); + ASSERT_NE(card, nullptr); + EXPECT_EQ(card->getNumber(), 2); + EXPECT_EQ(card->getCardProviderId(), "id"); + EXPECT_EQ(deck.addCards("Island", 0, DECK_ZONE_MAIN), nullptr); +} + +TEST(OnlineImport, ArchidektPreservesNameDescriptionAndCards) +{ + const auto input = QJsonDocument::fromJson(R"json({ + "name": "Online deck", "description": "Deck notes", + "cards": [{"quantity": 2, "card": { + "oracleCard": {"name": "Island"}, "edition": {"editioncode": "abc"}, "collectorNumber": "1" + }}] + })json") + .object(); + ArchidektJsonParser parser; + const DeckList deck = parser.parse(input); + int count = 0; + deck.forEachCard([&](InnerDecklistNode *zone, DecklistCardNode *card) { + if (zone->getName() == DECK_ZONE_MAIN && card->getName() == "Island") { + count += card->getNumber(); + } + }); + EXPECT_EQ(count, 2); + EXPECT_EQ(deck.getName(), "Online deck"); + EXPECT_EQ(deck.getComments(), "Deck notes"); +} From fd2a99e8973f4d44858b39a8539f25f6fe841c7e Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:41:47 -0500 Subject: [PATCH 09/17] Respect nonempty Qt hints and explain discovery failures [skip ci] (#7733) --- CMakeLists.txt | 10 ++++++---- cmake/FindQtRuntime.cmake | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a25fe2c6..677ef0c12 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,6 +71,8 @@ if(WIN32 OR USE_VCPKG) ) # Qt path set by user or env var if(QTDIR + OR QTDIR32 + OR QTDIR64 OR DEFINED ENV{QTDIR} OR DEFINED ENV{QTDIR32} OR DEFINED ENV{QTDIR64} @@ -236,13 +238,13 @@ else() set(_lib_suffix 32) endif() -if(DEFINED QTDIR${_lib_suffix}) +if(NOT "${QTDIR${_lib_suffix}}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "${QTDIR${_lib_suffix}}") -elseif(DEFINED QTDIR) +elseif(NOT "${QTDIR}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "${QTDIR}") -elseif(DEFINED ENV{QTDIR${_lib_suffix}}) +elseif(NOT "$ENV{QTDIR${_lib_suffix}}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "$ENV{QTDIR${_lib_suffix}}") -elseif(DEFINED ENV{QTDIR}) +elseif(NOT "$ENV{QTDIR}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "$ENV{QTDIR}") endif() diff --git a/cmake/FindQtRuntime.cmake b/cmake/FindQtRuntime.cmake index 2d74aca4d..7b644276d 100644 --- a/cmake/FindQtRuntime.cmake +++ b/cmake/FindQtRuntime.cmake @@ -1,6 +1,6 @@ # Find a compatible Qt version # Inputs: WITH_SERVER, WITH_CLIENT, WITH_ORACLE -# Optional Input: QT6_DIR -- Hint as to where Qt6 lives on the system +# Optional Input: Qt6_DIR -- Directory containing Qt6Config.cmake # Output: COCKATRICE_QT_VERSION_NAME -- Example values: Qt6 # Output: SERVATRICE_QT_MODULES # Output: COCKATRICE_QT_MODULES @@ -57,7 +57,17 @@ find_package( QUIET HINTS ${Qt6_DIR} ) if(NOT Qt6_FOUND) - message(FATAL_ERROR "No suitable version of Qt was found") + set(_QT_DIAGNOSTIC "") + if(Qt6_NOT_FOUND_MESSAGE) + set(_QT_DIAGNOSTIC "\nQt package diagnostic: ${Qt6_NOT_FOUND_MESSAGE}") + endif() + message(FATAL_ERROR + "No suitable version of Qt was found. Qt 6.4.2 or newer with these components is required: " + "${REQUIRED_QT_COMPONENTS};Linguist.\n" + "Set QTDIR or CMAKE_PREFIX_PATH to the Qt installation prefix, or Qt6_DIR to the directory " + "containing Qt6Config.cmake." + "${_QT_DIAGNOSTIC}" + ) endif() set(COCKATRICE_QT_VERSION_NAME Qt6) From 9588e1960d01817997bf8ea39c74fcf51b666ad2 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 05:10:50 -0500 Subject: [PATCH 10/17] Preserve saved deck formats through database reload and view refresh (#7745) --- .../deck_editor_deck_dock_widget.cpp | 49 +++++++++++-------- .../deck_editor_deck_dock_widget.h | 1 + 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp index 626a1586f..30fde1595 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -187,10 +188,17 @@ void DeckEditorDeckDockWidget::createDeckDock() formatLabel = new QLabel(this); formatComboBox = new QComboBox(this); + formatComboBox->setObjectName("deckFormatComboBox"); formatComboBox->addItem(tr("Loading Database...")); formatComboBox->setEnabled(false); // Disable until loaded commanderBracketWidget = new CommanderBracketWidget(this); + connect(formatComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, [this](int index) { + formatComboBox->setPlaceholderText(QString()); + formatComboBox->setToolTip(QString()); + deckStateManager->setFormat(index >= 0 ? formatComboBox->itemData(index).toString() : QString()); + commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); + }); commentsLabel = new QLabel(); commentsLabel->setObjectName("commentsLabel"); @@ -355,6 +363,9 @@ void DeckEditorDeckDockWidget::initializeFormats() { QStringList allFormats = CardDatabaseManager::query()->getAllFormatsWithCount().keys(); + // Rebuilding the list is a view update, not a user edit. In particular, + // clear() emits currentIndexChanged(-1), which must not clear the deck. + const QSignalBlocker blocker(formatComboBox); formatComboBox->clear(); // Remove "Loading Database..." formatComboBox->setEnabled(true); @@ -364,28 +375,25 @@ void DeckEditorDeckDockWidget::initializeFormats() formatComboBox->addItem(formatName, formatName); // store the raw key in itemData } - QString format = deckStateManager->getMetadata().gameFormat; - if (!format.isEmpty()) { - formatComboBox->setCurrentIndex(formatComboBox->findData(format)); - } else { - // Ensure no selection is visible initially - formatComboBox->setCurrentIndex(-1); - } - connect(formatComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, [this](int index) { - QString formatKey; - if (index >= 0) { - formatKey = formatComboBox->itemData(index).toString(); - deckStateManager->setFormat(formatKey); - } else { - deckStateManager->setFormat(""); // clear format if deselected - } - - commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); - }); + syncFormatSelection(); commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); } +void DeckEditorDeckDockWidget::syncFormatSelection() +{ + if (!formatComboBox->isEnabled()) { + return; + } + const QSignalBlocker blocker(formatComboBox); + const QString format = deckStateManager->getMetadata().gameFormat; + const int index = format.isEmpty() ? -1 : formatComboBox->findData(format); + const QString unavailable = index < 0 && !format.isEmpty() ? tr("Unavailable: %1").arg(format) : QString(); + formatComboBox->setPlaceholderText(unavailable); + formatComboBox->setToolTip(unavailable); + formatComboBox->setCurrentIndex(index); +} + ExactCard DeckEditorDeckDockWidget::getCurrentCard() { QModelIndex current = deckView->selectionModel()->currentIndex(); @@ -596,9 +604,7 @@ void DeckEditorDeckDockWidget::syncDisplayWidgetsToModel() updatePlaymatLabel(); updateHash(); - formatComboBox->blockSignals(true); - formatComboBox->setCurrentIndex(formatComboBox->findData(deckStateManager->getMetadata().gameFormat)); - formatComboBox->blockSignals(false); + syncFormatSelection(); commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); @@ -1042,6 +1048,7 @@ void DeckEditorDeckDockWidget::retranslateUi() commentsLabel->setText(tr("&Comments:")); activeGroupCriteriaLabel->setText(tr("Group by:")); formatLabel->setText(tr("Format:")); + syncFormatSelection(); commanderBracketWidget->retranslateUi(); hashLabel1->setText(tr("Hash:")); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h index fe03f02ae..02910e390 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h @@ -110,6 +110,7 @@ private: DeckListModel *getModel() const; [[nodiscard]] QModelIndexList getSelectedCardNodeSourceIndices() const; void offsetCountAtIndex(const QModelIndex &idx, bool isIncrement); + void syncFormatSelection(); void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex, const QString ¤tBoardName); void addChangeBoardMenu(QMenu *menu, const QString &zoneName); From c72192fd8105d24b21e3b089053418a145b74f71 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 3 Nov 2026 15:59:32 +0100 Subject: [PATCH 11/17] [Format] Small fixup (#7768) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Format] Small fixup. * [CardDB] Stop leaking the loader's mutexes. Both mutexes were heap-allocated through default member initializers and never freed, so every CardDatabase destruction leaked 16 bytes. LeakSanitizer flags this while discovering tests that create the database in main. Hold them by value instead, as the loader is a QObject and therefore neither copyable nor movable. --------- Co-authored-by: Lukas Brübach --- .../libcockatrice/card/database/card_database_loader.cpp | 4 ++-- .../libcockatrice/card/database/card_database_loader.h | 4 ++-- .../libcockatrice/network/client/remote/remote_client.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp index 00b21f88e..7ca5ba433 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp @@ -83,7 +83,7 @@ LoadStatus CardDatabaseLoader::loadCardDatabase(const QString &path, CardDatabas auto startTime = QTime::currentTime(); LoadStatus tempLoadStatus = NotLoaded; if (!path.isEmpty()) { - QMutexLocker locker(loadFromFileMutex); + QMutexLocker locker(&loadFromFileMutex); tempLoadStatus = loadFromFile(path, data); } @@ -102,7 +102,7 @@ LoadStatus CardDatabaseLoader::loadCardDatabases(bool ignoreCache) LoadStatus CardDatabaseLoader::doLoadCardDatabases(bool ignoreCache) { - QMutexLocker locker(reloadDatabaseMutex); + QMutexLocker locker(&reloadDatabaseMutex); if (!database) { qCWarning(CardDatabaseLoadingLog) << "Loader has no database pointer"; diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h index bfbaa0f51..72aeeff3e 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h @@ -177,8 +177,8 @@ private: ICardSetPriorityController *priorityController; /**< Controller for reconstructing set state. */ QList availableParsers; /**< List of available parsers for different formats. */ - QBasicMutex *loadFromFileMutex = new QBasicMutex(); /**< Mutex for single-file loading. */ - QBasicMutex *reloadDatabaseMutex = new QBasicMutex(); /**< Mutex for reloading entire database. */ + QBasicMutex loadFromFileMutex; /**< Mutex for single-file loading. */ + QBasicMutex reloadDatabaseMutex; /**< Mutex for reloading entire database. */ }; #endif // COCKATRICE_CARD_DATABASE_LOADER_H diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index df1413dd2..fa75e356a 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -173,8 +173,8 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica // Quote the server-supplied strings in the log and limit their size. const QString serverName = QString::fromStdString(event.server_name()).left(256); const QString serverVersion = QString::fromStdString(event.server_version()).left(256); - qCInfo(RemoteClientLog) << "Server identification:" << serverName - << "version" << serverVersion << "protocol version" << event.protocol_version(); + qCInfo(RemoteClientLog) << "Server identification:" << serverName << "version" << serverVersion + << "protocol version" << event.protocol_version(); if (event.protocol_version() != protocolVersion) { emit protocolVersionMismatch(protocolVersion, event.protocol_version()); setStatus(StatusDisconnecting); From 7768416091197d197807cc335785ae1b23ab59f3 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 22:37:25 -0500 Subject: [PATCH 12/17] Place current deck-editor docks before restoring saved layouts [skip ci] (#7734) --- .../src/interface/widgets/tabs/tab_deck_editor.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp index 7ca179cea..e88b2aad6 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp @@ -131,10 +131,16 @@ void TabDeckEditor::loadLayout() LayoutsSettings &layouts = SettingsCache::instance().layouts(); auto layoutState = layouts.getDeckEditorLayoutState(); - if (layoutState.isNull()) { - restartLayout(); - } else { - restoreState(layoutState); + // Start with every current dock placed. Saved layouts from older versions + // cannot describe docks introduced later; restoreState leaves those docks + // in their existing positions while restoring the saved docks, including + // their visibility and floating state. + restartLayout(); + if (!layoutState.isEmpty()) { + if (!restoreState(layoutState)) { + // A corrupt or incompatible state must not leave a partial layout. + restartLayout(); + } restoreGeometry(layouts.getDeckEditorGeometry()); } } From 94f7de42f202799bccaae9735698f0b3746b49ed Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 23:29:51 -0500 Subject: [PATCH 13/17] Fix user card games for offline users and stale requests (#7738) --- cockatrice/CMakeLists.txt | 2 + .../widgets/server/user/user_games_model.cpp | 149 +++++++++++ .../widgets/server/user/user_games_model.h | 63 +++++ .../widgets/server/user/user_info_popup.cpp | 131 +++++---- .../widgets/server/user/user_info_popup.h | 12 +- .../widgets/server/user/user_list_widget.cpp | 9 +- tests/CMakeLists.txt | 12 + tests/user_games_model_test.cpp | 248 ++++++++++++++++++ 8 files changed, 550 insertions(+), 76 deletions(-) create mode 100644 cockatrice/src/interface/widgets/server/user/user_games_model.cpp create mode 100644 cockatrice/src/interface/widgets/server/user/user_games_model.h create mode 100644 tests/user_games_model_test.cpp diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 642d998a0..ccf2dfcea 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -449,6 +449,8 @@ set(cockatrice_SOURCES src/interface/intents/intent_open_shared_deck.h src/interface/intents/url_parser.cpp src/interface/intents/url_parser.h + src/interface/widgets/server/user/user_games_model.cpp + src/interface/widgets/server/user/user_games_model.h src/interface/widgets/server/user/user_info_popup.cpp src/interface/widgets/server/user/user_info_popup.h ) diff --git a/cockatrice/src/interface/widgets/server/user/user_games_model.cpp b/cockatrice/src/interface/widgets/server/user/user_games_model.cpp new file mode 100644 index 000000000..b7c728c45 --- /dev/null +++ b/cockatrice/src/interface/widgets/server/user/user_games_model.cpp @@ -0,0 +1,149 @@ +#include "user_games_model.h" + +#include +#include +#include +#include +#include +#include +#include + +UserGamesModel::UserGamesModel(AbstractClient *_client, int requestTimeoutMs, QObject *parent) + : QAbstractListModel(parent), client(_client) +{ + timeout.setSingleShot(true); + timeout.setInterval(requestTimeoutMs); + connect(&timeout, &QTimer::timeout, this, [this] { + invalidateRequest(); + setState(State::Error); + }); + connected = client->getStatus() == StatusLoggedIn; + connect(client, &AbstractClient::statusChanged, this, [this](ClientStatus status) { + // AbstractClient::setStatus emits this signal while holding the (non-recursive) + // status mutex, which getStatus() would try to lock again. Track the emitted + // value here, and read this cache from canRefresh() for the same reason. + connected = status == StatusLoggedIn; + if (!connected) { + invalidateRequest(); + replaceGames({}); + setState(online ? State::Disconnected : State::Offline); + } else if (online) { + refresh(); + } + }); + connect(client, &QObject::destroyed, this, [this] { + client = nullptr; + connected = false; + invalidateRequest(); + replaceGames({}); + setState(online ? State::Disconnected : State::Offline); + }); +} + +int UserGamesModel::rowCount(const QModelIndex &parent) const +{ + return parent.isValid() ? 0 : games.size(); +} + +QVariant UserGamesModel::data(const QModelIndex &index, int role) const +{ + if (!index.isValid() || index.model() != this || index.column() != 0 || index.row() < 0 || + index.row() >= games.size() || role != GameRole) { + return {}; + } + return QVariant::fromValue(games.at(index.row())); +} + +void UserGamesModel::setUser(const QString &name, bool isOnline) +{ + userName = name; + online = isOnline; + refresh(); +} + +UserGamesModel::State UserGamesModel::getState() const +{ + return state; +} + +bool UserGamesModel::canRefresh() const +{ + return online && !userName.isEmpty() && client && connected && state != State::Loading; +} + +void UserGamesModel::invalidateRequest() +{ + ++requestGeneration; + // The client owns pending commands. Detach our interest without deleting + // an object that may already have moved to the client's thread. + disconnect(responseConnection); + responseConnection = {}; + timeout.stop(); +} + +void UserGamesModel::replaceGames(QList replacement) +{ + beginResetModel(); + games = std::move(replacement); + endResetModel(); +} + +void UserGamesModel::setState(State newState) +{ + state = newState; + emit stateChanged(); +} + +void UserGamesModel::refresh() +{ + invalidateRequest(); + const quint64 request = requestGeneration; + replaceGames({}); + if (!online || userName.isEmpty()) { + setState(State::Offline); + return; + } + if (!client || !connected) { + setState(State::Disconnected); + return; + } + + setState(State::Loading); + // State listeners may select another user or disconnect the client. + if (request != requestGeneration) { + return; + } + + Command_GetGamesOfUser command; + command.set_user_name(userName.toStdString()); + PendingCommand *pending = client->prepareSessionCommand(command); + responseConnection = connect(pending, &PendingCommand::finished, this, + [this, request](const Response &response) { finishRequest(response, request); }); + timeout.start(); + client->sendCommand(pending); +} + +void UserGamesModel::finishRequest(const Response &response, quint64 request) +{ + if (request != requestGeneration) { + return; + } + invalidateRequest(); + const quint64 completion = requestGeneration; + if (response.response_code() != Response::RespOk || !response.HasExtension(Response_GetGamesOfUser::ext)) { + setState(State::Error); + return; + } + + const auto &result = response.GetExtension(Response_GetGamesOfUser::ext); + QList replacement; + replacement.reserve(result.game_list_size()); + for (const auto &game : result.game_list()) { + replacement.append(game); + } + replaceGames(std::move(replacement)); + if (completion != requestGeneration) { + return; + } + setState(games.isEmpty() ? State::Empty : State::Ready); +} diff --git a/cockatrice/src/interface/widgets/server/user/user_games_model.h b/cockatrice/src/interface/widgets/server/user/user_games_model.h new file mode 100644 index 000000000..b8ad71a67 --- /dev/null +++ b/cockatrice/src/interface/widgets/server/user/user_games_model.h @@ -0,0 +1,63 @@ +#ifndef COCKATRICE_USER_GAMES_MODEL_H +#define COCKATRICE_USER_GAMES_MODEL_H + +#include +#include +#include +#include +#include +#include + +class AbstractClient; +class Response; + +/** Current user's games and the lifetime of the request that supplies them. */ +class UserGamesModel : public QAbstractListModel +{ + Q_OBJECT + +public: + enum class State + { + Offline, + Disconnected, + Loading, + Ready, + Empty, + Error + }; + static constexpr int DefaultRequestTimeoutMs = 15000; + static constexpr int GameRole = Qt::UserRole + 10; + + explicit UserGamesModel(AbstractClient *client, + int requestTimeoutMs = DefaultRequestTimeoutMs, + QObject *parent = nullptr); + int rowCount(const QModelIndex &parent = {}) const override; + QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; + + void setUser(const QString &name, bool isOnline); + void refresh(); + State getState() const; + bool canRefresh() const; + +signals: + void stateChanged(); + +private: + void invalidateRequest(); + void replaceGames(QList replacement); + void finishRequest(const Response &response, quint64 request); + void setState(State newState); + + QPointer client; + QString userName; + bool online = false; + bool connected = false; + State state = State::Offline; + QList games; + quint64 requestGeneration = 0; + QMetaObject::Connection responseConnection; + QTimer timeout; +}; + +#endif // COCKATRICE_USER_GAMES_MODEL_H diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp index efb3b2e31..9f864d903 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.cpp @@ -6,10 +6,9 @@ #include "../../interface/widgets/tabs/tab_supervisor.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/card/printing/exact_card.h" -#include "libcockatrice/protocol/pb/response.pb.h" #include "libcockatrice/protocol/pb/serverinfo_game.pb.h" #include "libcockatrice/protocol/pb/serverinfo_user.pb.h" -#include "libcockatrice/protocol/pb/session_commands.pb.h" +#include "user_games_model.h" #include "user_level.h" #include "user_list_painter.h" @@ -34,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -41,8 +41,6 @@ #include #include #include -#include -#include #include #include #include @@ -52,8 +50,6 @@ #include #include #include -#include -#include #include #include #include @@ -427,6 +423,7 @@ void UserInfoPopup::buildUi() refreshBtn = new QPushButton(QStringLiteral("↻"), this); refreshBtn->setFixedSize(20, 20); refreshBtn->setFlat(true); + refreshBtn->setToolTip(tr("Refresh games")); connect(refreshBtn, &QPushButton::clicked, this, &UserInfoPopup::refreshGames); gh->addWidget(refreshBtn); root->addLayout(gh); @@ -437,7 +434,8 @@ void UserInfoPopup::buildUi() root->addWidget(gamesStatus); // Games list - gamesModel = new QStandardItemModel(this); + gamesModel = new UserGamesModel(client, UserGamesModel::DefaultRequestTimeoutMs, this); + connect(gamesModel, &UserGamesModel::stateChanged, this, &UserInfoPopup::updateGamesDisplay); gamesView = new QListView(this); gamesView->setModel(gamesModel); gamesView->setItemDelegate(new PopupGameDelegate(gamesView)); @@ -645,13 +643,21 @@ void UserInfoPopup::rebuildActionButtons(const ServerInfo_User &userInfo, bool o void UserInfoPopup::updateActionButtons(const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored) { - rebuildActionButtons(userInfo, online, isBuddy, isIgnored); - adjustSize(); + currentUserInfo = userInfo; + currentBuddy = isBuddy; + currentIgnored = isIgnored; + if (currentOnline != online) { + setOnline(online); + } else { + refreshHeader(); + rebuildActionButtons(userInfo, online, isBuddy, isIgnored); + adjustSize(); + } } void UserInfoPopup::onGamesContextMenu(const QPoint &pos) { - const QModelIndex idx = gamesView->indexAt(pos); + const QPersistentModelIndex idx = gamesView->indexAt(pos); if (!idx.isValid()) { return; } @@ -679,7 +685,7 @@ void UserInfoPopup::onGamesContextMenu(const QPoint &pos) } const QAction *chosen = menu.exec(gamesView->viewport()->mapToGlobal(pos)); - if (!chosen) { + if (!chosen || !idx.isValid()) { return; } @@ -716,6 +722,8 @@ void UserInfoPopup::showForUser(const QString &userName, currentUser = userName; currentUserInfo = userInfo; currentOnline = online; + currentBuddy = isBuddy; + currentIgnored = isIgnored; // Header refreshHeader(); @@ -723,88 +731,71 @@ void UserInfoPopup::showForUser(const QString &userName, // Actions rebuildActionButtons(userInfo, online, isBuddy, isIgnored); - // Games list reset - gamesModel->clear(); - gamesView->hide(); - gamesStatus->setText(tr("Loading games…")); - gamesStatus->show(); - // Close button, top right corner, above everything closeBtn->move(PopupWidth - closeBtn->width() - 6, 6); closeBtn->raise(); + gamesModel->setUser(currentUser, currentOnline); adjustSize(); - fetchGames(); } // ── Games fetch ─────────────────────────────────────────────────────────────── void UserInfoPopup::fetchGames() { - if (!client || currentUser.isEmpty()) { - return; - } - - Command_GetGamesOfUser cmd; - cmd.set_user_name(currentUser.toStdString()); - - const QString snapshot = currentUser; - PendingCommand *pend = client->prepareSessionCommand(cmd); - connect(pend, &PendingCommand::finished, this, - [this, snapshot](const Response &r) { onGamesReceived(r, snapshot); }); - client->sendCommand(pend); + gamesModel->refresh(); } -void UserInfoPopup::onGamesReceived(const Response &r, const QString &forUser) +void UserInfoPopup::setOnline(bool online) { - if (forUser != currentUser) { - return; // stale response, different user showing now - } - - gamesModel->clear(); - - if (r.response_code() != Response::RespOk) { - gamesStatus->setText(tr("Could not load games.")); - gamesStatus->show(); - gamesView->hide(); + if (currentOnline == online) { return; } + currentOnline = online; + refreshHeader(); + rebuildActionButtons(currentUserInfo, online, currentBuddy, currentIgnored); + gamesModel->setUser(currentUser, online); +} - const auto &resp = r.GetExtension(Response_GetGamesOfUser::ext); - if (resp.game_list_size() == 0) { - gamesStatus->setText(tr("No active games.")); - gamesStatus->show(); - gamesView->hide(); - return; +void UserInfoPopup::updateGamesDisplay() +{ + refreshBtn->setEnabled(gamesModel->canRefresh()); + const auto state = gamesModel->getState(); + const bool ready = state == UserGamesModel::State::Ready; + gamesView->setVisible(ready); + gamesStatus->setVisible(!ready); + + switch (state) { + case UserGamesModel::State::Offline: + gamesStatus->setText(tr("User is offline.")); + break; + case UserGamesModel::State::Disconnected: + gamesStatus->setText(tr("Not connected to the server.")); + break; + case UserGamesModel::State::Loading: + gamesStatus->setText(tr("Loading games…")); + break; + case UserGamesModel::State::Empty: + gamesStatus->setText(tr("No active games.")); + break; + case UserGamesModel::State::Error: + gamesStatus->setText(tr("Could not load games.")); + break; + case UserGamesModel::State::Ready: { + // Fit exactly to the number of visible rows, scroll when more than 5. + constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint + constexpr int maxRows = 5; + const int count = gamesModel->rowCount(); + gamesView->setFixedHeight(qMin(count, maxRows) * rowH + 2); + gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff); + break; + } } - - for (int i = 0; i < resp.game_list_size(); ++i) { - auto *item = new QStandardItem; - item->setData(QVariant::fromValue(resp.game_list(i)), PopupRoles::GameData); - item->setEditable(false); - gamesModel->appendRow(item); - } - - gamesStatus->hide(); - gamesView->show(); - - // Fit exactly to the number of visible rows, scroll when more than 5 - constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint - constexpr int maxRows = 5; - const int count = gamesModel->rowCount(); - const int visible = qMin(count, maxRows); - gamesView->setFixedHeight(visible * rowH + 2); - gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff); - adjustSize(); } void UserInfoPopup::refreshGames() { - gamesModel->clear(); - gamesView->hide(); - gamesStatus->setText(tr("Loading games…")); - gamesStatus->show(); fetchGames(); } diff --git a/cockatrice/src/interface/widgets/server/user/user_info_popup.h b/cockatrice/src/interface/widgets/server/user/user_info_popup.h index f2a91d025..1af4ad0e1 100644 --- a/cockatrice/src/interface/widgets/server/user/user_info_popup.h +++ b/cockatrice/src/interface/widgets/server/user/user_info_popup.h @@ -1,6 +1,7 @@ #ifndef COCKATRICE_USER_INFO_POPUP_H #define COCKATRICE_USER_INFO_POPUP_H +#include "user_games_model.h" #include "user_list_painter.h" #include @@ -21,15 +22,13 @@ class QPushButton; class TabSupervisor; class QListView; class QPoint; -class QStandardItemModel; -class Response; template class QMap; // ── Roles ───────────────────────────────────────────────────────────────────── namespace PopupRoles { -constexpr int GameData = Qt::UserRole + 10; +constexpr int GameData = UserGamesModel::GameRole; } // Popup theme @@ -143,6 +142,7 @@ public: void showForUser(const QString &userName, const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored); void fetchGames(); + void setOnline(bool online); [[nodiscard]] QString getCurrentUser() const { @@ -199,7 +199,7 @@ protected: private slots: void refreshGames(); - void onGamesReceived(const Response &r, const QString &forUser); + void updateGamesDisplay(); void onGamesContextMenu(const QPoint &pos); private: @@ -218,6 +218,8 @@ private: QString currentUser; ServerInfo_User currentUserInfo; bool currentOnline = false; + bool currentBuddy = false; + bool currentIgnored = false; std::function gameInviteAvailable; UserInfoHeaderWidget *header; @@ -225,7 +227,7 @@ private: QLabel *gamesLabel; QFrame *separator; QListView *gamesView; - QStandardItemModel *gamesModel; + UserGamesModel *gamesModel; QLabel *gamesStatus; QPushButton *closeBtn; QPushButton *refreshBtn; diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index a20b39b82..3cb48c49e 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -617,7 +617,14 @@ void UserListWidget::bind(UserListManager *mgr) connect(manager, &UserListManager::addedToIgnoreList, this, refreshCurrentPopup); connect(manager, &UserListManager::removedFromIgnoreList, this, refreshIfPopupOpen); connect(manager, &UserListManager::userJoinedOnline, this, refreshCurrentPopup); - connect(manager, &UserListManager::userLeftOnline, this, refreshIfPopupOpen); + connect(manager, &UserListManager::userLeftOnline, this, [this, refreshIfPopupOpen](const QString &name) { + // The online row may already have been removed; the pinned popup must + // still invalidate its games request and display the offline state. + if (userInfoPopup && userInfoPopup->getCurrentUser() == name) { + userInfoPopup->setOnline(false); + } + refreshIfPopupOpen(name); + }); rebuild(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6acd225c2..4d7a02ea4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -178,6 +178,18 @@ cockatrice_add_test(NAME user_data_backup_test SOURCES user_data_backup_test.cpp cockatrice_add_test(NAME password_hash_test SOURCES password_hash_test.cpp LIBS libcockatrice_utility) cockatrice_add_test(NAME report_categories_test SOURCES report_categories_test.cpp LIBS libcockatrice_utility) cockatrice_add_test(NAME warning_categories_test SOURCES warning_categories_test.cpp LIBS libcockatrice_utility) +cockatrice_add_test( + NAME + user_games_model_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/server/user/user_games_model.cpp + user_games_model_test.cpp + LIBS + libcockatrice_network_client_abstract + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src + ${CMAKE_SOURCE_DIR}/libcockatrice_network +) # The installer's holder probe is Windows-only, but the wording it produces and the # question which programs are the payload's own are plain C++ and are what a stuck user # has to act on, so they are tested everywhere. diff --git a/tests/user_games_model_test.cpp b/tests/user_games_model_test.cpp new file mode 100644 index 000000000..33c29d6d4 --- /dev/null +++ b/tests/user_games_model_test.cpp @@ -0,0 +1,248 @@ +#include "interface/widgets/server/user/user_games_model.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +RNG_Abstract *rng = nullptr; // required by the transitive server library; this suite sends no game commands + +namespace +{ +class GamesClient : public AbstractClient +{ +public: + QList sent; + + GamesClient() + { + setStatus(StatusLoggedIn); + } + + ~GamesClient() override + { + qDeleteAll(pendingCommands); + pendingCommands.clear(); + } + + void changeStatus(ClientStatus status) + { + setStatus(status); + } + + void answer(int request, Response response) + { + response.set_cmd_id(sent.at(request).cmd_id()); + ServerMessage message; + message.set_message_type(ServerMessage::RESPONSE); + *message.mutable_response() = response; + processProtocolItem(message); + } + +protected: + void sendCommandContainer(const CommandContainer &command) override + { + sent.append(command); + } +}; + +Response gamesResponse(std::initializer_list ids) +{ + Response response; + response.set_response_code(Response::RespOk); + auto *games = response.MutableExtension(Response_GetGamesOfUser::ext); + for (int id : ids) { + auto *game = games->add_game_list(); + game->set_game_id(id); + game->set_room_id(7); + } + return response; +} + +using State = UserGamesModel::State; + +TEST(UserGamesModel, OfflineUserDoesNotSendARequest) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", false); + model.refresh(); + EXPECT_TRUE(client.sent.isEmpty()); + EXPECT_EQ(State::Offline, model.getState()); + EXPECT_FALSE(model.canRefresh()); + EXPECT_EQ(0, model.rowCount()); +} + +TEST(UserGamesModel, PresenceLossInvalidatesTheOutstandingResponse) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + EXPECT_EQ(State::Loading, model.getState()); + EXPECT_FALSE(model.canRefresh()); + model.setUser("Alice", false); + client.answer(0, gamesResponse({1})); + EXPECT_EQ(State::Offline, model.getState()); + EXPECT_EQ(0, model.rowCount()); + + model.setUser("Alice", true); + client.answer(1, gamesResponse({2})); + EXPECT_EQ(State::Ready, model.getState()); + EXPECT_TRUE(model.canRefresh()); + EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); +} + +TEST(UserGamesModel, SwitchingBackToTheSameUserDoesNotAcceptTheirOldRequest) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + model.setUser("Bob", true); + model.setUser("Alice", true); + client.answer(2, gamesResponse({3})); + client.answer(0, gamesResponse({1})); + client.answer(1, gamesResponse({2})); + EXPECT_EQ(1, model.rowCount()); + EXPECT_EQ(3, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); +} + +TEST(UserGamesModel, RefreshSupersedesThePreviousRequest) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + model.refresh(); + client.answer(0, gamesResponse({1})); + EXPECT_EQ(State::Loading, model.getState()); + EXPECT_EQ(0, model.rowCount()); + client.answer(1, gamesResponse({2})); + EXPECT_EQ(State::Ready, model.getState()); + EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); +} + +TEST(UserGamesModel, EmptySuccessAndMissingPayloadAreDifferentStates) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + client.answer(0, gamesResponse({})); + EXPECT_EQ(State::Empty, model.getState()); + EXPECT_TRUE(model.canRefresh()); + + model.refresh(); + Response missingPayload; + missingPayload.set_response_code(Response::RespOk); + client.answer(1, missingPayload); + EXPECT_EQ(State::Error, model.getState()); + EXPECT_TRUE(model.canRefresh()); +} + +TEST(UserGamesModel, FailedRequestsCanBeRetried) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + Response error; + error.set_response_code(Response::RespNameNotFound); + client.answer(0, error); + EXPECT_EQ(State::Error, model.getState()); + model.refresh(); + client.answer(1, gamesResponse({4})); + EXPECT_EQ(State::Ready, model.getState()); +} + +TEST(UserGamesModel, DisconnectRejectsLateRepliesAndReconnectFetchesAgain) +{ + GamesClient client; + UserGamesModel model(&client); + model.setUser("Alice", true); + client.changeStatus(StatusDisconnecting); + client.answer(0, gamesResponse({1})); + EXPECT_EQ(State::Disconnected, model.getState()); + EXPECT_FALSE(model.canRefresh()); + model.refresh(); + EXPECT_EQ(1, client.sent.size()); + client.changeStatus(StatusLoggedIn); + client.answer(1, gamesResponse({2})); + EXPECT_EQ(State::Ready, model.getState()); +} + +TEST(UserGamesModel, ClientDestructionClearsGamesAndDisablesRefresh) +{ + auto client = std::make_unique(); + UserGamesModel model(client.get()); + model.setUser("Alice", true); + client->answer(0, gamesResponse({1})); + client.reset(); + EXPECT_EQ(State::Disconnected, model.getState()); + EXPECT_EQ(0, model.rowCount()); + EXPECT_FALSE(model.canRefresh()); + model.refresh(); + EXPECT_EQ(State::Disconnected, model.getState()); +} + +TEST(UserGamesModel, ResetInvalidatesAnOpenGameActionAndPreservesTheModelContract) +{ + GamesClient client; + UserGamesModel model(&client); + QAbstractItemModelTester tester(&model, QAbstractItemModelTester::FailureReportingMode::Fatal); + model.setUser("Alice", true); + client.answer(0, gamesResponse({1, 2, 3})); + QPersistentModelIndex selected(model.index(1)); + EXPECT_TRUE(selected.isValid()); + model.refresh(); + EXPECT_FALSE(selected.isValid()); + EXPECT_EQ(0, model.rowCount()); + client.answer(1, gamesResponse({4})); + EXPECT_EQ(1, model.rowCount()); +} + +TEST(UserGamesModel, StateListenerCanSelectAnotherUserBeforeTheRequestIsSent) +{ + GamesClient client; + UserGamesModel model(&client); + bool switched = false; + QObject::connect(&model, &UserGamesModel::stateChanged, &model, [&] { + if (!switched && model.getState() == State::Loading) { + switched = true; + model.setUser("Bob", false); + } + }); + model.setUser("Alice", true); + EXPECT_TRUE(client.sent.isEmpty()); + EXPECT_EQ(State::Offline, model.getState()); +} + +TEST(UserGamesModel, TimedOutResponseCannotReplaceARetry) +{ + GamesClient client; + UserGamesModel model(&client, 50); + model.setUser("Alice", true); + QSignalSpy changes(&model, &UserGamesModel::stateChanged); + ASSERT_TRUE(changes.wait(1000)); + ASSERT_EQ(State::Error, model.getState()); + EXPECT_TRUE(model.canRefresh()); + model.refresh(); + client.answer(0, gamesResponse({1})); + EXPECT_EQ(State::Loading, model.getState()); + client.answer(1, gamesResponse({2})); + EXPECT_EQ(State::Ready, model.getState()); + EXPECT_EQ(2, model.data(model.index(0), UserGamesModel::GameRole).value().game_id()); +} +} // namespace + +int main(int argc, char **argv) +{ + QCoreApplication app(argc, argv); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 624163916f6a7356653ceaafe35c37e624f4e696 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 23:49:31 -0500 Subject: [PATCH 14/17] [Client] Keep Commander Spellbook requests bound to the active deck (#7722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Keep Commander Spellbook requests bound to the active deck [skip ci] * [Client] Restore the bracket info tooltip when idle retranslateUi() used to install the static "Why this bracket?" tooltip, but it now calls displayEstimate(), which returned early while no estimate existed. That left the info button without its idle tooltip. Set the fallback where displayEstimate() owns the button's tooltip. An existing estimate still wins, and a language change refreshes the text. --------- Co-authored-by: Lukas Brübach --- .../commander_bracket_service.cpp | 5 + .../commander_bracket_service.h | 1 + .../commander_bracket_widget.cpp | 101 +++++++++++++++--- .../commander_bracket_widget.h | 8 ++ .../commander_spellbook_api_accessor.cpp | 49 ++++++--- .../commander_spellbook_api_accessor.h | 10 +- .../handle_commander_brackets.cpp | 49 +++++++-- .../handle_commander_brackets.h | 4 +- 8 files changed, 182 insertions(+), 45 deletions(-) diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp index 8e0448faa..7b60ed23c 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.cpp @@ -26,6 +26,11 @@ quint64 CommanderBracketService::estimateBracket(const DeckList &deck, QObject * return CommanderSpellbookApiAccessor::instance().estimateBracket(deck, requester); } +void CommanderBracketService::cancelEstimate(quint64 requestId) +{ + CommanderSpellbookApiAccessor::instance().cancelEstimate(requestId); +} + void CommanderBracketService::onEstimateBracketFinished(CommanderSpellbookApiAccessor::RequestId id, QObject *requester, const EstimateBracketResult &result) diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h index 22962cc95..00debce86 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_service.h @@ -30,6 +30,7 @@ public: static CommanderBracketService &instance(); quint64 estimateBracket(const DeckList &deck, QObject *requester); + void cancelEstimate(quint64 requestId); signals: void estimateFinished(quint64 requestId, QObject *requester, const CommanderBracketEstimate &estimate); diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp index 5279165af..9adda008e 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.cpp @@ -28,9 +28,18 @@ template class QSharedPointer; CommanderBracketWidget::CommanderBracketWidget(QWidget *parent) : QWidget(parent) { + autoEstimateTimer = new QTimer(this); + autoEstimateTimer->setSingleShot(true); + connect(autoEstimateTimer, &QTimer::timeout, this, [this]() { + if (SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled() == + commanderSpellbookIntegrationEnabledIndexAutomatic) { + requestBracketEstimate(); + } + }); bracketLabel = new QLabel(tr("Bracket:"), this); bracketValueLabel = new QLabel(this); + bracketValueLabel->setTextFormat(Qt::PlainText); bracketValueLabel->setText("-"); bracketValueLabel->setObjectName("bracketValueLabel"); @@ -65,18 +74,38 @@ CommanderBracketWidget::CommanderBracketWidget(QWidget *parent) : QWidget(parent this, &CommanderBracketWidget::maybeAutoEstimateBracket); connect(&SettingsCache::instance().deckEditor(), &DeckEditorSettings::commanderSpellbookIntegrationUseOfficialBracketNamesChanged, this, - &CommanderBracketWidget::maybeAutoEstimateBracket); + &CommanderBracketWidget::displayEstimate); setVisible(false); } +CommanderBracketWidget::~CommanderBracketWidget() +{ + cancelEstimate(); +} + +void CommanderBracketWidget::cancelEstimate() +{ + autoEstimateTimer->stop(); + const quint64 canceledRequest = requestId; + requestId = 0; + CommanderBracketService::instance().cancelEstimate(canceledRequest); + bracketRefreshButton->setEnabled(true); + if (canceledRequest) { + bracketValueLabel->setText("-"); + bracketValueLabel->setToolTip({}); + } +} + void CommanderBracketWidget::setDeck(const QSharedPointer &_deck) { + cancelEstimate(); deck = _deck; - requestId = 0; // invalidate any in-flight estimate for the previous deck + hasEstimate = false; // Reset the displayed bracket bracketValueLabel->setText("-"); + bracketValueLabel->setToolTip({}); bracketInfoButton->setToolTip({}); bracketInfoButton->setEnabled(false); bracketRefreshButton->setEnabled(true); @@ -144,7 +173,7 @@ bool CommanderBracketWidget::promptCommanderSpellbookIntegration() dialog.exec(); // Persist naming choice (if not disabled) - if (clickedButton != disableBtn) { + if (clickedButton == enableBtn || clickedButton == automaticBtn) { bool useOfficial = namingCombo->currentIndex() == 1; SettingsCache::instance().deckEditor().setCommanderSpellbookIntegrationUseOfficialBracketNames(useOfficial); } @@ -176,9 +205,20 @@ void CommanderBracketWidget::updateBracketVisibility(bool visible) void CommanderBracketWidget::requestBracketEstimate() { + const int mode = SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled(); + if (!deck || deck->getGameFormat().compare("commander", Qt::CaseInsensitive) != 0 || requestId || + (mode != commanderSpellbookIntegrationEnabledIndexEnabled && + mode != commanderSpellbookIntegrationEnabledIndexAutomatic)) { + return; + } + + autoEstimateTimer->stop(); + hasEstimate = false; bracketRefreshButton->setEnabled(false); bracketInfoButton->setEnabled(false); + bracketInfoButton->setToolTip({}); bracketValueLabel->setText(tr("Calculating…")); + bracketValueLabel->setToolTip({}); requestId = CommanderBracketService::instance().estimateBracket(*deck, this); } @@ -191,22 +231,35 @@ void CommanderBracketWidget::onEstimateBracketFinished(quint64 id, return; } + requestId = 0; + hasEstimate = true; + lastEstimate = result; + displayEstimate(); + bracketRefreshButton->setEnabled(true); +} + +void CommanderBracketWidget::displayEstimate() +{ + if (!hasEstimate) { + bracketInfoButton->setToolTip(tr("Why this bracket?")); + return; + } + BracketExplainer explainer; - lastBracketExplanation = explainer.explain(result.rawResult); + lastBracketExplanation = explainer.explain(lastEstimate.rawResult); // Display bracket bracketValueLabel->setText( SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationUseOfficialBracketNames() - ? result.officialName - : result.displayName); - bracketRefreshButton->setEnabled(true); + ? lastEstimate.officialName + : lastEstimate.displayName); // Build tooltip QString tooltip; for (const auto §ion : lastBracketExplanation.sections) { - tooltip += "" + section.title + "
"; + tooltip += "" + section.title.toHtmlEscaped() + "
"; for (const auto &line : section.bulletPoints) { - tooltip += "• " + line + "
"; + tooltip += "• " + line.toHtmlEscaped() + "
"; } tooltip += "
"; } @@ -215,13 +268,17 @@ void CommanderBracketWidget::onEstimateBracketFinished(quint64 id, bracketInfoButton->setEnabled(!tooltip.isEmpty()); } -void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *requester, const QString & /*error*/) +void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *requester, const QString &error) { if (requester != this || id != requestId) { return; } + requestId = 0; + hasEstimate = false; bracketValueLabel->setText("-"); + bracketValueLabel->setToolTip( + QStringLiteral("%1").arg(tr("Unable to estimate bracket: %1").arg(error).toHtmlEscaped())); bracketRefreshButton->setEnabled(true); bracketInfoButton->setToolTip({}); bracketInfoButton->setEnabled(false); @@ -229,6 +286,15 @@ void CommanderBracketWidget::onEstimateBracketError(quint64 id, QObject *request void CommanderBracketWidget::maybeAutoEstimateBracket() { + if (prompting) { + return; + } + autoEstimateTimer->stop(); + if (!deck) { + cancelEstimate(); + updateBracketVisibility(false); + return; + } const QString formatKey = deck->getGameFormat(); const bool isCommander = (formatKey.compare("commander", Qt::CaseInsensitive) == 0); @@ -236,17 +302,20 @@ void CommanderBracketWidget::maybeAutoEstimateBracket() int mode = SettingsCache::instance().deckEditor().getCommanderSpellbookIntegrationEnabled(); if (!isCommander || mode == commanderSpellbookIntegrationEnabledIndexDisabled) { + cancelEstimate(); updateBracketVisibility(false); return; } if (mode == commanderSpellbookIntegrationEnabledIndexUnprompted) { - if (prompting) { - return; - } + const auto promptedDeck = deck; prompting = true; const bool accepted = promptCommanderSpellbookIntegration(); prompting = false; + if (deck != promptedDeck || !deck || deck->getGameFormat().compare("commander", Qt::CaseInsensitive) != 0) { + maybeAutoEstimateBracket(); + return; + } if (!accepted) { updateBracketVisibility(false); return; @@ -261,17 +330,17 @@ void CommanderBracketWidget::maybeAutoEstimateBracket() } // Avoid firing if we already have a result or a request in flight - if (!bracketRefreshButton->isEnabled()) { + if (requestId || hasEstimate) { return; } // Defer to avoid races during init / model rebuild - QTimer::singleShot(0, this, &CommanderBracketWidget::requestBracketEstimate); + autoEstimateTimer->start(0); } void CommanderBracketWidget::retranslateUi() { bracketLabel->setText(tr("Bracket:")); - bracketInfoButton->setToolTip(tr("Why this bracket?")); + displayEstimate(); bracketRefreshButton->setToolTip(tr("Recalculate bracket")); } diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h index f5b2b7958..0ef1eedee 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_bracket_widget.h @@ -1,6 +1,7 @@ #ifndef COCKATRICE_COMMANDER_BRACKET_WIDGET_H #define COCKATRICE_COMMANDER_BRACKET_WIDGET_H +#include "commander_bracket_service.h" #include "commander_spellbook_bracket_explainer.h" #include @@ -11,6 +12,7 @@ class QLabel; class QToolButton; +class QTimer; struct CommanderBracketEstimate; class DeckList; class QObject; @@ -22,6 +24,7 @@ class CommanderBracketWidget : public QWidget public: explicit CommanderBracketWidget(QWidget *parent = nullptr); + ~CommanderBracketWidget() override; void setDeck(const QSharedPointer &_deck); void retranslateUi(); @@ -35,10 +38,15 @@ private slots: private: bool promptCommanderSpellbookIntegration(); void updateBracketVisibility(bool visible); + void cancelEstimate(); + void displayEstimate(); QSharedPointer deck; bool prompting = false; quint64 requestId = 0; + QTimer *autoEstimateTimer; + bool hasEstimate = false; + CommanderBracketEstimate lastEstimate; QLabel *bracketLabel; QLabel *bracketValueLabel; diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp index bd8d9379d..8456e2e21 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.cpp @@ -9,9 +9,10 @@ #include #include #include +#include #include #include -#include +#include #include class DeckList; @@ -24,13 +25,18 @@ CommanderSpellbookApiAccessor &CommanderSpellbookApiAccessor::instance() return instance; } -CommanderSpellbookApiAccessor::CommanderSpellbookApiAccessor(QObject *parent) : QObject(parent) +CommanderSpellbookApiAccessor::CommanderSpellbookApiAccessor(QObject *parent) + : QObject(parent), network(NetworkManagerFactory::create(this)) { } CommanderSpellbookApiAccessor::RequestId CommanderSpellbookApiAccessor::estimateBracket(const DeckList &deck, QObject *requester) { + if (!requester) { + return 0; + } + CommanderSpellbookDeckRequest deckRequest = CommanderSpellbookDeckRequest::fromDeckList(deck); QJsonDocument doc(deckRequest.toJson()); @@ -40,30 +46,39 @@ CommanderSpellbookApiAccessor::RequestId CommanderSpellbookApiAccessor::estimate req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); req.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING)); - QNetworkReply *reply = network.post(req, body); + QNetworkReply *reply = network->post(req, body); const RequestId id = nextRequestId++; - reply->setProperty("requestId", QVariant::fromValue(id)); - reply->setProperty("requester", QVariant::fromValue(requester)); - - connect(reply, &QNetworkReply::finished, this, [this, reply]() { onEstimateReplyFinished(reply); }); + pendingRequests.insert(id, reply); + const QPointer guardedRequester(requester); + connect(requester, &QObject::destroyed, reply, [this, id]() { cancelEstimate(id); }); + connect(reply, &QNetworkReply::finished, this, [this, reply, id, guardedRequester]() { + if (pendingRequests.take(id) != reply) { + return; + } + reply->deleteLater(); + if (guardedRequester) { + onEstimateReplyFinished(reply, id, guardedRequester); + } + }); return id; } -void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply) +void CommanderSpellbookApiAccessor::cancelEstimate(RequestId id) { - reply->deleteLater(); - - const RequestId id = reply->property("requestId").toULongLong(); - QObject *requester = reply->property("requester").value(); - - if (!requester) { - // Requester died — silently drop + const QPointer reply = pendingRequests.take(id); + if (!reply) { return; } + disconnect(reply, nullptr, this, nullptr); + reply->abort(); + reply->deleteLater(); +} +void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply, RequestId id, QObject *requester) +{ if (reply->error() != QNetworkReply::NoError) { emit estimateBracketError(id, requester, reply->errorString()); return; @@ -78,6 +93,10 @@ void CommanderSpellbookApiAccessor::onEstimateReplyFinished(QNetworkReply *reply } EstimateBracketResult result = EstimateBracketResult::fromJson(doc.object()); + if (result.bracketTag.trimmed().isEmpty()) { + emit estimateBracketError(id, requester, QStringLiteral("Missing bracket in response")); + return; + } emit estimateBracketFinished(id, requester, result); } diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h index 98e7e3710..a1b2936bb 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/commander_spellbook_api_accessor.h @@ -1,14 +1,16 @@ #ifndef COCKATRICE_COMMANDER_SPELLBOOK_API_ACCESSOR_H #define COCKATRICE_COMMANDER_SPELLBOOK_API_ACCESSOR_H -#include +#include #include +#include #include #include #include class DeckList; class QNetworkReply; +class QNetworkAccessManager; struct EstimateBracketResult; class CommanderSpellbookApiAccessor final : public QObject @@ -21,6 +23,7 @@ public: using RequestId = quint64; RequestId estimateBracket(const DeckList &deck, QObject *requester); + void cancelEstimate(RequestId id); signals: void estimateBracketFinished(RequestId id, QObject *requester, const EstimateBracketResult &result); @@ -31,9 +34,10 @@ private: explicit CommanderSpellbookApiAccessor(QObject *parent = nullptr); Q_DISABLE_COPY_MOVE(CommanderSpellbookApiAccessor) - void onEstimateReplyFinished(QNetworkReply *reply); + void onEstimateReplyFinished(QNetworkReply *reply, RequestId id, QObject *requester); - QNetworkAccessManager network; + QNetworkAccessManager *network; + QHash> pendingRequests; RequestId nextRequestId = 1; }; diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp index 407f53e99..be490ce37 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp @@ -3,11 +3,14 @@ #include "../../../../../client/settings/cache_settings.h" #include +#include #include +#include #include #include #include #include +#include #include #include #include @@ -22,6 +25,14 @@ HandleCommanderBrackets::HandleCommanderBrackets(QObject *parent) { } +HandleCommanderBrackets::~HandleCommanderBrackets() +{ + if (reply) { + disconnect(reply, nullptr, this, nullptr); + reply->abort(); + } +} + void HandleCommanderBrackets::downloadBracketDefinitions() { if (reply) { @@ -35,31 +46,49 @@ void HandleCommanderBrackets::downloadBracketDefinitions() void HandleCommanderBrackets::actFinishParsingDownloadedData() { - reply = qobject_cast(sender()); + auto *finishedReply = qobject_cast(sender()); + if (!finishedReply || finishedReply != reply) { + return; + } - if (reply->error() != QNetworkReply::NoError) { - emit sigBracketDefinitionsDownloadFailed(reply->error()); + // Clear the active request before notifying listeners, which may immediately retry. + reply = nullptr; + finishedReply->deleteLater(); - reply->deleteLater(); + if (finishedReply->error() != QNetworkReply::NoError) { + emit sigBracketDefinitionsDownloadFailed(finishedReply->error()); return; } QJsonParseError parseError; - auto document = QJsonDocument::fromJson(reply->readAll(), &parseError); + auto document = QJsonDocument::fromJson(finishedReply->readAll(), &parseError); - if (parseError.error != QJsonParseError::NoError) { + if (parseError.error != QJsonParseError::NoError || !document.isObject() || + !document.object().value("brackets").isArray() || document.object().value("brackets").toArray().isEmpty()) { emit sigBracketDefinitionsDownloadFailed(QNetworkReply::UnknownContentError); - - reply->deleteLater(); return; } + // Validate the complete replacement before touching the last usable definitions. + // Tags are QSettings group names, so path separators must not create nested groups. + QSet tags; + for (const auto &value : document.object().value("brackets").toArray()) { + const auto definition = value.toObject(); + const auto tag = definition.value("tag").toString(); + if (!value.isObject() || tag.trimmed().isEmpty() || tag.contains('/') || tag.contains('\\') || + tags.contains(tag) || definition.value("officialName").toString().trimmed().isEmpty() || + definition.value("displayName").toString().trimmed().isEmpty() || + !definition.value("explanation").isString()) { + emit sigBracketDefinitionsDownloadFailed(QNetworkReply::UnknownContentError); + return; + } + tags.insert(tag); + } + updateBracketDefinitions(document.toVariant().toMap()); emit sigBracketDefinitionsDownloaded(); - - reply->deleteLater(); } void HandleCommanderBrackets::updateBracketDefinitions(const QVariantMap &jsonMap) diff --git a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h index 39bddba05..99b15832c 100644 --- a/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h +++ b/cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -14,6 +15,7 @@ class HandleCommanderBrackets : public QObject public: explicit HandleCommanderBrackets(QObject *parent = nullptr); + ~HandleCommanderBrackets() override; void downloadBracketDefinitions(); @@ -28,7 +30,7 @@ private: void updateBracketDefinitions(const QVariantMap &jsonMap); QNetworkAccessManager *nam; - QNetworkReply *reply; + QPointer reply; }; #endif // COCKATRICE_HANDLE_COMMANDER_BRACKETS_H From 81a0b9724025e05ef32b3afd7ade66d79a53b513 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Wed, 4 Nov 2026 05:56:32 +0100 Subject: [PATCH 15/17] [Replay] Seek to the exact time of an event (#7610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Timeline seeks only expose whole timer intervals, so jumping to an event could leave that event unprocessed until the next tick. Add an event-time seek that also applies events at the target timestamp. Co-authored-by: Lukas Brübach --- .../widgets/replay/replay_manager.cpp | 44 ++++++++++++++----- .../interface/widgets/replay/replay_manager.h | 9 ++-- .../widgets/replay/replay_widget.cpp | 11 +++++ .../interface/widgets/replay/replay_widget.h | 7 +++ 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/cockatrice/src/interface/widgets/replay/replay_manager.cpp b/cockatrice/src/interface/widgets/replay/replay_manager.cpp index 8ba0cb653..b01d7f9d6 100644 --- a/cockatrice/src/interface/widgets/replay/replay_manager.cpp +++ b/cockatrice/src/interface/widgets/replay/replay_manager.cpp @@ -57,7 +57,7 @@ ReplayManager::ReplayManager(QObject *parent, GameReplay *replay) rewindBufferingTimer = new QTimer(this); rewindBufferingTimer->setSingleShot(true); - connect(rewindBufferingTimer, &QTimer::timeout, this, &ReplayManager::processRewind); + connect(rewindBufferingTimer, &QTimer::timeout, this, [this] { processRewind(); }); } ReplayManager::~ReplayManager() @@ -65,7 +65,7 @@ ReplayManager::~ReplayManager() delete replay; } -void ReplayManager::skipToTime(int newTime, bool doRewindBuffering) +void ReplayManager::skipToTime(int newTime, bool doRewindBuffering, bool includeEventsAtTime) { // check boundary conditions if (newTime < 0) { @@ -75,15 +75,17 @@ void ReplayManager::skipToTime(int newTime, bool doRewindBuffering) newTime = maxTime; } - newTime -= newTime % TIMER_INTERVAL_MS; // Time should always be a multiple of the interval + if (!includeEventsAtTime) { + newTime -= newTime % TIMER_INTERVAL_MS; // Time should always be a multiple of the interval + } const bool isBackwardsSkip = newTime < currentProcessedTime; currentVisualTime = newTime; if (isBackwardsSkip) { - handleBackwardsSkip(doRewindBuffering); + handleBackwardsSkip(doRewindBuffering, includeEventsAtTime); } else { - processNewEvents(FORWARD_SKIP); + processNewEvents(FORWARD_SKIP, includeEventsAtTime); } timeChanged(currentVisualTime); @@ -95,7 +97,7 @@ void ReplayManager::skipToTime(int newTime, bool doRewindBuffering) * @param doRewindBuffering When true, if multiple backward skips are made in quick succession, only a single rewind * is processed at the end. When false, the backwards skip will always cause an immediate rewind. */ -void ReplayManager::handleBackwardsSkip(bool doRewindBuffering) +void ReplayManager::handleBackwardsSkip(bool doRewindBuffering, bool includeEventsAtTime) { if (doRewindBuffering) { // We use a one-shot timer to implement the rewind buffering. @@ -105,11 +107,11 @@ void ReplayManager::handleBackwardsSkip(bool doRewindBuffering) rewindBufferingTimer->start(SettingsCache::instance().userInterface().getRewindBufferingMs()); } else { // otherwise, process the rewind immediately - processRewind(); + processRewind(includeEventsAtTime); } } -void ReplayManager::processRewind() +void ReplayManager::processRewind(bool includeEventsAtTime) { // stop any queued-up rewinds rewindBufferingTimer->stop(); @@ -117,7 +119,7 @@ void ReplayManager::processRewind() // process the rewind currentEvent = 0; emit rewound(); - processNewEvents(BACKWARD_SKIP); + processNewEvents(BACKWARD_SKIP, includeEventsAtTime); } void ReplayManager::replayTimerTimeout() @@ -134,11 +136,13 @@ void ReplayManager::replayTimerTimeout() } /** @brief Processes all unprocessed events up to the current time. */ -void ReplayManager::processNewEvents(PlaybackMode playbackMode) +void ReplayManager::processNewEvents(PlaybackMode playbackMode, bool includeEventsAtTime) { currentProcessedTime = currentVisualTime; - while (currentEvent < replayTimeline.size() && replayTimeline[currentEvent] < currentProcessedTime) { + while (currentEvent < replayTimeline.size() && + (replayTimeline[currentEvent] < currentProcessedTime || + (includeEventsAtTime && replayTimeline[currentEvent] == currentProcessedTime))) { EventProcessingOptions options; // backwards skip => always skip reveal windows @@ -236,6 +240,24 @@ void ReplayManager::setTime(int time) skipToTime(time, false); } +void ReplayManager::setTimeToEvent(int eventIndex) +{ + if (eventIndex < 0 || eventIndex >= replayTimeline.size()) { + return; + } + + // Flush a pending buffered rewind before seeking, otherwise its timer fires after this seek and rebuilds the + // replay state from an outdated currentVisualTime, dropping the event we seek to. Stopping the timer instead + // of processing it is not enough: playback ticks advance currentProcessedTime without rewinding currentEvent, + // so the board could still be ahead of the target. + if (rewindBufferingTimer->isActive()) { + processRewind(); + } + + // Unlike a timeline seek, jumping to an event should apply that event immediately. + skipToTime(replayTimeline.at(eventIndex), false, true); +} + void ReplayManager::skipByAmount(int amount) { skipToTime(currentVisualTime + amount, amount < 0); diff --git a/cockatrice/src/interface/widgets/replay/replay_manager.h b/cockatrice/src/interface/widgets/replay/replay_manager.h index 3849b86ae..34bae5f9a 100644 --- a/cockatrice/src/interface/widgets/replay/replay_manager.h +++ b/cockatrice/src/interface/widgets/replay/replay_manager.h @@ -40,10 +40,10 @@ class ReplayManager : public QObject ///< to rewind buffering int currentEvent = 0; ///< current event's index - void skipToTime(int newTime, bool doRewindBuffering); - void handleBackwardsSkip(bool doRewindBuffering); - void processRewind(); - void processNewEvents(PlaybackMode playbackMode); + void skipToTime(int newTime, bool doRewindBuffering, bool includeEventsAtTime = false); + void handleBackwardsSkip(bool doRewindBuffering, bool includeEventsAtTime = false); + void processRewind(bool includeEventsAtTime = false); + void processNewEvents(PlaybackMode playbackMode, bool includeEventsAtTime = false); void handleSkipEmptySection(); private slots: @@ -73,6 +73,7 @@ public slots: void startReplay(); void stopReplay(); void setTime(int time); + void setTimeToEvent(int eventIndex); void skipByAmount(int amount); // use a negative amount to skip backwards signals: diff --git a/cockatrice/src/interface/widgets/replay/replay_widget.cpp b/cockatrice/src/interface/widgets/replay/replay_widget.cpp index 07b4e72ce..3f0bf35d9 100644 --- a/cockatrice/src/interface/widgets/replay/replay_widget.cpp +++ b/cockatrice/src/interface/widgets/replay/replay_widget.cpp @@ -28,6 +28,7 @@ ReplayWidget::ReplayWidget(QWidget *parent, GameReplay *replay) connect(replayManager, &ReplayManager::eventReplayed, this, &ReplayWidget::eventReplayed); connect(replayManager, &ReplayManager::replayFinished, this, &ReplayWidget::replayFinished); connect(replayManager, &ReplayManager::rewound, this, &ReplayWidget::rewound); + connect(replayManager, &ReplayManager::timeChanged, this, &ReplayWidget::timeChanged); // timeline widget timelineWidget = new ReplayTimelineWidget; @@ -96,6 +97,16 @@ ReplayWidget::ReplayWidget(QWidget *parent, GameReplay *replay) refreshShortcuts(); } +const QList &ReplayWidget::getReplayTimeline() const +{ + return replayManager->getReplayTimeline(); +} + +void ReplayWidget::seekToEvent(int eventIndex) +{ + replayManager->setTimeToEvent(eventIndex); +} + void ReplayWidget::replayFinished() { replayPlayButton->setChecked(false); diff --git a/cockatrice/src/interface/widgets/replay/replay_widget.h b/cockatrice/src/interface/widgets/replay/replay_widget.h index 0e78442ce..5d23b358d 100644 --- a/cockatrice/src/interface/widgets/replay/replay_widget.h +++ b/cockatrice/src/interface/widgets/replay/replay_widget.h @@ -8,6 +8,7 @@ #ifndef REPLAY_WIDGET_H #define REPLAY_WIDGET_H +#include #include #include #include @@ -35,8 +36,14 @@ public: */ ReplayWidget(QWidget *parent, GameReplay *replay); + [[nodiscard]] const QList &getReplayTimeline() const; + +public slots: + void seekToEvent(int eventIndex); + signals: void rewound(); + void timeChanged(int time); void eventReplayed(const GameEventContainer &cont, EventProcessingOptions options); private: From 3e4b8cabbe7b0183ef4c21c8c4ff2344533ee9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 20 Oct 2026 22:14:34 +0200 Subject: [PATCH 16/17] [Replay] Build a compact event history index Turn replay protocol events into short localized summaries with an actor, badge, and time so the history UI can render them without retaining the replay or duplicating the chat log. --- cockatrice/CMakeLists.txt | 1 + .../widgets/replay/replay_event_index.cpp | 876 ++++++++++++++++++ .../widgets/replay/replay_event_index.h | 43 + 3 files changed, 920 insertions(+) create mode 100644 cockatrice/src/interface/widgets/replay/replay_event_index.cpp create mode 100644 cockatrice/src/interface/widgets/replay/replay_event_index.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index ccf2dfcea..d7906a418 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -249,6 +249,7 @@ set(cockatrice_SOURCES src/interface/widgets/playmat/playmat_settings_dialog.cpp src/interface/widgets/quick_settings/settings_button_widget.cpp src/interface/widgets/quick_settings/settings_popup_widget.cpp + src/interface/widgets/replay/replay_event_index.cpp src/interface/widgets/replay/replay_manager.cpp src/interface/widgets/replay/replay_quick_settings_widget.cpp src/interface/widgets/replay/replay_timeline_widget.cpp diff --git a/cockatrice/src/interface/widgets/replay/replay_event_index.cpp b/cockatrice/src/interface/widgets/replay/replay_event_index.cpp new file mode 100644 index 000000000..d21671b5a --- /dev/null +++ b/cockatrice/src/interface/widgets/replay/replay_event_index.cpp @@ -0,0 +1,876 @@ +#include "replay_event_index.h" + +#include "libcockatrice/protocol/get_pb_extension.h" +#include "libcockatrice/protocol/pb/card_attributes.pb.h" +#include "libcockatrice/protocol/pb/context_concede.pb.h" +#include "libcockatrice/protocol/pb/context_connection_state_changed.pb.h" +#include "libcockatrice/protocol/pb/context_deck_select.pb.h" +#include "libcockatrice/protocol/pb/context_mulligan.pb.h" +#include "libcockatrice/protocol/pb/context_ready_start.pb.h" +#include "libcockatrice/protocol/pb/context_set_sideboard_lock.pb.h" +#include "libcockatrice/protocol/pb/context_undo_draw.pb.h" +#include "libcockatrice/protocol/pb/event_attach_card.pb.h" +#include "libcockatrice/protocol/pb/event_change_zone_properties.pb.h" +#include "libcockatrice/protocol/pb/event_create_counter.pb.h" +#include "libcockatrice/protocol/pb/event_create_token.pb.h" +#include "libcockatrice/protocol/pb/event_del_counter.pb.h" +#include "libcockatrice/protocol/pb/event_destroy_card.pb.h" +#include "libcockatrice/protocol/pb/event_draw_cards.pb.h" +#include "libcockatrice/protocol/pb/event_dump_zone.pb.h" +#include "libcockatrice/protocol/pb/event_flip_card.pb.h" +#include "libcockatrice/protocol/pb/event_game_closed.pb.h" +#include "libcockatrice/protocol/pb/event_game_host_changed.pb.h" +#include "libcockatrice/protocol/pb/event_game_log_notice.pb.h" +#include "libcockatrice/protocol/pb/event_game_say.pb.h" +#include "libcockatrice/protocol/pb/event_game_state_changed.pb.h" +#include "libcockatrice/protocol/pb/event_join.pb.h" +#include "libcockatrice/protocol/pb/event_kicked.pb.h" +#include "libcockatrice/protocol/pb/event_leave.pb.h" +#include "libcockatrice/protocol/pb/event_move_card.pb.h" +#include "libcockatrice/protocol/pb/event_player_properties_changed.pb.h" +#include "libcockatrice/protocol/pb/event_reveal_cards.pb.h" +#include "libcockatrice/protocol/pb/event_reverse_turn.pb.h" +#include "libcockatrice/protocol/pb/event_roll_die.pb.h" +#include "libcockatrice/protocol/pb/event_set_active_phase.pb.h" +#include "libcockatrice/protocol/pb/event_set_active_player.pb.h" +#include "libcockatrice/protocol/pb/event_set_card_attr.pb.h" +#include "libcockatrice/protocol/pb/event_set_card_counter.pb.h" +#include "libcockatrice/protocol/pb/event_set_counter.pb.h" +#include "libcockatrice/protocol/pb/event_shuffle.pb.h" +#include "libcockatrice/protocol/pb/game_event.pb.h" +#include "libcockatrice/protocol/pb/game_event_container.pb.h" +#include "libcockatrice/protocol/pb/game_event_context.pb.h" +#include "libcockatrice/protocol/pb/game_replay.pb.h" +#include "libcockatrice/protocol/pb/serverinfo_card.pb.h" +#include "libcockatrice/protocol/pb/serverinfo_counter.pb.h" +#include "libcockatrice/protocol/pb/serverinfo_player.pb.h" +#include "libcockatrice/protocol/pb/serverinfo_playerproperties.pb.h" +#include "libcockatrice/protocol/pb/serverinfo_zone.pb.h" +#include "libcockatrice/utility/zone_names.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ +struct PendingDetail +{ + QString text; + QString badge; + ReplayEventKind kind = ReplayEventKind::Action; + QString actor; +}; + +QString formatTime(int timeMs) +{ + const int totalSeconds = qMax(0, timeMs) / 1000; + const int hours = totalSeconds / 3600; + const int minutes = (totalSeconds / 60) % 60; + const int seconds = totalSeconds % 60; + + if (hours > 0) { + return QStringLiteral("%1:%2:%3") + .arg(hours) + .arg(minutes, 2, 10, QLatin1Char('0')) + .arg(seconds, 2, 10, QLatin1Char('0')); + } + + return QStringLiteral("%1:%2").arg(minutes, 2, 10, QLatin1Char('0')).arg(seconds, 2, 10, QLatin1Char('0')); +} + +class ReplayEventIndexBuilder +{ + Q_DECLARE_TR_FUNCTIONS(ReplayEventIndex) + +public: + ReplayEventIndexBuilder(const GameReplay *replay, const QList &timeline) : replay(replay), timeline(timeline) + { + } + + QList build() + { + QList entries; + + for (int i = 0; i < replay->event_list_size(); ++i) { + const GameEventContainer &container = replay->event_list(i); + const QList details = describeContainer(container); + if (details.isEmpty()) { + continue; + } + + ReplayEventEntry entry; + entry.eventIndex = i; + entry.timeMs = timeline.value(i, static_cast(container.seconds_elapsed()) * 1000); + entry.timeLabel = formatTime(entry.timeMs); + entry.kind = details.first().kind; + entry.badge = details.first().badge; + if (details.size() > 1) { + entry.badge += QStringLiteral(" +%1").arg(details.size() - 1); + } + + const QString sharedActor = details.first().actor; + const bool mixedActors = std::any_of(details.cbegin(), details.cend(), [&sharedActor](const auto &detail) { + return detail.actor != sharedActor; + }); + + QStringList detailTexts; + for (const PendingDetail &detail : details) { + if (mixedActors && !detail.actor.isEmpty()) { + detailTexts.append(tr("%1: %2").arg(detail.actor, detail.text)); + } else { + detailTexts.append(detail.text); + } + } + entry.actor = mixedActors ? QString() : sharedActor; + entry.detail = detailTexts.join(QStringLiteral(" • ")); + + entries.append(entry); + } + + return entries; + } + +private: + const GameReplay *replay; + const QList &timeline; + + QHash playerNames; + QHash spectators; + QHash cardNames; + QHash counterNames; + + int activePlayerId = -1; + bool gameStarted = false; + bool sawGameState = false; + + static QString cardKey(int playerId, const QString &zone, int cardId) + { + return QStringLiteral("%1/%2/%3").arg(playerId).arg(zone).arg(cardId); + } + + static QString counterKey(int playerId, int counterId) + { + return QStringLiteral("%1/%2").arg(playerId).arg(counterId); + } + + QString playerName(int playerId) const + { + if (playerId < 0) { + return tr("Game"); + } + return playerNames.value(playerId, tr("Player %1").arg(playerId)); + } + + QString actorName(int playerId) const + { + if (playerId < 0) { + return QString(); + } + return playerNames.value(playerId, tr("Player %1").arg(playerId)); + } + + bool isSpectator(int playerId) const + { + return spectators.value(playerId, false); + } + + void rememberPlayer(const ServerInfo_PlayerProperties &properties) + { + const int playerId = properties.player_id(); + if (playerId < 0) { + return; + } + + const QString name = QString::fromStdString(properties.user_info().name()); + if (!name.isEmpty()) { + playerNames.insert(playerId, name); + } + spectators.insert(playerId, properties.spectator()); + } + + void rememberCard(int playerId, const QString &zone, int cardId, const QString &name) + { + if (cardId >= 0 && !name.isEmpty()) { + cardNames.insert(cardKey(playerId, zone, cardId), name); + } + } + + void forgetCard(int playerId, const QString &zone, int cardId) + { + if (cardId >= 0) { + cardNames.remove(cardKey(playerId, zone, cardId)); + } + } + + QString cardName(int playerId, const QString &zone, int cardId) const + { + if (cardId < 0) { + return QString(); + } + return cardNames.value(cardKey(playerId, zone, cardId)); + } + + void rememberCounter(int playerId, const ServerInfo_Counter &counter) + { + const QString name = QString::fromStdString(counter.name()); + if (!name.isEmpty()) { + counterNames.insert(counterKey(playerId, counter.id()), name); + } + } + + QString counterName(int playerId, int counterId) const + { + return counterNames.value(counterKey(playerId, counterId)); + } + + QString zoneName(const QString &zone) const + { + if (zone == ZoneNames::TABLE) { + return tr("battlefield"); + } + if (zone == ZoneNames::GRAVE) { + return tr("graveyard"); + } + if (zone == ZoneNames::EXILE) { + return tr("exile"); + } + if (zone == ZoneNames::HAND) { + return tr("hand"); + } + if (zone == ZoneNames::DECK) { + return tr("library"); + } + if (zone == ZoneNames::SIDEBOARD) { + return tr("sideboard"); + } + if (zone == ZoneNames::STACK) { + return tr("stack"); + } + return zone; + } + + QString zonePhrase(int playerId, const QString &zone, int actorId) const + { + if (zone == ZoneNames::TABLE) { + return tr("the battlefield"); + } + if (zone == ZoneNames::EXILE) { + return tr("exile"); + } + if (zone == ZoneNames::STACK) { + return tr("the stack"); + } + + const QString name = zoneName(zone); + if (playerId < 0 || playerId == actorId) { + return tr("their %1").arg(name); + } + return tr("%1's %2").arg(playerName(playerId), name); + } + + QString formatCardList(const QStringList &names, int total) const + { + // Keep repeats: each entry is a drawn/revealed card, so the list length must agree with the count. + QStringList listedNames; + for (const QString &name : names) { + if (!name.isEmpty()) { + listedNames.append(name); + } + } + + if (listedNames.isEmpty()) { + if (total <= 0) { + return tr("no cards"); + } + return total == 1 ? tr("1 card") : tr("%1 cards").arg(total); + } + + if (total < listedNames.size()) { + total = listedNames.size(); + } + + if (listedNames.size() > 3) { + return tr("%1 and %2 more").arg(listedNames.mid(0, 3).join(QStringLiteral(", "))).arg(total - 3); + } + if (total > listedNames.size()) { + return tr("%1 and %2 more").arg(listedNames.join(QStringLiteral(", "))).arg(total - listedNames.size()); + } + + return listedNames.join(QStringLiteral(", ")); + } + + void updateGameState(const Event_GameStateChanged &event) + { + for (int i = 0; i < event.player_list_size(); ++i) { + const ServerInfo_Player &player = event.player_list(i); + const int playerId = player.properties().player_id(); + rememberPlayer(player.properties()); + + for (int j = 0; j < player.zone_list_size(); ++j) { + const ServerInfo_Zone &zone = player.zone_list(j); + for (int k = 0; k < zone.card_list_size(); ++k) { + const ServerInfo_Card &card = zone.card_list(k); + rememberCard(playerId, QString::fromStdString(zone.name()), card.id(), + QString::fromStdString(card.name())); + } + } + + for (int j = 0; j < player.counter_list_size(); ++j) { + rememberCounter(playerId, player.counter_list(j)); + } + } + + if (event.has_active_player_id()) { + activePlayerId = event.active_player_id(); + } + if (event.has_game_started()) { + gameStarted = event.game_started(); + } + } + + QString describeMoveCard(const Event_MoveCard &event, int actorId, const GameEventContext &context) + { + const int startPlayerId = event.start_player_id() >= 0 ? event.start_player_id() : actorId; + const int targetPlayerId = event.target_player_id() >= 0 ? event.target_player_id() : startPlayerId; + const QString startZone = QString::fromStdString(event.start_zone()); + const QString targetZone = event.has_target_zone() ? QString::fromStdString(event.target_zone()) : startZone; + + QString name; + if (event.has_card_name()) { + name = QString::fromStdString(event.card_name()); + } + if (name.isEmpty()) { + name = cardName(startPlayerId, startZone, event.card_id()); + } + + const bool knownName = !name.isEmpty(); + if (name.isEmpty()) { + name = event.face_down() ? tr("a face-down card") : tr("a card"); + } + + const int newCardId = event.has_new_card_id() ? event.new_card_id() : event.card_id(); + forgetCard(startPlayerId, startZone, event.card_id()); + if (knownName) { + rememberCard(targetPlayerId, targetZone, newCardId, name); + } + + if (startZone == targetZone) { + return tr("moved %1 within %2.").arg(name, zonePhrase(targetPlayerId, targetZone, actorId)); + } + if (context.HasExtension(Context_UndoDraw::ext)) { + return tr("returned %1 to %2.").arg(name, zonePhrase(targetPlayerId, targetZone, actorId)); + } + + return tr("moved %1 from %2 to %3.") + .arg(name, zonePhrase(startPlayerId, startZone, actorId), zonePhrase(targetPlayerId, targetZone, actorId)); + } + + QString describeSetCardAttribute(const Event_SetCardAttr &event, int actorId) + { + const QString zone = QString::fromStdString(event.zone_name()); + QString name; + if (event.has_card_id()) { + name = cardName(actorId, zone, event.card_id()); + } + if (name.isEmpty()) { + name = event.has_card_id() ? tr("a card") : tr("all cards"); + } + + const QString value = QString::fromStdString(event.attr_value()); + switch (event.attribute()) { + case AttrTapped: + return value == QLatin1String("1") ? tr("tapped %1.").arg(name) : tr("untapped %1.").arg(name); + case AttrAttacking: + return value == QLatin1String("1") ? tr("%1 is now attacking.").arg(name) + : tr("%1 is no longer attacking.").arg(name); + case AttrFaceDown: + return value == QLatin1String("1") ? tr("turned %1 face down.").arg(name) + : tr("turned %1 face up.").arg(name); + case AttrColor: + return tr("changed %1's color to %2.").arg(name, value); + case AttrPT: + return tr("set %1's power/toughness to %2.").arg(name, value); + case AttrAnnotation: + return value.isEmpty() ? tr("cleared %1's annotation.").arg(name) + : tr("annotated %1: %2").arg(name, value); + case AttrDoesntUntap: + return value == QLatin1String("1") ? tr("%1 will no longer untap.").arg(name) + : tr("%1 will untap normally again.").arg(name); + } + + return tr("changed %1.").arg(name); + } + + QString badgeForCardAttribute(CardAttribute attribute) const + { + switch (attribute) { + case AttrTapped: + return tr("TAP"); + case AttrAttacking: + return tr("ATTACK"); + case AttrFaceDown: + return tr("FLIP"); + case AttrColor: + return tr("COLOR"); + case AttrPT: + return tr("PT"); + case AttrAnnotation: + return tr("NOTE"); + case AttrDoesntUntap: + return tr("TAP"); + } + return tr("CARD"); + } + + QString describeLeaveReason(Event_Leave::LeaveReason reason) const + { + switch (reason) { + case Event_Leave::USER_KICKED: + return tr("kicked by game host or moderator"); + case Event_Leave::USER_LEFT: + return tr("left the game"); + case Event_Leave::USER_DISCONNECTED: + return tr("disconnected"); + case Event_Leave::OTHER: + default: + return tr("reason unknown"); + } + } + + void addDetail(QList &details, + const QString &text, + const QString &badge, + ReplayEventKind kind, + int actorId, + bool forcedByJudge) const + { + if (text.isEmpty()) { + return; + } + + const QString finalText = forcedByJudge ? tr("%1 (judge)").arg(text) : text; + details.append(PendingDetail{.text = finalText, .badge = badge, .kind = kind, .actor = actorName(actorId)}); + } + + QList describeContainer(const GameEventContainer &container) + { + QList details; + const GameEventContext &context = container.context(); + const bool forcedByJudge = container.has_forced_by_judge(); + const auto contextType = static_cast(getPbExtension(context)); + + for (int i = 0; i < container.event_list_size(); ++i) { + const GameEvent &event = container.event_list(i); + const int eventPlayerId = event.player_id(); + const auto eventType = static_cast(getPbExtension(event)); + + switch (eventType) { + case GameEvent::GAME_STATE_CHANGED: { + const Event_GameStateChanged &state = event.GetExtension(Event_GameStateChanged::ext); + const bool wasStarted = gameStarted; + updateGameState(state); + if (state.game_started() && !wasStarted) { + addDetail(details, tr("Game started."), tr("GAME"), ReplayEventKind::System, -1, forcedByJudge); + } else if (!sawGameState) { + addDetail(details, tr("Game state loaded."), tr("GAME"), ReplayEventKind::System, -1, + forcedByJudge); + } + sawGameState = true; + break; + } + case GameEvent::PLAYER_PROPERTIES_CHANGED: { + const Event_PlayerPropertiesChanged &propertiesEvent = + event.GetExtension(Event_PlayerPropertiesChanged::ext); + const bool hasProperties = propertiesEvent.has_player_properties(); + const ServerInfo_PlayerProperties &properties = propertiesEvent.player_properties(); + // The server only fills in the field that changed, so player_id is absent from these events + // unless they are about a specific player; fall back to the event's own player id. + const bool describesPlayer = hasProperties && properties.has_player_id(); + const int playerId = describesPlayer ? properties.player_id() : eventPlayerId; + if (describesPlayer) { + rememberPlayer(properties); + } + + switch (contextType) { + case GameEventContext::READY_START: + addDetail(details, + properties.ready_start() ? tr("is ready to start.") : tr("is no longer ready."), + tr("READY"), ReplayEventKind::System, playerId, forcedByJudge); + break; + case GameEventContext::CONCEDE: + addDetail(details, tr("conceded the game."), tr("CONCEDE"), ReplayEventKind::System, + playerId, forcedByJudge); + break; + case GameEventContext::UNCONCEDE: + addDetail(details, tr("unconceded the game."), tr("CONCEDE"), ReplayEventKind::System, + playerId, forcedByJudge); + break; + case GameEventContext::DECK_SELECT: { + const Context_DeckSelect &deckSelect = context.GetExtension(Context_DeckSelect::ext); + const int sideboardSize = deckSelect.sideboard_size(); + addDetail(details, + sideboardSize >= 0 + ? tr("selected a deck with %1 sideboard cards.").arg(sideboardSize) + : tr("selected a deck."), + tr("DECK"), ReplayEventKind::System, playerId, forcedByJudge); + break; + } + case GameEventContext::MULLIGAN: { + const Context_Mulligan &mulligan = context.GetExtension(Context_Mulligan::ext); + addDetail(details, + mulligan.number() > 0 ? tr("took a mulligan (%1).").arg(mulligan.number()) + : tr("took a mulligan."), + tr("MULLIGAN"), ReplayEventKind::System, playerId, forcedByJudge); + break; + } + case GameEventContext::SET_SIDEBOARD_LOCK: + addDetail(details, + properties.sideboard_locked() ? tr("locked their sideboard.") + : tr("unlocked their sideboard."), + tr("SIDEBOARD"), ReplayEventKind::System, playerId, forcedByJudge); + break; + case GameEventContext::CONNECTION_STATE_CHANGED: + addDetail(details, + properties.ping_seconds() != -1 ? tr("reconnected to the game.") + : tr("lost connection to the game."), + tr("CONNECTION"), ReplayEventKind::System, playerId, forcedByJudge); + break; + default: + break; + } + break; + } + case GameEvent::JOIN: { + const Event_Join &join = event.GetExtension(Event_Join::ext); + if (join.has_player_properties()) { + rememberPlayer(join.player_properties()); + } + const int playerId = join.player_properties().player_id() >= 0 + ? join.player_properties().player_id() + : eventPlayerId; + addDetail(details, isSpectator(playerId) ? tr("joined as a spectator.") : tr("joined the game."), + tr("JOIN"), ReplayEventKind::System, playerId, forcedByJudge); + break; + } + case GameEvent::LEAVE: { + const Event_Leave &leave = event.GetExtension(Event_Leave::ext); + const QString reason = describeLeaveReason(leave.reason()); + if (eventPlayerId < 0) { + addDetail(details, tr("A player left the game (%1).").arg(reason), tr("LEAVE"), + ReplayEventKind::System, -1, forcedByJudge); + } else { + addDetail(details, + isSpectator(eventPlayerId) ? tr("stopped spectating (%1).").arg(reason) + : tr("left the game (%1).").arg(reason), + tr("LEAVE"), ReplayEventKind::System, eventPlayerId, forcedByJudge); + } + break; + } + case GameEvent::KICKED: + addDetail(details, + eventPlayerId >= 0 ? tr("was kicked from the game.") + : tr("A player was kicked from the game."), + tr("KICK"), ReplayEventKind::System, eventPlayerId, forcedByJudge); + break; + case GameEvent::GAME_CLOSED: + addDetail(details, tr("Game closed."), tr("GAME"), ReplayEventKind::System, -1, forcedByJudge); + break; + case GameEvent::GAME_HOST_CHANGED: + addDetail(details, eventPlayerId >= 0 ? tr("is now the game host.") : tr("The game host changed."), + tr("HOST"), ReplayEventKind::System, eventPlayerId, forcedByJudge); + break; + case GameEvent::GAME_SAY: { + const Event_GameSay &say = event.GetExtension(Event_GameSay::ext); + addDetail(details, QString::fromStdString(say.message()), tr("CHAT"), ReplayEventKind::Chat, + eventPlayerId, forcedByJudge); + break; + } + case GameEvent::SET_ACTIVE_PLAYER: { + const Event_SetActivePlayer &setActivePlayer = event.GetExtension(Event_SetActivePlayer::ext); + activePlayerId = setActivePlayer.active_player_id(); + if (activePlayerId >= 0) { + addDetail(details, tr("begins their turn."), tr("TURN"), ReplayEventKind::Turn, activePlayerId, + forcedByJudge); + } + break; + } + case GameEvent::SET_ACTIVE_PHASE: { + const Event_SetActivePhase &setActivePhase = event.GetExtension(Event_SetActivePhase::ext); + if (setActivePhase.phase() >= 0) { + const QString phaseName = Phases::getPhase(setActivePhase.phase()).getName(); + addDetail(details, tr("entered the %1 phase.").arg(phaseName), tr("PHASE"), + ReplayEventKind::Phase, activePlayerId >= 0 ? activePlayerId : eventPlayerId, + forcedByJudge); + } + break; + } + case GameEvent::REVERSE_TURN: { + const Event_ReverseTurn &reverseTurn = event.GetExtension(Event_ReverseTurn::ext); + addDetail(details, + reverseTurn.reversed() ? tr("reversed the turn order.") : tr("restored the turn order."), + tr("TURN"), ReplayEventKind::Turn, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::MOVE_CARD: { + const Event_MoveCard &moveCard = event.GetExtension(Event_MoveCard::ext); + const QString text = describeMoveCard(moveCard, eventPlayerId, context); + const QString startZone = QString::fromStdString(moveCard.start_zone()); + const QString targetZone = QString::fromStdString(moveCard.target_zone()); + QString badge = tr("MOVE"); + if (context.HasExtension(Context_UndoDraw::ext)) { + badge = tr("UNDO"); + } else if (startZone == ZoneNames::STACK) { + badge = tr("RESOLVE"); + } else if (startZone == ZoneNames::HAND && targetZone == ZoneNames::TABLE) { + badge = tr("PLAY"); + } + addDetail(details, text, badge, ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::DRAW_CARDS: { + const Event_DrawCards &drawCards = event.GetExtension(Event_DrawCards::ext); + QStringList names; + for (int j = 0; j < drawCards.cards_size(); ++j) { + const ServerInfo_Card &card = drawCards.cards(j); + const QString name = QString::fromStdString(card.name()); + names.append(name); + rememberCard(eventPlayerId, ZoneNames::HAND, card.id(), name); + } + if (drawCards.number() > 0) { + addDetail(details, tr("drew %1.").arg(formatCardList(names, drawCards.number())), tr("DRAW"), + ReplayEventKind::Action, eventPlayerId, forcedByJudge); + } + break; + } + case GameEvent::REVEAL_CARDS: { + const Event_RevealCards &revealCards = event.GetExtension(Event_RevealCards::ext); + const QString zone = QString::fromStdString(revealCards.zone_name()); + QStringList names; + for (int j = 0; j < revealCards.cards_size(); ++j) { + const ServerInfo_Card &card = revealCards.cards(j); + const QString name = QString::fromStdString(card.name()); + names.append(name); + rememberCard(eventPlayerId, zone, card.id(), name); + } + const int total = + revealCards.number_of_cards() > 0 ? revealCards.number_of_cards() : revealCards.cards_size(); + QString text = + tr("revealed %1 from %2.") + .arg(formatCardList(names, total), zonePhrase(eventPlayerId, zone, eventPlayerId)); + if (revealCards.other_player_id() >= 0 && revealCards.other_player_id() != eventPlayerId) { + text = tr("revealed %1 from %2 to %3.") + .arg(formatCardList(names, total), zonePhrase(eventPlayerId, zone, eventPlayerId), + playerName(revealCards.other_player_id())); + } + addDetail(details, text, tr("REVEAL"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::SHUFFLE: { + const Event_Shuffle &shuffle = event.GetExtension(Event_Shuffle::ext); + addDetail( + details, + tr("shuffled %1.") + .arg(zonePhrase(eventPlayerId, QString::fromStdString(shuffle.zone_name()), eventPlayerId)), + tr("SHUFFLE"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::ROLL_DIE: { + const Event_RollDie &rollDie = event.GetExtension(Event_RollDie::ext); + QStringList rolls; + for (int j = 0; j < rollDie.values_size(); ++j) { + rolls.append(QString::number(rollDie.values(j))); + } + if (rolls.isEmpty() && rollDie.value() > 0) { + rolls.append(QString::number(rollDie.value())); + } + const QString sides = rollDie.sides() > 0 ? QString::number(rollDie.sides()) : QStringLiteral("?"); + addDetail(details, tr("rolled a d%1: %2.").arg(sides, rolls.join(QStringLiteral(", "))), tr("DICE"), + ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::FLIP_CARD: { + const Event_FlipCard &flipCard = event.GetExtension(Event_FlipCard::ext); + const QString zone = QString::fromStdString(flipCard.zone_name()); + if (flipCard.has_card_name()) { + rememberCard(eventPlayerId, zone, flipCard.card_id(), + QString::fromStdString(flipCard.card_name())); + } + QString name = flipCard.has_card_name() ? QString::fromStdString(flipCard.card_name()) + : cardName(eventPlayerId, zone, flipCard.card_id()); + if (name.isEmpty()) { + name = tr("a card"); + } + addDetail(details, + flipCard.face_down() ? tr("flipped %1 face down.").arg(name) + : tr("flipped %1 face up.").arg(name), + tr("FLIP"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::DESTROY_CARD: { + const Event_DestroyCard &destroyCard = event.GetExtension(Event_DestroyCard::ext); + const QString zone = QString::fromStdString(destroyCard.zone_name()); + const QString name = cardName(eventPlayerId, zone, destroyCard.card_id()); + forgetCard(eventPlayerId, zone, destroyCard.card_id()); + addDetail(details, tr("destroyed %1.").arg(name.isEmpty() ? tr("a card") : name), tr("DESTROY"), + ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::ATTACH_CARD: { + const Event_AttachCard &attachCard = event.GetExtension(Event_AttachCard::ext); + const QString startZone = QString::fromStdString(attachCard.start_zone()); + const QString source = cardName(eventPlayerId, startZone, attachCard.card_id()); + const QString sourceText = source.isEmpty() ? tr("a card") : source; + // An unattach event carries no target at all, so the target fields default to player 0 / no zone + // / card 0; only a present target_player_id means the card was attached to something. + if (!attachCard.has_target_player_id()) { + addDetail(details, tr("unattached %1.").arg(sourceText), tr("ATTACH"), ReplayEventKind::Action, + eventPlayerId, forcedByJudge); + break; + } + const QString targetZone = QString::fromStdString(attachCard.target_zone()); + const int targetPlayerId = + attachCard.target_player_id() >= 0 ? attachCard.target_player_id() : eventPlayerId; + const QString target = cardName(targetPlayerId, targetZone, attachCard.target_card_id()); + addDetail(details, + tr("attached %1 to %2.").arg(sourceText, target.isEmpty() ? tr("a card") : target), + tr("ATTACH"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::CREATE_TOKEN: { + const Event_CreateToken &createToken = event.GetExtension(Event_CreateToken::ext); + const QString name = QString::fromStdString(createToken.card_name()); + rememberCard(eventPlayerId, QString::fromStdString(createToken.zone_name()), createToken.card_id(), + name); + const QString pt = QString::fromStdString(createToken.pt()); + addDetail( + details, + tr("created a token: %1%2.") + .arg(name.isEmpty() ? tr("token") : name, pt.isEmpty() ? QString() : tr(" (%1)").arg(pt)), + tr("TOKEN"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::SET_CARD_ATTR: { + const Event_SetCardAttr &setCardAttr = event.GetExtension(Event_SetCardAttr::ext); + addDetail(details, describeSetCardAttribute(setCardAttr, eventPlayerId), + badgeForCardAttribute(setCardAttr.attribute()), ReplayEventKind::Action, eventPlayerId, + forcedByJudge); + break; + } + case GameEvent::SET_CARD_COUNTER: { + const Event_SetCardCounter &setCardCounter = event.GetExtension(Event_SetCardCounter::ext); + const QString name = cardName(eventPlayerId, QString::fromStdString(setCardCounter.zone_name()), + setCardCounter.card_id()); + addDetail( + details, + tr("set a counter on %1 to %2.") + .arg(name.isEmpty() ? tr("a card") : name, QString::number(setCardCounter.counter_value())), + tr("COUNTER"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::CREATE_COUNTER: { + const Event_CreateCounter &createCounter = event.GetExtension(Event_CreateCounter::ext); + const ServerInfo_Counter &counter = createCounter.counter_info(); + rememberCounter(eventPlayerId, counter); + const QString name = QString::fromStdString(counter.name()); + addDetail(details, + tr("created the %1 counter (%2).") + .arg(name.isEmpty() ? tr("new") : name, QString::number(counter.count())), + tr("COUNTER"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::SET_COUNTER: { + const Event_SetCounter &setCounter = event.GetExtension(Event_SetCounter::ext); + const QString name = counterName(eventPlayerId, setCounter.counter_id()); + addDetail(details, + tr("set %1 to %2.") + .arg(name.isEmpty() ? tr("a counter") : tr("the %1 counter").arg(name), + QString::number(setCounter.value())), + tr("COUNTER"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::DEL_COUNTER: { + const Event_DelCounter &delCounter = event.GetExtension(Event_DelCounter::ext); + const QString name = counterName(eventPlayerId, delCounter.counter_id()); + counterNames.remove(counterKey(eventPlayerId, delCounter.counter_id())); + addDetail(details, + tr("removed %1.").arg(name.isEmpty() ? tr("a counter") : tr("the %1 counter").arg(name)), + tr("COUNTER"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::DUMP_ZONE: { + const Event_DumpZone &dumpZone = event.GetExtension(Event_DumpZone::ext); + const QString zoneOwnerPhrase = zonePhrase( + dumpZone.zone_owner_id(), QString::fromStdString(dumpZone.zone_name()), eventPlayerId); + if (dumpZone.number_cards() < 0) { + addDetail(details, tr("looked at %1.").arg(zoneOwnerPhrase), tr("LOOK"), + ReplayEventKind::Action, eventPlayerId, forcedByJudge); + } else { + addDetail(details, + tr("looked at %1 from %2.") + .arg(formatCardList(QStringList(), dumpZone.number_cards()), zoneOwnerPhrase), + tr("LOOK"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + } + break; + } + case GameEvent::CHANGE_ZONE_PROPERTIES: { + const Event_ChangeZoneProperties &changeZone = event.GetExtension(Event_ChangeZoneProperties::ext); + const QString zone = QString::fromStdString(changeZone.zone_name()); + QStringList changes; + if (changeZone.always_reveal_top_card()) { + changes.append(tr("top card revealed to all")); + } + if (changeZone.always_look_at_top_card()) { + changes.append(tr("top card visible to its owner")); + } + addDetail( + details, + changes.isEmpty() + ? tr("changed the properties of %1.").arg(zonePhrase(eventPlayerId, zone, eventPlayerId)) + : tr("changed %1: %2.") + .arg(zonePhrase(eventPlayerId, zone, eventPlayerId), + changes.join(QStringLiteral(", "))), + tr("ZONE"), ReplayEventKind::Action, eventPlayerId, forcedByJudge); + break; + } + case GameEvent::GAME_LOG_NOTICE: { + const Event_GameLogNotice ¬ice = event.GetExtension(Event_GameLogNotice::ext); + switch (notice.notice_type()) { + case Event_GameLogNotice::UNDO_DRAW_FAILED: + addDetail(details, tr("failed to undo a draw."), tr("NOTICE"), ReplayEventKind::System, + eventPlayerId, forcedByJudge); + break; + default: + addDetail(details, tr("A game notice was logged."), tr("NOTICE"), ReplayEventKind::System, + -1, forcedByJudge); + break; + } + break; + } + case GameEvent::CREATE_ARROW: + case GameEvent::DELETE_ARROW: + // Arrows are transient visual annotations and would make the history very noisy. + break; + default: + addDetail(details, tr("Unrecognized game event."), tr("EVENT"), ReplayEventKind::Action, + eventPlayerId, forcedByJudge); + break; + } + } + + return details; + } +}; +} // namespace + +QList buildReplayEventEntries(const GameReplay *replay, const QList &timeline) +{ + if (!replay) { + return {}; + } + + return ReplayEventIndexBuilder(replay, timeline).build(); +} diff --git a/cockatrice/src/interface/widgets/replay/replay_event_index.h b/cockatrice/src/interface/widgets/replay/replay_event_index.h new file mode 100644 index 000000000..40f72a965 --- /dev/null +++ b/cockatrice/src/interface/widgets/replay/replay_event_index.h @@ -0,0 +1,43 @@ +#ifndef COCKATRICE_REPLAY_EVENT_INDEX_H +#define COCKATRICE_REPLAY_EVENT_INDEX_H + +#include +#include + +class GameReplay; + +enum class ReplayEventKind +{ + Action, + Turn, + Phase, + Chat, + System +}; + +/** + * @brief One user-visible entry in the replay history sidebar. + * + * The event index refers to the corresponding GameEventContainer in the replay. It is used to seek the replay to that + * container. + */ +struct ReplayEventEntry +{ + int eventIndex = -1; + int timeMs = 0; + QString timeLabel; + QString actor; + QString badge; + QString detail; + ReplayEventKind kind = ReplayEventKind::Action; +}; + +/** + * @brief Builds a compact, human-readable index of the events in a replay. + * + * The returned entries copy all the data they need, so the caller does not need to keep the replay object alive after + * this function returns. + */ +QList buildReplayEventEntries(const GameReplay *replay, const QList &timeline); + +#endif // COCKATRICE_REPLAY_EVENT_INDEX_H From 11f0dfac2d971d4124d9dbed5fb6a7480b884de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 20 Oct 2026 22:15:02 +0200 Subject: [PATCH 17/17] [Replay] Add a clickable replay history sidebar Render the event index as a scrollable list of event boxes. Selecting an entry seeks the replay to that event and keeps the current event highlighted while playback advances. --- cockatrice/CMakeLists.txt | 1 + .../widgets/replay/replay_event_sidebar.cpp | 284 ++++++++++++++++++ .../widgets/replay/replay_event_sidebar.h | 50 +++ 3 files changed, 335 insertions(+) create mode 100644 cockatrice/src/interface/widgets/replay/replay_event_sidebar.cpp create mode 100644 cockatrice/src/interface/widgets/replay/replay_event_sidebar.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index d7906a418..0d3c40a0c 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -250,6 +250,7 @@ set(cockatrice_SOURCES src/interface/widgets/quick_settings/settings_button_widget.cpp src/interface/widgets/quick_settings/settings_popup_widget.cpp src/interface/widgets/replay/replay_event_index.cpp + src/interface/widgets/replay/replay_event_sidebar.cpp src/interface/widgets/replay/replay_manager.cpp src/interface/widgets/replay/replay_quick_settings_widget.cpp src/interface/widgets/replay/replay_timeline_widget.cpp diff --git a/cockatrice/src/interface/widgets/replay/replay_event_sidebar.cpp b/cockatrice/src/interface/widgets/replay/replay_event_sidebar.cpp new file mode 100644 index 000000000..b5fab2794 --- /dev/null +++ b/cockatrice/src/interface/widgets/replay/replay_event_sidebar.cpp @@ -0,0 +1,284 @@ +#include "replay_event_sidebar.h" + +#include "replay_event_index.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +enum ItemDataRole +{ + EventIndexRole = Qt::UserRole + 1, + TimeLabelRole, + KindRole, + ActorRole, + BadgeRole, + DetailRole +}; + +QColor accentForKind(ReplayEventKind kind, const QPalette &palette) +{ + switch (kind) { + case ReplayEventKind::Turn: + return QColor(0x4c, 0xaf, 0x50); + case ReplayEventKind::Phase: + return QColor(0x42, 0xa5, 0xf5); + case ReplayEventKind::Chat: + return QColor(0xff, 0xb3, 0x00); + case ReplayEventKind::System: + return palette.mid().color(); + case ReplayEventKind::Action: + default: + return palette.highlight().color(); + } +} + +class ReplayEventDelegate : public QStyledItemDelegate +{ +public: + explicit ReplayEventDelegate(QObject *parent) : QStyledItemDelegate(parent) + { + } + + void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override + { + painter->save(); + painter->setRenderHint(QPainter::Antialiasing, true); + + const QPalette &palette = option.palette; + const bool selected = option.state.testFlag(QStyle::State_Selected); + const bool hovered = option.state.testFlag(QStyle::State_MouseOver); + const auto kind = static_cast(index.data(KindRole).toInt()); + const QColor accent = accentForKind(kind, palette); + + const QRect card = option.rect.adjusted(4, 2, -4, -2); + QColor cardBackground = selected ? palette.highlight().color() : palette.base().color(); + if (!selected && hovered) { + cardBackground = palette.alternateBase().color(); + } + + painter->setPen(Qt::NoPen); + painter->setBrush(cardBackground); + painter->drawRoundedRect(card, 6, 6); + + QColor borderColor = selected ? palette.highlightedText().color() : palette.mid().color(); + borderColor.setAlpha(selected ? 100 : 130); + painter->setBrush(Qt::NoBrush); + painter->setPen(QPen(borderColor, 1)); + painter->drawRoundedRect(card.adjusted(0, 0, -1, -1), 6, 6); + + QColor accentColor = accent; + if (selected) { + accentColor = palette.highlightedText().color(); + accentColor.setAlpha(210); + } + painter->setPen(Qt::NoPen); + painter->setBrush(accentColor); + painter->drawRoundedRect(QRect(card.left() + 7, card.top() + 7, 3, card.height() - 14), 1.5, 1.5); + + const QColor textColor = selected ? palette.highlightedText().color() : palette.text().color(); + QColor mutedColor = textColor; + mutedColor.setAlpha(170); + + QFont timeFont = option.font; + timeFont.setBold(true); + timeFont.setPointSizeF(qMax(7.0, timeFont.pointSizeF() * 0.9)); + + QFont actorFont = option.font; + actorFont.setBold(true); + + QFont badgeFont = option.font; + badgeFont.setBold(true); + badgeFont.setPointSizeF(qMax(6.5, badgeFont.pointSizeF() * 0.72)); + + QFont detailFont = option.font; + detailFont.setPointSizeF(qMax(7.0, detailFont.pointSizeF() * 0.95)); + + const QFontMetrics timeMetrics(timeFont); + const QFontMetrics actorMetrics(actorFont); + const QFontMetrics badgeMetrics(badgeFont); + const QFontMetrics detailMetrics(detailFont); + + const int contentLeft = card.left() + 15; + const int contentRight = card.right() - 10; + const int topRowTop = card.top() + 6; + const int topRowHeight = qMax(timeMetrics.height(), badgeMetrics.height()) + 2; + + const QString timeText = index.data(TimeLabelRole).toString(); + const QString actorText = index.data(ActorRole).toString(); + const QString badgeText = index.data(BadgeRole).toString(); + const QString detailText = index.data(DetailRole).toString(); + + const int timeWidth = timeMetrics.horizontalAdvance(timeText); + painter->setFont(timeFont); + painter->setPen(mutedColor); + painter->drawText(QRect(contentLeft, topRowTop, timeWidth + 2, topRowHeight), Qt::AlignLeft | Qt::AlignVCenter, + timeText); + + int actorRight = contentRight; + if (!badgeText.isEmpty()) { + const int badgeWidth = badgeMetrics.horizontalAdvance(badgeText) + 10; + const int badgeHeight = badgeMetrics.height() + 2; + const QRect badgeRect(contentRight - badgeWidth, topRowTop + (topRowHeight - badgeHeight) / 2, badgeWidth, + badgeHeight); + + QColor badgeBackground = accentColor; + badgeBackground.setAlpha(selected ? 45 : 32); + painter->setPen(Qt::NoPen); + painter->setBrush(badgeBackground); + painter->drawRoundedRect(badgeRect, 3, 3); + + painter->setFont(badgeFont); + painter->setPen(accentColor); + painter->drawText(badgeRect, Qt::AlignCenter, badgeText); + + actorRight = badgeRect.left() - 8; + } + + const int actorLeft = contentLeft + timeWidth + 10; + if (actorRight > actorLeft) { + const QString elidedActor = + actorMetrics.elidedText(actorText, Qt::ElideRight, qMax(0, actorRight - actorLeft)); + painter->setFont(actorFont); + painter->setPen(textColor); + painter->drawText(QRect(actorLeft, topRowTop, actorRight - actorLeft, topRowHeight), + Qt::AlignLeft | Qt::AlignVCenter, elidedActor); + } + + const int detailTop = topRowTop + topRowHeight + 2; + const int detailHeight = qMax(detailMetrics.height(), card.bottom() - detailTop); + const QString elidedDetail = + detailMetrics.elidedText(detailText, Qt::ElideRight, qMax(0, contentRight - contentLeft)); + painter->setFont(detailFont); + painter->setPen(textColor); + painter->drawText(QRect(contentLeft, detailTop, contentRight - contentLeft, detailHeight), + Qt::AlignLeft | Qt::AlignVCenter, elidedDetail); + + painter->restore(); + } + + QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex & /*index*/) const override + { + const QFontMetrics metrics(option.font); + return {0, qMax(58, metrics.height() * 3 + 10)}; + } +}; +} // namespace + +ReplayEventSidebar::ReplayEventSidebar(QWidget *parent, const GameReplay *replay, const QList &timeline) + : QWidget(parent), eventList(new QListWidget(this)), emptyLabel(new QLabel(this)) +{ + setObjectName("replayEventSidebar"); + + eventList->setObjectName("replayEventList"); + eventList->setItemDelegate(new ReplayEventDelegate(eventList)); + eventList->setFrameShape(QFrame::NoFrame); + eventList->setFocusPolicy(Qt::NoFocus); + eventList->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + eventList->setSelectionMode(QAbstractItemView::SingleSelection); + eventList->setMouseTracking(true); + eventList->setUniformItemSizes(true); + eventList->setVerticalScrollMode(QAbstractItemView::ScrollPerPixel); + + emptyLabel->setAlignment(Qt::AlignCenter); + emptyLabel->setWordWrap(true); + emptyLabel->setMargin(16); + emptyLabel->setForegroundRole(QPalette::PlaceholderText); + + auto *layout = QtUtils::makeLayout(this); + layout->addWidget(eventList); + layout->addWidget(emptyLabel); + + const QList entries = buildReplayEventEntries(replay, timeline); + eventIndices.reserve(entries.size()); + + for (const ReplayEventEntry &entry : entries) { + auto *item = new QListWidgetItem(eventList); + item->setData(EventIndexRole, entry.eventIndex); + item->setData(TimeLabelRole, entry.timeLabel); + item->setData(KindRole, static_cast(entry.kind)); + item->setData(ActorRole, entry.actor); + item->setData(BadgeRole, entry.badge); + item->setData(DetailRole, entry.detail); + const QString tooltip = entry.actor.isEmpty() + ? tr("%1\n%2").arg(entry.timeLabel, entry.detail) + : tr("%1 · %2\n%3").arg(entry.timeLabel, entry.actor, entry.detail); + item->setToolTip(tooltip); + item->setData(Qt::AccessibleTextRole, entry.actor.isEmpty() + ? tr("%1: %2").arg(entry.timeLabel, entry.detail) + : tr("%1, %2: %3").arg(entry.timeLabel, entry.actor, entry.detail)); + eventIndices.append(entry.eventIndex); + } + + const bool hasEvents = !entries.isEmpty(); + eventList->setVisible(hasEvents); + emptyLabel->setVisible(!hasEvents); + emptyLabel->setText(tr("No game events in this replay.")); + + connect(eventList, &QListWidget::itemClicked, this, [this](QListWidgetItem *item) { + if (!item) { + return; + } + + setCurrentRow(eventList->row(item)); + emit eventSelected(item->data(EventIndexRole).toInt()); + }); + + if (hasEvents) { + setCurrentRow(-1); + } +} + +void ReplayEventSidebar::setCurrentEvent(int eventIndex) +{ + if (eventIndices.isEmpty()) { + setCurrentRow(-1); + return; + } + + // The rows hold a subset of the replay's events, in playback order, so the last row at or before eventIndex is the + // one the game state has reached. An eventIndex of -1 (nothing replayed yet) selects no row. + const auto it = std::upper_bound(eventIndices.cbegin(), eventIndices.cend(), eventIndex); + const int row = static_cast(it - eventIndices.cbegin()) - 1; + setCurrentRow(row); +} + +void ReplayEventSidebar::setCurrentRow(int row) +{ + if (row < 0 || row >= eventList->count()) { + row = -1; + } + + if (row >= 0) { + // The view's current index also moves on mouse press and can be deselected by a ctrl-click, + // so the cached row alone cannot tell whether the view is already in the desired state. + const QListWidgetItem *item = eventList->item(row); + if (row == eventList->currentRow() && item && item->isSelected()) { + return; + } + } else if (row == currentRow) { + return; + } + + currentRow = row; + eventList->setCurrentRow(currentRow); + + if (currentRow >= 0) { + QListWidgetItem *item = eventList->item(currentRow); + if (!eventList->viewport()->rect().contains(eventList->visualItemRect(item))) { + eventList->scrollToItem(item, QAbstractItemView::PositionAtCenter); + } + } +} diff --git a/cockatrice/src/interface/widgets/replay/replay_event_sidebar.h b/cockatrice/src/interface/widgets/replay/replay_event_sidebar.h new file mode 100644 index 000000000..ee163356e --- /dev/null +++ b/cockatrice/src/interface/widgets/replay/replay_event_sidebar.h @@ -0,0 +1,50 @@ +#ifndef COCKATRICE_REPLAY_EVENT_SIDEBAR_H +#define COCKATRICE_REPLAY_EVENT_SIDEBAR_H + +#include +#include +#include + +class GameReplay; +class QLabel; +class QListWidget; + +/** + * @brief A scrollable list of replay events that can be used to seek the replay. + * + * The sidebar copies the event data it needs at construction time. It does not keep a reference to the replay or to + * the replay manager. + */ +class ReplayEventSidebar : public QWidget +{ + Q_OBJECT + +public: + /** + * @param parent The parent widget + * @param replay The replay to build the event list from + * @param timeline Event index to time in milliseconds, as used by ReplayManager + */ + ReplayEventSidebar(QWidget *parent, const GameReplay *replay, const QList &timeline); + +public slots: + /** + * @brief Highlights the row of the event the game state has reached. + * + * @param eventIndex Index into the replay's event list, or -1 when nothing has been replayed yet + */ + void setCurrentEvent(int eventIndex); + +signals: + void eventSelected(int eventIndex); + +private: + void setCurrentRow(int row); + + QListWidget *eventList; + QLabel *emptyLabel; + QList eventIndices; ///< Replay event index of each row, ascending + int currentRow = -1; +}; + +#endif // COCKATRICE_REPLAY_EVENT_SIDEBAR_H