Don't select messages' strings during condition eval

This simplifies the API, clarifies the semantics of condition
evaluation, and allows API users to query plugins for messages in
several languages after condition evaluation (so they know the
message is applicable).

include/loot/database_interface.h

@@ -66,10 +66,8 @@ public:
    *  @brief Evaluates all conditions and regular expression metadata entries.
    *  @details Repeated calls re-evaluate the metadata from scratch. This
    *           function affects the output of all the database access functions.
-   *  @param language
-   *         The language code that is used for message language comparisons.
    */
-  virtual void EvalLists(const LanguageCode language) = 0;
+  virtual void EvalLists() = 0;
 
   /**
    *  @}
@@ -172,9 +170,7 @@ public:
    *         The filename of the plugin to look up messages for.
    *  @param language
    *         The language to use when choosing which message content strings
-   *         to return. This has no effect if `EvalLists` has been called,
-   *         as it selects content strings, discarding non-selected strings,
-   *         during its operation.
+   *         to return.
    *  @returns A vector of messages associated with the specified plugin. Empty
    *           if the plugin has no messages associated with it.
    */

src/api/api_database.cpp

@@ -67,7 +67,7 @@ void ApiDatabase::LoadLists(const std::string& masterlistPath,
   unevaluatedUserlist_ = userTemp;
 }
 
