Merge branch 'decomposition' into 'master'

Use decompose to remove more instances of pragma pack

See merge request OpenMW/openmw!3906

12 files changed, 131 insertions, 44 deletions

diff --git a/apps/openmw/mwclass/creature.cpp b/apps/openmw/mwclass/creature.cpp
index 2de58c6127..b6c607b415 100644
--- a/apps/openmw/mwclass/creature.cpp
+++ b/apps/openmw/mwclass/creature.cpp
@@ -316,11 +316,11 @@ namespace MWClass
         {
             const unsigned char* attack = nullptr;
             if (type == ESM::Weapon::AT_Chop)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop.data();
             else if (type == ESM::Weapon::AT_Slash)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mSlash;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mSlash.data();
             else if (type == ESM::Weapon::AT_Thrust)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mThrust;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mThrust.data();
             if (attack)
             {
                 damage = attack[0] + ((attack[1] - attack[0]) * attackStrength);
diff --git a/apps/openmw/mwclass/npc.cpp b/apps/openmw/mwclass/npc.cpp
index b7540ebe04..98384254d3 100644
--- a/apps/openmw/mwclass/npc.cpp
+++ b/apps/openmw/mwclass/npc.cpp
@@ -635,11 +635,11 @@ namespace MWClass
         {
             const unsigned char* attack = nullptr;
             if (type == ESM::Weapon::AT_Chop)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop.data();
             else if (type == ESM::Weapon::AT_Slash)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mSlash;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mSlash.data();
             else if (type == ESM::Weapon::AT_Thrust)
-                attack = weapon.get<ESM::Weapon>()->mBase->mData.mThrust;
+                attack = weapon.get<ESM::Weapon>()->mBase->mData.mThrust.data();
             if (attack)
             {
                 damage = attack[0] + ((attack[1] - attack[0]) * attackStrength);
diff --git a/apps/openmw/mwmechanics/combat.cpp b/apps/openmw/mwmechanics/combat.cpp
index 831c3ff7ab..b9852e1b41 100644
--- a/apps/openmw/mwmechanics/combat.cpp
+++ b/apps/openmw/mwmechanics/combat.cpp
@@ -246,14 +246,16 @@ namespace MWMechanics
                 return;
             }
 
-            const unsigned char* attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop;
-            damage = attack[0] + ((attack[1] - attack[0]) * attackStrength); // Bow/crossbow damage
-
-            // Arrow/bolt damage
-            // NB in case of thrown weapons, we are applying the damage twice since projectile == weapon
-            attack = projectile.get<ESM::Weapon>()->mBase->mData.mChop;
-            damage += attack[0] + ((attack[1] - attack[0]) * attackStrength);
-
+            {
+                const auto& attack = weapon.get<ESM::Weapon>()->mBase->mData.mChop;
+                damage = attack[0] + ((attack[1] - attack[0]) * attackStrength); // Bow/crossbow damage
+            }
+            {
+                // Arrow/bolt damage
+                // NB in case of thrown weapons, we are applying the damage twice since projectile == weapon
+                const auto& attack = projectile.get<ESM::Weapon>()->mBase->mData.mChop;
+                damage += attack[0] + ((attack[1] - attack[0]) * attackStrength);
+            }
             adjustWeaponDamage(damage, weapon, attacker);
         }
 
diff --git a/apps/openmw_test_suite/esm3/testsaveload.cpp b/apps/openmw_test_suite/esm3/testsaveload.cpp
index eda1fa963e..6d5fdf1c14 100644
--- a/apps/openmw_test_suite/esm3/testsaveload.cpp
+++ b/apps/openmw_test_suite/esm3/testsaveload.cpp
@@ -1,12 +1,14 @@
 #include <components/esm/fourcc.hpp>
 #include <components/esm3/aipackage.hpp>
 #include <components/esm3/aisequence.hpp>
+#include <components/esm3/effectlist.hpp>
 #include <components/esm3/esmreader.hpp>
 #include <components/esm3/esmwriter.hpp>
 #include <components/esm3/loadcont.hpp>
 #include <components/esm3/loaddial.hpp>
 #include <components/esm3/loadregn.hpp>
 #include <components/esm3/loadscpt.hpp>
+#include <components/esm3/loadweap.hpp>
 #include <components/esm3/player.hpp>
 #include <components/esm3/quickkeys.hpp>
 
@@ -525,6 +527,81 @@ namespace ESM
             EXPECT_EQ(result.mServices, record.mServices);
         }
 
+        TEST_P(Esm3SaveLoadRecordTest, enamShouldNotChange)
+        {
+            EffectList record;
+            record.mList.emplace_back(ENAMstruct{
+                .mEffectID = 1,
+                .mSkill = 2,
+                .mAttribute = 3,
+                .mRange = 4,
+                .mArea = 5,
+                .mDuration = 6,
+                .mMagnMin = 7,
+                .mMagnMax = 8,
+            });
+
+            EffectList result;
+            saveAndLoadRecord(record, GetParam(), result);
+
+            EXPECT_EQ(result.mList.size(), record.mList.size());
+            EXPECT_EQ(result.mList[0].mEffectID, record.mList[0].mEffectID);
+            EXPECT_EQ(result.mList[0].mSkill, record.mList[0].mSkill);
+            EXPECT_EQ(result.mList[0].mAttribute, record.mList[0].mAttribute);
+            EXPECT_EQ(result.mList[0].mRange, record.mList[0].mRange);
+            EXPECT_EQ(result.mList[0].mArea, record.mList[0].mArea);
+            EXPECT_EQ(result.mList[0].mDuration, record.mList[0].mDuration);
+            EXPECT_EQ(result.mList[0].mMagnMin, record.mList[0].mMagnMin);
+            EXPECT_EQ(result.mList[0].mMagnMax, record.mList[0].mMagnMax);
+        }
+
+        TEST_P(Esm3SaveLoadRecordTest, weaponShouldNotChange)
+        {
+            Weapon record = {
+                .mData = {
+                    .mWeight = 0,
+                    .mValue = 1,
+                    .mType = 2,
+                    .mHealth = 3,
+                    .mSpeed = 4,
+                    .mReach = 5,
+                    .mEnchant = 6,
+                    .mChop = { 7, 8 },
+                    .mSlash = { 9, 10 },
+                    .mThrust = { 11, 12 },
+                    .mFlags = 13,
+                },
+                .mRecordFlags = 0,
+                .mId = generateRandomRefId(32),
+                .mEnchant = generateRandomRefId(32),
+                .mScript = generateRandomRefId(32),
+                .mName = generateRandomString(32),
+                .mModel = generateRandomString(32),
+                .mIcon = generateRandomString(32),
+            };
+
+            Weapon result;
+            saveAndLoadRecord(record, GetParam(), result);
+
+            EXPECT_EQ(result.mData.mWeight, record.mData.mWeight);
+            EXPECT_EQ(result.mData.mValue, record.mData.mValue);
+            EXPECT_EQ(result.mData.mType, record.mData.mType);
+            EXPECT_EQ(result.mData.mHealth, record.mData.mHealth);
+            EXPECT_EQ(result.mData.mSpeed, record.mData.mSpeed);
+            EXPECT_EQ(result.mData.mReach, record.mData.mReach);
+            EXPECT_EQ(result.mData.mEnchant, record.mData.mEnchant);
+            EXPECT_EQ(result.mData.mChop, record.mData.mChop);
+            EXPECT_EQ(result.mData.mSlash, record.mData.mSlash);
+            EXPECT_EQ(result.mData.mThrust, record.mData.mThrust);
+            EXPECT_EQ(result.mData.mFlags, record.mData.mFlags);
+            EXPECT_EQ(result.mId, record.mId);
+            EXPECT_EQ(result.mEnchant, record.mEnchant);
+            EXPECT_EQ(result.mScript, record.mScript);
+            EXPECT_EQ(result.mName, record.mName);
+            EXPECT_EQ(result.mModel, record.mModel);
+            EXPECT_EQ(result.mIcon, record.mIcon);
+        }
+
         INSTANTIATE_TEST_SUITE_P(FormatVersions, Esm3SaveLoadRecordTest, ValuesIn(getFormats()));
     }
 }
