Sprite and AnimatedSprite

Question

I have a Sprite class and an AnimatedSprite subclass, and I'd like to decouple these them and maintain substitutability, according to the Liskov Substitution Principle. I find that when dealing with pointers to Sprites I always use GetFrameWidth and GetFrameHeight instead of GetWidth and GetHeightjust in case the pointer is actually pointing to an AnimatedSprite that requires just the frame width or height instead of the entire sprite sheet.

Sprite.h

#ifndef CSPRITE_H
#define CSPRITE_H

#include <utility>
#include <allegro\fixed.h>
#include <allegro\fmaths.h>
#include <allegro\draw.h>
#include <allegro\file.h>
#include <allegro\datafile.h>
#include <allegro\color.h>
#include <string>
#include <vector>

#include "CBitmapCache.h"

class Sprite {

public:

static Sprite* CreateSprite(BITMAP* file, int centerX, int centerY);
static Sprite* CreateSprite(std::string file, int width, int height);
static Sprite* CreateSprite(std::string file, int width, int height, int centerX, int centerY);
static Sprite* CreateSprite(const Sprite& sprite);
virtual ~Sprite();
std::string* GetFilename() const;
virtual BITMAP* GetImage() const;
double GetX() const;
double GetY() const;
double GetZ() const;
Point GetPosition() const;
virtual int GetWidth() const;
virtual int GetHeight() const;
int GetFrameWidth() const;
int GetFrameHeight() const;
int GetCenterX() const;
int GetCenterY() const;
double GetRotation() const;
double GetScaleX() const;
double GetScaleY() const;
fixed GetScaleAsFixed() const;
int GetTint() const;
unsigned char GetTintIntensity() const;
unsigned char GetAlpha() const;
int GetRotationRadius() const;
void SetX(double x);
void SetY(double y);
void SetZ(double z);
void SetPosition(double x, double y);
void SetPosition(double x, double y, double z);
void SetPosition(const Point& position);
void SetCenterX(int x);
void SetCenterY(int y);
void SetScaleX(double x);
void SetScaleY(double y);
void SetRotation(double angle);
void SetTint(unsigned int tint);
void SetTintIntensity(unsigned char intensity);
void SetAlpha(unsigned char alpha);
void SetRadius(int radius);
Sprite& Clone(const Sprite& sprite);

/////////////////////////////////////////////////////////////////
// FOLLOWING METHODS ARE USED IN DERIVED ANIMATED SPRITE CLASS //
/////////////////////////////////////////////////////////////////

virtual int GetNumFrames() const;
virtual int GetNextFrameNum() const;
virtual int GetCurFrameNum() const;
virtual int GetPrevFrameNum() const;
virtual void Animate(int start, double deltaTime);
virtual void Animate(int start, int end, double deltaTime);
virtual int GetNumColumns() const;
virtual BITMAP** GetFrames() const;
virtual void SetFrames(std::vector<BITMAP*> _frames, int frameWidth, int frameHeight, int numCols);
virtual void SetFrames(BITMAP** frames, int frameWidth, int frameHeight, int numFrames, int numCols);
virtual bool CanLoop() const;
virtual void SetLoop(bool canLoop);
virtual void SetFrameRate(double deltaTime);
virtual double GetFrameRate();
virtual void DrawFrame(BITMAP* dest, int frame, int x,  int y, bool transparent);
virtual BITMAP* GetFrame(int numFrame);

protected:

std::string* _file;
BITMAP* _image;

Point* _position;
std::pair<int, int> _dimensions;
std::pair<int, int> _frameDimensions;
std::pair<int, int> _center;
std::pair<double, double> _scaleDimensions;
double _angle;
int _radius;
int _tint;
unsigned char _tintIntensity;
unsigned char _alpha;

void CalcCenterFrame();
virtual void SetImage(BITMAP* image);
virtual void SetCurFrame(int frame);
Sprite(BITMAP* file, int centerX, int centerY);
Sprite(std::string file, int width, int height, int centerX, int centerY);
Sprite(const Sprite& sprite);
Sprite& operator=(const Sprite& rhs);

private:

};

#endif

AnimatedSprite.h

#ifndef CANIMATEDSPRITE_H
#define CANIMATEDSPRITE_H

#include "CSprite.h"
#include <vector>

