From db068ff16aa689c180037af831d9ba6fca7594d7 Mon Sep 17 00:00:00 2001 From: Thorbjørn Lindeijer Date: Sat, 28 Jan 2012 21:59:34 +0100 Subject: Removed inheritance from std::vector by CompoundSprite In my opinion, the code is clearer when using aggregation. For performance it makes no difference. This also fixes a memory leak in CompountSprite::clear, which forgot to delete any existing sprites. Reviewed-by: Erik Schilling Conflicts: src/compoundsprite.cpp src/compoundsprite.h --- src/compoundsprite.cpp | 80 +++++++++++++++++++++++++++----------------------- src/compoundsprite.h | 21 ++++++------- 2 files changed, 54 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/compoundsprite.cpp b/src/compoundsprite.cpp index e76a55dcf..3c16ff540 100644 --- a/src/compoundsprite.cpp +++ b/src/compoundsprite.cpp @@ -61,11 +61,8 @@ CompoundSprite::CompoundSprite() : CompoundSprite::~CompoundSprite() { - SpriteIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) - delete (*it); - - clear(); + delete_all(mSprites); + mSprites.clear(); // delete mImage; mImage = nullptr; @@ -78,7 +75,7 @@ bool CompoundSprite::reset() bool ret = false; SpriteIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) ret |= (*it)->reset(); @@ -93,7 +90,7 @@ bool CompoundSprite::play(std::string action) bool ret = false; SpriteIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) ret |= (*it)->play(action); @@ -108,7 +105,7 @@ bool CompoundSprite::update(int time) bool ret = false; SpriteIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) ret |= (*it)->update(time); @@ -118,11 +115,17 @@ bool CompoundSprite::update(int time) return ret; } -bool CompoundSprite::draw(Graphics* graphics, int posX, int posY) const +bool CompoundSprite::draw(Graphics *graphics, int posX, int posY) const { if (mNeedsRedraw) updateImages(); + if (mSprites.empty()) // Nothing to draw + return false; + +// posX += mOffsetX; +// posY += mOffsetY; + if (mAlpha == 1.0f && mImage) { return graphics->drawImage(mImage, posX + mOffsetX, posY + mOffsetY); @@ -144,7 +147,7 @@ bool CompoundSprite::draw(Graphics* graphics, int posX, int posY) const void CompoundSprite::drawSprites(Graphics* graphics, int posX, int posY) const { SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) { @@ -158,7 +161,7 @@ void CompoundSprite::drawSpritesSDL(Graphics* graphics, int posX, int posY) const { SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) (*it)->draw(graphics, posX, posY); @@ -170,7 +173,7 @@ int CompoundSprite::getWidth() const Sprite *base = nullptr; SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if ((base = (*it))) break; @@ -187,7 +190,7 @@ int CompoundSprite::getHeight() const Sprite *base = nullptr; SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if ((base = (*it))) break; @@ -199,7 +202,7 @@ int CompoundSprite::getHeight() const return 0; } -const Image* CompoundSprite::getImage() const +const Image *CompoundSprite::getImage() const { return mImage; } @@ -209,7 +212,7 @@ bool CompoundSprite::setSpriteDirection(SpriteDirection direction) bool ret = false; SpriteIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) ret |= (*it)->setSpriteDirection(direction); @@ -230,7 +233,7 @@ int CompoundSprite::getNumberOfLayers() const unsigned int CompoundSprite::getCurrentFrame() const { SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) return (*it)->getCurrentFrame(); @@ -242,7 +245,7 @@ unsigned int CompoundSprite::getCurrentFrame() const unsigned int CompoundSprite::getFrameCount() const { SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) return (*it)->getFrameCount(); @@ -251,42 +254,43 @@ unsigned int CompoundSprite::getFrameCount() const return 0; } -void CompoundSprite::addSprite(Sprite* sprite) +void CompoundSprite::addSprite(Sprite *sprite) { - push_back(sprite); + mSprites.push_back(sprite); mNeedsRedraw = true; } void CompoundSprite::setSprite(int layer, Sprite* sprite) { // Skip if it won't change anything - if (at(layer) == sprite) + if (mSprites.at(layer) == sprite) return; - if (at(layer)) - delete at(layer); - at(layer) = sprite; + if (mSprites.at(layer)) + delete mSprites.at(layer); + mSprites[layer] = sprite; mNeedsRedraw = true; } void CompoundSprite::removeSprite(int layer) { // Skip if it won't change anything - if (at(layer) == nullptr) + if (!mSprites.at(layer)) return; - delete at(layer); - at(layer) = nullptr; + delete mSprites.at(layer); + mSprites.at(layer) = nullptr; mNeedsRedraw = true; } void CompoundSprite::clear() { // Skip if it won't change anything - if (empty()) + if (mSprites.empty()) return; - std::vector::clear(); + delete_all(mSprites); + mSprites.clear(); mNeedsRedraw = true; delete_all(imagesCache); imagesCache.clear(); @@ -297,10 +301,11 @@ void CompoundSprite::clear() void CompoundSprite::ensureSize(size_t layerCount) { // Skip if it won't change anything - if (size() >= layerCount) + if (mSprites.size() >= layerCount) return; - resize(layerCount, nullptr); +// resize(layerCount, nullptr); + mSprites.resize(layerCount); } /** @@ -308,7 +313,7 @@ void CompoundSprite::ensureSize(size_t layerCount) */ unsigned int CompoundSprite::getCurrentFrame(unsigned int layer) { - if (layer >= size()) + if (layer >= mSprites.size()) return 0; Sprite *s = getSprite(layer); @@ -323,7 +328,7 @@ unsigned int CompoundSprite::getCurrentFrame(unsigned int layer) */ unsigned int CompoundSprite::getFrameCount(unsigned int layer) { - if (layer >= size()) + if (layer >= mSprites.size()) return 0; Sprite *s = getSprite(layer); @@ -418,7 +423,8 @@ void CompoundSprite::setAlpha(float alpha) #endif { SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++ it) + for (it = mSprites.begin(), it_end = mSprites.end(); + it != it_end; ++ it) { if (*it) (*it)->setAlpha(alpha); @@ -490,8 +496,8 @@ bool CompoundSprite::updateFromCache() const if (ic && ic->data.size() == size()) { bool fail(false); - SpriteConstIterator it1 = begin(); - SpriteConstIterator it1_end = end(); + SpriteConstIterator it1 = mSprites.begin(); + SpriteConstIterator it1_end = mSprites.end(); VectorPointers::const_iterator it2 = ic->data.begin(); VectorPointers::const_iterator it2_end = ic->data.end(); @@ -535,7 +541,7 @@ void CompoundSprite::initCurrentCacheItem() const // mCacheItem->alpha = mAlpha; SpriteConstIterator it, it_end; - for (it = begin(), it_end = end(); it != it_end; ++ it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) mCacheItem->data.push_back((*it)->getHash()); @@ -548,7 +554,7 @@ bool CompoundSprite::updateNumber(unsigned num) { SpriteConstIterator it, it_end; bool res(false); - for (it = begin(), it_end = end(); it != it_end; ++ it) + for (it = mSprites.begin(), it_end = mSprites.end(); it != it_end; ++ it) { if (*it) { diff --git a/src/compoundsprite.h b/src/compoundsprite.h index 1c04e44d2..1b9eeca34 100644 --- a/src/compoundsprite.h +++ b/src/compoundsprite.h @@ -44,11 +44,11 @@ class CompoundItem Image *alphaImage; }; -class CompoundSprite : public Sprite, private std::vector +class CompoundSprite : public Sprite { public: - typedef CompoundSprite::iterator SpriteIterator; - typedef CompoundSprite::const_iterator SpriteConstIterator; + typedef std::vector::iterator SpriteIterator; + typedef std::vector::const_iterator SpriteConstIterator; CompoundSprite(); @@ -60,7 +60,7 @@ public: virtual bool update(int time); - virtual bool draw(Graphics* graphics, int posX, int posY) const; + virtual bool draw(Graphics *graphics, int posX, int posY) const; /** * Gets the width in pixels of the first sprite in the list. @@ -72,7 +72,7 @@ public: */ virtual int getHeight() const; - virtual const Image* getImage() const; + virtual const Image *getImage() const; virtual bool setSpriteDirection(SpriteDirection direction); @@ -83,17 +83,17 @@ public: unsigned int getFrameCount() const; size_t size() const - { return std::vector::size(); } + { return mSprites.size(); } bool empty() const - { return std::vector::empty(); } + { return mSprites.empty(); } - void addSprite(Sprite* sprite); + void addSprite(Sprite *sprite); - void setSprite(int layer, Sprite* sprite); + void setSprite(int layer, Sprite *sprite); Sprite *getSprite(int layer) const - { return at(layer); } + { return mSprites.at(layer); } void removeSprite(int layer); @@ -144,6 +144,7 @@ private: bool mEnableAlphaFix; bool mDisableAdvBeingCaching; bool mDisableBeingCaching; + std::vector mSprites; }; #endif // COMPOUNDSPRITE_H -- cgit v1.2.3-60-g2f50