diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.cpp b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.cpp index de502a4f9..46330ac46 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.cpp @@ -17,41 +17,6 @@ namespace { -/** - * @brief Information about the behaviour of a filter mode. - */ -struct FilterModeInfo -{ - // When a color symbol is enabled, add filters of this type. Skipped if equal to CardFilter::Type::TypeEnd. - CardFilter::Type includeType; - - // When a color symbol is disabled, add filters of this type. Skipped if equal to CardFilter::Type::TypeEnd. - CardFilter::Type excludeType; - - // The attribute to filter for when adding the filters. - CardFilter::Attr attr; -}; - -/** @brief Return information about the given filter mode. */ -static FilterModeInfo getFilterModeInfo(FilterMode filterMode) -{ - switch (filterMode) { - case FilterMode::Includes: - return {CardFilter::Type::TypeOr, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrColor}; - case FilterMode::Excludes: - return {CardFilter::Type::TypeAndNot, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrColor}; - case FilterMode::IncludeExclude: - return {CardFilter::Type::TypeOr, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColor}; - case FilterMode::ExactMatch: - return {CardFilter::Type::TypeAnd, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColor}; - case FilterMode::ColorIdentity: - return {CardFilter::Type::TypeEnd, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColorIdentity}; - case FilterMode::Custom: - case FilterMode::End: - break; - } - return {CardFilter::Type::TypeEnd, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrEnd}; -} /** @brief Remove the filters that apply to the given color and attribute from the given model. */ static void removeFilters(FilterTreeModel *filterModel, QChar color, CardFilter::Attr attr) @@ -247,54 +212,13 @@ QList VisualDatabaseDisplayColorFilterWidget::getActiveColors() void VisualDatabaseDisplayColorFilterWidget::syncWithFilterModel() { - // Determine the correct currentMode from the filter model by finding which modes - // are not possible due to the model containing (a combination of) filters that - // that mode cannot add. - constexpr int modeCount = static_cast(FilterMode::End); - bool modeImpossible[modeCount] = {}; - QList filters; filters.append(filterModel->getFiltersOfType(CardFilter::Attr::AttrColor)); filters.append(filterModel->getFiltersOfType(CardFilter::Attr::AttrColorIdentity)); - for (int i = 0; i < modeCount; ++i) { - FilterModeInfo modeInfo = getFilterModeInfo(static_cast(i)); - - // Remove the modes that cannot create one of the filters. - for (const CardFilter *filter : filters) { - if (filter->attr() != modeInfo.attr || - (filter->type() != modeInfo.includeType && filter->type() != modeInfo.excludeType)) { - modeImpossible[i] = true; - break; - } - } - - // Modes that have both an include and exclude filter should always have a filter for each color. - if (modeInfo.includeType != CardFilter::Type::TypeEnd && modeInfo.excludeType != CardFilter::Type::TypeEnd && - !modeImpossible[i]) { - for (const QChar &color : QString("WUBRG")) { - int count = 0; - for (const CardFilter *filter : filters) { - count += filter->term() == color; - } - if (count != 1) { - modeImpossible[i] = true; - break; - } - } - } - } - - // If the current mode is not possible with the current filters, set it to the first - // one that is. Switching to and from the custom mode is always possible. - if (modeImpossible[static_cast(currentMode)] || currentMode == FilterMode::Custom) { - for (int i = 0; i < modeCount; ++i) { - if (!modeImpossible[i] || i == static_cast(FilterMode::Custom)) { - currentMode = static_cast(i); - break; - } - } - } + // Re-derive the mode from the filters in the model, so that a loaded filter set selects the mode that + // produced it (or Custom when none could have). + currentMode = deriveFilterMode(filters, currentMode); // Update the combobox. Add a Custom item only if we have a custom filter mode. int customIndex = modeComboBox->findData(QVariant::fromValue(FilterMode::Custom)); diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.h b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.h index eb3c1ec3b..561c92d09 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.h +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.h @@ -10,8 +10,8 @@ #include #include #include -#include #include +#include #include #include @@ -38,20 +38,6 @@ private: int circleDiameter; }; -enum class FilterMode -{ - Includes = 0, // Include selected colors (OR condition). - Excludes, // Exclude selected colors (AND NOT). - IncludeExclude, // Include selected colors (OR) and exclude unselected colors (AND NOT). - ExactMatch, // Only selected colors are included, all others are excluded. - ColorIdentity, // Keep cards whose color identity is a subset of the selected colors (stored as AND NOT for each - // unselected color). - Custom, // The color filters do not match any of the above modes. Not included in the combo box. - End -}; - -Q_DECLARE_METATYPE(FilterMode) - class VisualDatabaseDisplayColorFilterWidget : public QWidget { Q_OBJECT diff --git a/libcockatrice_filters/CMakeLists.txt b/libcockatrice_filters/CMakeLists.txt index 070c04637..1b4303fc9 100644 --- a/libcockatrice_filters/CMakeLists.txt +++ b/libcockatrice_filters/CMakeLists.txt @@ -1,10 +1,10 @@ -set(HEADERS libcockatrice/filters/filter_card.h libcockatrice/filters/filter_string.h - libcockatrice/filters/filter_tree.h +set(HEADERS libcockatrice/filters/filter_card.h libcockatrice/filters/filter_mode.h + libcockatrice/filters/filter_string.h libcockatrice/filters/filter_tree.h ) add_library( - libcockatrice_filters STATIC ${HEADERS} libcockatrice/filters/filter_card.cpp libcockatrice/filters/filter_string.cpp - libcockatrice/filters/filter_tree.cpp + libcockatrice_filters STATIC ${HEADERS} libcockatrice/filters/filter_card.cpp libcockatrice/filters/filter_mode.cpp + libcockatrice/filters/filter_string.cpp libcockatrice/filters/filter_tree.cpp ) add_dependencies(libcockatrice_filters libcockatrice_card) diff --git a/libcockatrice_filters/libcockatrice/filters/filter_mode.cpp b/libcockatrice_filters/libcockatrice/filters/filter_mode.cpp new file mode 100644 index 000000000..356ccf608 --- /dev/null +++ b/libcockatrice_filters/libcockatrice/filters/filter_mode.cpp @@ -0,0 +1,77 @@ +/** + * @file filter_mode.cpp + * @ingroup Filters + */ + +#include "libcockatrice/filters/filter_mode.h" + +#include + +FilterModeInfo getFilterModeInfo(FilterMode filterMode) +{ + switch (filterMode) { + case FilterMode::Includes: + return {CardFilter::Type::TypeOr, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrColor}; + case FilterMode::Excludes: + return {CardFilter::Type::TypeAndNot, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrColor}; + case FilterMode::IncludeExclude: + return {CardFilter::Type::TypeOr, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColor}; + case FilterMode::ExactMatch: + return {CardFilter::Type::TypeAnd, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColor}; + case FilterMode::ColorIdentity: + return {CardFilter::Type::TypeEnd, CardFilter::Type::TypeAndNot, CardFilter::Attr::AttrColorIdentity}; + case FilterMode::Custom: + case FilterMode::End: + break; + } + return {CardFilter::Type::TypeEnd, CardFilter::Type::TypeEnd, CardFilter::Attr::AttrEnd}; +} + +FilterMode deriveFilterMode(const QList &filters, FilterMode currentMode) +{ + constexpr int modeCount = static_cast(FilterMode::End); + bool modeImpossible[modeCount] = {}; + const QString colors = "WUBRG"; + + for (int i = 0; i < modeCount; ++i) { + const FilterModeInfo modeInfo = getFilterModeInfo(static_cast(i)); + + // Remove the modes that cannot create one of the filters. + for (const CardFilter *filter : filters) { + if (filter->attr() != modeInfo.attr || + (filter->type() != modeInfo.includeType && filter->type() != modeInfo.excludeType)) { + modeImpossible[i] = true; + break; + } + } + + // Modes that have both an include and an exclude filter should always have a filter for each color. + if (modeInfo.includeType != CardFilter::Type::TypeEnd && modeInfo.excludeType != CardFilter::Type::TypeEnd && + !modeImpossible[i]) { + for (const QChar &color : colors) { + int count = 0; + for (const CardFilter *filter : filters) { + count += filter->term() == color; + } + if (count != 1) { + modeImpossible[i] = true; + break; + } + } + } + } + + // Keep the current mode if it can produce the filters, so that a live mode is never swapped for another. + // Switching to and from the custom mode is always possible. + if (!modeImpossible[static_cast(currentMode)] && currentMode != FilterMode::Custom) { + return currentMode; + } + + for (int i = 0; i < modeCount; ++i) { + if (!modeImpossible[i] || i == static_cast(FilterMode::Custom)) { + return static_cast(i); + } + } + + return FilterMode::Custom; +} diff --git a/libcockatrice_filters/libcockatrice/filters/filter_mode.h b/libcockatrice_filters/libcockatrice/filters/filter_mode.h new file mode 100644 index 000000000..f7662d662 --- /dev/null +++ b/libcockatrice_filters/libcockatrice/filters/filter_mode.h @@ -0,0 +1,62 @@ +/** + * @file filter_mode.h + * @ingroup Filters + */ + +#ifndef LIBCOCKATRICE_FILTERS_FILTER_MODE_H +#define LIBCOCKATRICE_FILTERS_FILTER_MODE_H + +#include "libcockatrice/filters/filter_card.h" + +#include +#include +#include + +/** + * @brief The way selected and unselected colors are combined into card filters. + */ +enum class FilterMode +{ + Includes = 0, // Include selected colors (OR condition). + Excludes, // Exclude selected colors (AND NOT). + IncludeExclude, // Include selected colors (OR) and exclude unselected colors (AND NOT). + ExactMatch, // Only selected colors are included, all others are excluded. + ColorIdentity, // Keep cards whose color identity is a subset of the selected colors (stored as AND NOT for each + // unselected color). + Custom, // The color filters do not match any of the above modes. Not included in the combo box. + End +}; + +Q_DECLARE_METATYPE(FilterMode) + +/** + * @brief The filters a single color contributes in a given mode. + */ +struct FilterModeInfo +{ + // When a color symbol is enabled, add filters of this type. Skipped if equal to CardFilter::Type::TypeEnd. + CardFilter::Type includeType; + + // When a color symbol is disabled, add filters of this type. Skipped if equal to CardFilter::Type::TypeEnd. + CardFilter::Type excludeType; + + // The attribute to filter for when adding the filters. + CardFilter::Attr attr; +}; + +/** @brief Return the filters the given mode contributes for one color. */ +FilterModeInfo getFilterModeInfo(FilterMode filterMode); + +/** + * @brief Determine the mode that produced the given color filters. + * + * Returns @p currentMode unchanged when that mode can still produce @p filters, so that a live mode is never + * replaced by an equally valid one. Otherwise returns the first mode that can produce them, falling back to + * FilterMode::Custom when none can. + * + * @param filters The filters on CardFilter::Attr::AttrColor and CardFilter::Attr::AttrColorIdentity. + * @param currentMode The mode currently selected in the UI. + */ +FilterMode deriveFilterMode(const QList &filters, FilterMode currentMode); + +#endif // LIBCOCKATRICE_FILTERS_FILTER_MODE_H diff --git a/tests/carddatabase/CMakeLists.txt b/tests/carddatabase/CMakeLists.txt index c8cfbe543..012081fd2 100644 --- a/tests/carddatabase/CMakeLists.txt +++ b/tests/carddatabase/CMakeLists.txt @@ -52,6 +52,20 @@ if(WITH_ORACLE OR WITH_CLIENT) 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 # ------------------------ diff --git a/tests/carddatabase/filter_mode_test.cpp b/tests/carddatabase/filter_mode_test.cpp new file mode 100644 index 000000000..3fec7e25d --- /dev/null +++ b/tests/carddatabase/filter_mode_test.cpp @@ -0,0 +1,147 @@ +#include "libcockatrice/filters/filter_mode.h" + +#include + +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 owned; + QList 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); +}