struct BITMAP;

class AnimatedSprite : public Sprite {

public:
static AnimatedSprite* CreateAnimatedSprite(BITMAP* file, int frameWidth, int frameHeight, int numFrames, int numCols, double frameRate, bool loop);
static AnimatedSprite* CreateAnimatedSprite(std::string file, int width, int height, int frameWidth, int frameHeight, int numFrames, int numCols, double frameRate, bool loop);
static AnimatedSprite* CreateAnimatedSprite(std::string file, int width, int height, int frameWidth, int frameHeight, int centerX, int centerY, int numFrames, int numCols, double frameRate, bool loop);
static AnimatedSprite* CreateAnimatedSprite(const AnimatedSprite& animatedSprite);
virtual ~AnimatedSprite();
virtual bool CanLoop() const;
virtual int GetNumFrames() const;
virtual int GetNextFrameNum() const;
virtual int GetCurFrameNum() const;
virtual int GetPrevFrameNum() const;
virtual int GetNumColumns() const;
virtual BITMAP* GetImage() const;
virtual BITMAP** GetFrames() const;
virtual void SetLoop(bool canLoop);
virtual void Animate(int start, double deltaTime);
virtual void Animate(int start, int end, double deltaTime);
AnimatedSprite& Clone(const AnimatedSprite& animatedSprite);
virtual void SetFrameRate(double deltaTime);
virtual double GetFrameRate();
virtual void SetFrames(std::vector<BITMAP*> _frames, int frameWidth, int frameHeight, int numCols);
virtual void SetFrames(BITMAP** frames, int frameWidth, int frameHeight, int numFrames, int numCols);
protected:

virtual void SetImage(BITMAP* image);
virtual BITMAP* GetFrame(int numFrame);
virtual void DrawFrame(BITMAP* dest, int frame, int x,  int y, bool transparent);
virtual void SetCurFrame(int frame);
void GrabFrame(BITMAP* image, int width, int height, int startx, int starty, int columns, int numFrame);
AnimatedSprite(BITMAP* file, int frameWidth, int frameHeight, int centerX, int centerY, int numFrames, int numCols, double frameRate, bool loop);
AnimatedSprite(std::string file, int width, int height, int frameWidth, int frameHeight, int centerX, int centerY, int numFrames, int numCols, double frameRate, bool loop);
AnimatedSprite(const AnimatedSprite& animSprite);
AnimatedSprite& operator=(const AnimatedSprite& rhs);

private:

int _curFrame;
int _numFrames;
bool _loop;
int _numColumns;
BITMAP** _frames;
BITMAP* _frameImage;
double _frameRate;

};

#endif

Answer 1

Looking over your code and comments, might I suggest a different strategy?

AnimatedSprite could be thought of as a collection of individual sprites. Rather than deriving it from Sprite, use composition to resuse Sprite objects inside of an AnimatedSprite.

If there is a genuine need to use both interchangeably (without knowing specifically whether you have a Sprite or AnimatedSprite) I would create an abstract class (interface) that collects the common methods and implement it in each of Sprite and AnimatedSprite.

Answer 2

Solution 1:

• have a texture/img class, with GetImgWidth/GetImgHeight. these return the dimensions of the physical texture, so in case of a sprite sheet, the size of the whole sheet
• make sprite extend image. getwidth/getheight return the size of the sprite, i.e. a single frame
• make animatedsprite extend sprite. getwidth/getheight return the size of a single frame.

So if you only accept instances of class Sprite, you can be sure that you are always retrieving the right property, and you still have the chance to get the size of the whole image on a sprite sheet. This solution also easily extends to other techniques, like packed textures.

Solution 2 (if you can live with the overhead):

Use RTTI and ask for the type of object dynamically. Cast with dynamic_cast<>() and if it is an animated sprite instead of a simple sprite, call the appropriate methods. IMHO, this is not a nice solution.

Answer 3

As others have mentioned.... I'd introduce more classes to make the interface simpler ( Image, Point, etc )

Then extract an interface that both sprite and animated sprite use.

But....maybe a whole different take on it...

I'd first ask myself, do I really need a sprite and an animated sprite? Or is what I think of as a "sprite" just a single frame animated sprite?

Therefore, just rename animatedsprite as sprite.