-void ApiDatabase::EvalLists(const LanguageCode language) {
+void ApiDatabase::EvalLists() {
   // Clear caches before evaluating conditions.
   game_.ClearCachedConditions();
 
@@ -75,8 +75,8 @@ void ApiDatabase::EvalLists(const LanguageCode language) {
   MetadataList userTemp = unevaluatedUserlist_;
 
   // Refresh active plugins before evaluating conditions.
-  temp.EvalAllConditions(game_, Language(LanguageCode(language)).GetCode());
-  userTemp.EvalAllConditions(game_, Language(LanguageCode(language)).GetCode());
+  temp.EvalAllConditions(game_);
+  userTemp.EvalAllConditions(game_);
 
   game_.GetMasterlist() = temp;
   game_.GetUserlist() = userTemp;

src/api/api_database.h

@@ -41,7 +41,7 @@ struct ApiDatabase : public DatabaseInterface {
   void LoadLists(const std::string& masterlist_path,
                  const std::string& userlist_path = "");
 
-  void EvalLists(const LanguageCode language);
+  void EvalLists();
 
   std::vector<std::string> SortPlugins(const std::vector<std::string>& plugins);
 

src/backend/metadata/message.cpp

@@ -64,13 +64,6 @@ bool Message::operator == (const Message& rhs) const {
   return (content_ == rhs.GetContent());
 }
 
-bool Message::EvalCondition(loot::Game& game, const LanguageCode language) {
-  BOOST_LOG_TRIVIAL(trace) << "Choosing message content for language: " << Language(language).GetName();
-  content_.assign({GetContent(language)});
-
-  return ConditionalMetadata::EvalCondition(game);
-}
-
 MessageType Message::GetType() const {
   return type_;
 }

src/backend/metadata/message.h

@@ -37,8 +37,6 @@
 #include "backend/metadata/message_content.h"
 
 namespace loot {
-class Game;
-
 class Message : public ConditionalMetadata {
 public:
   Message();
@@ -50,8 +48,6 @@ public:
   bool operator < (const Message& rhs) const;
   bool operator == (const Message& rhs) const;
 
-  bool EvalCondition(Game& game, const LanguageCode language);
-
   MessageType GetType() const;
   std::vector<MessageContent> GetContent() const;
   MessageContent GetContent(const LanguageCode language) const;

src/backend/metadata/plugin_metadata.cpp

@@ -345,7 +345,7 @@ void PluginMetadata::Locations(const std::set<Location>& locations) {
   locations_ = locations;
 }
 
-PluginMetadata& PluginMetadata::EvalAllConditions(Game& game, const LanguageCode language) {
+PluginMetadata& PluginMetadata::EvalAllConditions(Game& game) {
   for (auto it = loadAfter_.begin(); it != loadAfter_.end();) {
     if (!it->EvalCondition(game))
       loadAfter_.erase(it++);
@@ -368,7 +368,7 @@ PluginMetadata& PluginMetadata::EvalAllConditions(Game& game, const LanguageCode
   }
 
   for (auto it = messages_.begin(); it != messages_.end();) {
-    if (!it->EvalCondition(game, language))
+    if (!it->EvalCondition(game))
       it = messages_.erase(it);
     else
       ++it;

src/backend/metadata/plugin_metadata.h

@@ -89,7 +89,7 @@ public:
   void CleanInfo(const std::set<PluginCleaningData>& info);
   void Locations(const std::set<Location>& locations);
 
-  PluginMetadata& EvalAllConditions(Game& game, const LanguageCode language);
+  PluginMetadata& EvalAllConditions(Game& game);
   bool HasNameOnly() const;
   bool IsRegexPlugin() const;
 

src/backend/metadata_list.cpp

@@ -148,19 +148,19 @@ void MetadataList::AppendMessage(const Message& message) {
   messages_.push_back(message);
 }
 
-void MetadataList::EvalAllConditions(Game& game, const LanguageCode language) {
+void MetadataList::EvalAllConditions(Game& game) {
   std::unordered_set<PluginMetadata> replacementSet;
   for (auto &plugin : plugins_) {
     PluginMetadata p(plugin);
-    p.EvalAllConditions(game, language);
+    p.EvalAllConditions(game);
     replacementSet.insert(p);
   }
   plugins_ = replacementSet;
   for (auto &plugin : regexPlugins_) {
-    plugin.EvalAllConditions(game, language);
+    plugin.EvalAllConditions(game);
   }
   for (auto &message : messages_) {
-    message.EvalCondition(game, language);
+    message.EvalCondition(game);
   }
 }
 }

src/backend/metadata_list.h

@@ -57,7 +57,7 @@ public:
   void AppendMessage(const Message& message);
 
   // Eval plugin conditions.
-  void EvalAllConditions(Game& game, const LanguageCode language);
+  void EvalAllConditions(Game& game);
 
 protected:
   std::set<std::string> bashTags_;

src/backend/plugin/plugin_sorter.cpp

@@ -210,7 +210,7 @@ void PluginSorter::AddPluginVertices(Game& game, const LanguageCode language) {
     //Now that items are merged, evaluate any conditions they have.
     BOOST_LOG_TRIVIAL(trace) << "Evaluate conditions for merged plugin data.";
     try {
-      graph_[v].EvalAllConditions(game, language);
+      graph_[v].EvalAllConditions(game);
     } catch (std::exception& e) {
       BOOST_LOG_TRIVIAL(error) << "\"" << graph_[v].Name() << "\" contains a condition that could not be evaluated. Details: " << e.what();
       list<Message> messages(graph_[v].Messages());

src/gui/query_handler.cpp

@@ -929,7 +929,7 @@ std::vector<Message> QueryHandler::GetGeneralMessages() const {
     BOOST_LOG_TRIVIAL(info) << "Using message language: " << lootState_.getLanguage().GetName();
     auto it = begin(messages);
     while (it != end(messages)) {
-      if (!it->EvalCondition(lootState_.getCurrentGame(), lootState_.getLanguage().GetCode()))
+      if (!it->EvalCondition(lootState_.getCurrentGame()))
         it = messages.erase(it);
       else
         ++it;
@@ -954,7 +954,7 @@ YAML::Node QueryHandler::GenerateDerivedMetadata(const Plugin& file, const Plugi
   //Evaluate any conditions
   BOOST_LOG_TRIVIAL(trace) << "Evaluate conditions for merged plugin data.";
   try {
-    tempPlugin.EvalAllConditions(lootState_.getCurrentGame(), lootState_.getLanguage().GetCode());
+    tempPlugin.EvalAllConditions(lootState_.getCurrentGame());
   } catch (std::exception& e) {
     BOOST_LOG_TRIVIAL(error) << "\"" << tempPlugin.Name() << "\" contains a condition that could not be evaluated. Details: " << e.what();
     list<Message> messages(tempPlugin.Messages());

src/tests/api/database_interface_test.h

@@ -122,35 +122,15 @@ TEST_P(DatabaseInterfaceTest, loadListsShouldSucceedIfTheMasterlistAndUserlistAr
   EXPECT_NO_THROW(db_->LoadLists(masterlistPath.string(), userlistPath_.string()));
 }
 
-TEST_P(DatabaseInterfaceTest, evalListsShouldReturnOkForAllLanguagesWithNoListsLoaded) {
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::english));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::english));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::spanish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::russian));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::french));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::chinese));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::polish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::brazilian_portuguese));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::finnish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::german));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::danish));
+TEST_P(DatabaseInterfaceTest, evalListsShouldReturnOkWithNoListsLoaded) {
+  EXPECT_NO_THROW(db_->EvalLists());
 }
 
 TEST_P(DatabaseInterfaceTest, evalListsShouldReturnOKForAllLanguagesWithAMasterlistLoaded) {
   ASSERT_NO_THROW(GenerateMasterlist());
   ASSERT_NO_THROW(db_->LoadLists(masterlistPath.string(), ""));
 
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::english));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::english));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::spanish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::russian));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::french));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::chinese));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::polish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::brazilian_portuguese));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::finnish));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::german));
-  EXPECT_NO_THROW(db_->EvalLists(LanguageCode::danish));
+  EXPECT_NO_THROW(db_->EvalLists());
 }
 
 TEST_P(DatabaseInterfaceTest, sortPluginsShouldSucceedIfPassedValidArguments) {

src/tests/backend/metadata/message_test.h

@@ -94,7 +94,7 @@ TEST_P(MessageTest, messagesWithEqualContentStringsShouldBeEqual) {
   EXPECT_TRUE(message1 == message2);
 }
 
-TEST_P(MessageTest, LessThanOperatorShouldUseCaseInsensitiveLexicographicalContentStringComparison) {
+TEST_P(MessageTest, lessThanOperatorShouldUseCaseInsensitiveLexicographicalContentStringComparison) {
   Message message1(MessageType::say, MessageContents({MessageContent("content1", LanguageCode::english)}), "condition1");
   Message message2(MessageType::warn, MessageContents({MessageContent("content1", LanguageCode::french)}), "condition2");
   EXPECT_FALSE(message1 < message2);
@@ -106,63 +106,12 @@ TEST_P(MessageTest, LessThanOperatorShouldUseCaseInsensitiveLexicographicalConte
   EXPECT_FALSE(message2 < message1);
 }
 
-TEST_P(MessageTest, evalConditionShouldCreateADefaultContentObjectIfNoneExists) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
+TEST_P(MessageTest, getContentShouldReturnADefaultContentObjectIfNoneExists) {
   Message message;
-  EXPECT_TRUE(message.EvalCondition(game, LanguageCode::english));
-  EXPECT_EQ(MessageContents({MessageContent()}), message.GetContent());
-}
-
-TEST_P(MessageTest, evalConditionShouldSelectTheStringForTheGivenLanguageIfOneExists) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
-  Message message(MessageType::say, MessageContents({
-      MessageContent("content1", LanguageCode::german),
-      MessageContent("content2", LanguageCode::english),
-      MessageContent("content3", LanguageCode::french),
-  }));
-
-  EXPECT_TRUE(message.EvalCondition(game, LanguageCode::french));
-  EXPECT_EQ(1, message.GetContent().size());
-  EXPECT_EQ(MessageContent("content3", LanguageCode::french), message.GetContent()[0]);
-}
-
-TEST_P(MessageTest, evalConditionShouldLeaveTheContentUnchangedIfOnlyOneStringExists) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-  MessageContent content("content1", LanguageCode::english);
-  Message message(MessageType::say, MessageContents({content}));
-
-  EXPECT_TRUE(message.EvalCondition(game, LanguageCode::french));
-  EXPECT_EQ(MessageContents({content}), message.GetContent());
-}
-
-TEST_P(MessageTest, evalConditionShouldSelectTheEnglishStringIfNoStringExistsForTheGivenLanguage) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
-  MessageContent content("content1", LanguageCode::english);
-  Message message(MessageType::say, MessageContents({
-      content,
-      MessageContent("content1", LanguageCode::german),
-  }));
-
-  EXPECT_TRUE(message.EvalCondition(game, LanguageCode::french));
-  EXPECT_EQ(MessageContents({content}), message.GetContent());
+  EXPECT_EQ(MessageContent(), message.GetContent(LanguageCode::english));
 }
 
 TEST_P(MessageTest, getContentShouldSelectTheEnglishStringIfThereIsNoStringForTheGivenLanguage) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
   Message message(MessageType::say, MessageContents({
     MessageContent("content1", LanguageCode::german),
     MessageContent("content2", LanguageCode::english),
@@ -173,10 +122,6 @@ TEST_P(MessageTest, getContentShouldSelectTheEnglishStringIfThereIsNoStringForTh
 }
 
 TEST_P(MessageTest, getContentShouldSelectTheGivenLanguageStringIfItExists) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
   Message message(MessageType::say, MessageContents({
     MessageContent("content1", LanguageCode::german),
     MessageContent("content2", LanguageCode::english),
@@ -186,11 +131,7 @@ TEST_P(MessageTest, getContentShouldSelectTheGivenLanguageStringIfItExists) {
   EXPECT_EQ("content3", message.GetContent(LanguageCode::french).GetText());
 }
 
-TEST_P(MessageTest, getTextShouldSelectTheContentStringIfOnlyOneExists) {
-  Game game(GetParam());
-  game.SetGamePath(dataPath.parent_path());
-  ASSERT_NO_THROW(game.Init(false, localPath));
-
+TEST_P(MessageTest, getContentShouldSelectTheContentStringIfOnlyOneExists) {
   Message message(MessageType::say, MessageContents({
     MessageContent("content1", LanguageCode::german),
   }));

src/tests/backend/metadata/plugin_metadata_test.h

@@ -659,7 +659,7 @@ TEST_P(PluginMetadataTest, evalAllConditionsShouldEvaluateAllMetadataConditions)
   plugin.DirtyInfo({info1, info2});
   plugin.CleanInfo({info1, info2});
 
-  EXPECT_NO_THROW(plugin.EvalAllConditions(game, LanguageCode::english));
+  EXPECT_NO_THROW(plugin.EvalAllConditions(game));
 
   std::set<File> expectedFiles({file1});
   EXPECT_EQ(expectedFiles, plugin.LoadAfter());

src/tests/backend/metadata_list_test.h

@@ -299,7 +299,7 @@ TEST_P(MetadataListTest, evalAllConditionsShouldEvaluateTheConditionsForThePlugi
   ASSERT_EQ(blankEsp, plugin.Name());
   ASSERT_FALSE(plugin.HasNameOnly());
 
-  EXPECT_NO_THROW(metadataList.EvalAllConditions(game, LanguageCode::english));
+  EXPECT_NO_THROW(metadataList.EvalAllConditions(game));
 
   plugin = metadataList.FindPlugin(PluginMetadata(blankEsm));
   EXPECT_EQ(std::list<Message>({