diff --git a/libcockatrice_card/libcockatrice/card/card_info.cpp b/libcockatrice_card/libcockatrice/card/card_info.cpp index ae2382166..c90027442 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.cpp +++ b/libcockatrice_card/libcockatrice/card/card_info.cpp @@ -67,10 +67,16 @@ CardInfo::CardInfo(const QString &_name, reverseRelatedCards(_reverseRelatedCards), setsToPrintings(std::move(_sets)), uiAttributes(_uiAttributes), altNames(std::move(_altNames)) { - // propertiesBlob is materialized lazily on first query; simpleName and - // altNames are supplied by the caller (binary cache). Only the set-name - // display list still depends on the current enabled-set state. + // propertiesBlob is materialized lazily on first query. The binary cache supplies simpleName + // and altNames to skip the Unicode normalization and the printing scan; the XML parsers leave + // them empty and get the same derived state the property-hash constructor computes. + if (simpleName.isEmpty()) { + simpleName = simplifyName(name); + } refreshCachedSetNames(); + if (altNames.isEmpty()) { + refreshCachedAltNames(); + } } CardInfoPtr CardInfo::newInstance(const QString &_name) diff --git a/libcockatrice_card/libcockatrice/card/card_info.h b/libcockatrice_card/libcockatrice/card/card_info.h index c0dccb885..77ddce5ce 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.h +++ b/libcockatrice_card/libcockatrice/card/card_info.h @@ -121,14 +121,12 @@ public: QMap _localizedTexts = {}); /** - * @brief Constructs a CardInfo from a cache snapshot with precomputed derived - * state. + * @brief Constructs a CardInfo from a serialized properties blob. * - * Used by the binary cache reader to skip recomputing @p _simpleName and - * @p _altNames (which otherwise require a Unicode normalization and a full - * printing scan). Properties are supplied as a pre-serialized @p _propertiesBlob - * so the QHash is not built at load time (it is materialized on first - * query). + * Properties stay in @p _propertiesBlob so the QHash is not + * built at load time (it is materialized on first query). @p _simpleName and + * @p _altNames are filled in when empty; the binary cache supplies them to + * skip the Unicode normalization and the printing scan. * * @param _name The card name. * @param _text Rules text or description of the card. @@ -138,8 +136,8 @@ public: * @param _reverseRelatedCards Reverse relationships. * @param _sets Printing information per set. * @param _uiAttributes Attributes that affect display and game logic. - * @param _simpleName Precomputed simplified name. - * @param _altNames Precomputed alternate names. + * @param _simpleName Precomputed simplified name; derived from @p _name when empty. + * @param _altNames Precomputed alternate names; derived from the printings when empty. * @param _localizedNames Localized card names, keyed by language code. * @param _localizedTexts Localized rules text, keyed by language code. */ @@ -210,20 +208,26 @@ public: QMap _localizedTexts = {}); /** - * @brief Creates a new instance from a cache snapshot with precomputed - * derived state. + * @brief Creates a new instance from a serialized properties blob. + * + * The properties stay in blob form, so the QHash is not + * built at load time (it is materialized on first query). The derived state + * is required so it is never recomputed here, and the overload cannot be a + * candidate for a shorter call that passes a property hash instead of a blob. * * @param _name Name of the card. * @param _text Rules text or description. * @param _isToken Token flag. - * @param _propertiesBlob Pre-serialized properties blob (as written by the cache). + * @param _propertiesBlob Serialized properties blob. * @param _relatedCards Forward relationships. * @param _reverseRelatedCards Reverse relationships. * @param _sets Printing information per set. * @param _uiAttributes Attributes that affect display and game logic. - * @param _simpleName Precomputed simplified name. - * @param _altNames Precomputed alternate names. - * @param _appendToSets When true (default), the card is appended to each of + * @param _simpleName Precomputed simplified name; pass {} to derive it from + * @p _name. + * @param _altNames Precomputed alternate names; pass {} to derive them from + * the printings. + * @param _appendToSets When true, the card is appended to each of * its CardSets. Pass false when building cards in parallel so the * (non-thread-safe) set membership is populated in a later * single-threaded pass. @@ -241,7 +245,7 @@ public: UiAttributes _uiAttributes, QString _simpleName, QSet _altNames, - bool _appendToSets = true, + bool _appendToSets, QMap _localizedNames = {}, QMap _localizedTexts = {}); @@ -389,20 +393,34 @@ public: } [[nodiscard]] QStringList getProperties() const { - return getPropertiesHash().keys(); + return properties.keys(); } + + /** + * @brief Returns the properties in their serialized blob form. + * + * Cheap while the properties were never materialized (the cache and the XML parsers pass the blob through). + */ + [[nodiscard]] QByteArray getPropertiesBlob() const + { + return properties.serialized(); + } + + /** + * @brief Returns the materialized property hash, forcing deserialization of the blob. + * + * Prefer getProperty(), hasProperty() or getPropertiesBlob(), which avoid building the hash. + */ [[nodiscard]] const QHash &getPropertiesHash() const; [[nodiscard]] QString getProperty(const QString &propertyName) const { - const auto &propertiesHash = getPropertiesHash(); - const auto it = propertiesHash.constFind(propertyName); - return it != propertiesHash.constEnd() ? it.value() : QString(); + return properties.value(propertyName); } void setProperty(const QString &_name, const QString &_value); [[nodiscard]] bool hasProperty(const QString &propertyName) const { - return getPropertiesHash().contains(propertyName); + return properties.contains(propertyName); } [[nodiscard]] const SetToPrintingsMap &getSets() const { diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp index 42658dd6b..611b24ad8 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_cache.cpp @@ -53,13 +53,10 @@ QString readString(QDataStream &in) // Stores a QHash as a single pre-serialized blob. The reader keeps the // blob as-is and materializes the QHash lazily on first query, which is // what removes the allocation storm from database load (see card_info.cpp / -// printing_info.cpp). -void writeHashBlob(QDataStream &out, const QHash &h) +// printing_info.cpp). Writing the already-serialized blob also keeps cache writes from +// materializing every card's property hash. +void writeBlob(QDataStream &out, const QByteArray &blob) { - QByteArray blob; - QDataStream blobOut(&blob, QIODevice::WriteOnly); - blobOut.setVersion(QDataStream::Qt_6_4); - blobOut << h; out << static_cast(blob.size()); out.writeRawData(blob.constData(), blob.size()); } @@ -156,7 +153,7 @@ CardRelation *readRelation(QDataStream &in) void writePrinting(QDataStream &out, const PrintingInfo &p) { writeString(out, p.getSet() ? p.getSet()->getShortName() : QString()); - writeHashBlob(out, p.getPropertiesHash()); + writeBlob(out, p.getPropertiesBlob()); } PrintingInfo readPrinting(QDataStream &in, const SetNameMap &sets) @@ -198,7 +195,7 @@ void writeCard(QDataStream &out, const CardInfoPtr &card) writeString(out, card->getName()); writeString(out, card->getText()); out << card->getIsToken(); - writeHashBlob(out, card->getPropertiesHash()); + writeBlob(out, card->getPropertiesBlob()); CardInfo::UiAttributes ui = card->getUiAttributes(); out << ui.cipt; 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 33a0e0849..d40c1c063 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp @@ -268,7 +268,7 @@ void CockatriceXml3Parser::loadCardsFromXml(QXmlStreamReader &xml) if (attrs.hasAttribute("rarity")) { printingProps.insert("rarity", attrs.value("rarity").toString()); } - PrintingInfo setInfo(set, LazyPropertiesHash(printingProps)); + PrintingInfo setInfo(set, LazyPropertiesHash::serializedFrom(printingProps)); _sets[setName].append(setInfo); } // related cards @@ -325,8 +325,9 @@ void CockatriceXml3Parser::loadCardsFromXml(QXmlStreamReader &xml) .landscapeOrientation = landscapeOrientation, .tableRow = tableRow, .upsideDownArt = upsideDown}; - CardInfoPtr newCard = CardInfo::newInstance(name, text, isToken, properties, relatedCards, - reverseRelatedCards, _sets, attributes); + CardInfoPtr newCard = + CardInfo::newInstance(name, text, isToken, LazyPropertiesHash::serialize(properties), relatedCards, + reverseRelatedCards, _sets, attributes, {}, {}, true); if (targetData) { if (auto existing = targetData->cards.value(name)) { for (const auto &printings : newCard->getSets()) { 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 4d975e533..3ee1e808e 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp @@ -383,7 +383,7 @@ void CockatriceXml4Parser::loadCardsFromXml(QXmlStreamReader &xml) } printingProps.insert(attrName, attr.value().toString()); } - PrintingInfo printingInfo(set, LazyPropertiesHash(printingProps)); + PrintingInfo printingInfo(set, LazyPropertiesHash::serializedFrom(printingProps)); _sets[setName].append(printingInfo); } // related cards @@ -450,9 +450,9 @@ void CockatriceXml4Parser::loadCardsFromXml(QXmlStreamReader &xml) .landscapeOrientation = landscapeOrientation, .tableRow = tableRow, .upsideDownArt = upsideDown}; - CardInfoPtr newCard = - CardInfo::newInstance(name, text, isToken, properties, relatedCards, reverseRelatedCards, _sets, - attributes, std::move(localizedNames), std::move(localizedTexts)); + CardInfoPtr newCard = CardInfo::newInstance(name, text, isToken, LazyPropertiesHash::serialize(properties), + relatedCards, reverseRelatedCards, _sets, attributes, {}, {}, + true, std::move(localizedNames), std::move(localizedTexts)); if (targetData) { // Mirror CardDatabase::addCard: if a card with this name already // exists, merge the new printings into it instead of replacing. diff --git a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp index 90628224a..3798bcd62 100644 --- a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp +++ b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.cpp @@ -1,8 +1,168 @@ #include "lazy_properties_hash.h" #include +#include #include -#include + +namespace +{ +constexpr quint32 NULL_STRING_LENGTH = 0xFFFFFFFFu; + +/** + * @brief Reads a QDataStream-serialized QString length at @p pos, advancing past the length field. + * + * QDataStream writes a QString as a big-endian quint32 byte length followed by UTF-16 code units (also big-endian), + * or 0xFFFFFFFF for a null string. Parsing at the byte level lets us skip and compare properties without + * constructing a QString for every entry. + */ +bool readBlobLength(const QByteArray &blob, int &pos, quint32 &length) +{ + if (pos < 0 || static_cast(pos) + 4 > blob.size()) { + return false; + } + + const auto *bytes = reinterpret_cast(blob.constData()) + pos; + length = (quint32(bytes[0]) << 24) | (quint32(bytes[1]) << 16) | (quint32(bytes[2]) << 8) | quint32(bytes[3]); + pos += 4; + + if (length == NULL_STRING_LENGTH) { + length = 0; + return true; + } + + if (length > static_cast(blob.size() - pos)) { + return false; + } + + return true; +} + +/** + * @brief Compares the serialized bytes at @p pos with @p key without allocating a QString. + */ +bool blobKeyEquals(const QByteArray &blob, int pos, quint32 length, const QString &key) +{ + if (length != static_cast(key.size()) * 2) { + return false; + } + + const auto *bytes = reinterpret_cast(blob.constData()) + pos; + for (int i = 0; i < key.size(); ++i) { + const ushort codeUnit = static_cast((bytes[2 * i] << 8) | bytes[2 * i + 1]); + if (key.at(i).unicode() != codeUnit) { + return false; + } + } + + return true; +} + +/** + * @brief Builds a QString from the serialized UTF-16BE bytes at @p pos. + */ +QString blobString(const QByteArray &blob, int pos, quint32 length) +{ + const int codeUnits = static_cast(length / 2); + QString result; + result.resize(codeUnits); + + const auto *bytes = reinterpret_cast(blob.constData()) + pos; + for (int i = 0; i < codeUnits; ++i) { + result[i] = QChar(static_cast((bytes[2 * i] << 8) | bytes[2 * i + 1])); + } + + return result; +} +} // namespace + +LazyPropertiesHash::BlobReader::BlobReader(const QByteArray &_blob) : blob(_blob) +{ + if (blob.size() < 4) { + return; + } + + const auto *bytes = reinterpret_cast(blob.constData()); + remaining = (quint32(bytes[0]) << 24) | (quint32(bytes[1]) << 16) | (quint32(bytes[2]) << 8) | quint32(bytes[3]); + valid = true; +} + +bool LazyPropertiesHash::BlobReader::next(int &keyPos, quint32 &keyLength, int &valuePos, quint32 &valueLength) +{ + if (!valid || remaining == 0) { + return false; + } + + if (!readBlobLength(blob, pos, keyLength)) { + valid = false; + return false; + } + keyPos = pos; + pos += static_cast(keyLength); + + if (!readBlobLength(blob, pos, valueLength)) { + valid = false; + return false; + } + valuePos = pos; + pos += static_cast(valueLength); + + --remaining; + return true; +} + +quint32 LazyPropertiesHash::BlobReader::entryCount() const +{ + return valid ? remaining : 0; +} + +QString LazyPropertiesHash::blobValue(const QByteArray &blob, const QString &key) +{ + BlobReader reader(blob); + int keyPos = 0; + int valuePos = 0; + quint32 keyLength = 0; + quint32 valueLength = 0; + while (reader.next(keyPos, keyLength, valuePos, valueLength)) { + if (blobKeyEquals(blob, keyPos, keyLength, key)) { + return blobString(blob, valuePos, valueLength); + } + } + + return {}; +} + +bool LazyPropertiesHash::blobContains(const QByteArray &blob, const QString &key) +{ + BlobReader reader(blob); + int keyPos = 0; + int valuePos = 0; + quint32 keyLength = 0; + quint32 valueLength = 0; + while (reader.next(keyPos, keyLength, valuePos, valueLength)) { + if (blobKeyEquals(blob, keyPos, keyLength, key)) { + return true; + } + } + + return false; +} + +QStringList LazyPropertiesHash::blobKeys(const QByteArray &blob) +{ + BlobReader reader(blob); + QStringList result; + result.reserve(static_cast(reader.entryCount())); + + int keyPos = 0; + int valuePos = 0; + quint32 keyLength = 0; + quint32 valueLength = 0; + while (reader.next(keyPos, keyLength, valuePos, valueLength)) { + result.append(blobString(blob, keyPos, keyLength)); + } + + return result; +} LazyPropertiesHash::LazyPropertiesHash() : isMaterialized(true) { @@ -78,10 +238,57 @@ void LazyPropertiesHash::ensureMaterialized() const isMaterialized.store(true, std::memory_order_release); } +QByteArray LazyPropertiesHash::serialize(const QHash &properties) +{ + QByteArray result; + QDataStream out(&result, QIODevice::WriteOnly); + out.setVersion(QDataStream::Qt_6_4); + out << properties; + return result; +} + +LazyPropertiesHash LazyPropertiesHash::serializedFrom(const QHash &properties) +{ + return LazyPropertiesHash(serialize(properties)); +} + QString LazyPropertiesHash::value(const QString &key) const { - ensureMaterialized(); - return properties.value(key); + return read([&key](const QHash &materialized) { return materialized.value(key); }, + [&key](const QByteArray &blob) { return LazyPropertiesHash::blobValue(blob, key); }); +} + +bool LazyPropertiesHash::contains(const QString &key) const +{ + return read([&key](const QHash &materialized) { return materialized.contains(key); }, + [&key](const QByteArray &blob) { return LazyPropertiesHash::blobContains(blob, key); }); +} + +int LazyPropertiesHash::size() const +{ + return read([](const QHash &materialized) { return static_cast(materialized.size()); }, + [](const QByteArray &blob) { return static_cast(BlobReader(blob).entryCount()); }); +} + +bool LazyPropertiesHash::isEmpty() const +{ + return size() == 0; +} + +QStringList LazyPropertiesHash::keys() const +{ + return read([](const QHash &materialized) { return QStringList(materialized.keys()); }, + [](const QByteArray &blob) { return blobKeys(blob); }); +} + +QByteArray LazyPropertiesHash::serialized() const +{ + return read([](const QHash &materialized) { return serialize(materialized); }, + [](const QByteArray &blob) { + // An empty blob is the "unknown/absent" form used by failed cache reads; give callers a + // well-formed serialization of the (empty) property map instead. + return blob.isEmpty() ? serialize({}) : blob; + }); } bool LazyPropertiesHash::insert(const QString &key, const QString &value) diff --git a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h index 9e34050e5..c8170b7c2 100644 --- a/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h +++ b/libcockatrice_card/libcockatrice/card/lazy_properties_hash.h @@ -4,20 +4,62 @@ #include #include #include +#include #include +#include #include /** * @brief A property map that can lazily deserialize blobs to avoid loading overhead. * - * Properties are stored as a pre-serialized blob (cheap to load), and the QString is materialized on the first query, - * so the database load avoids constructing thousands of QString per card. + * Properties are stored as a pre-serialized blob (cheap to load), and individual values are read straight out of that + * blob, so the database load and ordinary queries avoid constructing thousands of QString per card. * - * Once the properties are materialized, it cannot be unmaterialized. - * If you want to reset the properties to an unmaterialized state, you should create a new LazyPropertiesHash. + * A full QHash is only built on the first insert() or getProperties() call. Once materialized, it + * cannot be unmaterialized; create a new LazyPropertiesHash to return to the blob form. */ class LazyPropertiesHash { + /** + * @brief Iterates over the key/value pairs of a serialized QHash blob without materializing it. + * + * The blob layout is the one produced by QDataStream for QHash: a quint32 entry count followed by each entry's + * serialized key and value. + */ + class BlobReader + { + public: + explicit BlobReader(const QByteArray &_blob); + + /** + * @brief Reads the next pair's offsets. + * @return False when there are no more pairs or the blob is malformed. + */ + bool next(int &keyPos, quint32 &keyLength, int &valuePos, quint32 &valueLength); + + [[nodiscard]] quint32 entryCount() const; + + private: + const QByteArray &blob; + int pos = 4; + quint32 remaining = 0; + bool valid = false; + }; + + /** + * @brief Looks up @p key in a serialized blob without materializing it. + */ + static QString blobValue(const QByteArray &blob, const QString &key); + + /** + * @brief Checks whether @p key is present in a serialized blob without materializing it. + */ + static bool blobContains(const QByteArray &blob, const QString &key); + + /** + * @brief Lists the keys of a serialized blob without materializing it. + */ + static QStringList blobKeys(const QByteArray &blob); mutable QByteArray blob; ///< Serialized properties (load form). mutable QHash properties; ///< Materialized properties (query form). @@ -30,6 +72,32 @@ class LazyPropertiesHash */ void ensureMaterialized() const; + /** + * @brief Runs the accessor for the current storage form. + * + * The common case skips the mutex once the properties are materialized. While they are + * still in blob form, the mutex is taken and @p onBlob reads the values straight out of + * the blob without building the hash, so ordinary queries stay allocation-light. + * + * @param onMaterialized Called with the materialized hash when it is available. + * @param onBlob Called with the blob when it is still the source of truth. + * @return Whatever the invoked accessor returns; both accessors must return the same type. + */ + template + auto read(MaterializedFn &&onMaterialized, BlobFn &&onBlob) const + { + if (isMaterialized.load(std::memory_order_acquire)) { + return onMaterialized(properties); + } + + QMutexLocker lock(&propertiesMutex); + if (isMaterialized.load(std::memory_order_relaxed)) { + return onMaterialized(properties); + } + + return onBlob(blob); + } + public: /** * @brief Default constructor. @@ -53,12 +121,60 @@ public: LazyPropertiesHash &operator=(const LazyPropertiesHash &other); /** - * @brief Gets the value from the materialized properties hash + * @brief Serializes a property map into the blob form understood by this class. + * @param properties The properties to serialize + * @return The serialized blob + */ + static QByteArray serialize(const QHash &properties); + + /** + * @brief Creates an unmaterialized LazyPropertiesHash from a property map. + * + * Serializes @p properties once so queries read them from the blob form instead of first materializing a second + * copy of the hash. + * + * @param properties The properties to serialize + * @return An unmaterialized LazyPropertiesHash holding the serialized properties + */ + static LazyPropertiesHash serializedFrom(const QHash &properties); + + /** + * @brief Gets the value for a key, reading it from the blob when possible. * @param key The key * @return The value, or an empty string if the key is not present */ QString value(const QString &key) const; + /** + * @brief Checks whether a key is present, reading the blob when possible. + * @param key The key + * @return True if the key is present + */ + bool contains(const QString &key) const; + + /** + * @brief Returns the number of properties. + */ + int size() const; + + /** + * @brief Returns whether there are no properties. + */ + bool isEmpty() const; + + /** + * @brief Returns the property names, reading them from the blob when possible. + */ + QStringList keys() const; + + /** + * @brief Returns the properties in serialized blob form. + * + * While the object is unmaterialized this is a cheap copy of the stored blob; after materialization the hash is + * serialized on demand. + */ + QByteArray serialized() const; + /** * @brief Inserts a value into the materialized properties hash * @param key The key @@ -68,7 +184,7 @@ public: bool insert(const QString &key, const QString &value); /** - * @brief Gets a view of the materialized properties hash. + * @brief Gets a view of the materialized properties hash, materializing it if needed. * @return The properties hash */ const QHash &getProperties() const; diff --git a/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp b/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp index 9440eb4b0..9bc62a07c 100644 --- a/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp +++ b/libcockatrice_card/libcockatrice/card/printing/printing_info.cpp @@ -12,6 +12,23 @@ void PrintingInfo::setProperty(const QString &_name, const QString &_value) properties.insert(_name, _value); } +bool PrintingInfo::operator==(const PrintingInfo &other) const +{ + if (set != other.set || properties.size() != other.properties.size()) { + return false; + } + + // Both property maps are usually still in blob form here, so compare key by key instead of + // materializing two full hashes just to compare them. + for (const QString &key : properties.keys()) { + if (!other.properties.contains(key) || properties.value(key) != other.properties.value(key)) { + return false; + } + } + + return true; +} + /** * Gets the uuid property of the printing, or an empty string if the property isn't present */ diff --git a/libcockatrice_card/libcockatrice/card/printing/printing_info.h b/libcockatrice_card/libcockatrice/card/printing/printing_info.h index 75f634c3c..dcfcc846b 100644 --- a/libcockatrice_card/libcockatrice/card/printing/printing_info.h +++ b/libcockatrice_card/libcockatrice/card/printing/printing_info.h @@ -54,10 +54,7 @@ public: * @param other Another PrintingInfo to compare against. * @return True if both set and properties are equal, otherwise false. */ - bool operator==(const PrintingInfo &other) const - { - return this->set == other.set && this->getPropertiesHash() == other.getPropertiesHash(); - } + bool operator==(const PrintingInfo &other) const; /** * @brief check if the info is empty, as if default constructed. @@ -66,7 +63,7 @@ public: */ bool isEmpty() const { - return set == nullptr && getPropertiesHash().isEmpty(); + return set == nullptr && properties.isEmpty(); } private: @@ -91,9 +88,22 @@ public: */ [[nodiscard]] QStringList getProperties() const { - return getPropertiesHash().keys(); + return properties.keys(); } + /** + * @brief Returns the printing properties in their serialized blob form. + */ + [[nodiscard]] QByteArray getPropertiesBlob() const + { + return properties.serialized(); + } + + /** + * @brief Returns the materialized property hash, forcing deserialization of the blob. + * + * Prefer getProperty() or getPropertiesBlob(), which avoid building the hash. + */ [[nodiscard]] const QHash &getPropertiesHash() const { return properties.getProperties(); @@ -107,9 +117,7 @@ public: */ [[nodiscard]] QString getProperty(const QString &propertyName) const { - const auto &propertiesHash = getPropertiesHash(); - const auto it = propertiesHash.constFind(propertyName); - return it != propertiesHash.constEnd() ? it.value() : QString(); + return properties.value(propertyName); } /** diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 3382e36d6..477c54d21 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -486,7 +486,7 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson } } - PrintingInfo printingInfo(currentSet, LazyPropertiesHash(printingProps)); + PrintingInfo printingInfo(currentSet, LazyPropertiesHash::serializedFrom(printingProps)); QString numComponent; const QString numProperty = printingInfo.getProperty("num"); diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index 2b216d5b6..1b68b8166 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -62,6 +62,43 @@ TEST(CardDatabaseTest, LoadXml) ASSERT_EQ(NotLoaded, db->getLoadStatus()) << "Incorrect status after clear"; } +TEST(CardDatabaseTest, LazyPropertiesHashStreamsBlobAccessors) +{ + QHash props; + props.insert("manacost", "1R"); + props.insert("colors", "R"); + props.insert("format-standard", "legal"); + + LazyPropertiesHash lazy = LazyPropertiesHash::serializedFrom(props); + const QByteArray blob = lazy.serialized(); + + EXPECT_EQ(3, lazy.size()); + EXPECT_FALSE(lazy.isEmpty()); + EXPECT_TRUE(lazy.contains("colors")); + EXPECT_FALSE(lazy.contains("missing")); + EXPECT_EQ("1R", lazy.value("manacost")); + EXPECT_EQ(QString(), lazy.value("missing")); + EXPECT_EQ(blob, lazy.serialized()) << "an untouched blob must be passed through unchanged"; + + QStringList keys = lazy.keys(); + keys.sort(); + QStringList expectedKeys = props.keys(); + expectedKeys.sort(); + EXPECT_EQ(expectedKeys, keys); + + // Mutating materializes the hash, after which every accessor keeps working. + EXPECT_TRUE(lazy.insert("loyalty", "3")); + EXPECT_FALSE(lazy.insert("loyalty", "3")); + EXPECT_EQ(4, lazy.size()); + EXPECT_EQ("3", lazy.value("loyalty")); + EXPECT_TRUE(lazy.contains("loyalty")); + + LazyPropertiesHash roundTrip(lazy.serialized()); + EXPECT_EQ(4, roundTrip.size()); + EXPECT_EQ("3", roundTrip.value("loyalty")); + EXPECT_EQ("legal", roundTrip.value("format-standard")); +} + TEST(CardDatabaseTest, GuessCardWithProviderIdOnUnknownCardReturnsEmpty) { // An unknown name plus a providerId used to dereference the missing card @@ -104,6 +141,8 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) CardInfo::newInstance("Lightning Bolt", "Deal 3 damage.", false, {}, {}, {}, setsInfo, attributes); card->setLocalizedName("de", "Blitzschlag"); card->setLocalizedText("de", "Blitzschlag fügt 3 Schadenspunkte zu."); + card->setProperty("format-standard", "legal"); + card->setProperty("colors", "R"); SetNameMap sets; sets.insert("TST", set); @@ -127,6 +166,13 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) ASSERT_EQ(loaded->getLocalizedName("de"), "Blitzschlag"); ASSERT_EQ(loaded->getLocalizedText("de"), "Blitzschlag fügt 3 Schadenspunkte zu."); ASSERT_EQ(loaded->getLocalizedText("fr"), "Deal 3 damage."); + ASSERT_EQ(loaded->getProperty("format-standard"), "legal"); + ASSERT_TRUE(loaded->hasProperty("colors")); + ASSERT_EQ(loaded->getProperties().size(), 2); + + const QList printings = loaded->getSets().value("TST"); + ASSERT_FALSE(printings.isEmpty()); + ASSERT_EQ(printings.first().getProperty("manacost"), "1R"); } /** diff --git a/tests/carddatabase/load_benchmark.cpp b/tests/carddatabase/load_benchmark.cpp index 5492b9531..da077a50f 100644 --- a/tests/carddatabase/load_benchmark.cpp +++ b/tests/carddatabase/load_benchmark.cpp @@ -125,7 +125,7 @@ RunResult runLoad(CardDatabase *db, ChurnCounters &counters) result.churn.modelResets = counters.modelResets - before.modelResets; for (const CardInfoPtr &card : db->getCardList()) { - result.propTotal += card->getPropertiesHash().size(); + result.propTotal += card->getProperties().size(); if (!card->getProperty("colors").isEmpty()) { ++result.propsQueried; }