Resolve Image* of animation phase at load time instead of storing just the

spriteset index and looking it up later (checking validity should still be
added). Also calculate animation length during loading instead of summing it up
each time it is requested.

7 files changed, 91 insertions, 89 deletions

diff --git a/ChangeLog b/ChangeLog
index d81e426f..f257b684 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,12 @@
 	* src/action.h, src/action.cpp, src/animation.h, src/CMakeLists.txt,
 	src/animatedsprite.cpp, src/animation.cpp, src/Makefile.am: Separated
 	Action class to its own module.
+	* src/action.h, src/action.cpp, src/animatedsprite.h, src/animation.h,
+	src/animatedsprite.cpp, src/animation.cpp: Resolve Image* of animation
+	phase at load time instead of storing just the spriteset index and
+	looking it up later (checking validity should still be added). Also
+	calculate animation length during loading instead of summing it up
+	each time it is requested.
 
 2006-11-12  Bjørn Lindeijer  <bjorn@lindeijer.nl>
 
diff --git a/src/action.cpp b/src/action.cpp
index 4abfa229..4a5a941a 100644
--- a/src/action.cpp
+++ b/src/action.cpp
@@ -29,15 +29,14 @@
 #include "utils/dtor.h"
 
 
-Action::Action():
-    mSpriteset(NULL)
+Action::Action()
 {
 }
 
 Action::~Action()
 {
-    std::for_each(mAnimations.begin(), mAnimations.end(), make_dtor(mAnimations));
-    mAnimations.clear();
+    std::for_each(mAnimations.begin(), mAnimations.end(),
+                  make_dtor(mAnimations));
 }
 
 Animation*
diff --git a/src/action.h b/src/action.h
index a686e722..6872ea5c 100644
--- a/src/action.h
+++ b/src/action.h
@@ -29,7 +29,6 @@
 #include <libxml/tree.h>
 
 class Image;
-class Spriteset;
 
 struct AnimationPhase;
 class Animation;
@@ -50,18 +49,6 @@ class Action
          */
         ~Action();
 
-        /**
-         * Sets the spriteset used by this action.
-         */
-        void
-        setSpriteset(Spriteset *spriteset) { mSpriteset = spriteset; }
-
-        /**
-         * Returns the spriteset used by this action.
-         */
-        Spriteset*
-        getSpriteset() const { return mSpriteset; }
-
         void
         setAnimation(int direction, Animation *animation);
 
@@ -75,7 +62,6 @@ class Action
         getAnimation(int direction) const;
 
     protected:
-        Spriteset *mSpriteset;
         typedef std::map<int, Animation*> Animations;
         typedef Animations::iterator AnimationIterator;
         Animations mAnimations;
diff --git a/src/animatedsprite.cpp b/src/animatedsprite.cpp
index e5a4df15..61d2f3cf 100644
--- a/src/animatedsprite.cpp
+++ b/src/animatedsprite.cpp
@@ -30,6 +30,7 @@
 
 #include "resources/resourcemanager.h"
 #include "resources/spriteset.h"
+#include "resources/image.h"
 
 AnimatedSprite::AnimatedSprite(const std::string& animationFile, int variant):
     mAction(NULL),
