Merge branch 'load_save_composites' into 'master'

Write AiSequence and Script data field by field via decompose function

See merge request OpenMW/openmw!3770

apps/openmw_test_suite/esm3/testsaveload.cpp

@@ -1,4 +1,5 @@
 #include <components/esm/fourcc.hpp>
+#include <components/esm3/aisequence.hpp>
 #include <components/esm3/esmreader.hpp>
 #include <components/esm3/esmwriter.hpp>
 #include <components/esm3/loadcont.hpp>
@@ -410,6 +411,85 @@ namespace ESM
             EXPECT_EQ(result.mStringId, record.mStringId);
         }
 
+        TEST_P(Esm3SaveLoadRecordTest, aiSequenceAiWanderShouldNotChange)
+        {
+            AiSequence::AiWander record;
+            record.mData.mDistance = 1;
+            record.mData.mDuration = 2;
+            record.mData.mTimeOfDay = 3;
+            constexpr std::uint8_t idle[8] = { 4, 5, 6, 7, 8, 9, 10, 11 };
+            static_assert(std::size(idle) == std::size(record.mData.mIdle));
+            std::copy(std::begin(idle), std::end(idle), record.mData.mIdle);
+            record.mData.mShouldRepeat = 12;
+            record.mDurationData.mRemainingDuration = 13;
+            record.mStoredInitialActorPosition = true;
+            constexpr float initialActorPosition[3] = { 15, 16, 17 };
+            static_assert(std::size(initialActorPosition) == std::size(record.mInitialActorPosition.mValues));
+            std::copy(
+                std::begin(initialActorPosition), std::end(initialActorPosition), record.mInitialActorPosition.mValues);
+
+            AiSequence::AiWander result;
+            saveAndLoadRecord(record, GetParam(), result);
+
+            EXPECT_EQ(result.mData.mDistance, record.mData.mDistance);
+            EXPECT_EQ(result.mData.mDuration, record.mData.mDuration);
+            EXPECT_EQ(result.mData.mTimeOfDay, record.mData.mTimeOfDay);
+            EXPECT_THAT(result.mData.mIdle, ElementsAreArray(record.mData.mIdle));
+            EXPECT_EQ(result.mData.mShouldRepeat, record.mData.mShouldRepeat);
+            EXPECT_EQ(result.mDurationData.mRemainingDuration, record.mDurationData.mRemainingDuration);
+            EXPECT_EQ(result.mStoredInitialActorPosition, record.mStoredInitialActorPosition);
+            EXPECT_THAT(result.mInitialActorPosition.mValues, ElementsAreArray(record.mInitialActorPosition.mValues));
+        }
+
+        TEST_P(Esm3SaveLoadRecordTest, aiSequenceAiTravelShouldNotChange)
+        {
+            AiSequence::AiTravel record;
+            record.mData.mX = 1;
+            record.mData.mY = 2;
+            record.mData.mZ = 3;
+            record.mHidden = true;
+            record.mRepeat = true;
+
+            AiSequence::AiTravel result;
+            saveAndLoadRecord(record, GetParam(), result);
+
+            EXPECT_EQ(result.mData.mX, record.mData.mX);
+            EXPECT_EQ(result.mData.mY, record.mData.mY);
+            EXPECT_EQ(result.mData.mZ, record.mData.mZ);
+            EXPECT_EQ(result.mHidden, record.mHidden);
+            EXPECT_EQ(result.mRepeat, record.mRepeat);
+        }
+
+        TEST_P(Esm3SaveLoadRecordTest, aiSequenceAiEscortShouldNotChange)
+        {
+            AiSequence::AiEscort record;
+            record.mData.mX = 1;
+            record.mData.mY = 2;
+            record.mData.mZ = 3;
+            record.mData.mDuration = 4;
+            record.mTargetActorId = 5;
+            record.mTargetId = generateRandomRefId(32);
+            record.mCellId = generateRandomString(257);
+            record.mRemainingDuration = 6;
+            record.mRepeat = true;
+
+            AiSequence::AiEscort result;
+            saveAndLoadRecord(record, GetParam(), result);
+
+            EXPECT_EQ(result.mData.mX, record.mData.mX);
+            EXPECT_EQ(result.mData.mY, record.mData.mY);
+            EXPECT_EQ(result.mData.mZ, record.mData.mZ);
+            if (GetParam() <= MaxOldAiPackageFormatVersion)
+                EXPECT_EQ(result.mData.mDuration, record.mRemainingDuration);
+            else
+                EXPECT_EQ(result.mData.mDuration, record.mData.mDuration);
+            EXPECT_EQ(result.mTargetActorId, record.mTargetActorId);
+            EXPECT_EQ(result.mTargetId, record.mTargetId);
+            EXPECT_EQ(result.mCellId, record.mCellId);
+            EXPECT_EQ(result.mRemainingDuration, record.mRemainingDuration);
+            EXPECT_EQ(result.mRepeat, record.mRepeat);
+        }
+
         INSTANTIATE_TEST_SUITE_P(FormatVersions, Esm3SaveLoadRecordTest, ValuesIn(getFormats()));
     }
 }

