diff --git a/libcockatrice_filters/libcockatrice/filters/filter_string.cpp b/libcockatrice_filters/libcockatrice/filters/filter_string.cpp index bb9f00cc2..aff922f3f 100644 --- a/libcockatrice_filters/libcockatrice/filters/filter_string.cpp +++ b/libcockatrice_filters/libcockatrice/filters/filter_string.cpp @@ -74,13 +74,18 @@ NumericValue <- [0-9]+ static std::once_flag init; -// The peg parser rules are set up once per process, which means the GenericQuery/ -// OracleQuery lambdas cannot capture per-instance state. The card language the -// plain-text name and text queries search in is therefore kept here and applied -// by those lambdas; every FilterString shares it because it reflects a single -// global user setting. -static QString globalSearchLanguage; -static CardSearchLanguage globalSearchLanguageMode = CardSearchLanguage::English; +// The peglib parser rules (and therefore their rule actions) are set up once per +// process, so a rule action cannot capture per-instance state. The card language +// plain-text name and text queries search in is therefore handed to the GenericQuery +// and OracleQuery rule actions through this thread-local context, which is live only +// while a FilterString is being parsed. The rule actions copy it into the filter +// closures they produce, so card evaluation never reads process-global state. +struct SearchLanguageContext +{ + QString searchLanguage; + CardSearchLanguage searchLanguageMode = CardSearchLanguage::English; +}; +thread_local SearchLanguageContext searchLanguageContext; namespace { @@ -365,9 +370,11 @@ static void setupParserRules() search["OracleQuery"] = [](const peg::SemanticValues &sv) -> Filter { const auto matcher = std::any_cast(sv[0]); + const QString searchLanguage = searchLanguageContext.searchLanguage; + const CardSearchLanguage searchLanguageMode = searchLanguageContext.searchLanguageMode; return [=](const CardData &x) { - return matchesInSearchLanguage(x->getText(), x->getLocalizedText(globalSearchLanguage), - globalSearchLanguage, globalSearchLanguageMode, matcher); + return matchesInSearchLanguage(x->getText(), x->getLocalizedText(searchLanguage), searchLanguage, + searchLanguageMode, matcher); }; }; @@ -445,9 +452,11 @@ static void setupParserRules() }; search["GenericQuery"] = [](const peg::SemanticValues &sv) -> Filter { const auto matcher = std::any_cast(sv[0]); + const QString searchLanguage = searchLanguageContext.searchLanguage; + const CardSearchLanguage searchLanguageMode = searchLanguageContext.searchLanguageMode; return [=](const CardData &x) { - return matchesInSearchLanguage(x->getName(), x->getLocalizedName(globalSearchLanguage), - globalSearchLanguage, globalSearchLanguageMode, matcher); + return matchesInSearchLanguage(x->getName(), x->getLocalizedName(searchLanguage), searchLanguage, + searchLanguageMode, matcher); }; }; @@ -463,7 +472,7 @@ FilterString::FilterString() _error = "Not initialized"; } -FilterString::FilterString(const QString &expr) +FilterString::FilterString(const QString &expr, const QString &searchLanguage, CardSearchLanguage searchLanguageMode) { QByteArray ba = expr.simplified().toUtf8(); @@ -476,6 +485,8 @@ FilterString::FilterString(const QString &expr) return; } + searchLanguageContext = SearchLanguageContext{searchLanguage, searchLanguageMode}; + search.set_logger([&](size_t /*ln*/, size_t col, const std::string &msg) { _error = QString("Error at position %1: %2").arg(col).arg(QString::fromStdString(msg)); }); @@ -485,9 +496,3 @@ FilterString::FilterString(const QString &expr) result = [](const CardData &) -> bool { return false; }; } } - -void FilterString::setSearchLanguage(const QString &searchLanguage, CardSearchLanguage searchLanguageMode) -{ - globalSearchLanguage = searchLanguage; - globalSearchLanguageMode = searchLanguageMode; -} diff --git a/libcockatrice_filters/libcockatrice/filters/filter_string.h b/libcockatrice_filters/libcockatrice/filters/filter_string.h index 7bcab9c2e..e0ed5650c 100644 --- a/libcockatrice_filters/libcockatrice/filters/filter_string.h +++ b/libcockatrice_filters/libcockatrice/filters/filter_string.h @@ -36,7 +36,9 @@ class FilterString { public: FilterString(); - explicit FilterString(const QString &exp); + explicit FilterString(const QString &exp, + const QString &searchLanguage = QString(), + CardSearchLanguage searchLanguageMode = CardSearchLanguage::English); [[nodiscard]] bool check(const CardData &card) const { if (card.isNull()) { @@ -56,17 +58,6 @@ public: return _error; } - /** - * @brief Sets the card language plain-text name and text queries run against. - * - * The peg parser rules are set up once per process, so this propagates to - * every FilterString instance created through the shared parser. - * - * @param searchLanguage Empty string for English only, otherwise the card language code. - * @param searchLanguageMode The search language mode from CardSearchLanguage. - */ - void setSearchLanguage(const QString &searchLanguage, CardSearchLanguage searchLanguageMode); - private: QString _error; Filter result; diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp index acef20440..89e2fbfdd 100644 --- a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp +++ b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp @@ -239,9 +239,9 @@ void CardDatabaseDisplayModel::setFilterTree(FilterTree *_filterTree) void CardDatabaseDisplayModel::setStringFilter(const QString &_src) { + searchText = _src; delete filterString; - filterString = new FilterString(_src); - filterString->setSearchLanguage(searchLanguage, searchLanguageMode); + filterString = new FilterString(_src, searchLanguage, searchLanguageMode); dirty(); } @@ -255,7 +255,7 @@ void CardDatabaseDisplayModel::setSearchLanguage(const QString &searchLang, Card searchLanguageMode = mode; if (filterString != nullptr) { - filterString->setSearchLanguage(searchLanguage, searchLanguageMode); + setStringFilter(searchText); } dirty(); } diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h index 5eba2ae0a..2a09a54d0 100644 --- a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h +++ b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h @@ -35,6 +35,7 @@ private: QTimer dirtyTimer; QString searchLanguage; CardSearchLanguage searchLanguageMode = CardSearchLanguage::English; + QString searchText; /** The translation table that will be used for sanitizeCardName. */ static QMap characterTranslation; diff --git a/tests/carddatabase/filter_string_test.cpp b/tests/carddatabase/filter_string_test.cpp index c6d68be1f..85ef7c1c2 100644 --- a/tests/carddatabase/filter_string_test.cpp +++ b/tests/carddatabase/filter_string_test.cpp @@ -73,6 +73,57 @@ QUERY(Color4, cat, "c!gw", false) QUERY(BracketNextToUnquotedString, cat, "(o:woof OR o:meow)", true) +CardInfoPtr localizedCat() +{ + CardInfoPtr localized = CardInfo::newInstance("Cat", "Meow!", false, {}, {}, {}, {}, {}); + localized->setLocalizedName("de", "Kater"); + localized->setLocalizedText("de", "miaut"); + return localized; +} + +TEST_F(CardQuery, SearchLanguageEnglishMatchesOnlyEnglish) +{ + const CardData localized = localizedCat(); + ASSERT_TRUE(FilterString("Cat", "de", CardSearchLanguage::English).check(localized)); + ASSERT_FALSE(FilterString("Kater", "de", CardSearchLanguage::English).check(localized)); +} + +TEST_F(CardQuery, SearchLanguageSelectedMatchesLocalizedNameAndText) +{ + const CardData localized = localizedCat(); + ASSERT_TRUE(FilterString("Kater", "de", CardSearchLanguage::Selected).check(localized)); + ASSERT_TRUE(FilterString("o:miaut", "de", CardSearchLanguage::Selected).check(localized)); + ASSERT_FALSE(FilterString("Cat", "de", CardSearchLanguage::Selected).check(localized)); +} + +TEST_F(CardQuery, SearchLanguageSelectedFallsBackToEnglishForUntranslatedCards) +{ + const CardData localized = localizedCat(); + ASSERT_TRUE(FilterString("Cat", "fr", CardSearchLanguage::Selected).check(localized)); + ASSERT_FALSE(FilterString("Kater", "fr", CardSearchLanguage::Selected).check(localized)); +} + +TEST_F(CardQuery, SearchLanguageBothMatchesEitherLanguage) +{ + const CardData localized = localizedCat(); + ASSERT_TRUE(FilterString("Cat", "de", CardSearchLanguage::Both).check(localized)); + ASSERT_TRUE(FilterString("Kater", "de", CardSearchLanguage::Both).check(localized)); +} + +TEST_F(CardQuery, SearchLanguageIsBoundPerInstance) +{ + const CardData localized = localizedCat(); + + FilterString germanQuery("Kater", "de", CardSearchLanguage::Selected); + ASSERT_TRUE(germanQuery.check(localized)); + + // Constructing an English-bound instance afterwards must not change the + // language the earlier instance searches in. + FilterString englishQuery("Kater", "", CardSearchLanguage::English); + ASSERT_FALSE(englishQuery.check(localized)); + ASSERT_TRUE(germanQuery.check(localized)); +} + } // namespace int main(int argc, char **argv)