How to avoid casting from interface to class

Question

In trying to design a collision detection component for a game, I came up with the following solution. I define an interface ICollideable that looks something like:

interface ICollideable
{
    Sprite Sprite { get; }
    int    Damage { get; }

    void HandleCollision(ICollideable collidedWith);
}

Basically, any game objects that want to participate in collision detection have to implement this interface, then register themselves with the detector, which maintains a list of ICollideables. When it detects a collision, it calls the HandleCollision method on the object and passes in a reference to the object it collided with.

I like this, because it lets me keep all my collision algorithms in one place, and lets the game objects themselves decide how to handle the collision. But because of the latter, I find I am having to check the underlying object type. For example, I don't want Players to collide with each other, so in the Player class there might be something like:

void HandleCollision(ICollideable collidedWith)
{
    if (!(collidedWith is Player)) { // do stuff }
}

and so on, and I am wondering if this is telling me that I have a bad design and what the alternatives might be.

Second question, further along the lines of the first. For scoring purposes, if an Enemy is destroyed by a Projectile, someone needs to know the "Owning Player" member of the Projectile class. However, none of my other collideables have or need this property, so I find myself wanting to do (in the Enemy HandleCollision):

void HandleCollision(ICollideable collidedWith)
{
    if (collidedWith is Projectile) {
        Health -= collidedWith.Damage;

        if (Health <= 0) {
            Player whoDestroyedMe = (collidedWith as Projectile).FiredBy
            // ...
        }
    }
}

I haven't a clue as to how to handle this with a better design. Any insights would be appreciated.

EDIT: I wanted to pull focus towards the second question, because my gut tells me a way of handling this will solve the first question. As for the first question, I thought of a way to abstract this behavior. I could define an enum:

enum Team
{
    Player,
    Enemy,
    Neither
}

and have ICollideables implement this property. Then the collision detector simply doesn't register collisions between collideables on the same "Team". So, Player and Player Projectiles would be on one team, Enemy and Enemy Projectiles on the other, and the environment (which can damage both) can be on neither. It doesn't have to be an enum, could be an int or a string or anything, with the idea that objects with the same value for this property do not collide with each other.

I like this idea of modeling behavior with a simple attribute. For instance, if I want to turn "allow friendly fire" on, all I have to do is create Projectiles with a Team value other than the Player's Team value. However, I still may have cases where this is not enough. For example, a Player may have shields that are temporarily impervious to projectiles but will not protect against a direct collision with an enemy, and so on.

Answer 1

Sometimes the simplest method is the best method. Unless you want to separate your collision interactions into numerous subtypes, you could instead place a bool IsPlayer property within the Interface.

The upside here is that you have a cheaper, and type safe method of determination over casting.

If (isplayer == true)
{
      Handlethisway;
}

The downside is that you're still having to do some sort of state checking, but this is more efficient.

To avoid any state checks, you'd need to do the following: Make an ICollidablePlayer Interface which accepts generic Icollideable and handles them differently. Since the Icollideable is your injected dependency, the ICollideablePlayer dependencies are inherent. The objects of Icollideable would have no knowledge of this separate process, and interact with each other in the same manner.

ICollideablePlayer:ICollideable
{
   //DependenciesHere

   HandlePlayerCollision(ICollideable)
   { 
       HandleDifferently
       {
       }

       ICollideable
       {
           //DependenciesHere
           HandleCollision(ICollideable)
       }
    }
}

In an interaction, the ICollideable will treat the player as any other ICollideable, but the ICollideablePlayer will reject the interaction when it does the check itself.

For things like shields and all that, You're talking about state changes which implies that those such things should be properties within either of those Interfaces such that something like bool ColliderOff to temporarily change the state.

Answer 2

I think this is a good question @idlewire and I have to say that I don't think there is anything fundamentally wrong with your original solution. In asking whether object Foo should be allowed to cast the ICollideable to a Bar, the important question is only: is undesirable to have Foo knowing anything at all about Bar? If the answer is 'no' because Foo already knows about Bars (for behaviours other than collisions, perhaps) then I see no problem in the cast and, as you say, it allows you to better encapsulate the behaviour of both.

Where you need to be wary is only where this would introduces a dependency between two things you'd like kept apart - which would make re-use of either without the other (in a different game application for example) impossible. There you might want to either have more specific sub-interfaces from ICollideable (e.g. IElastic and IInelastic), or use properties on the interface as you have proposed with the Enum.

In short, I think your original posting shows good evidence of OO thinking, not bad.

Answer 3

I think you're going the wrong way altogether in handling the collision inside of the class of one of the colliders. I would put this logic into a third object, outside of the entity objects. You could do all of the checking of the types in this third object, and even handle most of the logic there too. Why should a Ship or a Projectile have a monopoly over the logic that happens when one hits the other?

The following is how I might handle this, although it means using an object for each style of collision (Ship vs Ship, Ship vs Projectile, Ship vs Asteroid, etc.) You might be more comfortable putting all that logic into a single object, or even a single method on that object.

public interface ICollisionHandler
{
    bool HandleCollision(Entity first, Entity second);
}

public class PlayerShipVsProjectile : ICollisionHandler
{
    private GameOptions options;
    public PlayersOwnShipHandler(GameOptions options)
    {
        this.options = options;
    }

    public bool HandleCollision(Entity first, Entity second)
    {
        // Exactly how you go about doing this line, whether using the object types
        // or using a Type property, or some other method, is not really that important.
        // You have so much more important things to worry about than these little
        // code design details.
        if ((!first is Ship) || (!second is Projectile)) return false;
        Ship ship = (Ship)first;
        Projectile projectile = (Projectile)second;

        // Because we've decided to put this logic in it's own class, we can easily
        // use a constructor parameter to get access to the game options. Here, we
        // can have access to whether friendly fire is turned on or not.
        if (ship.Owner.IsFriendlyWith(projectile.Shooter) &&
              !this.options.FriendlyFire) {
            return false;
        }

        if (!ship.InvulnerableTypes.Contains(InvulnerableTypes.PROJECTILE))
        {
             ship.DoDamage(projectile.Damage);
        }

        return true;
    }
}

Like this, you can then do...

// Somewhere in the setup...
CollisionMapper mapper = new CollisionMapper();
mapper.AddHandler(new ShipVsProjectile(gameOptions));
mapper.AddHandler(new ShipVsShip(gameOptions));

// Somewhere in your collision handling...
mapper.Resolve(entityOne, entityTwo);

The implementation of CollisionMapper is left as an exercise for the reader. Remember that you might need to have Resolve call the ICollisionHandler's "Handle" method twice, with the second time reversing the entities (otherwise your collision handler objects will need to check for the reverse situation, which might be ok as well).

I feel this makes the code easier to read. A single object describes exactly what will happen when two entities collide, rather than trying to put all this info into one of the entity objects.

Answer 4

How about something like this?

Collidable.cs

abstract class Collidable
{
    public Sprite Sprite { get; protected set;  }
    public int Damage { get; protected set; }

    protected delegate void CollisionAction(Collidable with);

    protected Dictionary<Type, CollisionAction> collisionTypes = new Dictionary<Type, CollisionAction>();

    public void HandleCollision(Collidable with)
    {
        Type collisionTargetType = with.GetType();

        CollisionAction action;

        bool keyFound = collisionTypes.TryGetValue(collisionTargetType, out action);

        if (keyFound)
        {
            action(with);
        }
    }
}

Bullet.cs

class Bullet: Collidable
{
    public Bullet()
    {
        collisionTypes.Add(typeof(Player), HandleBulletPlayerCollision);
        collisionTypes.Add(typeof(Bullet), HandleBulletBulletCollision);
    }

    private void HandleBulletPlayerCollision(Collidable with)
    {
        Console.WriteLine("Bullet collided with {0}", with.ToString());
    }

    private void HandleBulletBulletCollision(Collidable with)
    {
        Console.WriteLine("Bullet collided with {0}.", with.ToString());
    }
}

Player.cs

class Player : Collidable
{
    public Player()
    {
        collisionTypes.Add(typeof(Bullet), HandlePlayerBulletCollision);
        collisionTypes.Add(typeof(Player), HandlePlayerPlayerCollision);
    }

    private void HandlePlayerBulletCollision(Collidable with)
    {
        Console.WriteLine("Player collided with {0}.", with.ToString());
    }

    private void HandlePlayerPlayerCollision(Collidable with)
    {
        Console.WriteLine("Player collided with {0}.", with.ToString());
    }
}

Answer 5

For the first case, I would add the following extra method to ICollidable:

bool CanCollideWith(ICollidable collidedWith)

As the name suggests, it would return true or false depending upon whether it can collide with the passed in object.

Your Player.HandleCollision method would just do its stuff because the calling method could do that test and not even call the method if it wasn't required.