Cleanup physics callbacks

* Do not copy with allocations.
* Remove unused DeepestNotMeContactTestResultCallback.
* Avoid using pointers which should not be nullptr.
* Move constructors implementation to headers.
* Move types defined in .cpp are to unnamed namespace.
* Comment unused arguments.
* Avoid C-style casts.

apps/openmw/CMakeLists.txt

@@ -84,7 +84,7 @@ add_openmw_dir (mwworld
 
 add_openmw_dir (mwphysics
     physicssystem trace collisiontype actor convert object heightfield closestnotmerayresultcallback
-    contacttestresultcallback deepestnotmecontacttestresultcallback stepper movementsolver projectile
+    contacttestresultcallback stepper movementsolver projectile
     actorconvexcallback raycasting mtphysics contacttestwrapper projectileconvexcallback
     )
 

apps/openmw/mwphysics/actorconvexcallback.cpp

@@ -9,35 +9,28 @@
 
 namespace MWPhysics
 {
-    class ActorOverlapTester : public btCollisionWorld::ContactResultCallback
+    namespace
     {
-    public:
+        struct ActorOverlapTester : public btCollisionWorld::ContactResultCallback
-        bool overlapping = false;
-
-        btScalar addSingleResult(btManifoldPoint& cp, const btCollisionObjectWrapper* colObj0Wrap, int partId0,
-            int index0, const btCollisionObjectWrapper* colObj1Wrap, int partId1, int index1) override
         {
-            if (cp.getDistance() <= 0.0f)
+            bool mOverlapping = false;
-                overlapping = true;
-            return btScalar(1);
-        }
-    };
 
-    ActorConvexCallback::ActorConvexCallback(
+            btScalar addSingleResult(btManifoldPoint& cp, const btCollisionObjectWrapper* /*colObj0Wrap*/,
-        const btCollisionObject* me, const btVector3& motion, btScalar minCollisionDot, const btCollisionWorld* world)
+                int /*partId0*/, int /*index0*/, const btCollisionObjectWrapper* /*colObj1Wrap*/, int /*partId1*/,
-        : btCollisionWorld::ClosestConvexResultCallback(btVector3(0.0, 0.0, 0.0), btVector3(0.0, 0.0, 0.0))
+                int /*index1*/) override
-        , mMe(me)
+            {
-        , mMotion(motion)
+                if (cp.getDistance() <= 0.0f)
-        , mMinCollisionDot(minCollisionDot)
+                    mOverlapping = true;
-        , mWorld(world)
+                return 1;
-    {
+            }
+        };
     }
 
     btScalar ActorConvexCallback::addSingleResult(
         btCollisionWorld::LocalConvexResult& convexResult, bool normalInWorldSpace)
     {
         if (convexResult.m_hitCollisionObject == mMe)
-            return btScalar(1);
+            return 1;
 
         // override data for actor-actor collisions
         // vanilla Morrowind seems to make overlapping actors collide as though they are both cylinders with a diameter
@@ -52,7 +45,7 @@ namespace MWPhysics
                 const_cast<btCollisionObject*>(mMe), const_cast<btCollisionObject*>(convexResult.m_hitCollisionObject),
                 isOverlapping);
 
-            if (isOverlapping.overlapping)
+            if (isOverlapping.mOverlapping)
             {
                 auto originA = Misc::Convert::toOsg(mMe->getWorldTransform().getOrigin());
                 auto originB = Misc::Convert::toOsg(convexResult.m_hitCollisionObject->getWorldTransform().getOrigin());
@@ -73,7 +66,7 @@ namespace MWPhysics
                 }
                 else
                 {
-                    return btScalar(1);
+                    return 1;
                 }
             }
         }
@@ -82,10 +75,10 @@ namespace MWPhysics
         {
             auto* projectileHolder = static_cast<Projectile*>(convexResult.m_hitCollisionObject->getUserPointer());
             if (!projectileHolder->isActive())
-                return btScalar(1);
+                return 1;
             if (projectileHolder->isValidTarget(mMe))
                 projectileHolder->hit(mMe, convexResult.m_hitPointLocal, convexResult.m_hitNormalLocal);
-            return btScalar(1);
+            return 1;
         }
 
         btVector3 hitNormalWorld;
@@ -101,7 +94,7 @@ namespace MWPhysics
         // dot product of the motion vector against the collision contact normal
         btScalar dotCollision = mMotion.dot(hitNormalWorld);
         if (dotCollision <= mMinCollisionDot)
-            return btScalar(1);
+            return 1;
 
         return ClosestConvexResultCallback::addSingleResult(convexResult, normalInWorldSpace);
     }

