From 194c11f214dad1147fb993cf0a0f89abf309ff7c Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Tue, 12 Apr 2022 17:27:59 +0200
Subject: [PATCH] Fix loading order in EsmLoader

Need to load the last present record from a sequence of loaded records. That
means reverse should be called before unique or unique should be applied for
a reversed range. Since unique keeps only the first element from a sub
sequence of equal elements.

Use forEachUnique with reversed range to avoid redundant container
modifications.
---
 apps/openmw_test_suite/CMakeLists.txt       |   1 +
 apps/openmw_test_suite/esmloader/record.cpp | 105 ++++++++++++++++++++
 components/esmloader/record.hpp             |   7 +-
 3 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 apps/openmw_test_suite/esmloader/record.cpp

diff --git a/apps/openmw_test_suite/CMakeLists.txt b/apps/openmw_test_suite/CMakeLists.txt
index 3b12e7c227..74418a004b 100644
--- a/apps/openmw_test_suite/CMakeLists.txt
+++ b/apps/openmw_test_suite/CMakeLists.txt
@@ -67,6 +67,7 @@ if (GTEST_FOUND AND GMOCK_FOUND)
 
         esmloader/load.cpp
         esmloader/esmdata.cpp
+        esmloader/record.cpp
 
         files/hash.cpp
 
diff --git a/apps/openmw_test_suite/esmloader/record.cpp b/apps/openmw_test_suite/esmloader/record.cpp
new file mode 100644
index 0000000000..fbbbdf18d8
--- /dev/null
+++ b/apps/openmw_test_suite/esmloader/record.cpp
@@ -0,0 +1,105 @@
+#include <components/esmloader/record.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <array>
+#include <tuple>
+
+namespace
+{
+    using namespace testing;
+    using namespace EsmLoader;
+
+    struct Value
+    {
+        int mKey;
+        int mValue;
+    };
+
+    auto tie(const Value& v)
+    {
+        return std::tie(v.mKey, v.mValue);
+    }
+
+    bool operator==(const Value& l, const Value& r)
+    {
+        return tie(l) == tie(r);
+    }
+
+    std::ostream& operator<<(std::ostream& s, const Value& v)
+    {
+        return s << "Value {" << v.mKey << ", " << v.mValue << "}";
+    }
+
+    Record<Value> present(const Value& v)
+    {
+        return Record<Value>(false, v);
+    }
+
+    Record<Value> deleted(const Value& v)
+    {
+        return Record<Value>(true, v);
+    }
+
+    struct Params
+    {
+        Records<Value> mRecords;
+        std::vector<Value> mResult;
+    };
+
+    struct EsmLoaderPrepareRecordTest : TestWithParam<Params> {};
+
+    TEST_P(EsmLoaderPrepareRecordTest, prepareRecords)
+    {
+        auto records = GetParam().mRecords;
+        const auto getKey = [&] (const Record<Value>& v) { return v.mValue.mKey; };
+        EXPECT_THAT(prepareRecords(records, getKey), ElementsAreArray(GetParam().mResult));
+    }
+
+    const std::array params = {
+        Params {{}, {}},
+        Params {
+            {present(Value {1, 1})},
+            {Value {1, 1}}
+        },
+        Params {
+            {deleted(Value {1, 1})},
+            {}
+        },
+        Params {
+            {present(Value {1, 1}), present(Value {2, 2})},
+            {Value {1, 1}, Value {2, 2}}
+        },
+        Params {
+            {present(Value {2, 2}), present(Value {1, 1})},
+            {Value {1, 1}, Value {2, 2}}
+        },
+        Params {
+            {present(Value {1, 1}), present(Value {1, 2})},
+            {Value {1, 2}}
+        },
+        Params {
+            {present(Value {1, 2}), present(Value {1, 1})},
+            {Value {1, 1}}
+        },
+        Params {
+            {present(Value {1, 1}), deleted(Value {1, 2})},
+            {}
+        },
+        Params {
+            {deleted(Value {1, 1}), present(Value {1, 2})},
+            {Value {1, 2}}
+        },
+        Params {
+            {present(Value {1, 2}), deleted(Value {1, 1})},
+            {}
+        },
+        Params {
+            {deleted(Value {1, 2}), present(Value {1, 1})},
+            {Value {1, 1}}
+        },
+    };
+
+    INSTANTIATE_TEST_SUITE_P(Params, EsmLoaderPrepareRecordTest, ValuesIn(params));
+}
diff --git a/components/esmloader/record.hpp b/components/esmloader/record.hpp
index b88991cca1..ec9705e823 100644
--- a/components/esmloader/record.hpp
+++ b/components/esmloader/record.hpp
@@ -2,6 +2,7 @@
 #define OPENMW_COMPONENTS_ESMLOADER_RECORD_H
 
 #include <components/esm3/loadcell.hpp>
+#include <components/misc/algorithm.hpp>
 
 #include <algorithm>
 #include <utility>
@@ -31,12 +32,12 @@ namespace EsmLoader
         const auto greaterByKey = [&] (const auto& l, const auto& r) { return getKey(r) < getKey(l); };
         const auto equalByKey = [&] (const auto& l, const auto& r) { return getKey(l) == getKey(r); };
         std::stable_sort(records.begin(), records.end(), greaterByKey);
-        records.erase(std::unique(records.begin(), records.end(), equalByKey), records.end());
-        std::reverse(records.begin(), records.end());
         std::vector<T> result;
-        for (Record<T>& v : records)
+        Misc::forEachUnique(records.rbegin(), records.rend(), equalByKey, [&] (const auto& v)
+        {
             if (!v.mDeleted)
                 result.emplace_back(std::move(v.mValue));
+        });
         return result;
     }
 }