Don't rely on subrecord order part 2

Nice side effects:
 - Subrecord name comparison now uses magic number instead of string (faster)
 - Improves the error message for unknown subrecords: will print the record in question instead of failing to read the next record with a strange error

components/esm/aipackage.cpp

@@ -41,29 +41,6 @@ namespace ESM
 
     }
 
-    void AIPackageList::load(ESMReader &esm)
-    {
-        mList.clear();
-        while (esm.hasMoreSubs()) {
-            // initialize every iteration
-            esm.getSubName();
-            switch (esm.retSubName().val)
-            {
-                case AI_Wander:
-                case AI_Activate:
-                case AI_Escort:
-                case AI_Follow:
-                case AI_Travel:
-                case AI_CNDT:
-
-                    add(esm);
-                    break;
-            default:
-                return;
-            }
-        }
-    }
-
     void AIPackageList::save(ESMWriter &esm) const
     {
         typedef std::vector<AIPackage>::const_iterator PackageIter;

components/esm/aipackage.hpp

@@ -94,9 +94,6 @@ namespace ESM
         /// Add a single AIPackage, assumes subrecord name was already read
         void add(ESMReader &esm);
 
-        /// TODO: remove this method. The ESM format does not guarantee that all AI packages follow one another
-        void load(ESMReader &esm);
-
         void save(ESMWriter &esm) const;
     };
 }

components/esm/effectlist.cpp

@@ -8,13 +8,18 @@ namespace ESM {
 void EffectList::load(ESMReader &esm)
 {
     mList.clear();
-    ENAMstruct s;
     while (esm.isNextSub("ENAM")) {
-        esm.getHT(s, 24);
-        mList.push_back(s);
+        add(esm);
     }
 }
 
+void EffectList::add(ESMReader &esm)
+{
+    ENAMstruct s;
+    esm.getHT(s, 24);
+    mList.push_back(s);
+}
+
 void EffectList::save(ESMWriter &esm) const
 {
     for (std::vector<ENAMstruct>::const_iterator it = mList.begin(); it != mList.end(); ++it) {

components/esm/effectlist.hpp

@@ -29,11 +29,15 @@ namespace ESM
     };
     #pragma pack(pop)
 
+    /// EffectList, ENAM subrecord
     struct EffectList
     {
-
         std::vector<ENAMstruct> mList;
 
+        /// Load one effect, assumes subrecord name was already read
+        void add(ESMReader &esm);
+
+        /// Load all effects
         void load(ESMReader &esm);
         void save(ESMWriter &esm) const;
     };

components/esm/loadacti.cpp

@@ -8,18 +8,34 @@ namespace ESM
 {
     unsigned int Activator::sRecordId = REC_ACTI;
 
-void Activator::load(ESMReader &esm)
-{
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-    mScript = esm.getHNOString("SCRI");
-}
-void Activator::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNOCString("SCRI", mScript);
-}
+    void Activator::load(ESMReader &esm)
+    {
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+    }
+    void Activator::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNOCString("SCRI", mScript);
+    }
 
     void Activator::blank()
     {

components/esm/loadalch.cpp

@@ -8,24 +8,51 @@ namespace ESM
 {
     unsigned int Potion::sRecordId = REC_ALCH;
 
-void Potion::load(ESMReader &esm)
-{
-    mModel = esm.getHNOString("MODL");
-    mIcon = esm.getHNOString("TEXT"); // not ITEX here for some reason
-    mScript = esm.getHNOString("SCRI");
-    mName = esm.getHNOString("FNAM");
-    esm.getHNT(mData, "ALDT", 12);
-    mEffects.load(esm);
-}
-void Potion::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("TEXT", mIcon);
-    esm.writeHNOCString("SCRI", mScript);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNT("ALDT", mData, 12);
-    mEffects.save(esm);
-}
+    void Potion::load(ESMReader &esm)
+    {
+        mEffects.mList.clear();
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'T','E','X','T'>::value: // not ITEX here for some reason
+                    mIcon = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'A','L','D','T'>::value:
+                    esm.getHT(mData, 12);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'E','N','A','M'>::value:
+                    mEffects.add(esm);
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing ALDT");
+    }
+    void Potion::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("TEXT", mIcon);
+        esm.writeHNOCString("SCRI", mScript);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNT("ALDT", mData, 12);
+        mEffects.save(esm);
+    }
 
     void Potion::blank()
     {

components/esm/loadappa.cpp

@@ -10,25 +10,35 @@ namespace ESM
 
 void Apparatus::load(ESMReader &esm)
 {
-    // we will not treat duplicated subrecords as errors here
+    bool hasData = false;
     while (esm.hasMoreSubs())
     {
         esm.getSubName();
-        NAME subName = esm.retSubName();
-
-        if (subName == "MODL")
-            mModel = esm.getHString();
-        else if (subName == "FNAM")
-            mName = esm.getHString();
-        else if (subName == "AADT")
-            esm.getHT(mData);
-        else if (subName == "SCRI")
-            mScript = esm.getHString();
-        else if (subName == "ITEX")
-            mIcon = esm.getHString();
-        else
-            esm.fail("wrong subrecord type " + subName.toString() + " for APPA record");
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'M','O','D','L'>::value:
+                mModel = esm.getHString();
+                break;
+            case ESM::FourCC<'F','N','A','M'>::value:
+                mName = esm.getHString();
+                break;
+            case ESM::FourCC<'A','A','D','T'>::value:
+                esm.getHT(mData);
+                hasData = true;
+                break;
+            case ESM::FourCC<'S','C','R','I'>::value:
+                mScript = esm.getHString();
+                break;
+            case ESM::FourCC<'I','T','E','X'>::value:
+                mIcon = esm.getHString();
+                break;
+            default:
+                esm.fail("Unknown subrecord");
+        }
     }
+    if (!hasData)
+        esm.fail("Missing AADT");
 }
 
 void Apparatus::save(ESMWriter &esm) const

components/esm/loadbody.cpp

@@ -11,9 +11,30 @@ namespace ESM
 
 void BodyPart::load(ESMReader &esm)
 {
-    mModel = esm.getHNString("MODL");
-    mRace = esm.getHNOString("FNAM");
-    esm.getHNT(mData, "BYDT", 4);
+    bool hasData = false;
+    while (esm.hasMoreSubs())
+    {
+        esm.getSubName();
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'M','O','D','L'>::value:
+                mModel = esm.getHString();
+                break;
+            case ESM::FourCC<'F','N','A','M'>::value:
+                mRace = esm.getHString();
+                break;
+            case ESM::FourCC<'B','Y','D','T'>::value:
+                esm.getHT(mData, 4);
+                hasData = true;
+                break;
+            default:
+                esm.fail("Unknown subrecord");
+        }
+    }
+
+    if (!hasData)
+        esm.fail("Missing BYDT subrecord");
 }
 void BodyPart::save(ESMWriter &esm) const
 {

components/esm/loadbsgn.cpp

@@ -10,11 +10,29 @@ namespace ESM
 
 void BirthSign::load(ESMReader &esm)
 {
-    mName = esm.getHNOString("FNAM");
-    mTexture = esm.getHNOString("TNAM");
-    mDescription = esm.getHNOString("DESC");
-
-    mPowers.load(esm);
+    mPowers.mList.clear();
+    while (esm.hasMoreSubs())
+    {
+        esm.getSubName();
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'F','N','A','M'>::value:
+                mName = esm.getHString();
+                break;
+            case ESM::FourCC<'T','N','A','M'>::value:
+                mTexture = esm.getHString();
+                break;
+            case ESM::FourCC<'D','E','S','C'>::value:
+                mDescription = esm.getHString();
+                break;
+            case ESM::FourCC<'N','P','C','S'>::value:
+                mPowers.add(esm);
+                break;
+            default:
+                esm.fail("Unknown subrecord");
+        }
+    }
 }
 
 void BirthSign::save(ESMWriter &esm) const

