From 32b6d27cbb22fe38d4e458c1528c8110568bb00a Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 4 Nov 2015 22:41:49 +0000 Subject: [PATCH] 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) {