Swap some std::string in API with std::filesystem::path

Specifically the functions that were changed in 0.19.4 to accept paths
without changing their type signatures to avoid breaking compatibility.

include/loot/game_interface.h

@@ -77,7 +77,7 @@ public:
    *         as given.
    * @returns True if the file is a valid plugin, false otherwise.
    */
-  virtual bool IsValidPlugin(const std::string& pluginPath) const = 0;
+  virtual bool IsValidPlugin(const std::filesystem::path& pluginPath) const = 0;
 
   /**
    * @brief Parses plugins and loads their data.
@@ -93,8 +93,9 @@ public:
    *        file if it has been identified by a previous call to
    *        ``IdentifyMainMasterFile()``.
    */
-  virtual void LoadPlugins(const std::vector<std::string>& pluginPaths,
-                           bool loadHeadersOnly) = 0;
+  virtual void LoadPlugins(
+      const std::vector<std::filesystem::path>& pluginPaths,
+      bool loadHeadersOnly) = 0;
 
   /**
    * @brief Get data for a loaded plugin.
@@ -144,7 +145,7 @@ public:
    *           order.
    */
   virtual std::vector<std::string> SortPlugins(
-      const std::vector<std::string>& pluginPaths) = 0;
+      const std::vector<std::filesystem::path>& pluginPaths) = 0;
 
   /**
    *  @}

src/api/game/game.cpp

@@ -142,16 +142,6 @@ std::vector<std::filesystem::path> FindArchives(
 
   return archivePaths;
 }
-
-std::vector<std::filesystem::path> StringsToPaths(
-    const std::vector<std::string>& pluginPathStrings) {
-  std::vector<std::filesystem::path> pluginPaths;
-  for (const auto& pluginPathString : pluginPathStrings) {
-    pluginPaths.push_back(u8path(pluginPathString));
-  }
-
-  return pluginPaths;
-}
 }
 
 namespace loot {
@@ -201,19 +191,10 @@ void Game::SetAdditionalDataPaths(
   loadOrderHandler_.SetAdditionalDataPaths(additionalDataPaths_);
 }
 
-bool Game::IsValidPlugin(const std::string& pluginPath) const {
-  return IsValidPlugin(u8path(pluginPath));
-}
-
 bool Game::IsValidPlugin(const std::filesystem::path& pluginPath) const {
   return Plugin::IsValid(GetType(), ResolvePluginPath(DataPath(), pluginPath));
 }
 
-void Game::LoadPlugins(const std::vector<std::string>& pluginPathStrings,
-                       bool loadHeadersOnly) {
-  return LoadPlugins(StringsToPaths(pluginPathStrings), loadHeadersOnly);
-}
-
 void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
                        bool loadHeadersOnly) {
   const auto logger = getLogger();
@@ -310,11 +291,6 @@ void Game::IdentifyMainMasterFile(const std::string& masterFile) {
   masterFilename_ = masterFile;
 }
 
-std::vector<std::string> Game::SortPlugins(
-    const std::vector<std::string>& pluginPathStrings) {
-  return SortPlugins(StringsToPaths(pluginPathStrings));
-}
-
 std::vector<std::string> Game::SortPlugins(
     const std::vector<std::filesystem::path>& pluginPaths) {
   LoadPlugins(pluginPaths, false);

src/api/game/game.h

@@ -55,14 +55,6 @@ public:
   void SetAdditionalDataPaths(
       const std::vector<std::filesystem::path>& additionalDataPaths);
 
-  bool IsValidPlugin(const std::filesystem::path& pluginPath) const;
-
-  void LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
-                   bool loadHeadersOnly);
-
-  std::vector<std::string> SortPlugins(
-      const std::vector<std::filesystem::path>& pluginPaths);
-
   // Game Interface Methods //
   ////////////////////////////
 
@@ -71,9 +63,9 @@ public:
   DatabaseInterface& GetDatabase() override;
   const DatabaseInterface& GetDatabase() const override;
 
-  bool IsValidPlugin(const std::string& pluginPath) const override;
+  bool IsValidPlugin(const std::filesystem::path& pluginPath) const override;
 
-  void LoadPlugins(const std::vector<std::string>& pluginPaths,
+  void LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
                    bool loadHeadersOnly) override;
 
   const PluginInterface* GetPlugin(
@@ -84,7 +76,7 @@ public:
   void IdentifyMainMasterFile(const std::string& masterFile) override;
 
   std::vector<std::string> SortPlugins(
-      const std::vector<std::string>& pluginPaths) override;
+      const std::vector<std::filesystem::path>& pluginPaths) override;
 
   void LoadCurrentLoadOrderState() override;
 

src/tests/api/interface/game_interface_test.h

@@ -36,6 +36,7 @@ protected:
       emptyFile("EmptyFile.esm"),
       nonAsciiEsm(u8"non\u00C1scii.esm"),
       pluginsToLoad({
+          // These are all ASCII filenames.
           masterFile,
           blankEsm,
           blankDifferentEsm,
@@ -55,7 +56,7 @@ protected:
 
   const std::string emptyFile;
   const std::string nonAsciiEsm;
-  const std::vector<std::string> pluginsToLoad;
+  const std::vector<std::filesystem::path> pluginsToLoad;
 };
 
 // Pass an empty first argument, as it's a prefix for the test instantation,
@@ -75,7 +76,7 @@ TEST_P(GameInterfaceTest, isValidPluginShouldReturnTrueForAValidPlugin) {
 
 TEST_P(GameInterfaceTest,
        isValidPluginShouldReturnTrueForAValidNonAsciiPlugin) {
-  EXPECT_TRUE(handle_->IsValidPlugin(nonAsciiEsm));
+  EXPECT_TRUE(handle_->IsValidPlugin(std::filesystem::u8path(nonAsciiEsm)));
 }
 
 TEST_P(GameInterfaceTest, isValidPluginShouldReturnFalseForANonPluginFile) {
@@ -131,7 +132,7 @@ TEST_P(GameInterfaceTest,
 }
 
 TEST_P(GameInterfaceTest, loadPluginsWithANonAsciiPluginShouldLoadIt) {
-  handle_->LoadPlugins({nonAsciiEsm}, false);
+  handle_->LoadPlugins({std::filesystem::u8path(nonAsciiEsm)}, false);
   EXPECT_EQ(1, handle_->GetLoadedPlugins().size());
 
   // Check that one plugin's header has been read.
@@ -173,7 +174,8 @@ TEST_P(GameInterfaceTest, sortPluginsShouldSucceedIfPassedValidArguments) {
   ASSERT_NO_THROW(GenerateMasterlist());
   ASSERT_NO_THROW(handle_->GetDatabase().LoadLists(masterlistPath, ""));
 
-  std::vector<std::string> pluginsToSort({
+  std::vector<std::filesystem::path> pluginsToSort({
+      // These are all ASCII filenames.
       blankEsp,
       blankPluginDependentEsp,
       blankDifferentMasterDependentEsm,

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

@@ -38,7 +38,8 @@ protected:
   }
 
   void loadInstalledPlugins(Game& game, bool headersOnly) {
-    const std::vector<std::string> plugins({
+    const std::vector<std::filesystem::path> plugins({
+        // These are all ASCII filenames.
         masterFile,
         blankEsm,
         blankDifferentEsm,
@@ -149,7 +150,9 @@ TEST_P(
 TEST_P(GameTest, isValidPluginShouldResolveRelativePathsRelativeToDataPath) {
   const Game game(GetParam(), dataPath.parent_path(), localPath);
 
-  game.IsValidPlugin("../" + dataPath.filename().u8string() + "/" + blankEsm);
+  const auto path = ".." / dataPath.filename() / blankEsm;
+
+  EXPECT_TRUE(game.IsValidPlugin(path));
 }
 
 TEST_P(GameTest, isValidPluginShouldUseAbsolutePathsAsGiven) {
@@ -158,7 +161,8 @@ TEST_P(GameTest, isValidPluginShouldUseAbsolutePathsAsGiven) {
   ASSERT_TRUE(dataPath.is_absolute());
 
   const auto path = dataPath / std::filesystem::u8path(blankEsm);
-  game.IsValidPlugin(path.u8string());
+
+  EXPECT_TRUE(game.IsValidPlugin(path));
 }
 
 TEST_P(
@@ -181,9 +185,9 @@ TEST_P(
 TEST_P(GameTest, loadPluginsWithANonPluginShouldNotAddItToTheLoadedPlugins) {
   Game game = Game(GetParam(), dataPath.parent_path(), localPath);
 
-  ASSERT_THROW(
-      game.LoadPlugins(std::vector<std::string>({nonPluginFile}), false),
-      std::invalid_argument);
+  ASSERT_THROW(game.LoadPlugins(
+                   std::vector<std::filesystem::path>({nonPluginFile}), false),
+               std::invalid_argument);
 
   ASSERT_TRUE(game.GetLoadedPlugins().empty());
 }
@@ -200,8 +204,8 @@ TEST_P(GameTest,
 
   Game game = Game(GetParam(), dataPath.parent_path(), localPath);
 
-  ASSERT_NO_THROW(
-      game.LoadPlugins(std::vector<std::string>({invalidPlugin}), false));
+  ASSERT_NO_THROW(game.LoadPlugins(
+      std::vector<std::filesystem::path>({invalidPlugin}), false));
 
   ASSERT_TRUE(game.GetLoadedPlugins().empty());
 }
@@ -300,20 +304,18 @@ TEST_P(GameTest,
   const auto sourcePluginPath =
       getSourcePluginsPath() / std::filesystem::u8path(blankEsm);
 
-  EXPECT_THROW(
-      game.LoadPlugins(std::vector<std::string>({dataPluginPath.u8string(),
-                                                 sourcePluginPath.u8string()}),
-                       true),
-      std::invalid_argument);
+  EXPECT_THROW(game.LoadPlugins(std::vector<std::filesystem::path>(
+                                    {dataPluginPath, sourcePluginPath}),
+                                true),
+               std::invalid_argument);
 }
 
 TEST_P(GameTest, loadPluginsShouldResolveRelativePathsRelativeToDataPath) {
   Game game = Game(GetParam(), dataPath.parent_path(), localPath);
 
-  const auto relativePath =
-      "../" + dataPath.filename().u8string() + "/" + blankEsm;
+  const auto relativePath = ".." / dataPath.filename() / blankEsm;
 
-  game.LoadPlugins(std::vector<std::string>({relativePath}), true);
+  game.LoadPlugins(std::vector<std::filesystem::path>({relativePath}), true);
 
   EXPECT_NE(nullptr, game.GetPlugin(blankEsm));
 }
@@ -323,7 +325,7 @@ TEST_P(GameTest, loadPluginsShouldUseAbsolutePathsAsGiven) {
 
   const auto absolutePath = dataPath / std::filesystem::u8path(blankEsm);
 
-  game.LoadPlugins(std::vector<std::string>({absolutePath.u8string()}), true);
+  game.LoadPlugins(std::vector<std::filesystem::path>({absolutePath}), true);
 
   EXPECT_NE(nullptr, game.GetPlugin(blankEsm));
 }
@@ -334,7 +336,7 @@ TEST_P(GameTest, sortPluginsShouldHandlePluginPathsThatAreNotJustFilenames) {
   const auto absolutePath = dataPath / std::filesystem::u8path(blankEsm);
 
   const auto newLoadOrder =
-      game.SortPlugins(std::vector<std::string>({absolutePath.u8string()}));
+      game.SortPlugins(std::vector<std::filesystem::path>({absolutePath}));
 
   EXPECT_EQ(std::vector<std::string>{blankEsm}, newLoadOrder);
 }

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

@@ -64,7 +64,8 @@ protected:
   }
 
   void loadInstalledPlugins() {
-    std::vector<std::string> plugins({
+    std::vector<std::filesystem::path> plugins({
+        // These are mostly ASCII filenames.
         masterFile,
         blankEsm,
         blankDifferentEsm,
@@ -76,7 +77,7 @@ protected:
         blankDifferentMasterDependentEsp,
         blankPluginDependentEsp,
         blankDifferentPluginDependentEsp,
-        nonAsciiEsm,
+        std::filesystem::u8path(nonAsciiEsm),
     });
 
     if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) {

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

@@ -41,7 +41,8 @@ protected:
       cccPath_(dataPath.parent_path() / getCCCFilename()) {}
 
   void loadInstalledPlugins(Game& game, bool headersOnly) {
-    std::vector<std::string> plugins({
+    std::vector<std::filesystem::path> plugins({
+        // These are all ASCII filenames.
         masterFile,
         blankEsm,
         blankDifferentEsm,

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

@@ -37,7 +37,8 @@ protected:
       blankEslEsp("Blank.esl.esp") {}
 
   void loadInstalledPlugins(Game &game, bool headersOnly) {
-    std::vector<std::string> plugins({
+    std::vector<std::filesystem::path> plugins({
+        // These are all ASCII filenames.
         masterFile,
         blankEsm,
         blankDifferentEsm,