[Client] Added an exclude and color identity filter for the visual database display (#7587)

* [Client] Add exclude and color identity filters to the visual database display

* [Filters] Extract the color filter mode derivation into libcockatrice_filters

The mode table and the 'which mode produced this filter set' derivation are
filter semantics, not widget code, so they belong next to CardFilter rather
than in an anonymous namespace in the client. Moving them also makes them
testable: filter_mode_test pins every mode, the Custom fallback and the
'keep the live mode when it still fits' rule.

* [Tests] Cover the negated color identity query

ci! read the camelCase colorIdentity property instead of the lowercase
coloridentity key, so it matched the empty string and rejected every card.
The new case uses a card whose colors and color identity differ, so it fails
on the old lookup and passes on the fixed one.

---------

Co-authored-by: Lukas Brübach <44814898+BruebachL@users.noreply.github.com>

tests/carddatabase/CMakeLists.txt

@@ -21,10 +21,12 @@ cockatrice_add_test(
 )
 
 # ------------------------
-# Filter String Test
-# (guard must match the condition for libcockatrice_filters in the root CMakeLists.txt)
+# Guard to match condition for libcockatrice_filters in the root CMakeLists.txt
 # ------------------------
 if(WITH_ORACLE OR WITH_CLIENT)
+  # ------------------------
+  # Filter String Test
+  # ------------------------
   cockatrice_add_test(
     NAME
     filter_string_test
@@ -36,6 +38,34 @@ if(WITH_ORACLE OR WITH_CLIENT)
     libcockatrice_filters
   )
 
+  # ------------------------
+  # Filter Tree Test
+  # ------------------------
+  cockatrice_add_test(
+    NAME
+    filter_tree_test
+    SOURCES
+    ${VERSION_STRING_CPP}
+    filter_tree_test.cpp
+    mocks.cpp
+    LIBS
+    libcockatrice_filters
+  )
+
+  # ------------------------
+  # Filter Mode Test
+  # ------------------------
+  cockatrice_add_test(
+    NAME
+    filter_mode_test
+    SOURCES
+    ${VERSION_STRING_CPP}
+    filter_mode_test.cpp
+    mocks.cpp
+    LIBS
+    libcockatrice_filters
+  )
+
   # ------------------------
   # Visual Display Model Test
   # ------------------------

tests/carddatabase/filter_mode_test.cpp

@@ -0,0 +1,147 @@
+#include "libcockatrice/filters/filter_mode.h"
+
+#include <gtest/gtest.h>
+
+class FilterModeTest : public ::testing::Test
+{
+protected:
+    ~FilterModeTest() override
+    {
+        for (CardFilter *filter : owned) {
+            delete filter;
+        }
+    }
+
+    /** @brief Create a filter owned by the fixture and append it to the set under test. */
+    const CardFilter *
+    add(const QString &term, CardFilter::Type type, CardFilter::Attr attr = CardFilter::Attr::AttrColor)
+    {
+        owned.append(new CardFilter(term, type, attr));
+        filters.append(owned.last());
+        return owned.last();
+    }
+
+    QList<CardFilter *> owned;
+    QList<const CardFilter *> filters;
+};
+
+TEST(FilterModeInfoTest, EachModeDescribesItsOwnFilters)
+{
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Includes).includeType, CardFilter::Type::TypeOr);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Includes).excludeType, CardFilter::Type::TypeEnd);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Includes).attr, CardFilter::Attr::AttrColor);
+
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Excludes).includeType, CardFilter::Type::TypeAndNot);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Excludes).excludeType, CardFilter::Type::TypeEnd);
+
+    EXPECT_EQ(getFilterModeInfo(FilterMode::IncludeExclude).includeType, CardFilter::Type::TypeOr);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::IncludeExclude).excludeType, CardFilter::Type::TypeAndNot);
+
+    EXPECT_EQ(getFilterModeInfo(FilterMode::ExactMatch).includeType, CardFilter::Type::TypeAnd);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::ExactMatch).excludeType, CardFilter::Type::TypeAndNot);
+
+    // Color identity only excludes: the selected colors must be the identity, so nothing is included positively.
+    EXPECT_EQ(getFilterModeInfo(FilterMode::ColorIdentity).includeType, CardFilter::Type::TypeEnd);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::ColorIdentity).excludeType, CardFilter::Type::TypeAndNot);
+    EXPECT_EQ(getFilterModeInfo(FilterMode::ColorIdentity).attr, CardFilter::Attr::AttrColorIdentity);
+
+    EXPECT_EQ(getFilterModeInfo(FilterMode::Custom).attr, CardFilter::Attr::AttrEnd);
+}
+
+TEST_F(FilterModeTest, EmptyFilterSetKeepsACompatibleMode)
+{
+    // Includes and Color identity both allow an empty set, so the live mode is not swapped for another.
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Includes), FilterMode::Includes);
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::ColorIdentity), FilterMode::ColorIdentity);
+}
+
+// Include / Exclude and Exact match always hold one filter per color, so an empty set cannot have come from
+// them and falls back to the first mode that can describe it.
+TEST_F(FilterModeTest, EmptyFilterSetFallsBackWhenTheModeNeedsOneFilterPerColor)
+{
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::IncludeExclude), FilterMode::Includes);
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::ExactMatch), FilterMode::Includes);
+}
+
+TEST_F(FilterModeTest, IncludesFromOrFilters)
+{
+    add("W", CardFilter::Type::TypeOr);
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Includes), FilterMode::Includes);
+    // A single OR filter cannot come from Excludes, so the first mode that can produce it is selected.
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Excludes), FilterMode::Includes);
+}
+
+TEST_F(FilterModeTest, ExcludesFromAndNotFilters)
+{
+    add("W", CardFilter::Type::TypeAndNot);
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Excludes), FilterMode::Excludes);
+    // Includes needs an OR or an end filter, so an AND NOT filter selects Excludes.
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Includes), FilterMode::Excludes);
+}
+
+TEST_F(FilterModeTest, IncludeExcludeNeedsOneFilterPerColor)
+{
+    add("W", CardFilter::Type::TypeOr);
+    for (const QChar color : QString("UBRG")) {
+        add(QString(color), CardFilter::Type::TypeAndNot);
+    }
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::IncludeExclude), FilterMode::IncludeExclude);
+}
+
+TEST_F(FilterModeTest, IncludeExcludeRejectsAnIncompleteColorSet)
+{
+    add("W", CardFilter::Type::TypeOr);
+    add("U", CardFilter::Type::TypeAndNot);
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::IncludeExclude), FilterMode::Custom);
+}
+
+TEST_F(FilterModeTest, ExactMatchFromAndAndAndNotFilters)
+{
+    add("W", CardFilter::Type::TypeAnd);
+    for (const QChar color : QString("UBRG")) {
+        add(QString(color), CardFilter::Type::TypeAndNot);
+    }
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::ExactMatch), FilterMode::ExactMatch);
+}
+
+TEST_F(FilterModeTest, ColorIdentityFromIdentityAndNotFilters)
+{
+    for (const QChar color : QString("BRG")) {
+        add(QString(color), CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColorIdentity);
+    }
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::ColorIdentity), FilterMode::ColorIdentity);
+}
+
+TEST_F(FilterModeTest, MixedAttributesAreCustom)
+{
+    add("W", CardFilter::Type::TypeOr, CardFilter::Attr::AttrColor);
+    add("U", CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColorIdentity);
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Includes), FilterMode::Custom);
+}
+
+TEST_F(FilterModeTest, CustomLeavesAsSoonAsAModeCanExplainTheFilters)
+{
+    add("W", CardFilter::Type::TypeOr);
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Custom), FilterMode::Includes);
+}
+
+// Exact match with no color selected stores one AND NOT filter per color. That is a legitimate Exact match
+// state, so the live mode is kept; a mode that cannot produce the set re-derives to Excludes, which produces
+// the identical filters.
+TEST_F(FilterModeTest, FullyExcludedColorSetIsExactMatchOrExcludes)
+{
+    for (const QChar color : QString("WUBRG")) {
+        add(QString(color), CardFilter::Type::TypeAndNot);
+    }
+
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::ExactMatch), FilterMode::ExactMatch);
+    EXPECT_EQ(deriveFilterMode(filters, FilterMode::Includes), FilterMode::Excludes);
+}

