diff --git a/cpp/include/loot/game_interface.h b/cpp/include/loot/game_interface.h index ce2e7059..71238b84 100644 --- a/cpp/include/loot/game_interface.h +++ b/cpp/include/loot/game_interface.h @@ -135,7 +135,9 @@ public: * @param pluginName * The filename of the plugin to get data for. * @returns A shared pointer to a const PluginInterface implementation. The - * pointer is null if the given plugin has not been loaded. + * pointer is null if the given plugin has not been loaded. Repeated + * calls given the same filename may return different pointers to + * equivalent objects. */ virtual std::shared_ptr GetPlugin( std::string_view pluginName) const = 0; @@ -144,6 +146,8 @@ public: * @brief Get a set of const references to all loaded plugins' PluginInterface * objects. * @returns A set of shared pointers to const PluginInterface objects. + * Repeated calls may return different pointers to equivalent + * objects. */ virtual std::vector> GetLoadedPlugins() const = 0; diff --git a/cpp/src/api/game.cpp b/cpp/src/api/game.cpp index 1a4612af..9932a895 100644 --- a/cpp/src/api/game.cpp +++ b/cpp/src/api/game.cpp @@ -167,45 +167,20 @@ void Game::LoadPlugins(const std::vector& pluginPaths, } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } - - std::lock_guard guard(pluginsMutex_); - - for (const auto& path : pluginPaths) { - auto key = Filename(path.filename().u8string()); - auto it = plugins_.find(key); - if (it != plugins_.end()) { - plugins_.erase(it); - } - } } -void Game::ClearLoadedPlugins() { - game_->clear_loaded_plugins(); - - std::lock_guard guard(pluginsMutex_); - plugins_.clear(); -} +void Game::ClearLoadedPlugins() { game_->clear_loaded_plugins(); } std::shared_ptr Game::GetPlugin( std::string_view pluginName) const { - std::lock_guard guard(pluginsMutex_); - - auto key = Filename(pluginName); - const auto it = plugins_.find(key); - if (it != plugins_.end()) { - return it->second; - } - const auto pluginOpt = game_->plugin(convert(pluginName)); if (!pluginOpt->is_some()) { return nullptr; } try { - std::shared_ptr plugin = - std::make_shared(std::move(pluginOpt->as_ref().boxed_clone())); - - return plugins_.emplace(key, plugin).first->second; + return std::make_shared( + std::move(pluginOpt->as_ref().boxed_clone())); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } @@ -213,18 +188,10 @@ std::shared_ptr Game::GetPlugin( std::vector> Game::GetLoadedPlugins() const { - std::lock_guard guard(pluginsMutex_); - std::vector> plugins; for (const auto& pluginRef : game_->loaded_plugins()) { - auto key = Filename(convert(pluginRef.name())); - auto it = plugins_.find(key); - if (it == plugins_.end()) { - std::shared_ptr plugin = - std::make_shared(std::move(pluginRef.boxed_clone())); - it = plugins_.emplace(key, plugin).first; - } - plugins.push_back(it->second); + plugins.push_back( + std::make_shared(std::move(pluginRef.boxed_clone()))); } return plugins; diff --git a/cpp/src/api/game.h b/cpp/src/api/game.h index bd688571..ea660a77 100644 --- a/cpp/src/api/game.h +++ b/cpp/src/api/game.h @@ -59,9 +59,6 @@ public: private: ::rust::Box game_; Database database_; - - mutable std::map> plugins_; - mutable std::mutex pluginsMutex_; }; } diff --git a/cpp/src/tests/api/interface/game_interface_test.h b/cpp/src/tests/api/interface/game_interface_test.h index 965dda2a..692d2af4 100644 --- a/cpp/src/tests/api/interface/game_interface_test.h +++ b/cpp/src/tests/api/interface/game_interface_test.h @@ -317,10 +317,13 @@ TEST_P(GameInterfaceTest, TEST_P(GameInterfaceTest, loadPluginsShouldNotClearThePluginsCache) { handle_->LoadPlugins({std::filesystem::u8path(blankEsm)}, true); ASSERT_EQ(1, handle_->GetLoadedPlugins().size()); + ASSERT_NE(nullptr, handle_->GetPlugin(blankEsm)); handle_->LoadPlugins({std::filesystem::u8path(blankEsp)}, true); EXPECT_EQ(2, handle_->GetLoadedPlugins().size()); + ASSERT_NE(nullptr, handle_->GetPlugin(blankEsm)); + ASSERT_NE(nullptr, handle_->GetPlugin(blankEsp)); } TEST_P(GameInterfaceTest, @@ -336,17 +339,6 @@ TEST_P(GameInterfaceTest, EXPECT_NE(pointer, newPointer); } -TEST_P(GameInterfaceTest, loadPluginsShouldNotAffectExistingPluginPointers) { - handle_->LoadPlugins({std::filesystem::u8path(blankEsm)}, true); - const auto pointer = handle_->GetPlugin(blankEsm); - ASSERT_NE(nullptr, pointer); - - handle_->LoadPlugins({std::filesystem::u8path(blankEsp)}, true); - - const auto newPointer = handle_->GetPlugin(blankEsm); - EXPECT_EQ(pointer, newPointer); -} - TEST_P(GameInterfaceTest, loadPluginsShouldThrowIfGivenVectorElementsWithTheSameFilename) { const auto dataPluginPath = dataPath / std::filesystem::u8path(blankEsm); @@ -533,13 +525,13 @@ TEST_P(GameInterfaceTest, getPluginThatIsNotCachedShouldReturnANullPointer) { } TEST_P(GameInterfaceTest, - getPluginReturnsTheSamePointerForConsecutiveCallsGivenTheSamePlugin) { + getPluginReturnsDifferentPointersForConsecutiveCallsGivenTheSamePlugin) { handle_->LoadPlugins({std::filesystem::u8path(blankEsm)}, true); const auto pointer1 = handle_->GetPlugin(blankEsm); const auto pointer2 = handle_->GetPlugin(blankEsm); - EXPECT_EQ(pointer1, pointer2); + EXPECT_NE(pointer1, pointer2); } TEST_P(GameInterfaceTest, @@ -548,13 +540,13 @@ TEST_P(GameInterfaceTest, } TEST_P(GameInterfaceTest, - getLoadedPluginReturnsTheSamePointersForConsecutiveCalls) { + getLoadedPluginsReturnsDifferentPointersForConsecutiveCalls) { handle_->LoadPlugins({std::filesystem::u8path(blankEsm)}, true); const auto pointers1 = handle_->GetLoadedPlugins(); const auto pointers2 = handle_->GetLoadedPlugins(); - EXPECT_EQ(pointers1, pointers2); + EXPECT_NE(pointers1, pointers2); } TEST_P(GameInterfaceTest, sortPluginsShouldSucceedIfPassedValidArguments) {