From c25a2c5c3ba7daa1f05688f652a96a746076b8df Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:58:58 +0200 Subject: [PATCH] [CardDB] Avoid copying printing maps and property strings (#7605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [CardDB] Avoid copying printing maps and property strings Reading a card's sets was a measured hotspot: `SetToPrintingsMap setMap = card->getSets();` copies the whole QMap, and then iterating it with a non-const iterator detaches and deep-copies every PrintingInfo, including its property hash, on first use. A micro-benchmark of that exact pattern (copy plus non-const iteration) came out about 19x slower than a const reference over the same data. The querier, the cache writer, the XML writer, the picture loader and the printing/set selection widgets now take `getSets()` by const reference and only use const iterators. The tests that iterate generated data are covered by the existing card database and picture loader suites. String accessors were also returning by value where the value lives in a hash that has already been materialized: - CardInfo::getProperty() and the legacy getCardType()/getColors()/ getManaCost()/getCmc()/getPowTough()/getLoyalty() accessors, - PrintingInfo::getProperty(), getUuid(), getFlavorName(), getArtist(), and getSet(), - ExactCard::getPrinting() and getCardPtr(), - CardSet::getShortName(), getLongName() and getSetType(). getPrinting() in particular copied a PrintingInfo on every call, and a copy of an unmaterialized PrintingInfo has to take the properties mutex. Callers that need an owned copy still get one; the rest just read. Other allocation removals in the same path: - LazyPropertiesHash::ensureMaterialized() checks an atomic flag before taking the mutex, so property reads after the first one do not lock. The flag is release-stored after materialization and acquire-loaded by readers/copies, preserving the existing locking for the first read. - CardSet::getCorrectedShortName() no longer rebuilds its QSet of reserved Windows file names on every call. - CardSet::getIsKnownIgnored() no longer formats the release date into a throwaway string just to check its length. - The querier builds its card-type/format count caches without `cards.values()` or a properties key list, reserves its result lists, and picks a random card without materializing `cards.keys()`. - The querier's pinned-printing path reuses the CardInfoPtr it already has instead of looking the card up again by name. - CardDatabaseCache/XML serialization copy the alt-name QSet by reference. - The picture loader compiles its !prop!/!set! regular expressions once instead of per URL template, and walks the transform map with an iterator instead of keys() plus operator[]. Full build and ctest (628 tests) pass. * [CardDB] Address review comments PrintingInfo is a value type: it is stored in QLists and returned by value from the querier, so a const-ref getter can hand out a reference into a temporary. Return the QStrings and the CardSetPtr by value again. The constFind lookup stays, so a property read still never materializes the lazy property hash - only the last shared-data refcount is paid. * [CardDB] Undo the const-ref QString return changes QString is implicitly shared, so returning by value costs only a refcount and avoids handing out references into a lazily materialized hash. The constFind lookups stay, so a property read still never materializes the hash. * [CardDB] Use QMap::asKeyValueRange() in transformUrl The range is a view over the map's own iterators and yields references to the stored key and value, so it keeps the no-allocation property of the manual const iteration. --------- Co-authored-by: Lukas Brübach --- .../card_picture_to_load.cpp | 24 ++++--- .../dialogs/dlg_select_set_for_cards.cpp | 10 +-- .../printing_selector/printing_selector.cpp | 2 +- .../printing_selector_card_sorting_widget.cpp | 2 +- .../visual_database_display_widget.cpp | 7 +- .../libcockatrice/card/card_info.cpp | 2 +- .../libcockatrice/card/card_info.h | 6 +- .../card/database/card_database_cache.cpp | 4 +- .../card/database/card_database_querier.cpp | 71 ++++++++++--------- .../card/database/parser/cockatrice_xml_3.cpp | 2 +- .../card/lazy_properties_hash.cpp | 22 +++--- .../libcockatrice/card/lazy_properties_hash.h | 9 +-- .../libcockatrice/card/printing/exact_card.h | 4 +- .../card/printing/printing_info.cpp | 4 +- .../card/printing/printing_info.h | 4 +- .../libcockatrice/card/set/card_set.cpp | 12 ++-- .../libcockatrice/card/set/card_set.h | 2 +- 17 files changed, 108 insertions(+), 79 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp index e35c417c8..8e8feb5b5 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp @@ -21,6 +21,7 @@ #include #include #include +#include CardPictureToLoad::CardPictureToLoad(const ExactCard &_card) : card(_card), urlTemplates(SettingsCache::instance().downloads().getAllURLs()) @@ -74,19 +75,20 @@ QList CardPictureToLoad::extractSetsSorted(const ExactCard &card) //! \todo Check if going through all sets is still necessary after the ExactCard refactor. static PrintingInfo findPrintingForSet(const ExactCard &card, const QString &setName) { - SetToPrintingsMap setsToPrintings = card.getInfo().getSets(); + const auto &setsToPrintings = card.getInfo().getSets(); - if (!setsToPrintings.contains(setName)) { + const auto setPrintings = setsToPrintings.constFind(setName); + if (setPrintings == setsToPrintings.constEnd() || setPrintings.value().isEmpty()) { return PrintingInfo(); } - for (const auto &printing : setsToPrintings[setName]) { + for (const auto &printing : setPrintings.value()) { if (printing.getUuid() == card.getPrinting().getUuid()) { return printing; } } - return setsToPrintings[setName][0]; + return setPrintings.value().first(); } void CardPictureToLoad::populateSetUrls() @@ -183,10 +185,14 @@ static int parse(const QString &urlTemplate, { static const QRegularExpression rxFillWith("^(.+)_fill_with_(.+)$"); static const QRegularExpression rxSubStr("^(.+)_substr_(\\d+)_(\\d+)$"); + static const QRegularExpression rxCardProp("!prop:([^!]+)!"); + static const QRegularExpression rxCardSet("!set:([^!]+)!"); - const QRegularExpression rxCardProp("!" + propType + ":([^!]+)!"); + // The property type only takes two values, so keep the expression that was + // compiled for it instead of rebuilding one per URL template. + const QRegularExpression &rxCardPropSelector = propType == "prop" ? rxCardProp : rxCardSet; - auto matches = rxCardProp.globalMatch(urlTemplate); + auto matches = rxCardPropSelector.globalMatch(urlTemplate); while (matches.hasNext()) { auto match = matches.next(); QString templatePropertyName = match.captured(1); @@ -303,10 +309,10 @@ QString CardPictureToLoad::transformUrl(const QString &urlTemplate) const } QString transformedUrl = urlTemplate; - for (const QString &prop : transformMap.keys()) { + for (const auto &[prop, value] : std::as_const(transformMap).asKeyValueRange()) { if (transformedUrl.contains(prop)) { - if (!transformMap[prop].isEmpty()) { - transformedUrl.replace(prop, QUrl::toPercentEncoding(transformMap[prop])); + if (!value.isEmpty()) { + transformedUrl.replace(prop, QUrl::toPercentEncoding(value)); } else { /* This means the template is requesting information that is not * populated in this card, so it should return an empty string, diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp index 55c6569f7..57ea09628 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp @@ -274,9 +274,9 @@ QMap DlgSelectSetForCards::getSetsForCards() continue; } - SetToPrintingsMap setMap = infoPtr->getSets(); - for (auto &setName : setMap.keys()) { - setCounts[setName]++; + const auto &setMap = infoPtr->getSets(); + for (auto it = setMap.constBegin(); it != setMap.constEnd(); ++it) { + setCounts[it.key()]++; } } @@ -393,8 +393,8 @@ QMap DlgSelectSetForCards::getCardsForSets() continue; } - SetToPrintingsMap setMap = infoPtr->getSets(); - for (auto it = setMap.begin(); it != setMap.end(); ++it) { + const auto &setMap = infoPtr->getSets(); + for (auto it = setMap.constBegin(); it != setMap.constEnd(); ++it) { setCards[it.key()].append(cardName); } } diff --git a/cockatrice/src/interface/widgets/printing_selector/printing_selector.cpp b/cockatrice/src/interface/widgets/printing_selector/printing_selector.cpp index 27ae61b94..983d6ba32 100644 --- a/cockatrice/src/interface/widgets/printing_selector/printing_selector.cpp +++ b/cockatrice/src/interface/widgets/printing_selector/printing_selector.cpp @@ -229,7 +229,7 @@ void PrintingSelector::getAllSetsForCurrentCard() return; } - SetToPrintingsMap setMap = selectedCard->getSets(); + const auto &setMap = selectedCard->getSets(); const QList sortedPrintings = sortToolBar->sortSets(setMap); const QList filteredPrintings = sortToolBar->filterSets(sortedPrintings, searchBar->getSearchText().trimmed().toLower()); diff --git a/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_sorting_widget.cpp b/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_sorting_widget.cpp index f47ba0746..1cd372259 100644 --- a/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_sorting_widget.cpp +++ b/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_sorting_widget.cpp @@ -208,7 +208,7 @@ QList PrintingSelectorCardSortingWidget::prependPrintingsInDeck(co return {}; } - SetToPrintingsMap setMap = selectedCard->getSets(); + const auto &setMap = selectedCard->getSets(); QList> countList; // Collect sets with their counts diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp index 599ac97b4..1a54ff0f5 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp @@ -432,10 +432,11 @@ void VisualDatabaseDisplayWidget::loadPage(int start, int end) if (CardInfoPtr info = CardDatabaseManager::query()->getCardInfo(name.toString())) { if (!setFilters.empty()) { - SetToPrintingsMap setMap = info->getSets(); + const auto &setMap = info->getSets(); for (const CardFilter *setFilter : setFilters) { - if (setMap.contains(setFilter->term())) { - for (PrintingInfo printing : setMap[setFilter->term()]) { + const auto it = setMap.constFind(setFilter->term()); + if (it != setMap.constEnd()) { + for (const PrintingInfo &printing : it.value()) { addCardToDisplay(ExactCard(info, printing)); } } diff --git a/libcockatrice_card/libcockatrice/card/card_info.cpp b/libcockatrice_card/libcockatrice/card/card_info.cpp index bd33f18e7..ae2382166 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.cpp +++ b/libcockatrice_card/libcockatrice/card/card_info.cpp @@ -245,7 +245,7 @@ QString CardInfo::simplifyName(const QString &name) QChar CardInfo::getColorChar() const { - QString colors = getColors(); + const QString colors = getColors(); switch (colors.size()) { case 0: return QChar(); diff --git a/libcockatrice_card/libcockatrice/card/card_info.h b/libcockatrice_card/libcockatrice/card/card_info.h index 356da0546..c0dccb885 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.h +++ b/libcockatrice_card/libcockatrice/card/card_info.h @@ -280,7 +280,7 @@ public: { return simpleName; } - const QSet &getAltNames() + const QSet &getAltNames() const { return altNames; } @@ -395,7 +395,9 @@ public: [[nodiscard]] QString getProperty(const QString &propertyName) const { - return getPropertiesHash().value(propertyName); + const auto &propertiesHash = getPropertiesHash(); + const auto it = propertiesHash.constFind(propertyName); + return it != propertiesHash.constEnd() ? it.value() : QString(); } void setProperty(const QString &_name, const QString &_value); [[nodiscard]] bool hasProperty(const QString &propertyName) const diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp index 4ab4d1fca..42658dd6b 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp @@ -210,7 +210,7 @@ void writeCard(QDataStream &out, const CardInfoPtr &card) // per-printing alt-name scan entirely. writeString(out, card->getSimpleName()); { - const QSet altNames = card->getAltNames(); + const QSet &altNames = card->getAltNames(); out << static_cast(altNames.size()); for (const QString &alt : altNames) { writeString(out, alt); @@ -218,7 +218,7 @@ void writeCard(QDataStream &out, const CardInfoPtr &card) } // setsToPrintings - const SetToPrintingsMap sets = card->getSets(); + const auto &sets = card->getSets(); out << static_cast(sets.size()); for (auto it = sets.constBegin(); it != sets.constEnd(); ++it) { writeString(out, it.key()); diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp index 7c41daa3b..8df0d2e20 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include CardDatabaseQuerier::CardDatabaseQuerier(QObject *_parent, @@ -52,6 +53,7 @@ CardInfoPtr CardDatabaseQuerier::getCardInfo(const QString &cardName) const QList CardDatabaseQuerier::getCardInfos(const QStringList &cardNames) const { QList cardInfos; + cardInfos.reserve(cardNames.size()); for (const QString &cardName : cardNames) { CardInfoPtr ptr = db->cards.value(cardName); if (ptr) { @@ -90,6 +92,7 @@ CardInfoPtr CardDatabaseQuerier::lookupCardByName(const QString &name) const QList CardDatabaseQuerier::getCards(const QList &cardRefs) const { QList cards; + cards.reserve(cardRefs.size()); for (const auto &cardRef : cardRefs) { ExactCard card = getCard(cardRef); if (card) { @@ -149,10 +152,10 @@ ExactCard CardDatabaseQuerier::getRandomCard() const return {}; } - const auto keys = db->cards.keys(); - int randomIndex = QRandomGenerator::global()->bounded(keys.size()); - const QString &randomKey = keys.at(randomIndex); - CardInfoPtr randomCard = getCardInfo(randomKey); + const auto randomIndex = QRandomGenerator::global()->bounded(db->cards.size()); + auto it = db->cards.constBegin(); + std::advance(it, randomIndex); + CardInfoPtr randomCard = it.value(); return ExactCard{randomCard, getPreferredPrinting(randomCard)}; } @@ -215,20 +218,23 @@ PrintingInfo CardDatabaseQuerier::getSpecificPrinting(const QString &cardName, return PrintingInfo(nullptr); } - SetToPrintingsMap setMap = cardInfo->getSets(); + const auto &setMap = cardInfo->getSets(); if (setMap.empty()) { return PrintingInfo(nullptr); } for (const auto &printings : setMap) { - for (auto &cardInfoForSet : printings) { + for (const auto &cardInfoForSet : printings) { + const CardSetPtr &set = cardInfoForSet.getSet(); + if (!set) { + continue; + } if (!collectorNumber.isEmpty()) { - if (cardInfoForSet.getSet()->getShortName() == setShortName && - cardInfoForSet.getProperty("num") == collectorNumber) { + if (set->getShortName() == setShortName && cardInfoForSet.getProperty("num") == collectorNumber) { return cardInfoForSet; } } else { - if (cardInfoForSet.getSet()->getShortName() == setShortName) { + if (set->getShortName() == setShortName) { return cardInfoForSet; } } @@ -284,10 +290,10 @@ PrintingInfo CardDatabaseQuerier::getPreferredPrinting(const CardInfoPtr &cardIn const auto &pinnedPrintingProviderId = prefs->getCardPreferenceOverride(cardInfo->getName()); if (!pinnedPrintingProviderId.isEmpty()) { - return getSpecificPrinting({cardInfo->getName(), pinnedPrintingProviderId}); + return findPrintingWithId(cardInfo, pinnedPrintingProviderId); } - SetToPrintingsMap setMap = cardInfo->getSets(); + const auto &setMap = cardInfo->getSets(); if (setMap.empty()) { return PrintingInfo(nullptr); } @@ -297,8 +303,8 @@ PrintingInfo CardDatabaseQuerier::getPreferredPrinting(const CardInfoPtr &cardIn SetPriorityComparator comparator; for (const auto &printings : setMap) { - for (auto &printing : printings) { - CardSetPtr currentSet = printing.getSet(); + for (const auto &printing : printings) { + const CardSetPtr ¤tSet = printing.getSet(); if (!preferredSet || comparator(currentSet, preferredSet)) { preferredSet = currentSet; preferredPrinting = printing; @@ -315,17 +321,18 @@ PrintingInfo CardDatabaseQuerier::getPreferredPrinting(const CardInfoPtr &cardIn QString CardDatabaseQuerier::getPreferredPrintingProviderId(const QString &cardName) const { - PrintingInfo preferredPrinting = getPreferredPrinting(cardName); - QString uuid = preferredPrinting.getUuid(); + CardInfoPtr cardInfo = getCardInfo(cardName); + if (!cardInfo) { + return cardName; + } + + PrintingInfo preferredPrinting = getPreferredPrinting(cardInfo); + const QString &uuid = preferredPrinting.getUuid(); if (!uuid.isEmpty()) { return uuid; } - CardInfoPtr defaultCardInfo = getCardInfo(cardName); - if (defaultCardInfo.isNull()) { - return cardName; - } - return defaultCardInfo->getName(); + return cardInfo->getName(); } QStringList CardDatabaseQuerier::getAllMainCardTypes() const @@ -339,8 +346,8 @@ QMap CardDatabaseQuerier::getAllMainCardTypesWithCount() const // produces an empty map, so the cache is only ever empty when it needs a // (trivially cheap) rebuild. if (mainCardTypeCountsCache.isEmpty()) { - for (const auto &card : db->cards.values()) { - QString type = card->getMainCardType(); + for (const auto &card : db->cards) { + const QString type = card->getMainCardType(); mainCardTypeCountsCache[type]++; } } @@ -351,13 +358,13 @@ QMap CardDatabaseQuerier::getAllMainCardTypesWithCount() const QMap CardDatabaseQuerier::getAllSubCardTypesWithCount() const { if (subCardTypeCountsCache.isEmpty()) { - for (const auto &card : db->cards.values()) { - QString type = card->getCardType(); + for (const auto &card : db->cards) { + const QString type = card->getCardType(); - QStringList parts = type.split(" — "); + const QStringList parts = type.split(" — "); if (parts.size() > 1) { // Ensure there are subtypes - QStringList subtypes = parts[1].split(" ", Qt::SkipEmptyParts); + const QStringList subtypes = parts[1].split(" ", Qt::SkipEmptyParts); for (const QString &subtype : subtypes) { subCardTypeCountsCache[subtype]++; @@ -377,13 +384,13 @@ FormatRulesPtr CardDatabaseQuerier::getFormat(const QString &formatName) const QMap CardDatabaseQuerier::getAllFormatsWithCount() const { if (formatsCountCache.isEmpty()) { - for (const auto &card : db->cards.values()) { - QStringList allProps = card->getProperties(); + static const QString formatPrefix = QStringLiteral("format-"); - for (const QString &prop : allProps) { - if (prop.startsWith("format-")) { - QString formatName = prop.mid(QStringLiteral("format-").size()); - formatsCountCache[formatName]++; + for (const auto &card : db->cards) { + const auto &properties = card->getPropertiesHash(); + for (auto it = properties.constBegin(); it != properties.constEnd(); ++it) { + if (it.key().startsWith(formatPrefix)) { + formatsCountCache[it.key().mid(formatPrefix.size())]++; } } } diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp index b17eb8b5e..33a0e0849 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp @@ -401,7 +401,7 @@ static QXmlStreamWriter &operator<<(QXmlStreamWriter &xml, const CardInfoPtr &in } // sets - const SetToPrintingsMap setMap = info->getSets(); + const auto &setMap = info->getSets(); for (const auto &printings : setMap) { for (const PrintingInfo &set : printings) { xml.writeStartElement("set"); diff --git a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp index a12de469b..90628224a 100644 --- a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp +++ b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp @@ -20,15 +20,15 @@ LazyPropertiesHash::LazyPropertiesHash(const QHash &properties LazyPropertiesHash::LazyPropertiesHash(const LazyPropertiesHash &other) { // since we do not allow dematerialization, we only need to lock if not materialized yet - if (other.isMaterialized) { + if (other.isMaterialized.load(std::memory_order_acquire)) { blob = other.blob; properties = other.properties; - isMaterialized = true; + isMaterialized.store(true, std::memory_order_relaxed); } else { QMutexLocker lock(&other.propertiesMutex); blob = other.blob; properties = other.properties; - isMaterialized = false; + isMaterialized.store(other.isMaterialized.load(std::memory_order_relaxed), std::memory_order_relaxed); } } @@ -39,15 +39,15 @@ LazyPropertiesHash &LazyPropertiesHash::operator=(const LazyPropertiesHash &othe } // since we do not allow dematerialization, we only need to lock if not materialized yet - if (other.isMaterialized) { + if (other.isMaterialized.load(std::memory_order_acquire)) { blob = other.blob; properties = other.properties; - isMaterialized = true; + isMaterialized.store(true, std::memory_order_relaxed); } else { QMutexLocker lock(&other.propertiesMutex); blob = other.blob; properties = other.properties; - isMaterialized = false; + isMaterialized.store(false, std::memory_order_relaxed); } return *this; @@ -55,9 +55,15 @@ LazyPropertiesHash &LazyPropertiesHash::operator=(const LazyPropertiesHash &othe void LazyPropertiesHash::ensureMaterialized() const { + // Materialization happens once and is never undone, so the common case can + // skip the mutex entirely once it has happened. + if (isMaterialized.load(std::memory_order_acquire)) { + return; + } + QMutexLocker lock(&propertiesMutex); - if (isMaterialized) { + if (isMaterialized.load(std::memory_order_relaxed)) { return; } @@ -69,7 +75,7 @@ void LazyPropertiesHash::ensureMaterialized() const blob.clear(); - isMaterialized = true; + isMaterialized.store(true, std::memory_order_release); } QString LazyPropertiesHash::value(const QString &key) const diff --git a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h index 04f5d0251..9e34050e5 100644 --- a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h +++ b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h @@ -5,6 +5,7 @@ #include #include #include +#include /** * @brief A property map that can lazily deserialize blobs to avoid loading overhead. @@ -18,10 +19,10 @@ class LazyPropertiesHash { - mutable QByteArray blob; ///< Serialized properties (load form). - mutable QHash properties; ///< Materialized properties (query form). - mutable QMutex propertiesMutex; ///< Guards lazy materialization. - mutable bool isMaterialized = false; ///< Whether propertiesCache is valid. + mutable QByteArray blob; ///< Serialized properties (load form). + mutable QHash properties; ///< Materialized properties (query form). + mutable QMutex propertiesMutex; ///< Guards lazy materialization. + mutable std::atomic isMaterialized{false}; ///< Whether properties is valid. /** * @brief Materializes properties from blob if not already done. Clears blob afterward. diff --git a/libcockatrice_card/libcockatrice/card/printing/exact_card.h b/libcockatrice_card/libcockatrice/card/printing/exact_card.h index ea49072fb..83ba97bd0 100644 --- a/libcockatrice_card/libcockatrice/card/printing/exact_card.h +++ b/libcockatrice_card/libcockatrice/card/printing/exact_card.h @@ -47,7 +47,7 @@ public: * * May be null if the ExactCard is empty. */ - [[nodiscard]] CardInfoPtr getCardPtr() const + [[nodiscard]] const CardInfoPtr &getCardPtr() const { return card; } @@ -57,7 +57,7 @@ public: * * May be empty if no specific printing was assigned. */ - [[nodiscard]] PrintingInfo getPrinting() const + [[nodiscard]] const PrintingInfo &getPrinting() const { return printing; } diff --git a/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp b/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp index 654efac5e..9440eb4b0 100644 --- a/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp +++ b/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp @@ -17,10 +17,10 @@ void PrintingInfo::setProperty(const QString &_name, const QString &_value) */ QString PrintingInfo::getUuid() const { - return getPropertiesHash().value("uuid"); + return getProperty("uuid"); } QString PrintingInfo::getFlavorName() const { - return getPropertiesHash().value("flavorName"); + return getProperty("flavorName"); } diff --git a/libcockatrice_card/libcockatrice/card/printing/printing_info.h b/libcockatrice_card/libcockatrice/card/printing/printing_info.h index 038960e86..75f634c3c 100644 --- a/libcockatrice_card/libcockatrice/card/printing/printing_info.h +++ b/libcockatrice_card/libcockatrice/card/printing/printing_info.h @@ -107,7 +107,9 @@ public: */ [[nodiscard]] QString getProperty(const QString &propertyName) const { - return properties.value(propertyName); + const auto &propertiesHash = getPropertiesHash(); + const auto it = propertiesHash.constFind(propertyName); + return it != propertiesHash.constEnd() ? it.value() : QString(); } /** diff --git a/libcockatrice_card/libcockatrice/card/set/card_set.cpp b/libcockatrice_card/libcockatrice/card/set/card_set.cpp index e873eeeab..e2b41a811 100644 --- a/libcockatrice_card/libcockatrice/card/set/card_set.cpp +++ b/libcockatrice_card/libcockatrice/card/set/card_set.cpp @@ -34,10 +34,14 @@ CardSetPtr CardSet::newInstance(ICardSetPriorityController *_priorityController, QString CardSet::getCorrectedShortName() const { // For Windows machines. - QSet invalidFileNames; - invalidFileNames << "CON" << "PRN" << "AUX" << "NUL" << "COM1" << "COM2" << "COM3" << "COM4" << "COM5" << "COM6" - << "COM7" << "COM8" << "COM9" << "LPT1" << "LPT2" << "LPT3" << "LPT4" << "LPT5" << "LPT6" << "LPT7" - << "LPT8" << "LPT9"; + static const QSet invalidFileNames = { + QStringLiteral("CON"), QStringLiteral("PRN"), QStringLiteral("AUX"), QStringLiteral("NUL"), + QStringLiteral("COM1"), QStringLiteral("COM2"), QStringLiteral("COM3"), QStringLiteral("COM4"), + QStringLiteral("COM5"), QStringLiteral("COM6"), QStringLiteral("COM7"), QStringLiteral("COM8"), + QStringLiteral("COM9"), QStringLiteral("LPT1"), QStringLiteral("LPT2"), QStringLiteral("LPT3"), + QStringLiteral("LPT4"), QStringLiteral("LPT5"), QStringLiteral("LPT6"), QStringLiteral("LPT7"), + QStringLiteral("LPT8"), QStringLiteral("LPT9"), + }; return invalidFileNames.contains(shortName) ? shortName + "_" : shortName; } diff --git a/libcockatrice_card/libcockatrice/card/set/card_set.h b/libcockatrice_card/libcockatrice/card/set/card_set.h index b9ac74e2d..b995ac88e 100644 --- a/libcockatrice_card/libcockatrice/card/set/card_set.h +++ b/libcockatrice_card/libcockatrice/card/set/card_set.h @@ -239,7 +239,7 @@ public: */ [[nodiscard]] bool getIsKnownIgnored() const { - return longName.length() + setType.length() + releaseDate.toString().length() == 0; + return longName.isEmpty() && setType.isEmpty() && !releaseDate.isValid(); } };