3
\$\begingroup\$

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);
}
\$\endgroup\$
  • \$\begingroup\$ Is there any reason why you don't have subclasses of CEntity which override the CEntity::Intersect(CEntity*) method? \$\endgroup\$ – Polar Dec 12 '13 at 22:18
  • \$\begingroup\$ I do have subclasses of CEntity, these are CEntityPlayer, CEntitySector, CEntityItems,... but they don't override the Intersect method, how would that be more helpful here? Can you elaborate please? \$\endgroup\$ – cdmstr Dec 12 '13 at 22:32
  • \$\begingroup\$ Wouldn't that be more work for me ? I would have to do the checks in every entity type i add, instead of handling the checks in one place (as it is right now in CEntity::Intersect(...)) \$\endgroup\$ – cdmstr Dec 12 '13 at 22:37
  • \$\begingroup\$ Fair enough, it was just a suggestion. You could however use polymorphism with CCollisionShape or whatever type m_pCollisionShape is - that at least eliminates one layer of if statements. \$\endgroup\$ – Polar Dec 12 '13 at 23:44
  • \$\begingroup\$ Not direct answer but might help you. There is whole chapter in Modern C++ Design book by Andrei Alexandrescu dedicated for multimethods/virtual dispatching on more than one object. \$\endgroup\$ – Lufi Dec 13 '13 at 8:07
3
\$\begingroup\$

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;

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.

\$\endgroup\$
  • \$\begingroup\$ I liked the approach suggested, i think i have totally lost myself after hours and hours of working witth the engine and somewhere i missed the plot.. Below i present your approach in the code Sorry, don't know how to format it as a code, as using `` doesn't make any \new lines? \$\endgroup\$ – cdmstr Dec 13 '13 at 10:40
  • \$\begingroup\$ // This is where the whole update magic happens CWorld::Tick() { // Iterate through the classified for collision Entities for(...entities-that-were-classified-for-collision…) { // Grab both Entities Collision Shapes, both can be diff type CCollisionShape *pCollisionShapeA = pEntityA->GetCollisionShape(); CCollisionShape *pCollisionShapeB = pEntityB->GetCollisionShape(); // Check if they intersect/collide bool bCollide = CCollisionShape::Intersect(pCollisionShapeA, pCollisionShapeB); if(bCollide == true) { // Perform task } } } \$\endgroup\$ – cdmstr Dec 13 '13 at 10:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.