components/esm/loadclas.cpp

@@ -10,17 +10,17 @@ namespace ESM
 {
     unsigned int Class::sRecordId = REC_CLAS;
 
-const Class::Specialization Class::sSpecializationIds[3] = {
-  Class::Combat,
-  Class::Magic,
-  Class::Stealth
-};
+    const Class::Specialization Class::sSpecializationIds[3] = {
+      Class::Combat,
+      Class::Magic,
+      Class::Stealth
+    };
 
-const char *Class::sGmstSpecializationIds[3] = {
-  "sSpecializationCombat",
-  "sSpecializationMagic",
-  "sSpecializationStealth"
-};
+    const char *Class::sGmstSpecializationIds[3] = {
+      "sSpecializationCombat",
+      "sSpecializationMagic",
+      "sSpecializationStealth"
+    };
 
 
     int& Class::CLDTstruct::getSkill (int index, bool major)
@@ -39,22 +39,40 @@ const char *Class::sGmstSpecializationIds[3] = {
         return mSkills[index][major ? 1 : 0];
     }
 
-void Class::load(ESMReader &esm)
-{
-    mName = esm.getHNOString("FNAM");
-    esm.getHNT(mData, "CLDT", 60);
-
-    if (mData.mIsPlayable > 1)
-        esm.fail("Unknown bool value");
-
-    mDescription = esm.getHNOString("DESC");
-}
-void Class::save(ESMWriter &esm) const
-{
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNT("CLDT", mData, 60);
-    esm.writeHNOString("DESC", mDescription);
-}
+    void Class::load(ESMReader &esm)
+    {
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'C','L','D','T'>::value:
+                    esm.getHT(mData, 60);
+                    if (mData.mIsPlayable > 1)
+                        esm.fail("Unknown bool value");
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'D','E','S','C'>::value:
+                    mDescription = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing CLDT subrecord");
+    }
+    void Class::save(ESMWriter &esm) const
+    {
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNT("CLDT", mData, 60);
+        esm.writeHNOString("DESC", mDescription);
+    }
 
     void Class::blank()
     {

components/esm/loadcont.cpp

@@ -7,60 +7,79 @@
 namespace ESM
 {
 
-void InventoryList::add(ESMReader &esm)
-{
-    ContItem ci;
-    esm.getHT(ci, 36);
-    mList.push_back(ci);
-}
-
-void InventoryList::load(ESMReader &esm)
-{
-    mList.clear();
-    while (esm.isNextSub("NPCO"))
+    void InventoryList::add(ESMReader &esm)
     {
-        add(esm);
+        ContItem ci;
+        esm.getHT(ci, 36);
+        mList.push_back(ci);
     }
-}
 
-void InventoryList::save(ESMWriter &esm) const
-{
-    for (std::vector<ContItem>::const_iterator it = mList.begin(); it != mList.end(); ++it)
+    void InventoryList::save(ESMWriter &esm) const
     {
-        esm.writeHNT("NPCO", *it, 36);
+        for (std::vector<ContItem>::const_iterator it = mList.begin(); it != mList.end(); ++it)
+        {
+            esm.writeHNT("NPCO", *it, 36);
+        }
     }
-}
 
     unsigned int Container::sRecordId = REC_CONT;
 
-void Container::load(ESMReader &esm)
-{
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-    esm.getHNT(mWeight, "CNDT", 4);
-    esm.getHNT(mFlags, "FLAG", 4);
-
-    if (mFlags & 0xf4)
-        esm.fail("Unknown flags");
-    if (!(mFlags & 0x8))
-        esm.fail("Flag 8 not set");
-
-    mScript = esm.getHNOString("SCRI");
-
-    mInventory.load(esm);
-}
+    void Container::load(ESMReader &esm)
+    {
+        mInventory.mList.clear();
+        bool hasWeight = false;
+        bool hasFlags = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'C','N','D','T'>::value:
+                    esm.getHT(mWeight, 4);
+                    hasWeight = true;
+                    break;
+                case ESM::FourCC<'F','L','A','G'>::value:
+                    esm.getHT(mFlags, 4);
+                    if (mFlags & 0xf4)
+                        esm.fail("Unknown flags");
+                    if (!(mFlags & 0x8))
+                        esm.fail("Flag 8 not set");
+                    hasFlags = true;
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'N','P','C','O'>::value:
+                    mInventory.add(esm);
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasWeight)
+            esm.fail("Missing CNDT subrecord");
+        if (!hasFlags)
+            esm.fail("Missing FLAG subrecord");
+    }
 
-void Container::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNT("CNDT", mWeight, 4);
-    esm.writeHNT("FLAG", mFlags, 4);
+    void Container::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNT("CNDT", mWeight, 4);
+        esm.writeHNT("FLAG", mFlags, 4);
 
-    esm.writeHNOCString("SCRI", mScript);
+        esm.writeHNOCString("SCRI", mScript);
 
-    mInventory.save(esm);
-}
+        mInventory.save(esm);
+    }
 
     void Container::blank()
     {

components/esm/loadcont.hpp

@@ -22,6 +22,7 @@ struct ContItem
     NAME32 mItem;
 };
 