tests/carddatabase/filter_string_test.cpp

@@ -123,6 +123,25 @@ CardInfoPtr greenIdentityCard()
     return CardInfo::newInstance("Identity Cat", "", false, {{"colors", "G"}, {"coloridentity", "G"}}, {}, {}, {}, {});
 }
 
+// A card whose color identity differs from its colors, so that a query reading the wrong one is visible.
+CardInfoPtr splitIdentityCard()
+{
+    return CardInfo::newInstance("Split Identity", "", false, {{"colors", "G"}, {"coloridentity", "W"}}, {}, {}, {},
+                                 {});
+}
+
+// "ci!" (exactly these color identities) used to read the camelCase "colorIdentity" property instead of the
+// lowercase "coloridentity" key, so it matched the empty string and rejected every card.
+TEST_F(CardQuery, ColorIdentityExactReadsTheColorIdentityProperty)
+{
+    const CardData card = splitIdentityCard();
+    ASSERT_TRUE(FilterString("ci!w").check(card));
+    ASSERT_FALSE(FilterString("ci!g").check(card));
+    // "c!" still reads colors, not identity.
+    ASSERT_TRUE(FilterString("c!g").check(card));
+    ASSERT_FALSE(FilterString("c!w").check(card));
+}
+
 // 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)

tests/carddatabase/filter_tree_test.cpp

