fix: Keep search paths in the specified order (#1069)

* fix: Keep search paths in the specified order

* chore: remove namespaces from tests

* fix: alphabetical order in linker default paths for backwards compatibility

* chore: extract AddPathsStringIfDistinct function

* chore: make sure linker default source search path stays backwards compatible

---------

Co-authored-by: Jan Laupetin <[email protected]>

src/Linking/LinkerArgs.h

@@ -1,7 +1,6 @@
 #pragma once
 #include "Utils/Arguments/ArgumentParser.h"
 
-#include <set>
 #include <vector>
 
 class LinkerArgs
@@ -10,9 +9,9 @@ public:
     static constexpr auto DEFAULT_BASE_FOLDER = ".";
     static constexpr auto DEFAULT_CACHE_FOLDER = "?base?/.oat/cache/?project?";
     static constexpr auto DEFAULT_OUTPUT_FOLDER = "?base?/zone_out/?project?";
-    static constexpr auto DEFAULT_ASSET_SEARCH_PATH = "?bin?/raw/?game?;?base?/raw;?base?/raw/?game?;?base?/zone_raw/?project?";
+    static constexpr auto DEFAULT_ASSET_SEARCH_PATH = "?base?/raw;?base?/raw/?game?;?base?/zone_raw/?project?;?bin?/raw/?game?";
     static constexpr auto DEFAULT_GDT_SEARCH_PATH = "?base?/source_data;?base?/zone_raw/?project?/source_data";
-    static constexpr auto DEFAULT_SOURCE_SEARCH_PATH = "?base?/zone_source;?base?/zone_raw/?project?;?base?/zone_raw/?project?/zone_source";
+    static constexpr auto DEFAULT_SOURCE_SEARCH_PATH = "?base?/zone_raw/?project?;?base?/zone_raw/?project?/zone_source;?base?/zone_source";
 
     LinkerArgs();
     bool ParseArgs(int argc, const char** argv, bool& shouldContinue);
@@ -24,9 +23,9 @@ public:
     std::string m_base_folder;
     std::string m_out_folder;
 
-    std::set<std::string> m_asset_search_paths;
-    std::set<std::string> m_gdt_search_paths;
-    std::set<std::string> m_source_search_paths;
+    std::vector<std::string> m_asset_search_paths;
+    std::vector<std::string> m_gdt_search_paths;
+    std::vector<std::string> m_source_search_paths;
 
 private:
     /**

src/Linking/LinkerPaths.cpp

@@ -152,7 +152,7 @@ namespace
         {
         }
 
-        void BuildFromArgs(const std::set<std::string>& templates)
+        void BuildFromArgs(const std::vector<std::string>& templates)
         {
             m_templates.reserve(templates.size());
             for (const auto& templateString : templates)

src/Unlinking/UnlinkerArgs.h

@@ -5,7 +5,6 @@
 
 #include <cstdint>
 #include <regex>
-#include <set>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -46,7 +45,7 @@ public:
 
     std::vector<std::string> m_zones_to_load;
     std::vector<std::string> m_zones_to_unlink;
-    std::set<std::string> m_user_search_paths;
+    std::vector<std::string> m_user_search_paths;
 
     ProcessingTask m_task;
     std::string m_output_folder;

src/Utils/Utils/FileUtils.cpp

@@ -1,13 +1,23 @@
 #include "FileUtils.h"
 
+#include <algorithm>
 #include <cstring>
 #include <sstream>
 
 namespace fs = std::filesystem;
 
+namespace
+{
+    void AddPathsStringIfDistinct(std::vector<std::string>& output, std::string path)
+    {
+        if (std::ranges::find(output, path) == output.end())
+            output.emplace_back(std::move(path));
+    };
+} // namespace
+
 namespace utils
 {
-    bool ParsePathsString(const std::string& pathsString, std::set<std::string>& output)
+    bool ParsePathsString(const std::string& pathsString, std::vector<std::string>& output)
     {
         std::ostringstream currentPath;
         auto pathStart = true;
@@ -39,7 +49,7 @@ namespace utils
                     auto path = currentPath.str();
                     if (!path.empty())
                     {
-                        output.insert(path);
+                        AddPathsStringIfDistinct(output, std::move(path));
                         currentPath = std::ostringstream();
                     }
 
@@ -61,7 +71,7 @@ namespace utils
 
         if (currentPath.tellp() > 0)
         {
-            output.insert(currentPath.str());
+            AddPathsStringIfDistinct(output, currentPath.str());
         }
 
         return true;

src/Utils/Utils/FileUtils.h

@@ -3,9 +3,9 @@
 #include <cstdint>
 #include <filesystem>
 #include <fstream>
-#include <set>
 #include <sstream>
 #include <string>
+#include <vector>
 
 namespace utils
 {
@@ -17,10 +17,10 @@ namespace utils
     /**
      * \brief Splits a path string as user input into a list of paths.
      * \param pathsString The path string that was taken as user input.
-     * \param output A set for strings to save the output to.
+     * \param output A list to append the paths to in the order they are specified, skipping paths it already contains.
      * \return \c true if the user input was valid and could be processed successfully, otherwise \c false.
      */
-    bool ParsePathsString(const std::string& pathsString, std::set<std::string>& output);
+    bool ParsePathsString(const std::string& pathsString, std::vector<std::string>& output);
 
     enum class TextFileCheckDirtyResult : std::uint8_t
     {

test/CommonTests/Utils/FileUtilsTests.cpp

@@ -0,0 +1,21 @@
+#include "Utils/FileUtils.h"
+
+#include <catch2/catch_test_macros.hpp>
+#include <string>
+#include <vector>
+
+TEST_CASE("FileUtils: Parses paths string in specified order", "[fileutils]")
+{
+    std::vector<std::string> paths;
+
+    REQUIRE(utils::ParsePathsString("zone_raw/b;zone_raw/a;\"raw;c\"", paths));
+    REQUIRE(paths == std::vector<std::string>{"zone_raw/b", "zone_raw/a", "raw;c"});
+}
+
+TEST_CASE("FileUtils: Parses paths string keeping first of duplicate paths", "[fileutils]")
+{
+    std::vector<std::string> paths{"raw"};
+
+    REQUIRE(utils::ParsePathsString("zone_raw;raw;zone_raw;lang", paths));
+    REQUIRE(paths == std::vector<std::string>{"raw", "zone_raw", "lang"});
+}