From 915ffe224169f76355ed0cf547f2ee193e006d4f Mon Sep 17 00:00:00 2001 From: Capostrophic Date: Thu, 14 May 2020 00:48:28 +0300 Subject: [PATCH 1/2] Handle non-node roots more gracefully (bug #5416) --- CHANGELOG.md | 1 + components/nifbullet/bulletnifloader.cpp | 21 ++++++-------- components/nifosg/nifloader.cpp | 36 +++++++++++++----------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b88b5f9d..5ba33d939 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index dafee1b49..b1a970f91 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -132,23 +132,20 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) mStaticMesh.reset(); mAvoidStaticMesh.reset(); - if (nif.numRoots() < 1) + Nif::Node* node; + for (size_t i = 0; i < nif.numRoots(); ++i) + { + Nif::Record* r = nif.getRoot(i); + assert(r != nullptr); + if ((node = dynamic_cast(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(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 compound (new btCompoundShape); diff --git a/components/nifosg/nifloader.cpp b/components/nifosg/nifloader.cpp index 650d8db94..e115ad104 100644 --- a/components/nifosg/nifloader.cpp +++ b/components/nifosg/nifloader.cpp @@ -247,22 +247,23 @@ namespace NifOsg static void loadKf(Nif::NIFFilePtr nif, KeyframeHolder& target) { - if(nif->numRoots() < 1) + const Nif::NiSequenceStreamHelper *seq; + for (size_t i = 0; i < nif->numRoots(); ++i) { - nif->warn("Found no root nodes"); - return; + const Nif::Record *r = nif->getRoot(i); + assert(r != nullptr); + if (r->recType == Nif::RC_NiSequenceStreamHelper) + { + seq = static_cast(r); + break; + } } - const Nif::Record *r = nif->getRoot(0); - assert(r != nullptr); - - if(r->recType != Nif::RC_NiSequenceStreamHelper) + if (!seq) { - nif->warn("First root was not a NiSequenceStreamHelper, but a "+ - r->recName+"."); + nif->warn("Found no NiSequenceStreamHelper root record"); return; } - const Nif::NiSequenceStreamHelper *seq = static_cast(r); Nif::ExtraPtr extra = seq->extra; if(extra.empty() || extra->recType != Nif::RC_NiTextKeyExtraData) @@ -303,15 +304,16 @@ namespace NifOsg osg::ref_ptr load(Nif::NIFFilePtr nif, Resource::ImageManager* imageManager) { - if (nif->numRoots() < 1) + const Nif::Node* nifNode; + for (size_t i = 0; i < nif->numRoots(); ++i) + { + const Nif::Record* r = nif->getRoot(i); + if ((nifNode = dynamic_cast(r))) + break; + } + if (!nifNode) nif->fail("Found no root nodes"); - const Nif::Record* r = nif->getRoot(0); - - const Nif::Node* nifNode = dynamic_cast(r); - if (nifNode == nullptr) - nif->fail("First root was not a node, but a " + r->recName); - osg::ref_ptr textkeys (new TextKeyMapHolder); osg::ref_ptr created = handleNode(nifNode, nullptr, imageManager, std::vector(), 0, false, false, false, &textkeys->mTextKeys); From 30558c2434a75ed5e57473b85b1728b2f3e63b29 Mon Sep 17 00:00:00 2001 From: Capostrophic Date: Thu, 14 May 2020 10:00:33 +0300 Subject: [PATCH 2/2] Try to resolve CI concerns --- .../nifloader/testbulletnifloader.cpp | 13 ------------- components/nifbullet/bulletnifloader.cpp | 5 +++-- components/nifosg/nifloader.cpp | 10 ++++++---- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index 36f251246..7c2797236 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/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 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)); diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index b1a970f91..15834ffad 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -132,8 +132,9 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) mStaticMesh.reset(); mAvoidStaticMesh.reset(); - Nif::Node* node; - for (size_t i = 0; i < nif.numRoots(); ++i) + 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); diff --git a/components/nifosg/nifloader.cpp b/components/nifosg/nifloader.cpp index e115ad104..8d08ebef1 100644 --- a/components/nifosg/nifloader.cpp +++ b/components/nifosg/nifloader.cpp @@ -247,8 +247,9 @@ namespace NifOsg static void loadKf(Nif::NIFFilePtr nif, KeyframeHolder& target) { - const Nif::NiSequenceStreamHelper *seq; - for (size_t i = 0; i < nif->numRoots(); ++i) + const Nif::NiSequenceStreamHelper *seq = nullptr; + const size_t numRoots = nif->numRoots(); + for (size_t i = 0; i < numRoots; ++i) { const Nif::Record *r = nif->getRoot(i); assert(r != nullptr); @@ -304,8 +305,9 @@ namespace NifOsg osg::ref_ptr load(Nif::NIFFilePtr nif, Resource::ImageManager* imageManager) { - const Nif::Node* nifNode; - for (size_t i = 0; i < nif->numRoots(); ++i) + 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(r)))