Stop storing plugin pointers in C++ wrapper

With the documented lifetime semantics updated to just use those of std::shared_ptr<>, there's no need to cache them in the wrapper.

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<const PluginInterface> 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<std::shared_ptr<const PluginInterface>> GetLoadedPlugins()
       const = 0;

cpp/src/api/game.cpp

@@ -167,45 +167,20 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
   } catch (const ::rust::Error& e) {
     std::rethrow_exception(mapError(e));
   }
-
-  std::lock_guard<std::mutex> 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<std::mutex> guard(pluginsMutex_);
-  plugins_.clear();
-}
+void Game::ClearLoadedPlugins() { game_->clear_loaded_plugins(); }
 
 std::shared_ptr<const PluginInterface> Game::GetPlugin(
     std::string_view pluginName) const {
-  std::lock_guard<std::mutex> 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<const PluginInterface> plugin =
-        std::make_shared<Plugin>(std::move(pluginOpt->as_ref().boxed_clone()));
-
-    return plugins_.emplace(key, plugin).first->second;
+    return std::make_shared<Plugin>(
+        std::move(pluginOpt->as_ref().boxed_clone()));
   } catch (const ::rust::Error& e) {
     std::rethrow_exception(mapError(e));
   }
@@ -213,18 +188,10 @@ std::shared_ptr<const PluginInterface> Game::GetPlugin(
 
 std::vector<std::shared_ptr<const PluginInterface>> Game::GetLoadedPlugins()
     const {
-  std::lock_guard<std::mutex> guard(pluginsMutex_);
-
   std::vector<std::shared_ptr<const PluginInterface>> 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<const PluginInterface> plugin =
-          std::make_shared<Plugin>(std::move(pluginRef.boxed_clone()));
-      it = plugins_.emplace(key, plugin).first;
-    }
-    plugins.push_back(it->second);
+    plugins.push_back(
+        std::make_shared<Plugin>(std::move(pluginRef.boxed_clone())));
   }
 
   return plugins;

cpp/src/api/game.h

@@ -59,9 +59,6 @@ public:
 private:
   ::rust::Box<loot::rust::Game> game_;
   Database database_;
-
-  mutable std::map<Filename, std::shared_ptr<const PluginInterface>> plugins_;
-  mutable std::mutex pluginsMutex_;
 };
 }
 

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) {