Merge pull request #2842 from Capostrophic/nifroot

Handle non-node roots more gracefully (bug #5416)

CHANGELOG.md

@@ -13,6 +13,7 @@
     Bug #5369: Spawnpoint in the Grazelands doesn't produce oversized creatures
     Bug #5370: Opening an unlocked but trapped door uses the key
     Bug #5400: Editor: Verifier checks race of non-skin bodyparts
+    Bug #5416: Junk non-node records before the root node are not handled gracefully
     Feature #5362: Show the soul gems' trapped soul in count dialog
 
 0.46.0

apps/openmw_test_suite/nifloader/testbulletnifloader.cpp

@@ -349,19 +349,6 @@ namespace
         EXPECT_EQ(*result, expected);
     }
 
-    TEST_F(TestBulletNifLoader, for_root_not_nif_node_should_return_default)
-    {
-        StrictMock<RecordMock> record;
-
-        EXPECT_CALL(mNifFile, numRoots()).WillOnce(Return(1));
-        EXPECT_CALL(mNifFile, getRoot(0)).WillOnce(Return(&record));
-        const auto result = mLoader.load(mNifFile);
-
-        Resource::BulletShape expected;
-
-        EXPECT_EQ(*result, expected);
-    }
-
     TEST_F(TestBulletNifLoader, for_default_root_nif_node_should_return_default)
     {
         EXPECT_CALL(mNifFile, numRoots()).WillOnce(Return(1));

components/nifbullet/bulletnifloader.cpp

@@ -132,23 +132,21 @@ osg::ref_ptr<Resource::BulletShape> BulletNifLoader::load(const Nif::File& nif)
     mStaticMesh.reset();
     mAvoidStaticMesh.reset();
 
-    if (nif.numRoots() < 1)
+    Nif::Node* node = nullptr;
+    const size_t numRoots = nif.numRoots();
+    for (size_t i = 0; i < numRoots; ++i)
+    {
+        Nif::Record* r = nif.getRoot(i);
+        assert(r != nullptr);
+        if ((node = dynamic_cast<Nif::Node*>(r)))
+            break;
+    }
+    if (!node)
     {
         warn("Found no root nodes in NIF.");
         return mShape;
     }
 
-    Nif::Record *r = nif.getRoot(0);
-    assert(r != nullptr);
-
-    Nif::Node *node = dynamic_cast<Nif::Node*>(r);
-    if (node == nullptr)
-    {
-        warn("First root in file was not a node, but a " +
-             r->recName + ". Skipping file.");
-        return mShape;
-    }
-
     if (findBoundingBox(node))
     {
         std::unique_ptr<btCompoundShape> compound (new btCompoundShape);

components/nifosg/nifloader.cpp

@@ -247,22 +247,24 @@ namespace NifOsg
 
         static void loadKf(Nif::NIFFilePtr nif, KeyframeHolder& target)
         {
-            if(nif->numRoots() < 1)
+            const Nif::NiSequenceStreamHelper *seq = nullptr;
+            const size_t numRoots = nif->numRoots();
+            for (size_t i = 0; i < numRoots; ++i)
             {
-                nif->warn("Found no root nodes");
+                const Nif::Record *r = nif->getRoot(i);
-                return;
+                assert(r != nullptr);
+                if (r->recType == Nif::RC_NiSequenceStreamHelper)
+                {
+                    seq = static_cast<const Nif::NiSequenceStreamHelper*>(r);
+                    break;
+                }
             }
 
-            const Nif::Record *r = nif->getRoot(0);
+            if (!seq)
-            assert(r != nullptr);
-
-            if(r->recType != Nif::RC_NiSequenceStreamHelper)
             {
-                nif->warn("First root was not a NiSequenceStreamHelper, but a "+
+                nif->warn("Found no NiSequenceStreamHelper root record");
-                          r->recName+".");
                 return;
             }
-            const Nif::NiSequenceStreamHelper *seq = static_cast<const Nif::NiSequenceStreamHelper*>(r);
 
             Nif::ExtraPtr extra = seq->extra;
             if(extra.empty() || extra->recType != Nif::RC_NiTextKeyExtraData)
@@ -303,15 +305,17 @@ namespace NifOsg
 
         osg::ref_ptr<osg::Node> load(Nif::NIFFilePtr nif, Resource::ImageManager* imageManager)
         {
-            if (nif->numRoots() < 1)
+            const Nif::Node* nifNode = nullptr;
+            const size_t numRoots = nif->numRoots();
+            for (size_t i = 0; i < numRoots; ++i)
+            {
+                const Nif::Record* r = nif->getRoot(i);
+                if ((nifNode = dynamic_cast<const Nif::Node*>(r)))
+                    break;
+            }
+            if (!nifNode)
                 nif->fail("Found no root nodes");
 
-            const Nif::Record* r = nif->getRoot(0);
-
-            const Nif::Node* nifNode = dynamic_cast<const Nif::Node*>(r);
-            if (nifNode == nullptr)
-                nif->fail("First root was not a node, but a " + r->recName);
-
             osg::ref_ptr<TextKeyMapHolder> textkeys (new TextKeyMapHolder);
 
             osg::ref_ptr<osg::Node> created = handleNode(nifNode, nullptr, imageManager, std::vector<unsigned int>(), 0, false, false, false, &textkeys->mTextKeys);