Make plugin loading and sorting more granular

- Don't clear the cache in LoadPlugins()
- Don't load plugins in SortPlugins(), and make it take a vector of strings, not paths.
- Add a ClearLoadedPlugins() method to clear the loaded plugins cache.
- Remove IdentifyMainMasterFile()

Instead of calling IdentifyMainMasterFile(), callers can use LoadPlugins() to initially load all plugin headers only, then omit the main master file when calling LoadPlugins() to fully load plugins.

docs/api/changelog.rst

@@ -26,6 +26,8 @@ Added
     paths to ensure that they are resolved correctly.
   - Ghosted plugins are not supported for OpenMW.
 
+- :cpp:any:`loot::GameInterface::ClearLoadedPlugins()`
+
 Fixed
 -----
 
@@ -38,9 +40,13 @@ Fixed
 Changed
 -------
 
-- :cpp:any:`loot::GameInterface::IdentifyMainMasterFile()` now takes a
-  ``const std::filesystem::path&`` instead of a
-  ``const std::string&``.
+- :cpp:any:`loot::GameInterface::LoadPlugins()` no longer clears the data of
+  previously-loaded plugins, though if any of the given paths have filenames
+  that match previously-loaded plugins, the previously-loaded data will be
+  still be replaced.
+- :cpp:any:`loot::GameInterface::SortPlugins()` now takes a vector of filenames
+  instead of a vector of strings, and no longer loads the given plugins. It
+  instead expects the plugins to have already been loaded.
 - The application of plugin groups as part of the sorting process has been
   overhauled. As well as fixing several known bugs, the new approach avoids
   causing cyclic interaction errors, handles groups more consistently and is
@@ -64,6 +70,10 @@ Changed
 Removed
 -------
 
+- ``loot::GameInterface::IdentifyMainMasterFile()``: callers should instead
+  call :cpp:any:`loot::GameInterface::LoadPlugins()` with the main master file
+  to load only its headers, and omit the main master file when calling
+  :cpp:any:`loot::GameInterface::LoadPlugins()` to fully load plugins.
 - Prebuilt Linux release binaries are no longer provided, as the binaries that
   were previously provided were not very portable beyond the Linux distribution
   versions that they were built on.

include/loot/game_interface.h

@@ -103,27 +103,42 @@ public:
 
   /**
    * @brief Parses plugins and loads their data.
-   * @details Any previously-loaded plugin data is discarded when this function
-   *          is called.
+   * @details If a given plugin filename (or one that is case-insensitively
+   *          equal) has already been loaded, its previously-loaded data
+   *          data is discarded, invalidating any existing shared pointers to
+   *          that plugin's PluginInterface object.
+   *
+   *          If the game is Morrowind, OpenMW or Starfield, it's only valid to
+   *          fully load a plugin if its masters are already loaded or included
+   *          in the same input vector.
    * @param pluginPaths
    *        The plugin paths to load. Relative paths are resolved relative to
    *        the game's plugins directory, while absolute paths are used as
    *        given. Each plugin filename must be unique within the vector.
    * @param loadHeadersOnly
    *        If true, only the plugins' headers are loaded. If false, all records
-   *        in the plugins are parsed, apart from the main master file if it has
-   *        been identified by a previous call to ``IdentifyMainMasterFile()``.
+   *        in the plugins are parsed.
    */
   virtual void LoadPlugins(
       const std::vector<std::filesystem::path>& pluginPaths,
       bool loadHeadersOnly) = 0;
 
+  /**
+   * @brief Clears the plugins loaded by previous calls to `LoadPlugins()`.
+   * @details This invalidates any PluginInterface pointers retrieved using
+   *          `GetPlugin()` or `GetLoadedPlugins()`.
+   */
+  virtual void ClearLoadedPlugins() = 0;
+
   /**
    * @brief Get data for a loaded plugin.
    * @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. The
+   *          pointer remains valid until the `ClearLoadedPlugins()` function
+   *          is called, this GameInterface is destroyed, or until a plugin with
+   *          a case-insensitively equal filename is loaded.
    */
   virtual const PluginInterface* GetPlugin(
       const std::string& pluginName) const = 0;