apps/openmw/mwphysics/actorconvexcallback.hpp

@@ -10,8 +10,15 @@ namespace MWPhysics
     class ActorConvexCallback : public btCollisionWorld::ClosestConvexResultCallback
     {
     public:
-        ActorConvexCallback(const btCollisionObject* me, const btVector3& motion, btScalar minCollisionDot,
+        explicit ActorConvexCallback(const btCollisionObject* me, const btVector3& motion, btScalar minCollisionDot,
-            const btCollisionWorld* world);
+            const btCollisionWorld* world)
+            : btCollisionWorld::ClosestConvexResultCallback(btVector3(0.0, 0.0, 0.0), btVector3(0.0, 0.0, 0.0))
+            , mMe(me)
+            , mMotion(motion)
+            , mMinCollisionDot(minCollisionDot)
+            , mWorld(world)
+        {
+        }
 
         btScalar addSingleResult(btCollisionWorld::LocalConvexResult& convexResult, bool normalInWorldSpace) override;
 

apps/openmw/mwphysics/closestnotmerayresultcallback.cpp

@@ -1,7 +1,6 @@
 #include "closestnotmerayresultcallback.hpp"
 
 #include <algorithm>
-#include <utility>
 
 #include <BulletCollision/CollisionDispatch/btCollisionObject.h>
 
@@ -9,14 +8,6 @@
 
 namespace MWPhysics
 {
-    ClosestNotMeRayResultCallback::ClosestNotMeRayResultCallback(const btCollisionObject* me,
-        const std::vector<const btCollisionObject*>& targets, const btVector3& from, const btVector3& to)
-        : btCollisionWorld::ClosestRayResultCallback(from, to)
-        , mMe(me)
-        , mTargets(targets)
-    {
-    }
-
     btScalar ClosestNotMeRayResultCallback::addSingleResult(
         btCollisionWorld::LocalRayResult& rayResult, bool normalInWorldSpace)
     {

apps/openmw/mwphysics/closestnotmerayresultcallback.hpp

@@ -1,7 +1,7 @@
 #ifndef OPENMW_MWPHYSICS_CLOSESTNOTMERAYRESULTCALLBACK_H
 #define OPENMW_MWPHYSICS_CLOSESTNOTMERAYRESULTCALLBACK_H
 
-#include <vector>
+#include <span>
 
 #include <BulletCollision/CollisionDispatch/btCollisionWorld.h>
 
@@ -14,14 +14,19 @@ namespace MWPhysics
     class ClosestNotMeRayResultCallback : public btCollisionWorld::ClosestRayResultCallback
     {
     public:
-        ClosestNotMeRayResultCallback(const btCollisionObject* me, const std::vector<const btCollisionObject*>& targets,
+        explicit ClosestNotMeRayResultCallback(const btCollisionObject* me, std::span<const btCollisionObject*> targets,
-            const btVector3& from, const btVector3& to);
+            const btVector3& from, const btVector3& to)
+            : btCollisionWorld::ClosestRayResultCallback(from, to)
+            , mMe(me)
+            , mTargets(targets)
+        {
+        }
 
         btScalar addSingleResult(btCollisionWorld::LocalRayResult& rayResult, bool normalInWorldSpace) override;
 
     private:
         const btCollisionObject* mMe;
-        const std::vector<const btCollisionObject*> mTargets;
+        const std::span<const btCollisionObject*> mTargets;
     };
 }
 

apps/openmw/mwphysics/contacttestresultcallback.cpp

@@ -8,13 +8,8 @@
 
 namespace MWPhysics
 {
-    ContactTestResultCallback::ContactTestResultCallback(const btCollisionObject* testedAgainst)
-        : mTestedAgainst(testedAgainst)
-    {
-    }
-
     btScalar ContactTestResultCallback::addSingleResult(btManifoldPoint& cp, const btCollisionObjectWrapper* col0Wrap,
-        int partId0, int index0, const btCollisionObjectWrapper* col1Wrap, int partId1, int index1)
+        int /*partId0*/, int /*index0*/, const btCollisionObjectWrapper* col1Wrap, int /*partId1*/, int /*index1*/)
     {
         const btCollisionObject* collisionObject = col0Wrap->m_collisionObject;
         if (collisionObject == mTestedAgainst)

apps/openmw/mwphysics/contacttestresultcallback.hpp

@@ -14,15 +14,19 @@ namespace MWPhysics
 {
     class ContactTestResultCallback : public btCollisionWorld::ContactResultCallback
     {
-        const btCollisionObject* mTestedAgainst;
-
     public:
-        ContactTestResultCallback(const btCollisionObject* testedAgainst);
+        explicit ContactTestResultCallback(const btCollisionObject* testedAgainst)
+            : mTestedAgainst(testedAgainst)
+        {
+        }
 
         btScalar addSingleResult(btManifoldPoint& cp, const btCollisionObjectWrapper* col0Wrap, int partId0, int index0,
             const btCollisionObjectWrapper* col1Wrap, int partId1, int index1) override;
 
         std::vector<ContactPoint> mResult;
+
+    private:
+        const btCollisionObject* mTestedAgainst;
     };
 }
 

apps/openmw/mwphysics/deepestnotmecontacttestresultcallback.cpp

@@ -1,47 +0,0 @@
-#include "deepestnotmecontacttestresultcallback.hpp"
-
-#include <algorithm>
-
-#include <BulletCollision/CollisionDispatch/btCollisionObject.h>
-
-#include "collisiontype.hpp"
-
-namespace MWPhysics
-{
-
-    DeepestNotMeContactTestResultCallback::DeepestNotMeContactTestResultCallback(
-        const btCollisionObject* me, const std::vector<const btCollisionObject*>& targets, const btVector3& origin)
-        : mMe(me)
-        , mTargets(targets)
-        , mOrigin(origin)
-        , mLeastDistSqr(std::numeric_limits<float>::max())
-    {
-    }
-
-    btScalar DeepestNotMeContactTestResultCallback::addSingleResult(btManifoldPoint& cp,
-        const btCollisionObjectWrapper* col0Wrap, int partId0, int index0, const btCollisionObjectWrapper* col1Wrap,
-        int partId1, int index1)
-    {
-        const btCollisionObject* collisionObject = col1Wrap->m_collisionObject;
-        if (collisionObject != mMe)
-        {
-            if (collisionObject->getBroadphaseHandle()->m_collisionFilterGroup == CollisionType_Actor
-                && !mTargets.empty())
-            {
-                if ((std::find(mTargets.begin(), mTargets.end(), collisionObject) == mTargets.end()))
-                    return 0.f;
-            }
-
-            btScalar distsqr = mOrigin.distance2(cp.getPositionWorldOnA());
-            if (!mObject || distsqr < mLeastDistSqr)
-            {
-                mObject = collisionObject;
-                mLeastDistSqr = distsqr;
-                mContactPoint = cp.getPositionWorldOnA();
-                mContactNormal = cp.m_normalWorldOnB;
-            }
-        }
-
-        return 0.f;
-    }
-}

apps/openmw/mwphysics/deepestnotmecontacttestresultcallback.hpp

@@ -1,34 +0,0 @@
-#ifndef OPENMW_MWPHYSICS_DEEPESTNOTMECONTACTTESTRESULTCALLBACK_H
-#define OPENMW_MWPHYSICS_DEEPESTNOTMECONTACTTESTRESULTCALLBACK_H
-
-#include <vector>
-
-#include <BulletCollision/CollisionDispatch/btCollisionWorld.h>
-
-class btCollisionObject;
-
-namespace MWPhysics
-{
-    class DeepestNotMeContactTestResultCallback : public btCollisionWorld::ContactResultCallback
-    {
-        const btCollisionObject* mMe;
-        const std::vector<const btCollisionObject*> mTargets;
-
-        // Store the real origin, since the shape's origin is its center
-        btVector3 mOrigin;
-
-    public:
-        const btCollisionObject* mObject{ nullptr };
-        btVector3 mContactPoint{ 0, 0, 0 };
-        btVector3 mContactNormal{ 0, 0, 0 };
-        btScalar mLeastDistSqr;
-
-        DeepestNotMeContactTestResultCallback(
-            const btCollisionObject* me, const std::vector<const btCollisionObject*>& targets, const btVector3& origin);
-
-        btScalar addSingleResult(btManifoldPoint& cp, const btCollisionObjectWrapper* col0Wrap, int partId0, int index0,
-            const btCollisionObjectWrapper* col1Wrap, int partId1, int index1) override;
-    };
-}
-
-#endif

apps/openmw/mwphysics/movementsolver.cpp

@@ -32,53 +32,58 @@ namespace MWPhysics
         return obj->getBroadphaseHandle()->m_collisionFilterGroup == CollisionType_Actor;
     }
 
-    class ContactCollectionCallback : public btCollisionWorld::ContactResultCallback
+    namespace
     {
-    public:
+        class ContactCollectionCallback : public btCollisionWorld::ContactResultCallback
-        ContactCollectionCallback(const btCollisionObject* me, osg::Vec3f velocity)
-            : mMe(me)
         {
-            m_collisionFilterGroup = me->getBroadphaseHandle()->m_collisionFilterGroup;
+        public:
-            m_collisionFilterMask = me->getBroadphaseHandle()->m_collisionFilterMask & ~CollisionType_Projectile;
+            explicit ContactCollectionCallback(const btCollisionObject& me, const osg::Vec3f& velocity)
-            mVelocity = Misc::Convert::toBullet(velocity);
+                : mVelocity(Misc::Convert::toBullet(velocity))
-        }
-        btScalar addSingleResult(btManifoldPoint& contact, const btCollisionObjectWrapper* colObj0Wrap, int partId0,
-            int index0, const btCollisionObjectWrapper* colObj1Wrap, int partId1, int index1) override
-        {
-            if (isActor(colObj0Wrap->getCollisionObject()) && isActor(colObj1Wrap->getCollisionObject()))
-                return 0.0;
-            // ignore overlap if we're moving in the same direction as it would push us out (don't change this to >=,
-            // that would break detection when not moving)
-            if (contact.m_normalWorldOnB.dot(mVelocity) > 0.0)
-                return 0.0;
-            auto delta = contact.m_normalWorldOnB * -contact.m_distance1;
-            mContactSum += delta;
-            mMaxX = std::max(std::abs(delta.x()), mMaxX);
-            mMaxY = std::max(std::abs(delta.y()), mMaxY);
-            mMaxZ = std::max(std::abs(delta.z()), mMaxZ);
-            if (contact.m_distance1 < mDistance)
             {
-                mDistance = contact.m_distance1;
+                m_collisionFilterGroup = me.getBroadphaseHandle()->m_collisionFilterGroup;
-                mNormal = contact.m_normalWorldOnB;
+                m_collisionFilterMask = me.getBroadphaseHandle()->m_collisionFilterMask & ~CollisionType_Projectile;
-                mDelta = delta;
-                return mDistance;
             }
-            else
+
+            btScalar addSingleResult(btManifoldPoint& contact, const btCollisionObjectWrapper* colObj0Wrap,
+                int /*partId0*/, int /*index0*/, const btCollisionObjectWrapper* colObj1Wrap, int /*partId1*/,
+                int /*index1*/) override
             {
-                return 0.0;
+                if (isActor(colObj0Wrap->getCollisionObject()) && isActor(colObj1Wrap->getCollisionObject()))
+                    return 0.0;
+                // ignore overlap if we're moving in the same direction as it would push us out (don't change this to
+                // >=, that would break detection when not moving)
+                if (contact.m_normalWorldOnB.dot(mVelocity) > 0.0)
+                    return 0.0;
+                auto delta = contact.m_normalWorldOnB * -contact.m_distance1;
+                mContactSum += delta;
+                mMaxX = std::max(std::abs(delta.x()), mMaxX);
+                mMaxY = std::max(std::abs(delta.y()), mMaxY);
+                mMaxZ = std::max(std::abs(delta.z()), mMaxZ);
+                if (contact.m_distance1 < mDistance)
+                {
+                    mDistance = contact.m_distance1;
+                    mNormal = contact.m_normalWorldOnB;
+                    mDelta = delta;
+                    return mDistance;
+                }
+                else
+                {
+                    return 0.0;
+                }
             }
-        }
+
-        btScalar mMaxX = 0.0;
+            btScalar mMaxX = 0.0;
-        btScalar mMaxY = 0.0;
+            btScalar mMaxY = 0.0;
-        btScalar mMaxZ = 0.0;
+            btScalar mMaxZ = 0.0;
-        btVector3 mContactSum{ 0.0, 0.0, 0.0 };
+            btVector3 mContactSum{ 0.0, 0.0, 0.0 };
-        btVector3 mNormal{ 0.0, 0.0, 0.0 }; // points towards "me"
+            btVector3 mNormal{ 0.0, 0.0, 0.0 }; // points towards "me"
-        btVector3 mDelta{ 0.0, 0.0, 0.0 }; // points towards "me"
+            btVector3 mDelta{ 0.0, 0.0, 0.0 }; // points towards "me"
-        btScalar mDistance = 0.0; // negative or zero
+            btScalar mDistance = 0.0; // negative or zero
-    protected:
+
-        btVector3 mVelocity;
+        protected:
-        const btCollisionObject* mMe;
+            btVector3 mVelocity;
-    };
+        };
+    }
 
     osg::Vec3f MovementSolver::traceDown(const MWWorld::Ptr& ptr, const osg::Vec3f& position, Actor* actor,
         btCollisionWorld* collisionWorld, float maxHeight)
