From ec1c6ee1715be1aceb8498d2f7cc881b0472e19d Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 24 Feb 2024 14:03:24 +0100 Subject: [PATCH 1/2] Use ESM::decompose to handle ENAMstruct --- apps/openmw_test_suite/esm3/testsaveload.cpp | 29 ++++++++++++++++++++ components/esm3/aipackage.cpp | 12 +++----- components/esm3/effectlist.cpp | 13 +++++++-- components/esm3/effectlist.hpp | 4 --- components/esm3/esmreader.hpp | 11 ++++++++ components/esm3/loadcrea.cpp | 3 +- components/esm3/loadnpc.cpp | 3 +- 7 files changed, 56 insertions(+), 19 deletions(-) diff --git a/apps/openmw_test_suite/esm3/testsaveload.cpp b/apps/openmw_test_suite/esm3/testsaveload.cpp index eda1fa963e..8c7896b08a 100644 --- a/apps/openmw_test_suite/esm3/testsaveload.cpp +++ b/apps/openmw_test_suite/esm3/testsaveload.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -525,6 +526,34 @@ 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); + } + 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 + namespace ESM { + template 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::const_iterator it = mList.begin(); it != mList.end(); ++it) + for (const ENAMstruct& enam : mList) { - esm.writeHNT("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..ca9f191a5d 100644 --- a/components/esm3/esmreader.hpp +++ b/components/esm3/esmreader.hpp @@ -189,6 +189,17 @@ namespace ESM decompose(value, [&](auto&... args) { (getT(args), ...); }); } + void getSubComposite(auto& value) + { + decompose(value, [&](auto&... args) { + constexpr size_t size = (0 + ... + sizeof(decltype(args))); + getSubHeader(); + if (mCtx.leftSub != size) + reportSubSizeMismatch(size, mCtx.leftSub); + (getT(args), ...); + }); + } + template >> 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"): From 7d7e8939abc16301fba58c8975faae72e02b0936 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 24 Feb 2024 16:55:58 +0100 Subject: [PATCH 2/2] Use ESM::decompose to handle WPDTstruct --- apps/openmw/mwclass/creature.cpp | 6 +-- apps/openmw/mwclass/npc.cpp | 6 +-- apps/openmw/mwmechanics/combat.cpp | 18 ++++---- apps/openmw_test_suite/esm3/testsaveload.cpp | 48 ++++++++++++++++++++ components/esm3/esmreader.hpp | 8 +--- components/esm3/loadweap.cpp | 22 ++++++--- components/esm3/loadweap.hpp | 6 +-- 7 files changed, 82 insertions(+), 32 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()->mBase->mData.mChop; + attack = weapon.get()->mBase->mData.mChop.data(); else if (type == ESM::Weapon::AT_Slash) - attack = weapon.get()->mBase->mData.mSlash; + attack = weapon.get()->mBase->mData.mSlash.data(); else if (type == ESM::Weapon::AT_Thrust) - attack = weapon.get()->mBase->mData.mThrust; + attack = weapon.get()->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()->mBase->mData.mChop; + attack = weapon.get()->mBase->mData.mChop.data(); else if (type == ESM::Weapon::AT_Slash) - attack = weapon.get()->mBase->mData.mSlash; + attack = weapon.get()->mBase->mData.mSlash.data(); else if (type == ESM::Weapon::AT_Thrust) - attack = weapon.get()->mBase->mData.mThrust; + attack = weapon.get()->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()->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()->mBase->mData.mChop; - damage += attack[0] + ((attack[1] - attack[0]) * attackStrength); - + { + const auto& attack = weapon.get()->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()->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 8c7896b08a..6d5fdf1c14 100644 --- a/apps/openmw_test_suite/esm3/testsaveload.cpp +++ b/apps/openmw_test_suite/esm3/testsaveload.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -554,6 +555,53 @@ namespace ESM 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/esmreader.hpp b/components/esm3/esmreader.hpp index ca9f191a5d..4af2264828 100644 --- a/components/esm3/esmreader.hpp +++ b/components/esm3/esmreader.hpp @@ -191,13 +191,7 @@ namespace ESM void getSubComposite(auto& value) { - decompose(value, [&](auto&... args) { - constexpr size_t size = (0 + ... + sizeof(decltype(args))); - getSubHeader(); - if (mCtx.leftSub != size) - reportSubSizeMismatch(size, mCtx.leftSub); - (getT(args), ...); - }); + decompose(value, [&](auto&... args) { getHT(args...); }); } template >> 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 +#include + #include "esmreader.hpp" #include "esmwriter.hpp" namespace ESM { + template 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 #include #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 mChop, mSlash, mThrust; // Min and max int32_t mFlags; }; // 32 bytes -#pragma pack(pop) WPDTstruct mData;