@@ -0,0 +1,204 @@
+
+#include "libcockatrice/filters/filter_tree.h"
+
+#include <gtest/gtest.h>
+#include <libcockatrice/card/card_localization.h>
+#include <libcockatrice/filters/filter_string.h>
+
+namespace
+{
+class FilterTreeTest : public ::testing::Test
+{
+protected:
+    void SetUp() override
+    {
+        language = CardSearchLanguage{};
+
+        island = CardInfo::newInstance("Island", "", false, {{"colors", ""}, {"coloridentity", "U"}}, {}, {}, {}, {});
+        artifact =
+            CardInfo::newInstance("Artifact", "", false, {{"colors", ""}, {"coloridentity", ""}}, {}, {}, {}, {});
+        truth = CardInfo::newInstance("Truth", "", false, {{"colors", "U"}, {"coloridentity", "U"}}, {}, {}, {}, {});
+        notDead =
+            CardInfo::newInstance("Not Dead", "", false, {{"colors", "B"}, {"coloridentity", "B"}}, {}, {}, {}, {});
+        wallOfDenial = CardInfo::newInstance("Wall of Denial", "", false, {{"colors", "WU"}, {"coloridentity", "WU"}},
+                                             {}, {}, {}, {});
+        maelstromAngel = CardInfo::newInstance("Maelstrom Archangel", "", false,
+                                               {{"colors", "WUBRG"}, {"coloridentity", "WUBRG"}}, {}, {}, {}, {});
+    }
+
+    CardSearchLanguage language;
+
+    CardData island;         // c/u
+    CardData artifact;       // c
+    CardData truth;          // u
+    CardData notDead;        // b
+    CardData wallOfDenial;   // wu
+    CardData maelstromAngel; // wubrg
+};
+} // namespace
+
+// =====================================================================================================================
+// acceptColor
+// =====================================================================================================================
+
+TEST_F(FilterTreeTest, Color)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "wu");
+    ASSERT_FALSE(filterTree.acceptsCard(island, language));
+    ASSERT_FALSE(filterTree.acceptsCard(artifact, language));
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_TRUE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorColorless)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "c");
+    ASSERT_TRUE(filterTree.acceptsCard(island, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_FALSE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_FALSE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorNot)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAndNot, "b");
+    ASSERT_TRUE(filterTree.acceptsCard(island, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    ASSERT_TRUE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_FALSE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorAliases)
+{
+    FilterTree filterTree;
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "green");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(maelstromAngel, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "grn");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(maelstromAngel, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "blue");
+    ASSERT_TRUE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "blu");
+    ASSERT_TRUE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "black");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(notDead, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "blk");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(notDead, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "red");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(maelstromAngel, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "white");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "wht");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "colorless");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "colourless");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    filterTree.clear();
+
+    filterTree.termNode(CardFilter::Attr::AttrColor, CardFilter::Type::TypeAnd, "none");
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    filterTree.clear();
+}
+
+// =====================================================================================================================
+// acceptColorIdentity
+// =====================================================================================================================
+
+TEST_F(FilterTreeTest, ColorIdentity)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAnd, "wu");
+    ASSERT_FALSE(filterTree.acceptsCard(island, language));
+    ASSERT_FALSE(filterTree.acceptsCard(artifact, language));
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_TRUE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorIdentityColorless)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAnd, "c");
+    ASSERT_FALSE(filterTree.acceptsCard(island, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    ASSERT_FALSE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_FALSE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_FALSE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorIdentityNot)
+{
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAndNot, "b");
+    ASSERT_TRUE(filterTree.acceptsCard(island, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    ASSERT_TRUE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_FALSE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+TEST_F(FilterTreeTest, ColorIdentityMultiple)
+{
+    // Simulate a WU commander.
+    FilterTree filterTree;
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAndNot, "b");
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAndNot, "r");
+    filterTree.termNode(CardFilter::Attr::AttrColorIdentity, CardFilter::Type::TypeAndNot, "g");
+    ASSERT_TRUE(filterTree.acceptsCard(island, language));
+    ASSERT_TRUE(filterTree.acceptsCard(artifact, language));
+    ASSERT_TRUE(filterTree.acceptsCard(truth, language));
+    ASSERT_FALSE(filterTree.acceptsCard(notDead, language));
+    ASSERT_TRUE(filterTree.acceptsCard(wallOfDenial, language));
+    ASSERT_FALSE(filterTree.acceptsCard(maelstromAngel, language));
+}
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}