Merge branch 'metermarker' into 'master'

Don't discard collision for the entire tree if BSXFlags marker flag is set

See merge request OpenMW/openmw!3216

apps/openmw_test_suite/nifloader/testbulletnifloader.cpp

@@ -1158,12 +1158,13 @@ namespace
         EXPECT_EQ(*result, expected);
     }
 
-    TEST_F(TestBulletNifLoader, bsx_editor_marker_flag_disables_collision)
+    TEST_F(TestBulletNifLoader, bsx_editor_marker_flag_disables_collision_for_markers)
     {
         mNiIntegerExtraData.data = 32; // BSX flag "editor marker"
         mNiIntegerExtraData.recType = Nif::RC_BSXFlags;
         mNiTriShape.extralist.push_back(Nif::ExtraPtr(&mNiIntegerExtraData));
         mNiTriShape.parents.push_back(&mNiNode);
+        mNiTriShape.name = "EditorMarker";
         mNiNode.children = Nif::NodeList(std::vector<Nif::NodePtr>({ Nif::NodePtr(&mNiTriShape) }));
 
         Nif::NIFFile file("test.nif");

components/nifbullet/bulletnifloader.cpp

@@ -182,8 +182,10 @@ namespace NifBullet
             if (hasCollisionNode && !hasCollisionShape)
                 mShape->mVisualCollisionType = Resource::VisualCollisionType::Camera;
             bool generateCollisionShape = !hasCollisionShape;
-            handleNode(filename, *node, nullptr, 0, generateCollisionShape, isAnimated, generateCollisionShape, false,
+            HandleNodeArgs args;
-                mShape->mVisualCollisionType);
+            args.mAutogenerated = args.mIsCollisionNode = generateCollisionShape;
+            args.mAnimated = isAnimated;
+            handleNode(filename, *node, nullptr, args, mShape->mVisualCollisionType);
         }
 
         if (mCompoundShape)
@@ -269,36 +271,37 @@ namespace NifBullet
     }
 
     void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node& node, const Nif::Parent* parent,
-        int flags, bool isCollisionNode, bool isAnimated, bool autogenerated, bool avoid,
+        HandleNodeArgs args, Resource::VisualCollisionType& visualCollisionType)
-        Resource::VisualCollisionType& visualCollisionType)
     {
         // TODO: allow on-the fly collision switching via toggling this flag
         if (node.recType == Nif::RC_NiCollisionSwitch && !node.collisionActive())
             return;
 
-        // If RootCollisionNode is empty we treat it as NCC flag and autogenerate collision shape as there was no
-        // RootCollisionNode. So ignoring it here if `autogenerated` is true and collisionType was set to `Camera`.
-        if (node.recType == Nif::RC_RootCollisionNode && autogenerated
-            && visualCollisionType == Resource::VisualCollisionType::Camera)
-            return;
-
-        // Accumulate the flags from all the child nodes. This works for all
-        // the flags we currently use, at least.
-        flags |= node.flags;
-
         if (!node.controller.empty() && node.controller->recType == Nif::RC_NiKeyframeController
             && node.controller->isActive())
-            isAnimated = true;
+            args.mAnimated = true;
 
-        isCollisionNode = isCollisionNode || (node.recType == Nif::RC_RootCollisionNode);
+        if (node.recType == Nif::RC_RootCollisionNode)
+        {
+            args.mIsCollisionNode = true;
+            if (args.mAutogenerated)
+            {
+                // Encountered a RootCollisionNode inside an autogenerated mesh.
+
+                // We treat empty RootCollisionNodes as NCC flag (set collisionType to `Camera`)
+                // and generate the camera collision shape based on rendered geometry.
+                if (visualCollisionType == Resource::VisualCollisionType::Camera)
+                    return;
+
+                // Otherwise we'll want to notify the user.
+                Log(Debug::Info) << "RootCollisionNode is not attached to the root node in " << fileName
+                                 << ". Treating it as a common NiTriShape.";
+            }
+        }
 
         // Don't collide with AvoidNode shapes
-        avoid = avoid || (node.recType == Nif::RC_AvoidNode);
+        if (node.recType == Nif::RC_AvoidNode)
-
+            args.mAvoid = true;
-        // We encountered a RootCollisionNode inside autogenerated mesh. It is not right.
-        if (node.recType == Nif::RC_RootCollisionNode && autogenerated)
-            Log(Debug::Info) << "RootCollisionNode is not attached to the root node in " << fileName
-                             << ". Treating it as a common NiTriShape.";
 
         // Check for extra data
         std::vector<Nif::ExtraPtr> extraCollection;
@@ -326,7 +329,7 @@ namespace NifBullet
                         // No collision.
                         visualCollisionType = Resource::VisualCollisionType::Default;
                 }