+/// InventoryList, NPCO subrecord
 struct InventoryList
 {
     std::vector<ContItem> mList;
@@ -29,8 +30,6 @@ struct InventoryList
     /// Load one item, assumes subrecord name is already read
     void add(ESMReader &esm);
 
-    /// TODO: remove this method, the ESM format doesn't guarantee that all ContItems follow one another
-    void load(ESMReader &esm);
     void save(ESMWriter &esm) const;
 };
 

components/esm/loadcrea.cpp

@@ -8,55 +8,94 @@ namespace ESM {
 
     unsigned int Creature::sRecordId = REC_CREA;
 
-void Creature::load(ESMReader &esm)
-{
-    mPersistent = esm.getRecordFlags() & 0x0400;
-
-    mModel = esm.getHNString("MODL");
-    mOriginal = esm.getHNOString("CNAM");
-    mName = esm.getHNOString("FNAM");
-    mScript = esm.getHNOString("SCRI");
-
-    esm.getHNT(mData, "NPDT", 96);
-
-    esm.getHNT(mFlags, "FLAG");
-    mScale = 1.0;
-    esm.getHNOT(mScale, "XSCL");
-
-    mInventory.load(esm);
-    mSpells.load(esm);
-
-    if (esm.isNextSub("AIDT"))
+    void Creature::load(ESMReader &esm)
     {
-        esm.getHExact(&mAiData, sizeof(mAiData));
-        mHasAI = true;
-    }
-    else
-        mHasAI = false;
+        mPersistent = esm.getRecordFlags() & 0x0400;
 
-    mAiPackage.load(esm);
-    esm.skipRecord();
-}
+        mAiPackage.mList.clear();
+        mInventory.mList.clear();
+        mSpells.mList.clear();
 
-void Creature::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("CNAM", mOriginal);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNOCString("SCRI", mScript);
-    esm.writeHNT("NPDT", mData, 96);
-    esm.writeHNT("FLAG", mFlags);
-    if (mScale != 1.0) {
-        esm.writeHNT("XSCL", mScale);
+        mScale = 1.f;
+        mHasAI = false;
+        bool hasNpdt = false;
+        bool hasFlags = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'C','N','A','M'>::value:
+                    mOriginal = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'N','P','D','T'>::value:
+                    esm.getHT(mData, 96);
+                    hasNpdt = true;
+                    break;
+                case ESM::FourCC<'F','L','A','G'>::value:
+                    esm.getHT(mFlags);
+                    hasFlags = true;
+                    break;
+                case ESM::FourCC<'X','S','C','L'>::value:
+                    esm.getHT(mScale);
+                    break;
+                case ESM::FourCC<'N','P','C','O'>::value:
+                    mInventory.add(esm);
+                    break;
+                case ESM::FourCC<'N','P','C','S'>::value:
+                    mSpells.add(esm);
+                    break;
+                case ESM::FourCC<'A','I','D','T'>::value:
+                    esm.getHExact(&mAiData, sizeof(mAiData));
+                    mHasAI = true;
+                    break;
+                case AI_Wander:
+                case AI_Activate:
+                case AI_Escort:
+                case AI_Follow:
+                case AI_Travel:
+                case AI_CNDT:
+                    mAiPackage.add(esm);
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasNpdt)
+            esm.fail("Missing NPDT subrecord");
+        if (!hasFlags)
+            esm.fail("Missing FLAG subrecord");
     }
 
-    mInventory.save(esm);
-    mSpells.save(esm);
-    if (mHasAI) {
-        esm.writeHNT("AIDT", mAiData, sizeof(mAiData));
+    void Creature::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("CNAM", mOriginal);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNOCString("SCRI", mScript);
+        esm.writeHNT("NPDT", mData, 96);
+        esm.writeHNT("FLAG", mFlags);
+        if (mScale != 1.0) {
+            esm.writeHNT("XSCL", mScale);
+        }
+
+        mInventory.save(esm);
+        mSpells.save(esm);
+        if (mHasAI) {
+            esm.writeHNT("AIDT", mAiData, sizeof(mAiData));
+        }
+        mAiPackage.save(esm);
     }
-    mAiPackage.save(esm);
-}
 
     void Creature::blank()
     {

components/esm/loadench.cpp

@@ -10,8 +10,28 @@ namespace ESM
 
 void Enchantment::load(ESMReader &esm)
 {
-    esm.getHNT(mData, "ENDT", 16);
-    mEffects.load(esm);
+    mEffects.mList.clear();
+    bool hasData = false;
+    while (esm.hasMoreSubs())
+    {
+        esm.getSubName();
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'E','N','D','T'>::value:
+                esm.getHT(mData, 16);
+                hasData = true;
+                break;
+            case ESM::FourCC<'E','N','A','M'>::value:
+                mEffects.add(esm);
+                break;
+            default:
+                esm.fail("Unknown subrecord");
+                break;
+        }
+    }
+    if (!hasData)
+        esm.fail("Missing ENDT subrecord");
 }
 
 void Enchantment::save(ESMWriter &esm) const