@@ -91,17 +92,18 @@ AnimatedSprite::AnimatedSprite(const std::string& animationFile, int variant):
         else if (xmlStrEqual(node->name, BAD_CAST "action"))
         {
             std::string actionName = getProperty(node, "name", "");
-            std::string imageset = getProperty(node, "imageset", "");
+            std::string imagesetName = getProperty(node, "imageset", "");
 
-            if (mSpritesets.find(imageset) == mSpritesets.end()) {
+            SpritesetIterator si = mSpritesets.find(imagesetName);
+            if (si == mSpritesets.end()) {
                 logger->log("Warning: imageset \"%s\" not defined in %s",
-                            imageset.c_str(),
+                            imagesetName.c_str(),
                             animationFile.c_str());
 
                 // skip loading animations
                 continue;
             }
-
+            Spriteset *imageset = si->second;
 
             SpriteAction actionType = makeSpriteAction(actionName);
             if (actionType == ACTION_INVALID)
@@ -112,7 +114,6 @@ AnimatedSprite::AnimatedSprite(const std::string& animationFile, int variant):
                 continue;
             }
             Action *action = new Action();
-            action->setSpriteset(mSpritesets[imageset]);
             mActions[actionType] = action;
 
             // When first action set it as default direction
@@ -131,12 +132,15 @@ AnimatedSprite::AnimatedSprite(const std::string& animationFile, int variant):
                 if (!xmlStrEqual(animationNode->name, BAD_CAST "animation"))
                     continue;
 
-                std::string directionName = getProperty(animationNode, "direction", "");
+                std::string directionName =
+                    getProperty(animationNode, "direction", "");
+                SpriteDirection directionType =
+                    makeSpriteDirection(directionName);
 
-                SpriteDirection directionType = makeSpriteDirection(directionName);
                 if (directionType == DIRECTION_INVALID)
                 {
-                    logger->log("Warning: Unknown direction \"%s\" defined for action %s in %s",
+                    logger->log("Warning: Unknown direction \"%s\" defined "
+                                "for action %s in %s",
                         directionName.c_str(),
                         actionName.c_str(),
                         animationFile.c_str());
@@ -159,28 +163,28 @@ AnimatedSprite::AnimatedSprite(const std::string& animationFile, int variant):
                         int offsetX = getProperty(phaseNode, "offsetX", 0);
                         int offsetY = getProperty(phaseNode, "offsetY", 0);
 
-                        offsetY -= mSpritesets[imageset]->getHeight() - 32;
-                        offsetX -= mSpritesets[imageset]->getWidth() / 2 - 16;
-                        animation->addPhase(index + variant_offset, delay,
-                                offsetX, offsetY);
+                        offsetY -= imageset->getHeight() - 32;
+                        offsetX -= imageset->getWidth() / 2 - 16;
+                        Image *img = imageset->get(index + variant_offset);
+                        animation->addPhase(img, delay, offsetX, offsetY);
                     }
                     else if (xmlStrEqual(phaseNode->name, BAD_CAST "sequence"))
                     {
                         int start = getProperty(phaseNode, "start", 0);
                         int end = getProperty(phaseNode, "end", 0);
-                        int offsetY = -mSpritesets[imageset]->getHeight() + 32;
-                        int offsetX = -mSpritesets[imageset]->getWidth() / 2 + 16;
+                        int offsetY = -imageset->getHeight() + 32;
+                        int offsetX = -imageset->getWidth() / 2 + 16;
                         while (end >= start)
                         {
-                            animation->addPhase(start + variant_offset,
-                                    delay, offsetX, offsetY);
+                            Image *img = imageset->get(start + variant_offset);
+                            animation->addPhase(img, delay, offsetX, offsetY);
                             start++;
                         }
                     }
                     else if (xmlStrEqual(phaseNode->name, BAD_CAST "end"))
                     {
                         animation->addTerminator();
-                    };
+                    }
                 } // for phaseNode
             } // for animationNode
         } // if "<imageset>" else if "<action>"
@@ -223,8 +227,8 @@ AnimatedSprite::getProperty(xmlNodePtr node, const char* name, int def)
 }
 
 std::string
