Modernize extra data records

apps/openmw_test_suite/nif/node.hpp

@@ -13,7 +13,7 @@ namespace Nif::Testing
 
     inline void init(Extra& value)
     {
-        value.next = ExtraPtr(nullptr);
+        value.mNext = ExtraPtr(nullptr);
     }
 
     inline void init(Named& value)

apps/openmw_test_suite/nifloader/testbulletnifloader.cpp

@@ -996,7 +996,7 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision)
     {
-        mNiStringExtraData.string = "NCC__";
+        mNiStringExtraData.mData = "NCC__";
         mNiStringExtraData.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1024,8 +1024,8 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_not_first_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision)
     {
-        mNiStringExtraData.next = Nif::ExtraPtr(&mNiStringExtraData2);
+        mNiStringExtraData.mNext = Nif::ExtraPtr(&mNiStringExtraData2);
-        mNiStringExtraData2.string = "NCC__";
+        mNiStringExtraData2.mData = "NCC__";
         mNiStringExtraData2.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1052,7 +1052,7 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_extra_data_string_starting_with_nc_should_return_shape_with_nocollision)
     {
-        mNiStringExtraData.string = "NC___";
+        mNiStringExtraData.mData = "NC___";
         mNiStringExtraData.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1079,8 +1079,8 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_not_first_extra_data_string_starting_with_nc_should_return_shape_with_nocollision)
     {
-        mNiStringExtraData.next = Nif::ExtraPtr(&mNiStringExtraData2);
+        mNiStringExtraData.mNext = Nif::ExtraPtr(&mNiStringExtraData2);
-        mNiStringExtraData2.string = "NC___";
+        mNiStringExtraData2.mData = "NC___";
         mNiStringExtraData2.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1141,7 +1141,7 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_extra_data_string_mrk_should_return_shape_with_null_collision_shape)
     {
-        mNiStringExtraData.string = "MRK";
+        mNiStringExtraData.mData = "MRK";
         mNiStringExtraData.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1160,7 +1160,7 @@ namespace
 
     TEST_F(TestBulletNifLoader, bsx_editor_marker_flag_disables_collision_for_markers)
     {
-        mNiIntegerExtraData.data = 32; // BSX flag "editor marker"
+        mNiIntegerExtraData.mData = 32; // BSX flag "editor marker"
         mNiIntegerExtraData.recType = Nif::RC_BSXFlags;
         mNiTriShape.extralist.push_back(Nif::ExtraPtr(&mNiIntegerExtraData));
         mNiTriShape.parents.push_back(&mNiNode);
@@ -1181,7 +1181,7 @@ namespace
     TEST_F(TestBulletNifLoader,
         for_tri_shape_child_node_with_extra_data_string_mrk_and_other_collision_node_should_return_shape_with_triangle_mesh_shape_with_all_meshes)
     {
-        mNiStringExtraData.string = "MRK";
+        mNiStringExtraData.mData = "MRK";
         mNiStringExtraData.recType = Nif::RC_NiStringExtraData;
         mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData);
         mNiTriShape.parents.push_back(&mNiNode2);

components/nif/base.cpp

@@ -5,11 +5,11 @@ namespace Nif
     void Extra::read(NIFStream* nif)
     {
         if (nif->getVersion() >= NIFStream::generateVersion(10, 0, 1, 0))
-            name = nif->getString();
+            nif->read(mName);
         else if (nif->getVersion() <= NIFStream::generateVersion(4, 2, 2, 0))
         {
-            next.read(nif);
+            mNext.read(nif);
-            recordSize = nif->getUInt();
+            nif->read(mRecordSize);
         }
     }
 

components/nif/base.hpp

@@ -13,12 +13,12 @@ namespace Nif
     // An extra data record. All the extra data connected to an object form a linked list.
     struct Extra : public Record
     {
-        std::string name;
+        std::string mName;
-        ExtraPtr next; // Next extra data record in the list
+        ExtraPtr mNext; // Next extra data record in the list
-        unsigned int recordSize{ 0u };
+        uint32_t mRecordSize{ 0u };
 
         void read(NIFStream* nif) override;
-        void post(Reader& nif) override { next.post(nif); }
+        void post(Reader& nif) override { mNext.post(nif); }
     };
 
     struct Controller : public Record

components/nif/extra.cpp

@@ -6,25 +6,21 @@ namespace Nif
     void NiExtraData::read(NIFStream* nif)
     {
         Extra::read(nif);
-        nif->readVector(data, recordSize);
-    }
 
-    void NiStringExtraData::read(NIFStream* nif)
+        nif->readVector(mData, mRecordSize);
-    {
-        Extra::read(nif);
-        string = nif->getString();
     }
 
     void NiTextKeyExtraData::read(NIFStream* nif)
     {
         Extra::read(nif);
 
-        int keynum = nif->getInt();
+        uint32_t numKeys;
-        list.resize(keynum);
+        nif->read(numKeys);
-        for (int i = 0; i < keynum; i++)
+        mList.resize(numKeys);
+        for (TextKey& key : mList)
         {
-            list[i].time = nif->getFloat();
+            nif->read(key.mTime);
-            list[i].text = nif->getString();
+            nif->read(key.mText);
         }
     }
 
@@ -32,81 +28,39 @@ namespace Nif
     {
         Extra::read(nif);
 
-        nif->skip(nif->getUShort() * sizeof(float)); // vertex weights I guess
+        nif->skip(nif->get<uint16_t>() * sizeof(float)); // vertex weights I guess
-    }
-
-    void NiIntegerExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-
-        data = nif->getUInt();
-    }
-
-    void NiIntegersExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-
-        nif->readVector(data, nif->getUInt());
-    }
-
-    void NiBinaryExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-        nif->readVector(data, nif->getUInt());
-    }
-
-    void NiBooleanExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-        data = nif->getBoolean();
-    }
-
-    void NiVectorExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-        data = nif->getVector4();
-    }
-
-    void NiFloatExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-
-        data = nif->getFloat();
-    }
-
-    void NiFloatsExtraData::read(NIFStream* nif)
-    {
-        Extra::read(nif);
-        nif->readVector(data, nif->getUInt());
     }
 
     void BSBound::read(NIFStream* nif)
     {
         Extra::read(nif);
-        center = nif->getVector3();
+
-        halfExtents = nif->getVector3();
+        nif->read(mCenter);
+        nif->read(mExtents);
     }
 
     void BSFurnitureMarker::LegacyFurniturePosition::read(NIFStream* nif)
     {
-        mOffset = nif->getVector3();
+        nif->read(mOffset);
-        mOrientation = nif->getUShort();
+        nif->read(mOrientation);
-        mPositionRef = nif->getChar();
+        nif->read(mPositionRef);
         nif->skip(1); // Position ref 2
     }
 
     void BSFurnitureMarker::FurniturePosition::read(NIFStream* nif)
     {
-        mOffset = nif->getVector3();
+        nif->read(mOffset);
-        mHeading = nif->getFloat();
+        nif->read(mHeading);
-        mType = nif->getUShort();
+        nif->read(mType);
-        mEntryPoint = nif->getUShort();
+        nif->read(mEntryPoint);
     }
 
     void BSFurnitureMarker::read(NIFStream* nif)
     {
         Extra::read(nif);
-        unsigned int num = nif->getUInt();
+
+        uint32_t num;
+        nif->read(num);
         if (nif->getBethVersion() <= NIFFile::BethVersion::BETHVER_FO3)
         {
             mLegacyMarkers.resize(num);
@@ -124,19 +78,20 @@ namespace Nif
     void BSInvMarker::read(NIFStream* nif)
     {
         Extra::read(nif);
-        float rotX = nif->getUShort() / 1000.0;
-        float rotY = nif->getUShort() / 1000.0;
-        float rotZ = nif->getUShort() / 1000.0;
-        mScale = nif->getFloat();
 
+        float rotX = nif->get<uint16_t>() / 1000.f;
+        float rotY = nif->get<uint16_t>() / 1000.f;
+        float rotZ = nif->get<uint16_t>() / 1000.f;
         mRotation = osg::Quat(rotX, osg::X_AXIS, rotY, osg::Y_AXIS, rotZ, osg::Z_AXIS);
+        nif->read(mScale);
     }
 
     void BSBehaviorGraphExtraData::read(NIFStream* nif)
     {
         Extra::read(nif);
-        mFile = nif->getString();
+
-        mControlsBaseSkeleton = nif->getBoolean();
+        nif->read(mFile);
+        nif->read(mControlsBaseSkeleton);
     }
 
 }

components/nif/extra.hpp

@@ -1,26 +1,3 @@
-/*
-  OpenMW - The completely unofficial reimplementation of Morrowind
-  Copyright (C) 2008-2010  Nicolay Korslund
-  Email: < korslund@gmail.com >
-  WWW: https://openmw.org/
-
-  This file (extra.h) is part of the OpenMW package.
-
-  OpenMW is distributed as free software: you can redistribute it
-  and/or modify it under the terms of the GNU General Public License
-  version 3, as published by the Free Software Foundation.
-
-  This program is distributed in the hope that it will be useful, but
-  WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-  General Public License for more details.
-
-  You should have received a copy of the GNU General Public License
-  version 3 along with this program. If not, see
-  https://www.gnu.org/licenses/ .
-
- */
-
 #ifndef OPENMW_COMPONENTS_NIF_EXTRA_HPP
 #define OPENMW_COMPONENTS_NIF_EXTRA_HPP
 
@@ -29,9 +6,46 @@
 namespace Nif
 {
 
+    template <typename T>
+    struct TypedExtra : public Extra
+    {
+        T mData;
+
+        void read(NIFStream* nif) override
+        {
+            Extra::read(nif);
+
+            nif->read(mData);
+        }
+    };
+
+    template <typename T>
+    struct TypedVectorExtra : public Extra
+    {
+        std::vector<T> mData;
+
+        void read(NIFStream* nif) override
+        {
+            Extra::read(nif);
+
+            nif->readVector(mData, nif->get<uint32_t>());
+        }
+    };
+
+    using NiBooleanExtraData = TypedExtra<bool>;
+    using NiFloatExtraData = TypedExtra<float>;
+    using NiIntegerExtraData = TypedExtra<uint32_t>;
+    using NiStringExtraData = TypedExtra<std::string>;
+    using NiVectorExtraData = TypedExtra<osg::Vec4f>;
+
+    using NiBinaryExtraData = TypedVectorExtra<uint8_t>;
+    using NiFloatsExtraData = TypedVectorExtra<float>;
+    using NiIntegersExtraData = TypedVectorExtra<uint32_t>;
+
+    // Distinct from NiBinaryExtraData, uses mRecordSize as its size
     struct NiExtraData : public Extra
     {
-        std::vector<char> data;
+        std::vector<uint8_t> mData;
 
         void read(NIFStream* nif) override;
     };
@@ -45,78 +59,17 @@ namespace Nif
     {
         struct TextKey
         {
-            float time;
+            float mTime;
-            std::string text;
+            std::string mText;
         };
-        std::vector<TextKey> list;
+        std::vector<TextKey> mList;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiStringExtraData : public Extra
-    {
-        /* Known meanings:
-           "MRK" - marker, only visible in the editor, not rendered in-game
-           "NCC" - no collision except with the camera
-           Anything else starting with "NC" - no collision
-        */
-        std::string string;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiIntegerExtraData : public Extra
-    {
-        unsigned int data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiIntegersExtraData : public Extra
-    {
-        std::vector<unsigned int> data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiBinaryExtraData : public Extra
-    {
-        std::vector<char> data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiBooleanExtraData : public Extra
-    {
-        bool data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiVectorExtraData : public Extra
-    {
-        osg::Vec4f data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiFloatExtraData : public Extra
-    {
-        float data;
-
-        void read(NIFStream* nif) override;
-    };
-
-    struct NiFloatsExtraData : public Extra
-    {
-        std::vector<float> data;
 
         void read(NIFStream* nif) override;
     };
 
     struct BSBound : public Extra
     {
-        osg::Vec3f center, halfExtents;
+        osg::Vec3f mCenter, mExtents;
 
         void read(NIFStream* nif) override;
     };
@@ -149,7 +102,7 @@ namespace Nif
     struct BSInvMarker : public Extra
     {
         osg::Quat mRotation;
-        float mScale = 1.0f;
+        float mScale;
 
         void read(NIFStream* nif) override;
     };
@@ -162,5 +115,5 @@ namespace Nif
         void read(NIFStream* nif) override;
     };
 
-} // Namespace
+}
 #endif

components/nifbullet/bulletnifloader.cpp

@@ -305,7 +305,7 @@ namespace NifBullet
 
         // Check for extra data
         std::vector<Nif::ExtraPtr> extraCollection;
-        for (Nif::ExtraPtr e = node.extra; !e.empty(); e = e->next)
+        for (Nif::ExtraPtr e = node.extra; !e.empty(); e = e->mNext)
             extraCollection.emplace_back(e);
         for (const auto& extraNode : node.extralist)
             if (!extraNode.empty())
@@ -316,29 +316,30 @@ namespace NifBullet
             {
                 // String markers may contain important information
                 // affecting the entire subtree of this node
-                Nif::NiStringExtraData* sd = (Nif::NiStringExtraData*)e.getPtr();
+                auto sd = static_cast<const Nif::NiStringExtraData*>(e.getPtr());
 
-                if (Misc::StringUtils::ciStartsWith(sd->string, "NC"))
+                if (Misc::StringUtils::ciStartsWith(sd->mData, "NC"))
                 {
                     // NCC flag in vanilla is partly case sensitive: prefix NC is case insensitive but second C needs be
                     // uppercase
-                    if (sd->string.length() > 2 && sd->string[2] == 'C')
+                    if (sd->mData.length() > 2 && sd->mData[2] == 'C')
                         // Collide only with camera.
                         visualCollisionType = Resource::VisualCollisionType::Camera;
                     else
                         // No collision.
                         visualCollisionType = Resource::VisualCollisionType::Default;
                 }
-                else if (sd->string == "MRK" && args.mAutogenerated)
+                // Don't autogenerate collision if MRK is set.
+                // FIXME: verify if this covers the entire subtree
+                else if (sd->mData == "MRK" && args.mAutogenerated)
                 {
-                    // Marker can still have collision if the model explicitely specifies it via a RootCollisionNode.
                     return;
                 }
             }
             else if (e->recType == Nif::RC_BSXFlags)
             {
                 auto bsxFlags = static_cast<const Nif::NiIntegerExtraData*>(e.getPtr());
-                if (bsxFlags->data & 32) // Editor marker flag
+                if (bsxFlags->mData & 32) // Editor marker flag
                     args.mHasMarkers = true;
             }
         }

components/nifosg/nifloader.cpp

@@ -175,16 +175,16 @@ namespace
 
     void extractTextKeys(const Nif::NiTextKeyExtraData* tk, SceneUtil::TextKeyMap& textkeys)
     {
-        for (size_t i = 0; i < tk->list.size(); i++)
+        for (const Nif::NiTextKeyExtraData::TextKey& key : tk->mList)
         {
             std::vector<std::string> results;
-            Misc::StringUtils::split(tk->list[i].text, results, "\r\n");
+            Misc::StringUtils::split(key.mText, results, "\r\n");
             for (std::string& result : results)
             {
                 Misc::StringUtils::trim(result);
                 Misc::StringUtils::lowerCaseInPlace(result);
                 if (!result.empty())
-                    textkeys.emplace(tk->list[i].time, std::move(result));
+                    textkeys.emplace(key.mTime, std::move(result));
             }
         }
     }
@@ -286,9 +286,9 @@ namespace NifOsg
 
             extractTextKeys(static_cast<const Nif::NiTextKeyExtraData*>(extra.getPtr()), target.mTextKeys);
 
-            extra = extra->next;
+            extra = extra->mNext;
             Nif::ControllerPtr ctrl = seq->controller;
-            for (; !extra.empty() && !ctrl.empty(); (extra = extra->next), (ctrl = ctrl->next))
+            for (; !extra.empty() && !ctrl.empty(); (extra = extra->mNext), (ctrl = ctrl->next))
             {
                 if (extra->recType != Nif::RC_NiStringExtraData || ctrl->recType != Nif::RC_NiKeyframeController)
                 {
@@ -316,8 +316,8 @@ namespace NifOsg
                 osg::ref_ptr<SceneUtil::KeyframeController> callback = new NifOsg::KeyframeController(key);
                 setupController(key, callback, /*animflags*/ 0);
 
-                if (!target.mKeyframeControllers.emplace(strdata->string, callback).second)
+                if (!target.mKeyframeControllers.emplace(strdata->mData, callback).second)
-                    Log(Debug::Verbose) << "Controller " << strdata->string << " present more than once in "
+                    Log(Debug::Verbose) << "Controller " << strdata->mData << " present more than once in "
                                         << nif.getFilename() << ", ignoring later version";
             }
         }
@@ -648,7 +648,7 @@ namespace NifOsg
 
             std::vector<Nif::ExtraPtr> extraCollection;
 
-            for (Nif::ExtraPtr e = nifNode->extra; !e.empty(); e = e->next)
+            for (Nif::ExtraPtr e = nifNode->extra; !e.empty(); e = e->mNext)
                 extraCollection.emplace_back(e);
 
             for (const auto& extraNode : nifNode->extralist)
@@ -670,25 +670,25 @@ namespace NifOsg
 
                     // String markers may contain important information
                     // affecting the entire subtree of this obj
-                    if (sd->string == "MRK" && !Loader::getShowMarkers())
+                    if (sd->mData == "MRK" && !Loader::getShowMarkers())
                     {
                         // Marker objects. These meshes are only visible in the editor.
                         args.mHasMarkers = true;
                     }
-                    else if (sd->string == "BONE")
+                    else if (sd->mData == "BONE")
                     {
                         node->getOrCreateUserDataContainer()->addDescription("CustomBone");
                     }
-                    else if (sd->string.rfind(extraDataIdentifer, 0) == 0)
+                    else if (sd->mData.rfind(extraDataIdentifer, 0) == 0)
                     {
                         node->setUserValue(
-                            Misc::OsgUserValues::sExtraData, sd->string.substr(extraDataIdentifer.length()));
+                            Misc::OsgUserValues::sExtraData, sd->mData.substr(extraDataIdentifer.length()));
                     }
                 }
                 else if (e->recType == Nif::RC_BSXFlags)
                 {
                     auto bsxFlags = static_cast<const Nif::NiIntegerExtraData*>(e.getPtr());
-                    if (bsxFlags->data & 32) // Editor marker flag
+                    if (bsxFlags->mData & 32) // Editor marker flag
                         args.mHasMarkers = true;
                 }
             }