-                else if (sd->string == "MRK" && autogenerated)
+                else if (sd->string == "MRK" && args.mAutogenerated)
                 {
                     // Marker can still have collision if the model explicitely specifies it via a RootCollisionNode.
                     return;
@@ -336,11 +339,11 @@ namespace NifBullet
             {
                 auto bsxFlags = static_cast<const Nif::NiIntegerExtraData*>(e.getPtr());
                 if (bsxFlags->data & 32) // Editor marker flag
-                    return;
+                    args.mHasMarkers = true;
             }
         }
 
-        if (isCollisionNode)
+        if (args.mIsCollisionNode)
         {
             // NOTE: a trishape with hasBounds=true, but no BBoxCollision flag should NOT go through handleNiTriShape!
             // It must be ignored completely.
@@ -349,7 +352,7 @@ namespace NifBullet
                 && (node.recType == Nif::RC_NiTriShape || node.recType == Nif::RC_NiTriStrips
                     || node.recType == Nif::RC_BSLODTriShape))
             {
-                handleNiTriShape(static_cast<const Nif::NiGeometry&>(node), parent, isAnimated, avoid);
+                handleNiTriShape(static_cast<const Nif::NiGeometry&>(node), parent, args);
             }
         }
 
@@ -364,20 +367,24 @@ namespace NifBullet
                     continue;
 
                 assert(std::find(child->parents.begin(), child->parents.end(), ninode) != child->parents.end());
-                handleNode(fileName, child.get(), &currentParent, flags, isCollisionNode, isAnimated, autogenerated,
+                handleNode(fileName, child.get(), &currentParent, args, visualCollisionType);
-                    avoid, visualCollisionType);
             }
         }
     }
 
     void BulletNifLoader::handleNiTriShape(
-        const Nif::NiGeometry& niGeometry, const Nif::Parent* nodeParent, bool isAnimated, bool avoid)
+        const Nif::NiGeometry& niGeometry, const Nif::Parent* nodeParent, HandleNodeArgs args)
     {
+        // mHasMarkers is specifically BSXFlags editor marker flag.
+        // If this changes, the check must be corrected.
+        if (args.mHasMarkers && Misc::StringUtils::ciStartsWith(niGeometry.name, "EditorMarker"))
+            return;
+
         if (niGeometry.data.empty() || niGeometry.data->vertices.empty())
             return;
 
         if (!niGeometry.skin.empty())
-            isAnimated = false;
+            args.mAnimated = false;
 
         std::unique_ptr<btTriangleMesh> childMesh = makeChildMesh(niGeometry);
         if (childMesh == nullptr || childMesh->getNumTriangles() == 0)
@@ -398,12 +405,12 @@ namespace NifBullet
             for (int j = 0; j < 3; ++j)
                 trans.getBasis()[i][j] = transform(j, i);
 
-        if (!avoid)
+        if (!args.mAvoid)
         {
             if (!mCompoundShape)
                 mCompoundShape.reset(new btCompoundShape);
 
-            if (isAnimated)
+            if (args.mAnimated)
                 mShape->mAnimatedShapes.emplace(niGeometry.recIndex, mCompoundShape->getNumChildShapes());
             mCompoundShape->addChildShape(trans, childShape.get());
         }

components/nifbullet/bulletnifloader.hpp

@@ -52,14 +52,22 @@ namespace NifBullet
     private:
         bool findBoundingBox(const Nif::Node& node, const std::string& filename);
 
-        void handleNode(const std::string& fileName, const Nif::Node& node, const Nif::Parent* parent, int flags,
+        struct HandleNodeArgs
-            bool isCollisionNode, bool isAnimated, bool autogenerated, bool avoid,
+        {
-            Resource::VisualCollisionType& visualCollisionType);
+            bool mHasMarkers{ false };
+            bool mAnimated{ false };
+            bool mIsCollisionNode{ false };
+            bool mAutogenerated{ false };
+            bool mAvoid{ false };
+        };
+
+        void handleNode(const std::string& fileName, const Nif::Node& node, const Nif::Parent* parent,
+            HandleNodeArgs args, Resource::VisualCollisionType& visualCollisionType);
 
         bool hasRootCollisionNode(const Nif::Node& rootNode) const;
         bool collisionShapeIsEmpty(const Nif::Node& rootNode) const;
 
-        void handleNiTriShape(const Nif::NiGeometry& nifNode, const Nif::Parent* parent, bool isAnimated, bool avoid);
+        void handleNiTriShape(const Nif::NiGeometry& nifNode, const Nif::Parent* parent, HandleNodeArgs args);
 
         std::unique_ptr<btCompoundShape, Resource::DeleteCollisionShape> mCompoundShape;
         std::unique_ptr<btCompoundShape, Resource::DeleteCollisionShape> mAvoidCompoundShape;