diff --git a/components/esm3/aipackage.cpp b/components/esm3/aipackage.cpp
index 2cadb9fb22..33b8a0bca2 100644
--- a/components/esm3/aipackage.cpp
+++ b/components/esm3/aipackage.cpp
@@ -54,29 +54,25 @@ namespace ESM
         else if (esm.retSubName() == AI_Wander)
         {
             pack.mType = AI_Wander;
-            esm.getSubHeader();
-            esm.getComposite(pack.mWander);
+            esm.getSubComposite(pack.mWander);
             mList.push_back(pack);
         }
         else if (esm.retSubName() == AI_Travel)
         {
             pack.mType = AI_Travel;
-            esm.getSubHeader();
-            esm.getComposite(pack.mTravel);
+            esm.getSubComposite(pack.mTravel);
             mList.push_back(pack);
         }
         else if (esm.retSubName() == AI_Escort || esm.retSubName() == AI_Follow)
         {
             pack.mType = (esm.retSubName() == AI_Escort) ? AI_Escort : AI_Follow;
-            esm.getSubHeader();
-            esm.getComposite(pack.mTarget);
+            esm.getSubComposite(pack.mTarget);
             mList.push_back(pack);
         }
         else if (esm.retSubName() == AI_Activate)
         {
             pack.mType = AI_Activate;
-            esm.getSubHeader();
-            esm.getComposite(pack.mActivate);
+            esm.getSubComposite(pack.mActivate);
             mList.push_back(pack);
         }
     }
