From 9980c90cea8a6fcee3842e074ae4b5662ff6fd20 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:20:10 +0200 Subject: [PATCH 1/2] [Filter] Allow spaces around the colon in search queries (#7529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Filter] Allow spaces around the colon in search queries * [Filter] Keep whitespace out of the color-identity operator Allowing `ws?` between `color` and the identity marker made `c i: g` parse as identity green, changing the meaning of text that previously read as a generic `c` plus a field query on `i`. Whitespace belongs around the separator, not inside an operator token: `ci : g` and `ci: g` stay valid, `c i: g` keeps its old parse. --------- Co-authored-by: Lukas Brübach --- .../libcockatrice/filters/filter_string.cpp | 16 ++++----- tests/carddatabase/filter_string_test.cpp | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/libcockatrice_filters/libcockatrice/filters/filter_string.cpp b/libcockatrice_filters/libcockatrice/filters/filter_string.cpp index bc137b0f6..0acc0eca0 100644 --- a/libcockatrice_filters/libcockatrice/filters/filter_string.cpp +++ b/libcockatrice_filters/libcockatrice/filters/filter_string.cpp @@ -43,31 +43,31 @@ SomewhatComplexQueryPart <- [(] QueryPartList [)] / QueryPart QueryPart <- NotQuery / SetQuery / RarityQuery / CMCQuery / FormatQuery / PowerQuery / ToughnessQuery / ColorQuery / TypeQuery / OracleQuery / FieldQuery / GenericQuery NotQuery <- ('NOT' ws/'-') SomewhatComplexQueryPart -SetQuery <- ('e'/'set') SetQueryValue -SetQueryValue <- ([=:] FlexStringValue) / (<[!][=]?> FlexStringValue) / SetExpression -OracleQuery <- 'o' [:] MatcherString +SetQuery <- ('e'/'set') ws? SetQueryValue +SetQueryValue <- ([=:] ws? FlexStringValue) / (<[!][=]?> ws? FlexStringValue) / SetExpression +OracleQuery <- 'o' ws? [:] ws? MatcherString CMCQuery <- ('cmc'/'mv') ws? NumericExpression PowerQuery <- [Pp] 'ow' 'er'? ws? NumericExpression ToughnessQuery <- [Tt] 'ou' 'ghness'? ws? NumericExpression -RarityQuery <- [rR] ':' Rarity +RarityQuery <- [rR] ws? ':' ws? Rarity Rarity <- [Cc] 'ommon'? / [Uu] 'ncommon'? / [Rr] 'are'? / [Mm] 'ythic'? / [Ss] 'pecial'? / [a-zA-Z] [a-z]* -FormatQuery <- 'f' ':' Format / Legality ':' Format +FormatQuery <- 'f' ws? ':' ws? Format / Legality ws? ':' ws? Format Format <- [a-zA-Z] [a-z]* Legality <- [Ll] 'egal'? / [Bb] 'anned'? / [Rr] 'estricted' -TypeQuery <- [tT] 'ype'? [:] StringValue +TypeQuery <- [tT] 'ype'? ws? [:] ws? StringValue Color <- < [Ww] 'hite'? / [Uu] / [Bb] 'lack'? / [Rr] 'ed'? / [Gg] 'reen'? / [Bb] 'lue'? > ColorEx <- Color / [mc] -ColorQuery <- [cC] 'olor'? <[iI]?> <[:!]> ColorEx* +ColorQuery <- [cC] 'olor'? <[iI]?> ws? <[:!]> ws? ColorEx* -FieldQuery <- String [:] MatcherString / String ws? NumericExpression +FieldQuery <- String ws? [:] ws? MatcherString / String ws? NumericExpression NonDoubleQuoteUnlessEscaped <- '\\\"'. / !["]. NonSingleQuoteUnlessEscaped <- "\\\'". / ![']. diff --git a/tests/carddatabase/filter_string_test.cpp b/tests/carddatabase/filter_string_test.cpp index 95225e28c..e561db590 100644 --- a/tests/carddatabase/filter_string_test.cpp +++ b/tests/carddatabase/filter_string_test.cpp @@ -99,6 +99,17 @@ QUERY(SetCodeWrongSet2, doctor, "set:cat", false) QUERY(BracketNextToUnquotedString, cat, "(o:woof OR o:meow)", true) +// Spaces around the colon are optional for all field queries (e.g. "o: flying") +QUERY(OracleSpaceAfterColon, cat, "o: meow", true) +QUERY(OracleSpaceBeforeColon, cat, "o :meow", true) +QUERY(OracleSpacesAroundColon, cat, "o : meow", true) +QUERY(OracleSpaceAfterColonNoMatch, cat, "o: woof", false) +QUERY(TypeSpaceAfterColon, cat, "t: creature", true) +QUERY(FieldSpaceAfterColon, cat, "pt: \"3/3\"", true) +QUERY(ColorSpaceAfterColon, cat, "c: g", true) +QUERY(ColorSpaceBeforeOperator, cat, "c !g", true) +QUERY(SetSpaceAfterColon, cat, "set: cat", true) + CardInfoPtr localizedCat() { CardInfoPtr localized = CardInfo::newInstance("Cat", "Meow!", false, {}, {}, {}, {}, {}); @@ -107,6 +118,22 @@ CardInfoPtr localizedCat() return localized; } +CardInfoPtr greenIdentityCard() +{ + return CardInfo::newInstance("Identity Cat", "", false, {{"colors", "G"}, {"coloridentity", "G"}}, {}, {}, {}, {}); +} + +// Whitespace belongs around the separator, never inside an operator token: "c i: g" must not +// be read as the color-identity operator "ci" (it is a generic "c" AND a field query on "i"). +TEST_F(CardQuery, ColorIdentityOperatorIsNotSplitByWhitespace) +{ + const CardData identityCard = greenIdentityCard(); + ASSERT_TRUE(FilterString("ci:g").check(identityCard)); + ASSERT_TRUE(FilterString("ci: g").check(identityCard)); + ASSERT_TRUE(FilterString("ci : g").check(identityCard)); + ASSERT_FALSE(FilterString("c i: g").check(identityCard)); +} + TEST_F(CardQuery, SearchLanguageEnglishMatchesOnlyEnglish) { const CardData localized = localizedCat(); @@ -136,6 +163,13 @@ TEST_F(CardQuery, SearchLanguageBothMatchesEitherLanguage) ASSERT_TRUE(FilterString("Kater", CardSearchLanguage{"de", SearchLanguageMode::Both}).check(localized)); } +TEST_F(CardQuery, SearchLanguageBothMatchesOracleTextWithSpaceAfterColon) +{ + const CardData localized = localizedCat(); + ASSERT_TRUE(FilterString("o: Meow", CardSearchLanguage{"de", SearchLanguageMode::Both}).check(localized)); + ASSERT_TRUE(FilterString("o: miaut", CardSearchLanguage{"de", SearchLanguageMode::Both}).check(localized)); +} + TEST_F(CardQuery, SearchLanguageIsBoundPerInstance) { const CardData localized = localizedCat(); From 0fe4fd0d8cd41283eb4a92073d224af62a861b6b Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:33:56 +0200 Subject: [PATCH 2/2] [CardDB] Guard guessCard and findPrintingWithId against missing cards (#7592) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [CardDB] Guard guessCard and findPrintingWithId against missing cards guessCard() dereferenced a null CardInfoPtr through findPrintingWithId() when the name was unknown and a providerId was given. Return an empty ExactCard instead, and make findPrintingWithId() null-safe for every caller. Add regression tests covering both paths. * [CardDB] Rely on PrintingInfo's default null set in findPrintingWithId The explicit nullptr was redundant: the constructor already defaults the set to null. --------- Co-authored-by: Lukas Brübach --- .../card/database/card_database_querier.cpp | 10 ++++++- tests/carddatabase/carddatabase_test.cpp | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp index 2fad5212d..7c41daa3b 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_querier.cpp @@ -133,6 +133,10 @@ ExactCard CardDatabaseQuerier::getCard(const CardRef &cardRef) const ExactCard CardDatabaseQuerier::guessCard(const CardRef &cardRef) const { auto card = lookupCardByName(cardRef.name); + if (!card) { + return {}; + } + auto printing = cardRef.providerId.isEmpty() ? getPreferredPrinting(card) : findPrintingWithId(card, cardRef.providerId); @@ -173,10 +177,14 @@ ExactCard CardDatabaseQuerier::getCardFromSameSet(const QString &cardName, const * * @param cardInfo The CardInfo to search * @param providerId The uuid to look for - * @return The PrintingInfo, or a default-constructed PrintingInfo if not found. + * @return The PrintingInfo, or a default-constructed PrintingInfo if not found or if cardInfo is null. */ PrintingInfo CardDatabaseQuerier::findPrintingWithId(const CardInfoPtr &cardInfo, const QString &providerId) const { + if (!cardInfo) { + return PrintingInfo(); + } + for (const auto &printings : cardInfo->getSets()) { for (const auto &printing : printings) { if (printing.getUuid() == providerId) { diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index 1b48d934b..2b216d5b6 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -1,6 +1,7 @@ #include "libcockatrice/card/database/card_database.h" #include "libcockatrice/card/database/card_database_loader.h" #include "libcockatrice/card/database/card_database_querier.h" +#include "libcockatrice/card/printing/exact_card.h" #include "libcockatrice/card/set/card_set.h" #include "libcockatrice/card/set/card_set_list.h" #include "test_card_database_path_provider.h" @@ -29,6 +30,7 @@ #include #include #include +#include #include namespace @@ -60,6 +62,31 @@ TEST(CardDatabaseTest, LoadXml) ASSERT_EQ(NotLoaded, db->getLoadStatus()) << "Incorrect status after clear"; } +TEST(CardDatabaseTest, GuessCardWithProviderIdOnUnknownCardReturnsEmpty) +{ + // An unknown name plus a providerId used to dereference the missing card + // inside findPrintingWithId(); the lookup must come back empty instead. + NoopCardPreferenceProvider prefs; + TestCardDatabasePathProvider pathProvider; + NoopCardSetPriorityController controller; + CardDatabase db(nullptr, &prefs, &pathProvider, &controller); + + const ExactCard guessed = db.query()->guessCard({"Not a Card", "card_Not a Card_fixture"}); + + EXPECT_TRUE(guessed.isEmpty()); + EXPECT_FALSE(guessed); +} + +TEST(CardDatabaseTest, FindPrintingWithIdOnNullCardInfoReturnsEmpty) +{ + NoopCardPreferenceProvider prefs; + TestCardDatabasePathProvider pathProvider; + NoopCardSetPriorityController controller; + CardDatabase db(nullptr, &prefs, &pathProvider, &controller); + + EXPECT_TRUE(db.query()->findPrintingWithId(nullptr, "card_Not a Card_fixture").isEmpty()); +} + TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) { NoopCardSetPriorityController controller;