From 2497b5b6ce2fe7cd20a5cfe65c8e55bee61756f2 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 24 Apr 2023 20:52:46 +0100 Subject: [PATCH] Refactor IsValidPlugin, LoadPlugins and SortPlugins Add new methods that take std::filesystem::path objects instead of std::string objects, and which contain the logic, and make the methods taking std::string objects into thin wrappers around them. The new methods avoid a repeated conversions to paths, and will replace the old methods in the public API at some point. --- src/api/game/game.cpp | 64 ++++++++++++++++-------- src/api/game/game.h | 8 +++ src/tests/api/internals/game/game_test.h | 17 ++++--- 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index 1dbb18f1..83ccde0c 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -127,6 +127,16 @@ std::vector FindArchives( return archivePaths; } + +std::vector StringsToPaths( + const std::vector& pluginPathStrings) { + std::vector pluginPaths; + for (const auto& pluginPathString : pluginPathStrings) { + pluginPaths.push_back(u8path(pluginPathString)); + } + + return pluginPaths; +} } namespace loot { @@ -177,19 +187,26 @@ void Game::SetAdditionalDataPaths( } bool Game::IsValidPlugin(const std::string& pluginPath) const { - return Plugin::IsValid(Type(), - ResolvePluginPath(DataPath(), u8path(pluginPath))); + return IsValidPlugin(u8path(pluginPath)); +} + +bool Game::IsValidPlugin(const std::filesystem::path& pluginPath) const { + return Plugin::IsValid(Type(), ResolvePluginPath(DataPath(), pluginPath)); } -void Game::LoadPlugins(const std::vector& pluginPaths, +void Game::LoadPlugins(const std::vector& pluginPathStrings, + bool loadHeadersOnly) { + return LoadPlugins(StringsToPaths(pluginPathStrings), loadHeadersOnly); +} + +void Game::LoadPlugins(const std::vector& pluginPaths, bool loadHeadersOnly) { const auto logger = getLogger(); // Check that all plugin filenames are unique. std::unordered_set filenames; for (const auto& pluginPath : pluginPaths) { - const auto filename = - NormalizeFilename(u8path(pluginPath).filename().u8string()); + const auto filename = NormalizeFilename(pluginPath.filename().u8string()); const auto inserted = filenames.insert(filename).second; if (!inserted) { throw std::invalid_argument("The filename \"" + filename + @@ -203,7 +220,7 @@ void Game::LoadPlugins(const std::vector& pluginPaths, std::find_if(std::execution::par_unseq, pluginPaths.cbegin(), pluginPaths.cend(), - [this](const std::string& pluginPath) { + [this](const std::filesystem::path& pluginPath) { try { return !IsValidPlugin(pluginPath); } catch (...) { @@ -212,7 +229,7 @@ void Game::LoadPlugins(const std::vector& pluginPaths, }); if (invalidPluginIt != pluginPaths.end()) { - throw std::invalid_argument("\"" + *invalidPluginIt + + throw std::invalid_argument("\"" + invalidPluginIt->u8string() + "\" is not a valid plugin"); } @@ -232,24 +249,27 @@ void Game::LoadPlugins(const std::vector& pluginPaths, std::execution::par_unseq, pluginPaths.begin(), pluginPaths.end(), - [&](const std::string& pluginPathString) { + [&](const std::filesystem::path& pluginPath) { try { - const auto endIt = - boost::iends_with(pluginPathString, GHOST_FILE_EXTENSION) - ? pluginPathString.end() - GHOST_FILE_EXTENSION_LENGTH - : pluginPathString.end(); + const auto resolvedPluginPath = + boost::iequals(pluginPath.extension().u8string(), + GHOST_FILE_EXTENSION) + ? ResolvePluginPath( + DataPath(), + std::filesystem::path(pluginPath).replace_extension()) + : ResolvePluginPath(DataPath(), pluginPath); - const auto pluginPath = ResolvePluginPath( - DataPath(), u8path(pluginPathString.begin(), endIt)); const bool loadHeader = - loadHeadersOnly || loot::equivalent(pluginPath, masterPath); + loadHeadersOnly || + loot::equivalent(resolvedPluginPath, masterPath); - cache_.AddPlugin(Plugin(Type(), cache_, pluginPath, loadHeader)); + cache_.AddPlugin( + Plugin(Type(), cache_, resolvedPluginPath, loadHeader)); } catch (const std::exception& e) { if (logger) { logger->error( "Caught exception while trying to add {} to the cache: {}", - pluginPathString, + pluginPath.u8string(), e.what()); } } @@ -276,13 +296,17 @@ void Game::IdentifyMainMasterFile(const std::string& masterFile) { } std::vector Game::SortPlugins( - const std::vector& pluginPaths) { + const std::vector& pluginPathStrings) { + return SortPlugins(StringsToPaths(pluginPathStrings)); +} + +std::vector Game::SortPlugins( + const std::vector& pluginPaths) { LoadPlugins(pluginPaths, false); std::vector loadOrder; for (const auto& pluginPath : pluginPaths) { - const auto filename = u8path(pluginPath).filename().u8string(); - loadOrder.push_back(filename); + loadOrder.push_back(pluginPath.filename().u8string()); } // Sort plugins into their load order. diff --git a/src/api/game/game.h b/src/api/game/game.h index e8730ce8..f9e815f5 100644 --- a/src/api/game/game.h +++ b/src/api/game/game.h @@ -58,6 +58,14 @@ class Game final : public GameInterface { void SetAdditionalDataPaths( const std::vector& additionalDataPaths); + bool IsValidPlugin(const std::filesystem::path& pluginPath) const; + + void LoadPlugins(const std::vector& pluginPaths, + bool loadHeadersOnly); + + std::vector SortPlugins( + const std::vector& pluginPaths); + // Game Interface Methods // //////////////////////////// diff --git a/src/tests/api/internals/game/game_test.h b/src/tests/api/internals/game/game_test.h index b6f976df..d7a8a6cd 100644 --- a/src/tests/api/internals/game/game_test.h +++ b/src/tests/api/internals/game/game_test.h @@ -181,7 +181,9 @@ TEST_P( TEST_P(GameTest, loadPluginsWithANonPluginShouldNotAddItToTheLoadedPlugins) { Game game = Game(GetParam(), dataPath.parent_path(), localPath); - ASSERT_THROW(game.LoadPlugins({nonPluginFile}, false), std::invalid_argument); + ASSERT_THROW( + game.LoadPlugins(std::vector({nonPluginFile}), false), + std::invalid_argument); ASSERT_TRUE(game.GetLoadedPlugins().empty()); } @@ -198,7 +200,8 @@ TEST_P(GameTest, Game game = Game(GetParam(), dataPath.parent_path(), localPath); - ASSERT_NO_THROW(game.LoadPlugins({invalidPlugin}, false)); + ASSERT_NO_THROW( + game.LoadPlugins(std::vector({invalidPlugin}), false)); ASSERT_TRUE(game.GetLoadedPlugins().empty()); } @@ -298,7 +301,8 @@ TEST_P(GameTest, getSourcePluginsPath() / std::filesystem::u8path(blankEsm); EXPECT_THROW( - game.LoadPlugins({dataPluginPath.u8string(), sourcePluginPath.u8string()}, + game.LoadPlugins(std::vector({dataPluginPath.u8string(), + sourcePluginPath.u8string()}), true), std::invalid_argument); } @@ -309,7 +313,7 @@ TEST_P(GameTest, loadPluginsShouldResolveRelativePathsRelativeToDataPath) { const auto relativePath = "../" + dataPath.filename().u8string() + "/" + blankEsm; - game.LoadPlugins({relativePath}, true); + game.LoadPlugins(std::vector({relativePath}), true); EXPECT_NE(nullptr, game.GetPlugin(blankEsm)); } @@ -319,7 +323,7 @@ TEST_P(GameTest, loadPluginsShouldUseAbsolutePathsAsGiven) { const auto absolutePath = dataPath / std::filesystem::u8path(blankEsm); - game.LoadPlugins({absolutePath.u8string()}, true); + game.LoadPlugins(std::vector({absolutePath.u8string()}), true); EXPECT_NE(nullptr, game.GetPlugin(blankEsm)); } @@ -329,7 +333,8 @@ TEST_P(GameTest, sortPluginsShouldHandlePluginPathsThatAreNotJustFilenames) { const auto absolutePath = dataPath / std::filesystem::u8path(blankEsm); - const auto newLoadOrder = game.SortPlugins({absolutePath.u8string()}); + const auto newLoadOrder = + game.SortPlugins(std::vector({absolutePath.u8string()})); EXPECT_EQ(std::vector{blankEsm}, newLoadOrder); }