From 63e458eea6bd0063fdde10ef2e5a7f6bbb3c17f8 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 31 Jan 2017 19:03:43 +0000 Subject: [PATCH] Refactor some GameCache functions into GUI code --- src/backend/game/game_cache.cpp | 40 +------- src/backend/game/game_cache.h | 9 -- src/backend/plugin/plugin_sorter.cpp | 6 -- src/gui/query/sort_plugins_query.h | 6 ++ src/gui/state/game.cpp | 60 +++++++++++- src/gui/state/game.h | 18 +++- src/tests/backend/game/game_cache_test.h | 90 ----------------- src/tests/backend/plugin/plugin_sorter_test.h | 23 ----- src/tests/gui/state/game_test.h | 96 ++++++++++++++++++- 9 files changed, 178 insertions(+), 170 deletions(-) diff --git a/src/backend/game/game_cache.cpp b/src/backend/game/game_cache.cpp index 99af6a8f..5746d794 100644 --- a/src/backend/game/game_cache.cpp +++ b/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(std::move(plugin))); } -std::vector GameCache::GetMessages() const { - std::vector 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 guard(mutex_); - - messages_.push_back(message); -} - std::vector GameCache::GetLoadOrder() const { return loadOrder_; } @@ -136,19 +118,6 @@ void GameCache::StoreLoadOrder(const std::vector& loadOrder) { loadOrder_ = loadOrder; } -void GameCache::IncrementLoadOrderSortCount() { - lock_guard guard(mutex_); - - ++loadOrderSortCount_; -} - -void GameCache::DecrementLoadOrderSortCount() { - lock_guard guard(mutex_); - - if (loadOrderSortCount_ > 0) - --loadOrderSortCount_; -} - void GameCache::ClearCachedConditions() { lock_guard guard(mutex_); @@ -160,9 +129,4 @@ void GameCache::ClearCachedPlugins() { plugins_.clear(); } -void GameCache::ClearMessages() { - lock_guard guard(mutex_); - - messages_.clear(); -} } diff --git a/src/backend/game/game_cache.h b/src/backend/game/game_cache.h index 9b6e54e5..3312460b 100644 --- a/src/backend/game/game_cache.h +++ b/src/backend/game/game_cache.h @@ -52,26 +52,17 @@ public: std::shared_ptr GetPlugin(const std::string& pluginName) const; void AddPlugin(const Plugin&& plugin); - std::vector GetMessages() const; - void AppendMessage(const Message& message); - std::vector GetLoadOrder() const; void StoreLoadOrder(const std::vector& loadOrder); - void IncrementLoadOrderSortCount(); - void DecrementLoadOrderSortCount(); - void ClearCachedConditions(); void ClearCachedPlugins(); - void ClearMessages(); private: Masterlist masterlist_; MetadataList userlist_; std::unordered_map conditions_; std::unordered_map> plugins_; - std::vector messages_; std::vector loadOrder_; - unsigned short loadOrderSortCount_; mutable std::mutex mutex_; }; diff --git a/src/backend/plugin/plugin_sorter.cpp b/src/backend/plugin/plugin_sorter.cpp index 312ec4d7..5888134e 100644 --- a/src/backend/plugin/plugin_sorter.cpp +++ b/src/backend/plugin/plugin_sorter.cpp @@ -108,10 +108,6 @@ std::vector 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 PluginSorter::Sort(Game& game, const LanguageCode langu BOOST_LOG_TRIVIAL(info) << '\t' << plugins.back(); } - game.IncrementLoadOrderSortCount(); - return plugins; } diff --git a/src/gui/query/sort_plugins_query.h b/src/gui/query/sort_plugins_query.h index 0dfd0bb0..e4b074ed 100644 --- a/src/gui/query/sort_plugins_query.h +++ b/src/gui/query/sort_plugins_query.h @@ -70,8 +70,14 @@ private: sendProgressUpdate(frame_, boost::locale::translate("Sorting load order...")); std::vector 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, diff --git a/src/gui/state/game.cpp b/src/gui/state/game.cpp index 8a387dc9..06fedb01 100644 --- a/src/gui/state/game.cpp +++ b/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 guard(mutex_); + + ++loadOrderSortCount_; +} + +void Game::DecrementLoadOrderSortCount() { + lock_guard guard(mutex_); + + if (loadOrderSortCount_ > 0) + --loadOrderSortCount_; +} + +std::vector Game::GetMessages() const { + std::vector 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 guard(mutex_); + + messages_.push_back(message); +} + +void Game::ClearMessages() { + lock_guard guard(mutex_); + + messages_.clear(); +} + boost::filesystem::path Game::DetectGamePath(const GameSettings & gameSettings) { try { BOOST_LOG_TRIVIAL(trace) << "Checking if game \"" << gameSettings.Name() << "\" is installed."; diff --git a/src/gui/state/game.h b/src/gui/state/game.h index 283c1203..754bf55a 100644 --- a/src/gui/state/game.h +++ b/src/gui/state/game.h @@ -25,6 +25,7 @@ #ifndef LOOT_GUI_STATE_GAME #define LOOT_GUI_STATE_GAME +#include #include #include @@ -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& loadOrder) const; + + void IncrementLoadOrderSortCount(); + void DecrementLoadOrderSortCount(); + + std::vector 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& loadOrder, const boost::filesystem::path& backupDirectory); - const boost::filesystem::path lootDataPath_; + boost::filesystem::path lootDataPath_; std::shared_ptr gameHandle_; bool pluginsFullyLoaded_; + + std::vector messages_; + unsigned short loadOrderSortCount_; + + mutable std::mutex mutex_; }; } } diff --git a/src/tests/backend/game/game_cache_test.h b/src/tests/backend/game/game_cache_test.h index d9cb4f28..06263979 100644 --- a/src/tests/backend/game/game_cache_test.h +++ b/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 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 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()); -} } } diff --git a/src/tests/backend/plugin/plugin_sorter_test.h b/src/tests/backend/plugin/plugin_sorter_test.h index 9ed89d79..db3530f5 100644 --- a/src/tests/backend/plugin/plugin_sorter_test.h +++ b/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 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); diff --git a/src/tests/gui/state/game_test.h b/src/tests/gui/state/game_test.h index 70071dfa..e5598cf4 100644 --- a/src/tests/gui/state/game_test.h +++ b/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 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 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()); +} } } }