From 44955e8bc0391fd9ab438150ebb954aaabce3a1e Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sun, 21 Oct 2018 08:48:53 +0100 Subject: [PATCH] Don't cache active state in Plugin objects Use libloadorder as the single source of truth. --- src/api/game/game.cpp | 8 +-- src/api/metadata/condition_evaluator.cpp | 2 +- src/api/plugin.cpp | 6 --- src/api/plugin.h | 4 -- .../api/internals/game/game_cache_test.h | 7 --- src/tests/api/internals/plugin_test.h | 49 ------------------- 6 files changed, 2 insertions(+), 74 deletions(-) diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index 359fb3f7..8927e535 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -174,7 +174,7 @@ void Game::LoadPlugins(const std::vector& plugins, loadHeadersOnly || loot::equivalent(pluginPath, masterPath); try { cache_->AddPlugin(Plugin( - Type(), cache_, loadOrderHandler_, pluginPath, loadHeader)); + Type(), cache_, pluginPath, loadHeader)); } catch (std::exception& e) { if (logger) { logger->error( @@ -228,12 +228,6 @@ void Game::LoadCurrentLoadOrderState() { } bool Game::IsPluginActive(const std::string& pluginName) const { - auto plugin = cache_->GetPlugin(pluginName); - - if (plugin) { - return plugin->IsActive(); - } - return loadOrderHandler_->IsPluginActive(pluginName); } diff --git a/src/api/metadata/condition_evaluator.cpp b/src/api/metadata/condition_evaluator.cpp index f43a05c1..d9fc9adf 100644 --- a/src/api/metadata/condition_evaluator.cpp +++ b/src/api/metadata/condition_evaluator.cpp @@ -424,7 +424,7 @@ Version ConditionEvaluator::getVersion(const std::string& filePath) const { auto pluginPath = dataPath_ / u8path(filePath); if (Plugin::IsValid(gameType_, pluginPath)) return Version( - Plugin(gameType_, gameCache_, loadOrderHandler_, pluginPath, true) + Plugin(gameType_, gameCache_, pluginPath, true) .GetVersion() .value_or("")); diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index 19f3dc75..0145aa00 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -42,13 +42,11 @@ using std::string; namespace loot { Plugin::Plugin(const GameType gameType, std::shared_ptr gameCache, - std::shared_ptr loadOrderHandler, std::filesystem::path pluginPath, const bool headerOnly) : name_(pluginPath.filename().u8string()), esPlugin(nullptr), isEmpty_(true), - isActive_(false), loadsArchive_(false), numOverrideRecords_(0) { auto logger = getLogger(); @@ -112,8 +110,6 @@ Plugin::Plugin(const GameType gameType, } } } - // Get whether the plugin is active or not. - isActive_ = loadOrderHandler->IsPluginActive(name_); loadsArchive_ = LoadsArchive(gameType, gameCache, pluginPath); } catch (std::exception& e) { @@ -285,8 +281,6 @@ bool Plugin::operator<(const Plugin& rhs) const { return boost::locale::to_lower(name_) < boost::locale::to_lower(rhs.name_); } -bool Plugin::IsActive() const { return isActive_; } - void Plugin::Load(const std::filesystem::path& path, GameType gameType, bool headerOnly) { diff --git a/src/api/plugin.h b/src/api/plugin.h index b1312850..66ec55fc 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -45,7 +45,6 @@ class Plugin : public PluginInterface { public: Plugin(const GameType gameType, std::shared_ptr gameCache, - std::shared_ptr loadOrderHandler, std::filesystem::path pluginPath, const bool headerOnly); @@ -63,8 +62,6 @@ public: bool LoadsArchive() const; bool DoFormIDsOverlap(const PluginInterface& plugin) const; - bool IsActive() const; - // Load ordering functions. size_t NumOverrideFormIDs() const; @@ -88,7 +85,6 @@ private: bool isEmpty_; // Does the plugin contain any records other than the TES4 // header? - bool isActive_; bool loadsArchive_; const std::string name_; std::optional version_; // Obtained from description field. diff --git a/src/tests/api/internals/game/game_cache_test.h b/src/tests/api/internals/game/game_cache_test.h index 3ef24e9e..99d45fa6 100644 --- a/src/tests/api/internals/game/game_cache_test.h +++ b/src/tests/api/internals/game/game_cache_test.h @@ -89,7 +89,6 @@ TEST_P(GameCacheTest, gettingAnUncachedCrcShouldReturnZero) { TEST_P(GameCacheTest, addingAPluginThatDoesNotExistShouldSucceed) { cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true)); EXPECT_EQ(blankEsm, cache_.GetPlugin(blankEsm)->GetName()); @@ -99,14 +98,12 @@ TEST_P(GameCacheTest, addingAPluginThatIsAlreadyCachedShouldOverwriteExistingEntry) { cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true)); EXPECT_FALSE(cache_.GetPlugin(blankEsm)->GetCRC()); cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, false)); EXPECT_EQ(blankEsmCrc, cache_.GetPlugin(blankEsm)->GetCRC().value()); @@ -119,7 +116,6 @@ TEST_P(GameCacheTest, gettingAPluginThatIsNotCachedShouldReturnANullPointer) { TEST_P(GameCacheTest, gettingAPluginShouldBeCaseInsensitive) { cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true)); EXPECT_EQ(blankEsm, cache_.GetPlugin(blankEsm)->GetName()); @@ -134,12 +130,10 @@ TEST_P(GameCacheTest, gettingPluginsShouldReturnASetOfCachedPluginsIfPluginsHaveBeenCached) { cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true)); cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsm, true)); @@ -187,7 +181,6 @@ TEST_P(GameCacheTest, clearingCachedPluginsShouldNotThrowIfNoPluginsAreCached) { TEST_P(GameCacheTest, clearingCachedPluginsShouldClearAnyCachedPlugins) { cache_.AddPlugin(Plugin(game_.Type(), std::make_shared(GameCache()), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true)); cache_.ClearCachedPlugins(); diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index 777a55fa..7b3e29bb 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -162,7 +162,6 @@ INSTANTIATE_TEST_CASE_P(, TEST_P(PluginTest, loadingShouldHandleNonAsciiFilenamesCorrectly) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / std::filesystem::u8path(nonAsciiEsp), true); @@ -173,7 +172,6 @@ TEST_P(PluginTest, loadingShouldHandleNonAsciiFilenamesCorrectly) { TEST_P(PluginTest, loadingHeaderOnlyShouldReadHeaderData) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); @@ -193,7 +191,6 @@ TEST_P(PluginTest, loadingHeaderOnlyShouldReadHeaderData) { TEST_P(PluginTest, loadingHeaderOnlyShouldNotReadFieldsOrCalculateCrc) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); @@ -203,7 +200,6 @@ TEST_P(PluginTest, loadingHeaderOnlyShouldNotReadFieldsOrCalculateCrc) { TEST_P(PluginTest, loadingWholePluginShouldReadHeaderData) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); @@ -223,7 +219,6 @@ TEST_P(PluginTest, loadingWholePluginShouldReadHeaderData) { TEST_P(PluginTest, loadingWholePluginShouldReadFields) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsm, false); @@ -233,7 +228,6 @@ TEST_P(PluginTest, loadingWholePluginShouldReadFields) { TEST_P(PluginTest, loadingWholePluginShouldCalculateCrc) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, false); @@ -243,7 +237,6 @@ TEST_P(PluginTest, loadingWholePluginShouldCalculateCrc) { TEST_P(PluginTest, loadingANonMasterPluginShouldReadTheMasterFlagAsFalse) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsp, true); @@ -255,17 +248,14 @@ TEST_P( isLightMasterShouldBeTrueForAPluginWithEslFileExtensionForFallout4AndSkyrimSeAndFalseOtherwise) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); Plugin plugin2(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsp, true); Plugin plugin3(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsl, true); @@ -278,7 +268,6 @@ TEST_P( TEST_P(PluginTest, loadingAPluginWithMastersShouldReadThemCorrectly) { Plugin plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsp, true); @@ -288,7 +277,6 @@ TEST_P(PluginTest, loadingAPluginWithMastersShouldReadThemCorrectly) { TEST_P(PluginTest, loadingAPluginThatDoesNotExistShouldThrow) { EXPECT_THROW(Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / "Blank\\.esp", true), FileAccessError); @@ -299,7 +287,6 @@ TEST_P( loadsArchiveForAnArchiveThatExactlyMatchesAnEsmFileBasenameShouldReturnTrueForAllGamesExceptOblivion) { bool loadsArchive = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true) .LoadsArchive(); @@ -316,7 +303,6 @@ TEST_P( loadsArchiveForAnArchiveThatExactlyMatchesANonAsciiEspFileBasenameShouldReturnTrue) { EXPECT_TRUE(Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / std::filesystem::u8path(nonAsciiEsp), true) .LoadsArchive()); @@ -328,7 +314,6 @@ TEST_P( loadsArchiveForAnArchiveThatExactlyMatchesAnEspFileBasenameShouldReturnTrue) { EXPECT_TRUE(Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsp, true) .LoadsArchive()); @@ -339,7 +324,6 @@ TEST_P( loadsArchiveForAnArchiveWithAFilenameWhichStartsWithTheEsmFileBasenameShouldReturnTrueForAllGamesExceptOblivionAndSkyrim) { bool loadsArchive = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankDifferentEsm, true) .LoadsArchive(); @@ -355,7 +339,6 @@ TEST_P( loadsArchiveForAnArchiveWithAFilenameWhichStartsWithTheEspFileBasenameShouldReturnTrueForAllGamesExceptSkyrim) { bool loadsArchive = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankDifferentEsp, true) .LoadsArchive(); @@ -373,7 +356,6 @@ TEST_P( bool loadsArchive = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / std::filesystem::u8path(otherNonAsciiEsp), true) .LoadsArchive(); @@ -389,7 +371,6 @@ TEST_P(PluginTest, loadsArchiveShouldReturnFalseForAPluginThatDoesNotLoadAnArchive) { EXPECT_FALSE(Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsp, true) .LoadsArchive()); @@ -417,7 +398,6 @@ TEST_P( isValidAsLightMasterShouldReturnTrueOnlyForASkyrimSEOrFallout4PluginWithNewFormIdsBetween0x800And0xFFFInclusive) { bool valid = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true) .IsValidAsLightMaster(); @@ -444,34 +424,14 @@ TEST_P(PluginTest, getFileSizeShouldReturnCorrectValueForAGhostedPlugin) { Plugin::GetFileSize(game_.DataPath() / blankMasterDependentEsm)); } -TEST_P(PluginTest, isActiveShouldReturnTrueForAPluginThatIsActive) { - EXPECT_TRUE(Plugin(game_.Type(), - game_.GetCache(), - game_.GetLoadOrderHandler(), - game_.DataPath() / blankEsm, - true) - .IsActive()); -} - -TEST_P(PluginTest, isActiveShouldReturnFalseForAPluginThatIsNotActive) { - EXPECT_FALSE(Plugin(game_.Type(), - game_.GetCache(), - game_.GetLoadOrderHandler(), - game_.DataPath() / blankEsp, - true) - .IsActive()); -} - TEST_P(PluginTest, lessThanOperatorShouldUseCaseInsensitiveLexicographicalNameComparison) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsp, true); Plugin plugin2(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / lowercaseBlankEsp, true); @@ -480,12 +440,10 @@ TEST_P(PluginTest, Plugin plugin3 = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); Plugin plugin4 = Plugin(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsp, true); @@ -497,7 +455,6 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseIfTheArgumentIsNotAPluginObject) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, false); OtherPluginType plugin2; @@ -510,12 +467,10 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseForTwoPluginsWithOnlyHeadersLoaded) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, true); Plugin plugin2(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsm, true); @@ -527,12 +482,10 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseIfThePluginsHaveUnrelatedRecords) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, false); Plugin plugin2(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsp, false); @@ -544,12 +497,10 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnTrueIfOnePluginOverridesTheOthersRecords) { Plugin plugin1(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankEsm, false); Plugin plugin2(game_.Type(), game_.GetCache(), - game_.GetLoadOrderHandler(), game_.DataPath() / blankMasterDependentEsm, false);