components/esm/loadfact.cpp

@@ -28,27 +28,46 @@ namespace ESM
 
 void Faction::load(ESMReader &esm)
 {
-    mName = esm.getHNOString("FNAM");
+    mReactions.clear();
+    for (int i=0;i<10;++i)
+        mRanks[i].clear();
 
-    // Read rank names. These are optional.
-    int i = 0;
-    while (esm.isNextSub("RNAM") && i < 10)
-        mRanks[i++] = esm.getHString();
-
-    // Main data struct
-    esm.getHNT(mData, "FADT", 240);
-
-    if (mData.mIsHidden > 1)
-        esm.fail("Unknown flag!");
-
-    // Read faction response values
+    int rankCounter=0;
+    bool hasData = false;
     while (esm.hasMoreSubs())
     {
-        std::string faction = esm.getHNString("ANAM");
-        int reaction;
-        esm.getHNT(reaction, "INTV");
-        mReactions[faction] = reaction;
+        esm.getSubName();
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'F','N','A','M'>::value:
+                mName = esm.getHString();
+                break;
+            case ESM::FourCC<'R','N','A','M'>::value:
+                if (rankCounter >= 10)
+                    esm.fail("Rank out of range");
+                mRanks[rankCounter++] = esm.getHString();
+                break;
+            case ESM::FourCC<'F','A','D','T'>::value:
+                esm.getHT(mData, 240);
+                if (mData.mIsHidden > 1)
+                    esm.fail("Unknown flag!");
+                hasData = true;
+                break;
+            case ESM::FourCC<'A','N','A','M'>::value:
+            {
+                std::string faction = esm.getHString();
+                int reaction;
+                esm.getHNT(reaction, "INTV");
+                mReactions[faction] = reaction;
+                break;
+            }
+            default:
+                esm.fail("Unknown subrecord");
+        }
     }