diff --git a/components/esm3/effectlist.cpp b/components/esm3/effectlist.cpp
index 701552b312..4f21f47fa2 100644
--- a/components/esm3/effectlist.cpp
+++ b/components/esm3/effectlist.cpp
@@ -3,8 +3,15 @@
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
+#include <components/misc/concepts.hpp>
+
 namespace ESM
 {
+    template <Misc::SameAsWithoutCvref<ENAMstruct> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mEffectID, v.mSkill, v.mAttribute, v.mRange, v.mArea, v.mDuration, v.mMagnMin, v.mMagnMax);
+    }
 
     void EffectList::load(ESMReader& esm)
     {
@@ -18,15 +25,15 @@ namespace ESM
     void EffectList::add(ESMReader& esm)
     {
         ENAMstruct s;
-        esm.getHT(s.mEffectID, s.mSkill, s.mAttribute, s.mRange, s.mArea, s.mDuration, s.mMagnMin, s.mMagnMax);
+        esm.getSubComposite(s);
         mList.push_back(s);
     }
 
     void EffectList::save(ESMWriter& esm) const
     {
-        for (std::vector<ENAMstruct>::const_iterator it = mList.begin(); it != mList.end(); ++it)
+        for (const ENAMstruct& enam : mList)
         {
-            esm.writeHNT<ENAMstruct>("ENAM", *it, 24);
+            esm.writeNamedComposite("ENAM", enam);
         }
     }
 
diff --git a/components/esm3/effectlist.hpp b/components/esm3/effectlist.hpp
index 8f2cb959d6..de13797496 100644
--- a/components/esm3/effectlist.hpp
+++ b/components/esm3/effectlist.hpp
@@ -9,9 +9,6 @@ namespace ESM
     class ESMReader;
     class ESMWriter;
 
-#pragma pack(push)
-#pragma pack(1)
-
     /** Defines a spell effect. Shared between SPEL (Spells), ALCH
      (Potions) and ENCH (Item enchantments) records
      */
@@ -28,7 +25,6 @@ namespace ESM
         int32_t mRange; // 0 - self, 1 - touch, 2 - target (RangeType enum)
         int32_t mArea, mDuration, mMagnMin, mMagnMax;
     };
-#pragma pack(pop)
 
     /// EffectList, ENAM subrecord
     struct EffectList
diff --git a/components/esm3/esmreader.hpp b/components/esm3/esmreader.hpp
index 276adf749c..4af2264828 100644
--- a/components/esm3/esmreader.hpp
+++ b/components/esm3/esmreader.hpp
@@ -189,6 +189,11 @@ namespace ESM
             decompose(value, [&](auto&... args) { (getT(args), ...); });
         }
 
