4
\$\begingroup\$

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
\$\endgroup\$
2
  • 2
    \$\begingroup\$ I'd prioritize Single Responsibility Principle and Interface Segregation Principle before LSP in this case. \$\endgroup\$
    – rjnilsson
    Aug 16, 2011 at 9:30
  • \$\begingroup\$ I concure, damn these classes do a lot. Reminds me of the bloaters code smell. \$\endgroup\$ Jun 8, 2013 at 1:30

3 Answers 3

4
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ Solution 1 is essentially what the code already does. A sprite object is used to represent non-animating single-framed images and an animatedsprite object is used to represent either a collection of images or a series of frames...which after thinking aloud about that I'm going to separate those into two classes, a collection and animation. \$\endgroup\$
    – Casey
    Aug 16, 2011 at 15:58
  • \$\begingroup\$ The way I understood your original question is that you want to separate Sprite and AnimatedSprite, because you'd need to call GetHeight on Sprite and GetFrameHeight on AnimatedSprite. Now you're saying that the two classes you want to separate are two different kinds of AnimatedSprite. Given this, my first recommendation is not what your code is already doing. I recommended inserting an Image class with a different property to ask for the 'real' size of any image, and I changed the definition of GetHeight of AnimatedSprite. \$\endgroup\$
    – ultimA
    Aug 17, 2011 at 9:30
0
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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