Do not use collision shapes with visual only collision to generate navmesh

These collision shapes are not used for actors movement physics simulation.

apps/navmeshtool/worldspacedata.cpp

@@ -305,6 +305,9 @@ namespace NavMeshTool
             forEachObject(cell, esmData, vfs, bulletShapeManager, readers,
                 [&] (BulletObject object)
                 {
+                    if (object.getShapeInstance()->mVisualCollisionType != Resource::VisualCollisionType::None)
+                        return;
+
                     const btTransform& transform = object.getCollisionObject().getWorldTransform();
                     const btAABB aabb = BulletHelpers::getAabb(*object.getCollisionObject().getCollisionShape(), transform);
                     mergeOrAssign(aabb, navMeshInput.mAabb, navMeshInput.mAabbInitialized);

apps/openmw/mwphysics/actor.hpp

@@ -19,7 +19,7 @@ class btConvexShape;
 
 namespace Resource
 {
-    class BulletShape;
+    struct BulletShape;
 }
 
 namespace MWPhysics

apps/openmw/mwphysics/physicssystem.cpp

@@ -489,14 +489,16 @@ namespace MWPhysics
         assert(!getObject(ptr));
 
         // Override collision type based on shape content.
-        switch (shapeInstance->mCollisionType)
+        switch (shapeInstance->mVisualCollisionType)
         {
-            case Resource::BulletShape::CollisionType::Camera:
+            case Resource::VisualCollisionType::None:
-                collisionType = CollisionType_CameraOnly;
                 break;
-            case Resource::BulletShape::CollisionType::None:
+            case Resource::VisualCollisionType::Default:
                 collisionType = CollisionType_VisualOnly;
                 break;
+            case Resource::VisualCollisionType::Camera:
+                collisionType = CollisionType_CameraOnly;
+                break;
         }
 
         auto obj = std::make_shared<Object>(ptr, shapeInstance, rotation, collisionType, mTaskScheduler.get());

apps/openmw/mwphysics/projectile.hpp

@@ -20,7 +20,7 @@ namespace osg
 
 namespace Resource
 {
-    class BulletShape;
+    struct BulletShape;
 }
 
 namespace MWPhysics

apps/openmw/mwworld/scene.cpp

@@ -176,7 +176,7 @@ namespace
                     transform
                 );
             }
-            else
+            else if (object->getShapeInstance()->mVisualCollisionType == Resource::VisualCollisionType::None)
             {
                 navigator.addObject(
                     DetourNavigator::ObjectId(object),
@@ -335,7 +335,8 @@ namespace MWWorld
         {
             if (const auto object = mPhysics->getObject(ptr))
             {
-                mNavigator.removeObject(DetourNavigator::ObjectId(object));
+                if (object->getShapeInstance()->mVisualCollisionType == Resource::VisualCollisionType::None)
+                    mNavigator.removeObject(DetourNavigator::ObjectId(object));
                 mPhysics->remove(ptr);
                 ptr.mRef->mData.mPhysicsPostponed = false;
             }
@@ -932,7 +933,8 @@ namespace MWWorld
         MWBase::Environment::get().getLuaManager()->objectRemovedFromScene(ptr);
         if (const auto object = mPhysics->getObject(ptr))
         {
-            mNavigator.removeObject(DetourNavigator::ObjectId(object));
+            if (object->getShapeInstance()->mVisualCollisionType == Resource::VisualCollisionType::None)
+                mNavigator.removeObject(DetourNavigator::ObjectId(object));
         }
         else if (mPhysics->getActor(ptr))
         {

apps/openmw/mwworld/worldimp.cpp

@@ -1540,6 +1540,8 @@ namespace MWWorld
 
     void World::updateNavigatorObject(const MWPhysics::Object& object)
     {
+        if (object.getShapeInstance()->mVisualCollisionType != Resource::VisualCollisionType::None)
+            return;
         const MWWorld::Ptr ptr = object.getPtr();
         const DetourNavigator::ObjectShapes shapes(object.getShapeInstance(),
             DetourNavigator::ObjectTransform {ptr.getRefData().getPosition(), ptr.getCellRef().getScale()});

apps/openmw_test_suite/nifloader/testbulletnifloader.cpp

@@ -12,6 +12,7 @@
 #include <gmock/gmock.h>
 
 #include <algorithm>
+#include <type_traits>
 
 namespace
 {
@@ -137,13 +138,13 @@ static std::ostream& operator <<(std::ostream& stream, const TriangleMeshShape&
     return stream << "}}";
 }
 
-static bool operator ==(const BulletShape::CollisionBox& l, const BulletShape::CollisionBox& r)
+static bool operator ==(const CollisionBox& l, const CollisionBox& r)
 {
-    const auto tie = [] (const BulletShape::CollisionBox& v) { return std::tie(v.mExtents, v.mCenter); };
+    const auto tie = [] (const CollisionBox& v) { return std::tie(v.mExtents, v.mCenter); };
     return tie(l) == tie(r);
 }
 
-static std::ostream& operator <<(std::ostream& stream, const BulletShape::CollisionBox& value)
+static std::ostream& operator <<(std::ostream& stream, const CollisionBox& value)
 {
     return stream << "CollisionBox {" << WriteVec3f {value.mExtents} << ", " << WriteVec3f {value.mCenter} << "}";
 }
@@ -189,18 +190,32 @@ namespace Resource
         return compareObjects(lhs.mCollisionShape.get(), rhs.mCollisionShape.get())
             && compareObjects(lhs.mAvoidCollisionShape.get(), rhs.mAvoidCollisionShape.get())
             && lhs.mCollisionBox == rhs.mCollisionBox
-            && lhs.mCollisionType == rhs.mCollisionType
+            && lhs.mVisualCollisionType == rhs.mVisualCollisionType
             && lhs.mAnimatedShapes == rhs.mAnimatedShapes;
     }
 
+    static std::ostream& operator <<(std::ostream& stream, Resource::VisualCollisionType value)
+    {
+        switch (value)
+        {
+            case Resource::VisualCollisionType::None:
+                return stream << "Resource::VisualCollisionType::None";
+            case Resource::VisualCollisionType::Default:
+                return stream << "Resource::VisualCollisionType::Default";
+            case Resource::VisualCollisionType::Camera:
+                return stream << "Resource::VisualCollisionType::Camera";
+        }
+        return stream << static_cast<std::underlying_type_t<Resource::VisualCollisionType>>(value);
+    }
+
     static std::ostream& operator <<(std::ostream& stream, const Resource::BulletShape& value)
     {
         return stream << "Resource::BulletShape {"
             << value.mCollisionShape.get() << ", "
             << value.mAvoidCollisionShape.get() << ", "
             << value.mCollisionBox << ", "
-            << value.mAnimatedShapes
+            << value.mAnimatedShapes<< ", "
-            << ", collisionType=" << value.mCollisionType
+            << value.mVisualCollisionType
             << "}";
     }
 }
@@ -1030,7 +1045,7 @@ namespace
         triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
         Resource::BulletShape expected;
         expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true));
-        expected.mCollisionType = Resource::BulletShape::CollisionType::Camera;
+        expected.mVisualCollisionType = Resource::VisualCollisionType::Camera;
 
         EXPECT_EQ(*result, expected);
     }
@@ -1053,7 +1068,7 @@ namespace
         triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
         Resource::BulletShape expected;
         expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true));
