From 49fc7910eaf9123282cc35d291644bdca1985ff1 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 1 Mar 2025 12:02:14 +0000 Subject: [PATCH] Cache plugins only after resolving record IDs This means that if there's an issue while doing that you won't be left with half-loaded plugins. --- src/api/game/game.cpp | 14 ++++-- src/api/plugin.cpp | 6 +-- src/api/plugin.h | 2 +- src/tests/api/internals/plugin_test.h | 71 +++++++++++++++------------ 4 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index 725582a2..650b643b 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -264,6 +264,7 @@ void Game::LoadPlugins(const std::vector& pluginPaths, } std::mutex mutex; + std::vector plugins; std::for_each( std::execution::par_unseq, pluginPaths.begin(), @@ -273,10 +274,12 @@ void Game::LoadPlugins(const std::vector& pluginPaths, const auto resolvedPluginPath = ResolvePluginPath(GetType(), DataPath(), pluginPath); + auto plugin = + Plugin(GetType(), cache_, resolvedPluginPath, loadHeadersOnly); + std::lock_guard lock(mutex); - cache_.AddPlugin( - Plugin(GetType(), cache_, resolvedPluginPath, loadHeadersOnly)); + plugins.push_back(std::move(plugin)); } catch (const std::exception& e) { if (logger) { logger->error( @@ -290,13 +293,16 @@ void Game::LoadPlugins(const std::vector& pluginPaths, if (!loadHeadersOnly && (GetType() == GameType::tes3 || GetType() == GameType::openmw || GetType() == GameType::starfield)) { - auto plugins = cache_.GetPlugins(); const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); for (auto& plugin : plugins) { - plugin->ResolveRecordIds(pluginsMetadata.get()); + plugin.ResolveRecordIds(pluginsMetadata.get()); } } + for (auto& plugin : plugins) { + cache_.AddPlugin(std::move(plugin)); + } + conditionEvaluator_->RefreshLoadedPluginsState(GetLoadedPlugins()); } diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index e4298ae9..21fc30ac 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 5c741eec..543eb22d 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/plugin_test.h b/src/tests/api/internals/plugin_test.h index f6ebf19d..fbb53b66 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -286,19 +286,21 @@ TEST_P(PluginTest, loadingWholePluginShouldReadFields) { game_.GetType(), game_.GetCache(), game_.DataPath() / pluginName, false); if (GetParam() == GameType::tes3 || GetParam() == GameType::openmw) { - Plugin master( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false); - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&master}); + std::vector masters; + masters.push_back(Plugin( + game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false)); + const auto pluginsMetadata = Plugin::GetPluginsMetadata(masters); EXPECT_NO_THROW(plugin.ResolveRecordIds(pluginsMetadata.get())); EXPECT_EQ(4, plugin.GetOverrideRecordCount()); } else if (GetParam() == GameType::starfield) { - Plugin master(game_.GetType(), - game_.GetCache(), - game_.DataPath() / blankFullEsm, - true); - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&master}); + std::vector masters; + masters.push_back(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / blankFullEsm, + true)); + const auto pluginsMetadata = Plugin::GetPluginsMetadata(masters); EXPECT_NO_THROW(plugin.ResolveRecordIds(pluginsMetadata.get())); @@ -629,25 +631,27 @@ TEST_P( ? blankMasterDependentEsp : blankDifferentPluginDependentEsp; - Plugin plugin1(game_.GetType(), - game_.GetCache(), - game_.DataPath() / sourcePluginName, - false); - Plugin plugin2(game_.GetType(), - game_.GetCache(), - game_.DataPath() / updatePluginName, - false); + std::vector plugins; + plugins.push_back(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / sourcePluginName, + false)); + plugins.push_back(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / updatePluginName, + false)); if (GetParam() == GameType::starfield) { - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&plugin1}); - plugin2.ResolveRecordIds(pluginsMetadata.get()); + const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); + plugins[1].ResolveRecordIds(pluginsMetadata.get()); - plugin1.ResolveRecordIds(nullptr); - plugin2.ResolveRecordIds(nullptr); + plugins[0].ResolveRecordIds(nullptr); + plugins[1].ResolveRecordIds(nullptr); } - EXPECT_FALSE(plugin1.IsValidAsUpdatePlugin()); - EXPECT_EQ(GetParam() == GameType::starfield, plugin2.IsValidAsUpdatePlugin()); + EXPECT_FALSE(plugins[0].IsValidAsUpdatePlugin()); + EXPECT_EQ(GetParam() == GameType::starfield, + plugins[1].IsValidAsUpdatePlugin()); } TEST_P(PluginTest, @@ -699,20 +703,25 @@ TEST_P(PluginTest, ? blankMasterDependentEsm : blankMasterDependentEsm + ".ghost"; - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / plugin1Name, false); - Plugin plugin2( - game_.GetType(), game_.GetCache(), game_.DataPath() / plugin2Name, false); + std::vector plugins; + plugins.push_back(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / plugin1Name, + false)); + plugins.push_back(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / plugin2Name, + false)); if (GetParam() == GameType::starfield) { - plugin1.ResolveRecordIds(nullptr); + plugins[0].ResolveRecordIds(nullptr); - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&plugin1}); - plugin2.ResolveRecordIds(pluginsMetadata.get()); + const auto pluginsMetadata = Plugin::GetPluginsMetadata(plugins); + plugins[1].ResolveRecordIds(pluginsMetadata.get()); } - EXPECT_TRUE(plugin1.DoRecordsOverlap(plugin2)); - EXPECT_TRUE(plugin2.DoRecordsOverlap(plugin1)); + EXPECT_TRUE(plugins[0].DoRecordsOverlap(plugins[1])); + EXPECT_TRUE(plugins[1].DoRecordsOverlap(plugins[0])); } TEST_P(PluginTest,