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