Make PluginMetadata::GetLoadAfterFiles() return a std::vector

The collection is only ever iterated over, and the uniqueness and
ordering don't matter.

CMakeLists.txt

@@ -272,6 +272,7 @@ set (LIBLOOT_HEADERS "${CMAKE_SOURCE_DIR}/include/loot/api.h"
                       "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_graph.h"
                       "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorting_data.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/git_helper.h"
+                      "${CMAKE_SOURCE_DIR}/src/api/helpers/collections.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/crc.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/logging.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/text.h")

include/loot/metadata/plugin_metadata.h

@@ -100,7 +100,7 @@ public:
    * Get the plugins that the plugin must load after.
    * @return The plugins that the plugin must load after.
    */
-  LOOT_API std::set<File> GetLoadAfterFiles() const;
+  LOOT_API std::vector<File> GetLoadAfterFiles() const;
 
   /**
    * Get the files that the plugin requires to be installed.
@@ -170,7 +170,7 @@ public:
    * @param after
    *        The files to set.
    */
-  LOOT_API void SetLoadAfterFiles(const std::set<File>& after);
+  LOOT_API void SetLoadAfterFiles(const std::vector<File>& after);
 
   /**
    * Set the files that the plugin requires to be installed.
@@ -268,7 +268,7 @@ public:
 private:
   std::string name_;
   std::optional<std::string> group_;
-  std::set<File> loadAfter_;
+  std::vector<File> loadAfter_;
   std::set<File> requirements_;
   std::set<File> incompatibilities_;
   std::vector<Message> messages_;

src/api/helpers/collections.h

@@ -0,0 +1,68 @@
+/*  LOOT
+
+    A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and
+    Fallout: New Vegas.
+
+    Copyright (C) 2012-2016    WrinklyNinja
+
+    This file is part of LOOT.
+
+    LOOT is free software: you can redistribute
+    it and/or modify it under the terms of the GNU General Public License
+    as published by the Free Software Foundation, either version 3 of
+    the License, or (at your option) any later version.
+
+    LOOT is distributed in the hope that it will
+    be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with LOOT.  If not, see
+    <https://www.gnu.org/licenses/>.
+    */
+
+#ifndef LOOT_API_HELPERS_COLLECTIONS
+#define LOOT_API_HELPERS_COLLECTIONS
+
+#include <algorithm>
+#include <vector>
+
+namespace loot {
+// Append second to first, skipping any elements that are already present in
+// first. Although this is O(U * M), both input vectors are expected to be
+// small (with tens of elements being an unusually large number).
+template<typename T>
+std::vector<T> mergeVectors(std::vector<T> first,
+                            const std::vector<T>& second) {
+  auto initialSizeOfFirst = first.size();
+  for (const auto& element : second) {
+    auto end = first.cbegin() + initialSizeOfFirst;
+
+    if (std::find(first.cbegin(), end, element) == end) {
+      first.push_back(element);
+    }
+  }
+
+  return first;
+}
+
+// Returns the elements in first that are not in second. Although this is
+// O(F * S), both input vectors are expected to be small (with tens of elements
+// being an unusually large number).
+template<typename T>
+std::vector<T> diffVectors(const std::vector<T>& first,
+                           const std::vector<T>& second) {
+  std::vector<T> result;
+
+  for (const auto& element : first) {
+    if (std::find(second.begin(), second.end(), element) == second.end()) {
+      result.push_back(element);
+    }
+  }
+
+  return result;
+}
+}
+
+#endif

src/api/metadata/condition_evaluator.cpp

@@ -127,13 +127,14 @@ PluginMetadata ConditionEvaluator::EvaluateAll(const PluginMetadata& pluginMetad
     evaluatedMetadata.SetGroup(pluginMetadata.GetGroup().value());
   }
 
-  std::set<File> fileSet;
+  std::vector<File> files;
   for (const auto& file : pluginMetadata.GetLoadAfterFiles()) {
     if (Evaluate(file.GetCondition()))
-      fileSet.insert(file);
+      files.push_back(file);
   }
-  evaluatedMetadata.SetLoadAfterFiles(fileSet);
+  evaluatedMetadata.SetLoadAfterFiles(files);
 