+        void getSubComposite(auto& value)
+        {
+            decompose(value, [&](auto&... args) { getHT(args...); });
+        }
+
         template <typename T, typename = std::enable_if_t<IsReadable<T>>>
         void skipHT()
         {
diff --git a/components/esm3/loadcrea.cpp b/components/esm3/loadcrea.cpp
index 1db79e8e76..5a0d8048bc 100644
--- a/components/esm3/loadcrea.cpp
+++ b/components/esm3/loadcrea.cpp
@@ -69,8 +69,7 @@ namespace ESM
                     mSpells.add(esm);
                     break;
                 case fourCC("AIDT"):
-                    esm.getSubHeader();
-                    esm.getComposite(mAiData);
+                    esm.getSubComposite(mAiData);
                     break;
                 case fourCC("DODT"):
                 case fourCC("DNAM"):
diff --git a/components/esm3/loadnpc.cpp b/components/esm3/loadnpc.cpp
index 92b16638c2..58a8bfa55e 100644
--- a/components/esm3/loadnpc.cpp
+++ b/components/esm3/loadnpc.cpp
@@ -102,8 +102,7 @@ namespace ESM
                     mInventory.add(esm);
                     break;
                 case fourCC("AIDT"):
-                    esm.getSubHeader();
-                    esm.getComposite(mAiData);
+                    esm.getSubComposite(mAiData);
                     break;
                 case fourCC("DODT"):
                 case fourCC("DNAM"):
diff --git a/components/esm3/loadweap.cpp b/components/esm3/loadweap.cpp
index 31c03b00fe..f06abf4e7c 100644
--- a/components/esm3/loadweap.cpp
+++ b/components/esm3/loadweap.cpp
@@ -1,11 +1,20 @@
 #include "loadweap.hpp"
 
-#include "components/esm/defs.hpp"
+#include <components/esm/defs.hpp>
+#include <components/misc/concepts.hpp>
+
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
 namespace ESM
 {
+    template <Misc::SameAsWithoutCvref<Weapon::WPDTstruct> T>
+    void decompose(T&& v, const auto& f)
+    {
+        f(v.mWeight, v.mValue, v.mType, v.mHealth, v.mSpeed, v.mReach, v.mEnchant, v.mChop, v.mSlash, v.mThrust,
+            v.mFlags);
+    }
+
     void Weapon::load(ESMReader& esm, bool& isDeleted)
     {
         isDeleted = false;
@@ -29,8 +38,7 @@ namespace ESM
                     mName = esm.getHString();
                     break;
                 case fourCC("WPDT"):
-                    esm.getHT(mData.mWeight, mData.mValue, mData.mType, mData.mHealth, mData.mSpeed, mData.mReach,
-                        mData.mEnchant, mData.mChop, mData.mSlash, mData.mThrust, mData.mFlags);
+                    esm.getSubComposite(mData);
                     hasData = true;
                     break;
                 case fourCC("SCRI"):
@@ -68,7 +76,7 @@ namespace ESM
 
         esm.writeHNCString("MODL", mModel);
         esm.writeHNOCString("FNAM", mName);
-        esm.writeHNT("WPDT", mData, 32);
+        esm.writeNamedComposite("WPDT", mData);
         esm.writeHNOCRefId("SCRI", mScript);
         esm.writeHNOCString("ITEX", mIcon);
         esm.writeHNOCRefId("ENAM", mEnchant);
@@ -84,9 +92,9 @@ namespace ESM
         mData.mSpeed = 0;
         mData.mReach = 0;
         mData.mEnchant = 0;
-        mData.mChop[0] = mData.mChop[1] = 0;
-        mData.mSlash[0] = mData.mSlash[1] = 0;
-        mData.mThrust[0] = mData.mThrust[1] = 0;
+        mData.mChop.fill(0);
+        mData.mSlash.fill(0);
+        mData.mThrust.fill(0);
         mData.mFlags = 0;
 
         mName.clear();
diff --git a/components/esm3/loadweap.hpp b/components/esm3/loadweap.hpp
index ba1599b1df..8323176a64 100644
--- a/components/esm3/loadweap.hpp
+++ b/components/esm3/loadweap.hpp
@@ -1,6 +1,7 @@
 #ifndef OPENMW_ESM_WEAP_H
 #define OPENMW_ESM_WEAP_H
 
+#include <array>
 #include <string>
 
 #include "components/esm/refid.hpp"
@@ -59,8 +60,6 @@ namespace ESM
             Silver = 0x02
         };
 
-#pragma pack(push)
-#pragma pack(1)
         struct WPDTstruct
         {
             float mWeight;
@@ -69,10 +68,9 @@ namespace ESM
             uint16_t mHealth;
             float mSpeed, mReach;
             uint16_t mEnchant; // Enchantment points. The real value is mEnchant/10.f
-            unsigned char mChop[2], mSlash[2], mThrust[2]; // Min and max
+            std::array<unsigned char, 2> mChop, mSlash, mThrust; // Min and max
             int32_t mFlags;
         }; // 32 bytes
-#pragma pack(pop)
 
         WPDTstruct mData;