@@ -454,8 +459,10 @@ namespace MWPhysics
         if (btFrom == btTo)
             return;
 
+        assert(projectile.mProjectile != nullptr);
+
         ProjectileConvexCallback resultCallback(
-            projectile.mCaster, projectile.mCollisionObject, btFrom, btTo, projectile.mProjectile);
+            projectile.mCaster, projectile.mCollisionObject, btFrom, btTo, *projectile.mProjectile);
         resultCallback.m_collisionFilterMask = CollisionType_AnyPhysical;
         resultCallback.m_collisionFilterGroup = CollisionType_Projectile;
 
@@ -524,7 +531,7 @@ namespace MWPhysics
             newTransform.setOrigin(Misc::Convert::toBullet(goodPosition));
             actor.mCollisionObject->setWorldTransform(newTransform);
 
-            ContactCollectionCallback callback{ actor.mCollisionObject, velocity };
+            ContactCollectionCallback callback(*actor.mCollisionObject, velocity);
             ContactTestWrapper::contactTest(
                 const_cast<btCollisionWorld*>(collisionWorld), actor.mCollisionObject, callback);
             return callback;

apps/openmw/mwphysics/physicssystem.cpp

@@ -49,7 +49,6 @@
 
 #include "closestnotmerayresultcallback.hpp"
 #include "contacttestresultcallback.hpp"