+    if (!hasData)
+        esm.fail("Missing FADT subrecord");
 }
 void Faction::save(ESMWriter &esm) const
 {

components/esm/loadingr.cpp

@@ -8,45 +8,71 @@ namespace ESM
 {
     unsigned int Ingredient::sRecordId = REC_INGR;
 
-void Ingredient::load(ESMReader &esm)
-{
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-    esm.getHNT(mData, "IRDT", 56);
-    mScript = esm.getHNOString("SCRI");
-    mIcon = esm.getHNOString("ITEX");
-    // horrible hack to fix broken data in records
-    for (int i=0; i<4; ++i)
+    void Ingredient::load(ESMReader &esm)
     {
-        if (mData.mEffectID[i] != 85 &&
-            mData.mEffectID[i] != 22 &&
-            mData.mEffectID[i] != 17 &&
-            mData.mEffectID[i] != 79 &&
-            mData.mEffectID[i] != 74)
+        bool hasData = false;
+        while (esm.hasMoreSubs())
         {
-            mData.mAttributes[i] = -1;
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'I','R','D','T'>::value:
+                    esm.getHT(mData, 56);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'I','T','E','X'>::value:
+                    mIcon = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
         }
 
-        // is this relevant in cycle from 0 to 4?
-        if (mData.mEffectID[i] != 89 &&
-            mData.mEffectID[i] != 26 &&
-            mData.mEffectID[i] != 21 &&
-            mData.mEffectID[i] != 83 &&
-            mData.mEffectID[i] != 78)
+        if (!hasData)
+            esm.fail("Missing IRDT subrecord");
+
+        // horrible hack to fix broken data in records
+        for (int i=0; i<4; ++i)
         {
-            mData.mSkills[i] = -1;
+            if (mData.mEffectID[i] != 85 &&
+                mData.mEffectID[i] != 22 &&
+                mData.mEffectID[i] != 17 &&
+                mData.mEffectID[i] != 79 &&
+                mData.mEffectID[i] != 74)
+            {
+                mData.mAttributes[i] = -1;
+            }
+
+            // is this relevant in cycle from 0 to 4?
+            if (mData.mEffectID[i] != 89 &&
+                mData.mEffectID[i] != 26 &&
+                mData.mEffectID[i] != 21 &&
+                mData.mEffectID[i] != 83 &&
+                mData.mEffectID[i] != 78)
+            {
+                mData.mSkills[i] = -1;
+            }
         }
     }
-}
 
-void Ingredient::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNT("IRDT", mData, 56);
-    esm.writeHNOCString("SCRI", mScript);
-    esm.writeHNOCString("ITEX", mIcon);
-}
+    void Ingredient::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNT("IRDT", mData, 56);
+        esm.writeHNOCString("SCRI", mScript);
+        esm.writeHNOCString("ITEX", mIcon);
+    }
 
     void Ingredient::blank()
     {

components/esm/loadlevlist.cpp

@@ -7,49 +7,50 @@
 namespace ESM
 {
 
-void LevelledListBase::load(ESMReader &esm)
-{
-    esm.getHNT(mFlags, "DATA");
-    esm.getHNT(mChanceNone, "NNAM");
-
-    if (esm.isNextSub("INDX"))
-    {
-        int len;
-        esm.getHT(len);
-        mList.resize(len);
-    }
-    else
+    void LevelledListBase::load(ESMReader &esm)
     {
-        esm.skipRecord();
-        return;
-    }
+        esm.getHNT(mFlags, "DATA");
+        esm.getHNT(mChanceNone, "NNAM");
 
-    // If this levelled list was already loaded by a previous content file,
-    // we overwrite the list. Merging lists should probably be left to external tools,
-    // with the limited amount of information there is in the records, all merging methods
-    // will be flawed in some way. For a proper fix the ESM format would have to be changed
-    // to actually track list changes instead of including the whole list for every file
-    // that does something with that list.
+        if (esm.isNextSub("INDX"))
+        {
+            int len;
+            esm.getHT(len);
+            mList.resize(len);
+        }
+        else
+        {
+            // Original engine ignores rest of the record, even if there are items following
+            esm.skipRecord();
+            return;
+        }
 
-    for (size_t i = 0; i < mList.size(); i++)
-    {
-        LevelItem &li = mList[i];
-        li.mId = esm.getHNString(mRecName);
-        esm.getHNT(li.mLevel, "INTV");
-    }
-}
-void LevelledListBase::save(ESMWriter &esm) const
-{
-    esm.writeHNT("DATA", mFlags);
-    esm.writeHNT("NNAM", mChanceNone);
-    esm.writeHNT<int>("INDX", mList.size());
+        // If this levelled list was already loaded by a previous content file,
+        // we overwrite the list. Merging lists should probably be left to external tools,
+        // with the limited amount of information there is in the records, all merging methods
+        // will be flawed in some way. For a proper fix the ESM format would have to be changed
+        // to actually track list changes instead of including the whole list for every file
+        // that does something with that list.
 
-    for (std::vector<LevelItem>::const_iterator it = mList.begin(); it != mList.end(); ++it)
+        for (size_t i = 0; i < mList.size(); i++)
+        {
+            LevelItem &li = mList[i];
+            li.mId = esm.getHNString(mRecName);
+            esm.getHNT(li.mLevel, "INTV");
+        }
+    }
+    void LevelledListBase::save(ESMWriter &esm) const
     {
-        esm.writeHNCString(mRecName, it->mId);
-        esm.writeHNT("INTV", it->mLevel);
+        esm.writeHNT("DATA", mFlags);
+        esm.writeHNT("NNAM", mChanceNone);
+        esm.writeHNT<int>("INDX", mList.size());
+
+        for (std::vector<LevelItem>::const_iterator it = mList.begin(); it != mList.end(); ++it)
+        {
+            esm.writeHNCString(mRecName, it->mId);
+            esm.writeHNT("INTV", it->mLevel);
+        }
     }
-}
 
     void LevelledListBase::blank()
     {

components/esm/loadligh.cpp

@@ -8,25 +8,50 @@ namespace ESM
 {
     unsigned int Light::sRecordId = REC_LIGH;
 
-void Light::load(ESMReader &esm)
-{
-    mModel = esm.getHNOString("MODL");
-    mName = esm.getHNOString("FNAM");
-    mIcon = esm.getHNOString("ITEX");
-    assert(sizeof(mData) == 24);
-    esm.getHNT(mData, "LHDT", 24);
-    mScript = esm.getHNOString("SCRI");
-    mSound = esm.getHNOString("SNAM");
-}
-void Light::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNOCString("ITEX", mIcon);
-    esm.writeHNT("LHDT", mData, 24);
-    esm.writeHNOCString("SCRI", mScript);
-    esm.writeHNOCString("SNAM", mSound);
-}
+    void Light::load(ESMReader &esm)
+    {
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'I','T','E','X'>::value:
+                    mIcon = esm.getHString();
+                    break;
+                case ESM::FourCC<'L','H','D','T'>::value:
+                    esm.getHT(mData, 24);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','N','A','M'>::value:
+                    mSound = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing LHDT subrecord");
+    }
+    void Light::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNOCString("ITEX", mIcon);
+        esm.writeHNT("LHDT", mData, 24);
+        esm.writeHNOCString("SCRI", mScript);
+        esm.writeHNOCString("SNAM", mSound);
+    }
 
     void Light::blank()
     {

components/esm/loadlock.cpp

@@ -8,26 +8,48 @@ namespace ESM
 {
     unsigned int Lockpick::sRecordId = REC_LOCK;
 
-void Lockpick::load(ESMReader &esm)
-{
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-
-    esm.getHNT(mData, "LKDT", 16);
-
-    mScript = esm.getHNOString("SCRI");
-    mIcon = esm.getHNOString("ITEX");
-}
+    void Lockpick::load(ESMReader &esm)
+    {
+        bool hasData = true;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'L','K','D','T'>::value:
+                    esm.getHT(mData, 16);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'I','T','E','X'>::value:
+                    mIcon = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing LKDT subrecord");
+    }
 
-void Lockpick::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
+    void Lockpick::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
 
-    esm.writeHNT("LKDT", mData, 16);
-    esm.writeHNOString("SCRI", mScript);
-    esm.writeHNOCString("ITEX", mIcon);
-}
+        esm.writeHNT("LKDT", mData, 16);
+        esm.writeHNOString("SCRI", mScript);
+        esm.writeHNOCString("ITEX", mIcon);
+    }
 
     void Lockpick::blank()
     {

components/esm/loadmgef.cpp

@@ -246,7 +246,7 @@ void MagicEffect::load(ESMReader &esm)
                 mDescription = esm.getHString();
                 break;
             default:
-                esm.fail("Unknown subrecord " + esm.retSubName().toString());
+                esm.fail("Unknown subrecord");
         }
     }
 }

components/esm/loadmgef.hpp

@@ -57,9 +57,9 @@ struct MagicEffect
         // Glow color for enchanted items with this effect
         int mRed, mGreen, mBlue;
 
-        float mUnknown1;
+        float mUnknown1; // Called "Size X" in CS
         float mSpeed; // Speed of fired projectile
-        float mUnknown2;
+        float mUnknown2; // Called "Size Cap" in CS
     }; // 36 bytes
 
     static const std::map<short,std::string> sNames;

components/esm/loadnpc.cpp

@@ -99,7 +99,7 @@ namespace ESM
                     mAiPackage.add(esm);
                     break;
                 default:
