From e4fa201dd855b06507cbf91ecbca0afeb52783ab Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 15 Mar 2025 20:53:06 +0000 Subject: [PATCH] Fix LoadPlugins() with missing already-loaded plugins --- src/api/game/game.cpp | 14 ++++----- src/api/game/game_cache.cpp | 22 +++++++++++++-- src/api/game/game_cache.h | 3 ++ src/api/plugin.cpp | 6 ++-- src/api/plugin.h | 2 +- src/tests/api/internals/game/game_test.h | 36 ++++++++++++++++++++++++ src/tests/api/internals/plugin_test.h | 24 ++++++++-------- 7 files changed, 81 insertions(+), 26 deletions(-) diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index 549dd02c..25d8112a 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -293,7 +293,9 @@ void Game::LoadPlugins(const std::vector& pluginPaths, if (!loadHeadersOnly && (GetType() == GameType::tes3 || GetType() == GameType::openmw || GetType() == GameType::starfield)) { - const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); + const auto loadedPlugins = cache_.GetPluginsWithReplacements(plugins); + + const auto pluginsMetadata = Plugin::GetPluginsMetadata(loadedPlugins); for (auto& plugin : plugins) { plugin.ResolveRecordIds(pluginsMetadata.get()); } @@ -306,9 +308,7 @@ void Game::LoadPlugins(const std::vector& pluginPaths, conditionEvaluator_->RefreshLoadedPluginsState(GetLoadedPlugins()); } -void Game::ClearLoadedPlugins() { - cache_.ClearCachedPlugins(); -} +void Game::ClearLoadedPlugins() { cache_.ClearCachedPlugins(); } const PluginInterface* Game::GetPlugin(const std::string& pluginName) const { return cache_.GetPlugin(pluginName); @@ -348,9 +348,9 @@ std::vector Game::SortPlugins( const auto newLoadOrder = loot::SortPlugins(std::move(pluginsSortingData), - database_.GetGroups(false), - database_.GetUserGroups(), - loadOrderHandler_.GetEarlyLoadingPlugins()); + database_.GetGroups(false), + database_.GetUserGroups(), + loadOrderHandler_.GetEarlyLoadingPlugins()); if (logger) { logger->debug("Calculated order:"); diff --git a/src/api/game/game_cache.cpp b/src/api/game/game_cache.cpp index 2fef1751..a00b3c85 100644 --- a/src/api/game/game_cache.cpp +++ b/src/api/game/game_cache.cpp @@ -56,6 +56,24 @@ void GameCache::AddPlugin(Plugin&& plugin) { } } +std::vector GameCache::GetPluginsWithReplacements( + const std::vector& newPlugins) const { + std::unordered_map pluginsMap; + for (const auto& plugin : newPlugins) { + pluginsMap.emplace(NormalizeFilename(plugin.GetName()), &plugin); + } + for (const auto& [key, plugin] : plugins_) { + pluginsMap.emplace(key, plugin.get()); + } + + std::vector loadedPlugins; + for (const auto& [key, plugin] : pluginsMap) { + loadedPlugins.push_back(plugin); + } + + return loadedPlugins; +} + std::set GameCache::GetArchivePaths() const { return archivePaths_; } @@ -64,7 +82,5 @@ void GameCache::CacheArchivePaths(std::set&& paths) { archivePaths_ = std::move(paths); } -void GameCache::ClearCachedPlugins() { - plugins_.clear(); -} +void GameCache::ClearCachedPlugins() { plugins_.clear(); } } diff --git a/src/api/game/game_cache.h b/src/api/game/game_cache.h index 343d7c0f..af70a17f 100644 --- a/src/api/game/game_cache.h +++ b/src/api/game/game_cache.h @@ -37,6 +37,9 @@ public: const Plugin* GetPlugin(const std::string& pluginName) const; void AddPlugin(Plugin&& plugin); + std::vector GetPluginsWithReplacements( + const std::vector& newPlugins) const; + std::set GetArchivePaths() const; void CacheArchivePaths(std::set&& paths); diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index 21fc30ac..e4298ae9 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -593,7 +593,7 @@ std::string Plugin::GetDescription() const { } std::unique_ptr -Plugin::GetPluginsMetadata(const std::vector& plugins) { +Plugin::GetPluginsMetadata(const std::vector& plugins) { if (plugins.empty()) { return std::unique_ptr( @@ -603,9 +603,9 @@ Plugin::GetPluginsMetadata(const std::vector& plugins) { std::vector esPlugins; esPlugins.reserve(plugins.size()); for (const auto& plugin : plugins) { - const auto esPlugin = plugin.esPlugin.get(); + const auto esPlugin = plugin->esPlugin.get(); if (esPlugin != nullptr) { - esPlugins.push_back(plugin.esPlugin.get()); + esPlugins.push_back(plugin->esPlugin.get()); } } diff --git a/src/api/plugin.h b/src/api/plugin.h index 543eb22d..5c741eec 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -91,7 +91,7 @@ public: static std::unique_ptr - GetPluginsMetadata(const std::vector& plugins); + GetPluginsMetadata(const std::vector& plugins); private: void Load(const std::filesystem::path& path, diff --git a/src/tests/api/internals/game/game_test.h b/src/tests/api/internals/game/game_test.h index 9dc5df62..491827d1 100644 --- a/src/tests/api/internals/game/game_test.h +++ b/src/tests/api/internals/game/game_test.h @@ -445,6 +445,42 @@ TEST_P( } } +TEST_P( + GameTest, + loadPluginsShouldThrowIfAPluginHasAMasterThatIsNotInTheInputAndIsNotAlreadyLoadedAndGameIsMorrowindOrStarfield) { + Game game = Game(GetParam(), gamePath, localPath); + + if (GetParam() == GameType::tes3 || GetParam() == GameType::openmw || + GetParam() == GameType::starfield) { + try { + game.LoadPlugins({blankMasterDependentEsm}, false); + FAIL(); + } catch (const std::system_error& e) { + EXPECT_EQ(ESP_ERROR_PLUGIN_METADATA_NOT_FOUND, e.code().value()); + EXPECT_EQ(esplugin_category(), e.code().category()); + } + } else { + game.LoadPlugins({blankMasterDependentEsm}, false); + + EXPECT_NE(nullptr, game.GetPlugin(blankMasterDependentEsm)); + } +} + +TEST_P( + GameTest, + loadPluginsShouldNotThrowIfAPluginHasAMasterThatIsNotInTheInputButIsAlreadyLoaded) { + Game game = Game(GetParam(), gamePath, localPath); + + const auto pluginName = + GetParam() == GameType::starfield ? blankFullEsm : blankEsm; + + game.LoadPlugins({pluginName}, true); + + game.LoadPlugins({blankMasterDependentEsm}, false); + + EXPECT_NE(nullptr, game.GetPlugin(blankMasterDependentEsm)); +} + TEST_P(GameTest, sortPluginsWithNoLoadedPluginsShouldReturnAnEmptyList) { Game game = Game(GetParam(), gamePath, localPath); diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index ce009bf7..1470a861 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -353,21 +353,19 @@ TEST_P(PluginTest, loadingWholePluginShouldReadFields) { game_.GetType(), game_.GetCache(), game_.DataPath() / pluginName, false); if (GetParam() == GameType::tes3 || GetParam() == GameType::openmw) { - std::vector masters; - masters.push_back(Plugin( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false)); - const auto pluginsMetadata = Plugin::GetPluginsMetadata(masters); + Plugin master( + game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false); + const auto pluginsMetadata = Plugin::GetPluginsMetadata({&master}); EXPECT_NO_THROW(plugin.ResolveRecordIds(pluginsMetadata.get())); EXPECT_EQ(4, plugin.GetOverrideRecordCount()); } else if (GetParam() == GameType::starfield) { - std::vector masters; - masters.push_back(Plugin(game_.GetType(), - game_.GetCache(), - game_.DataPath() / blankFullEsm, - true)); - const auto pluginsMetadata = Plugin::GetPluginsMetadata(masters); + Plugin master(game_.GetType(), + game_.GetCache(), + game_.DataPath() / blankFullEsm, + true); + const auto pluginsMetadata = Plugin::GetPluginsMetadata({&master}); EXPECT_NO_THROW(plugin.ResolveRecordIds(pluginsMetadata.get())); @@ -709,7 +707,8 @@ TEST_P( false)); if (GetParam() == GameType::starfield) { - const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); + const auto pluginsMetadata = + Plugin::GetPluginsMetadata({&plugins[0], &plugins[1]}); plugins[1].ResolveRecordIds(pluginsMetadata.get()); plugins[0].ResolveRecordIds(nullptr); @@ -782,7 +781,8 @@ TEST_P(PluginTest, if (GetParam() == GameType::starfield) { plugins[0].ResolveRecordIds(nullptr); - const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); + const auto pluginsMetadata = + Plugin::GetPluginsMetadata({&plugins[0], &plugins[1]}); plugins[1].ResolveRecordIds(pluginsMetadata.get()); }