From 8f3432945e5ac22f95cd338c249242ded5cbb639 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 6 Sep 2018 20:43:17 +0100 Subject: [PATCH] Don't scan data directory for archives each time a plugin is loaded Scan once per LoadPlugins() call and cache. --- src/api/game/game.cpp | 24 +++++++++++- src/api/game/game.h | 2 + src/api/game/game_cache.cpp | 18 +++++++++ src/api/game/game_cache.h | 5 +++ src/api/metadata/condition_evaluator.cpp | 2 +- src/api/plugin.cpp | 13 +++---- src/api/plugin.h | 7 +++- .../api/internals/game/game_cache_test.h | 37 +++++++++++++++++++ src/tests/api/internals/game/game_test.h | 30 +++++++++++++++ src/tests/api/internals/plugin_test.h | 32 ++++++++++++++++ 10 files changed, 159 insertions(+), 11 deletions(-) diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index 5518a300..0efaae75 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -149,8 +149,12 @@ void Game::LoadPlugins(const std::vector& plugins, ++currentGroup; } - // Clear the existing plugin cache. + // Clear the existing plugin and archive caches. cache_->ClearCachedPlugins(); + cache_->ClearCachedArchivePaths(); + + // Search for and cache archives. + CacheArchives(); // Load the plugins. if (logger) { @@ -168,7 +172,7 @@ void Game::LoadPlugins(const std::vector& plugins, boost::iequals(pluginName, masterFile_) || loadHeadersOnly; try { cache_->AddPlugin(Plugin( - Type(), DataPath(), loadOrderHandler_, pluginName, loadHeader)); + Type(), DataPath(), cache_, loadOrderHandler_, pluginName, loadHeader)); } catch (std::exception& e) { if (logger) { logger->error( @@ -238,4 +242,20 @@ std::vector Game::GetLoadOrder() const { void Game::SetLoadOrder(const std::vector& loadOrder) { loadOrderHandler_->SetLoadOrder(loadOrder); } + +void Game::CacheArchives() { + const auto archiveFileExtension = GetArchiveFileExtension(Type()); + + for (boost::filesystem::directory_iterator it(DataPath()); + it != boost::filesystem::directory_iterator(); + ++it) { + // Check if the path is an archive by checking if replacing its + // file extension with the archive extension resolves to the same file. + // Could use boost::iends_with, but it's less obvious here that the + // test string is ASCII-only. + if (boost::iequals(it->path().extension().string(), archiveFileExtension)) { + cache_->CacheArchivePath(it->path()); + } + } +} } diff --git a/src/api/game/game.h b/src/api/game/game.h index 4ed8a820..140f89ca 100644 --- a/src/api/game/game.h +++ b/src/api/game/game.h @@ -77,6 +77,8 @@ public: void SetLoadOrder(const std::vector& loadOrder); private: + void CacheArchives(); + std::shared_ptr cache_; std::shared_ptr loadOrderHandler_; std::shared_ptr database_; diff --git a/src/api/game/game_cache.cpp b/src/api/game/game_cache.cpp index 50529c10..7771f624 100644 --- a/src/api/game/game_cache.cpp +++ b/src/api/game/game_cache.cpp @@ -100,6 +100,18 @@ void GameCache::AddPlugin(const Plugin&& plugin) { std::make_shared(std::move(plugin))); } +std::set GameCache::GetArchivePaths() const +{ + return archivePaths_; +} + +void GameCache::CacheArchivePath(const boost::filesystem::path& path) +{ + lock_guard lock(mutex_); + + archivePaths_.insert(path); +} + void GameCache::ClearCachedConditions() { lock_guard guard(mutex_); @@ -111,4 +123,10 @@ void GameCache::ClearCachedPlugins() { plugins_.clear(); } + +void GameCache::ClearCachedArchivePaths() { + lock_guard guard(mutex_); + + archivePaths_.clear(); +} } diff --git a/src/api/game/game_cache.h b/src/api/game/game_cache.h index 8edd2c44..5a718572 100644 --- a/src/api/game/game_cache.h +++ b/src/api/game/game_cache.h @@ -47,12 +47,17 @@ public: std::shared_ptr GetPlugin(const std::string& pluginName) const; void AddPlugin(const Plugin&& plugin); + std::set GetArchivePaths() const; + void CacheArchivePath(const boost::filesystem::path& path); + void ClearCachedConditions(); void ClearCachedPlugins(); + void ClearCachedArchivePaths(); private: std::unordered_map conditions_; std::unordered_map> plugins_; + std::set archivePaths_; mutable std::mutex mutex_; }; diff --git a/src/api/metadata/condition_evaluator.cpp b/src/api/metadata/condition_evaluator.cpp index 8e6a1a13..c3c76502 100644 --- a/src/api/metadata/condition_evaluator.cpp +++ b/src/api/metadata/condition_evaluator.cpp @@ -458,7 +458,7 @@ Version ConditionEvaluator::getVersion(const std::string& filePath) const { // plugin file. if (Plugin::IsValid(filePath, gameType_, dataPath_)) return Version( - Plugin(gameType_, dataPath_, loadOrderHandler_, filePath, true) + Plugin(gameType_, dataPath_, gameCache_, loadOrderHandler_, filePath, true) .GetVersion()); return Version(dataPath_ / filePath); diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index 39768482..dbe01722 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -42,6 +42,7 @@ using std::string; namespace loot { Plugin::Plugin(const GameType gameType, const boost::filesystem::path& dataPath, + std::shared_ptr gameCache, std::shared_ptr loadOrderHandler, const std::string& name, const bool headerOnly) : @@ -118,7 +119,7 @@ Plugin::Plugin(const GameType gameType, // Get whether the plugin is active or not. isActive_ = loadOrderHandler->IsPluginActive(name_); - loadsArchive_ = LoadsArchive(name_, gameType, dataPath); + loadsArchive_ = LoadsArchive(name_, gameType, gameCache, dataPath); } catch (std::exception& e) { if (logger) { logger->error( @@ -302,7 +303,7 @@ std::string Plugin::GetDescription() const { return descriptionStr; } -std::string Plugin::GetArchiveFileExtension(const GameType gameType) { +std::string GetArchiveFileExtension(const GameType gameType) { if (gameType == GameType::fo4 || gameType == GameType::fo4vr) return ".ba2"; else @@ -311,6 +312,7 @@ std::string Plugin::GetArchiveFileExtension(const GameType gameType) { bool Plugin::LoadsArchive(const std::string& pluginName, const GameType gameType, + const std::shared_ptr gameCache, const boost::filesystem::path& dataPath) { // Get whether the plugin loads an archive (BSA/BA2) or not. const string archiveExtension = GetArchiveFileExtension(gameType); @@ -325,11 +327,8 @@ bool Plugin::LoadsArchive(const std::string& pluginName, // Oblivion .esp files and FO3, FNV, FO4 plugins can load archives which // begin with the plugin basename. string basename = pluginName.substr(0, pluginName.length() - 4); - for (boost::filesystem::directory_iterator it(dataPath); - it != boost::filesystem::directory_iterator(); - ++it) { - if (boost::iequals(it->path().extension().string(), archiveExtension) && - boost::istarts_with(it->path().filename().string(), basename)) { + for (const auto& archivePath : gameCache->GetArchivePaths()) { + if (boost::istarts_with(archivePath.filename().string(), basename)) { return true; } } diff --git a/src/api/plugin.h b/src/api/plugin.h index 1b2ed5db..241b97f4 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -39,10 +39,13 @@ #include "loot/plugin_interface.h" namespace loot { +class GameCache; + class Plugin : public PluginInterface { public: Plugin(const GameType gameType, const boost::filesystem::path& dataPath, + std::shared_ptr gameCache, std::shared_ptr loadOrderHandler, const std::string& name, const bool headerOnly); @@ -80,9 +83,9 @@ private: bool headerOnly); std::string GetDescription() const; - static std::string GetArchiveFileExtension(const GameType gameType); static bool LoadsArchive(const std::string& pluginName, const GameType gameType, + const std::shared_ptr gameCache, const boost::filesystem::path& dataPath); static unsigned int GetEspluginGameId(GameType gameType); @@ -101,6 +104,8 @@ private: std::shared_ptr::type> esPlugin; }; +std::string GetArchiveFileExtension(const GameType gameType); + bool hasPluginFileExtension(const std::string& filename, GameType gameType); } diff --git a/src/tests/api/internals/game/game_cache_test.h b/src/tests/api/internals/game/game_cache_test.h index 84728223..4669942e 100644 --- a/src/tests/api/internals/game/game_cache_test.h +++ b/src/tests/api/internals/game/game_cache_test.h @@ -73,6 +73,7 @@ TEST_P(GameCacheTest, gettingANonCachedConditionShouldReturnAFalseFalsePair) { TEST_P(GameCacheTest, addingAPluginThatDoesNotExistShouldSucceed) { cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, true)); @@ -83,6 +84,7 @@ TEST_P(GameCacheTest, addingAPluginThatIsAlreadyCachedShouldOverwriteExistingEntry) { cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, true)); @@ -90,6 +92,7 @@ TEST_P(GameCacheTest, cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, false)); @@ -103,6 +106,7 @@ TEST_P(GameCacheTest, gettingAPluginThatIsNotCachedShouldThrow) { TEST_P(GameCacheTest, gettingAPluginShouldBeCaseInsensitive) { cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, true)); @@ -118,11 +122,13 @@ TEST_P(GameCacheTest, gettingPluginsShouldReturnASetOfCachedPluginsIfPluginsHaveBeenCached) { cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, true)); cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankMasterDependentEsm, true)); @@ -130,6 +136,24 @@ TEST_P(GameCacheTest, EXPECT_FALSE(cache_.GetPlugins().empty()); } +TEST_P(GameCacheTest, + gettingArchivePathsShouldReturnAnEmptySetIfNoPathsHaveBeenCached) { + EXPECT_TRUE(cache_.GetArchivePaths().empty()); +} + +TEST_P(GameCacheTest, + gettingArchivePathsShouldReturnASetOfPathsIfPathsHaveBeenCached) { + cache_.CacheArchivePath(game_.DataPath() / blankEsm); + cache_.CacheArchivePath(game_.DataPath() / blankMasterDependentEsm); + + auto expected = std::set({ + game_.DataPath() / blankEsm, + game_.DataPath() / blankMasterDependentEsm, + }); + + EXPECT_EQ(expected, cache_.GetArchivePaths()); +} + TEST_P(GameCacheTest, clearingCachedConditionsShouldNotThrowIfNoConditionsAreCached) { EXPECT_NO_THROW(cache_.ClearCachedConditions()); @@ -151,6 +175,7 @@ TEST_P(GameCacheTest, clearingCachedPluginsShouldNotThrowIfNoPluginsAreCached) { TEST_P(GameCacheTest, clearingCachedPluginsShouldClearAnyCachedPlugins) { cache_.AddPlugin(Plugin(game_.Type(), game_.DataPath(), + std::make_shared(GameCache()), game_.GetLoadOrderHandler(), blankEsm, true)); @@ -158,6 +183,18 @@ TEST_P(GameCacheTest, clearingCachedPluginsShouldClearAnyCachedPlugins) { EXPECT_TRUE(cache_.GetPlugins().empty()); } + +TEST_P(GameCacheTest, + clearingCachedArchivePathsShouldNotThrowIfNoPathsAreCached) { + EXPECT_NO_THROW(cache_.GetArchivePaths()); +} + +TEST_P(GameCacheTest, clearingCachedArchivePathsShouldClearAnyCachedPaths) { + cache_.CacheArchivePath(game_.DataPath() / blankEsm); + cache_.ClearCachedArchivePaths(); + + EXPECT_TRUE(cache_.GetArchivePaths().empty()); +} } } diff --git a/src/tests/api/internals/game/game_test.h b/src/tests/api/internals/game/game_test.h index deb90f4f..db3d8f83 100644 --- a/src/tests/api/internals/game/game_test.h +++ b/src/tests/api/internals/game/game_test.h @@ -33,6 +33,13 @@ namespace loot { namespace test { class GameTest : public CommonGameTestFixture { protected: + GameTest() : + blankArchive("Blank" + GetArchiveFileExtension(GetParam())) { + + boost::filesystem::ofstream out(dataPath / blankArchive); + out.close(); + } + void loadInstalledPlugins(Game& game, bool headersOnly) { const std::vector plugins({ masterFile, @@ -49,6 +56,8 @@ protected: }); game.LoadPlugins(plugins, headersOnly); } + + const std::string blankArchive; }; // Pass an empty first argument, as it's a prefix for the test instantation, @@ -147,6 +156,27 @@ TEST_P(GameTest, EXPECT_EQ(blankEsmCrc, plugin->GetCRC()); } +TEST_P(GameTest, + loadPluginsShouldFindAndCacheArchivesForLoadDetectionWhenLoadingPlugins) { + Game game = Game(GetParam(), dataPath.parent_path(), localPath); + + EXPECT_NO_THROW(loadInstalledPlugins(game, false)); + + auto expected = std::set({ + dataPath / blankArchive + }); + EXPECT_EQ(expected, game.GetCache()->GetArchivePaths()); +} + +TEST_P(GameTest, + loadPluginsShouldClearTheArchivesCacheBeforeFindingArchives) { + Game game = Game(GetParam(), dataPath.parent_path(), localPath); + + EXPECT_NO_THROW(loadInstalledPlugins(game, false)); + EXPECT_NO_THROW(loadInstalledPlugins(game, false)); + EXPECT_EQ(1, game.GetCache()->GetArchivePaths().size()); +} + TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasNotBeenLoaded) { Game game = Game(GetParam(), dataPath.parent_path(), localPath); game.LoadCurrentLoadOrderState(); diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index f0bca1b9..04b22820 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -67,6 +67,9 @@ protected: out.close(); out.open(dataPath / blankSuffixArchive); out.close(); + + game_.GetCache()->CacheArchivePath(dataPath / blankArchive); + game_.GetCache()->CacheArchivePath(dataPath / blankSuffixArchive); } Game game_; @@ -117,6 +120,7 @@ INSTANTIATE_TEST_CASE_P(, TEST_P(PluginTest, loadingHeaderOnlyShouldReadHeaderData) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); @@ -131,6 +135,7 @@ TEST_P(PluginTest, loadingHeaderOnlyShouldReadHeaderData) { TEST_P(PluginTest, loadingHeaderOnlyShouldNotReadFieldsOrCalculateCrc) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); @@ -141,6 +146,7 @@ TEST_P(PluginTest, loadingHeaderOnlyShouldNotReadFieldsOrCalculateCrc) { TEST_P(PluginTest, loadingWholePluginShouldReadHeaderData) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); @@ -155,6 +161,7 @@ TEST_P(PluginTest, loadingWholePluginShouldReadHeaderData) { TEST_P(PluginTest, loadingWholePluginShouldReadFields) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsm, false); @@ -165,6 +172,7 @@ TEST_P(PluginTest, loadingWholePluginShouldReadFields) { TEST_P(PluginTest, loadingWholePluginShouldCalculateCrc) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, false); @@ -175,6 +183,7 @@ TEST_P(PluginTest, loadingWholePluginShouldCalculateCrc) { TEST_P(PluginTest, loadingANonMasterPluginShouldReadTheMasterFlagAsFalse) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsp, true); @@ -187,16 +196,19 @@ TEST_P( isLightMasterShouldBeTrueForAPluginWithEslFileExtensionForFallout4AndSkyrimSeAndFalseOtherwise) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); Plugin plugin2(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsp, true); Plugin plugin3(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsl, true); @@ -210,6 +222,7 @@ TEST_P( TEST_P(PluginTest, loadingAPluginWithMastersShouldReadThemCorrectly) { Plugin plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsp, true); @@ -220,6 +233,7 @@ TEST_P(PluginTest, loadingAPluginWithMastersShouldReadThemCorrectly) { TEST_P(PluginTest, loadingAPluginThatDoesNotExistShouldThrow) { EXPECT_THROW(Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), "Blank\\.esp", true), @@ -231,6 +245,7 @@ TEST_P( loadsArchiveForAnArchiveThatExactlyMatchesAnEsmFileBasenameShouldReturnTrueForAllGamesExceptOblivion) { bool loadsArchive = Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true) @@ -247,6 +262,7 @@ TEST_P( loadsArchiveForAnArchiveThatExactlyMatchesAnEspFileBasenameShouldReturnTrue) { EXPECT_TRUE(Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsp, true) @@ -258,6 +274,7 @@ TEST_P( loadsArchiveForAnArchiveWithAFilenameWhichStartsWithTheEsmFileBasenameShouldReturnTrueForAllGamesExceptOblivionAndSkyrim) { bool loadsArchive = Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankDifferentEsm, true) @@ -274,6 +291,7 @@ TEST_P( loadsArchiveForAnArchiveWithAFilenameWhichStartsWithTheEspFileBasenameShouldReturnTrueForAllGamesExceptSkyrim) { bool loadsArchive = Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankDifferentEsp, true) @@ -289,6 +307,7 @@ TEST_P(PluginTest, loadsArchiveShouldReturnFalseForAPluginThatDoesNotLoadAnArchive) { EXPECT_FALSE(Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsp, true) @@ -310,6 +329,7 @@ TEST_P(PluginTest, isValidShouldReturnFalseForAnEmptyFile) { TEST_P(PluginTest, isActiveShouldReturnTrueForAPluginThatIsActive) { EXPECT_TRUE(Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true) @@ -319,6 +339,7 @@ TEST_P(PluginTest, isActiveShouldReturnTrueForAPluginThatIsActive) { TEST_P(PluginTest, isActiveShouldReturnFalseForAPluginThatIsNotActive) { EXPECT_FALSE(Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsp, true) @@ -329,11 +350,13 @@ TEST_P(PluginTest, lessThanOperatorShouldUseCaseInsensitiveLexicographicalNameComparison) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsp, true); Plugin plugin2(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), lowercaseBlankEsp, true); @@ -343,11 +366,13 @@ TEST_P(PluginTest, Plugin plugin3 = Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); Plugin plugin4 = Plugin(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsp, true); @@ -360,6 +385,7 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseIfTheArgumentIsNotAPluginObject) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, false); @@ -373,11 +399,13 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseForTwoPluginsWithOnlyHeadersLoaded) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, true); Plugin plugin2(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsm, true); @@ -390,11 +418,13 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnFalseIfThePluginsHaveUnrelatedRecords) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, false); Plugin plugin2(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsp, false); @@ -407,11 +437,13 @@ TEST_P(PluginTest, doFormIDsOverlapShouldReturnTrueIfOnePluginOverridesTheOthersRecords) { Plugin plugin1(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankEsm, false); Plugin plugin2(game_.Type(), game_.DataPath(), + game_.GetCache(), game_.GetLoadOrderHandler(), blankMasterDependentEsm, false);