Refactor some GameCache functions into GUI code

src/backend/game/game_cache.cpp

@@ -37,15 +37,13 @@ using std::pair;
 using std::string;
 
 namespace loot {
-GameCache::GameCache() : loadOrderSortCount_(0) {}
+GameCache::GameCache() {}
 
 GameCache::GameCache(const GameCache& cache) :
   masterlist_(cache.masterlist_),
   userlist_(cache.userlist_),
   conditions_(cache.conditions_),
-  plugins_(cache.plugins_),
-  messages_(cache.messages_),
-  loadOrderSortCount_(cache.loadOrderSortCount_) {}
+  plugins_(cache.plugins_) {}
 
 GameCache& GameCache::operator=(const GameCache& cache) {
   if (&cache != this) {
@@ -53,8 +51,6 @@ GameCache& GameCache::operator=(const GameCache& cache) {
     userlist_ = cache.userlist_;
     conditions_ = cache.conditions_;
     plugins_ = cache.plugins_;
-    messages_ = cache.messages_;
-    loadOrderSortCount_ = cache.loadOrderSortCount_;
   }
 
   return *this;
@@ -114,20 +110,6 @@ void GameCache::AddPlugin(const Plugin&& plugin) {
   plugins_.emplace(plugin.GetLowercasedName(), std::make_shared<Plugin>(std::move(plugin)));
 }
 
-std::vector<Message> GameCache::GetMessages() const {
-  std::vector<Message> output(messages_);
-  if (loadOrderSortCount_ == 0)
-    output.push_back(Message(MessageType::warn, boost::locale::translate("You have not sorted your load order this session.")));
-
-  return output;
-}
-
-void GameCache::AppendMessage(const Message& message) {
-  lock_guard<mutex> guard(mutex_);
-
-  messages_.push_back(message);
-}
-
 std::vector<std::string> GameCache::GetLoadOrder() const {
   return loadOrder_;
 }
@@ -136,19 +118,6 @@ void GameCache::StoreLoadOrder(const std::vector<std::string>& loadOrder) {
   loadOrder_ = loadOrder;
 }
 
-void GameCache::IncrementLoadOrderSortCount() {
-  lock_guard<mutex> guard(mutex_);
-
-  ++loadOrderSortCount_;
-}
-
-void GameCache::DecrementLoadOrderSortCount() {
-  lock_guard<mutex> guard(mutex_);
-
-  if (loadOrderSortCount_ > 0)
-    --loadOrderSortCount_;
-}
-
 void GameCache::ClearCachedConditions() {
   lock_guard<mutex> guard(mutex_);
 
@@ -160,9 +129,4 @@ void GameCache::ClearCachedPlugins() {
 
   plugins_.clear();
 }
-void GameCache::ClearMessages() {
-  lock_guard<mutex> guard(mutex_);
-
-  messages_.clear();
-}
 }

src/backend/game/game_cache.h

@@ -52,26 +52,17 @@ public:
   std::shared_ptr<const Plugin> GetPlugin(const std::string& pluginName) const;
   void AddPlugin(const Plugin&& plugin);
 
-  std::vector<Message> GetMessages() const;
-  void AppendMessage(const Message& message);
-
   std::vector<std::string> GetLoadOrder() const;
   void StoreLoadOrder(const std::vector<std::string>& loadOrder);
 
-  void IncrementLoadOrderSortCount();
-  void DecrementLoadOrderSortCount();
-
   void ClearCachedConditions();
   void ClearCachedPlugins();
-  void ClearMessages();
 private:
   Masterlist masterlist_;
   MetadataList userlist_;
   std::unordered_map<std::string, bool> conditions_;
   std::unordered_map<std::string, std::shared_ptr<const Plugin>> plugins_;
-  std::vector<Message> messages_;
   std::vector<std::string> loadOrder_;
-  unsigned short loadOrderSortCount_;
 
   mutable std::mutex mutex_;
 };

src/backend/plugin/plugin_sorter.cpp

@@ -108,10 +108,6 @@ std::vector<std::string> PluginSorter::Sort(Game& game, const LanguageCode langu
   indexMap_.clear();
   oldLoadOrder_.clear();
 
-  // Clear any existing game-specific messages, as these only relate to
-  // state that has been changed by sorting.
-  game.ClearMessages();
-
   AddPluginVertices(game, language);
 
   // If there aren't any vertices, exit early, because sorting assumes
@@ -165,8 +161,6 @@ std::vector<std::string> PluginSorter::Sort(Game& game, const LanguageCode langu
     BOOST_LOG_TRIVIAL(info) << '\t' << plugins.back();
   }
 
-  game.IncrementLoadOrderSortCount();
-
   return plugins;
 }
 

src/gui/query/sort_plugins_query.h

@@ -70,8 +70,14 @@ private:
     sendProgressUpdate(frame_, boost::locale::translate("Sorting load order..."));
     std::vector<std::string> plugins;
     try {
+      // Clear any existing game-specific messages, as these only relate to
+      // state that has been changed by sorting.
+      state_.getCurrentGame().ClearMessages();
+
       PluginSorter sorter;
       plugins = sorter.Sort(state_.getCurrentGame(), state_.getLanguage().GetCode());
+
+      state_.getCurrentGame().IncrementLoadOrderSortCount();
     } catch (CyclicInteractionError& e) {
       BOOST_LOG_TRIVIAL(error) << "Failed to sort plugins. Details: " << e.what();
       state_.getCurrentGame().AppendMessage(Message(MessageType::error,

src/gui/state/game.cpp

@@ -51,6 +51,8 @@
 #endif
 
 using std::list;
+using std::lock_guard;
+using std::mutex;
 using std::string;
 using std::thread;
 using std::vector;
@@ -65,10 +67,33 @@ Game::Game(const GameSettings& gameSettings,
   GameSettings(gameSettings),
   lootDataPath_(lootDataPath),
   gameHandle_(CreateGameHandle(gameSettings.Type(), gameSettings.GamePath().string(), localDataPath.string())),
-  pluginsFullyLoaded_(false) {
+  pluginsFullyLoaded_(false),
+  loadOrderSortCount_(0) {
   gameHandle_->IdentifyMainMasterFile(gameSettings.Master());
 }
 
+Game::Game(const Game& game) :
+  GameSettings(game),
+  lootDataPath_(game.lootDataPath_),
+  gameHandle_(game.gameHandle_),
+  pluginsFullyLoaded_(game.pluginsFullyLoaded_),
+  messages_(game.messages_),
+  loadOrderSortCount_(0) {}
+
+Game& Game::operator=(const Game& game) {
+  if (&game != this) {
+    GameSettings::operator=(game);
+
+    lootDataPath_ = game.lootDataPath_;
+    gameHandle_ = game.gameHandle_;
+    pluginsFullyLoaded_ = game.pluginsFullyLoaded_;
+    messages_ = game.messages_;
+    loadOrderSortCount_ = game.loadOrderSortCount_;
+  }
+
+  return *this;
+}
+
 bool Game::IsInstalled(const GameSettings& gameSettings) {
   auto gamePath = DetectGamePath(gameSettings);
 
@@ -204,6 +229,39 @@ short Game::GetActiveLoadOrderIndex(const std::string & pluginName, const std::v
   return -1;
 }
 
+void Game::IncrementLoadOrderSortCount() {
+  lock_guard<mutex> guard(mutex_);
+
+  ++loadOrderSortCount_;
+}
+
+void Game::DecrementLoadOrderSortCount() {
+  lock_guard<mutex> guard(mutex_);
+
+  if (loadOrderSortCount_ > 0)
+    --loadOrderSortCount_;
+}
+
+std::vector<Message> Game::GetMessages() const {
+  std::vector<Message> output(messages_);
+  if (loadOrderSortCount_ == 0)
+    output.push_back(Message(MessageType::warn, boost::locale::translate("You have not sorted your load order this session.")));
+
+  return output;
+}
+
+void Game::AppendMessage(const Message& message) {
+  lock_guard<mutex> guard(mutex_);
+
+  messages_.push_back(message);
+}
+
+void Game::ClearMessages() {
+  lock_guard<mutex> guard(mutex_);
+
+  messages_.clear();
+}
+
 boost::filesystem::path Game::DetectGamePath(const GameSettings & gameSettings) {
   try {
     BOOST_LOG_TRIVIAL(trace) << "Checking if game \"" << gameSettings.Name() << "\" is installed.";

src/gui/state/game.h

@@ -25,6 +25,7 @@
 #ifndef LOOT_GUI_STATE_GAME
 #define LOOT_GUI_STATE_GAME
 
+#include <mutex>
 #include <string>
 
 #include <boost/filesystem.hpp>
@@ -39,6 +40,9 @@ public:
   Game(const GameSettings& gameSettings,
        const boost::filesystem::path& lootDataPath,
        const boost::filesystem::path& localDataPath = "");
+  Game(const Game& game);
+
+  Game& operator=(const Game& game);
 
   using GameSettings::Type;
 
@@ -62,6 +66,13 @@ public:
 
   short GetActiveLoadOrderIndex(const std::string & pluginName) const;
   short GetActiveLoadOrderIndex(const std::string & pluginName, const std::vector<std::string>& loadOrder) const;
+
+  void IncrementLoadOrderSortCount();
+  void DecrementLoadOrderSortCount();
+
+  std::vector<Message> GetMessages() const;
+  void AppendMessage(const Message& message);
+  void ClearMessages();
 private:
 #ifdef _WIN32
   static std::string RegKeyStringValue(const std::string& keyStr, const std::string& subkey, const std::string& value);
@@ -70,10 +81,15 @@ private:
   static void BackupLoadOrder(const std::vector<std::string>& loadOrder,
                               const boost::filesystem::path& backupDirectory);
 
-  const boost::filesystem::path lootDataPath_;
+  boost::filesystem::path lootDataPath_;
 
   std::shared_ptr<GameInterface> gameHandle_;
   bool pluginsFullyLoaded_;
+
+  std::vector<Message> messages_;
+  unsigned short loadOrderSortCount_;
+
+  mutable std::mutex mutex_;
 };
 }
 }

src/tests/backend/game/game_cache_test.h

@@ -55,34 +55,6 @@ INSTANTIATE_TEST_CASE_P(,
                         ::testing::Values(
                           GameType::tes5));
 
-TEST_P(GameCacheTest, copyConstructorShouldCopyCachedData) {
-  cache_.CacheCondition(condition, true);
-  cache_.AddPlugin(Plugin(game_, blankEsm, true));
-  Message expectedMessage(MessageType::say, "1");
-  cache_.AppendMessage(expectedMessage);
-  cache_.IncrementLoadOrderSortCount();
-
-  GameCache otherCache(cache_);
-  EXPECT_EQ(std::make_pair(true, true), otherCache.GetCachedCondition(conditionLowercase));
-  EXPECT_EQ(blankEsm, otherCache.GetPlugin(blankEsm)->GetName());
-  ASSERT_EQ(1, otherCache.GetMessages().size());
-  EXPECT_EQ(expectedMessage, otherCache.GetMessages()[0]);
-}
-
-TEST_P(GameCacheTest, assignmentOperatorShouldCopyCachedData) {
-  cache_.CacheCondition(condition, true);
-  cache_.AddPlugin(Plugin(game_, blankEsm, true));
-  Message expectedMessage(MessageType::say, "1");
-  cache_.AppendMessage(expectedMessage);
-  cache_.IncrementLoadOrderSortCount();
-
-  GameCache otherCache = cache_;
-  EXPECT_EQ(std::make_pair(true, true), otherCache.GetCachedCondition(conditionLowercase));
-  EXPECT_EQ(blankEsm, otherCache.GetPlugin(blankEsm)->GetName());
-  ASSERT_EQ(1, otherCache.GetMessages().size());
-  EXPECT_EQ(expectedMessage, otherCache.GetMessages()[0]);
-}
-
 TEST_P(GameCacheTest, gettingATrueConditionShouldReturnATrueTruePair) {
   EXPECT_NO_THROW(cache_.CacheCondition(condition, true));
 
@@ -154,68 +126,6 @@ TEST_P(GameCacheTest, clearingCachedPluginsShouldClearAnyCachedPlugins) {
 
   EXPECT_TRUE(cache_.GetPlugins().empty());
 }
-
-TEST_P(GameCacheTest, aMessageShouldBeCachedByDefault) {
-  ASSERT_EQ(1, cache_.GetMessages().size());
-}
-
-TEST_P(GameCacheTest, incrementLoadOrderSortCountShouldSupressTheDefaultCachedMessage) {
-  cache_.IncrementLoadOrderSortCount();
-
-  EXPECT_TRUE(cache_.GetMessages().empty());
-}
-
-TEST_P(GameCacheTest, decrementingLoadOrderSortCountToZeroShouldShowTheDefaultCachedMessage) {
-  auto expectedMessages = cache_.GetMessages();
-  cache_.IncrementLoadOrderSortCount();
-  cache_.DecrementLoadOrderSortCount();
-
-  EXPECT_EQ(expectedMessages, cache_.GetMessages());
-}
-
-TEST_P(GameCacheTest, decrementingLoadOrderSortCountThatIsAlreadyZeroShouldShowTheDefaultCachedMessage) {
-  auto expectedMessages = cache_.GetMessages();
-  cache_.DecrementLoadOrderSortCount();
-
-  EXPECT_EQ(expectedMessages, cache_.GetMessages());
-}
-
-TEST_P(GameCacheTest, decrementingLoadOrderSortCountToANonZeroValueShouldSupressTheDefaultCachedMessage) {
-  auto expectedMessages = cache_.GetMessages();
-  cache_.IncrementLoadOrderSortCount();
-  cache_.IncrementLoadOrderSortCount();
-  cache_.DecrementLoadOrderSortCount();
-
-  EXPECT_TRUE(cache_.GetMessages().empty());
-}
-
-TEST_P(GameCacheTest, appendingMessagesShouldStoreThemInTheGivenOrder) {
-  std::vector<Message> messages({
-      Message(MessageType::say, "1"),
-      Message(MessageType::error, "2"),
-  });
-  for (const auto& message : messages)
-    cache_.AppendMessage(message);
-
-  ASSERT_EQ(3, cache_.GetMessages().size());
-  EXPECT_EQ(messages[0], cache_.GetMessages()[0]);
-  EXPECT_EQ(messages[1], cache_.GetMessages()[1]);
-}
-
-TEST_P(GameCacheTest, clearingMessagesShouldRemoveAllAppendedMessages) {
-  std::vector<Message> messages({
-      Message(MessageType::say, "1"),
-      Message(MessageType::error, "2"),
-  });
-  for (const auto& message : messages)
-    cache_.AppendMessage(message);
-
-  auto previousSize = cache_.GetMessages().size();
-
-  cache_.ClearMessages();
-
-  EXPECT_EQ(previousSize - messages.size(), cache_.GetMessages().size());
-}
 }
 }
 

src/tests/backend/plugin/plugin_sorter_test.h

@@ -90,29 +90,6 @@ TEST_P(PluginSorterTest, sortingShouldNotMakeUnnecessaryChangesToAnExistingLoadO
   EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder)));
 }
 
-TEST_P(PluginSorterTest, sortingShouldClearExistingGameMessages) {
-  ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
-  game_.AppendMessage(Message(MessageType::say, "1"));
-  ASSERT_FALSE(game_.GetMessages().empty());
-
-  PluginSorter ps;
-  std::vector<std::string> sorted = ps.Sort(game_, LanguageCode::english);
-  EXPECT_TRUE(game_.GetMessages().empty());
-}
-
-TEST_P(PluginSorterTest, failedSortShouldNotClearExistingGameMessages) {
-  ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
-  PluginMetadata plugin(blankEsm);
-  plugin.LoadAfter({File(blankMasterDependentEsm)});
-  game_.GetUserlist().AddPlugin(plugin);
-  game_.AppendMessage(Message(MessageType::say, "1"));
-  ASSERT_FALSE(game_.GetMessages().empty());
-
-  PluginSorter ps;
-  EXPECT_THROW(ps.Sort(game_, LanguageCode::english), CyclicInteractionError);
-  EXPECT_FALSE(game_.GetMessages().empty());
-}
-
 TEST_P(PluginSorterTest, sortingShouldEvaluateRelativeGlobalPriorities) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
   PluginMetadata plugin(blankDifferentMasterDependentEsp);

src/tests/gui/state/game_test.h

@@ -84,7 +84,7 @@ TEST_P(GameTest, constructingFromGameSettingsShouldUseTheirValues) {
   settings.SetRepoURL("foo");
   settings.SetRepoBranch("foo");
   settings.SetGamePath(localPath);
-  Game game = Game(settings, lootDataPath, localPath);
+  Game game(settings, lootDataPath, localPath);
 
   EXPECT_EQ(GetParam(), game.Type());
   EXPECT_EQ(settings.Name(), game.Name());
@@ -117,6 +117,28 @@ TEST_P(GameTest, constructingShouldNotThrowOnWindowsIfLocalPathIsNotGiven) {
 }
 #endif
 
+TEST_P(GameTest, copyConstructorShouldCopyGameData) {
+  Game game1(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  game1.AppendMessage(Message(MessageType::say, "1"));
+
+  Game game2(game1);
+
+  EXPECT_EQ(game1.MasterlistPath(), game2.MasterlistPath());
+  EXPECT_EQ(game1.ArePluginsFullyLoaded(), game2.ArePluginsFullyLoaded());
+  EXPECT_EQ(game1.GetMessages(), game2.GetMessages());
+}
+
+TEST_P(GameTest, assignmentOperatorShouldCopyGameData) {
+  Game game1(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  game1.AppendMessage(Message(MessageType::say, "1"));
+
+  Game game2 = game1;
+
+  EXPECT_EQ(game1.MasterlistPath(), game2.MasterlistPath());
+  EXPECT_EQ(game1.ArePluginsFullyLoaded(), game2.ArePluginsFullyLoaded());
+  EXPECT_EQ(game1.GetMessages(), game2.GetMessages());
+}
+
 TEST_P(GameTest, isInstalledShouldBeFalseIfGamePathIsNotSet) {
   EXPECT_FALSE(Game::IsInstalled(GameSettings(GetParam())));
 }
@@ -126,7 +148,7 @@ TEST_P(GameTest, isInstalledShouldBeTrueIfGamePathIsValid) {
 }
 
 TEST_P(GameTest, initShouldNotCreateAGameFolderIfTheLootDataPathIsEmpty) {
-  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), "", localPath);
+  Game game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), "", localPath);
 
   ASSERT_FALSE(boost::filesystem::exists(lootDataPath / game.FolderName()));
   EXPECT_NO_THROW(game.Init());
@@ -361,6 +383,76 @@ TEST_P(GameTest, setLoadOrderShouldKeepUpToThreeBackups) {
   loadOrder = readFileLines(lootDataPath / game.FolderName() / loadOrderBackupFile2);
   EXPECT_EQ(firstSetLoadOrder, loadOrder);
 }
+
+TEST_P(GameTest, aMessageShouldBeCachedByDefault) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+
+  ASSERT_EQ(1, game.GetMessages().size());
+}
+
+TEST_P(GameTest, incrementLoadOrderSortCountShouldSupressTheDefaultCachedMessage) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  game.IncrementLoadOrderSortCount();
+
+  EXPECT_TRUE(game.GetMessages().empty());
+}
+
+TEST_P(GameTest, decrementingLoadOrderSortCountToZeroShouldShowTheDefaultCachedMessage) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  auto expectedMessages = game.GetMessages();
+  game.IncrementLoadOrderSortCount();
+  game.DecrementLoadOrderSortCount();
+
+  EXPECT_EQ(expectedMessages, game.GetMessages());
+}
+
+TEST_P(GameTest, decrementingLoadOrderSortCountThatIsAlreadyZeroShouldShowTheDefaultCachedMessage) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  auto expectedMessages = game.GetMessages();
+  game.DecrementLoadOrderSortCount();
+
+  EXPECT_EQ(expectedMessages, game.GetMessages());
+}
+
+TEST_P(GameTest, decrementingLoadOrderSortCountToANonZeroValueShouldSupressTheDefaultCachedMessage) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  auto expectedMessages = game.GetMessages();
+  game.IncrementLoadOrderSortCount();
+  game.IncrementLoadOrderSortCount();
+  game.DecrementLoadOrderSortCount();
+
+  EXPECT_TRUE(game.GetMessages().empty());
+}
+
+TEST_P(GameTest, appendingMessagesShouldStoreThemInTheGivenOrder) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  std::vector<Message> messages({
+    Message(MessageType::say, "1"),
+    Message(MessageType::error, "2"),
+  });
+  for (const auto& message : messages)
+    game.AppendMessage(message);
+
+  ASSERT_EQ(3, game.GetMessages().size());
+  EXPECT_EQ(messages[0], game.GetMessages()[0]);
+  EXPECT_EQ(messages[1], game.GetMessages()[1]);
+}
+
+TEST_P(GameTest, clearingMessagesShouldRemoveAllAppendedMessages) {
+  Game game = Game(GameSettings(GetParam()).SetGamePath(dataPath.parent_path()), lootDataPath, localPath);
+  std::vector<Message> messages({
+    Message(MessageType::say, "1"),
+    Message(MessageType::error, "2"),
+  });
+  for (const auto& message : messages)
+    game.AppendMessage(message);
+
+  auto previousSize = game.GetMessages().size();
+
+  game.ClearMessages();
+
+  EXPECT_EQ(previousSize - messages.size(), game.GetMessages().size());
+}
 }
 }
 }