diff --git a/cockatrice/src/database/interface/settings_card_preference_provider.h b/cockatrice/src/database/interface/settings_card_preference_provider.h index 64502e3a7..1feb2006b 100644 --- a/cockatrice/src/database/interface/settings_card_preference_provider.h +++ b/cockatrice/src/database/interface/settings_card_preference_provider.h @@ -13,11 +13,6 @@ public: { return SettingsCache::instance().cardOverrides().getCardPreferenceOverride(cardName); } - - [[nodiscard]] bool getIncludeRebalancedCards() const override - { - return SettingsCache::instance().cardsDisplay().getIncludeRebalancedCards(); - } }; #endif // COCKATRICE_SETTINGS_CARD_PREFERENCE_PROVIDER_H diff --git a/cockatrice/src/game_graphics/zones/card_zone.cpp b/cockatrice/src/game_graphics/zones/card_zone.cpp index b2ba8618e..d09bb588c 100644 --- a/cockatrice/src/game_graphics/zones/card_zone.cpp +++ b/cockatrice/src/game_graphics/zones/card_zone.cpp @@ -48,7 +48,7 @@ void CardZone::mouseDoubleClickEvent(QGraphicsSceneMouseEvent * /*event*/) bool CardZone::showContextMenu(const QPoint &screenPos) { if (menu) { - menu->exec(screenPos); + menu->popup(screenPos); return true; } return false; diff --git a/cockatrice/src/game_graphics/zones/view_zone_widget.cpp b/cockatrice/src/game_graphics/zones/view_zone_widget.cpp index b4f73d348..49d9955e0 100644 --- a/cockatrice/src/game_graphics/zones/view_zone_widget.cpp +++ b/cockatrice/src/game_graphics/zones/view_zone_widget.cpp @@ -51,7 +51,13 @@ namespace { -constexpr qreal kTitleBarHeight = 24.0; +qreal titleBarHeight(QWidget *styleWidget) +{ + if (styleWidget) { + return static_cast(styleWidget->style()->pixelMetric(QStyle::PM_TitleBarHeight, nullptr, styleWidget)); + } + return 24.0; +} constexpr qreal kMinVisibleWidth = 100.0; } // namespace @@ -315,7 +321,8 @@ QRectF ZoneViewWidget::closeButtonRect(QWidget *styleWidget) const // query the style for the close button position (handles macOS top-left placement) // Title bar rect MUST be local (0,0-based) for QStyle - const QRect titleBarRect(0, 0, static_cast(frameRectF.width()), static_cast(kTitleBarHeight)); + const qreal tbh = titleBarHeight(styleWidget); + const QRect titleBarRect(0, 0, static_cast(frameRectF.width()), static_cast(tbh)); if (styleWidget) { QStyleOptionTitleBar opt; @@ -344,7 +351,7 @@ QRectF ZoneViewWidget::closeButtonRect(QWidget *styleWidget) const } // Fallback: frame-relative top-right - return QRectF(frameRectF.right() - kTitleBarHeight, frameRectF.top(), kTitleBarHeight, kTitleBarHeight); + return QRectF(frameRectF.right() - tbh, frameRectF.top(), tbh, tbh); } QGraphicsView *ZoneViewWidget::findDragView(QWidget *eventWidget) const @@ -468,8 +475,9 @@ QVariant ZoneViewWidget::itemChange(GraphicsItemChange change, const QVariant &v const qreal minX = sceneRect.left(); const qreal maxX = qMax(minX, sceneRect.right() - kMinVisibleWidth); - const qreal minY = sceneRect.top() + kTitleBarHeight; - const qreal maxY = qMax(minY, sceneRect.bottom() - kTitleBarHeight); + const qreal tbh = titleBarHeight(nullptr); + const qreal minY = sceneRect.top() + tbh; + const qreal maxY = qMax(minY, sceneRect.bottom() - tbh); desiredPos.setX(qBound(minX, desiredPos.x(), maxX)); desiredPos.setY(qBound(minY, desiredPos.y(), maxY)); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp index b3da6d19c..dbfe38e26 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.cpp @@ -178,12 +178,6 @@ WndSets::WndSets(QWidget *parent) : QMainWindow(parent) sortWarning->setLayout(sortWarningLayout); sortWarning->setVisible(false); - includeRebalancedCards = SettingsCache::instance().cardsDisplay().getIncludeRebalancedCards(); - QCheckBox *includeRebalancedCardsCheckBox = - new QCheckBox(tr("Include cards rebalanced for Alchemy [requires restart]")); - includeRebalancedCardsCheckBox->setChecked(includeRebalancedCards); - connect(includeRebalancedCardsCheckBox, &QAbstractButton::toggled, this, &WndSets::includeRebalancedCardsChanged); - buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); connect(buttonBox, &QDialogButtonBox::accepted, this, &WndSets::actSave); connect(buttonBox, &QDialogButtonBox::rejected, this, &WndSets::actRestore); @@ -197,9 +191,8 @@ WndSets::WndSets(QWidget *parent) : QMainWindow(parent) mainLayout->addWidget(enableSomeButton, 2, 1); mainLayout->addWidget(disableSomeButton, 2, 2); mainLayout->addWidget(sortWarning, 3, 1, 1, 2); - mainLayout->addWidget(includeRebalancedCardsCheckBox, 4, 1, 1, 2); - mainLayout->addWidget(hintsGroupBox, 5, 1, 1, 2); - mainLayout->addWidget(buttonBox, 6, 1, 1, 2); + mainLayout->addWidget(hintsGroupBox, 4, 1, 1, 2); + mainLayout->addWidget(buttonBox, 5, 1, 1, 2); mainLayout->setColumnStretch(1, 1); mainLayout->setColumnStretch(2, 1); @@ -270,15 +263,9 @@ void WndSets::resetSort() sortWarning->setVisible(false); } -void WndSets::includeRebalancedCardsChanged(bool _includeRebalancedCards) -{ - includeRebalancedCards = _includeRebalancedCards; -} - void WndSets::actSave() { model->save(CardDatabaseManager::getInstance()); - SettingsCache::instance().cardsDisplay().setIncludeRebalancedCards(includeRebalancedCards); CardPictureLoader::clearPixmapCache(); const auto reloadOk1 = QtConcurrent::run([] { CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify(); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.h b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.h index d08412c51..67b4c90b1 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_manage_sets.h @@ -52,7 +52,6 @@ private: int sortIndex; Qt::SortOrder sortOrder; bool setOrderIsSorted; - bool includeRebalancedCards; enum { NO_SETS_SELECTED, @@ -87,7 +86,6 @@ private slots: void actDisableResetButton(const QString &filterText); void actSort(int index); void actIgnoreWarning(); - void includeRebalancedCardsChanged(bool _includeRebalancedCardsChanged); }; #endif diff --git a/doc/carddatabase_v4/cards.xsd b/doc/carddatabase_v4/cards.xsd index 82aed32ab..df52d6327 100644 --- a/doc/carddatabase_v4/cards.xsd +++ b/doc/carddatabase_v4/cards.xsd @@ -29,8 +29,6 @@ - - diff --git a/libcockatrice_card/libcockatrice/card/database/card_database.cpp b/libcockatrice_card/libcockatrice/card/database/card_database.cpp index f61da204f..469c8d02c 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database.cpp @@ -33,7 +33,7 @@ CardDatabase::CardDatabase(QObject *parent, qRegisterMetaType("CardDatabaseData"); // create loader and wire it up - loader = new CardDatabaseLoader(this, this, pathProvider, prefs, setPriorityController); + loader = new CardDatabaseLoader(this, this, pathProvider, setPriorityController); // re-emit loader signals (so other code doesn't need to know about internals) connect(loader, &CardDatabaseLoader::loadingFinished, this, &CardDatabase::cardDatabaseLoadingFinished); connect(loader, &CardDatabaseLoader::loadingFailed, this, &CardDatabase::cardDatabaseLoadingFailed); @@ -78,7 +78,9 @@ void CardDatabase::loadCardDatabases() void CardDatabase::reloadCardDatabasesAndNotify() { - loadCardDatabases(); + // An explicit reload bypasses the binary cache so the files on disk are + // always re-read, even when their size/mtime hash still matches the cache. + loadStatus = loader->loadCardDatabases(/* ignoreCache */ true); if (loadStatus == Ok) { checkUnknownSets(); diff --git a/libcockatrice_card/libcockatrice/card/database/card_database.h b/libcockatrice_card/libcockatrice/card/database/card_database.h index 44d554df7..9f0d67b67 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database.h @@ -177,8 +177,12 @@ public slots: void addFormat(const FormatRulesPtr &format); - /** @brief Loads card databases from configured paths. */ + /** @brief Loads card databases from configured paths, using the binary cache when valid. */ void loadCardDatabases(); + /** + * @brief Reloads card databases from disk, bypassing the binary cache, + * and notifies observers of the fresh state. + */ void reloadCardDatabasesAndNotify(); /** @brief Saves custom tokens to file. diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp index 0f0b28b2a..194083615 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp @@ -6,6 +6,7 @@ #include "libcockatrice/card/database/card_database_data.h" #include "libcockatrice/card/set/card_set.h" #include "libcockatrice/interfaces/interface_card_database_path_provider.h" +#include "libcockatrice/interfaces/interface_card_set_priority_controller.h" #include "parser/card_database_parser.h" #include "parser/cockatrice_xml_3.h" #include "parser/cockatrice_xml_4.h" @@ -28,25 +29,22 @@ #include class ICardPreferenceProvider; -class ICardSetPriorityController; CardDatabaseLoader::CardDatabaseLoader(QObject *parent, CardDatabase *db, ICardDatabasePathProvider *_pathProvider, - ICardPreferenceProvider *_preferenceProvider, ICardSetPriorityController *_priorityController) : QObject(parent), database(db), pathProvider(_pathProvider), priorityController(_priorityController) { // instantiate available parsers here - availableParsers << new CockatriceXml4Parser(_preferenceProvider, _priorityController); + availableParsers << new CockatriceXml4Parser(_priorityController); availableParsers << new CockatriceXml3Parser(_priorityController); // The load path parses into a snapshot and never emits per-card signals; // the finished snapshot is swapped into the live database on the GUI thread. // when SettingsCache's path changes, trigger reloads - connect(pathProvider, &ICardDatabasePathProvider::cardDatabasePathChanged, this, - &CardDatabaseLoader::loadCardDatabases); + connect(pathProvider, &ICardDatabasePathProvider::cardDatabasePathChanged, this, [this] { loadCardDatabases(); }); } CardDatabaseLoader::~CardDatabaseLoader() @@ -91,12 +89,12 @@ LoadStatus CardDatabaseLoader::loadCardDatabase(const QString &path, CardDatabas return tempLoadStatus; } -LoadStatus CardDatabaseLoader::loadCardDatabases() +LoadStatus CardDatabaseLoader::loadCardDatabases(bool ignoreCache) { - return doLoadCardDatabases(); + return doLoadCardDatabases(ignoreCache); } -LoadStatus CardDatabaseLoader::doLoadCardDatabases() +LoadStatus CardDatabaseLoader::doLoadCardDatabases(bool ignoreCache) { QMutexLocker locker(reloadDatabaseMutex); @@ -114,9 +112,14 @@ LoadStatus CardDatabaseLoader::doLoadCardDatabases() LoadStatus loadStatus = NotLoaded; // Try the binary cache first: a cache hit avoids re-parsing the (large) XML. + // An explicit reload skips the lookup so the files on disk are always + // re-read, even when their size/mtime hash still matches the cache. const QStringList customPaths = collectCustomDatabasePaths(); const QByteArray sourceHash = computeSourceHash(customPaths); - if (loadFromCache(data, sourceHash)) { + if (ignoreCache) { + qCInfo(CardDatabaseLoadingLog) << "Ignoring binary cache, re-parsing source files"; + } + if (!ignoreCache && loadFromCache(data, sourceHash)) { qCInfo(CardDatabaseLoadingLog) << "Loaded card database from binary cache"; loadStatus = Ok; } else { @@ -181,6 +184,12 @@ QByteArray CardDatabaseLoader::computeSourceHash(const QStringList &customPaths) hash.addData(QCoreApplication::applicationVersion().toUtf8()); hash.addData(QByteArray(1, '\0')); + // Include parser schema version to invalidate caches when parser behavior changes. + // Increment this if the parser logic changes in a way that affects cached data. + static const char SCHEMA_VERSION[] = "carddb-parse-schema-v1"; + hash.addData(QByteArray(SCHEMA_VERSION, sizeof(SCHEMA_VERSION) - 1)); + hash.addData(QByteArray(1, '\0')); + const QStringList inputs = QStringList() << pathProvider->getCardDatabasePath() << pathProvider->getTokenDatabasePath() << pathProvider->getSpoilerCardDatabasePath() << customPaths; @@ -195,6 +204,17 @@ QByteArray CardDatabaseLoader::computeSourceHash(const QStringList &customPaths) hash.addData(QByteArray(1, '\0')); } } + + // The parsers drop printings of disabled sets at parse time, so the cached + // snapshot depends on enablement: fold the enabled set names in so that + // toggling a set invalidates the cache and forces a re-parse. + if (priorityController) { + const QStringList enabledSets = priorityController->getEnabledSetNames(); + for (const QString &setName : enabledSets) { + hash.addData(setName.toUtf8()); + hash.addData(QByteArray(1, '\0')); + } + } return hash.result(); } diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h index 706e31ab2..fffd31d2f 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h @@ -11,7 +11,6 @@ #include class ICardDatabasePathProvider; -class ICardPreferenceProvider; class ICardSetPriorityController; // IWYU pragma: keep @@ -59,12 +58,10 @@ public: * @param parent QObject parent. * @param db Pointer to the CardDatabase to populate (non-owning). * @param pathProvider Provider for card database file paths. - * @param preferenceProvider Optional card preference provider for pinned printings. */ explicit CardDatabaseLoader(QObject *parent, CardDatabase *db, ICardDatabasePathProvider *pathProvider, - ICardPreferenceProvider *preferenceProvider, ICardSetPriorityController *_priorityController); /** @brief Destructor cleans up allocated parsers. */ @@ -76,9 +73,13 @@ public slots: * * Runs synchronously on the calling thread. The caller should ensure * that any signal receivers are already connected before invoking this. + * @param ignoreCache Skip the binary cache lookup and re-parse the source + * files from disk, refreshing the cache afterwards. Used for + * explicit reloads so that edits the source hash cannot see are + * still picked up. * @return Status of the main database load. */ - LoadStatus loadCardDatabases(); + LoadStatus loadCardDatabases(bool ignoreCache = false); /** * @brief Loads a single card database file into the given snapshot. @@ -131,9 +132,10 @@ private: /** * @brief Performs the actual load work synchronously on the calling thread. + * @param ignoreCache Skip the binary cache lookup and re-parse from disk. * @return Status of the main database load. */ - LoadStatus doLoadCardDatabases(); + LoadStatus doLoadCardDatabases(bool ignoreCache); /** * @brief Collects custom card database paths recursively. diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp index eb1416817..4d975e533 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp @@ -37,9 +37,8 @@ class ICardSetPriorityController; #define COCKATRICE_XML4_SCHEMALOCATION \ "https://raw.githubusercontent.com/Cockatrice/Cockatrice/master/doc/carddatabase_v4/cards.xsd" -CockatriceXml4Parser::CockatriceXml4Parser(ICardPreferenceProvider *_cardPreferenceProvider, - ICardSetPriorityController *_cardSetPriorityController) - : ICardDatabaseParser(_cardSetPriorityController), cardPreferenceProvider(_cardPreferenceProvider) +CockatriceXml4Parser::CockatriceXml4Parser(ICardSetPriorityController *_cardSetPriorityController) + : ICardDatabaseParser(_cardSetPriorityController) { } @@ -284,7 +283,6 @@ QHash CockatriceXml4Parser::loadCardPropertiesFromXml(QXmlStre void CockatriceXml4Parser::loadCardsFromXml(QXmlStreamReader &xml) { - bool includeRebalancedCards = cardPreferenceProvider->getIncludeRebalancedCards(); while (!xml.atEnd()) { if (xml.readNext() == QXmlStreamReader::EndElement) { break; @@ -386,17 +384,7 @@ void CockatriceXml4Parser::loadCardsFromXml(QXmlStreamReader &xml) printingProps.insert(attrName, attr.value().toString()); } PrintingInfo printingInfo(set, LazyPropertiesHash(printingProps)); - - // This is very much a hack and not the right place to - // put this check, as it requires a reload of Cockatrice - // to be apply. - // - // However, this is also true of the `set->getEnabled()` - // check above (which is currently bugged as well), so - // we'll fix both at the same time. - if (includeRebalancedCards || printingInfo.getProperty("isRebalanced") != "true") { - _sets[setName].append(printingInfo); - } + _sets[setName].append(printingInfo); } // related cards } else if (xmlName == "related" || xmlName == "reverse-related") { diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.h b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.h index 5ddf672ee..390282796 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.h +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.h @@ -9,7 +9,6 @@ #include #include -class ICardPreferenceProvider; class ICardSetPriorityController; class QXmlStreamReader; @@ -27,7 +26,6 @@ inline Q_LOGGING_CATEGORY(CockatriceXml4Log, "cockatrice_xml.xml_4_parser"); * @note Differences from v3: * - Card properties are stored in blocks as a QHash. * - Sets can include a element. - * - Supports user preferences via ICardPreferenceProvider (e.g., skipping rebalanced cards). * - Related cards support persistent relations and multiple attach types (e.g., transform). * - More robust serialization; easier to extend schema in the future. */ @@ -35,8 +33,7 @@ class CockatriceXml4Parser : public ICardDatabaseParser { Q_OBJECT public: - explicit CockatriceXml4Parser(ICardPreferenceProvider *cardPreferenceProvider, - ICardSetPriorityController *cardSetPriorityController); + explicit CockatriceXml4Parser(ICardSetPriorityController *cardSetPriorityController); ~CockatriceXml4Parser() override = default; /** @@ -71,8 +68,6 @@ public: const QString &sourceVersion = "unknown") override; private: - ICardPreferenceProvider *cardPreferenceProvider; ///< Interface to handle user preferences - /** * @brief Loads a generic block from a element. * @param xml The open QXmlStreamReader positioned at a element. @@ -83,7 +78,6 @@ private: /** * @brief Load all elements from the XML stream. * @param xml The open QXmlStreamReader positioned at the element. - * Honors the user's preference regarding rebalanced cards. */ void loadCardsFromXml(QXmlStreamReader &xml); diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_preference_provider.h b/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_preference_provider.h index a6cf941fb..2510919ca 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_preference_provider.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_preference_provider.h @@ -8,7 +8,6 @@ class ICardPreferenceProvider public: virtual ~ICardPreferenceProvider() = default; [[nodiscard]] virtual QString getCardPreferenceOverride(const QString &cardName) const = 0; - [[nodiscard]] virtual bool getIncludeRebalancedCards() const = 0; }; #endif // COCKATRICE_INTERFACE_CARD_PREFERENCE_PROVIDER_H diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h b/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h index 36e748006..6b3e684d7 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h @@ -2,6 +2,7 @@ #define COCKATRICE_INTERFACE_CARD_SET_PRIORITY_CONTROLLER_H #include +#include class ICardSetPriorityController { @@ -46,6 +47,16 @@ public: */ virtual SetOptions getSetOptions(QString shortName) const = 0; + /** + * @brief Returns the short names of every set currently marked as enabled. + * + * Callers fold this into the card database cache source hash so that + * toggling set enablement invalidates the cache. + * + * @return The enabled set short names, sorted alphabetically. + */ + virtual QStringList getEnabledSetNames() const = 0; + virtual void saveSets(const QVector &data) = 0; }; diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/interface_cards_display_settings_provider.h b/libcockatrice_interfaces/libcockatrice/interfaces/interface_cards_display_settings_provider.h index 2821fb96e..a5ab4fc37 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/interface_cards_display_settings_provider.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/interface_cards_display_settings_provider.h @@ -14,7 +14,6 @@ public: [[nodiscard]] virtual bool getBumpSetsWithCardsInDeckToTop() const = 0; [[nodiscard]] virtual int getPrintingSelectorSortOrder() const = 0; [[nodiscard]] virtual int getPrintingSelectorCardSize() const = 0; - [[nodiscard]] virtual bool getIncludeRebalancedCards() const = 0; [[nodiscard]] virtual bool getPrintingSelectorNavigationButtonsVisible() const = 0; [[nodiscard]] virtual bool getTapAnimation() const = 0; [[nodiscard]] virtual bool getArrowDrawAnimation() const = 0; diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_preference_provider.h b/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_preference_provider.h index 3781f4b63..f846d51e8 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_preference_provider.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_preference_provider.h @@ -9,11 +9,6 @@ public: { return {}; } - - [[nodiscard]] bool getIncludeRebalancedCards() const override - { - return true; - } }; #endif // COCKATRICE_NOOP_CARD_PREFERENCE_PROVIDER_H diff --git a/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_set_priority_controller.h b/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_set_priority_controller.h index 8cc2011bd..94e1f0c9a 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_set_priority_controller.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/noop_card_set_priority_controller.h @@ -34,6 +34,11 @@ public: return {0, true, true}; } + QStringList getEnabledSetNames() const override + { + return {}; + } + void saveSets(const QVector & /* data */) override { } diff --git a/libcockatrice_settings/libcockatrice/settings/card_database_settings.cpp b/libcockatrice_settings/libcockatrice/settings/card_database_settings.cpp index 7d41e5165..acac1125f 100644 --- a/libcockatrice_settings/libcockatrice/settings/card_database_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/card_database_settings.cpp @@ -88,6 +88,20 @@ ICardSetPriorityController::SetOptions CardDatabaseSettings::getSetOptions(QStri return setOptionsCache.value(shortName); } +QStringList CardDatabaseSettings::getEnabledSetNames() const +{ + QMutexLocker lock(&setOptionsMutex); + ensureSetOptionsLoaded(); + QStringList names; + for (auto it = setOptionsCache.constBegin(); it != setOptionsCache.constEnd(); ++it) { + if (it.value().enabled) { + names << it.key(); + } + } + names.sort(); + return names; +} + void CardDatabaseSettings::saveSets(const QVector &data) { batchWrite([&](QSettings &s) { diff --git a/libcockatrice_settings/libcockatrice/settings/card_database_settings.h b/libcockatrice_settings/libcockatrice/settings/card_database_settings.h index 07a5788b9..5bdf17aef 100644 --- a/libcockatrice_settings/libcockatrice/settings/card_database_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/card_database_settings.h @@ -35,6 +35,8 @@ public: SetOptions getSetOptions(QString shortName) const override; + QStringList getEnabledSetNames() const override; + void saveSets(const QVector &data) override; private: diff --git a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp index 1addef70f..f047973f0 100644 --- a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp @@ -43,11 +43,6 @@ int CardsDisplaySettings::getPrintingSelectorCardSize() const return getValue("printingSelector", "cards", "cardSize", 100).toInt(); } -bool CardsDisplaySettings::getIncludeRebalancedCards() const -{ - return getValue("includerebalancedcards", QString(), QString(), true).toBool(); -} - bool CardsDisplaySettings::getPrintingSelectorNavigationButtonsVisible() const { return getValue("navigationButtonsVisible", "cards", "printingSelector", true).toBool(); @@ -167,15 +162,6 @@ void CardsDisplaySettings::setPrintingSelectorCardSize(int _printingSelectorCard emit printingSelectorCardSizeChanged(); } -void CardsDisplaySettings::setIncludeRebalancedCards(bool _includeRebalancedCards) -{ - if (_includeRebalancedCards == getIncludeRebalancedCards()) { - return; - } - setValue(_includeRebalancedCards, "includerebalancedcards"); - emit includeRebalancedCardsChanged(_includeRebalancedCards); -} - void CardsDisplaySettings::setPrintingSelectorNavigationButtonsVisible(bool _navigationButtonsVisible) { setValue(_navigationButtonsVisible, "navigationButtonsVisible", "cards", "printingSelector"); diff --git a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h index 4e6b92d27..961550d3c 100644 --- a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h @@ -21,7 +21,6 @@ public: [[nodiscard]] bool getBumpSetsWithCardsInDeckToTop() const override; [[nodiscard]] int getPrintingSelectorSortOrder() const override; [[nodiscard]] int getPrintingSelectorCardSize() const override; - [[nodiscard]] bool getIncludeRebalancedCards() const override; [[nodiscard]] bool getPrintingSelectorNavigationButtonsVisible() const override; [[nodiscard]] bool getTapAnimation() const override; [[nodiscard]] bool getArrowDrawAnimation() const override; @@ -45,7 +44,6 @@ public: void setBumpSetsWithCardsInDeckToTop(bool _bumpSetsWithCardsInDeckToTop); void setPrintingSelectorSortOrder(int _printingSelectorSortOrder); void setPrintingSelectorCardSize(int _printingSelectorCardSize); - void setIncludeRebalancedCards(bool _includeRebalancedCards); void setPrintingSelectorNavigationButtonsVisible(bool _navigationButtonsVisible); void setTapAnimation(bool _tapAnimation); void setArrowDrawAnimation(bool _arrowDrawAnimation); @@ -70,7 +68,6 @@ signals: void bumpSetsWithCardsInDeckToTopChanged(); void printingSelectorSortOrderChanged(); void printingSelectorCardSizeChanged(); - void includeRebalancedCardsChanged(bool _includeRebalancedCards); void printingSelectorNavigationButtonsVisibleChanged(); void visualDeckStorageCardSizeChanged(); void visualDatabaseDisplayCardSizeChanged(); diff --git a/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp b/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp index d8b637210..c3f299f40 100644 --- a/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp +++ b/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp @@ -259,7 +259,6 @@ static void migrateCardsDisplaySettings(const QString &settingsPath, QSettings & {"cards/roundcardcorners", "cards/roundCardCorners"}, {"cards/overrideallcardartwithpersonalpreference", "cards/overrideAllCardArtWithPersonalPreference"}, {"cards/bumpsetswithcardsindecktotop", "cards/bumpSetsWithCardsInDeckToTop"}, - {"cards/includerebalancedcards", "cards/includerebalancedcards"}, {"cards/tapanimation", "cards/tapAnimation"}, {"cards/autorotatesidewayslayoutcards", "cards/autoRotateSidewaysLayoutCards"}, {"cards/scaleCards", "cards/scaleCards"}, @@ -571,9 +570,15 @@ static void migrateLegacySets(const QString &settingsPath) QSettings cardDbIni(settingsPath + "cardDatabase.ini", QSettings::IniFormat); for (const auto &shortName : groups) { legacySetting.beginGroup(shortName); - cardDbIni.setValue("sets/" + shortName + "/sortKey", legacySetting.value("sortkey")); - cardDbIni.setValue("sets/" + shortName + "/enabled", legacySetting.value("enabled")); - cardDbIni.setValue("sets/" + shortName + "/isKnown", legacySetting.value("isknown")); + if (legacySetting.contains("sortkey")) { + cardDbIni.setValue("sets/" + shortName + "/sortKey", legacySetting.value("sortkey")); + } + if (legacySetting.contains("enabled")) { + cardDbIni.setValue("sets/" + shortName + "/enabled", legacySetting.value("enabled")); + } + if (legacySetting.contains("isknown")) { + cardDbIni.setValue("sets/" + shortName + "/isKnown", legacySetting.value("isknown")); + } legacySetting.endGroup(); } legacySetting.endGroup(); diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index b30565ed0..3382e36d6 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -4,7 +4,6 @@ #include "libcockatrice/card/format/format_legality_rules.h" #include "libcockatrice/card/lazy_properties_hash.h" #include "libcockatrice/card/relation/card_relation_type.h" -#include "libcockatrice/interfaces/noop_card_preference_provider.h" #include "libcockatrice/interfaces/noop_card_set_priority_controller.h" #include "parsehelpers.h" @@ -302,9 +301,9 @@ CardInfoPtr OracleImporter::addCard(QString name, static QString getJsonString(const QJsonObject &obj, const QString &key) { - // QVariant coerces numbers and booleans to text, while QJsonValue::toString() - // returns a null string for them — some MTGJSON fields (manaValue, - // convertedManaCost, isOnlineOnly, isRebalanced) carry those types. + // QVariant coerces numbers to text, while QJsonValue::toString() returns a + // null string for them — MTGJSON fields like manaValue and convertedManaCost + // carry those types. return obj.value(key).toVariant().toString(); } @@ -413,11 +412,8 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson }; // mtgjson name => xml name - static const QMap setInfoProperties{{"number", "num"}, - {"rarity", "rarity"}, - {"isOnlineOnly", "isOnlineOnly"}, - {"isRebalanced", "isRebalanced"}, - {"artist", "artist"}}; + static const QMap setInfoProperties{ + {"number", "num"}, {"rarity", "rarity"}, {"artist", "artist"}}; // mtgjson name => xml name static const QMap identifierProperties{{"multiverseId", "muid"}, {"scryfallId", "uuid"}}; @@ -852,7 +848,7 @@ int OracleImporter::startImport() bool OracleImporter::saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion) { - CockatriceXml4Parser parser(new NoopCardPreferenceProvider(), new NoopCardSetPriorityController()); + CockatriceXml4Parser parser(new NoopCardSetPriorityController()); return parser.saveToFile(createDefaultMagicFormats(), sets, cards, fileName, sourceUrl, sourceVersion); } diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index 0bfd8a5dd..1ac259d9b 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -7,18 +7,26 @@ #include "gtest/gtest.h" #include +#include +#include #include +#include +#include #include #include #include #include #include +#include #include +#include #include #include #include #include #include +#include +#include #include #include #include @@ -77,12 +85,11 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) QTemporaryDir tempDir; const QString fileName = tempDir.filePath("cards.xml"); - NoopCardPreferenceProvider prefProvider; - CockatriceXml4Parser writer(&prefProvider, &controller); + CockatriceXml4Parser writer(&controller); ASSERT_TRUE(writer.saveToFile({}, sets, cards, fileName)); CardDatabaseData data; - CockatriceXml4Parser parser(&prefProvider, &controller); + CockatriceXml4Parser parser(&controller); QFile file(fileName); ASSERT_TRUE(file.open(QIODevice::ReadOnly)); parser.parseFileInto(file, data); @@ -94,6 +101,280 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) ASSERT_EQ(loaded->getLocalizedText("de"), "Blitzschlag fügt 3 Schadenspunkte zu."); ASSERT_EQ(loaded->getLocalizedText("fr"), "Deal 3 damage."); } + +/** + * @brief Path provider serving a sandboxed copy of the test card database. + */ +class SandboxPathProvider : public ICardDatabasePathProvider +{ +public: + explicit SandboxPathProvider(const QString &_root) : root(_root) + { + } + + QString getCardDatabasePath() const override + { + return root + "/cards.xml"; + } + + QString getCustomCardDatabasePath() const override + { + return root + "/customsets/"; + } + + QString getTokenDatabasePath() const override + { + return root + "/tokens.xml"; + } + + QString getSpoilerCardDatabasePath() const override + { + return root + "/spoiler.xml"; + } + +private: + QString root; +}; + +/** + * @brief Set priority controller whose enablement state the test drives directly. + * + * Seeded short names behave like entries present in cardDatabase.ini (enabled, + * known). Anything the parser discovers but the test did not seed falls back to + * the default SetOptions, i.e. disabled and unknown, mirroring + * CardDatabaseSettings for sets without an ini group. + */ +class SeededSetPriorityController : public ICardSetPriorityController +{ +public: + void seed(const QStringList &shortNames) + { + for (const QString &shortName : shortNames) { + SetOptions options; + options.enabled = true; + options.isKnown = true; + setOptions.insert(shortName, options); + } + } + + void setSortKey(QString shortName, unsigned int sortKey) override + { + setOptions[shortName].sortKey = sortKey; + } + + void setEnabled(QString shortName, bool enabled) override + { + setOptions[shortName].enabled = enabled; + } + + void setIsKnown(QString shortName, bool isknown) override + { + setOptions[shortName].isKnown = isknown; + } + + unsigned int getSortKey(QString shortName) const override + { + return setOptions.value(shortName).sortKey; + } + + bool isEnabled(QString shortName) const override + { + return setOptions.value(shortName).enabled; + } + + bool isKnown(QString shortName) const override + { + return setOptions.value(shortName).isKnown; + } + + SetOptions getSetOptions(QString shortName) const override + { + return setOptions.value(shortName); + } + + QStringList getEnabledSetNames() const override + { + QStringList names; + for (auto it = setOptions.constBegin(); it != setOptions.constEnd(); ++it) { + if (it.value().enabled) { + names << it.key(); + } + } + names.sort(); + return names; + } + + void saveSets(const QVector &data) override + { + for (const SetSaveData &entry : data) { + setOptions[entry.shortName].sortKey = entry.sortKey; + setOptions[entry.shortName].enabled = entry.enabled; + } + } + +private: + QHash setOptions; +}; + +/** + * @brief Sandboxed card database plus enablement controller for cache tests. + * + * Copies the checked-in fixture XML into a temporary directory so each test + * owns the files (and the binary cache written next to them). + */ +class CardDatabaseCacheInvalidationTest : public ::testing::Test +{ +protected: + void SetUp() override + { + ASSERT_TRUE(tempDir.isValid()); + root = tempDir.path(); + ASSERT_TRUE(QFile::copy(QString(CARDDB_DATADIR) + "cards.xml", root + "/cards.xml")); + ASSERT_TRUE(QFile::copy(QString(CARDDB_DATADIR) + "tokens.xml", root + "/tokens.xml")); + ASSERT_TRUE(QDir(root).mkpath("customsets")); + ASSERT_TRUE( + QFile::copy(QString(CARDDB_DATADIR) + "customsets/customset1.xml", root + "/customsets/customset1.xml")); + controller.seed({"CAT", "DOG", "WHO", " Not a Card", "BRD"}); + } + + /** + * @brief Contents of the binary cache written next to cards.xml. + * @return The cache file bytes, or an empty array if no cache exists. + */ + QByteArray cacheContents() const + { + QFile file(root + "/cards.xml.cache"); + if (!file.open(QIODevice::ReadOnly)) { + return {}; + } + return file.readAll(); + } + + QTemporaryDir tempDir; + QString root; + SeededSetPriorityController controller; +}; + +TEST_F(CardDatabaseCacheInvalidationTest, DisablingSetBustsCacheAndDropsPrintings) +{ + NoopCardPreferenceProvider prefs; + SandboxPathProvider pathProvider(root); + CardDatabase db(nullptr, &prefs, &pathProvider, &controller); + + db.loadCardDatabases(); + const QByteArray initialCache = cacheContents(); + ASSERT_FALSE(initialCache.isEmpty()) << "first load must write the binary cache"; + ASSERT_FALSE(db.getCardList().value("Cat").isNull()); + ASSERT_TRUE(db.getCardList().value("Cat")->getSets().contains("CAT")); + + // A cache-aware load with unchanged inputs keeps the cache: the parse + // result would be identical, so there is nothing to invalidate. + db.loadCardDatabases(); + EXPECT_EQ(initialCache, cacheContents()) << "load with unchanged inputs must keep the cache"; + + // Disabling a set changes the source hash, so the next load re-parses and + // the set's printings disappear everywhere. + controller.setEnabled("CAT", false); + db.reloadCardDatabasesAndNotify(); + EXPECT_NE(initialCache, cacheContents()) << "toggling set enablement must bust the cache"; + ASSERT_FALSE(db.getCardList().value("Cat").isNull()); + EXPECT_FALSE(db.getCardList().value("Cat")->getSets().contains("CAT")) << "disabled set printings must be dropped"; + ASSERT_FALSE(db.getCardList().value("Kitten").isNull()); + EXPECT_FALSE(db.getCardList().value("Kitten")->getSets().contains("CAT")); +} + +TEST_F(CardDatabaseCacheInvalidationTest, AddingAndEnablingCustomSetBustsCache) +{ + NoopCardPreferenceProvider prefs; + SandboxPathProvider pathProvider(root); + CardDatabase db(nullptr, &prefs, &pathProvider, &controller); + + db.loadCardDatabases(); + const QByteArray initialCache = cacheContents(); + ASSERT_FALSE(initialCache.isEmpty()) << "first load must write the binary cache"; + + // Drop a new custom set in: the input file list changes, so the next load + // must re-parse. BRD2 is unknown, so its set starts disabled. + QFile customSetFile(root + "/customsets/customset2.xml"); + ASSERT_TRUE(customSetFile.open(QIODevice::WriteOnly | QIODevice::Truncate)); + customSetFile.write("\n" + "\n" + " \n" + " \n" + " Finch\n" + " BRD2\n" + " 0\n" + " Chirp!\n" + " \n" + " \n" + "\n"); + customSetFile.close(); + + db.reloadCardDatabasesAndNotify(); + const QByteArray afterCustomSetAdded = cacheContents(); + EXPECT_NE(initialCache, afterCustomSetAdded) << "adding a custom card database must bust the cache"; + ASSERT_FALSE(db.getCardList().value("Finch").isNull()) << "the custom card must be loaded"; + EXPECT_TRUE(db.getCardList().value("Finch")->getSets().isEmpty()) + << "BRD2 starts disabled, so Finch has no printings yet"; + + // Enabling the freshly discovered set busts the cache again, so the next + // load finally parses its printings (#7455). + controller.setEnabled("BRD2", true); + db.reloadCardDatabasesAndNotify(); + EXPECT_NE(afterCustomSetAdded, cacheContents()) << "enabling a set must bust the cache"; + ASSERT_FALSE(db.getCardList().value("Finch").isNull()); + EXPECT_TRUE(db.getCardList().value("Finch")->getSets().contains("BRD2")); +} + +TEST_F(CardDatabaseCacheInvalidationTest, ExplicitReloadBypassesCache) +{ + NoopCardPreferenceProvider prefs; + SandboxPathProvider pathProvider(root); + CardDatabase db(nullptr, &prefs, &pathProvider, &controller); + + db.loadCardDatabases(); + ASSERT_FALSE(db.getCardList().value("Cat").isNull()); + ASSERT_TRUE(db.getCardList().value("Cat")->getSets().contains("CAT")); + + // Move Cat to the DOG set in place, keeping the file size identical and + // restoring the modification time afterwards: the source hash (path, size, + // mtime) still matches, so the cache stays valid by construction and only + // an explicit reload can pick the edit up. + const QString cardsPath = root + "/cards.xml"; + QFile file(cardsPath); + ASSERT_TRUE(file.open(QIODevice::ReadOnly)); + const qint64 stampedSecs = QFileInfo(cardsPath).lastModified().toSecsSinceEpoch(); + QByteArray contents = file.readAll(); + file.close(); + const QByteArray oldSetCode("CAT"); + const QByteArray newSetCode("DOG"); + ASSERT_EQ(contents.count(oldSetCode), 1) << "fixture must contain exactly one CAT printing"; + contents.replace(contents.indexOf(oldSetCode), oldSetCode.size(), newSetCode); + ASSERT_TRUE(file.open(QIODevice::ReadWrite)); + ASSERT_EQ(file.write(contents), contents.size()); + // Flush the pending write before stamping the time: close() would otherwise + // flush it afterwards and bump the modification time again, invalidating the + // cache on platforms that buffer writes (e.g. macOS). + ASSERT_TRUE(file.flush()); + ASSERT_TRUE(file.setFileTime(QDateTime::fromSecsSinceEpoch(stampedSecs), QFileDevice::FileModificationTime)); + file.close(); + ASSERT_EQ(QFileInfo(cardsPath).lastModified().toSecsSinceEpoch(), stampedSecs) + << "the sandbox file's modification time must be restored for the cache to stay valid"; + + // The cache is still valid for a regular load, which serves the stale data. + db.loadCardDatabases(); + ASSERT_FALSE(db.getCardList().value("Cat").isNull()); + EXPECT_TRUE(db.getCardList().value("Cat")->getSets().contains("CAT")) << "regular load must hit the cache"; + EXPECT_FALSE(db.getCardList().value("Cat")->getSets().contains("DOG")); + + // The explicit reload ignores the cache and re-reads the file. + const QByteArray staleCache = cacheContents(); + db.reloadCardDatabasesAndNotify(); + EXPECT_NE(staleCache, cacheContents()) << "explicit reload must rewrite the cache from disk"; + ASSERT_FALSE(db.getCardList().value("Cat").isNull()); + EXPECT_FALSE(db.getCardList().value("Cat")->getSets().contains("CAT")); + EXPECT_TRUE(db.getCardList().value("Cat")->getSets().contains("DOG")); +} } // namespace int main(int argc, char **argv) diff --git a/tests/oracle/oracle_importer_benchmark_test.cpp b/tests/oracle/oracle_importer_benchmark_test.cpp index 77b44af71..c2a4aaf4b 100644 --- a/tests/oracle/oracle_importer_benchmark_test.cpp +++ b/tests/oracle/oracle_importer_benchmark_test.cpp @@ -62,12 +62,10 @@ static QByteArray buildSyntheticData(int numSets, int cardsPerSet) card["colors"] = QJsonArray{"W"}; card["colorIdentity"] = QJsonArray{"W"}; card["types"] = QJsonArray{"Creature"}; - // Real MTGJSON types: floats and booleans, not strings. This + // Real MTGJSON types: floats, not strings. This // exercises the QVariant coercion in the property reader. card["convertedManaCost"] = 1.0; card["manaValue"] = 1.0; - card["isOnlineOnly"] = false; - card["isRebalanced"] = false; QJsonObject legalities; legalities["standard"] = "legal"; diff --git a/tests/settings/settings_migration_test.cpp b/tests/settings/settings_migration_test.cpp index 8994ebca6..440e49b74 100644 --- a/tests/settings/settings_migration_test.cpp +++ b/tests/settings/settings_migration_test.cpp @@ -236,7 +236,6 @@ TEST_F(SettingsMigrationTest, MigratesAllSettingsGroups) g.setValue("cards/roundcardcorners", false); g.setValue("cards/overrideallcardartwithpersonalpreference", true); g.setValue("cards/bumpsetswithcardsindecktotop", false); - g.setValue("cards/includerebalancedcards", false); g.setValue("cards/autorotatesidewayslayoutcards", false); g.setValue("cards/tapanimation", true); g.setValue("cards/scaleCards", false); @@ -395,7 +394,6 @@ TEST_F(SettingsMigrationTest, MigratesAllSettingsGroups) ASSERT_EQ(readFromIni("cards_display.ini", "cards/roundCardCorners"), QVariant(false)); ASSERT_EQ(readFromIni("cards_display.ini", "cards/overrideAllCardArtWithPersonalPreference"), QVariant(true)); ASSERT_EQ(readFromIni("cards_display.ini", "cards/bumpSetsWithCardsInDeckToTop"), QVariant(false)); - ASSERT_EQ(readFromIni("cards_display.ini", "cards/includerebalancedcards"), QVariant(false)); ASSERT_EQ(readFromIni("cards_display.ini", "cards/autoRotateSidewaysLayoutCards"), QVariant(false)); ASSERT_EQ(readFromIni("cards_display.ini", "cards/tapAnimation"), QVariant(true)); ASSERT_EQ(readFromIni("cards_display.ini", "cards/scaleCards"), QVariant(false));