-        expected.mCollisionType = Resource::BulletShape::CollisionType::Camera;
+        expected.mVisualCollisionType = Resource::VisualCollisionType::Camera;
 
         EXPECT_EQ(*result, expected);
     }
@@ -1075,7 +1090,7 @@ namespace
         triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
         Resource::BulletShape expected;
         expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true));
-        expected.mCollisionType = Resource::BulletShape::CollisionType::None;
+        expected.mVisualCollisionType = Resource::VisualCollisionType::Default;
 
         EXPECT_EQ(*result, expected);
     }
@@ -1098,7 +1113,7 @@ namespace
         triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
         Resource::BulletShape expected;
         expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true));
-        expected.mCollisionType = Resource::BulletShape::CollisionType::None;
+        expected.mVisualCollisionType = Resource::VisualCollisionType::Default;
 
         EXPECT_EQ(*result, expected);
     }
@@ -1128,7 +1143,7 @@ namespace
         triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
         Resource::BulletShape expected;
         expected.mCollisionShape.reset(new Resource::TriangleMeshShape(triangles.release(), true));
-        expected.mCollisionType = Resource::BulletShape::CollisionType::Camera;
+        expected.mVisualCollisionType = Resource::VisualCollisionType::Camera;
 
         EXPECT_EQ(*result, expected);
     }

components/nifbullet/bulletnifloader.cpp

@@ -224,9 +224,10 @@ osg::ref_ptr<Resource::BulletShape> BulletNifLoader::load(const Nif::File& nif)
         bool hasCollisionNode = hasRootCollisionNode(*node);
         bool hasCollisionShape = hasCollisionNode && !collisionShapeIsEmpty(*node);
         if (hasCollisionNode && !hasCollisionShape)
-            mShape->mCollisionType = Resource::BulletShape::CollisionType::Camera;
+            mShape->mVisualCollisionType = Resource::VisualCollisionType::Camera;
         bool generateCollisionShape = !hasCollisionShape;
-        handleNode(filename, *node, nullptr, 0, generateCollisionShape, isAnimated, generateCollisionShape, false, mShape->mCollisionType);
+        handleNode(filename, *node, nullptr, 0, generateCollisionShape, isAnimated, generateCollisionShape,
+                   false, mShape->mVisualCollisionType);
     }
 
     if (mCompoundShape)
@@ -337,7 +338,8 @@ bool BulletNifLoader::collisionShapeIsEmpty(const Nif::Node& rootNode) const
 }
 
 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, unsigned int& collisionType)
