Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a small game I'm working on with a set of interfaces:

IHavePosition:

public interface IHavePosition<T>
{
    T Position { get; set; }
}

ICanMove:

public interface ICanMove<T> : IHavePosition<T>
    {
        IEnumerable<IMotor<T>> Motors { get; set; }
    }

ICharacter:

public interface ICharacter<TPos> : IHavePosition<TPos>, ICanMove<TPos>
{
    /// <summary>
    /// Gets or sets the action points.
    /// </summary>
    /// <value>
    /// The action points.
    /// </value>
    int ActionPoints { get; set; }

    /// <summary>
    /// Gets or sets the maximum action points.
    /// </summary>
    /// <value>
    /// The maximum action points.
    /// </value>
    int MaxActionPoints { get; set; }
}

IPlayer:

public interface IPlayer<TPos> : ICharacter<TPos>, IResetable
{
    /// <summary>
    /// Gets or sets the gold in the player's inventory.
    /// </summary>
    /// <value>
    /// The gold.
    /// </value>
    int Gold { get; set; }
}

IEnemy:

public interface IEnemy<TPos> : ICharacter<TPos>
    {

    }

As you can see, they all share a generic parameter TPos, which is the data type used to store an object's position. I keep this generic because I hope to reuse these interfaces in later projects which might have different position storage types (e.g. floating point coordinates versus an integer grid. 3D coordinates versus 2D, etc). The problem, however, is that this means when creating my classes, I need to ferry around this TPos generic construct to places where it doesn't make sense to define it. I'm left creating new Player<Vector3> or Player<Int2> instances all over the place when really I just want to be creating a Player.

An additional problem is that this might not (conceivably) be the only time I need to do this. What if I want to add a Health field that could be an int, or a float, etc? I'm now creating Player<Vector3,int> or Enemy<Int2, float> instances all over the place.

Since TPos is the same project-wide, is there a better way of doing this than lugging around references to TPos everywhere?

share|improve this question

4 Answers 4

up vote 21 down vote accepted

The problem, however, is that this means when creating my classes, I need to ferry around this TPos generic construct to places where it doesn't make sense to define it. I'm left creating new Player<Vector3> or Player<Int2> instances all over the place when really I just want to be creating a Player.

Yes, specifying Player<Vector3> each time can be quite frustrating. I'm not sure though if you just want to create just a Player or if you want to create a specific kind of player

You could create a GameOnePlayer class which extends Player<Vector3>, so then you create a GameOnePlayer and not a Player<Vector3>. You might have use for the Abstract Factory Pattern for the creation of these objects.


I think however that your class inheritance will become quite complex, if it isn't complex enough already. Which you state yourself with this:

An additional problem is that this might not (conceivably) be the only time I need to do this. What if I want to add a Health field that could be an int, or a float, etc? I'm now creating Player<Vector3,int> or Enemy<Int2, float> instances all over the place.

It sounds like what you need is to use composition over inheritance. A common way to do this in modern games is to use the Entity Component System approach.

If you go with this approach, both a Player and an Enemy should both just be an Entity. Then you can add whatever components they need, such as a Vector3Position component or a Int2Position component. Also IntHealthComponent and FloatHealthComponent.

There is a Entity Component System library named Artemis which has been ported to C# (I haven't used this myself, but I believe it's suitable enough for your needs). You can also make a Entity Component System implementation yourself, which many others have done on Code Review already (see )

To get an additional understanding of ECS, there is also an excellent question on GameDev StackExchange

Even though ECS is mostly used in real-time games, it is also possible to use it in turn-based games, which I have started doing myself in my Trading Card Game code.

share|improve this answer
3  
ECS makes the most sense for my system, particularly with regards to extensibility. I have used them before, can't believe I hadn't considered it to solve this problem. Thanks. –  Nick Udell Jul 21 at 13:14

Your interfaces are inconvenient, and you are falling in to a trap of convenience that actually leads to inconvenience..... let me explain...

C# makes it easy to create the getter/setter methods for interfaces/classes. There are three places I can see where your use (abuse) of this convenience leads to poor usability of your interfaces:

public interface IPlayer<TPos> : ICharacter<TPos>, IResetable
{
    int Gold { get; set; }
}

and:

public interface ICharacter<TPos> : IHavePosition<TPos>, ICanMove<TPos>
{
    int ActionPoints { get; set; }

    .....

}

and also maybe (borderline):

public interface IHavePosition<T>
{
    T Position { get; set; }
}

In each of these three places (maybe not the Position), the use of the interfaces would be 'relative'. Using the Gold as an example, let's say you need to change the Gold, there are very few places where you already know what the final gold will be without first determining what the current gold is. I can think of only the start/end conditions of the game, and some occasional in-the-middle major events. Also, those will likely be fixed amounts, like:

Player.Gold =0;

or

Player.Gold = 100;

The most common use of the method will be something like:

/// pick up gold at this location:
Player.Gold += Spot.Gold;
Spot.Gold = 0;

Or, if they have to 'pay' for something:

if (Player.Gold >= price)
{
    Player.Gold -= price;
    // add item to inventory...
}

This logic would have to be duplicated in many, many places.

It would be a much better design if you centralized the Gold (and action point, and position management) logic, and had methods like:

/// add an amount of gold, and return the new total (amount can be negative)
int AddGold(int amount);

/// test whether an amount can be paid,
/// if it can, reduce the gold by that amount and return true
/// otherwise return false.
bool PayAmount(int amount);

methods like the above would simplify much of your interaction with the implementations. The principle of encapsulation (and DRY) would be preserved.

share|improve this answer
3  
I totally agree, and this is something which I believe can be done when writing the code using Entity Component System as well, good catch! –  Simon André Forsberg Jul 21 at 13:46
    
You're completely right, I've let properties take over when they don't make sense. –  Nick Udell Jul 21 at 14:17

Just a tip on your naming convention if you want to follow Microsofts naming guideline.

For example, IComponent (descriptive noun), ICustomAttributeProvider (noun phrase), and IPersistable (adjective) are appropriate interface names. As with other type names, avoid abbreviations.

So your IHavePosition<T> would perhaps become IPositionable<T> and ICanMove<T> would certainly become IMoveable<T>.

Seeing HavePosition or CanMove makes me think about booleans. Examples from .NET:

  • Process.HasExited or
  • Control.HasChildren
share|improve this answer

May I add that you could try using alias to alleviate the situation a bit. However, you will have to add the using alias on top of every source file :

using PlayerPosition = IHavePosition<Vector3>;
/* im not sure if you can reuse the same name as the generic type.
   if not, rename the one of them (like prefix it with acronym of the game) */
using Player = Player<Vector3, int>; 
using Enemy = Enemy<Int2, float>;
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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