From 6d91fe69b268e6b2a8fc5da4a4432a5de346195d Mon Sep 17 00:00:00 2001 From: Andrei Kortunov Date: Fri, 26 Oct 2018 12:51:04 +0400 Subject: [PATCH 1/3] Revert "Handle RootCollisionNode, attached to non-root node (bug #4311)" This reverts commit ec9a1b0d0526c9aba7d7cdd0e68789ab0e747d50. --- CHANGELOG.md | 1 - components/nifbullet/bulletnifloader.cpp | 42 ++++++++---------------- components/nifbullet/bulletnifloader.hpp | 4 +-- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07c0217da6..0f668010d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,6 @@ Bug #4293: Faction members are not aware of faction ownerships in barter Bug #4304: "Follow" not working as a second AI package Bug #4307: World cleanup should remove dead bodies only if death animation is finished - Bug #4311: OpenMW does not handle RootCollisionNode correctly Bug #4327: Missing animations during spell/weapon stance switching Bug #4333: Keyboard navigation in containers is not intuitive Bug #4358: Running animation is interrupted when magic mode is toggled diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 7b206e40c2..c18b96d395 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -96,17 +96,14 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) } else { + bool autogenerated = hasAutoGeneratedCollision(node); + // files with the name convention xmodel.nif usually have keyframes stored in a separate file xmodel.kf (see Animation::addAnimSource). // assume all nodes in the file will be animated const auto filename = nif.getFilename(); const bool isAnimated = pathFileNameStartsWithX(filename); - // If the mesh has RootCollisionNode, attached to actual root node, use it as collision mesh - const Nif::Node* rootCollisionNode = getCollisionNode(node); - if (rootCollisionNode) - handleNode(filename, rootCollisionNode, 0, false, isAnimated, false); - else - handleNode(filename, node, 0, true, isAnimated, true); + handleNode(node, 0, autogenerated, isAnimated, autogenerated); if (mCompoundShape) { @@ -163,34 +160,25 @@ bool BulletNifLoader::findBoundingBox(const Nif::Node* node, int flags) return false; } -const Nif::Node* BulletNifLoader::getCollisionNode(const Nif::Node* rootNode) +bool BulletNifLoader::hasAutoGeneratedCollision(const Nif::Node* rootNode) { const Nif::NiNode *ninode = dynamic_cast(rootNode); if(ninode) { - // If root NiNode has only other NiNode as child, consider it as a wrapper, not as actual root node const Nif::NodeList &list = ninode->children; - if (list.length() == 1 && - rootNode->recType == Nif::RC_NiNode && - list[0].getPtr()->recType == Nif::RC_NiNode) + for(size_t i = 0;i < list.length();i++) { - return getCollisionNode(list[0].getPtr()); - } - - for(size_t i = 0; i < list.length(); i++) - { - if(list[i].empty()) - continue; - - const Nif::Node* childNode = list[i].getPtr(); - if(childNode->recType == Nif::RC_RootCollisionNode) - return childNode; + if(!list[i].empty()) + { + if(list[i].getPtr()->recType == Nif::RC_RootCollisionNode) + return false; + } } } - return nullptr; + return true; } -void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node *node, int flags, +void BulletNifLoader::handleNode(const Nif::Node *node, int flags, bool isCollisionNode, bool isAnimated, bool autogenerated) { // Accumulate the flags from all the child nodes. This works for all @@ -203,9 +191,6 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node *n isCollisionNode = isCollisionNode || (node->recType == Nif::RC_RootCollisionNode); - if (node->recType == Nif::RC_RootCollisionNode && autogenerated) - Log(Debug::Info) << "Found RootCollisionNode attached to non-root node in " << fileName << ". Treat it as a common NiTriShape."; - // Don't collide with AvoidNode shapes if(node->recType == Nif::RC_AvoidNode) flags |= 0x800; @@ -234,6 +219,7 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node *n // Marker can still have collision if the model explicitely specifies it via a RootCollisionNode. return; } + } } @@ -256,7 +242,7 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node *n for(size_t i = 0;i < list.length();i++) { if(!list[i].empty()) - handleNode(fileName, list[i].getPtr(), flags, isCollisionNode, isAnimated, autogenerated); + handleNode(list[i].getPtr(), flags, isCollisionNode, isAnimated, autogenerated); } } } diff --git a/components/nifbullet/bulletnifloader.hpp b/components/nifbullet/bulletnifloader.hpp index f970b7f3e5..cc9aaa844b 100644 --- a/components/nifbullet/bulletnifloader.hpp +++ b/components/nifbullet/bulletnifloader.hpp @@ -55,9 +55,9 @@ public: private: bool findBoundingBox(const Nif::Node* node, int flags = 0); - void handleNode(const std::string& fileName, Nif::Node const *node, int flags, bool isCollisionNode, bool isAnimated=false, bool autogenerated=false); + void handleNode(Nif::Node const *node, int flags, bool isCollisionNode, bool isAnimated=false, bool autogenerated=false); - const Nif::Node* getCollisionNode(const Nif::Node* rootNode); + bool hasAutoGeneratedCollision(const Nif::Node *rootNode); void handleNiTriShape(const Nif::NiTriShape *shape, int flags, const osg::Matrixf& transform, bool isAnimated); From 61da6b6ecfbeb08a405e2ebd620c3bcbc77ea37d Mon Sep 17 00:00:00 2001 From: Andrei Kortunov Date: Fri, 26 Oct 2018 13:24:05 +0400 Subject: [PATCH 2/3] Print warning if the RootCollisionNode is attached to non-root node (bug #4311) --- CHANGELOG.md | 1 + components/nifbullet/bulletnifloader.cpp | 10 +++++++--- components/nifbullet/bulletnifloader.hpp | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f668010d5..07c0217da6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Bug #4293: Faction members are not aware of faction ownerships in barter Bug #4304: "Follow" not working as a second AI package Bug #4307: World cleanup should remove dead bodies only if death animation is finished + Bug #4311: OpenMW does not handle RootCollisionNode correctly Bug #4327: Missing animations during spell/weapon stance switching Bug #4333: Keyboard navigation in containers is not intuitive Bug #4358: Running animation is interrupted when magic mode is toggled diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index c18b96d395..20307f2526 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -103,7 +103,7 @@ osg::ref_ptr BulletNifLoader::load(const Nif::File& nif) const auto filename = nif.getFilename(); const bool isAnimated = pathFileNameStartsWithX(filename); - handleNode(node, 0, autogenerated, isAnimated, autogenerated); + handleNode(filename, node, 0, autogenerated, isAnimated, autogenerated); if (mCompoundShape) { @@ -178,7 +178,7 @@ bool BulletNifLoader::hasAutoGeneratedCollision(const Nif::Node* rootNode) return true; } -void BulletNifLoader::handleNode(const Nif::Node *node, int flags, +void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node *node, int flags, bool isCollisionNode, bool isAnimated, bool autogenerated) { // Accumulate the flags from all the child nodes. This works for all @@ -195,6 +195,10 @@ void BulletNifLoader::handleNode(const Nif::Node *node, int flags, if(node->recType == Nif::RC_AvoidNode) flags |= 0x800; + // We encountered a RootCollisionNode inside autogenerated mesh. It is not right. + if (node->recType == Nif::RC_RootCollisionNode && autogenerated) + Log(Debug::Info) << "Found RootCollisionNode attached to non-root node in " << fileName << ". Treat it as a common NiTriShape."; + // Check for extra data Nif::Extra const *e = node; while (!e->extra.empty()) @@ -242,7 +246,7 @@ void BulletNifLoader::handleNode(const Nif::Node *node, int flags, for(size_t i = 0;i < list.length();i++) { if(!list[i].empty()) - handleNode(list[i].getPtr(), flags, isCollisionNode, isAnimated, autogenerated); + handleNode(fileName, list[i].getPtr(), flags, isCollisionNode, isAnimated, autogenerated); } } } diff --git a/components/nifbullet/bulletnifloader.hpp b/components/nifbullet/bulletnifloader.hpp index cc9aaa844b..f67ab402ee 100644 --- a/components/nifbullet/bulletnifloader.hpp +++ b/components/nifbullet/bulletnifloader.hpp @@ -55,7 +55,7 @@ public: private: bool findBoundingBox(const Nif::Node* node, int flags = 0); - void handleNode(Nif::Node const *node, int flags, bool isCollisionNode, bool isAnimated=false, bool autogenerated=false); + void handleNode(const std::string& fileName, Nif::Node const *node, int flags, bool isCollisionNode, bool isAnimated=false, bool autogenerated=false); bool hasAutoGeneratedCollision(const Nif::Node *rootNode); From d2f3196ee861907b5b7048792726a64bf045fb9f Mon Sep 17 00:00:00 2001 From: Andrei Kortunov Date: Fri, 26 Oct 2018 21:21:34 +0400 Subject: [PATCH 3/3] Fix testcase for RootCollisionNode with MRK data --- apps/openmw_test_suite/nifloader/testbulletnifloader.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index a2311be490..2361718b19 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp @@ -929,10 +929,8 @@ namespace mNiStringExtraData.string = "MRK"; mNiStringExtraData.recType = Nif::RC_NiStringExtraData; mNiTriShape.extra = Nif::ExtraPtr(&mNiStringExtraData); - mNiNode3.children = Nif::NodeList(std::vector({Nif::NodePtr(&mNiTriShape)})); - mNiNode3.recType = Nif::RC_RootCollisionNode; - mNiNode2.children = Nif::NodeList(std::vector({Nif::NodePtr(nullptr), Nif::NodePtr(&mNiNode3)})); - mNiNode2.recType = Nif::RC_NiNode; + mNiNode2.children = Nif::NodeList(std::vector({Nif::NodePtr(&mNiTriShape)})); + mNiNode2.recType = Nif::RC_RootCollisionNode; mNiNode.children = Nif::NodeList(std::vector({Nif::NodePtr(&mNiNode2)})); mNiNode.recType = Nif::RC_NiNode;