-                    esm.fail("Unknown subrecord " + esm.retSubName().toString());
+                    esm.fail("Unknown subrecord");
             }
         }
         if (!hasNpdt)

components/esm/loadprob.cpp

@@ -8,26 +8,48 @@ namespace ESM
 {
     unsigned int Probe::sRecordId = REC_PROB;
 
-void Probe::load(ESMReader &esm)
-{
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-
-    esm.getHNT(mData, "PBDT", 16);
-
-    mScript = esm.getHNOString("SCRI");
-    mIcon = esm.getHNOString("ITEX");
-}
+    void Probe::load(ESMReader &esm)
+    {
+        bool hasData = true;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'M','O','D','L'>::value:
+                    mModel = esm.getHString();
+                    break;
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'P','B','D','T'>::value:
+                    esm.getHT(mData, 16);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'S','C','R','I'>::value:
+                    mScript = esm.getHString();
+                    break;
+                case ESM::FourCC<'I','T','E','X'>::value:
+                    mIcon = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing PBDT subrecord");
+    }
 
-void Probe::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-    esm.writeHNOCString("FNAM", mName);
+    void Probe::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+        esm.writeHNOCString("FNAM", mName);
 
-    esm.writeHNT("PBDT", mData, 16);
-    esm.writeHNOString("SCRI", mScript);
-    esm.writeHNOCString("ITEX", mIcon);
-}
+        esm.writeHNT("PBDT", mData, 16);
+        esm.writeHNOString("SCRI", mScript);
+        esm.writeHNOCString("ITEX", mIcon);
+    }
 
     void Probe::blank()
     {

components/esm/loadrace.cpp

@@ -43,7 +43,7 @@ void Race::load(ESMReader &esm)
                 mPowers.add(esm);
                 break;
             default:
-                esm.fail("Unknown subrecord " + esm.retSubName().toString());
+                esm.fail("Unknown subrecord");
         }
     }
     if (!hasData)

components/esm/loadrepa.cpp

@@ -10,13 +10,35 @@ namespace ESM
 
 void Repair::load(ESMReader &esm)
 {
-    mModel = esm.getHNString("MODL");
-    mName = esm.getHNOString("FNAM");
-
-    esm.getHNT(mData, "RIDT", 16);
-
-    mScript = esm.getHNOString("SCRI");
-    mIcon = esm.getHNOString("ITEX");
+    bool hasData = true;
+    while (esm.hasMoreSubs())
+    {
+        esm.getSubName();
+        uint32_t name = esm.retSubName().val;
+        switch (name)
+        {
+            case ESM::FourCC<'M','O','D','L'>::value:
+                mModel = esm.getHString();
+                break;
+            case ESM::FourCC<'F','N','A','M'>::value:
+                mName = esm.getHString();
+                break;
+            case ESM::FourCC<'R','I','D','T'>::value:
+                esm.getHT(mData, 16);
+                hasData = true;
+                break;
+            case ESM::FourCC<'S','C','R','I'>::value:
+                mScript = esm.getHString();
+                break;
+            case ESM::FourCC<'I','T','E','X'>::value:
+                mIcon = esm.getHString();
+                break;
+            default:
+                esm.fail("Unknown subrecord");
+        }
+    }
+    if (!hasData)
+        esm.fail("Missing RIDT subrecord");
 }
 
 void Repair::save(ESMWriter &esm) const

components/esm/loadscpt.cpp

@@ -9,17 +9,7 @@ namespace ESM
 
     unsigned int Script::sRecordId = REC_SCPT;
 
-void Script::load(ESMReader &esm)
-{
-    SCHD data;
-    esm.getHNT(data, "SCHD", 52);
-    mData = data.mData;
-    mId = data.mName.toString();
-
-    mVarNames.clear();
-
-    // List of local variables
-    if (esm.isNextSub("SCVR"))
+    void Script::loadSCVR(ESMReader &esm)
     {
         int s = mData.mStringTableSize;
 
@@ -66,58 +56,70 @@ void Script::load(ESMReader &esm)
         }
     }
 
-    // Script mData
-    if (esm.isNextSub("SCDT"))
+    void Script::load(ESMReader &esm)
     {
-        mScriptData.resize(mData.mScriptDataSize);
-        esm.getHExact(&mScriptData[0], mScriptData.size());
-    }
+        SCHD data;
+        esm.getHNT(data, "SCHD", 52);
+        mData = data.mData;
+        mId = data.mName.toString();
+
+        mVarNames.clear();
 
-    // Script text
-    mScriptText = esm.getHNOString("SCTX");
-
-    // NOTE: A minor hack/workaround...
-    //
-    // MAO_Containers.esp from Morrowind Acoustic Overhaul has SCVR records
-    // at the end (see Bug #1849). Since OpenMW does not use SCVR subrecords
-    // for variable names just skip these as a quick fix.  An alternative
-    // solution would be to decode and validate SCVR subrecords even if they
-    // appear here.
-    if (esm.isNextSub("SCVR")) {
-        esm.skipHSub();
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'S','C','V','R'>::value:
+                    // list of local variables
+                    loadSCVR(esm);
+                    break;
+                case ESM::FourCC<'S','C','D','T'>::value:
+                    // compiled script
+                    mScriptData.resize(mData.mScriptDataSize);
+                    esm.getHExact(&mScriptData[0], mScriptData.size());
+                    break;
+                case ESM::FourCC<'S','C','T','X'>::value:
+                    mScriptText = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
     }