+  std::set<File> fileSet;
   fileSet.clear();
   for (const auto& file : pluginMetadata.GetRequirements()) {
     if (Evaluate(file.GetCondition()))

src/api/metadata/plugin_metadata.cpp

@@ -30,6 +30,7 @@
 #include <boost/algorithm/string.hpp>
 
 #include "api/game/game.h"
+#include "api/helpers/collections.h"
 #include "api/helpers/logging.h"
 #include "api/helpers/text.h"
 
@@ -42,8 +43,7 @@ using std::vector;
 namespace loot {
 PluginMetadata::PluginMetadata() {}
 
-PluginMetadata::PluginMetadata(const std::string& n) :
-    name_(n) {
+PluginMetadata::PluginMetadata(const std::string& n) : name_(n) {
   // If the name passed ends in '.ghost', that should be trimmed.
   if (boost::iends_with(name_, ".ghost"))
     name_ = name_.substr(0, name_.length() - 6);
@@ -57,12 +57,7 @@ void PluginMetadata::MergeMetadata(const PluginMetadata& plugin) {
     group_ = plugin.GetGroup();
   }
 
-  // Merge the following. If any files in the source already exist in the
-  // destination, they will be skipped. Files have display strings and
-  // condition strings which aren't considered when comparing them, so
-  // will be lost if the plugin being merged in has additional data in
-  // these strings.
-  loadAfter_.insert(begin(plugin.loadAfter_), end(plugin.loadAfter_));
+  loadAfter_ = mergeVectors(loadAfter_, plugin.loadAfter_);
   requirements_.insert(begin(plugin.requirements_), end(plugin.requirements_));
   incompatibilities_.insert(begin(plugin.incompatibilities_),
                             end(plugin.incompatibilities_));
@@ -92,14 +87,9 @@ PluginMetadata PluginMetadata::NewMetadata(const PluginMetadata& plugin) const {
   }
 
   // Compare this plugin against the given plugin.
-  set<File> filesDiff;
-  set_difference(begin(loadAfter_),
-                 end(loadAfter_),
-                 begin(plugin.loadAfter_),
-                 end(plugin.loadAfter_),
-                 inserter(filesDiff, begin(filesDiff)));
-  p.SetLoadAfterFiles(filesDiff);
+  p.SetLoadAfterFiles(diffVectors(loadAfter_, plugin.loadAfter_));
 
+  set<File> filesDiff;
   filesDiff.clear();
   set_difference(begin(requirements_),
                  end(requirements_),
@@ -167,7 +157,9 @@ std::string PluginMetadata::GetName() const { return name_; }
 
 std::optional<std::string> PluginMetadata::GetGroup() const { return group_; }
 
-std::set<File> PluginMetadata::GetLoadAfterFiles() const { return loadAfter_; }
+std::vector<File> PluginMetadata::GetLoadAfterFiles() const {
+  return loadAfter_;
+}
 
 std::set<File> PluginMetadata::GetRequirements() const { return requirements_; }
 
@@ -202,15 +194,11 @@ std::vector<SimpleMessage> PluginMetadata::GetSimpleMessages(
   return simpleMessages;
 }
 
-void PluginMetadata::SetGroup(const std::string& group) {
-  group_ = group;
-}
+void PluginMetadata::SetGroup(const std::string& group) { group_ = group; }
 
-void PluginMetadata::UnsetGroup() {
-  group_ = std::nullopt;
-}
+void PluginMetadata::UnsetGroup() { group_ = std::nullopt; }
 
-void PluginMetadata::SetLoadAfterFiles(const std::set<File>& l) {
+void PluginMetadata::SetLoadAfterFiles(const std::vector<File>& l) {
   loadAfter_ = l;
 }
 
@@ -242,8 +230,7 @@ void PluginMetadata::SetLocations(const std::set<Location>& locations) {
 }
 
 bool PluginMetadata::HasNameOnly() const {
-  return !group_.has_value() &&
-         loadAfter_.empty() && requirements_.empty() &&
+  return !group_.has_value() && loadAfter_.empty() && requirements_.empty() &&
          incompatibilities_.empty() && messages_.empty() && tags_.empty() &&
          dirtyInfo_.empty() && cleanInfo_.empty() && locations_.empty();
 }

src/api/metadata/yaml/plugin_metadata.h

@@ -103,7 +103,7 @@ struct convert<loot::PluginMetadata> {
       rhs.SetGroup(node["group"].as<std::string>());
 
     if (node["after"])
-      rhs.SetLoadAfterFiles(node["after"].as<std::set<loot::File>>());
+      rhs.SetLoadAfterFiles(node["after"].as<std::vector<loot::File>>());
     if (node["req"])
       rhs.SetRequirements(node["req"].as<std::set<loot::File>>());
     if (node["inc"])

src/api/sorting/plugin_sorting_data.cpp

@@ -85,7 +85,7 @@ PluginSortingData::PluginSortingData(
       } else {
         // Not all masters are loaded, fall back to using the plugin's
         // total record count (Morrowind doesn't have groups). This is OK
-        // because plugins with missing masters can't be loaded by the game, 
+        // because plugins with missing masters can't be loaded by the game,
         // so the correctness of their load order positions is less important
         // (it may not matter at all, depending on the sophistication/usage of
         // merge patches in Morrowind). It's better for LOOT to sort a load
@@ -134,11 +134,11 @@ void PluginSortingData::SetAfterGroupPlugins(
   afterGroupPlugins_ = plugins;
 }
 
-const std::set<File>& PluginSortingData::GetMasterlistLoadAfterFiles() const {
+const std::vector<File>& PluginSortingData::GetMasterlistLoadAfterFiles() const {
   return masterlistLoadAfter_;
 }
 
-const std::set<File>& PluginSortingData::GetUserLoadAfterFiles() const {
+const std::vector<File>& PluginSortingData::GetUserLoadAfterFiles() const {
   return userLoadAfter_;
 }
 

src/api/sorting/plugin_sorting_data.h

@@ -50,8 +50,8 @@ public:
   std::unordered_set<std::string> GetAfterGroupPlugins() const;
   void SetAfterGroupPlugins(std::unordered_set<std::string> plugins);
 
-  const std::set<File>& GetMasterlistLoadAfterFiles() const;
-  const std::set<File>& GetUserLoadAfterFiles() const;
+  const std::vector<File>& GetMasterlistLoadAfterFiles() const;
+  const std::vector<File>& GetUserLoadAfterFiles() const;
   const std::set<File>& GetMasterlistRequirements() const;
   const std::set<File>& GetUserRequirements() const;
 
@@ -62,8 +62,8 @@ private:
   std::string group_;
   std::unordered_set<std::string> afterGroupPlugins_;
 
-  std::set<File> masterlistLoadAfter_;
-  std::set<File> userLoadAfter_;
+  std::vector<File> masterlistLoadAfter_;
+  std::vector<File> userLoadAfter_;
   std::set<File> masterlistReq_;
   std::set<File> userReq_;
 

src/tests/api/interface/database_interface_test.h

@@ -559,9 +559,9 @@ TEST_P(
 
   auto metadata = db_->GetPluginMetadata(blankEsm, true).value();
 
-  std::set<File> expectedLoadAfter({
-      File(masterFile),
+  std::vector<File> expectedLoadAfter({
       File(blankDifferentEsm),
+      File(masterFile),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());
 }
@@ -596,7 +596,7 @@ TEST_P(
 
   auto metadata = db_->GetPluginMetadata(blankEsm, false).value();
 
-  std::set<File> expectedLoadAfter({
+  std::vector<File> expectedLoadAfter({
       File(masterFile),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());
@@ -631,7 +631,7 @@ TEST_P(DatabaseInterfaceTest,
 
   auto metadata = db_->GetPluginUserMetadata(blankEsm).value();
 
-  std::set<File> expectedLoadAfter({
+  std::vector<File> expectedLoadAfter({
       File(blankDifferentEsm),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());
@@ -683,7 +683,7 @@ TEST_P(DatabaseInterfaceTest,
 
   auto metadata = db_->GetPluginMetadata(blankEsm).value();
 
-  std::set<File> expectedLoadAfter({
+  std::vector<File> expectedLoadAfter({
       File(masterFile),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());
@@ -711,7 +711,7 @@ TEST_P(
 
   auto metadata = db_->GetPluginMetadata(blankEsm).value();
 
-  std::set<File> expectedLoadAfter({
+  std::vector<File> expectedLoadAfter({
       File(masterFile),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());
@@ -781,7 +781,7 @@ TEST_P(
 
   auto metadata = db_->GetPluginMetadata(blankEsm).value();
 
-  std::set<File> expectedLoadAfter({
+  std::vector<File> expectedLoadAfter({
       File(masterFile),
   });
   EXPECT_EQ(expectedLoadAfter, metadata.GetLoadAfterFiles());

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

@@ -189,11 +189,12 @@ TEST_P(ConditionEvaluatorTest, evaluateAllShouldEvaluateAllMetadataConditions) {
 
   EXPECT_NO_THROW(plugin = evaluator_.EvaluateAll(plugin));
 
-  std::set<File> expectedFiles({file1});
+  std::vector<File> expectedFiles({file1});
+  std::set<File> expectedFileSet({file1});
   EXPECT_EQ("group1", plugin.GetGroup().value());
   EXPECT_EQ(expectedFiles, plugin.GetLoadAfterFiles());
-  EXPECT_EQ(expectedFiles, plugin.GetRequirements());
-  EXPECT_EQ(expectedFiles, plugin.GetIncompatibilities());
+  EXPECT_EQ(expectedFileSet, plugin.GetRequirements());
+  EXPECT_EQ(expectedFileSet, plugin.GetIncompatibilities());
   EXPECT_EQ(std::vector<Message>({message1}), plugin.GetMessages());
   EXPECT_EQ(std::set<Tag>({tag1}), plugin.GetTags());
   EXPECT_EQ(std::set<PluginCleaningData>({info1}), plugin.GetDirtyInfo());

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

@@ -169,7 +169,7 @@ TEST_P(PluginMetadataTest, mergeMetadataShouldMergeLoadAfterData) {
   plugin2.SetLoadAfterFiles({file1, file2});
   plugin1.MergeMetadata(plugin2);
 
-  EXPECT_EQ(std::set<File>({file1, file2}), plugin1.GetLoadAfterFiles());
+  EXPECT_EQ(std::vector<File>({file1, file2}), plugin1.GetLoadAfterFiles());
 }
 
 TEST_P(PluginMetadataTest, mergeMetadataShouldMergeRequirementData) {
@@ -324,7 +324,7 @@ TEST_P(PluginMetadataTest,
   plugin2.SetLoadAfterFiles({file1, file3});
   PluginMetadata newMetadata = plugin1.NewMetadata(plugin2);
 
-  EXPECT_EQ(std::set<File>({file2}), newMetadata.GetLoadAfterFiles());
+  EXPECT_EQ(std::vector<File>({file2}), newMetadata.GetLoadAfterFiles());
 }
 
 TEST_P(
@@ -785,7 +785,7 @@ TEST_P(PluginMetadataTest,
   YAML::Node node;
   node = plugin;
 
-  EXPECT_EQ(plugin.GetLoadAfterFiles(), node["after"].as<std::set<File>>());
+  EXPECT_EQ(plugin.GetLoadAfterFiles(), node["after"].as<std::vector<File>>());
 }
 
 TEST_P(PluginMetadataTest, encodingAsYamlShouldSetReqFieldIfRequirementsExist) {
@@ -881,7 +881,7 @@ TEST_P(PluginMetadataTest, decodingFromYamlShouldStoreAllGivenData) {
   PluginMetadata plugin = node.as<PluginMetadata>();
 
   EXPECT_EQ("Blank.esp", plugin.GetName());
-  EXPECT_EQ(std::set<File>({File("Blank.esm")}), plugin.GetLoadAfterFiles());
+  EXPECT_EQ(std::vector<File>({File("Blank.esm")}), plugin.GetLoadAfterFiles());
   EXPECT_EQ(std::set<File>({File("Blank.esm")}), plugin.GetRequirements());
   EXPECT_EQ(std::set<File>({File("Blank.esm")}), plugin.GetIncompatibilities());
   EXPECT_EQ(std::vector<Message>({Message(MessageType::say, "content")}),

src/tests/api/internals/metadata_list_test.h

@@ -293,7 +293,7 @@ TEST_P(
   PluginMetadata plugin = metadataList.FindPlugin(blankDifferentEsp).value();
 
   EXPECT_EQ(blankDifferentEsp, plugin.GetName());
-  EXPECT_EQ(std::set<File>({
+  EXPECT_EQ(std::vector<File>({
                 File(blankEsm),
             }),
             plugin.GetLoadAfterFiles());

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

@@ -240,7 +240,7 @@ TEST_P(
   PluginMetadata plugin(blankEsp);
 
   plugin = PluginMetadata(blankDifferentMasterDependentEsp);
-  plugin.SetLoadAfterFiles(std::set<File>({File(blankMasterDependentEsp)}));
+  plugin.SetLoadAfterFiles({File(blankMasterDependentEsp)});
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
   plugin = PluginMetadata(blankDifferentEsp);