@@ -131,9 +146,10 @@ public:
   /**
    * @brief Get a set of const references to all loaded plugins' PluginInterface
    *        objects.
-   * @returns A set of const PluginInterface references. The references remain
-   *          valid until the ``LoadPlugins()`` or ``SortPlugins()`` functions
-   *          are next called or this GameInterface is destroyed.
+   * @returns A set of shared pointers to const PluginInterface. The pointers
+   *          remain valid until the `ClearLoadedPlugins()` function is called,
+   *          this GameInterface is destroyed, or until a plugin with a
+   *          case-insensitively equal filename is loaded.
    */
   virtual std::vector<const PluginInterface*> GetLoadedPlugins() const = 0;
 
@@ -143,16 +159,6 @@ public:
    *  @{
    */
 
-  /**
-   *  @brief Identify the game's main master file.
-   *  @details When sorting, LOOT always only loads the headers of the game's
-   *           main master file as a performance optimisation.
-   *
-   *           A relative path is resolved relative to the game's plugins
-   *           directory, while an absolute path is used as given.
-   */
-  virtual void IdentifyMainMasterFile(const std::filesystem::path& masterFile) = 0;
-
   /**
    *  @brief Calculates a new load order for the game's installed plugins
    *         (including inactive plugins) and outputs the sorted order.
@@ -161,15 +167,13 @@ public:
    *           applied to the load order used by the game. This function does
    *           not load or evaluate the masterlist or userlist.
    *  @param pluginPaths
-   *         The plugin paths to sort, in their current load order. Relative
-   *         paths are resolved relative to the game's plugins directory, while
-   *         absolute paths are used as given. Each plugin filename must be
-   *         unique within the vector.
+   *         The plugins to sort, in their current load order. All given plugins
+   *         must have been loaded using `LoadPlugins()`.
    *  @returns A vector of the given plugin filenames in their sorted load
    *           order.
    */
   virtual std::vector<std::string> SortPlugins(
-      const std::vector<std::filesystem::path>& pluginPaths) = 0;
+      const std::vector<std::string>& pluginFilenames) = 0;
 
   /**
    *  @}

include/loot/metadata/plugin_metadata.h

@@ -211,7 +211,7 @@ public:
 
   /**
    * Check if the plugin name is a regular expression.
-   * @return True if the plugin name contains any of the characters ``:\*?|``,
+   * @return True if the plugin name contains any of the characters `:\*?|`,
    *         false otherwise.
    */
   LOOT_API bool IsRegexPlugin() const;

src/api/game/game.cpp

@@ -255,9 +255,6 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
                                 "\" is not a valid plugin");
   }
 
-  // Clear the existing plugin and archive caches.
-  cache_.ClearCachedPlugins();
-
   // Search for and cache archives.
   CacheArchives();
 
@@ -275,12 +272,8 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
           const auto resolvedPluginPath =
               ResolvePluginPath(GetType(), DataPath(), pluginPath);
 
-          const bool loadHeader =
-              loadHeadersOnly ||
-              loot::equivalent(resolvedPluginPath, masterFilePath_);
-
           cache_.AddPlugin(
-              Plugin(GetType(), cache_, resolvedPluginPath, loadHeader));
+              Plugin(GetType(), cache_, resolvedPluginPath, loadHeadersOnly));
         } catch (const std::exception& e) {
           if (logger) {
             logger->error(
@@ -304,6 +297,10 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
   conditionEvaluator_->RefreshLoadedPluginsState(GetLoadedPlugins());
 }
 
+void Game::ClearLoadedPlugins() {
+  cache_.ClearCachedPlugins();
+}
+
 const PluginInterface* Game::GetPlugin(const std::string& pluginName) const {
   return cache_.GetPlugin(pluginName);
 }
@@ -317,21 +314,9 @@ std::vector<const PluginInterface*> Game::GetLoadedPlugins() const {
   return interfacePointers;
 }
 
-void Game::IdentifyMainMasterFile(const std::filesystem::path& masterFile) {
-  masterFilePath_ = ResolvePluginPath(GetType(), DataPath(), masterFile);
-}
-
 std::vector<std::string> Game::SortPlugins(
-    const std::vector<std::filesystem::path>& pluginPaths) {
-  LoadPlugins(pluginPaths, false);
-
-  std::vector<std::string> loadOrder;
-  for (const auto& pluginPath : pluginPaths) {
-    loadOrder.push_back(pluginPath.filename().u8string());
-  }
-
-  // Sort plugins into their load order.
-  return loot::SortPlugins(*this, loadOrder);
+    const std::vector<std::string>& pluginFilenames) {
+  return loot::SortPlugins(*this, pluginFilenames);
 }
 
 void Game::LoadCurrentLoadOrderState() {

src/api/game/game.h

@@ -70,15 +70,15 @@ public:
   void LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
                    bool loadHeadersOnly) override;
 
+  void ClearLoadedPlugins() override;
+
   const PluginInterface* GetPlugin(
       const std::string& pluginName) const override;
 
   std::vector<const PluginInterface*> GetLoadedPlugins() const override;
 
-  void IdentifyMainMasterFile(const std::filesystem::path& masterFile) override;
-
   std::vector<std::string> SortPlugins(
-      const std::vector<std::filesystem::path>& pluginPaths) override;
+      const std::vector<std::string>& pluginFilenames) override;
 
   void LoadCurrentLoadOrderState() override;
 
@@ -103,8 +103,6 @@ private:
   std::shared_ptr<ConditionEvaluator> conditionEvaluator_;
   ApiDatabase database_;
 
-  std::filesystem::path masterFilePath_;
-
   std::vector<std::filesystem::path> additionalDataPaths_;
 };
 }

