From ec80acb7abbd9d36562869632228a55aab17e3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Thu, 23 Jul 2026 04:17:45 +0200 Subject: [PATCH] Address comments --- cockatrice/src/interface/window_main.cpp | 4 ++++ cockatrice/src/main.cpp | 2 ++ .../card/database/card_database.cpp | 3 --- .../card/database/card_database.h | 3 ++- .../card/database/card_database_cache.cpp | 24 +++++++++++++------ .../card/database/card_database_loader.cpp | 12 +++++++++- .../card/database/card_database_loader.h | 6 ++--- .../interface_card_set_priority_controller.h | 4 ++-- 8 files changed, 41 insertions(+), 17 deletions(-) diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 3f44343ef..ef66b45cf 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -541,6 +541,10 @@ MainWindow::MainWindow(QWidget *parent) void MainWindow::startupConfigCheck() { + // Signals for unknown/failed DB were emitted during the front-loaded parse + // before MainWindow existed. Drive the UX now that receivers are connected. + CardDatabaseManager::getInstance()->checkUnknownSets(); + if (SettingsCache::instance().debug().getLocalGameOnStartup()) { LocalGameOptions options; options.numberPlayers = SettingsCache::instance().debug().getLocalGamePlayerCount(); diff --git a/cockatrice/src/main.cpp b/cockatrice/src/main.cpp index 6aee7ae26..57fd5a870 100644 --- a/cockatrice/src/main.cpp +++ b/cockatrice/src/main.cpp @@ -264,6 +264,8 @@ int main(int argc, char *argv[]) // contention that happens when the load runs alongside window construction. // The CardDatabaseModel populates from the already-loaded data in its // constructor, so the window appears fully populated with no startup lag. + // Note: unknown-set / failure signals are deferred; MainWindow triggers + // checkUnknownSets() in startupConfigCheck() once receivers are connected. CardDatabaseManager::getInstance()->loadCardDatabases(); MainWindow ui; diff --git a/libcockatrice_card/libcockatrice/card/database/card_database.cpp b/libcockatrice_card/libcockatrice/card/database/card_database.cpp index a7e35010e..443552eb3 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database.cpp @@ -248,9 +248,6 @@ void CardDatabase::swapInDatabaseData(CardDatabaseData data) loadStatus = cards.isEmpty() ? NotLoaded : Ok; - // Detect newly-encountered sets now that the live data is populated. - checkUnknownSets(); - // inform listeners that the whole database was replaced; they should // rebuild from the live containers in a single batch instead of reacting // to individual card additions. diff --git a/libcockatrice_card/libcockatrice/card/database/card_database.h b/libcockatrice_card/libcockatrice/card/database/card_database.h index 0a51785ed..8016953c9 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database.h @@ -54,12 +54,13 @@ protected: /** @brief Querier for higher-level card lookups. */ CardDatabaseQuerier *querier; -private: +public: /** * @brief Check for sets that are unknown and emit signals if needed. */ void checkUnknownSets(); +private: /** * @brief Refreshes the cached reverse-related cards for all cards. */ diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp index 22ce5d0c8..22690190b 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp @@ -6,11 +6,13 @@ #include "../relation/card_relation.h" #include "../relation/card_relation_type.h" #include "../set/card_set.h" +#include "card_database_loader.h" #include #include #include #include +#include #include namespace @@ -50,6 +52,9 @@ QByteArray readHashBlob(QDataStream &in) { quint32 len = 0; in >> len; + if (in.status() != QDataStream::Ok || static_cast(len) > in.device()->bytesAvailable()) { + return {}; + } QByteArray blob(len, Qt::Uninitialized); in.readRawData(blob.data(), static_cast(len)); return blob; @@ -72,7 +77,7 @@ QDate readDate(QDataStream &in) void writeRelation(QDataStream &out, const CardRelation *rel) { writeString(out, rel->getName()); - out << static_cast(static_cast(rel->getAttachType())); + out << static_cast(rel->getAttachType()); out << rel->getIsCreateAllExclusion(); out << rel->getIsVariable(); out << rel->getDefaultCount(); @@ -323,7 +328,7 @@ FormatRulesPtr readFormat(QDataStream &in) bool CardDatabaseCache::write(const QString &cachePath, const CardDatabaseData &data, const QByteArray &sourceHash) { - QFile file(cachePath); + QSaveFile file(cachePath); if (!file.open(QIODevice::WriteOnly)) { return false; } @@ -355,8 +360,7 @@ bool CardDatabaseCache::write(const QString &cachePath, const CardDatabaseData & writeFormat(out, format); } - file.close(); - return file.error() == QFile::NoError; + return file.commit(); } bool CardDatabaseCache::read(const QString &cachePath, @@ -371,12 +375,12 @@ bool CardDatabaseCache::read(const QString &cachePath, // Read the whole cache into memory up front; deserialization then works on an // in-memory buffer with no further disk I/O. - const QByteArray raw = file.readAll(); + QByteArray raw = file.readAll(); if (raw.isEmpty()) { return false; } - QBuffer buffer(const_cast(&raw)); + QBuffer buffer(&raw); buffer.open(QIODevice::ReadOnly); QDataStream in(&buffer); in.setVersion(QDataStream::Qt_6_4); @@ -416,6 +420,7 @@ bool CardDatabaseCache::read(const QString &cachePath, for (const PrintingInfo &printing : printings) { if (auto set = printing.getSet()) { set->append(card); + break; } } } @@ -430,7 +435,12 @@ bool CardDatabaseCache::read(const QString &cachePath, data.formats.insert(format->formatName.toLower(), format); } - qInfo() << "[cache] read + deserialize" << deserializeTimer.elapsed() << "ms for" << cardCount << "cards"; + qCInfo(CardDatabaseLoadingLog) << "[cache] read + deserialize" << deserializeTimer.elapsed() << "ms for" + << cardCount << "cards"; + + if (in.status() != QDataStream::Ok) { + return false; + } return file.error() == QFile::NoError; } diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp index e4a997f09..d4dc567b6 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 "parser/cockatrice_xml_4.h" #include +#include #include #include #include @@ -118,7 +119,7 @@ LoadStatus CardDatabaseLoader::doLoadCardDatabases() } // AFTER all the cards have been loaded: resolve the reverse-related tags - // against the fully-built snapshot (off the GUI thread). + // against the fully-built snapshot. database->refreshCachedReverseRelatedCards(data.cards); if (loadStatus == Ok) { @@ -143,6 +144,12 @@ QByteArray CardDatabaseLoader::computeSourceHash() const // Hash over the paths, sizes and modification times of every input file so // the cache invalidates when any source changes. Cheap (no content read). QCryptographicHash hash(QCryptographicHash::Sha256); + + // Include the application version so parser changes automatically invalidate + // old caches even when the XML files are byte-identical. + hash.addData(QCoreApplication::applicationVersion().toUtf8()); + hash.addData(QByteArray(1, '\0')); + const QStringList inputs = QStringList() << pathProvider->getCardDatabasePath() << pathProvider->getTokenDatabasePath() << pathProvider->getSpoilerCardDatabasePath() << collectCustomDatabasePaths(); @@ -150,8 +157,11 @@ QByteArray CardDatabaseLoader::computeSourceHash() const QFileInfo info(path); if (info.exists()) { hash.addData(path.toUtf8()); + hash.addData(QByteArray(1, '\0')); hash.addData(QByteArray::number(info.size())); + hash.addData(QByteArray(1, '\0')); hash.addData(QByteArray::number(info.lastModified().toSecsSinceEpoch())); + 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 45b02e4c8..21e097f17 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h @@ -64,8 +64,8 @@ public slots: /** * @brief Loads all configured card databases. * - * The heavy work runs on a dedicated high-priority thread so it is not - * starved by the GUI thread's startup work on low-core machines. + * Runs synchronously on the calling thread. The caller should ensure + * that any signal receivers are already connected before invoking this. * @return Status of the main database load. */ LoadStatus loadCardDatabases(); @@ -120,7 +120,7 @@ private: LoadStatus loadFromFile(const QString &fileName, CardDatabaseData &data); /** - * @brief Performs the actual load work on the dedicated loader thread. + * @brief Performs the actual load work synchronously on the calling thread. * @return Status of the main database load. */ LoadStatus doLoadCardDatabases(); 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 9263e55cc..36e748006 100644 --- a/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h +++ b/libcockatrice_interfaces/libcockatrice/interfaces/interface_card_set_priority_controller.h @@ -25,8 +25,8 @@ public: struct SetOptions { unsigned int sortKey = 0; - bool enabled = true; - bool isKnown = true; + bool enabled = false; + bool isKnown = false; }; virtual ~ICardSetPriorityController() = default;