From 29ddff40c3fe28b3f42db6ba014743139efdd1fe Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 5 Dec 2015 13:33:12 +0000 Subject: [PATCH 1/8] Remove game CRC cache Non-plugin CRCs that were stored in the game's cache don't need to be, as they are only used in conditions, which are cached separately. Plugin objects are now used to cache their CRC values along with almost all plugin data. This does remove the ability to cache plugin CRCs calculated when evaluating conditions, but I think they were never used. --- src/backend/game/game_cache.cpp | 19 ------------------ src/backend/game/game_cache.h | 4 ---- src/backend/metadata/condition_grammar.h | 25 ++++++++++++++---------- src/backend/plugin/plugin.cpp | 3 +-- src/backend/plugin/plugin.h | 2 +- src/tests/backend/game/test_game_cache.h | 11 ----------- 6 files changed, 17 insertions(+), 47 deletions(-) diff --git a/src/backend/game/game_cache.cpp b/src/backend/game/game_cache.cpp index 533e6a5a..fa4bdfff 100644 --- a/src/backend/game/game_cache.cpp +++ b/src/backend/game/game_cache.cpp @@ -42,22 +42,15 @@ namespace loot { GameCache::GameCache() {} GameCache::GameCache(const GameCache& cache) : conditionCache(cache.conditionCache), - crcCache(cache.crcCache), activePlugins(cache.activePlugins) {} GameCache& GameCache::operator=(const GameCache& cache) { conditionCache = cache.conditionCache; - crcCache = cache.crcCache; activePlugins = cache.activePlugins; return *this; } - void GameCache::CacheCrc(const std::string& plugin, uint32_t crc) { - std::lock_guard guard(mutex); - crcCache.insert(pair(boost::locale::to_lower(plugin), crc)); - } - void GameCache::CacheCondition(const std::string& condition, bool result) { std::lock_guard guard(mutex); conditionCache.insert(pair(boost::locale::to_lower(condition), result)); @@ -68,17 +61,6 @@ namespace loot { activePlugins = plugins; } - uint32_t GameCache::GetCachedCrc(const std::string& plugin) const { - std::lock_guard guard(mutex); - - auto it = crcCache.find(boost::locale::to_lower(plugin)); - - if (it != crcCache.end()) - return it->second; - else - return 0; - } - std::pair GameCache::GetCachedCondition(const std::string& condition) const { std::lock_guard guard(mutex); @@ -100,7 +82,6 @@ namespace loot { std::lock_guard guard(mutex); conditionCache.clear(); - crcCache.clear(); activePlugins.clear(); } } diff --git a/src/backend/game/game_cache.h b/src/backend/game/game_cache.h index ebc06881..b20fdde3 100644 --- a/src/backend/game/game_cache.h +++ b/src/backend/game/game_cache.h @@ -39,12 +39,9 @@ namespace loot { GameCache& operator=(const GameCache& cache); - void CacheCrc(const std::string& plugin, uint32_t crc); void CacheCondition(const std::string& condition, bool result); void CacheActivePlugins(const std::unordered_set& plugins); - // Returns 0 if no cached CRC. - uint32_t GetCachedCrc(const std::string& plugin) const; // Returns false for second bool if no cached condition. std::pair GetCachedCondition(const std::string& condition) const; bool IsPluginActive(const std::string& plugin) const; @@ -53,7 +50,6 @@ namespace loot { private: //Caches for condition results, CRCs and active plugins. std::unordered_map conditionCache; - std::unordered_map crcCache; std::unordered_set activePlugins; mutable std::mutex mutex; diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index fc6c74c4..4b280c1c 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -287,21 +287,26 @@ namespace loot { if (_game == nullptr) return; - uint32_t crc = _game->GetCachedCrc(file); + uint32_t crc = 0; + if (file == "LOOT") + crc = GetCrc32(boost::filesystem::absolute("LOOT.exe")); + else { + // CRC could be for a plugin or a file. + // Get the CRC from the game plugin cache if possible. + auto pluginPairIt = _game->plugins.find(boost::locale::to_lower(file)); + if (pluginPairIt != _game->plugins.end()) + crc = pluginPairIt->second.Crc(); - if (crc == 0) { - if (file == "LOOT") - crc = GetCrc32(boost::filesystem::absolute("LOOT.exe")); - if (boost::filesystem::exists(_game->DataPath() / file)) - crc = GetCrc32(_game->DataPath() / file); - else if ((boost::iends_with(file, ".esp") || boost::iends_with(file, ".esm")) && boost::filesystem::exists(_game->DataPath() / (file + ".ghost"))) - crc = GetCrc32(_game->DataPath() / (file + ".ghost")); + if (crc == 0) { + if (boost::filesystem::exists(_game->DataPath() / file)) + crc = GetCrc32(_game->DataPath() / file); + else if ((boost::iends_with(file, ".esp") || boost::iends_with(file, ".esm")) && boost::filesystem::exists(_game->DataPath() / (file + ".ghost"))) + crc = GetCrc32(_game->DataPath() / (file + ".ghost")); + } else { result = false; return; } - - _game->CacheCrc(file, crc); } result = checksum == crc; diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index c6915020..5c760dc1 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -46,7 +46,7 @@ namespace loot { crc(0), numOverrideRecords(0) {} - Plugin::Plugin(Game& game, const std::string& name, const bool headerOnly) : + Plugin::Plugin(const Game& game, const std::string& name, const bool headerOnly) : PluginMetadata(name), libespm::Plugin(game.LibespmId()), _isEmpty(true), @@ -67,7 +67,6 @@ namespace loot { if (!headerOnly) { BOOST_LOG_TRIVIAL(trace) << Name() << ": Caching CRC value."; crc = GetCrc32(filepath); - game.CacheCrc(Name(), crc); } BOOST_LOG_TRIVIAL(trace) << Name() << ": Counting override FormIDs."; diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index e673b793..feed73ce 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -42,7 +42,7 @@ namespace loot { class Plugin : public PluginMetadata, private libespm::Plugin { public: Plugin(const std::string& name); - Plugin(Game& game, const std::string& name, const bool headerOnly); + Plugin(const Game& game, const std::string& name, const bool headerOnly); using libespm::Plugin::getDescription; using libespm::Plugin::getFormIds; diff --git a/src/tests/backend/game/test_game_cache.h b/src/tests/backend/game/test_game_cache.h index 311091ad..56fd00a7 100644 --- a/src/tests/backend/game/test_game_cache.h +++ b/src/tests/backend/game/test_game_cache.h @@ -35,12 +35,10 @@ TEST_F(GameCache, Constructors) { loot::GameCache cache; std::unordered_set plugins({"skyrim.esm"}); - EXPECT_NO_THROW(cache.CacheCrc("Blank.esp", 5)); EXPECT_NO_THROW(cache.CacheCondition("True Condition", true)); EXPECT_NO_THROW(cache.CacheActivePlugins(plugins)); loot::GameCache cache2(cache); - EXPECT_EQ(5, cache2.GetCachedCrc("blank.Esp")); EXPECT_EQ(std::make_pair(true, true), cache2.GetCachedCondition("true Condition")); EXPECT_TRUE(cache2.IsPluginActive("Skyrim.esm")); } @@ -49,23 +47,14 @@ TEST_F(GameCache, AssignmentOperator) { loot::GameCache cache; std::unordered_set plugins({"skyrim.esm"}); - EXPECT_NO_THROW(cache.CacheCrc("Blank.esp", 5)); EXPECT_NO_THROW(cache.CacheCondition("True Condition", true)); EXPECT_NO_THROW(cache.CacheActivePlugins(plugins)); loot::GameCache cache2 = cache; - EXPECT_EQ(5, cache2.GetCachedCrc("blank.Esp")); EXPECT_EQ(std::make_pair(true, true), cache2.GetCachedCondition("true Condition")); EXPECT_TRUE(cache2.IsPluginActive("Skyrim.esm")); } -TEST_F(GameCache, CacheCrc) { - loot::GameCache cache; - EXPECT_NO_THROW(cache.CacheCrc("Blank.esp", 5)); - EXPECT_EQ(5, cache.GetCachedCrc("blank.Esp")); - EXPECT_EQ(0, cache.GetCachedCrc("Blank.missing.esp")); -} - TEST_F(GameCache, CacheCondition) { loot::GameCache cache; EXPECT_NO_THROW(cache.CacheCondition("True Condition", true)); From 32b6d27cbb22fe38d4e458c1528c8110568bb00a Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 4 Nov 2015 22:41:49 +0000 Subject: [PATCH 2/8] Remove active plugins from game cache Cache them in the Plugin objects instead. --- src/api/api.cpp | 1 - src/backend/game/game.cpp | 6 --- src/backend/game/game.h | 1 - src/backend/game/game_cache.cpp | 16 +----- src/backend/game/game_cache.h | 3 -- src/backend/game/load_order_handler.cpp | 25 ++++----- src/backend/game/load_order_handler.h | 3 +- src/backend/metadata/condition_grammar.h | 2 +- src/backend/plugin/plugin.cpp | 14 +++-- src/backend/plugin/plugin.h | 3 +- src/gui/handler.cpp | 7 +-- src/tests/backend/game/test_game.h | 51 ------------------- src/tests/backend/game/test_game_cache.h | 17 ------- .../backend/game/test_load_order_handler.h | 16 +++--- src/tests/backend/plugin/test_plugin.h | 8 +-- 15 files changed, 39 insertions(+), 134 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index fd29de1b..ca979866 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -400,7 +400,6 @@ LOOT_API unsigned int loot_eval_lists(loot_db db, const unsigned int language) { loot::MetadataList userTemp = db->rawUserMetadata; try { // Refresh active plugins before evaluating conditions. - db->RefreshActivePluginsList(); temp.EvalAllConditions(*db, language); userTemp.EvalAllConditions(*db, language); } diff --git a/src/backend/game/game.cpp b/src/backend/game/game.cpp index e63c7191..4a6334fb 100644 --- a/src/backend/game/game.cpp +++ b/src/backend/game/game.cpp @@ -77,12 +77,6 @@ namespace loot { } LoadOrderHandler::Init(*this, gameLocalAppData); - - RefreshActivePluginsList(); - } - - void Game::RefreshActivePluginsList() { - CacheActivePlugins(GetActivePlugins()); } void Game::RedatePlugins() { diff --git a/src/backend/game/game.h b/src/backend/game/game.h index 51a6bd6d..3d242291 100644 --- a/src/backend/game/game.h +++ b/src/backend/game/game.h @@ -47,7 +47,6 @@ namespace loot { void Init(bool createFolder, const boost::filesystem::path& gameLocalAppData = ""); - void RefreshActivePluginsList(); void RedatePlugins(); //Change timestamps to match load order (Skyrim only). void LoadPlugins(bool headersOnly); //Loads all installed plugins. diff --git a/src/backend/game/game_cache.cpp b/src/backend/game/game_cache.cpp index fa4bdfff..25f546d5 100644 --- a/src/backend/game/game_cache.cpp +++ b/src/backend/game/game_cache.cpp @@ -41,12 +41,10 @@ namespace lc = boost::locale; namespace loot { GameCache::GameCache() {} GameCache::GameCache(const GameCache& cache) - : conditionCache(cache.conditionCache), - activePlugins(cache.activePlugins) {} + : conditionCache(cache.conditionCache) {} GameCache& GameCache::operator=(const GameCache& cache) { conditionCache = cache.conditionCache; - activePlugins = cache.activePlugins; return *this; } @@ -56,11 +54,6 @@ namespace loot { conditionCache.insert(pair(boost::locale::to_lower(condition), result)); } - void GameCache::CacheActivePlugins(const std::unordered_set& plugins) { - std::lock_guard guard(mutex); - activePlugins = plugins; - } - std::pair GameCache::GetCachedCondition(const std::string& condition) const { std::lock_guard guard(mutex); @@ -72,16 +65,9 @@ namespace loot { return std::pair(false, false); } - bool GameCache::IsPluginActive(const std::string& plugin) const { - std::lock_guard guard(mutex); - - return activePlugins.find(boost::locale::to_lower(plugin)) != activePlugins.end(); - } - void GameCache::ClearCache() { std::lock_guard guard(mutex); conditionCache.clear(); - activePlugins.clear(); } } diff --git a/src/backend/game/game_cache.h b/src/backend/game/game_cache.h index b20fdde3..bccba2cf 100644 --- a/src/backend/game/game_cache.h +++ b/src/backend/game/game_cache.h @@ -40,17 +40,14 @@ namespace loot { GameCache& operator=(const GameCache& cache); void CacheCondition(const std::string& condition, bool result); - void CacheActivePlugins(const std::unordered_set& plugins); // Returns false for second bool if no cached condition. std::pair GetCachedCondition(const std::string& condition) const; - bool IsPluginActive(const std::string& plugin) const; void ClearCache(); private: //Caches for condition results, CRCs and active plugins. std::unordered_map conditionCache; - std::unordered_set activePlugins; mutable std::mutex mutex; }; diff --git a/src/backend/game/load_order_handler.cpp b/src/backend/game/load_order_handler.cpp index 02fb8d4d..63be2676 100644 --- a/src/backend/game/load_order_handler.cpp +++ b/src/backend/game/load_order_handler.cpp @@ -97,33 +97,28 @@ namespace loot { } } - std::unordered_set LoadOrderHandler::GetActivePlugins() const { - BOOST_LOG_TRIVIAL(debug) << "Getting active plugins."; + bool LoadOrderHandler::IsPluginActive(const std::string& pluginName) const { + BOOST_LOG_TRIVIAL(debug) << "Checking if plugin \"" << pluginName << "\" is active."; - char ** pluginArr; - size_t pluginArrSize; - unsigned int ret = lo_get_active_plugins(_gh, &pluginArr, &pluginArrSize); - if (ret != LIBLO_OK && ret != LIBLO_WARN_BAD_FILENAME && ret != LIBLO_WARN_INVALID_LIST && ret != LIBLO_WARN_LO_MISMATCH) { + bool result = false; + unsigned int ret = lo_get_plugin_active(_gh, pluginName.c_str(), &result); + if (ret != LIBLO_OK && ret != LIBLO_WARN_BAD_FILENAME) { const char * e = nullptr; string err; lo_get_error_message(&e); if (e == nullptr) { - BOOST_LOG_TRIVIAL(error) << "libloadorder failed to get the active plugins list. Details could not be fetched."; - err = lc::translate("libloadorder failed to get the active plugins list. Details could not be fetched.").str(); + BOOST_LOG_TRIVIAL(error) << "libloadorder failed to check if a plugin is active. Details could not be fetched."; + err = lc::translate("libloadorder failed to check if a plugin is active. Details could not be fetched.").str(); } else { - BOOST_LOG_TRIVIAL(error) << "libloadorder failed to get the active plugins list. Details: " << e; - err = lc::translate("libloadorder failed to get the active plugins list. Details:").str() + " " + e; + BOOST_LOG_TRIVIAL(error) << "libloadorder failed to check if a plugin is active. Details: " << e; + err = lc::translate("libloadorder failed to check if a plugin is active. Details:").str() + " " + e; } lo_cleanup(); throw error(error::liblo_error, err); } - std::unordered_set activePlugins; - for (size_t i = 0; i < pluginArrSize; ++i) { - activePlugins.insert(boost::locale::to_lower(string(pluginArr[i]))); - } - return activePlugins; + return result; } std::list LoadOrderHandler::GetLoadOrder() const { diff --git a/src/backend/game/load_order_handler.h b/src/backend/game/load_order_handler.h index dcaebcd0..1a6d0b3b 100644 --- a/src/backend/game/load_order_handler.h +++ b/src/backend/game/load_order_handler.h @@ -43,9 +43,10 @@ namespace loot { void Init(const GameSettings& game, const boost::filesystem::path& gameLocalAppData = ""); - std::unordered_set GetActivePlugins() const; std::list GetLoadOrder() const; + bool IsPluginActive(const std::string& pluginName) const; + //These modify game load order, even though const. void SetLoadOrder(const char * const * const loadOrder, const size_t numPlugins) const; // For API. void SetLoadOrder(const std::list& loadOrder) const; diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index 4b280c1c..39c46354 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -363,7 +363,7 @@ namespace loot { if (file == "LOOT") result = false; else - result = Plugin(file).IsActive(*_game); + result = Plugin(*_game, file, true).IsActive(); BOOST_LOG_TRIVIAL(trace) << "Active check result: " << result; } diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index 5c760dc1..b38494c9 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -42,6 +42,7 @@ namespace loot { PluginMetadata(n), libespm::Plugin(libespm::GameId::SKYRIM), _isEmpty(true), + _isActive(false), _loadsBsa(false), crc(0), numOverrideRecords(0) {} @@ -50,6 +51,7 @@ namespace loot { PluginMetadata(name), libespm::Plugin(game.LibespmId()), _isEmpty(true), + _isActive(false), _loadsBsa(false), crc(0), numOverrideRecords(0) { @@ -96,6 +98,8 @@ namespace loot { } } } + // Get whether the plugin is active or not. + _isActive = game.IsPluginActive(name); // Get whether the plugin loads a BSA or not. if (game.Id() == Game::tes5) { @@ -192,8 +196,8 @@ namespace loot { return false; } - bool Plugin::IsActive(const Game& game) const { - return game.IsPluginActive(Name()); + bool Plugin::IsActive() const { + return _isActive; } uint32_t Plugin::Crc() const { @@ -202,7 +206,7 @@ namespace loot { bool Plugin::CheckInstallValidity(const Game& game) { BOOST_LOG_TRIVIAL(trace) << "Checking that the current install is valid according to " << Name() << "'s data."; - if (IsActive(game)) { + if (IsActive()) { auto pluginExists = [](const Game& game, const std::string& file) { return boost::filesystem::exists(game.DataPath() / file) || ((boost::iends_with(file, ".esp") || boost::iends_with(file, ".esm")) && boost::filesystem::exists(game.DataPath() / (file + ".ghost"))); @@ -213,7 +217,7 @@ namespace loot { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" requires \"" << master << "\", but it is missing."; messages.push_back(Message(Message::error, (boost::format(boost::locale::translate("This plugin requires \"%1%\" to be installed, but it is missing.")) % master).str())); } - else if (!Plugin(master).IsActive(game)) { + else if (!Plugin(game, master, true).IsActive()) { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" requires \"" << master << "\", but it is inactive."; messages.push_back(Message(Message::error, (boost::format(boost::locale::translate("This plugin requires \"%1%\" to be active, but it is inactive.")) % master).str())); } @@ -227,7 +231,7 @@ namespace loot { } } for (const auto &inc : Incs()) { - if (pluginExists(game, inc.Name()) && Plugin(inc.Name()).IsActive(game)) { + if (pluginExists(game, inc.Name()) && Plugin(game, inc.Name(), true).IsActive()) { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" is incompatible with \"" << inc.Name() << "\", but both are present."; messages.push_back(loot::Message(Message::error, (boost::format(boost::locale::translate("This plugin is incompatible with \"%1%\", but both are present.")) % inc.Name()).str())); } diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index feed73ce..747068d7 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -54,7 +54,7 @@ namespace loot { size_t NumOverrideFormIDs() const; bool LoadsBSA() const; - bool IsActive(const Game& game) const; + bool IsActive() const; //Load ordering functions. bool DoFormIDsOverlap(const Plugin& plugin) const; @@ -65,6 +65,7 @@ namespace loot { static bool IsValid(const std::string& filename, const Game& game); private: bool _isEmpty; // Does the plugin contain any records other than the TES4 header? + bool _isActive; bool _loadsBsa; std::string version; //Obtained from description field. uint32_t crc; diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 99ffcc5b..6c3642cb 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -355,7 +355,7 @@ namespace loot { } size_t i = 0; for (const auto& plugin : plugins) { - if (Plugin(plugin).IsActive(_lootState.CurrentGame())) { + if (Plugin(_lootState.CurrentGame(), plugin, true).IsActive()) { ss << setw(decLength) << i << " " << hex << setw(2) << i << dec << " "; ++i; } @@ -642,9 +642,6 @@ namespace loot { // First clear CRC and condition caches, otherwise they could lead to incorrect evaluations. _lootState.CurrentGame().ClearCache(); - // Also refresh active plugins list. - _lootState.CurrentGame().RefreshActivePluginsList(); - bool isFirstLoad = _lootState.CurrentGame().plugins.empty(); _lootState.CurrentGame().LoadPlugins(true); @@ -741,7 +738,7 @@ namespace loot { pluginNode["__type"] = "Plugin"; // For conversion back into a JS typed object. pluginNode["name"] = plugin.Name(); - pluginNode["isActive"] = plugin.IsActive(_lootState.CurrentGame()); + pluginNode["isActive"] = plugin.IsActive(); pluginNode["isEmpty"] = plugin.IsEmpty(); pluginNode["isMaster"] = plugin.isMasterFile(); pluginNode["loadsBSA"] = plugin.LoadsBSA(); diff --git a/src/tests/backend/game/test_game.h b/src/tests/backend/game/test_game.h index e1daf1d6..178a39a0 100644 --- a/src/tests/backend/game/test_game.h +++ b/src/tests/backend/game/test_game.h @@ -133,33 +133,8 @@ TEST_F(Game, Init) { ASSERT_FALSE(boost::filesystem::exists(loot::g_path_local / game.FolderName())); EXPECT_THROW(game.Init(false), loot::error); EXPECT_FALSE(boost::filesystem::exists(loot::g_path_local / game.FolderName())); - EXPECT_FALSE(game.IsPluginActive("Skyrim.esm")); - EXPECT_FALSE(game.IsPluginActive("skyrim.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Master Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Plugin Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Plugin Dependent.esp")); game = loot::Game(loot::Game::tes5).SetGamePath(dataPath.parent_path()); - EXPECT_FALSE(game.IsPluginActive("Skyrim.esm")); - EXPECT_FALSE(game.IsPluginActive("skyrim.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Master Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Plugin Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Plugin Dependent.esp")); - EXPECT_NO_THROW(game.Init(false, localPath)); EXPECT_FALSE(boost::filesystem::exists(loot::g_path_local / game.FolderName())); EXPECT_TRUE(game.IsPluginActive("Skyrim.esm")); @@ -200,32 +175,6 @@ TEST_F(Game, Init) { #endif } -TEST_F(Game, RefreshActivePluginsList) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - - // Throw because the load order handler hasn't been initialised. - EXPECT_THROW(game.RefreshActivePluginsList(), loot::error); - - // Calling Init calls RefreshActivePluginsList, so clear it before testing - // separately. - game.Init(false, localPath); - EXPECT_NO_THROW(game.ClearCache()); - EXPECT_NO_THROW(game.RefreshActivePluginsList()); - EXPECT_TRUE(game.IsPluginActive("Skyrim.esm")); - EXPECT_TRUE(game.IsPluginActive("skyrim.esm")); - EXPECT_TRUE(game.IsPluginActive("Blank.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Master Dependent.esm")); - EXPECT_FALSE(game.IsPluginActive("Blank.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Master Dependent.esp")); - EXPECT_TRUE(game.IsPluginActive("Blank - Different Master Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Plugin Dependent.esp")); - EXPECT_FALSE(game.IsPluginActive("Blank - Different Plugin Dependent.esp")); -} - TEST_F(Game, RedatePlugins) { loot::Game game(loot::Game::tes5); game.SetGamePath(dataPath.parent_path()); diff --git a/src/tests/backend/game/test_game_cache.h b/src/tests/backend/game/test_game_cache.h index 56fd00a7..0fad8384 100644 --- a/src/tests/backend/game/test_game_cache.h +++ b/src/tests/backend/game/test_game_cache.h @@ -36,11 +36,9 @@ TEST_F(GameCache, Constructors) { std::unordered_set plugins({"skyrim.esm"}); EXPECT_NO_THROW(cache.CacheCondition("True Condition", true)); - EXPECT_NO_THROW(cache.CacheActivePlugins(plugins)); loot::GameCache cache2(cache); EXPECT_EQ(std::make_pair(true, true), cache2.GetCachedCondition("true Condition")); - EXPECT_TRUE(cache2.IsPluginActive("Skyrim.esm")); } TEST_F(GameCache, AssignmentOperator) { @@ -48,11 +46,9 @@ TEST_F(GameCache, AssignmentOperator) { std::unordered_set plugins({"skyrim.esm"}); EXPECT_NO_THROW(cache.CacheCondition("True Condition", true)); - EXPECT_NO_THROW(cache.CacheActivePlugins(plugins)); loot::GameCache cache2 = cache; EXPECT_EQ(std::make_pair(true, true), cache2.GetCachedCondition("true Condition")); - EXPECT_TRUE(cache2.IsPluginActive("Skyrim.esm")); } TEST_F(GameCache, CacheCondition) { @@ -67,19 +63,6 @@ TEST_F(GameCache, CacheCondition) { EXPECT_EQ(std::make_pair(false, false), cache.GetCachedCondition("false missing Condition")); } -TEST_F(GameCache, CacheActivePlugins) { - loot::GameCache cache; - std::unordered_set plugins({ - "skyrim.esm", - "blank.esp" - }); - EXPECT_NO_THROW(cache.CacheActivePlugins(plugins)); - - EXPECT_TRUE(cache.IsPluginActive("Skyrim.esm")); - EXPECT_TRUE(cache.IsPluginActive("Blank.esp")); - EXPECT_FALSE(cache.IsPluginActive("Blank.missing.esp")); -} - TEST_F(GameCache, ClearCache) {} #endif diff --git a/src/tests/backend/game/test_load_order_handler.h b/src/tests/backend/game/test_load_order_handler.h index eb3ee5f1..b97f1d28 100644 --- a/src/tests/backend/game/test_load_order_handler.h +++ b/src/tests/backend/game/test_load_order_handler.h @@ -66,19 +66,19 @@ TEST_F(LoadOrderHandler, Init) { EXPECT_NO_THROW(loh.Init(game, localPath)); } -TEST_F(LoadOrderHandler, GetActivePlugins) { +TEST_F(LoadOrderHandler, IsPluginActive) { loot::LoadOrderHandler loh; loot::GameSettings game(loot::GameSettings::tes5); game.SetGamePath(dataPath.parent_path()); + + EXPECT_THROW(loh.IsPluginActive("Skyrim.esm"), loot::error); + ASSERT_NO_THROW(loh.Init(game, localPath)); - std::unordered_set expected({ - "skyrim.esm", - "blank.esm", - "blank - different master dependent.esp", - }); - - EXPECT_EQ(expected, loh.GetActivePlugins()); + EXPECT_TRUE(loh.IsPluginActive("Skyrim.esm")); + EXPECT_TRUE(loh.IsPluginActive("Blank.esm")); + EXPECT_TRUE(loh.IsPluginActive("Blank - Different Master Dependent.esp")); + EXPECT_FALSE(loh.IsPluginActive("Blank.esp")); } TEST_F(LoadOrderHandler, GetLoadOrder) { diff --git a/src/tests/backend/plugin/test_plugin.h b/src/tests/backend/plugin/test_plugin.h index 45fac1ef..aa27a8a2 100644 --- a/src/tests/backend/plugin/test_plugin.h +++ b/src/tests/backend/plugin/test_plugin.h @@ -116,11 +116,11 @@ TEST_F(Plugin, IsActive) { game.SetGamePath(dataPath.parent_path()); ASSERT_NO_THROW(game.Init(false, localPath)); - loot::Plugin plugin("Blank.esm"); - EXPECT_TRUE(plugin.IsActive(game)); + loot::Plugin plugin(game, "Blank.esm", true); + EXPECT_TRUE(plugin.IsActive()); - plugin = loot::Plugin("Blank.esp"); - EXPECT_FALSE(plugin.IsActive(game)); + plugin = loot::Plugin(game, "Blank.esp", true); + EXPECT_FALSE(plugin.IsActive()); } TEST_F(Plugin, EqualityOperator) { From 038587c3e2944637127b08489d2337840b318c07 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 7 Dec 2015 14:57:41 +0000 Subject: [PATCH 3/8] Update libloadorder requirement Use the v7.0.0 release, which brings performance improvements, bug fixes and thread safety. --- README.md | 4 ++-- scripts/install-step.travis.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1cdf4a43..90ea02e9 100644 --- a/README.md +++ b/README.md @@ -33,8 +33,8 @@ LOOT requires the following C/C++ libraries (version numbers used in latest deve * [Google Test](https://github.com/google/googletest) v1.7: Required to build the tests, but not the API or the GUI. * [Libespm](http://github.com/WrinklyNinja/libespm) v2.5.0 * [Libgit2](http://libgit2.github.com/) v0.23.4 -* [Libloadorder](http://github.com/WrinklyNinja/libloadorder) revision 3a7d694 -* [Pseudosem](http://github.com/WrinklyNinja/pseudosem): v1.0.1 +* [Libloadorder](http://github.com/WrinklyNinja/libloadorder) v7.0.0 +* [Pseudosem](http://github.com/WrinklyNinja/pseudosem) v1.0.1 * [yaml-cpp](http://github.com/WrinklyNinja/yaml-cpp): Use the `patched-for-loot` branch. In addition, LOOT's UI relies on the web libraries below, which can be fetched by running `bower install` from the repository root. diff --git a/scripts/install-step.travis.sh b/scripts/install-step.travis.sh index 99686965..43e937f8 100644 --- a/scripts/install-step.travis.sh +++ b/scripts/install-step.travis.sh @@ -25,8 +25,8 @@ make git2 cd ../.. # Build libloadorder -wget https://github.com/WrinklyNinja/libloadorder/archive/3a7d694e2eab9957b745fe828da1ed00f537c989.tar.gz -O - | tar -xz -mv libloadorder-3a7d694e2eab9957b745fe828da1ed00f537c989 libloadorder +wget https://github.com/WrinklyNinja/libloadorder/archive/7.0.0.tar.gz -O - | tar -xz +mv libloadorder-7.0.0 libloadorder mkdir libloadorder/build && cd libloadorder/build cmake .. -DPROJECT_ARCH=64 -DPROJECT_STATIC_RUNTIME=OFF -DBUILD_SHARED_LIBS=OFF -DGTEST_ROOT=../gtest-1.7.0 make loadorder64 From 9d8d75615b8b8858f43bf140eac2187512555de4 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 6 Nov 2015 08:08:51 +0000 Subject: [PATCH 4/8] Remove Plugin(string) constructor Plugin objects are supposed to hold data for actual files, and having a name but no loaded data is a state that they should never be in. --- src/backend/game/game.cpp | 76 ++++++++++++------ src/backend/game/game.h | 6 ++ src/backend/plugin/plugin.cpp | 10 --- src/backend/plugin/plugin.h | 1 - src/tests/backend/plugin/test_plugin.h | 107 ++++++++----------------- 5 files changed, 90 insertions(+), 110 deletions(-) diff --git a/src/backend/game/game.cpp b/src/backend/game/game.cpp index 4a6334fb..67fea97b 100644 --- a/src/backend/game/game.cpp +++ b/src/backend/game/game.cpp @@ -41,7 +41,7 @@ namespace lc = boost::locale; namespace loot { Game::Game() : _pluginsFullyLoaded(false) {} - Game::Game(const GameSettings& gameSettings) : GameSettings(gameSettings.Id(), gameSettings.FolderName()), _pluginsFullyLoaded(false) { + Game::Game(const GameSettings& gameSettings) : GameSettings(gameSettings), _pluginsFullyLoaded(false) { this->SetName(gameSettings.Name()) .SetMaster(gameSettings.Master()) .SetRepoURL(gameSettings.RepoURL()) @@ -50,8 +50,31 @@ namespace loot { .SetRegistryKey(gameSettings.RegistryKey()); } + Game::Game(const Game& game) : + GameSettings(game), + LoadOrderHandler(game), + GameCache(game), + masterlist(game.masterlist), + userlist(game.userlist), + plugins(game.plugins), + _pluginsFullyLoaded(game.ArePluginsFullyLoaded()) {} + Game::Game(const unsigned int gameCode, const std::string& folder) : GameSettings(gameCode, folder), _pluginsFullyLoaded(false) {} + Game& Game::operator= (const Game& game) { + if (&game != this) { + GameSettings::operator=(game); + LoadOrderHandler::operator=(game); + GameCache::operator=(game); + + masterlist = game.masterlist; + userlist = game.userlist; + plugins = game.plugins; + _pluginsFullyLoaded = game.ArePluginsFullyLoaded(); + } + return *this; + } + void Game::Init(bool createFolder, const boost::filesystem::path& gameLocalAppData) { if (Id() != Game::tes4 && Id() != Game::tes5 && Id() != Game::fo3 && Id() != Game::fonv && Id() != Game::fo4) { throw error(error::invalid_args, lc::translate("Invalid game ID supplied.").str()); @@ -118,29 +141,35 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Scanning for plugins in " << this->DataPath(); for (fs::directory_iterator it(this->DataPath()); it != fs::directory_iterator(); ++it) { if (fs::is_regular_file(it->status()) && Plugin::IsValid(it->path().filename().string(), *this)) { - Plugin temp(it->path().filename().string()); - BOOST_LOG_TRIVIAL(info) << "Found plugin: " << temp.Name(); + string name = it->path().filename().string(); + BOOST_LOG_TRIVIAL(info) << "Found plugin: " << name; + + // Trim .ghost extension if present. + if (boost::iends_with(name, ".ghost")) + name = name.substr(0, name.length() - 6); uintmax_t fileSize = fs::file_size(it->path()); meanFileSize += fileSize; - //Insert the lowercased name as a key for case-insensitive matching. - string name = boost::locale::to_lower(temp.Name()); - plugins.insert(pair(name, temp)); - sizeMap.insert(pair(fileSize, name)); + sizeMap.emplace(fileSize, name); } } meanFileSize /= sizeMap.size(); //Rounding error, but not important. + // Reserve space in the plugins unordered_map to speed up inserting + // later and more importantly avoid any inserts invalidating + // iterators. + plugins.reserve(sizeMap.size()); + // Get the number of threads to use. // hardware_concurrency() may be zero, if so then use only one thread. - size_t threadsToUse = std::min((size_t)thread::hardware_concurrency(), plugins.size()); + size_t threadsToUse = std::min((size_t)thread::hardware_concurrency(), sizeMap.size()); threadsToUse = std::max(threadsToUse, (size_t)1); // Divide the plugins up by thread. - unsigned int pluginsPerThread = ceil((double)plugins.size() / threadsToUse); - vector::iterator>> pluginGroups(threadsToUse); - BOOST_LOG_TRIVIAL(info) << "Loading " << plugins.size() << " plugins using " << threadsToUse << " threads, with up to " << pluginsPerThread << " plugins per thread."; + unsigned int pluginsPerThread = ceil((double)sizeMap.size() / threadsToUse); + vector> pluginGroups(threadsToUse); + BOOST_LOG_TRIVIAL(info) << "Loading " << sizeMap.size() << " plugins using " << threadsToUse << " threads, with up to " << pluginsPerThread << " plugins per thread."; // The plugins should be split between the threads so that the data // load is as evenly spread as possible. @@ -149,7 +178,7 @@ namespace loot { if (currentGroup == threadsToUse) currentGroup = 0; BOOST_LOG_TRIVIAL(trace) << "Adding plugin " << plugin.second << " to loading group " << currentGroup; - pluginGroups[currentGroup].push_back(plugins.find(plugin.second)); + pluginGroups[currentGroup].push_back(plugin.second); ++currentGroup; } @@ -157,19 +186,11 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Starting plugin loading."; vector threads; while (threads.size() < threadsToUse) { - vector::iterator>& pluginGroup = pluginGroups[threads.size()]; - threads.push_back(thread([this, &pluginGroup, headersOnly]() { - for (auto it : pluginGroup) { - BOOST_LOG_TRIVIAL(trace) << "Loading " << it->second.Name(); - try { - it->second = Plugin(*this, it->second.Name(), headersOnly); - } - catch (exception &e) { - BOOST_LOG_TRIVIAL(error) << it->second.Name() << ": Exception occurred: " << e.what(); - Plugin p(it->second.Name()); - p.Messages(list(1, Message(Message::error, lc::translate("An exception occurred while loading this plugin. Details:").str() + " " + e.what()))); - it->second = p; - } + vector& pluginGroup = pluginGroups[threads.size()]; + threads.push_back(thread([&]() { + for (auto pluginName : pluginGroup) { + BOOST_LOG_TRIVIAL(trace) << "Loading " << pluginName; + addPlugin(Plugin(*this, pluginName, headersOnly)); } })); } @@ -187,6 +208,11 @@ namespace loot { return _pluginsFullyLoaded; } + void Game::addPlugin(const Plugin&& plugin) { + std::lock_guard lock(mutex); + plugins.emplace(boost::locale::to_lower(plugin.Name()), plugin); + } + std::list ToGames(const std::list& settings) { return list(settings.begin(), settings.end()); } diff --git a/src/backend/game/game.h b/src/backend/game/game.h index 3d242291..92a78638 100644 --- a/src/backend/game/game.h +++ b/src/backend/game/game.h @@ -43,8 +43,11 @@ namespace loot { //Game functions. Game(); //Sets game to LOOT_Game::autodetect, with all other vars being empty. Game(const GameSettings& gameSettings); + Game(const Game& game); Game(const unsigned int baseGameCode, const std::string& lootFolder = ""); + Game& operator= (const Game& game); + void Init(bool createFolder, const boost::filesystem::path& gameLocalAppData = ""); void RedatePlugins(); //Change timestamps to match load order (Skyrim only). @@ -58,6 +61,9 @@ namespace loot { std::unordered_map plugins; //Map so that plugin data can be edited. private: bool _pluginsFullyLoaded; + std::mutex mutex; + + void addPlugin(const Plugin&& plugin); }; std::list ToGames(const std::list& settings); diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index b38494c9..27b45b3a 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -37,16 +37,6 @@ using namespace std; using libespm::FormId; namespace loot { - // TODO: Remove the name-only constructor. - Plugin::Plugin(const std::string& n) : - PluginMetadata(n), - libespm::Plugin(libespm::GameId::SKYRIM), - _isEmpty(true), - _isActive(false), - _loadsBsa(false), - crc(0), - numOverrideRecords(0) {} - Plugin::Plugin(const Game& game, const std::string& name, const bool headerOnly) : PluginMetadata(name), libespm::Plugin(game.LibespmId()), diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index 747068d7..36a0ffe0 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -41,7 +41,6 @@ namespace loot { class Plugin : public PluginMetadata, private libespm::Plugin { public: - Plugin(const std::string& name); Plugin(const Game& game, const std::string& name, const bool headerOnly); using libespm::Plugin::getDescription; diff --git a/src/tests/backend/plugin/test_plugin.h b/src/tests/backend/plugin/test_plugin.h index aa27a8a2..b6144330 100644 --- a/src/tests/backend/plugin/test_plugin.h +++ b/src/tests/backend/plugin/test_plugin.h @@ -28,23 +28,21 @@ along with LOOT. If not, see #include "backend/plugin/plugin.h" #include "tests/fixtures.h" -class Plugin : public SkyrimTest {}; +class Plugin : public SkyrimTest { +protected: + inline virtual void SetUp() { + SkyrimTest::SetUp(); + + game = loot::Game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + } + + loot::Game game; +}; TEST_F(Plugin, ConstructorsAndDataAccess) { - loot::Plugin plugin("Blank.esm"); - EXPECT_EQ("Blank.esm", plugin.Name()); - EXPECT_TRUE(plugin.getFormIds().empty()); - EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_FALSE(plugin.isMasterFile()); - EXPECT_TRUE(plugin.IsEmpty()); - EXPECT_EQ("", plugin.getDescription()); - EXPECT_EQ(0, plugin.Crc()); - - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - - plugin = loot::Plugin(game, "Blank.esm", true); + loot::Plugin plugin(game, "Blank.esm", true); EXPECT_EQ("Blank.esm", plugin.Name()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); @@ -91,31 +89,18 @@ TEST_F(Plugin, ConstructorsAndDataAccess) { } TEST_F(Plugin, LoadsBSA) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - EXPECT_FALSE(loot::Plugin(game, "Blank - Different.esm", true).LoadsBSA()); EXPECT_FALSE(loot::Plugin(game, "Blank\\.esm", true).LoadsBSA()); - EXPECT_FALSE(loot::Plugin("Blank.esm").LoadsBSA()); EXPECT_TRUE(loot::Plugin(game, "Blank.esm", true).LoadsBSA()); } TEST_F(Plugin, IsValid) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - EXPECT_TRUE(loot::Plugin::IsValid("Blank.esm", game)); EXPECT_FALSE(loot::Plugin::IsValid("NotAPlugin.esm", game)); EXPECT_FALSE(loot::Plugin::IsValid("EmptyFile.esm", game)); } TEST_F(Plugin, IsActive) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - loot::Plugin plugin(game, "Blank.esm", true); EXPECT_TRUE(plugin.IsActive()); @@ -124,76 +109,62 @@ TEST_F(Plugin, IsActive) { } TEST_F(Plugin, EqualityOperator) { - loot::Plugin plugin1("Blank.esm"); - loot::Plugin plugin2("blank.esm"); + loot::Plugin plugin1(game, "Blank.esm", true); + loot::Plugin plugin2(game, "blank.esm", true); EXPECT_TRUE(plugin1 == plugin2); EXPECT_TRUE(plugin2 == plugin1); - plugin1 = loot::Plugin("Blank.esm"); - plugin2 = loot::Plugin("Blank.esp"); + plugin2 = loot::Plugin(game, "Blank.esp", true); EXPECT_FALSE(plugin1 == plugin2); EXPECT_FALSE(plugin2 == plugin1); - plugin1 = loot::Plugin("Blank.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin2 = loot::Plugin(game, "Blan.\\.esm", true); EXPECT_TRUE(plugin1 == plugin2); EXPECT_TRUE(plugin2 == plugin1); - plugin1 = loot::Plugin("Blan.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan.esm", true); EXPECT_FALSE(plugin1 == plugin2); EXPECT_FALSE(plugin2 == plugin1); - plugin1 = loot::Plugin("Blan.\\.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan.\\.esm", true); EXPECT_TRUE(plugin1 == plugin2); EXPECT_TRUE(plugin2 == plugin1); - plugin1 = loot::Plugin("Blan(k|p).esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan(k|p).esm", true); EXPECT_FALSE(plugin1 == plugin2); EXPECT_FALSE(plugin2 == plugin1); } TEST_F(Plugin, InequalityOperator) { - loot::Plugin plugin1("Blank.esm"); - loot::Plugin plugin2("blank.esm"); + loot::Plugin plugin1(game, "Blank.esm", true); + loot::Plugin plugin2(game, "blank.esm", true); EXPECT_FALSE(plugin1 != plugin2); EXPECT_FALSE(plugin2 != plugin1); - plugin1 = loot::Plugin("Blank.esm"); - plugin2 = loot::Plugin("Blank.esp"); + plugin2 = loot::Plugin(game, "Blank.esp", true); EXPECT_TRUE(plugin1 != plugin2); EXPECT_TRUE(plugin2 != plugin1); - plugin1 = loot::Plugin("Blank.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin2 = loot::Plugin(game, "Blan.\\.esm", true); EXPECT_FALSE(plugin1 != plugin2); EXPECT_FALSE(plugin2 != plugin1); - plugin1 = loot::Plugin("Blan.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan.esm", true); EXPECT_TRUE(plugin1 != plugin2); EXPECT_TRUE(plugin2 != plugin1); - plugin1 = loot::Plugin("Blan.\\.esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan.\\.esm", true); EXPECT_FALSE(plugin1 != plugin2); EXPECT_FALSE(plugin2 != plugin1); - plugin1 = loot::Plugin("Blan(k|p).esm"); - plugin2 = loot::Plugin("Blan.\\.esm"); + plugin1 = loot::Plugin(game, "Blan(k|p).esm", true); EXPECT_TRUE(plugin1 != plugin2); EXPECT_TRUE(plugin2 != plugin1); } TEST_F(Plugin, DoFormIDsOverlap) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - - loot::Plugin plugin1("Blank.esm"); - loot::Plugin plugin2("blank.esm"); + loot::Plugin plugin1(game, "Blank.esm", true); + loot::Plugin plugin2(game, "blank.esm", true); EXPECT_FALSE(plugin1.DoFormIDsOverlap(plugin2)); EXPECT_FALSE(plugin2.DoFormIDsOverlap(plugin1)); @@ -214,12 +185,8 @@ TEST_F(Plugin, DoFormIDsOverlap) { } TEST_F(Plugin, OverlapFormIDs) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - - loot::Plugin plugin1("Blank.esm"); - loot::Plugin plugin2("blank.esm"); + loot::Plugin plugin1(game, "Blank.esm", true); + loot::Plugin plugin2(game, "blank.esm", true); EXPECT_TRUE(plugin1.OverlapFormIDs(plugin2).empty()); EXPECT_TRUE(plugin2.OverlapFormIDs(plugin1).empty()); @@ -250,15 +217,7 @@ TEST_F(Plugin, OverlapFormIDs) { } TEST_F(Plugin, CheckInstallValidity) { - loot::Game game(loot::Game::tes5); - game.SetGamePath(dataPath.parent_path()); - ASSERT_NO_THROW(game.Init(false, localPath)); - - loot::Plugin plugin("Blank.esm"); - EXPECT_FALSE(plugin.CheckInstallValidity(game)); - EXPECT_TRUE(plugin.Messages().empty()); - - plugin = loot::Plugin(game, "Blank.esm", false); + loot::Plugin plugin(game, "Blank.esm", false); plugin.Reqs({ loot::File("Blank.missing.esm"), loot::File("Blank.esp"), @@ -286,7 +245,7 @@ TEST_F(Plugin, CheckInstallValidity) { loot::Message(loot::Message::error, "This plugin requires \"Blank - Different.esm\" to be active, but it is inactive."), }), plugin.Messages()); - plugin = loot::Plugin("Blank - Different Master Dependent.esp"); + plugin = loot::Plugin(game, "Blank - Different Master Dependent.esp", false); plugin.Tags({loot::Tag("Filter")}); EXPECT_FALSE(plugin.CheckInstallValidity(game)); EXPECT_TRUE(plugin.Messages().empty()); From 4205262fc4cc152d87f61587b9355ba4b4d12a80 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 7 Dec 2015 15:47:58 +0000 Subject: [PATCH 5/8] Fix incorrect variable usage --- src/backend/plugin/plugin.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index 27b45b3a..63c9af7b 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -89,7 +89,7 @@ namespace loot { } } // Get whether the plugin is active or not. - _isActive = game.IsPluginActive(name); + _isActive = game.IsPluginActive(Name()); // Get whether the plugin loads a BSA or not. if (game.Id() == Game::tes5) { @@ -108,7 +108,7 @@ namespace loot { } } catch (std::exception& e) { - BOOST_LOG_TRIVIAL(error) << "Cannot read plugin file \"" << Name() << "\". Details: " << e.what(); + BOOST_LOG_TRIVIAL(error) << "Cannot read plugin file \"" << name << "\". Details: " << e.what(); messages.push_back(loot::Message(loot::Message::error, (boost::format(boost::locale::translate("Cannot read \"%1%\". Details: %2%")) % name % e.what()).str())); } @@ -117,8 +117,6 @@ namespace loot { bool Plugin::DoFormIDsOverlap(const Plugin& plugin) const { //Basically std::set_intersection except with an early exit instead of an append to results. - //BOOST_LOG_TRIVIAL(trace) << "Checking for FormID overlap between \"" << name << "\" and \"" << plugin.Name() << "\"."; - set formIds(getFormIds()); set otherFormIds(plugin.getFormIds()); auto i = begin(formIds); From eb07ad579ddf2e9ba599944725136c5660d71efc Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 8 Dec 2015 14:11:04 +0000 Subject: [PATCH 6/8] Fix efficiency of getting plugin active state Use the existing Plugin object if present, or query libloadorder otherwise, instead of always loading a Plugin object.` --- src/backend/game/game.cpp | 8 +++ src/backend/game/game.h | 4 ++ src/backend/metadata/condition_grammar.h | 2 +- src/backend/plugin/plugin.cpp | 4 +- src/gui/handler.cpp | 6 +- src/tests/backend/game/test_game.h | 75 ++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/backend/game/game.cpp b/src/backend/game/game.cpp index 67fea97b..930ae034 100644 --- a/src/backend/game/game.cpp +++ b/src/backend/game/game.cpp @@ -208,6 +208,14 @@ namespace loot { return _pluginsFullyLoaded; } + bool Game::IsPluginActive(const std::string& pluginName) const { + auto it = plugins.find(boost::locale::to_lower(pluginName)); + if (it != end(plugins)) + return it->second.IsActive(); + else + return LoadOrderHandler::IsPluginActive(pluginName); + } + void Game::addPlugin(const Plugin&& plugin) { std::lock_guard lock(mutex); plugins.emplace(boost::locale::to_lower(plugin.Name()), plugin); diff --git a/src/backend/game/game.h b/src/backend/game/game.h index 92a78638..4b3afd07 100644 --- a/src/backend/game/game.h +++ b/src/backend/game/game.h @@ -55,6 +55,10 @@ namespace loot { void LoadPlugins(bool headersOnly); //Loads all installed plugins. bool ArePluginsFullyLoaded() const; // Checks if the game's plugins have already been loaded. + // Check if the plugin is active by using the cached value if + // available, and otherwise asking the load order handler. + bool IsPluginActive(const std::string& pluginName) const; + //Plugin data and metadata lists. Masterlist masterlist; MetadataList userlist; diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index 39c46354..3f29628a 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -363,7 +363,7 @@ namespace loot { if (file == "LOOT") result = false; else - result = Plugin(*_game, file, true).IsActive(); + result = _game->IsPluginActive(file); BOOST_LOG_TRIVIAL(trace) << "Active check result: " << result; } diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index 63c9af7b..c94a1318 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -205,7 +205,7 @@ namespace loot { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" requires \"" << master << "\", but it is missing."; messages.push_back(Message(Message::error, (boost::format(boost::locale::translate("This plugin requires \"%1%\" to be installed, but it is missing.")) % master).str())); } - else if (!Plugin(game, master, true).IsActive()) { + else if (!game.IsPluginActive(master)) { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" requires \"" << master << "\", but it is inactive."; messages.push_back(Message(Message::error, (boost::format(boost::locale::translate("This plugin requires \"%1%\" to be active, but it is inactive.")) % master).str())); } @@ -219,7 +219,7 @@ namespace loot { } } for (const auto &inc : Incs()) { - if (pluginExists(game, inc.Name()) && Plugin(game, inc.Name(), true).IsActive()) { + if (pluginExists(game, inc.Name()) && game.IsPluginActive(inc.Name())) { BOOST_LOG_TRIVIAL(error) << "\"" << Name() << "\" is incompatible with \"" << inc.Name() << "\", but both are present."; messages.push_back(loot::Message(Message::error, (boost::format(boost::locale::translate("This plugin is incompatible with \"%1%\", but both are present.")) % inc.Name()).str())); } diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 6c3642cb..e30c7a2e 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -354,15 +354,15 @@ namespace loot { decLength = 2; } size_t i = 0; - for (const auto& plugin : plugins) { - if (Plugin(_lootState.CurrentGame(), plugin, true).IsActive()) { + for (const auto& pluginName : plugins) { + if (_lootState.CurrentGame().IsPluginActive(pluginName)) { ss << setw(decLength) << i << " " << hex << setw(2) << i << dec << " "; ++i; } else { ss << setw(decLength + 4) << " "; } - ss << plugin << "\r\n"; + ss << pluginName << "\r\n"; } CopyToClipboard(ss.str()); callback->Success(""); diff --git a/src/tests/backend/game/test_game.h b/src/tests/backend/game/test_game.h index 178a39a0..3a1157d7 100644 --- a/src/tests/backend/game/test_game.h +++ b/src/tests/backend/game/test_game.h @@ -580,6 +580,81 @@ TEST_F(Game, ArePluginsFullyLoaded) { EXPECT_TRUE(game.ArePluginsFullyLoaded()); } +TEST_F(Game, shouldThrowIfCheckingIfPluginThatIsntLoadedIsActiveAndGameHasNotBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + + EXPECT_ANY_THROW(game.IsPluginActive("Blank.esm")); +} + +TEST_F(Game, shouldShowBlankEsmAsActiveIfItHasNotBeenLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + + EXPECT_TRUE(game.IsPluginActive("Blank.esm")); +} + +TEST_F(Game, shouldShowBlankEspAsInctiveIfItHasNotBeenLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + + EXPECT_FALSE(game.IsPluginActive("Blank.esp")); +} + +TEST_F(Game, shouldShowBlankEsmAsInactiveIfItsHeaderHasBeenLoadedAndGameHasNotBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.LoadPlugins(true)); + + EXPECT_FALSE(game.IsPluginActive("Blank.esm")); +} + +TEST_F(Game, shouldShowBlankEspAsActiveIfItsHeaderHasBeenLoadedAndGameHasNotBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.LoadPlugins(true)); + + EXPECT_FALSE(game.IsPluginActive("Blank.esp")); +} + +TEST_F(Game, shouldShowBlankEsmAsActiveIfItsHeaderHasBeenLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + ASSERT_NO_THROW(game.LoadPlugins(true)); + + EXPECT_TRUE(game.IsPluginActive("Blank.esm")); +} + +TEST_F(Game, shouldShowBlankEspAsActiveIfItsHeaderHasBeenLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + ASSERT_NO_THROW(game.LoadPlugins(true)); + + EXPECT_FALSE(game.IsPluginActive("Blank.esp")); +} + +TEST_F(Game, shouldShowBlankEsmAsActiveIfItHasBeenFullyLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + ASSERT_NO_THROW(game.LoadPlugins(false)); + + EXPECT_TRUE(game.IsPluginActive("Blank.esm")); +} + +TEST_F(Game, shouldShowBlankEspAsActiveIfItHasBeenFullyLoadedAndTheGameHasBeenInitialised) { + loot::Game game(loot::Game::tes5); + game.SetGamePath(dataPath.parent_path()); + ASSERT_NO_THROW(game.Init(false, localPath)); + ASSERT_NO_THROW(game.LoadPlugins(false)); + + EXPECT_FALSE(game.IsPluginActive("Blank.esp")); +} + TEST(ToGames, EmptySettings) { EXPECT_EQ(std::list(), loot::ToGames(std::list())); } From 9224e3015332ecc1b6a8d1d231c7368bb6d51670 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 8 Dec 2015 16:12:08 +0000 Subject: [PATCH 7/8] Make MetadataList.messages private --- src/backend/metadata_list.cpp | 8 ++++++++ src/backend/metadata_list.h | 5 ++++- src/gui/handler.cpp | 16 +++++++++------- src/tests/backend/test_metadata_list.h | 12 ++++++------ 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/backend/metadata_list.cpp b/src/backend/metadata_list.cpp index 29e20b8e..77dccee3 100644 --- a/src/backend/metadata_list.cpp +++ b/src/backend/metadata_list.cpp @@ -92,6 +92,10 @@ namespace loot { return pluginList; } + std::list MetadataList::Messages() const { + return messages; + } + // Merges multiple matching regex entries if any are found. PluginMetadata MetadataList::FindPlugin(const PluginMetadata& plugin) const { PluginMetadata match(plugin.Name()); @@ -132,6 +136,10 @@ namespace loot { } } + void MetadataList::AppendMessage(const Message& message) { + messages.push_back(message); + } + void MetadataList::EvalAllConditions(Game& game, const unsigned int language) { unordered_set replacementSet; for (auto &plugin : plugins) { diff --git a/src/backend/metadata_list.h b/src/backend/metadata_list.h index 16ab26ff..9f2877fe 100644 --- a/src/backend/metadata_list.h +++ b/src/backend/metadata_list.h @@ -53,6 +53,7 @@ namespace loot { void clear(); std::list Plugins() const; + std::list Messages() const; // Merges multiple matching regex entries if any are found. PluginMetadata FindPlugin(const PluginMetadata& plugin) const; @@ -62,13 +63,15 @@ namespace loot { // be required for other plugins. void ErasePlugin(const PluginMetadata& plugin); + void AppendMessage(const Message& message); + // Eval plugin conditions. void EvalAllConditions(Game& game, const unsigned int language); - std::list messages; protected: std::unordered_set plugins; std::list regexPlugins; + std::list messages; }; } diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index e30c7a2e..9269d72e 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -797,12 +797,14 @@ namespace loot { //Evaluate any conditions in the global messages. BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; list messages = parsingErrors; + auto metadataListMessages = _lootState.CurrentGame().masterlist.Messages(); + messages.insert(end(messages), + begin(metadataListMessages), + end(metadataListMessages)); + metadataListMessages = _lootState.CurrentGame().userlist.Messages(); messages.insert(messages.end(), - _lootState.CurrentGame().masterlist.messages.begin(), - _lootState.CurrentGame().masterlist.messages.end()); - messages.insert(messages.end(), - _lootState.CurrentGame().userlist.messages.begin(), - _lootState.CurrentGame().userlist.messages.end()); + begin(metadataListMessages), + end(metadataListMessages)); try { list::iterator it = messages.begin(); while (it != messages.end()) { @@ -855,7 +857,7 @@ namespace loot { // There was a parsing error, but roll-back was successful, so the process // should still complete. - _lootState.CurrentGame().masterlist.messages.push_back(Message(Message::error, e.what())); + _lootState.CurrentGame().masterlist.AppendMessage(Message(Message::error, e.what())); wasChanged = true; } else { @@ -915,7 +917,7 @@ namespace loot { //Evaluate any conditions in the global messages. BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; - list messages = _lootState.CurrentGame().masterlist.messages; + list messages = _lootState.CurrentGame().masterlist.Messages(); try { list::iterator it = messages.begin(); while (it != messages.end()) { diff --git a/src/tests/backend/test_metadata_list.h b/src/tests/backend/test_metadata_list.h index 17332ec6..afb531b8 100644 --- a/src/tests/backend/test_metadata_list.h +++ b/src/tests/backend/test_metadata_list.h @@ -73,7 +73,7 @@ TEST_F(MetadataList, Load) { EXPECT_NO_THROW(ml.Load(metadataPath)); EXPECT_EQ(std::list({ loot::Message(loot::Message::say, "A global message."), - }), ml.messages); + }), ml.Messages()); // Non-regex plugins can be outputted in any order, and regex entries can // match each other, so convert the list to a set of strings for @@ -92,14 +92,14 @@ TEST_F(MetadataList, Load) { }), names); EXPECT_ANY_THROW(ml.Load("NotAPlugin.esm")); - EXPECT_TRUE(ml.messages.empty()); + EXPECT_TRUE(ml.Messages().empty()); EXPECT_TRUE(ml.Plugins().empty()); // Fill the list again. ASSERT_NO_THROW(ml.Load(metadataPath)); EXPECT_ANY_THROW(ml.Load("Blank.missing.esm")); - EXPECT_TRUE(ml.messages.empty()); + EXPECT_TRUE(ml.Messages().empty()); EXPECT_TRUE(ml.Plugins().empty()); } @@ -120,7 +120,7 @@ TEST_F(MetadataList, Save) { EXPECT_EQ(std::list({ loot::Message(loot::Message::say, "A global message."), - }), ml.messages); + }), ml.Messages()); // Non-regex plugins can be outputted in any order, and regex entries can // match each other, so convert the list to a set of strings for @@ -142,11 +142,11 @@ TEST_F(MetadataList, Save) { TEST_F(MetadataList, clear) { loot::MetadataList ml; ASSERT_NO_THROW(ml.Load(metadataPath)); - ASSERT_FALSE(ml.messages.empty()); + ASSERT_FALSE(ml.Messages().empty()); ASSERT_FALSE(ml.Plugins().empty()); ml.clear(); - EXPECT_TRUE(ml.messages.empty()); + EXPECT_TRUE(ml.Messages().empty()); EXPECT_TRUE(ml.Plugins().empty()); } From 14a5076b6592a612d80ad23e1023c5112e75b14e Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 8 Dec 2015 18:18:24 +0000 Subject: [PATCH 8/8] Move Game data members into GameCache Also make them private. --- src/api/api.cpp | 30 +++--- src/backend/game/game.cpp | 44 ++------- src/backend/game/game.h | 15 --- src/backend/game/game_cache.cpp | 46 ++++++++- src/backend/game/game_cache.h | 19 +++- src/backend/metadata/condition_grammar.h | 7 +- src/backend/metadata/plugin_dirty_info.cpp | 7 +- src/backend/plugin/plugin.cpp | 4 + src/backend/plugin/plugin.h | 2 + src/backend/plugin_sorter.cpp | 13 +-- src/gui/handler.cpp | 108 +++++++++++---------- src/tests/backend/game/test_game.h | 92 +++++++++--------- src/tests/backend/test_plugin_sorter.h | 8 +- 13 files changed, 202 insertions(+), 193 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index ca979866..b08f282b 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -363,9 +363,9 @@ LOOT_API unsigned int loot_load_lists(loot_db db, const char * const masterlistP db->extTagMap = nullptr; db->extMessageArray = nullptr; - db->masterlist = temp; + db->GetMasterlist() = temp; db->rawMetadata = temp; - db->userlist = userTemp; + db->GetUserlist() = userTemp; db->rawUserMetadata = userTemp; return loot_ok; @@ -406,8 +406,8 @@ LOOT_API unsigned int loot_eval_lists(loot_db db, const unsigned int language) { catch (loot::error& e) { return c_error(e); } - db->masterlist = temp; - db->userlist = userTemp; + db->GetMasterlist() = temp; + db->GetUserlist() = userTemp; return loot_ok; } @@ -575,12 +575,12 @@ LOOT_API unsigned int loot_get_tag_map(loot_db db, char *** const tagMap, size_t std::set allTags; - for (const auto &plugin : db->masterlist.Plugins()) { + for (const auto &plugin : db->GetMasterlist().Plugins()) { for (const auto &tag : plugin.Tags()) { allTags.insert(tag.Name()); } } - for (const auto &plugin : db->userlist.Plugins()) { + for (const auto &plugin : db->GetUserlist().Plugins()) { for (const auto &tag : plugin.Tags()) { allTags.insert(tag.Name()); } @@ -649,7 +649,7 @@ LOOT_API unsigned int loot_get_plugin_tags(loot_db db, const char * const plugin *numTags_removed = 0; std::set tagsAdded, tagsRemoved; - loot::PluginMetadata p = db->masterlist.FindPlugin(loot::PluginMetadata(plugin)); + loot::PluginMetadata p = db->GetMasterlist().FindPlugin(loot::PluginMetadata(plugin)); for (const auto &tag : p.Tags()) { if (tag.IsAddition()) tagsAdded.insert(tag.Name()); @@ -657,7 +657,7 @@ LOOT_API unsigned int loot_get_plugin_tags(loot_db db, const char * const plugin tagsRemoved.insert(tag.Name()); } - p = db->userlist.FindPlugin(loot::PluginMetadata(plugin)); + p = db->GetUserlist().FindPlugin(loot::PluginMetadata(plugin)); *userlistModified = !p.Tags().empty(); for (const auto &tag : p.Tags()) { *userlistModified = true; @@ -730,10 +730,10 @@ LOOT_API unsigned int loot_get_plugin_messages(loot_db db, const char * const pl *messages = nullptr; *numMessages = 0; - loot::PluginMetadata p = db->masterlist.FindPlugin(loot::PluginMetadata(plugin)); + loot::PluginMetadata p = db->GetMasterlist().FindPlugin(loot::PluginMetadata(plugin)); std::list pluginMessages(p.Messages()); - p = db->userlist.FindPlugin(loot::PluginMetadata(plugin)); + p = db->GetUserlist().FindPlugin(loot::PluginMetadata(plugin)); std::list temp(p.Messages()); pluginMessages.insert(pluginMessages.end(), temp.begin(), temp.end()); @@ -766,8 +766,8 @@ LOOT_API unsigned int loot_get_dirty_info(loot_db db, const char * const plugin, *needsCleaning = loot_needs_cleaning_unknown; // Is there any dirty info? Testing for applicability happens in loot_eval_lists(). - if (!db->masterlist.FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty() - || !db->userlist.FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty()) { + if (!db->GetMasterlist().FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty() + || !db->GetUserlist().FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty()) { *needsCleaning = loot_needs_cleaning_yes; } @@ -775,9 +775,9 @@ LOOT_API unsigned int loot_get_dirty_info(loot_db db, const char * const plugin, // This isn't a very reliable system, because if the lists have been evaluated in some language // other than English, the strings will be in different languages (and the API can't tell what they'd be) // and the strings may be non-standard and begin with something other than "Do not clean." anyway. - std::list messages(db->masterlist.FindPlugin(loot::PluginMetadata(plugin)).Messages()); + std::list messages(db->GetMasterlist().FindPlugin(loot::PluginMetadata(plugin)).Messages()); - std::list temp(db->userlist.FindPlugin(loot::PluginMetadata(plugin)).Messages()); + std::list temp(db->GetUserlist().FindPlugin(loot::PluginMetadata(plugin)).Messages()); messages.insert(messages.end(), temp.begin(), temp.end()); for (const auto& message : messages) { @@ -804,7 +804,7 @@ LOOT_API unsigned int loot_write_minimal_list(loot_db db, const char * const out if (boost::filesystem::exists(outputFile) && !overwrite) return c_error(loot_error_file_write_fail, "Output file exists but overwrite is not set to true."); - loot::Masterlist temp = db->masterlist; + loot::Masterlist temp = db->GetMasterlist(); std::unordered_set minimalPlugins; for (const auto &plugin : temp.Plugins()) { loot::PluginMetadata p(plugin.Name()); diff --git a/src/backend/game/game.cpp b/src/backend/game/game.cpp index 930ae034..d7ae7071 100644 --- a/src/backend/game/game.cpp +++ b/src/backend/game/game.cpp @@ -50,31 +50,8 @@ namespace loot { .SetRegistryKey(gameSettings.RegistryKey()); } - Game::Game(const Game& game) : - GameSettings(game), - LoadOrderHandler(game), - GameCache(game), - masterlist(game.masterlist), - userlist(game.userlist), - plugins(game.plugins), - _pluginsFullyLoaded(game.ArePluginsFullyLoaded()) {} - Game::Game(const unsigned int gameCode, const std::string& folder) : GameSettings(gameCode, folder), _pluginsFullyLoaded(false) {} - Game& Game::operator= (const Game& game) { - if (&game != this) { - GameSettings::operator=(game); - LoadOrderHandler::operator=(game); - GameCache::operator=(game); - - masterlist = game.masterlist; - userlist = game.userlist; - plugins = game.plugins; - _pluginsFullyLoaded = game.ArePluginsFullyLoaded(); - } - return *this; - } - void Game::Init(bool createFolder, const boost::filesystem::path& gameLocalAppData) { if (Id() != Game::tes4 && Id() != Game::tes5 && Id() != Game::fo3 && Id() != Game::fonv && Id() != Game::fo4) { throw error(error::invalid_args, lc::translate("Invalid game ID supplied.").str()); @@ -156,11 +133,6 @@ namespace loot { } meanFileSize /= sizeMap.size(); //Rounding error, but not important. - // Reserve space in the plugins unordered_map to speed up inserting - // later and more importantly avoid any inserts invalidating - // iterators. - plugins.reserve(sizeMap.size()); - // Get the number of threads to use. // hardware_concurrency() may be zero, if so then use only one thread. size_t threadsToUse = std::min((size_t)thread::hardware_concurrency(), sizeMap.size()); @@ -190,7 +162,7 @@ namespace loot { threads.push_back(thread([&]() { for (auto pluginName : pluginGroup) { BOOST_LOG_TRIVIAL(trace) << "Loading " << pluginName; - addPlugin(Plugin(*this, pluginName, headersOnly)); + AddPlugin(Plugin(*this, pluginName, headersOnly)); } })); } @@ -209,16 +181,12 @@ namespace loot { } bool Game::IsPluginActive(const std::string& pluginName) const { - auto it = plugins.find(boost::locale::to_lower(pluginName)); - if (it != end(plugins)) - return it->second.IsActive(); - else + try { + return GetPlugin(pluginName).IsActive(); + } + catch (...) { return LoadOrderHandler::IsPluginActive(pluginName); - } - - void Game::addPlugin(const Plugin&& plugin) { - std::lock_guard lock(mutex); - plugins.emplace(boost::locale::to_lower(plugin.Name()), plugin); + } } std::list ToGames(const std::list& settings) { diff --git a/src/backend/game/game.h b/src/backend/game/game.h index 4b3afd07..43f7cb3a 100644 --- a/src/backend/game/game.h +++ b/src/backend/game/game.h @@ -28,12 +28,8 @@ #include "game_cache.h" #include "game_settings.h" #include "load_order_handler.h" -#include "../plugin/plugin.h" -#include "../metadata_list.h" -#include "../masterlist.h" #include -#include #include @@ -43,11 +39,8 @@ namespace loot { //Game functions. Game(); //Sets game to LOOT_Game::autodetect, with all other vars being empty. Game(const GameSettings& gameSettings); - Game(const Game& game); Game(const unsigned int baseGameCode, const std::string& lootFolder = ""); - Game& operator= (const Game& game); - void Init(bool createFolder, const boost::filesystem::path& gameLocalAppData = ""); void RedatePlugins(); //Change timestamps to match load order (Skyrim only). @@ -58,16 +51,8 @@ namespace loot { // Check if the plugin is active by using the cached value if // available, and otherwise asking the load order handler. bool IsPluginActive(const std::string& pluginName) const; - - //Plugin data and metadata lists. - Masterlist masterlist; - MetadataList userlist; - std::unordered_map plugins; //Map so that plugin data can be edited. private: bool _pluginsFullyLoaded; - std::mutex mutex; - - void addPlugin(const Plugin&& plugin); }; std::list ToGames(const std::list& settings); diff --git a/src/backend/game/game_cache.cpp b/src/backend/game/game_cache.cpp index 25f546d5..e818a840 100644 --- a/src/backend/game/game_cache.cpp +++ b/src/backend/game/game_cache.cpp @@ -40,15 +40,31 @@ namespace lc = boost::locale; namespace loot { GameCache::GameCache() {} - GameCache::GameCache(const GameCache& cache) - : conditionCache(cache.conditionCache) {} + GameCache::GameCache(const GameCache& cache) : + masterlist(cache.masterlist), + userlist(cache.userlist), + conditionCache(cache.conditionCache), + plugins(cache.plugins) {} GameCache& GameCache::operator=(const GameCache& cache) { - conditionCache = cache.conditionCache; + if (&cache != this) { + masterlist = cache.masterlist; + userlist = cache.userlist; + conditionCache = cache.conditionCache; + plugins = cache.plugins; + } return *this; } + Masterlist & GameCache::GetMasterlist() { + return masterlist; + } + + MetadataList & GameCache::GetUserlist() { + return userlist; + } + void GameCache::CacheCondition(const std::string& condition, bool result) { std::lock_guard guard(mutex); conditionCache.insert(pair(boost::locale::to_lower(condition), result)); @@ -65,6 +81,30 @@ namespace loot { return std::pair(false, false); } + std::set GameCache::GetPlugins() const { + std::set output; + std::transform(begin(plugins), + end(plugins), + inserter>(output, begin(output)), + [](const pair& pluginPair) { + return pluginPair.second; + }); + return output; + } + + const Plugin& GameCache::GetPlugin(const std::string & pluginName) const { + auto it = plugins.find(boost::locale::to_lower(pluginName)); + if (it != end(plugins)) + return it->second; + + throw error(error::invalid_args, "No plugin \"" + pluginName + "\" exists."); + } + + void GameCache::AddPlugin(const Plugin&& plugin) { + std::lock_guard lock(mutex); + plugins.emplace(boost::locale::to_lower(plugin.Name()), plugin); + } + void GameCache::ClearCache() { std::lock_guard guard(mutex); diff --git a/src/backend/game/game_cache.h b/src/backend/game/game_cache.h index bccba2cf..b8d88fbf 100644 --- a/src/backend/game/game_cache.h +++ b/src/backend/game/game_cache.h @@ -25,11 +25,14 @@ #ifndef __LOOT_GAME_CRC_CACHE__ #define __LOOT_GAME_CRC_CACHE__ -#include +#include "../metadata_list.h" +#include "../masterlist.h" +#include "../plugin/plugin.h" + #include +#include #include #include -#include namespace loot { class GameCache { @@ -39,15 +42,23 @@ namespace loot { GameCache& operator=(const GameCache& cache); - void CacheCondition(const std::string& condition, bool result); + Masterlist& GetMasterlist(); + MetadataList& GetUserlist(); // Returns false for second bool if no cached condition. std::pair GetCachedCondition(const std::string& condition) const; + void CacheCondition(const std::string& condition, bool result); + + std::set GetPlugins() const; + const Plugin& GetPlugin(const std::string & pluginName) const; + void AddPlugin(const Plugin&& plugin); void ClearCache(); private: - //Caches for condition results, CRCs and active plugins. + Masterlist masterlist; + MetadataList userlist; std::unordered_map conditionCache; + std::unordered_map plugins; mutable std::mutex mutex; }; diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index 3f29628a..f4532840 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -293,9 +293,10 @@ namespace loot { else { // CRC could be for a plugin or a file. // Get the CRC from the game plugin cache if possible. - auto pluginPairIt = _game->plugins.find(boost::locale::to_lower(file)); - if (pluginPairIt != _game->plugins.end()) - crc = pluginPairIt->second.Crc(); + try { + crc = _game->GetPlugin(file).Crc(); + } + catch (...) {} if (crc == 0) { if (boost::filesystem::exists(_game->DataPath() / file)) diff --git a/src/backend/metadata/plugin_dirty_info.cpp b/src/backend/metadata/plugin_dirty_info.cpp index 03f68346..0a847d4f 100644 --- a/src/backend/metadata/plugin_dirty_info.cpp +++ b/src/backend/metadata/plugin_dirty_info.cpp @@ -98,9 +98,10 @@ namespace loot { uint32_t crc = 0; // Get the CRC from the game plugin cache if possible. - auto pluginPairIt = game.plugins.find(boost::locale::to_lower(pluginName)); - if (pluginPairIt != game.plugins.end()) - crc = pluginPairIt->second.Crc(); + try { + crc = game.GetPlugin(pluginName).Crc(); + } + catch (...) {} // Otherwise calculate it from the file. if (crc == 0) { diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index c94a1318..aad6cee4 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -184,6 +184,10 @@ namespace loot { return false; } + bool Plugin::operator < (const Plugin & rhs) const { + return boost::ilexicographical_compare(Name(), rhs.Name());; + } + bool Plugin::IsActive() const { return _isActive; } diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index 36a0ffe0..7de75fd3 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -62,6 +62,8 @@ namespace loot { //Validity checks. bool CheckInstallValidity(const Game& game); //Checks that reqs and masters are all present, and that no incs are present. Returns true if the plugin is dirty. static bool IsValid(const std::string& filename, const Game& game); + + bool operator < (const Plugin& rhs) const; private: bool _isEmpty; // Does the plugin contain any records other than the TES4 header? bool _isActive; diff --git a/src/backend/plugin_sorter.cpp b/src/backend/plugin_sorter.cpp index 71dd420c..8564e551 100644 --- a/src/backend/plugin_sorter.cpp +++ b/src/backend/plugin_sorter.cpp @@ -177,21 +177,16 @@ namespace loot { // Using a set of plugin names followed by finding the matching key // in the unordered map, as it's probably faster than copying the // full plugin objects then sorting them. - set pluginNames; - for (const auto &plugin : game.plugins) { - pluginNames.insert(plugin.first); - } - - for (const auto &plugin : pluginNames) { - vertex_t v = boost::add_vertex(game.plugins.find(plugin)->second, graph); + for (const auto &plugin : game.GetPlugins()) { + vertex_t v = boost::add_vertex(plugin, graph); BOOST_LOG_TRIVIAL(trace) << "Merging for plugin \"" << graph[v].Name() << "\""; //Check if there is a plugin entry in the masterlist. This will also find matching regex entries. BOOST_LOG_TRIVIAL(trace) << "Merging masterlist data down to plugin list data."; - graph[v].MergeMetadata(game.masterlist.FindPlugin(graph[v])); + graph[v].MergeMetadata(game.GetMasterlist().FindPlugin(graph[v])); //Check if there is a plugin entry in the userlist. This will also find matching regex entries. - PluginMetadata ulistPlugin = game.userlist.FindPlugin(graph[v]); + PluginMetadata ulistPlugin = game.GetUserlist().FindPlugin(graph[v]); if (!ulistPlugin.HasNameOnly() && ulistPlugin.Enabled()) { BOOST_LOG_TRIVIAL(trace) << "Merging userlist data down to plugin list data."; diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 9269d72e..5213ad26 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -400,7 +400,7 @@ namespace loot { void Handler::GetConflictingPlugins(const std::string& pluginName, CefRefPtr frame, CefRefPtr callback) { BOOST_LOG_TRIVIAL(debug) << "Searching for plugins that conflict with " << pluginName; - auto pluginIt = _lootState.CurrentGame().plugins.find(boost::locale::to_lower(pluginName)); + auto plugin = _lootState.CurrentGame().GetPlugin(pluginName); // Checking for FormID overlap will only work if the plugins have been loaded, so check if // the plugins have been fully loaded, and if not load all plugins. @@ -411,13 +411,13 @@ namespace loot { SendProgressUpdate(frame, loc::translate("Checking for conflicting plugins...")); YAML::Node node; - for (const auto& pluginPair : _lootState.CurrentGame().plugins) { + for (const auto& otherPlugin : _lootState.CurrentGame().GetPlugins()) { YAML::Node pluginNode; - pluginNode["crc"] = pluginPair.second.Crc(); - pluginNode["isEmpty"] = pluginPair.second.IsEmpty(); - if (pluginIt != _lootState.CurrentGame().plugins.end() && pluginIt->second.DoFormIDsOverlap(pluginPair.second)) { - BOOST_LOG_TRIVIAL(debug) << "Found conflicting plugin: " << pluginPair.second.Name(); + pluginNode["crc"] = otherPlugin.Crc(); + pluginNode["isEmpty"] = otherPlugin.IsEmpty(); + if (plugin.DoFormIDsOverlap(otherPlugin)) { + BOOST_LOG_TRIVIAL(debug) << "Found conflicting plugin: " << otherPlugin.Name(); pluginNode["conflicts"] = true; } else { @@ -425,13 +425,13 @@ namespace loot { } // Plugin loading may have produced an error message, so rederive displayed data. - YAML::Node derivedNode = GenerateDerivedMetadata(pluginPair.second.Name()); + YAML::Node derivedNode = GenerateDerivedMetadata(otherPlugin.Name()); for (const auto &pair : derivedNode) { const string key = pair.first.as(); pluginNode[key] = pair.second; } - node[pluginPair.second.Name()] = pluginNode; + node[otherPlugin.Name()] = pluginNode; } if (node.size() > 0) @@ -444,8 +444,8 @@ namespace loot { BOOST_LOG_TRIVIAL(debug) << "Copying metadata for plugin " << pluginName; // Get metadata from masterlist and userlist. - PluginMetadata plugin = _lootState.CurrentGame().masterlist.FindPlugin(pluginName); - plugin.MergeMetadata(_lootState.CurrentGame().userlist.FindPlugin(pluginName)); + PluginMetadata plugin = _lootState.CurrentGame().GetMasterlist().FindPlugin(pluginName); + plugin.MergeMetadata(_lootState.CurrentGame().GetUserlist().FindPlugin(pluginName)); // Generate text representation. string text; @@ -465,10 +465,10 @@ namespace loot { std::string Handler::ClearPluginMetadata(const std::string& pluginName) { BOOST_LOG_TRIVIAL(debug) << "Clearing user metadata for plugin " << pluginName; - _lootState.CurrentGame().userlist.ErasePlugin(PluginMetadata(pluginName)); + _lootState.CurrentGame().GetUserlist().ErasePlugin(PluginMetadata(pluginName)); // Save userlist edits. - _lootState.CurrentGame().userlist.Save(_lootState.CurrentGame().UserlistPath()); + _lootState.CurrentGame().GetUserlist().Save(_lootState.CurrentGame().UserlistPath()); // Now rederive the displayed metadata from the masterlist. YAML::Node derivedMetadata = GenerateDerivedMetadata(pluginName); @@ -484,7 +484,7 @@ namespace loot { PluginMetadata newUserlistEntry(pluginMetadata["name"].as()); // Find existing userlist entry. - PluginMetadata ulistPlugin = _lootState.CurrentGame().userlist.FindPlugin(newUserlistEntry); + PluginMetadata ulistPlugin = _lootState.CurrentGame().GetUserlist().FindPlugin(newUserlistEntry); // First sort out the priority value. This is only given if it was changed. BOOST_LOG_TRIVIAL(trace) << "Calculating userlist metadata priority value from Javascript variables."; @@ -537,28 +537,28 @@ namespace loot { // For cleanliness, only data that does not duplicate masterlist and plugin data should be retained, so diff that. BOOST_LOG_TRIVIAL(trace) << "Removing any user metadata that duplicates masterlist metadata."; - auto pluginIt = _lootState.CurrentGame().plugins.find(boost::locale::to_lower(newUserlistEntry.Name())); - if (pluginIt != _lootState.CurrentGame().plugins.end()) { - Plugin tempPlugin(pluginIt->second); - tempPlugin.MergeMetadata(_lootState.CurrentGame().masterlist.FindPlugin(newUserlistEntry)); + try { + Plugin tempPlugin(_lootState.CurrentGame().GetPlugin(newUserlistEntry.Name())); + tempPlugin.MergeMetadata(_lootState.CurrentGame().GetMasterlist().FindPlugin(newUserlistEntry)); newUserlistEntry = newUserlistEntry.NewMetadata(tempPlugin); } - else - newUserlistEntry = newUserlistEntry.NewMetadata(_lootState.CurrentGame().masterlist.FindPlugin(newUserlistEntry)); + catch (...) { + newUserlistEntry = newUserlistEntry.NewMetadata(_lootState.CurrentGame().GetMasterlist().FindPlugin(newUserlistEntry)); + } // Now erase any existing userlist entry. if (!ulistPlugin.HasNameOnly()) { BOOST_LOG_TRIVIAL(trace) << "Erasing the existing userlist entry."; - _lootState.CurrentGame().userlist.ErasePlugin(ulistPlugin); + _lootState.CurrentGame().GetUserlist().ErasePlugin(ulistPlugin); } // Add a new userlist entry if necessary. if (!newUserlistEntry.HasNameOnly()) { BOOST_LOG_TRIVIAL(trace) << "Adding new metadata to new userlist entry."; - _lootState.CurrentGame().userlist.AddPlugin(newUserlistEntry); + _lootState.CurrentGame().GetUserlist().AddPlugin(newUserlistEntry); } // Save edited userlist. - _lootState.CurrentGame().userlist.Save(_lootState.CurrentGame().UserlistPath()); + _lootState.CurrentGame().GetUserlist().Save(_lootState.CurrentGame().UserlistPath()); // Now rederive the derived metadata. BOOST_LOG_TRIVIAL(trace) << "Returning newly derived display metadata."; @@ -642,17 +642,18 @@ namespace loot { // First clear CRC and condition caches, otherwise they could lead to incorrect evaluations. _lootState.CurrentGame().ClearCache(); - bool isFirstLoad = _lootState.CurrentGame().plugins.empty(); + bool isFirstLoad = _lootState.CurrentGame().GetPlugins().empty(); _lootState.CurrentGame().LoadPlugins(true); //Sort plugins into their load order. list installed; list loadOrder = _lootState.CurrentGame().GetLoadOrder(); for (const auto &pluginName : loadOrder) { - const auto pos = _lootState.CurrentGame().plugins.find(boost::locale::to_lower(pluginName)); - - if (pos != _lootState.CurrentGame().plugins.end()) - installed.push_back(pos->second); + try { + const auto plugin = _lootState.CurrentGame().GetPlugin(pluginName); + installed.push_back(plugin); + } + catch (...) {} } list parsingErrors; @@ -662,7 +663,7 @@ namespace loot { SendProgressUpdate(frame, loc::translate("Parsing masterlist...")); BOOST_LOG_TRIVIAL(debug) << "Parsing masterlist."; try { - _lootState.CurrentGame().masterlist.Load(_lootState.CurrentGame().MasterlistPath()); + _lootState.CurrentGame().GetMasterlist().Load(_lootState.CurrentGame().MasterlistPath()); } catch (exception &e) { parsingErrors.push_back(Message(Message::error, (boost::format(loc::translate( @@ -679,7 +680,7 @@ namespace loot { SendProgressUpdate(frame, loc::translate("Parsing userlist...")); BOOST_LOG_TRIVIAL(debug) << "Parsing userlist."; try { - _lootState.CurrentGame().userlist.Load(_lootState.CurrentGame().UserlistPath()); + _lootState.CurrentGame().GetUserlist().Load(_lootState.CurrentGame().UserlistPath()); } catch (exception &e) { parsingErrors.push_back(Message(Message::error, (boost::format(loc::translate( @@ -710,7 +711,7 @@ namespace loot { // Store the masterlist revision and date. try { - Masterlist::Info info = _lootState.CurrentGame().masterlist.GetInfo(_lootState.CurrentGame().MasterlistPath(), true); + Masterlist::Info info = _lootState.CurrentGame().GetMasterlist().GetInfo(_lootState.CurrentGame().MasterlistPath(), true); gameNode["masterlist"]["revision"] = info.revision; gameNode["masterlist"]["date"] = info.date; } @@ -730,11 +731,11 @@ namespace loot { // description as part of it. BOOST_LOG_TRIVIAL(trace) << "Getting masterlist metadata for: " << plugin.Name(); Plugin mlistPlugin(plugin); - mlistPlugin.MergeMetadata(_lootState.CurrentGame().masterlist.FindPlugin(plugin)); + mlistPlugin.MergeMetadata(_lootState.CurrentGame().GetMasterlist().FindPlugin(plugin)); // Now do the same again for any userlist data. BOOST_LOG_TRIVIAL(trace) << "Getting userlist metadata for: " << plugin.Name(); - PluginMetadata ulistPlugin(_lootState.CurrentGame().userlist.FindPlugin(plugin)); + PluginMetadata ulistPlugin(_lootState.CurrentGame().GetUserlist().FindPlugin(plugin)); pluginNode["__type"] = "Plugin"; // For conversion back into a JS typed object. pluginNode["name"] = plugin.Name(); @@ -797,11 +798,11 @@ namespace loot { //Evaluate any conditions in the global messages. BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; list messages = parsingErrors; - auto metadataListMessages = _lootState.CurrentGame().masterlist.Messages(); + auto metadataListMessages = _lootState.CurrentGame().GetMasterlist().Messages(); messages.insert(end(messages), begin(metadataListMessages), end(metadataListMessages)); - metadataListMessages = _lootState.CurrentGame().userlist.Messages(); + metadataListMessages = _lootState.CurrentGame().GetUserlist().Messages(); messages.insert(messages.end(), begin(metadataListMessages), end(metadataListMessages)); @@ -850,20 +851,20 @@ namespace loot { bool wasChanged = true; try { SendProgressUpdate(frame, loc::translate("Updating and parsing masterlist...")); - wasChanged = _lootState.CurrentGame().masterlist.Update(_lootState.CurrentGame()); + wasChanged = _lootState.CurrentGame().GetMasterlist().Update(_lootState.CurrentGame()); } catch (loot::error &e) { if (e.code() == loot::error::ok) { // There was a parsing error, but roll-back was successful, so the process // should still complete. - _lootState.CurrentGame().masterlist.AppendMessage(Message(Message::error, e.what())); + _lootState.CurrentGame().GetMasterlist().AppendMessage(Message(Message::error, e.what())); wasChanged = true; } else { // Error wasn't a parsing error. Need to try parsing masterlist if it exists. try { - _lootState.CurrentGame().masterlist.Load(_lootState.CurrentGame().MasterlistPath()); + _lootState.CurrentGame().GetMasterlist().Load(_lootState.CurrentGame().MasterlistPath()); } catch (...) {} } @@ -878,7 +879,7 @@ namespace loot { // Store the masterlist revision and date. try { - Masterlist::Info info = _lootState.CurrentGame().masterlist.GetInfo(_lootState.CurrentGame().MasterlistPath(), true); + Masterlist::Info info = _lootState.CurrentGame().GetMasterlist().GetInfo(_lootState.CurrentGame().MasterlistPath(), true); gameNode["masterlist"]["revision"] = info.revision; gameNode["masterlist"]["date"] = info.date; } @@ -887,9 +888,9 @@ namespace loot { gameNode["masterlist"]["date"] = e.what(); } - for (const auto& pluginPair : _lootState.CurrentGame().plugins) { - Plugin mlistPlugin(pluginPair.second); - mlistPlugin.MergeMetadata(_lootState.CurrentGame().masterlist.FindPlugin(pluginPair.second)); + for (const auto& plugin : _lootState.CurrentGame().GetPlugins()) { + Plugin mlistPlugin(plugin); + mlistPlugin.MergeMetadata(_lootState.CurrentGame().GetMasterlist().FindPlugin(plugin)); YAML::Node pluginNode; if (!mlistPlugin.HasNameOnly()) { @@ -905,7 +906,7 @@ namespace loot { // Now merge masterlist and userlist metadata and evaluate, // putting any resulting metadata into the base of the pluginNode. - YAML::Node derivedNode = GenerateDerivedMetadata(pluginPair.second.Name()); + YAML::Node derivedNode = GenerateDerivedMetadata(plugin.Name()); for (const auto &pair : derivedNode) { const string key = pair.first.as(); @@ -917,7 +918,7 @@ namespace loot { //Evaluate any conditions in the global messages. BOOST_LOG_TRIVIAL(debug) << "Evaluating global message conditions."; - list messages = _lootState.CurrentGame().masterlist.Messages(); + list messages = _lootState.CurrentGame().GetMasterlist().Messages(); try { list::iterator it = messages.begin(); while (it != messages.end()) { @@ -954,16 +955,16 @@ namespace loot { BOOST_LOG_TRIVIAL(debug) << "Clearing all user metadata."; // Record which plugins have userlist entries. vector userlistPlugins; - for (const auto &plugin : _lootState.CurrentGame().userlist.Plugins()) { + for (const auto &plugin : _lootState.CurrentGame().GetUserlist().Plugins()) { userlistPlugins.push_back(plugin.Name()); } BOOST_LOG_TRIVIAL(trace) << "User metadata exists for " << userlistPlugins.size() << " plugins."; // Clear the user metadata. - _lootState.CurrentGame().userlist.clear(); + _lootState.CurrentGame().GetUserlist().clear(); // Save userlist edits. - _lootState.CurrentGame().userlist.Save(_lootState.CurrentGame().UserlistPath()); + _lootState.CurrentGame().GetUserlist().Save(_lootState.CurrentGame().UserlistPath()); // Regenerate the derived metadata (priority, messages, tags and dirty state) // for any plugins with userlist entries. @@ -1074,15 +1075,16 @@ namespace loot { YAML::Node Handler::GenerateDerivedMetadata(const std::string& pluginName) { // Now rederive the displayed metadata from the masterlist and userlist. - auto pluginIt = _lootState.CurrentGame().plugins.find(boost::locale::to_lower(pluginName)); - if (pluginIt != _lootState.CurrentGame().plugins.end()) { - PluginMetadata master(_lootState.CurrentGame().masterlist.FindPlugin(pluginIt->second)); - PluginMetadata user(_lootState.CurrentGame().userlist.FindPlugin(pluginIt->second)); + try { + auto plugin = _lootState.CurrentGame().GetPlugin(pluginName); + PluginMetadata master(_lootState.CurrentGame().GetMasterlist().FindPlugin(plugin)); + PluginMetadata user(_lootState.CurrentGame().GetUserlist().FindPlugin(plugin)); - return this->GenerateDerivedMetadata(pluginIt->second, master, user); + return this->GenerateDerivedMetadata(plugin, master, user); + } + catch (...) { + return YAML::Node(); } - - return YAML::Node(); } void Handler::CopyToClipboard(const std::string& text) { diff --git a/src/tests/backend/game/test_game.h b/src/tests/backend/game/test_game.h index 3a1157d7..cb4cf5b4 100644 --- a/src/tests/backend/game/test_game.h +++ b/src/tests/backend/game/test_game.h @@ -216,11 +216,11 @@ TEST_F(Game, LoadPlugins) { game.SetGamePath(dataPath.parent_path()); EXPECT_NO_THROW(game.LoadPlugins(false)); - EXPECT_EQ(11, game.plugins.size()); + EXPECT_EQ(11, game.GetPlugins().size()); // Check that all the plugins' data have loaded correctly. - ASSERT_NE(game.plugins.end(), game.plugins.find("skyrim.esm")); - loot::Plugin plugin = game.plugins.find("skyrim.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("Skyrim.esm")); + loot::Plugin plugin = game.GetPlugin("Skyrim.esm"); EXPECT_EQ("Skyrim.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -241,8 +241,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x187BE342, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank.esm")); - plugin = game.plugins.find("blank.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank.esm")); + plugin = game.GetPlugin("blank.esm"); EXPECT_EQ("Blank.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -263,8 +263,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x187BE342, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different.esm")); - plugin = game.plugins.find("blank - different.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("Blank - Different.esm")); + plugin = game.GetPlugin("Blank - Different.esm"); EXPECT_EQ("Blank - Different.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -284,8 +284,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x64B9F757, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - master dependent.esm")); - plugin = game.plugins.find("blank - master dependent.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - master dependent.esm")); + plugin = game.GetPlugin("blank - master dependent.esm"); EXPECT_EQ("Blank - Master Dependent.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -306,8 +306,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0xB2D4119E, plugin.Crc()); EXPECT_EQ(4, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different master dependent.esm")); - plugin = game.plugins.find("blank - different master dependent.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different master dependent.esm")); + plugin = game.GetPlugin("blank - different master dependent.esm"); EXPECT_EQ("Blank - Different Master Dependent.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -327,8 +327,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0xAADF6710, plugin.Crc()); EXPECT_EQ(4, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank.esp")); - plugin = game.plugins.find("blank.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank.esp")); + plugin = game.GetPlugin("blank.esp"); EXPECT_EQ("Blank.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -345,8 +345,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x24F0E2A1, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different.esp")); - plugin = game.plugins.find("blank - different.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different.esp")); + plugin = game.GetPlugin("blank - different.esp"); EXPECT_EQ("Blank - Different.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -362,8 +362,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0xD4C9B7AE, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - master dependent.esp")); - plugin = game.plugins.find("blank - master dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - master dependent.esp")); + plugin = game.GetPlugin("blank - master dependent.esp"); EXPECT_EQ("Blank - Master Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -380,8 +380,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x832152DC, plugin.Crc()); EXPECT_EQ(2, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different master dependent.esp")); - plugin = game.plugins.find("blank - different master dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different master dependent.esp")); + plugin = game.GetPlugin("blank - different master dependent.esp"); EXPECT_EQ("Blank - Different Master Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -397,8 +397,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x3AD17683, plugin.Crc()); EXPECT_EQ(2, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - plugin dependent.esp")); - plugin = game.plugins.find("blank - plugin dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - plugin dependent.esp")); + plugin = game.GetPlugin("blank - plugin dependent.esp"); EXPECT_EQ("Blank - Plugin Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -413,8 +413,8 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(0x28EF26DB, plugin.Crc()); EXPECT_EQ(1, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different plugin dependent.esp")); - plugin = game.plugins.find("blank - different plugin dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different plugin dependent.esp")); + plugin = game.GetPlugin("blank - different plugin dependent.esp"); EXPECT_EQ("Blank - Different Plugin Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -434,11 +434,11 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { game.SetGamePath(dataPath.parent_path()); EXPECT_NO_THROW(game.LoadPlugins(true)); - EXPECT_EQ(11, game.plugins.size()); + EXPECT_EQ(11, game.GetPlugins().size()); // Check that all the plugins' data have loaded correctly. - ASSERT_NE(game.plugins.end(), game.plugins.find("skyrim.esm")); - loot::Plugin plugin = game.plugins.find("skyrim.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("Skyrim.esm")); + loot::Plugin plugin = game.GetPlugin("Skyrim.esm"); EXPECT_EQ("Skyrim.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -448,8 +448,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank.esm")); - plugin = game.plugins.find("blank.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank.esm")); + plugin = game.GetPlugin("blank.esm"); EXPECT_EQ("Blank.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -459,8 +459,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different.esm")); - plugin = game.plugins.find("blank - different.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("Blank - Different.esm")); + plugin = game.GetPlugin("Blank - Different.esm"); EXPECT_EQ("Blank - Different.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -470,8 +470,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - master dependent.esm")); - plugin = game.plugins.find("blank - master dependent.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - master dependent.esm")); + plugin = game.GetPlugin("blank - master dependent.esm"); EXPECT_EQ("Blank - Master Dependent.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -483,8 +483,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different master dependent.esm")); - plugin = game.plugins.find("blank - different master dependent.esm")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different master dependent.esm")); + plugin = game.GetPlugin("blank - different master dependent.esm"); EXPECT_EQ("Blank - Different Master Dependent.esm", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_TRUE(plugin.isMasterFile()); @@ -496,8 +496,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank.esp")); - plugin = game.plugins.find("blank.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank.esp")); + plugin = game.GetPlugin("blank.esp"); EXPECT_EQ("Blank.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -507,8 +507,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different.esp")); - plugin = game.plugins.find("blank - different.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different.esp")); + plugin = game.GetPlugin("blank - different.esp"); EXPECT_EQ("Blank - Different.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -518,8 +518,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - master dependent.esp")); - plugin = game.plugins.find("blank - master dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - master dependent.esp")); + plugin = game.GetPlugin("blank - master dependent.esp"); EXPECT_EQ("Blank - Master Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -531,8 +531,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different master dependent.esp")); - plugin = game.plugins.find("blank - different master dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different master dependent.esp")); + plugin = game.GetPlugin("blank - different master dependent.esp"); EXPECT_EQ("Blank - Different Master Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -544,8 +544,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - plugin dependent.esp")); - plugin = game.plugins.find("blank - plugin dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - plugin dependent.esp")); + plugin = game.GetPlugin("blank - plugin dependent.esp"); EXPECT_EQ("Blank - Plugin Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); @@ -557,8 +557,8 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); - ASSERT_NE(game.plugins.end(), game.plugins.find("blank - different plugin dependent.esp")); - plugin = game.plugins.find("blank - different plugin dependent.esp")->second; + ASSERT_NO_THROW(game.GetPlugin("blank - different plugin dependent.esp")); + plugin = game.GetPlugin("blank - different plugin dependent.esp"); EXPECT_EQ("Blank - Different Plugin Dependent.esp", plugin.Name()); EXPECT_FALSE(plugin.IsEmpty()); EXPECT_FALSE(plugin.isMasterFile()); diff --git a/src/tests/backend/test_plugin_sorter.h b/src/tests/backend/test_plugin_sorter.h index 8cb04b69..9d25dc3f 100644 --- a/src/tests/backend/test_plugin_sorter.h +++ b/src/tests/backend/test_plugin_sorter.h @@ -94,7 +94,7 @@ TEST_F(PluginSorter, Sort_WithPriority) { ASSERT_NO_THROW(game.LoadPlugins(false)); loot::PluginMetadata plugin("Blank - Different Master Dependent.esp"); plugin.Priority(-1100000); - game.userlist.AddPlugin(plugin); + game.GetUserlist().AddPlugin(plugin); loot::PluginSorter ps; std::list expectedSortedOrder({ @@ -122,7 +122,7 @@ TEST_F(PluginSorter, Sort_WithLoadAfter) { loot::File("Blank - Different.esp"), loot::File("Blank - Different Plugin Dependent.esp"), }); - game.userlist.AddPlugin(plugin); + game.GetUserlist().AddPlugin(plugin); loot::PluginSorter ps; std::list expectedSortedOrder({ @@ -150,7 +150,7 @@ TEST_F(PluginSorter, Sort_WithRequirements) { loot::File("Blank - Different.esp"), loot::File("Blank - Different Plugin Dependent.esp"), }); - game.userlist.AddPlugin(plugin); + game.GetUserlist().AddPlugin(plugin); loot::PluginSorter ps; std::list expectedSortedOrder({ @@ -175,7 +175,7 @@ TEST_F(PluginSorter, Sort_HasCycle) { ASSERT_NO_THROW(game.LoadPlugins(false)); loot::PluginMetadata plugin("Blank.esm"); plugin.LoadAfter({loot::File("Blank - Master Dependent.esm")}); - game.userlist.AddPlugin(plugin); + game.GetUserlist().AddPlugin(plugin); loot::PluginSorter ps; EXPECT_ANY_THROW(ps.Sort(game, loot::Language::english, callback));