Collision Shapes Abstraction Design

Question

Straight to the facts.

1. I have a base CEntity class for different entities in my game:

   • Static entities ( those do not move, simply props and items )
   • Dynamic entities ( these move, jump )

2. Each of these can have a different collision shape, the shapes supported are:

   • CCollisionShapeAABB ( AABB )
   • CCollisionShapeBSphere ( Bounding Sphere )
   • CCollisionShapeSoup ( Polygon soup, a list of vertices/indices )

3. CEntity the base class for all entities has a member object, that is initialized later on by inheriting classes depending on their collision shape. Below, CEntityPlayer initialisation, where Player uses AABB.

// Initialize proper Collision Shape for this Entity
m_pCollisionShape = new CCollisionShapeAABB(pNewNode->GetSizeMinimals(),
                                            pNewNode->GetSizeMaximals());

CEntity has a method that takes another entity, and performs intersection test. The intersection test depends on the type of Collision Shape entities have.

bool CEntity::Intersect(CEntity *pEntity)
{
    ASSERT(pEntity);

    CCollisionShape::eCollisionShapeClass _myClass  = m_pCollisionShape->GetCollisionClass();
    CCollisionShape::eCollisionShapeClass _hisClass = pEntity->GetCollision()->GetCollisionClass();


    if(_myClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_AABB)
    {
        CCollisionShapeAABB *pMyShape = static_cast<CCollisionShapeAABB*>(m_pCollisionShape);

        if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_AABB)
        {
            return pMyShape->Collides(*static_cast<CCollisionShapeAABB*>(pEntity->GetCollision()));
        }
        else if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_BOUNDINGSPHERE)
        {
            return pMyShape->Collides(*static_cast<CCollisionShapeBSphere*>(pEntity->GetCollision()));
        }
        else if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_POLYGONSOUP)
        {
            return pMyShape->Collides(*static_cast<CCollisionShapeSoup*>(pEntity->GetCollision()));
        }
    }


    return false;

}

I can't figure out today, how should i approach this to avoid a situation, where i need to check for each individual Collision Shape type, before doing any intersection test?

Thanks v.much for any input into this!

UPDATE

The CCollisionShape Class is a Base class for other CCollisionShape* classes, and it looks as follows.

class CCollisionShape { public:

    enum eCollisionShapeClass
    {
        COLLISION_SHAPE_CLASS_UNKNOWN = 0,
        COLLISION_SHAPE_CLASS_AABB,
        COLLISION_SHAPE_CLASS_BOUNDINGSPHERE,
        COLLISION_SHAPE_CLASS_POLYGONSOUP,
        COLLISION_SHAPE_CLASS_TOTAL
    };

    CCollisionShape();
    virtual ~CCollisionShape();

    const eCollisionShapeClass GetCollisionClass() const;


    bool        Collides            (CCollisionShape *pCollisionShape);


    virtual     bool Collides       (const class CCollisionShapeAABB& rCollisionShape) const = 0;
    virtual     bool Collides       (const class CCollisionShapeBSphere& rCollisionShape) const = 0;
    virtual     bool Collides       (const class CCollisionShapeSoup& rCollisionShape) const=0;
    virtual     bool Collides       (const  CVector3& vPosition) const = 0;
    virtual     bool Collides       (const  CFrustum& rFrustum) const = 0;
    virtual     bool Collides       (const  CRay& rRay) const = 0;
    virtual     int  Collides       (const  CPlane& rPlane) const = 0;
    virtual     bool Collides       (const  CTriangle& rTriangle) const = 0;

protected:
    eCollisionShapeClass m_eCollisionClass; };

Now the ::Collides() method looks as follows

bool CCollisionShape::Collides(CCollisionShape *pCollisionShape) { // ASSERT(pCollisionShape);

CCollisionShape::eCollisionShapeClass _hisClass = pCollisionShape->GetCollisionClass();

if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_AABB)
{
    return Collides(static_cast<CCollisionShapeAABB*>(pCollisionShape));
}
else if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_BOUNDINGSPHERE)
{
    return Collides(static_cast<CCollisionShapeBSphere*>(pCollisionShape));
}
else if(_hisClass == CCollisionShape::eCollisionShapeClass::COLLISION_SHAPE_CLASS_POLYGONSOUP)
{
    return Collides(static_cast<CCollisionShapeSoup*>(pCollisionShape));
}

return false; }

And collision testing from the Game goes with:

CEntity *pPlayer = GetEntity("Player"); CEntity *pSector = GetEntity("Sector");

CCollisionShape *pCA = pPlayer->GetCollision();
CCollisionShape *pCB = pSector->GetCollision();

if(pCA->Collides(pCB))
{
    exit(0);
}

Answer 1

First of all, CEntity should not have an Intersect method at all. This is simply not its functionality, in your case you are going towards a god CEntity class that knows about everything, when you should usually prefer minimal interface.

When designing a class I always strive for these two principles;

• Single Responsibility Principle
• Liskov Substitution principle.

If you ask how you should porform intersection at the entity level. Then the answer is simply you don't. The approriate way will become like this:

CollisionShape* shape = entity->GetCollisionShape(); //or use references.
shape->Intersect(other);

The intersect method will become in the CollisionShape (or even outside, but that's a different debatable subject) like this:

bool CCollisionShape::Intersect(CCollisionShape *otherShape)
//or as a free function
bool Intersect(CCollisionShape *shape, CCollisionShape *otherShape)

In your current approach you are losing all type safety for no good reason. And also exposing functionality to CEntity that it shouldn't know about.

My second point is what you are doing right now is actually simulate dynamic dispatch by checking for the type each time, this is usually a bad design, two simple reasons I can think off are:

• If you want to add new type you will always need to remember to add it to the type checking.
• That it's error prone, and your static case, while faster could cause hard to find bugs on release if you are not careful enough.

The cleaner way to do this is to actually use polymorphism for your collision shapes, override Intersect for each collision shape, and let the language run time figure out the type for you. This will eliminate the type checking if statement.

Now what are the gains you might take from your current dynamic dispatch simulation?

• You are not actually performing dynamic dispatch, hence no double indirection.
• Which also means your ccollision methods could be inlined.

But remeber that the above points are not valid unless you really profile your application and find that they cause a bottleneck. Otherwise always take the cleaner design.