From 5e88a0f0ccfa5d45d3f4d620adee97ae62830769 Mon Sep 17 00:00:00 2001
From: Paul Carroll <57120760+carrollpaul@users.noreply.github.com>
Date: Sun, 24 Aug 2025 12:37:25 -0400
Subject: [PATCH] Fix multi-word type matching in card filters (#6060)

* Fix multi-word type matching in card filters

Add phrase matching to StringValue before word-based fallback.
Enables searches like t:"time lord" for multi-word creature types.

* Use existing typedef

* Don't inline lambda

* update filter func

* Update card type FilterString unit tests

* refactor string matcher

* update card db test

* fix sets count in test

* Add regex cache in string matcher

* Update cockatrice/src/game/filters/filter_string.cpp

* Revert "Add regex cache in string matcher"

---------

Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com>
---
 cockatrice/src/game/filters/filter_string.cpp | 14 ++++++++++++--
 tests/carddatabase/carddatabase_test.cpp      |  4 ++--
 tests/carddatabase/data/cards.xml             | 19 +++++++++++++++++--
 tests/carddatabase/filter_string_test.cpp     |  5 +++++
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/cockatrice/src/game/filters/filter_string.cpp b/cockatrice/src/game/filters/filter_string.cpp
index 4a7a58b9e..3d168d64c 100644
--- a/cockatrice/src/game/filters/filter_string.cpp
+++ b/cockatrice/src/game/filters/filter_string.cpp
@@ -187,15 +187,25 @@ static void setupParserRules()
 
         return QString::fromStdString(std::string(sv.sv())).toLower();
     };
+
     search["StringValue"] = [](const peg::SemanticValues &sv) -> StringMatcher {
+        // Helper function for word boundary matching
+        auto createWordBoundaryMatcher = [](const QString &target) {
+            QString pattern = QString("\\b%1\\b").arg(QRegularExpression::escape(target));
+            QRegularExpression regex(pattern, QRegularExpression::CaseInsensitiveOption);
+            return [regex](const QString &s) { return regex.match(s).hasMatch(); };
+        };
+
         if (sv.choice() == 0) {
             const auto target = std::any_cast<QString>(sv[0]);
-            return [=](const QString &s) { return s.split(" ").contains(target, Qt::CaseInsensitive); };
+            return createWordBoundaryMatcher(target);
         }
 
         const auto target = std::any_cast<QStringList>(sv[0]);
         return [=](const QString &s) {
-            auto containsString = [&s](const QString &str) { return s.split(" ").contains(str, Qt::CaseInsensitive); };
+            auto containsString = [&s, &createWordBoundaryMatcher](const QString &str) {
+                return createWordBoundaryMatcher(str)(s);
+            };
             return std::any_of(target.begin(), target.end(), containsString);
         };
     };
diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp
index 4818cd22e..c572dcf3b 100644
--- a/tests/carddatabase/carddatabase_test.cpp
+++ b/tests/carddatabase/carddatabase_test.cpp
@@ -18,8 +18,8 @@ TEST(CardDatabaseTest, LoadXml)
 
     // load dummy cards and test result
     db->loadCardDatabases();
-    ASSERT_EQ(8, db->getCardList().size()) << "Wrong card count after load";
-    ASSERT_EQ(4, db->getSetList().size()) << "Wrong sets count after load";
+    ASSERT_EQ(9, db->getCardList().size()) << "Wrong card count after load";
+    ASSERT_EQ(5, db->getSetList().size()) << "Wrong sets count after load";
     ASSERT_EQ(3, db->getAllMainCardTypes().size()) << "Wrong types count after load";
     ASSERT_EQ(Ok, db->getLoadStatus()) << "Wrong status after load";
 
diff --git a/tests/carddatabase/data/cards.xml b/tests/carddatabase/data/cards.xml
index f235ab4f9..2c1c09ed8 100644
--- a/tests/carddatabase/data/cards.xml
+++ b/tests/carddatabase/data/cards.xml
@@ -11,7 +11,7 @@
                 <colors>G</colors>
                 <manacost>2G</manacost>
                 <cmc>2</cmc>
-                <type>Creature</type>
+                <type>Creature — Cat</type>
                 <maintype>Creature</maintype>
                 <pt>3/3</pt>
             </prop>
@@ -26,7 +26,22 @@
                 <colors>R</colors>
                 <manacost>2RR</manacost>
                 <cmc>4</cmc>
-                <type>Creature</type>
+                <type>Creature — Dog</type>
+                <maintype>Creature</maintype>
+                <pt>4/4</pt>
+            </prop>
+        </card>
+        <card>
+            <name>Doctor</name>
+            <set>WHO</set>
+            <tablerow>0</tablerow>
+            <text>Why did wizards introduce two-word creature types</text>
+            <prop>
+                <muid>222</muid>
+                <colors>R</colors>
+                <manacost>2RR</manacost>
+                <cmc>4</cmc>
+                <type>Creature — Human Time Lord Doctor</type>
                 <maintype>Creature</maintype>
                 <pt>4/4</pt>
             </prop>
diff --git a/tests/carddatabase/filter_string_test.cpp b/tests/carddatabase/filter_string_test.cpp
index b29660159..534600e84 100644
--- a/tests/carddatabase/filter_string_test.cpp
+++ b/tests/carddatabase/filter_string_test.cpp
@@ -21,12 +21,14 @@ protected:
         cat = CardDatabaseManager::getInstance()->getCardBySimpleName("Cat");
         notDeadAfterAll = CardDatabaseManager::getInstance()->getCardBySimpleName("Not Dead");
         truth = CardDatabaseManager::getInstance()->getCardBySimpleName("Truth");
+        doctor = CardDatabaseManager::getInstance()->getCardBySimpleName("Doctor");
     }
     // void TearDown() override {}
 
     CardData cat;
     CardData notDeadAfterAll;
     CardData truth;
+    CardData doctor;
 };
 
 QUERY(Empty, cat, "", true)
@@ -34,6 +36,9 @@ QUERY(Typing, cat, "t", true)
 
 QUERY(NonMatchingType, cat, "t:kithkin", false)
 QUERY(MatchingType, cat, "t:creature", true)
+QUERY(MatchingCreatureType, cat, "t:cat", true)
+QUERY(PartialMatchingType, cat, "t:ca", false)
+QUERY(MatchingMultiWordType, doctor, "t:\"Time Lord\"", true)
 QUERY(Not1, cat, "NOT t:kithkin", true)
 QUERY(Not2, cat, "NOT t:creature", false)
 QUERY(NonKeyword1, cat, "not t:kithkin", false)