-}
-void Script::save(ESMWriter &esm) const
-{
-    std::string varNameString;
-    if (!mVarNames.empty())
-        for (std::vector<std::string>::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it)
-            varNameString.append(*it);
 
-    SCHD data;
-    memset(&data, 0, sizeof(data));
+    void Script::save(ESMWriter &esm) const
+    {
+        std::string varNameString;
+        if (!mVarNames.empty())
+            for (std::vector<std::string>::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it)
+                varNameString.append(*it);
 
-    data.mData = mData;
-    memcpy(data.mName.name, mId.c_str(), mId.size());
+        SCHD data;
+        memset(&data, 0, sizeof(data));
 
-    esm.writeHNT("SCHD", data, 52);
+        data.mData = mData;
+        memcpy(data.mName.name, mId.c_str(), mId.size());
 
-    if (!mVarNames.empty())
-    {
-        esm.startSubRecord("SCVR");
-        for (std::vector<std::string>::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it)
+        esm.writeHNT("SCHD", data, 52);
+
+        if (!mVarNames.empty())
         {
-            esm.writeHCString(*it);
+            esm.startSubRecord("SCVR");
+            for (std::vector<std::string>::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it)
+            {
+                esm.writeHCString(*it);
+            }
+            esm.endRecord("SCVR");
         }
-        esm.endRecord("SCVR");
-    }
 
-    esm.startSubRecord("SCDT");
-    esm.write(reinterpret_cast<const char * >(&mScriptData[0]), mData.mScriptDataSize);
-    esm.endRecord("SCDT");
+        esm.startSubRecord("SCDT");
+        esm.write(reinterpret_cast<const char * >(&mScriptData[0]), mData.mScriptDataSize);
+        esm.endRecord("SCDT");
 
-    esm.writeHNOString("SCTX", mScriptText);
-}
+        esm.writeHNOString("SCTX", mScriptText);
+    }
 
     void Script::blank()
     {

components/esm/loadscpt.hpp

@@ -53,6 +53,9 @@ public:
 
     void blank();
     ///< Set record to default state (does not touch the ID/index).
+
+private:
+    void loadSCVR(ESMReader &esm);
 };
 }
 #endif

components/esm/loadskil.cpp

@@ -129,23 +129,47 @@ namespace ESM
 
     unsigned int Skill::sRecordId = REC_SKIL;
 
-void Skill::load(ESMReader &esm)
-{
-    esm.getHNT(mIndex, "INDX");
-    esm.getHNT(mData, "SKDT", 24);
-    mDescription = esm.getHNOString("DESC");
+    void Skill::load(ESMReader &esm)
+    {
+        bool hasIndex = false;
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'I','N','D','X'>::value:
+                    esm.getHT(mIndex);
+                    hasIndex = true;
+                    break;
+                case ESM::FourCC<'S','K','D','T'>::value:
+                    esm.getHT(mData, 24);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'D','E','S','C'>::value:
+                    mDescription = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasIndex)
+            esm.fail("Missing INDX");
+        if (!hasData)
+            esm.fail("Missing SKDT");
 
-    // create an ID from the index and the name (only used in the editor and likely to change in the
-    // future)
-    mId = indexToId (mIndex);
-}
+        // create an ID from the index and the name (only used in the editor and likely to change in the
+        // future)
+        mId = indexToId (mIndex);
+    }
 