-AnimatedSprite::getProperty(xmlNodePtr node, const char* name,
-                            const std::string& def)
+AnimatedSprite::getProperty(xmlNodePtr node, const char *name,
+                            const std::string &def)
 {
     xmlChar *prop = xmlGetProp(node, BAD_CAST name);
     if (prop) {
@@ -294,7 +298,7 @@ AnimatedSprite::play(SpriteAction action)
 void
 AnimatedSprite::update(int time)
 {
-    bool notFinished = true;
+    bool finished = false;
     // Avoid freaking out at first frame or when tick_time overflows
     if (time < mLastTime || mLastTime == 0)
         mLastTime = time;
@@ -304,11 +308,12 @@ AnimatedSprite::update(int time)
     {
         Animation *animation = mAction->getAnimation(mDirection);
         if (animation != NULL) {
-            notFinished = animation->update((unsigned int)(time - mLastTime));}
+            finished = !animation->update((unsigned int)(time - mLastTime));
+        }
         mLastTime = time;
     }
 
-    if (!notFinished)
+    if (finished)
     {
         play(ACTION_STAND);
     }
@@ -317,33 +322,46 @@ AnimatedSprite::update(int time)
 bool
 AnimatedSprite::draw(Graphics* graphics, Sint32 posX, Sint32 posY) const
 {
-    if (!mAction)
-        return false;
-
-    Animation *animation = mAction->getAnimation(mDirection);
-    if (animation == NULL) return false;
-
-    int phase = animation->getCurrentPhase();
-    if (phase < 0)
+    const AnimationPhase *phase = getCurrentPhase();
+    if (!phase || !phase->image)
+    {
         return false;
+    }
 
-    Spriteset *spriteset = mAction->getSpriteset();
-    Image *image = spriteset->get(phase);
-    Sint32 offsetX = animation->getOffsetX();
-    Sint32 offsetY = animation->getOffsetY();
-    return graphics->drawImage(image, posX + offsetX, posY + offsetY);
+    Sint32 offsetX = phase->offsetX;
+    Sint32 offsetY = phase->offsetY;
+    return graphics->drawImage(phase->image, posX + offsetX, posY + offsetY);
 }
 
 int
 AnimatedSprite::getWidth() const
 {
-    return mAction ? mAction->getSpriteset()->getWidth() : 0;
+    const AnimationPhase *phase = getCurrentPhase();
+    return (phase && phase->image) ? phase->image->getWidth() : 0;
 }
 
 int
 AnimatedSprite::getHeight() const
 {
-    return mAction ? mAction->getSpriteset()->getHeight() : 0;
+    const AnimationPhase *phase = getCurrentPhase();
+    return (phase && phase->image) ? phase->image->getHeight() : 0;
+}
+
+const AnimationPhase*
+AnimatedSprite::getCurrentPhase() const
+{
+    if (!mAction)
+    {
+        return NULL;
+    }
+
+    Animation *animation = mAction->getAnimation(mDirection);
+    if (animation == NULL)
+    {
+        return NULL;
+    }
+
+    return animation->getCurrentPhase();
 }
 
 SpriteAction
diff --git a/src/animatedsprite.h b/src/animatedsprite.h
index e87a9885..73dfa529 100644
--- a/src/animatedsprite.h
+++ b/src/animatedsprite.h
@@ -33,6 +33,7 @@
 class Action;
 class Graphics;
 class Spriteset;
+struct AnimationPhase;
 
 enum SpriteAction
 {
@@ -97,7 +98,8 @@ class AnimatedSprite
          * Inform the animation of the passed time so that it can output the
          * correct animation phase.
          */
-        void update(int time);
+        void
+        update(int time);
 
         /**
          * Draw the current animation phase at the coordinates given in screen
@@ -107,13 +109,13 @@ class AnimatedSprite
         draw(Graphics* graphics, Sint32 posX, Sint32 posY) const;
 
         /**
-         * gets the width in pixels of the current animation phase.
+         * Returns the width in pixels of the current animation phase.
          */
         int
         getWidth() const;
 
         /**
-         * gets the height in pixels of the current animation phase.
+         * Returns the height in pixels of the current animation phase.
          */
         int
         getHeight() const;
@@ -136,6 +138,12 @@ class AnimatedSprite
         substituteAction(SpriteAction complete, SpriteAction with);
 
         /**
+         * Returns the current animation frame.
+         */
+        const AnimationPhase*
+        getCurrentPhase() const;
+
+        /**
          * Gets an integer property from an xmlNodePtr.
          *
          * TODO: Same function is present in MapReader. Should probably be
diff --git a/src/animation.cpp b/src/animation.cpp
index bca4bb58..a91bdf8d 100644
--- a/src/animation.cpp
+++ b/src/animation.cpp
@@ -27,7 +27,8 @@
 
 #include "utils/dtor.h"
 
-Animation::Animation()
+Animation::Animation():
+    mLength(0)
 {
     reset();
 }
@@ -39,7 +40,6 @@ Animation::reset()
     iCurrentPhase = mAnimationPhases.begin();
 }
 
-
 bool
 Animation::update(unsigned int time)
 {
@@ -68,29 +68,28 @@ Animation::update(unsigned int time)
     return true;
 }
 
-
-int
+const AnimationPhase*
 Animation::getCurrentPhase() const
 {
-    return mAnimationPhases.empty() ? -1 : iCurrentPhase->image;
+    return mAnimationPhases.empty() ? NULL : &(*iCurrentPhase);
 }
 
-
 void
-Animation::addPhase(int image, unsigned int delay, int offsetX, int offsetY)
+Animation::addPhase(Image *image, unsigned int delay, int offsetX, int offsetY)
 {
-    //add new phase to animation list
+    // Add new phase to animation list
     AnimationPhase newPhase = { image, delay, offsetX, offsetY};
 
     mAnimationPhases.push_back(newPhase);
-    //reset animation circle
+    mLength += delay;
+    // Reset animation circle
     iCurrentPhase = mAnimationPhases.begin();
 }
 
 void
 Animation::addTerminator()
 {
-    AnimationPhase terminator = { -1, 0, 0, 0};
+    AnimationPhase terminator = { NULL, 0, 0, 0};
     mAnimationPhases.push_back(terminator);
     iCurrentPhase = mAnimationPhases.begin();
 }
@@ -98,20 +97,5 @@ Animation::addTerminator()
 bool
 Animation::isTerminator(AnimationPhase candidate)
 {
-    return (candidate.image < 0);
-}
-
-int
-Animation::getLength()
-{
-    if (mAnimationPhases.empty())
-        return 0;
-
-    std::list<AnimationPhase>::iterator i;
-    int length = 0;
-    for (i = mAnimationPhases.begin(); i != mAnimationPhases.end(); i++)
-    {
-        length += i->delay;
-    }
-    return length;
+    return (candidate.image == NULL);
 }
diff --git a/src/animation.h b/src/animation.h
index 8f62b9c4..0230e820 100644
--- a/src/animation.h
+++ b/src/animation.h
@@ -37,7 +37,7 @@ class Spriteset;
  */
 struct AnimationPhase
 {
-    int image;
+    Image *image;
     unsigned int delay;
     int offsetX;
     int offsetY;
@@ -65,7 +65,7 @@ class Animation
          * Appends a new animation at the end of the sequence
          */
         void
-        addPhase(int image, unsigned int delay, int offsetX, int offsetY);
+        addPhase(Image *image, unsigned int delay, int offsetX, int offsetY);
 
         /**
          * Appends an animation terminator that states that the animation
@@ -82,7 +82,7 @@ class Animation
         bool
         update(unsigned int time);
 
-        int
+        const AnimationPhase*
         getCurrentPhase() const;
 
         /**
@@ -101,13 +101,14 @@ class Animation
          * Returns the length of this animation.
          */
         int
-        getLength();
+        getLength() const { return mLength; }
 
     protected:
-        static bool isTerminator(AnimationPhase);
+        static bool isTerminator(AnimationPhase phase);
         std::list<AnimationPhase> mAnimationPhases;
         std::list<AnimationPhase>::iterator iCurrentPhase;
         unsigned int mTime;
+        int mLength;
 };
 
 #endif