src/api/game/game_cache.cpp

@@ -93,12 +93,14 @@ void GameCache::AddPlugin(Plugin&& plugin) {
   lock_guard<mutex> lock(mutex_);
 
   auto normalizedName = NormalizeFilename(plugin.GetName());
+  auto pluginPointer = std::make_shared<Plugin>(std::move(plugin));
 
   const auto it = plugins_.find(normalizedName);
-  if (it != end(plugins_))
-    plugins_.erase(it);
-
-  plugins_.emplace(normalizedName, std::make_shared<Plugin>(std::move(plugin)));
+  if (it != end(plugins_)) {
+    it->second = pluginPointer;
+  } else {
+    plugins_.emplace(normalizedName, pluginPointer);
+  }
 }
 
 std::set<std::filesystem::path> GameCache::GetArchivePaths() const {

src/api/sorting/plugin_sort.cpp

@@ -34,33 +34,23 @@
 namespace loot {
 std::vector<PluginSortingData> GetPluginsSortingData(
     const DatabaseInterface& db,
-    const std::vector<const PluginInterface*> loadedPluginInterfaces,
-    const std::vector<std::string>& loadOrder) {
+    const std::vector<const Plugin*>& loadOrder) {
   std::vector<PluginSortingData> pluginsSortingData;
-  pluginsSortingData.reserve(loadedPluginInterfaces.size());
+  pluginsSortingData.reserve(loadOrder.size());
 
   std::vector<ComparableFilename> comparableLoadOrder;
-  for (const auto& pluginName : loadOrder) {
-    comparableLoadOrder.push_back(ToComparableFilename(pluginName));
+  for (const auto& plugin : loadOrder) {
+    comparableLoadOrder.push_back(ToComparableFilename(plugin->GetName()));
   }
 
-  for (const auto& pluginInterface : loadedPluginInterfaces) {
-    if (!pluginInterface) {
-      continue;
-    }
-
-    const auto plugin = dynamic_cast<const Plugin* const>(pluginInterface);
-
-    if (!plugin) {
-      throw std::logic_error(
-          "Tried to case a PluginInterface pointer to a Plugin pointer.");
-    }
+  for (const auto& plugin : loadOrder) {
+    const auto pluginFilename = plugin->GetName();
 
     const auto masterlistMetadata =
-        db.GetPluginMetadata(plugin->GetName(), false, true)
-            .value_or(PluginMetadata(plugin->GetName()));
-    const auto userMetadata = db.GetPluginUserMetadata(plugin->GetName(), true)
-                                  .value_or(PluginMetadata(plugin->GetName()));
+        db.GetPluginMetadata(pluginFilename, false, true)
+            .value_or(PluginMetadata(pluginFilename));
+    const auto userMetadata = db.GetPluginUserMetadata(pluginFilename, true)
+                                  .value_or(PluginMetadata(pluginFilename));
 
     const auto pluginSortingData = PluginSortingData(
         plugin, masterlistMetadata, userMetadata, comparableLoadOrder);
@@ -313,8 +303,7 @@ std::vector<std::string> SortPlugins(
     return {};
   }
 
-  // Sort the plugins according to into their existing load order, or
-  // lexicographical ordering for pairs of plugins without load order positions.
+  // Sort the plugins according to the lexicographical order of their names.
   // This ensures a consistent iteration order for vertices given the same input
   // data. The vertex iteration order can affect what edges get added and so
   // the final sorting result, so consistency is important.
@@ -387,8 +376,18 @@ std::vector<std::string> SortPlugins(
 std::vector<std::string> SortPlugins(
     Game& game,
     const std::vector<std::string>& loadOrder) {
-  auto pluginsSortingData = GetPluginsSortingData(
-      game.GetDatabase(), game.GetLoadedPlugins(), loadOrder);
+  std::vector<const Plugin*> plugins;
+  for (const auto& pluginFilename : loadOrder) {
+    const auto plugin = game.GetCache().GetPlugin(pluginFilename);
+    if (plugin == nullptr) {
+      throw std::invalid_argument("The plugin \"" + pluginFilename +
+                                  "\" has not been loaded.");
+    }
+
+    plugins.push_back(plugin);
+  }
+
+  auto pluginsSortingData = GetPluginsSortingData(game.GetDatabase(), plugins);
 
   const auto logger = getLogger();
   if (logger) {

src/tests/api/interface/game_interface_test.h

@@ -118,7 +118,7 @@ TEST_P(GameInterfaceTest, isValidPluginShouldReturnFalseForAnEmptyFile) {
 
 TEST_P(
     GameInterfaceTest,
-    loadPluginsWithHeadersOnlyTrueShouldLoadTheHeadersOfAllInstalledPlugins) {
+    loadPluginsWithHeadersOnlyTrueShouldLoadTheHeadersOfAllGivenPlugins) {
   handle_->LoadPlugins(pluginsToLoad, true);
   if (GetParam() == GameType::starfield) {
     EXPECT_EQ(6, handle_->GetLoadedPlugins().size());
@@ -184,7 +184,17 @@ TEST_P(GameInterfaceTest, loadPluginsWithANonAsciiPluginShouldLoadIt) {
   EXPECT_EQ(blankEsmCrc, plugin->GetCRC().value());
 }
 
-TEST_P(GameInterfaceTest, getPluginThatIsNotCachedShouldReturnAnEmptyOptional) {
+TEST_P(GameInterfaceTest, clearLoadedPluginsShouldClearThePluginsCache) {
+  handle_->LoadPlugins({std::filesystem::u8path(blankEsm)}, true);
+  const auto pointer = handle_->GetPlugin(blankEsm);
+  ASSERT_NE(nullptr, pointer);
+
+  handle_->ClearLoadedPlugins();
+
+  EXPECT_EQ(nullptr, handle_->GetPlugin(blankEsm));
+}
+
+TEST_P(GameInterfaceTest, getPluginThatIsNotCachedShouldReturnANullPointer) {
   EXPECT_FALSE(handle_->GetPlugin(blankEsm));
 }
 
@@ -232,7 +242,14 @@ TEST_P(GameInterfaceTest, sortPluginsShouldSucceedIfPassedValidArguments) {
   }
 
   handle_->LoadCurrentLoadOrderState();
-  std::vector<std::string> actualOrder = handle_->SortPlugins(pluginsToLoad);
+  handle_->LoadPlugins(pluginsToLoad, false);
+
+  std::vector<std::string> pluginsToSort;
+  for (const auto& plugin : pluginsToLoad) {
+    pluginsToSort.push_back(plugin.filename().u8string());
+  }
+
+  std::vector<std::string> actualOrder = handle_->SortPlugins(pluginsToSort);
 
   EXPECT_EQ(expectedOrder, actualOrder);
 }

src/tests/api/internals/game/game_test.h

@@ -218,7 +218,7 @@ TEST_P(
 
 TEST_P(
     GameTest,
-    loadPluginsWithHeadersOnlyTrueShouldLoadTheHeadersOfAllInstalledPlugins) {
+    loadPluginsWithHeadersOnlyTrueShouldLoadTheHeadersOfGivenPlugins) {
   Game game = Game(GetParam(), gamePath, localPath);
 
   EXPECT_NO_THROW(loadInstalledPlugins(game, true));
@@ -266,7 +266,7 @@ TEST_P(GameTest,
 }
 
 TEST_P(GameTest,
-       loadPluginsWithHeadersOnlyFalseShouldFullyLoadAllInstalledPlugins) {
+       loadPluginsWithHeadersOnlyFalseShouldFullyLoadAllGivenPlugins) {
   Game game = Game(GetParam(), gamePath, localPath);
 
   EXPECT_NO_THROW(loadInstalledPlugins(game, false));
@@ -285,6 +285,32 @@ TEST_P(GameTest,
   EXPECT_EQ(blankEsmCrc, plugin->GetCRC().value());
 }
 
+TEST_P(GameTest, loadPluginsShouldNotClearThePluginsCache) {
+  Game game = Game(GetParam(), gamePath, localPath);
+
+  game.LoadPlugins({std::filesystem::u8path(blankEsm)}, true);
+  const auto pointer = game.GetPlugin(blankEsm);
+  ASSERT_NE(nullptr, pointer);
+
+  game.LoadPlugins({std::filesystem::u8path(blankEsp)}, true);
+
+  EXPECT_EQ(pointer, game.GetPlugin(blankEsm));
+}
+
+TEST_P(GameTest, loadPluginsShouldReplaceCacheEntriesForTheGivenPlugins) {
+  Game game = Game(GetParam(), gamePath, localPath);
+
+  game.LoadPlugins({std::filesystem::u8path(blankEsm)}, true);
+  const auto pointer = game.GetPlugin(blankEsm);
+  ASSERT_NE(nullptr, pointer);
+
+  game.LoadPlugins({std::filesystem::u8path(blankEsm)}, false);
+
+  const auto newPointer = game.GetPlugin(blankEsm);
+  ASSERT_NE(nullptr, newPointer);
+  EXPECT_NE(pointer, newPointer);
+}
+
 TEST_P(
     GameTest,
     loadPluginsShouldFindAndCacheArchivesForLoadDetectionWhenLoadingPlugins) {
@@ -408,15 +434,16 @@ TEST_P(
   }
 }
 
-TEST_P(GameTest, sortPluginsShouldHandlePluginPathsThatAreNotJustFilenames) {
+TEST_P(GameTest, clearLoadedPluginsShouldClearThePluginsCache) {
   Game game = Game(GetParam(), gamePath, localPath);
 
-  const auto absolutePath = dataPath / std::filesystem::u8path(blankEsm);
+  game.LoadPlugins({std::filesystem::u8path(blankEsm)}, true);
+  const auto pointer = game.GetPlugin(blankEsm);
+  ASSERT_NE(nullptr, pointer);
 
-  const auto newLoadOrder =
-      game.SortPlugins(std::vector<std::filesystem::path>({absolutePath}));
+  game.ClearLoadedPlugins();
 
-  EXPECT_EQ(std::vector<std::string>{blankEsm}, newLoadOrder);
+  EXPECT_EQ(nullptr, game.GetPlugin(blankEsm));
 }
 
 TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasNotBeenLoaded) {

src/tests/api/internals/metadata/condition_evaluator_test.h

@@ -68,7 +68,6 @@ protected:
       plugins.push_back(blankEsl);
     }
 
-    game_.IdentifyMainMasterFile(std::filesystem::u8path(masterFile));
     game_.LoadCurrentLoadOrderState();
     game_.LoadPlugins(plugins, true);
   }

src/tests/api/internals/sorting/plugin_sort_test.h

@@ -51,8 +51,14 @@ protected:
       }
     }
 
-    game.IdentifyMainMasterFile(std::filesystem::u8path(masterFile));
     game.LoadCurrentLoadOrderState();
+
+    if (!headersOnly) {
+      const auto gameMasterPlugin = plugins.front();
+      game.LoadPlugins({gameMasterPlugin}, true);
+      plugins.erase(plugins.begin());
+    }
+     
     game.LoadPlugins(plugins, headersOnly);
   }
 
@@ -1141,6 +1147,23 @@ TEST_P(
             }),
             sorted);
 }
+
+TEST_P(PluginSortTest, sortingShouldOnlySortTheGivenPlugins) {
+  loadInstalledPlugins(game_, false);
+
+  std::vector<std::string> plugins{blankEsp, blankDifferentEsp};
+  std::vector<std::string> sorted = SortPlugins(game_, plugins);
+
+  EXPECT_EQ(plugins, sorted);
+}
+
+TEST_P(PluginSortTest, sortingShouldThrowIfAGivenPluginIsNotLoaded) {
+  game_.ClearLoadedPlugins();
+
+  std::vector<std::string> plugins{blankEsp, blankDifferentEsp};
+  
+  EXPECT_THROW(SortPlugins(game_, plugins), std::invalid_argument);
+}
 }
 }
 

src/tests/api/internals/sorting/plugin_sorting_data_test.h

@@ -46,7 +46,6 @@ protected:
       }
     }
 
-    game.IdentifyMainMasterFile(std::filesystem::u8path(masterFile));
     game.LoadCurrentLoadOrderState();
     game.LoadPlugins(plugins, headersOnly);
   }