-#include "deepestnotmecontacttestresultcallback.hpp"
 #include "hasspherecollisioncallback.hpp"
 #include "heightfield.hpp"
 #include "movementsolver.hpp"

apps/openmw/mwphysics/projectileconvexcallback.cpp

@@ -6,16 +6,6 @@
 
 namespace MWPhysics
 {
-    ProjectileConvexCallback::ProjectileConvexCallback(const btCollisionObject* caster, const btCollisionObject* me,
-        const btVector3& from, const btVector3& to, Projectile* proj)
-        : btCollisionWorld::ClosestConvexResultCallback(from, to)
-        , mCaster(caster)
-        , mMe(me)
-        , mProjectile(proj)
-    {
-        assert(mProjectile);
-    }
-
     btScalar ProjectileConvexCallback::addSingleResult(
         btCollisionWorld::LocalConvexResult& result, bool normalInWorldSpace)
     {
@@ -33,25 +23,25 @@ namespace MWPhysics
         {
             case CollisionType_Actor:
             {
-                if (!mProjectile->isValidTarget(hitObject))
+                if (!mProjectile.isValidTarget(hitObject))
                     return 1.f;
                 break;
             }
             case CollisionType_Projectile:
             {
                 auto* target = static_cast<Projectile*>(hitObject->getUserPointer());
-                if (!mProjectile->isValidTarget(target->getCasterCollisionObject()))
+                if (!mProjectile.isValidTarget(target->getCasterCollisionObject()))
                     return 1.f;
                 target->hit(mMe, m_hitPointWorld, m_hitNormalWorld);
                 break;
             }
             case CollisionType_Water:
             {
-                mProjectile->setHitWater();
+                mProjectile.setHitWater();
                 break;
             }
         }
-        mProjectile->hit(hitObject, m_hitPointWorld, m_hitNormalWorld);
+        mProjectile.hit(hitObject, m_hitPointWorld, m_hitNormalWorld);
 
         return result.m_hitFraction;
     }

apps/openmw/mwphysics/projectileconvexcallback.hpp

@@ -12,15 +12,21 @@ namespace MWPhysics
     class ProjectileConvexCallback : public btCollisionWorld::ClosestConvexResultCallback
     {
     public:
-        ProjectileConvexCallback(const btCollisionObject* caster, const btCollisionObject* me, const btVector3& from,
+        explicit ProjectileConvexCallback(const btCollisionObject* caster, const btCollisionObject* me,
-            const btVector3& to, Projectile* proj);
+            const btVector3& from, const btVector3& to, Projectile& projectile)
+            : btCollisionWorld::ClosestConvexResultCallback(from, to)
+            , mCaster(caster)
+            , mMe(me)
+            , mProjectile(projectile)
+        {
+        }
 
         btScalar addSingleResult(btCollisionWorld::LocalConvexResult& result, bool normalInWorldSpace) override;
 
     private:
         const btCollisionObject* mCaster;
         const btCollisionObject* mMe;
-        Projectile* mProjectile;
+        Projectile& mProjectile;
     };
 }