HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Sprite animation for a game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gameanimationspritefor

Problem

I'm just getting started with XNA and game development in general. I followed these instructions to create a basic game, then this article to get sprite animations working.

Now, I'm trying to abstract my design, in particular the stuff with sprite animations. Before I get much deeper, I want to make sure I have a good foundation to work with.

First, I have a Sprite class:

public class Sprite
{
    public Sprite()
    {
        // Default values for public members
        Position = Vector2.Zero;
        Scale = 1f;
        LayerDepth = 0f;
    }

    public IAnimation Animation { get; set; }

    public IMovement Movement { get; set; }

    public string AssetName { get; set; }

    public Texture2D Texture { get; set; }

    public Vector2 Position { get; set; }

    public Rectangle Frame { get; set; }

    public float Scale { get; set; }

    public float LayerDepth { get; set; }

    public void Update( GameTime gameTime )
    {
        if ( Animation != null )
        {
            Frame = Animation.GetNextFrame( gameTime, Frame );
        }

        if ( Movement != null )
        {
            Position = Movement.GetNextPosition( gameTime, Position );
        }
    }

    public void Draw( SpriteBatch spriteBatch )
    {
        spriteBatch.Draw( Texture, Position, Frame, Color.White, 0f, Vector2.Zero, Scale, SpriteEffects.None, LayerDepth );
    }
}


I have a SpriteGroup class for keeping groups of sprites organized and making them easy to use:

public class SpriteGroup
{
    public IList Sprites { get; set; }

    public void Update( GameTime gameTime )
    {
        foreach ( var sprite in Sprites )
        {
            sprite.Update( gameTime );
        }
    }

    public void Draw( SpriteBatch spriteBatch )
    {
        foreach ( var sprite in Sprites )
        {
            sprite.Draw( spriteBatch );
        }
    }
}


This is inherited by the Character class (nothing special there), of which I have several static instances

Solution

-
I would move the inner loop which loads the texture into the SpriteGroup class as well as moving the texture loading for the single sprite into the Sprite class:

public class Sprite
{
    ....
    public void LoadTexture()
    {
         Texture = Content.Load(AssetName);
    }
}

public class SpriteGroup
{
     ...
     public void LoadTextures()
     {
         foreach (var s in Sprites)
         {
              s.LoadTexture();
         }
     }
}


  • In general you should try to not expose how stuff is stored internally in a class directly (e.g. don't make your Sprites list a public property). If you ever consider changing the internal implementation then this could mean a lot of work to change existing code. You could make SpriteGroup either IEnumerable or simply not give public access to the underlying sprite collection at all.



  • Consider adding a constructor for SpriteGroup taking an IEnumerable which are added to the internal sprite list by the constructor code.



  • If you need to add/remove sprites on the fly then add Add/Remove methods.



  • Your Game class should not deal with sprites directly but rather with the "world" objects.



  • Call Update and Draw on the character/sprite group objects and let them forward it to the internal sprites (you have some of that code in there anyway)

Code Snippets

public class Sprite
{
    ....
    public void LoadTexture()
    {
         Texture = Content.Load<Texture2D>(AssetName);
    }
}

public class SpriteGroup
{
     ...
     public void LoadTextures()
     {
         foreach (var s in Sprites)
         {
              s.LoadTexture();
         }
     }
}

Context

StackExchange Code Review Q#31124, answer score: 3

Revisions (0)

No revisions yet.