-void Skill::save(ESMWriter &esm) const
-{
-    esm.writeHNT("INDX", mIndex);
-    esm.writeHNT("SKDT", mData, 24);
-    esm.writeHNOString("DESC", mDescription);
-}
+    void Skill::save(ESMWriter &esm) const
+    {
+        esm.writeHNT("INDX", mIndex);
+        esm.writeHNT("SKDT", mData, 24);
+        esm.writeHNOString("DESC", mDescription);
+    }
 
     void Skill::blank()
     {

components/esm/loadsndg.cpp

@@ -8,19 +8,38 @@ namespace ESM
 {
     unsigned int SoundGenerator::sRecordId = REC_SNDG;
 
-void SoundGenerator::load(ESMReader &esm)
-{
-  esm.getHNT(mType, "DATA", 4);
-
-  mCreature = esm.getHNOString("CNAM");
-  mSound = esm.getHNOString("SNAM");
-}
-void SoundGenerator::save(ESMWriter &esm) const
-{
-    esm.writeHNT("DATA", mType, 4);
-    esm.writeHNOCString("CNAM", mCreature);
-    esm.writeHNOCString("SNAM", mSound);
-}
+    void SoundGenerator::load(ESMReader &esm)
+    {
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'D','A','T','A'>::value:
+                    esm.getHT(mType, 4);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'C','N','A','M'>::value:
+                    mCreature = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','N','A','M'>::value:
+                    mSound = esm.getHString();
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing DATA");
+    }
+    void SoundGenerator::save(ESMWriter &esm) const
+    {
+        esm.writeHNT("DATA", mType, 4);
+        esm.writeHNOCString("CNAM", mCreature);
+        esm.writeHNOCString("SNAM", mSound);
+    }
 
     void SoundGenerator::blank()
     {

components/esm/loadsoun.cpp

@@ -8,22 +8,35 @@ namespace ESM
 {
     unsigned int Sound::sRecordId = REC_SOUN;
 
-void Sound::load(ESMReader &esm)
-{
-    mSound = esm.getHNOString("FNAM");
-    esm.getHNT(mData, "DATA", 3);
-    /*
-     cout << "vol=" << (int)data.volume
-     << " min=" << (int)data.minRange
-     << " max=" << (int)data.maxRange
-     << endl;
-     */
-}
-void Sound::save(ESMWriter &esm) const
-{
-    esm.writeHNOCString("FNAM", mSound);
-    esm.writeHNT("DATA", mData, 3);
-}
+    void Sound::load(ESMReader &esm)
+    {
+        bool hasData = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mSound = esm.getHString();
+                    break;
+                case ESM::FourCC<'D','A','T','A'>::value:
+                    esm.getHT(mData, 3);
+                    hasData = true;
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing DATA");
+    }
+
+    void Sound::save(ESMWriter &esm) const
+    {
+        esm.writeHNOCString("FNAM", mSound);
+        esm.writeHNT("DATA", mData, 3);
+    }
 
     void Sound::blank()
     {

components/esm/loadspel.cpp

@@ -8,40 +8,40 @@ namespace ESM
 {
     unsigned int Spell::sRecordId = REC_SPEL;
 
-void Spell::load(ESMReader &esm)
-{
-    bool hasData = false;
-    while (esm.hasMoreSubs())
+    void Spell::load(ESMReader &esm)
     {
-        esm.getSubName();
-        uint32_t val = esm.retSubName().val;
-
-        switch (val)
+        bool hasData = false;
+        while (esm.hasMoreSubs())
         {
-            case ESM::FourCC<'F','N','A','M'>::value:
-                mName = esm.getHString();
-                break;
-            case ESM::FourCC<'S','P','D','T'>::value:
-                esm.getHT(mData, 12);
-                hasData = true;
-                break;
-            case ESM::FourCC<'E','N','A','M'>::value:
-                ENAMstruct s;
-                esm.getHT(s, 24);
-                mEffects.mList.push_back(s);
-                break;
+            esm.getSubName();
+            uint32_t val = esm.retSubName().val;
+
+            switch (val)
+            {
+                case ESM::FourCC<'F','N','A','M'>::value:
+                    mName = esm.getHString();
+                    break;
+                case ESM::FourCC<'S','P','D','T'>::value:
+                    esm.getHT(mData, 12);
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'E','N','A','M'>::value:
+                    ENAMstruct s;
+                    esm.getHT(s, 24);
+                    mEffects.mList.push_back(s);
+                    break;
+            }
         }
+        if (!hasData)
+            esm.fail("Missing SPDT subrecord");
     }
-    if (!hasData)
-        esm.fail("Missing SPDT subrecord");
-}
 
-void Spell::save(ESMWriter &esm) const
-{
-    esm.writeHNOCString("FNAM", mName);
-    esm.writeHNT("SPDT", mData, 12);
-    mEffects.save(esm);
-}
+    void Spell::save(ESMWriter &esm) const
+    {
+        esm.writeHNOCString("FNAM", mName);
+        esm.writeHNT("SPDT", mData, 12);
+        mEffects.save(esm);
+    }
 
     void Spell::blank()
     {

components/esm/loadsscr.cpp

@@ -8,15 +8,37 @@ namespace ESM
 {
     unsigned int StartScript::sRecordId = REC_SSCR;
 
-void StartScript::load(ESMReader &esm)
-{
-    mData = esm.getHNString("DATA");
-    mScript = esm.getHNString("NAME");
-}
-void StartScript::save(ESMWriter &esm) const
-{
-    esm.writeHNString("DATA", mData);
-    esm.writeHNString("NAME", mScript);
-}
+    void StartScript::load(ESMReader &esm)
+    {
+        bool hasData = false;
+        bool hasName = false;
+        while (esm.hasMoreSubs())
+        {
+            esm.getSubName();
+            uint32_t name = esm.retSubName().val;
+            switch (name)
+            {
+                case ESM::FourCC<'D','A','T','A'>::value:
+                    mData = esm.getHString();
+                    hasData = true;
+                    break;
+                case ESM::FourCC<'N','A','M','E'>::value:
+                    mScript = esm.getHString();
+                    hasName = true;
+                    break;
+                default:
+                    esm.fail("Unknown subrecord");
+            }
+        }
+        if (!hasData)
+            esm.fail("Missing DATA");
+        if (!hasName)
+            esm.fail("Missing NAME");
+    }
+    void StartScript::save(ESMWriter &esm) const
+    {
+        esm.writeHNString("DATA", mData);
+        esm.writeHNString("NAME", mScript);
+    }
 
 }

components/esm/loadstat.cpp

@@ -8,15 +8,16 @@ namespace ESM
 {
     unsigned int Static::sRecordId = REC_STAT;
 
-void Static::load(ESMReader &esm)
-{
-    mPersistent = esm.getRecordFlags() & 0x0400;
-    mModel = esm.getHNString("MODL");
-}
-void Static::save(ESMWriter &esm) const
-{
-    esm.writeHNCString("MODL", mModel);
-}
+    void Static::load(ESMReader &esm)
+    {
+        mPersistent = esm.getRecordFlags() & 0x0400;
+
+        mModel = esm.getHNString("MODL");
+    }
+    void Static::save(ESMWriter &esm) const
+    {
+        esm.writeHNCString("MODL", mModel);
+    }
 
     void Static::blank()
     {

components/esm/loadtes3.hpp

@@ -50,8 +50,8 @@ namespace ESM
             NAME32 mPlayerName;
         };
         GMDT mGameData; // Used in .ess savegames only
-        std::vector<unsigned char> mSCRD; // Used in .ess savegames only, screenshot?
-        std::vector<unsigned char> mSCRS; // Used in .ess savegames only, screenshot?
+        std::vector<unsigned char> mSCRD; // Used in .ess savegames only, unknown
+        std::vector<unsigned char> mSCRS; // Used in .ess savegames only, screenshot
 
         Data mData;
         int mFormat;

components/esm/spelllist.cpp

@@ -5,14 +5,6 @@
 
 namespace ESM {
 
-void SpellList::load(ESMReader &esm)
-{
-    mList.clear();
-    while (esm.isNextSub("NPCS")) {
-        add(esm);
-    }
-}
-
 void SpellList::add(ESMReader &esm)
 {
     mList.push_back(esm.getHString());

components/esm/spelllist.hpp

@@ -11,6 +11,7 @@ namespace ESM
 
     /** A list of references to spells and spell effects. This is shared
      between the records BSGN, NPC and RACE.
+     NPCS subrecord.
      */
     struct SpellList
     {
@@ -22,9 +23,6 @@ namespace ESM
         /// Load one spell, assumes the subrecord name was already read
         void add(ESMReader &esm);
 
-        /// Load all spells
-        /// TODO: remove this method, the ESM format doesn't guarantee that all spell subrecords follow one another
-        void load(ESMReader &esm);
         void save(ESMWriter &esm) const;
     };
 }