+    int flags, bool isCollisionNode, bool isAnimated, bool autogenerated, bool avoid,
+    Resource::VisualCollisionType& visualCollisionType)
 {
     // TODO: allow on-the fly collision switching via toggling this flag
     if (node.recType == Nif::RC_NiCollisionSwitch && !node.collisionActive())
@@ -345,7 +347,8 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node& n
 
     // 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 && collisionType == Resource::BulletShape::CollisionType::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
@@ -378,10 +381,10 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node& n
                 // 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')
                     // Collide only with camera.
-                    collisionType = Resource::BulletShape::CollisionType::Camera;
+                    visualCollisionType = Resource::VisualCollisionType::Camera;
                 else
                     // No collision.
-                    collisionType = Resource::BulletShape::CollisionType::None;
+                    visualCollisionType = Resource::VisualCollisionType::Default;
             }
             else if (sd->string == "MRK" && autogenerated)
             {
@@ -416,7 +419,7 @@ void BulletNifLoader::handleNode(const std::string& fileName, const Nif::Node& n
                 continue;
 
             assert(std::find(list[i]->parents.begin(), list[i]->parents.end(), ninode) != list[i]->parents.end());
-            handleNode(fileName, list[i].get(), &currentParent, flags, isCollisionNode, isAnimated, autogenerated, avoid, collisionType);
+            handleNode(fileName, list[i].get(), &currentParent, flags, isCollisionNode, isAnimated, autogenerated, avoid, visualCollisionType);
         }
     }
 }

components/nifbullet/bulletnifloader.hpp

@@ -56,8 +56,9 @@ public:
 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,
+    void handleNode(const std::string& fileName, const Nif::Node& node, const Nif::Parent* parent, int flags,
-        bool isCollisionNode, bool isAnimated, bool autogenerated, bool avoid, unsigned int& cameraOnlyCollision);
+        bool isCollisionNode, bool isAnimated, bool autogenerated, bool avoid,
+        Resource::VisualCollisionType& visualCollisionType);
 
     bool hasRootCollisionNode(const Nif::Node& rootNode) const;
     bool collisionShapeIsEmpty(const Nif::Node& rootNode) const;

components/resource/bulletshape.cpp

@@ -96,7 +96,7 @@ BulletShapeInstance::BulletShapeInstance(osg::ref_ptr<const BulletShape> source)
 {
     mCollisionBox = mSource->mCollisionBox;
     mAnimatedShapes = mSource->mAnimatedShapes;
-    mCollisionType = mSource->mCollisionType;
+    mVisualCollisionType = mSource->mVisualCollisionType;
     mCollisionShape = duplicateCollisionShape(mSource->mCollisionShape.get());
     mAvoidCollisionShape = duplicateCollisionShape(mSource->mAvoidCollisionShape.get());
 }

components/resource/bulletshape.hpp

@@ -27,22 +27,24 @@ namespace Resource
 
     using CollisionShapePtr = std::unique_ptr<btCollisionShape, DeleteCollisionShape>;
 
-    class BulletShape : public osg::Object
+    struct CollisionBox
     {
-    public:
+        osg::Vec3f mExtents;
-        BulletShape() = default;
+        osg::Vec3f mCenter;
-        BulletShape(const BulletShape& copy, const osg::CopyOp& copyop);
+    };
 
-        META_Object(Resource, BulletShape)
+    enum class VisualCollisionType
+    {
+        None,
+        Default,
+        Camera
+    };
 
+    struct BulletShape : public osg::Object
+    {
         CollisionShapePtr mCollisionShape;
         CollisionShapePtr mAvoidCollisionShape;
 
-        struct CollisionBox
-        {
-            osg::Vec3f mExtents;
-            osg::Vec3f mCenter;
-        };
         // Used for actors and projectiles. mCollisionShape is used for actors only when we need to autogenerate collision box for creatures.
         // For now, use one file <-> one resource for simplicity.
         CollisionBox mCollisionBox;
@@ -56,16 +58,16 @@ namespace Resource
         std::string mFileName;
         std::string mFileHash;
 
+        VisualCollisionType mVisualCollisionType = VisualCollisionType::None;
+
+        BulletShape() = default;
+        BulletShape(const BulletShape& copy, const osg::CopyOp& copyop);
+
+        META_Object(Resource, BulletShape)
+
         void setLocalScaling(const btVector3& scale);
 
         bool isAnimated() const { return !mAnimatedShapes.empty(); }
-
-        unsigned int mCollisionType = 0;
-        enum CollisionType
-        {
-            None = 0x1,
-            Camera = 0x2
-        };
     };
 
 

components/resource/bulletshapemanager.hpp

@@ -14,7 +14,7 @@ namespace Resource
     class SceneManager;
     class NifFileManager;
 
-    class BulletShape;
+    struct BulletShape;
     class BulletShapeInstance;
 
     class MultiObjectCache;