components/esm/decompose.hpp

@@ -0,0 +1,10 @@
+#ifndef OPENMW_COMPONENTS_ESM_DECOMPOSE_H
+#define OPENMW_COMPONENTS_ESM_DECOMPOSE_H
+
+namespace ESM
+{
+    template <class T>
+    void decompose(T&& value, const auto& apply) = delete;
+}
+
+#endif

components/esm3/aisequence.cpp

@@ -3,32 +3,58 @@
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
+#include <components/misc/concepts.hpp>
+
 #include <algorithm>
 #include <memory>
 
 namespace ESM
 {
+    template <Misc::SameAsWithoutCvref<AiSequence::AiWanderData> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mDistance, v.mDuration, v.mTimeOfDay, v.mIdle, v.mShouldRepeat);
+    }
+
+    template <Misc::SameAsWithoutCvref<AiSequence::AiWanderDuration> T>
+    void decompose(T&& v, const auto& f)
+    {
+        std::uint32_t unused = 0;
+        f(v.mRemainingDuration, unused);
+    }
+
+    template <Misc::SameAsWithoutCvref<AiSequence::AiTravelData> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mX, v.mY, v.mZ);
+    }
+
+    template <Misc::SameAsWithoutCvref<AiSequence::AiEscortData> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mX, v.mY, v.mZ, v.mDuration);
+    }
+
     namespace AiSequence
     {
-
         void AiWander::load(ESMReader& esm)
         {
-            esm.getHNT("DATA", mData.mDistance, mData.mDuration, mData.mTimeOfDay, mData.mIdle, mData.mShouldRepeat);
+            esm.getNamedComposite("DATA", mData);
-            esm.getHNT("STAR", mDurationData.mRemainingDuration, mDurationData.unused); // was mStartTime
+            esm.getNamedComposite("STAR", mDurationData); // was mStartTime
             mStoredInitialActorPosition = esm.getHNOT("POS_", mInitialActorPosition.mValues);
         }
 
         void AiWander::save(ESMWriter& esm) const
         {
-            esm.writeHNT("DATA", mData);
+            esm.writeNamedComposite("DATA", mData);
-            esm.writeHNT("STAR", mDurationData);
+            esm.writeNamedComposite("STAR", mDurationData); // was mStartTime
             if (mStoredInitialActorPosition)
-                esm.writeHNT("POS_", mInitialActorPosition);
+                esm.writeHNT("POS_", mInitialActorPosition.mValues);
         }
 
         void AiTravel::load(ESMReader& esm)
         {
-            esm.getHNT("DATA", mData.mX, mData.mY, mData.mZ);
+            esm.getNamedComposite("DATA", mData);
             esm.getHNT(mHidden, "HIDD");
             mRepeat = false;
             esm.getHNOT(mRepeat, "REPT");
@@ -36,7 +62,7 @@ namespace ESM
 
         void AiTravel::save(ESMWriter& esm) const
         {
-            esm.writeHNT("DATA", mData);
+            esm.writeNamedComposite("DATA", mData);
             esm.writeHNT("HIDD", mHidden);
             if (mRepeat)
                 esm.writeHNT("REPT", mRepeat);
@@ -44,7 +70,7 @@ namespace ESM
 
         void AiEscort::load(ESMReader& esm)
         {
-            esm.getHNT("DATA", mData.mX, mData.mY, mData.mZ, mData.mDuration);
+            esm.getNamedComposite("DATA", mData);
             mTargetId = esm.getHNRefId("TARG");
             mTargetActorId = -1;
             esm.getHNOT(mTargetActorId, "TAID");
@@ -64,7 +90,7 @@ namespace ESM
 
         void AiEscort::save(ESMWriter& esm) const
         {
-            esm.writeHNT("DATA", mData);
+            esm.writeNamedComposite("DATA", mData);
             esm.writeHNRefId("TARG", mTargetId);
             esm.writeHNT("TAID", mTargetActorId);
             esm.writeHNT("DURA", mRemainingDuration);
@@ -76,7 +102,7 @@ namespace ESM
 
         void AiFollow::load(ESMReader& esm)
         {
-            esm.getHNT("DATA", mData.mX, mData.mY, mData.mZ, mData.mDuration);
+            esm.getNamedComposite("DATA", mData);
             mTargetId = esm.getHNRefId("TARG");
             mTargetActorId = -1;
             esm.getHNOT(mTargetActorId, "TAID");
@@ -101,7 +127,7 @@ namespace ESM
 
         void AiFollow::save(ESMWriter& esm) const
         {
-            esm.writeHNT("DATA", mData);
+            esm.writeNamedComposite("DATA", mData);
             esm.writeHNRefId("TARG", mTargetId);
             esm.writeHNT("TAID", mTargetActorId);
             esm.writeHNT("DURA", mRemainingDuration);

components/esm3/aisequence.hpp

@@ -36,32 +36,31 @@ namespace ESM
             virtual ~AiPackage() {}
         };
 
-#pragma pack(push, 1)
         struct AiWanderData
         {
             int16_t mDistance;
             int16_t mDuration;
-            unsigned char mTimeOfDay;
+            std::uint8_t mTimeOfDay;
-            unsigned char mIdle[8];
+            std::uint8_t mIdle[8];
-            unsigned char mShouldRepeat;
+            std::uint8_t mShouldRepeat;
         };
+
         struct AiWanderDuration
         {
             float mRemainingDuration;
-            int32_t unused;
         };
+
         struct AiTravelData
         {
             float mX, mY, mZ;
         };
+
         struct AiEscortData
         {
             float mX, mY, mZ;
             int16_t mDuration;
         };
 
-#pragma pack(pop)
-
         struct AiWander : AiPackage
         {
             AiWanderData mData;

components/esm3/esmreader.hpp

@@ -12,8 +12,10 @@
 
 #include <components/to_utf8/to_utf8.hpp>
 
+#include "components/esm/decompose.hpp"
 #include "components/esm/esmcommon.hpp"
 #include "components/esm/refid.hpp"
+
 #include "loadtes3.hpp"
 
 namespace ESM
@@ -177,6 +179,16 @@ namespace ESM
             (getT(args), ...);
         }
 
+        void getNamedComposite(NAME name, auto& value)
+        {
+            decompose(value, [&](auto&... args) { getHNT(name, args...); });
+        }
+
+        void getComposite(auto& value)
+        {
+            decompose(value, [&](auto&... args) { (getT(args), ...); });
+        }
+
         template <typename T, typename = std::enable_if_t<IsReadable<T>>>
         void skipHT()
         {

components/esm3/esmwriter.hpp

@@ -5,6 +5,7 @@
 #include <list>
 #include <type_traits>
 
+#include "components/esm/decompose.hpp"
 #include "components/esm/esmcommon.hpp"
 #include "components/esm/refid.hpp"
 
@@ -121,6 +122,20 @@ namespace ESM
             endRecord(name);
         }
 
+        void writeNamedComposite(NAME name, const auto& value)
+        {
+            decompose(value, [&](const auto&... args) {
+                startSubRecord(name);
+                (writeT(args), ...);
+                endRecord(name);
+            });
+        }
+
+        void writeComposite(const auto& value)
+        {
+            decompose(value, [&](const auto&... args) { (writeT(args), ...); });
+        }
+
         // Prevent using writeHNT with strings. This already happened by accident and results in
         // state being discarded without any error on writing or reading it. :(
         // writeHNString and friends must be used instead.
@@ -132,7 +147,7 @@ namespace ESM
         void writeHNT(NAME name, const T (&data)[size], int) = delete;
 
         template <typename T>
-        void writeHNT(NAME name, const T& data, int size)
+        void writeHNT(NAME name, const T& data, std::size_t size)
         {
             startSubRecord(name);
             writeT(data, size);

components/esm3/loadscpt.cpp

@@ -4,12 +4,19 @@
 #include <sstream>
 
 #include <components/debug/debuglog.hpp>
+#include <components/misc/concepts.hpp>
 
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
 namespace ESM
 {
+    template <Misc::SameAsWithoutCvref<Script::SCHDstruct> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mNumShorts, v.mNumLongs, v.mNumFloats, v.mScriptDataSize, v.mStringTableSize);
+    }
+
     void Script::loadSCVR(ESMReader& esm)
     {
         uint32_t s = mData.mStringTableSize;
@@ -99,11 +106,7 @@ namespace ESM
                 {
                     esm.getSubHeader();
                     mId = esm.getMaybeFixedRefIdSize(32);
-                    esm.getT(mData.mNumShorts);
+                    esm.getComposite(mData);
-                    esm.getT(mData.mNumLongs);
-                    esm.getT(mData.mNumFloats);
-                    esm.getT(mData.mScriptDataSize);
-                    esm.getT(mData.mStringTableSize);
 
                     hasHeader = true;
                     break;
@@ -157,7 +160,7 @@ namespace ESM
 
         esm.startSubRecord("SCHD");
         esm.writeMaybeFixedSizeRefId(mId, 32);
-        esm.writeT(mData, 20);
+        esm.writeComposite(mData);
         esm.endRecord("SCHD");
 
         if (isDeleted)

components/misc/concepts.hpp

@@ -0,0 +1,13 @@
+#ifndef OPENMW_COMPONENTS_MISC_CONCEPTS_H
+#define OPENMW_COMPONENTS_MISC_CONCEPTS_H
+
+#include <concepts>
+#include <type_traits>
+
+namespace Misc
+{
+    template <class T, class U>
+    concept SameAsWithoutCvref = std::same_as<std::remove_cvref_t<T>, std::remove_cvref_t<U>>;
+}
+
+#endif