Use the native TMX tile animation format

Rewrote the tile animation loading code based on XML tags, replacing
the code that loaded tile animations from tile properties.

Also made a number of code simplifications and optimizations:

* Replaced a number of pointer members with value members.
* Pass around Animation and TileAnimation by value, using std::move to
  avoid allocating copies.
* push -> emplace
* push_front -> emplace_front
* push_back -> emplace_back
* Use range-based for loops
* Use std::vector instead of std::list for storing affected tiles
  (less fragmentation)
* Avoid string copies and allocations while parsing CSV layer data.
* Replaced xmlNodeGetContent with directly accessing 'content'.

17 files changed, 247 insertions, 288 deletions

diff --git a/src/actor.cpp b/src/actor.cpp
index a644c1e0..f8e754ad 100644
--- a/src/actor.cpp
+++ b/src/actor.cpp
@@ -26,7 +26,7 @@ Actor::Actor() = default;
 
 Actor::~Actor()
 {
-    setMap(nullptr);
+    Actor::setMap(nullptr);
 }
 
 void Actor::setMap(Map *map)
diff --git a/src/actorsprite.cpp b/src/actorsprite.cpp
index 088bc9f4..90176dcd 100644
--- a/src/actorsprite.cpp
+++ b/src/actorsprite.cpp
@@ -437,16 +437,16 @@ void ActorSprite::loadTargetCursor(const std::string &filename,
         return;
     }
 
-    auto *anim = new Animation;
+    Animation anim;
 
     for (unsigned int i = 0; i < currentImageSet->size(); ++i)
     {
-        anim->addFrame(currentImageSet->get(i), DEFAULT_FRAME_DELAY,
+        anim.addFrame(currentImageSet->get(i), DEFAULT_FRAME_DELAY,
                       -(currentImageSet->getWidth() / 2),
                       -(currentImageSet->getHeight() / 2));
     }
 
-    auto *currentCursor = new SimpleAnimation(anim);
+    auto *currentCursor = new SimpleAnimation(std::move(anim));
 
     targetCursorImages[type][size] = currentImageSet;
     targetCursor[type][size] = currentCursor;
diff --git a/src/animationparticle.cpp b/src/animationparticle.cpp
index a5461ecd..db38b526 100644
--- a/src/animationparticle.cpp
+++ b/src/animationparticle.cpp
@@ -23,29 +23,29 @@
 
 #include "simpleanimation.h"
 
-AnimationParticle::AnimationParticle(Map *map, Animation *animation):
+AnimationParticle::AnimationParticle(Map *map, Animation animation):
     ImageParticle(map, nullptr),
-    mAnimation(new SimpleAnimation(animation))
+    mAnimation(std::move(animation))
 {
 }
 
 AnimationParticle::AnimationParticle(Map *map, xmlNodePtr animationNode,
-                                     const std::string& dyePalettes):
+                                     const std::string &dyePalettes):
     ImageParticle(map, nullptr),
-    mAnimation(new SimpleAnimation(animationNode, dyePalettes))
+    mAnimation(animationNode, dyePalettes)
 {
 }
 
 AnimationParticle::~AnimationParticle()
 {
-    delete mAnimation;
+    // Prevent ImageParticle from decreasing the reference count of the image
     mImage = nullptr;
 }
 
 bool AnimationParticle::update()
 {
-    mAnimation->update(10); // particle engine is updated every 10ms
-    mImage = mAnimation->getCurrentImage();
+    mAnimation.update(10); // particle engine is updated every 10ms
+    mImage = mAnimation.getCurrentImage();
 
     return Particle::update();
 }
diff --git a/src/animationparticle.h b/src/animationparticle.h
index 16cc12a5..230d4e7d 100644
--- a/src/animationparticle.h
+++ b/src/animationparticle.h
@@ -23,27 +23,28 @@
 #define ANIMATION_PARTICLE_H
 
 #include "imageparticle.h"
+#include "simpleanimation.h"
 
 #include <libxml/tree.h>
 
-class Animation;
+#include <memory>
+
 class Map;
-class SimpleAnimation;
 
 class AnimationParticle : public ImageParticle
 {
     public:
-        AnimationParticle(Map *map, Animation *animation);
+        AnimationParticle(Map *map, Animation animation);
 
         AnimationParticle(Map *map, xmlNodePtr animationNode,
-                          const std::string& dyePalettes = std::string());
+                          const std::string &dyePalettes = std::string());
 
         ~AnimationParticle() override;
 
         bool update() override;
 
     private:
-        SimpleAnimation *mAnimation; /**< Used animation for this particle */
+        SimpleAnimation mAnimation; /**< Used animation for this particle */
 };
 
 #endif
diff --git a/src/gui/widgets/progressindicator.cpp b/src/gui/widgets/progressindicator.cpp
index 40523e9a..37aaab62 100644
--- a/src/gui/widgets/progressindicator.cpp
+++ b/src/gui/widgets/progressindicator.cpp
@@ -35,19 +35,16 @@ ProgressIndicator::ProgressIndicator()
     ImageSet *images = Theme::getImageSetFromTheme("progress-indicator.png",
                                                    32, 32);
 
-    auto *anim = new Animation;
+    Animation anim;
     for (size_t i = 0; i < images->size(); ++i)
-        anim->addFrame(images->get(i), 100, 0, 0);
+        anim.addFrame(images->get(i), 100, 0, 0);
 
-    mIndicator = new SimpleAnimation(anim);
+    mIndicator = std::make_unique<SimpleAnimation>(std::move(anim));
 
     setSize(32, 32);
 }
 
-ProgressIndicator::~ProgressIndicator()
-{
-    delete mIndicator;
-}
+ProgressIndicator::~ProgressIndicator() = default;
 
 void ProgressIndicator::logic()
 {
diff --git a/src/gui/widgets/progressindicator.h b/src/gui/widgets/progressindicator.h
index 13cab227..428bbd02 100644
--- a/src/gui/widgets/progressindicator.h
+++ b/src/gui/widgets/progressindicator.h
@@ -23,6 +23,8 @@
 
 #include <guichan/widget.hpp>
 
+#include <memory>
+
 class SimpleAnimation;
 
 /**
@@ -39,7 +41,7 @@ public:
     void draw(gcn::Graphics *graphics) override;
 
 private:
-    SimpleAnimation *mIndicator;
+    std::unique_ptr<SimpleAnimation> mIndicator;
 };
 
 #endif // PROGRESSINDICATOR_H
diff --git a/src/map.cpp b/src/map.cpp
index 1ddc8b65..1de22456 100644
--- a/src/map.cpp
+++ b/src/map.cpp
@@ -25,6 +25,7 @@
 #include "client.h"
 #include "configuration.h"
 #include "graphics.h"
+#include "log.h"
 #include "particle.h"
 #include "simpleanimation.h"
 #include "tileset.h"
@@ -62,32 +63,22 @@ struct Location
     MetaTile *tile;
 };
 
-TileAnimation::TileAnimation(Animation *ani):
-    mLastImage(nullptr)
+TileAnimation::TileAnimation(Animation animation)
+    : mAnimation(std::move(animation))
 {
-    mAnimation = new SimpleAnimation(ani);
-}
-
-TileAnimation::~TileAnimation()
-{
-    delete mAnimation;
 }
 
 void TileAnimation::update(int ticks)
 {
-    if (!mAnimation)
-        return;
-
-    // update animation
-    mAnimation->update(ticks);
+    mAnimation.update(ticks);
 
     // exchange images
-    Image *img = mAnimation->getCurrentImage();
+    Image *img = mAnimation.getCurrentImage();
     if (img != mLastImage)
     {
-        for (auto &affected : mAffected)
+        for (auto &[layer, index] : mAffected)
         {
-            affected.first->setTile(affected.second, img);
+            layer->setTile(index, img);
         }
         mLastImage = img;
     }
@@ -187,10 +178,9 @@ void MapLayer::draw(Graphics *graphics,
     // Draw any remaining actors
     if (mIsFringeLayer)
     {
-        while (ai != actors.end())
+        for (; ai != actors.end(); ++ai)
         {
             (*ai)->draw(graphics, -scrollX, -scrollY);
-            ai++;
         }
     }
 }
@@ -247,7 +237,6 @@ Map::~Map()
     delete_all(mTilesets);
     delete_all(mForegrounds);
     delete_all(mBackgrounds);
-    delete_all(mTileAnimations);
 }
 
 void Map::initializeAmbientLayers()
@@ -320,9 +309,9 @@ bool actorCompare(const Actor *a, const Actor *b)
 void Map::update(int ticks)
 {
     // Update animated tiles
-    for (auto &tileAnimation : mTileAnimations)
+    for (auto &[_, tileAnimation] : mTileAnimations)
     {
-        tileAnimation.second->update(ticks);
+        tileAnimation.update(ticks);
     }
 }
 
@@ -348,38 +337,22 @@ void Map::draw(Graphics *graphics, int scrollX, int scrollY)
                       config.getIntValue("OverlayDetail"));
 
     // draw the game world
-    Layers::const_iterator layeri = mLayers.begin();
-
-    bool overFringe = false;
-
-    if (mDebugFlags & DEBUG_SPECIAL3)
-    {
-        for (; layeri != mLayers.end(); ++layeri)
-        {
-            if ((*layeri)->isFringeLayer())
-            {
-                (*layeri)->draw(graphics,
-                                startX, startY, endX, endY,
-                                scrollX, scrollY,
-                                mActors, mDebugFlags);
-            }
-        }
-    }
-    else
+    for (auto &layer : mLayers)
     {
-        for (; layeri != mLayers.end() && !overFringe; ++layeri)
-        {
-            if (((*layeri)->getMask() & mMask) == 0)
-                continue;
+        if ((layer->getMask() & mMask) == 0)
+            continue;
+
+        if (!layer->isFringeLayer() && (mDebugFlags & DEBUG_SPECIAL3))
+            continue;
 
-            if ((*layeri)->isFringeLayer() && (mDebugFlags & DEBUG_SPECIAL2))
-                overFringe = true;
+        layer->draw(graphics,
+                    startX, startY, endX, endY,
+                    scrollX, scrollY,
+                    mActors, mDebugFlags);
 
-            (*layeri)->draw(graphics,
-                            startX, startY, endX, endY,
-                            scrollX, scrollY,
-                            mActors, mDebugFlags);
-        }
+        if (layer->isFringeLayer() && (mDebugFlags & (DEBUG_SPECIAL2 |
+                                                      DEBUG_SPECIAL3)))
+            break;
     }
 
     // If the transparency hasn't been disabled,
@@ -387,20 +360,15 @@ void Map::draw(Graphics *graphics, int scrollX, int scrollY)
     {
         // We draw beings with a lower opacity to make them visible
         // even when covered by a wall or some other elements...
-        auto ai = mActors.begin();
-        while (ai != mActors.end())
+        for (auto actor : mActors)
         {
-            if (Actor *actor = *ai)
+            // For now, just draw actors with only one layer.
+            if (actor->drawnWhenBehind())
             {
-                // For now, just draw actors with only one layer.
-                if (actor->drawnWhenBehind())
-                {
-                    actor->setAlpha(0.3f);
-                    actor->draw(graphics, -scrollX, -scrollY);
-                    actor->setAlpha(1.0f);
-                }
+                actor->setAlpha(0.3f);
+                actor->draw(graphics, -scrollX, -scrollY);
+                actor->setAlpha(1.0f);
             }
-            ai++;
         }
     }
 
@@ -409,7 +377,7 @@ void Map::draw(Graphics *graphics, int scrollX, int scrollY)
 }
 
 void Map::drawCollision(Graphics *graphics, int scrollX, int scrollY,
-                        int debugFlags)
+                        int debugFlags) const
 {
     int endPixelY = graphics->getHeight() + scrollY + mTileHeight - 1;
     int startX = scrollX / mTileWidth;
@@ -602,7 +570,7 @@ bool Map::occupied(int x, int y) const
     return false;
 }
 
-Vector Map::getTileCenter(int x, int y)
+Vector Map::getTileCenter(int x, int y) const
 {
     Vector tileCenterPos;
 
@@ -810,7 +778,7 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
     startTile->Gcost = 0;
 
     // Add the start point to the open list
-    openList.push(Location(startX, startY, startTile));
+    openList.emplace(startX, startY, startTile);
 
     bool foundPath = false;
 
@@ -907,7 +875,8 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
                        work reliably if the heuristic cost is higher than the
                        real cost. In particular, using Manhattan distance is
                        forbidden here. */
-                    int dx = std::abs(x - destX), dy = std::abs(y - destY);
+                    int dx = std::abs(x - destX);
+                    int dy = std::abs(y - destY);
                     newTile->Hcost = std::abs(dx - dy) * basicCost +
                         std::min(dx, dy) * (basicCost * 362 / 256);
 
@@ -923,7 +892,7 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
                     {
                         // Add this tile to the open list
                         newTile->whichList = mOnOpenList;
-                        openList.push(Location(x, y, newTile));
+                        openList.emplace(x, y, newTile);
                     }
                     else
                     {
@@ -944,7 +913,7 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
 
                     // Add this tile to the open list (it's already
                     // there, but this instance has a lower F score)
-                    openList.push(Location(x, y, newTile));
+                    openList.emplace(x, y, newTile);
                 }
             }
         }
@@ -979,7 +948,7 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
         while (pathX != startX || pathY != startY)
         {
             // Add the new path node to the start of the path list
-            path.push_front(Position(pathX, pathY));
+            path.emplace_front(pathX, pathY);
 
             // Find out the next parent
             MetaTile *tile = getMetaTile(pathX, pathY);
@@ -994,25 +963,24 @@ Path Map::findPath(int startX, int startY, int destX, int destY,
 void Map::addParticleEffect(const std::string &effectFile, int x, int y, int w,
                             int h)
 {
-    ParticleEffectData newEffect;
+    ParticleEffectData &newEffect = particleEffects.emplace_back();
     newEffect.file = effectFile;
     newEffect.x = x;
     newEffect.y = y;
     newEffect.w = w;
     newEffect.h = h;
-    particleEffects.push_back(newEffect);
-
 }
 
 void Map::initializeParticleEffects(Particle *particleEngine)
 {
-    Particle *p;
-
     if (config.getBoolValue("particleeffects"))
     {
         for (auto &particleEffect : particleEffects)
         {
-            p = particleEngine->addEffect(particleEffect.file, particleEffect.x, particleEffect.y);
+            Particle *p = particleEngine->addEffect(particleEffect.file,
+                                                    particleEffect.x,
+                                                    particleEffect.y);
+
             if (p && particleEffect.w > 0 && particleEffect.h > 0)
             {
                 p->adjustEmitterSize(particleEffect.w, particleEffect.h);
@@ -1021,10 +989,19 @@ void Map::initializeParticleEffects(Particle *particleEngine)
     }
 }
 
-TileAnimation *Map::getAnimationForGid(int gid) const
+void Map::addAnimation(int gid, TileAnimation animation)
+{
+    auto const [_, inserted] = mTileAnimations.try_emplace(gid, std::move(animation));
+    if (!inserted)
+    {
+        logger->error(strprintf("Duplicate tile animation for gid %d", gid));
+    }
+}
+
+TileAnimation *Map::getAnimationForGid(int gid)
 {
     auto i = mTileAnimations.find(gid);
-    return (i == mTileAnimations.end()) ? NULL : i->second;
+    return i == mTileAnimations.end() ? nullptr : &i->second;
 }
 
 void Map::setMask(int mask)
diff --git a/src/map.h b/src/map.h
index ae0b07a8..97d3df62 100644
--- a/src/map.h
+++ b/src/map.h
@@ -25,21 +25,17 @@
 #include "actor.h"
 #include "position.h"
 #include "properties.h"
+#include "simpleanimation.h"
 
 #include <list>
 #include <vector>
 
-class Animation;
 class AmbientLayer;
 class Graphics;
 class MapLayer;
 class Particle;
-class SimpleAnimation;
 class Tileset;
 
-using Tilesets = std::vector<Tileset *>;
-using Layers = std::vector<MapLayer *>;
-
 const int DEFAULT_TILE_LENGTH = 32;
 
 /**
@@ -65,15 +61,17 @@ struct MetaTile
 class TileAnimation
 {
     public:
-        TileAnimation(Animation *ani);
-        ~TileAnimation();
+        TileAnimation(Animation animation);
+
         void update(int ticks = 1);
+
         void addAffectedTile(MapLayer *layer, int index)
         { mAffected.emplace_back(layer, index); }
+
     private:
-        std::list<std::pair<MapLayer*, int> > mAffected;
-        SimpleAnimation *mAnimation;
-        Image *mLastImage;
+        std::vector<std::pair<MapLayer*, int> > mAffected;
+        SimpleAnimation mAnimation;
+        Image *mLastImage = nullptr;
 };
 
 /**
@@ -212,7 +210,7 @@ class Map : public Properties
          * Visualizes collision layer for debugging
          */
         void drawCollision(Graphics *graphics, int scrollX, int scrollY,
-                           int debugFlags);
+                           int debugFlags) const;
 
         /**
          * Adds a layer to this map. The map takes ownership of the layer.
@@ -279,7 +277,7 @@ class Map : public Properties
          * @param x the horizontal tile position
          * @param y the vertical tile position
          */
-        Vector getTileCenter(int x, int y);
+        Vector getTileCenter(int x, int y) const;
 
         std::string getMusicFile() const;
         std::string getName() const;
@@ -327,8 +325,7 @@ class Map : public Properties
         /**
          * Adds a tile animation to the map
          */
-        void addAnimation(int gid, TileAnimation *animation)
-        { mTileAnimations[gid] = animation; }
+        void addAnimation(int gid, TileAnimation animation);
 
         void setDebugFlags(int flags) { mDebugFlags = flags; }
 
@@ -337,7 +334,7 @@ class Map : public Properties
         /**
          * Gets the tile animation for a specific gid
          */
-        TileAnimation *getAnimationForGid(int gid) const;
+        TileAnimation *getAnimationForGid(int gid);
 
         void setMask(int mask);
 
@@ -392,8 +389,8 @@ class Map : public Properties
         int mTileWidth, mTileHeight;
         int mMaxTileHeight, mMaxTileWidth;
         MetaTile *mMetaTiles;
-        Layers mLayers;
-        Tilesets mTilesets;
+        std::vector<MapLayer *> mLayers;
+        std::vector<Tileset *> mTilesets;
         Actors mActors;
 
         // debug flags
@@ -419,7 +416,7 @@ class Map : public Properties
         };
         std::list<ParticleEffectData> particleEffects;
 
-        std::map<int, TileAnimation*> mTileAnimations;
+        std::map<int, TileAnimation> mTileAnimations;
 
         int mMask = 1;
 };
diff --git a/src/particleemitter.cpp b/src/particleemitter.cpp
index 81258d80..196a43d2 100644
--- a/src/particleemitter.cpp
+++ b/src/particleemitter.cpp
@@ -464,13 +464,11 @@ std::list<Particle *> ParticleEmitter::createParticles(int tick)
         }
         else if (mParticleRotation.getLength() > 0)
         {
-            auto *newAnimation = new Animation(mParticleRotation);
-            newParticle = new RotationalParticle(mMap, newAnimation);
+            newParticle = new RotationalParticle(mMap, mParticleRotation);
         }
         else if (mParticleAnimation.getLength() > 0)
         {
-            auto *newAnimation = new Animation(mParticleAnimation);
-            newParticle = new AnimationParticle(mMap, newAnimation);
+            newParticle = new AnimationParticle(mMap, mParticleAnimation);
         }
         else
         {
diff --git a/src/resources/animation.cpp b/src/resources/animation.cpp
index 1be27c2c..b48e8cff 100644
--- a/src/resources/animation.cpp
+++ b/src/resources/animation.cpp
@@ -23,8 +23,12 @@
 
 void Animation::addFrame(Image *image, int delay, int offsetX, int offsetY)
 {
-    Frame frame = { image, delay, offsetX, offsetY };
-    mFrames.push_back(frame);
+    auto &frame = mFrames.emplace_back();
+    frame.image = image;
+    frame.delay = delay;
+    frame.offsetX = offsetX;
+    frame.offsetY = offsetY;
+
     mDuration += delay;
 }
 
diff --git a/src/resources/animation.h b/src/resources/animation.h
index 59601dc4..7fc14857 100644
--- a/src/resources/animation.h
+++ b/src/resources/animation.h
@@ -43,7 +43,7 @@ struct Frame
  * An animation consists of several frames, each with their own delay and
  * offset.
  */
-class Animation
+class Animation final
 {
     public:
         Animation() = default;
diff --git a/src/resources/mapreader.cpp b/src/resources/mapreader.cpp
index 8479cc6c..2136ed58 100644
--- a/src/resources/mapreader.cpp
+++ b/src/resources/mapreader.cpp
@@ -37,8 +37,19 @@
 
 #include <iostream>
 
-// DO NOT CHANGE THESE STRINGS TO BE PASSED BY REFERENCE, AS THIS METHOD ALTERS
-// (THAT IS, DESTROYS) THEM.
+static void readProperties(xmlNodePtr node, Properties* props);
+
+static void readLayer(xmlNodePtr node, Map *map);
+
+static Tileset *readTileset(xmlNodePtr node,
+                            const std::string &path,
+                            Map *map);
+
+static void readTileAnimation(xmlNodePtr tileNode,
+                              Tileset *set,
+                              unsigned tileGID,
+                              Map *map);
+
 static std::string resolveRelativePath(std::string base, std::string relative)
 {
     // Remove trailing "/", if present
@@ -91,7 +102,8 @@ Map *MapReader::readMap(const std::string &filename)
         logger->log("Error while parsing map file (%s)!", filename.c_str());
     }
 
-    if (map) map->setProperty("_filename", filename);
+    if (map)
+        map->setProperty("_filename", filename);
 
     return map;
 }
@@ -204,7 +216,14 @@ Map *MapReader::readMap(xmlNodePtr node, const std::string &path)
     return map;
 }
 
-void MapReader::readProperties(xmlNodePtr node, Properties *props)
+/**
+ * Reads the properties element.
+ *
+ * @param node  The <code>properties</code> element.
+ * @param props The Properties instance to which the properties will
+ *              be assigned.
+ */
+static void readProperties(xmlNodePtr node, Properties *props)
 {
     for_each_xml_child_node(childNode, node)
     {
@@ -251,7 +270,10 @@ static void setTile(Map *map, MapLayer *layer, int x, int y, unsigned gid)
     }
 }
 
-void MapReader::readLayer(xmlNodePtr node, Map *map)
+/**
+ * Reads a map layer and adds it to the given map.
+ */
+static void readLayer(xmlNodePtr node, Map *map)
 {
     // Layers are not necessarily the same size as the map
     const int w = XML::getProperty(node, "width", map->getWidth());
@@ -319,17 +341,17 @@ void MapReader::readLayer(xmlNodePtr node, Map *map)
 
             // Read base64 encoded map file
             xmlNodePtr dataChild = childNode->xmlChildrenNode;
-            if (!dataChild)
+            if (!dataChild || !dataChild->content)
                 continue;
 
-            int len = strlen((const char*)dataChild->content) + 1;
-            auto *charData = new unsigned char[len + 1];
-            const char *charStart = (const char*) xmlNodeGetContent(dataChild);
+            auto *charStart = reinterpret_cast<const char*>(dataChild->content);
+            auto *charData = new unsigned char[strlen(charStart) + 1];
             unsigned char *charIndex = charData;
 
             while (*charStart)
             {
-                if (*charStart != ' ' && *charStart != '\t' &&
+                if (*charStart != ' ' &&
+                    *charStart != '\t' &&
                     *charStart != '\n')
                 {
                     *charIndex = *charStart;
@@ -341,7 +363,9 @@ void MapReader::readLayer(xmlNodePtr node, Map *map)
 
             int binLen;
             unsigned char *binData =
-                php3_base64_decode(charData, strlen((char*)charData), &binLen);
+                php3_base64_decode(charData,
+                                   strlen(reinterpret_cast<const char*>(charData)),
+                                   &binLen);
 
             delete[] charData;
 
@@ -389,21 +413,29 @@ void MapReader::readLayer(xmlNodePtr node, Map *map)
         }
         else if (encoding == "csv")
         {
-            xmlNodePtr dataChild = childNode->xmlChildrenNode;
-            if (!dataChild)
+            if (!childNode->children || !childNode->children->content)
+            {
+                logger->log("Error: CSV layer data is empty!");
                 continue;
+            }
 
-            const char *data = (const char*) xmlNodeGetContent(dataChild);
-            std::string csv(data);
-
-            size_t pos = 0;
-            size_t oldPos = 0;
+            xmlChar *data = childNode->children->content;
+            auto *pos = reinterpret_cast<const char*>(data);
 
-            while (oldPos != csv.npos)
+            for (;;)
             {
-                pos = csv.find_first_of(",", oldPos);
-
-                unsigned gid = atol(csv.substr(oldPos, pos - oldPos).c_str());
+                // Try to parse the next number at 'pos'
+                errno = 0;
+                char *end;
+                unsigned gid = strtol(pos, &end, 10);
+                if (pos == end) // No number found
+                    break;
+
+                if (errno == ERANGE)
+                {
+                    logger->log("Error: Range error in tile layer data!");
+                    break;
+                }
 
                 setTile(map, layer, x, y, gid);
 
@@ -417,7 +449,14 @@ void MapReader::readLayer(xmlNodePtr node, Map *map)
                         break;
                 }
 
-                oldPos = pos + 1;
+                // Skip the comma, or break if we're done
+                pos = strchr(end, ',');
+                if (!pos)
+                {
+                    logger->log("Error: CSV layer data too short!");
+                    break;
+                }
+                ++pos;
             }
         }
         else
@@ -451,13 +490,16 @@ void MapReader::readLayer(xmlNodePtr node, Map *map)
     }
 }
 
-Tileset *MapReader::readTileset(xmlNodePtr node, const std::string &path,
-                                Map *map)
+/**
+ * Reads a tile set.
+ */
+static Tileset *readTileset(xmlNodePtr node, const std::string &path,
+                            Map *map)
 {
     unsigned firstGid = XML::getProperty(node, "firstgid", 0);
     int margin = XML::getProperty(node, "margin", 0);
     int spacing = XML::getProperty(node, "spacing", 0);
-    XML::Document* doc = nullptr;
+    XML::Document *doc = nullptr;
     Tileset *set = nullptr;
     std::string pathDir(path);
 
@@ -487,7 +529,7 @@ Tileset *MapReader::readTileset(xmlNodePtr node, const std::string &path,
                 std::string sourceStr = resolveRelativePath(pathDir, source);
 
                 ResourceManager *resman = ResourceManager::getInstance();
-                Image* tilebmp = resman->getImage(sourceStr);
+                Image *tilebmp = resman->getImage(sourceStr);
 
                 if (tilebmp)
                 {
@@ -502,49 +544,14 @@ Tileset *MapReader::readTileset(xmlNodePtr node, const std::string &path,
                 }
             }
         }
-        else if (xmlStrEqual(childNode->name, BAD_CAST "tile"))
+        else if (set && xmlStrEqual(childNode->name, BAD_CAST "tile"))
         {
+            const int tileGID = firstGid + XML::getProperty(childNode, "id", 0);
+
             for_each_xml_child_node(tileNode, childNode)
             {
-                if (!xmlStrEqual(tileNode->name, BAD_CAST "properties")) continue;
-
-                int tileGID = firstGid + XML::getProperty(childNode, "id", 0);
-
-                // read tile properties to a map for simpler handling
-                std::map<std::string, int> tileProperties;
-                for_each_xml_child_node(propertyNode, tileNode)
-                {
-                    if (!xmlStrEqual(propertyNode->name, BAD_CAST "property")) continue;
-                    std::string name = XML::getProperty(propertyNode, "name", "");
-                    int value = XML::getProperty(propertyNode, "value", 0);
-                    tileProperties[name] = value;
-                    logger->log("Tile Prop of %d \"%s\" = \"%d\"", tileGID, name.c_str(), value);
-                }
-
-                // create animation
-                if (!set) continue;
-
-                auto *ani = new Animation;
-                for (int i = 0; ;i++)
-                {
-                    std::map<std::string, int>::iterator iFrame, iDelay;
-                    iFrame = tileProperties.find("animation-frame" + toString(i));
-                    iDelay = tileProperties.find("animation-delay" + toString(i));
-                    if (iFrame != tileProperties.end() && iDelay != tileProperties.end())
-                        ani->addFrame(set->get(iFrame->second), iDelay->second, 0, 0);
-                    else
-                        break;
-                }
-
-                if (ani->getLength() > 0)
-                {
-                    map->addAnimation(tileGID, new TileAnimation(ani));
-                    logger->log("Animation length: %d", ani->getLength());
-                }
-                else
-                {
-                    delete ani;
-                }
+                if (xmlStrEqual(tileNode->name, BAD_CAST "animation"))
+                    readTileAnimation(tileNode, set, tileGID, map);
             }
         }
     }
@@ -553,3 +560,23 @@ Tileset *MapReader::readTileset(xmlNodePtr node, const std::string &path,
 
     return set;
 }
+
+static void readTileAnimation(xmlNodePtr tileNode,
+                              Tileset *set,
+                              unsigned tileGID,
+                              Map *map)
+{
+    Animation ani;
+    for_each_xml_child_node(frameNode, tileNode)
+    {
+        if (xmlStrEqual(frameNode->name, BAD_CAST "frame"))
+        {
+            const int tileId = XML::getProperty(frameNode, "tileid", 0);
+            const int duration = XML::getProperty(frameNode, "duration", 0) / 10;
+            ani.addFrame(set->get(tileId), duration, 0, 0);
+        }
+    }
+
+    if (ani.getLength() > 0)
+        map->addAnimation(tileGID, TileAnimation(std::move(ani)));
+}
diff --git a/src/resources/mapreader.h b/src/resources/mapreader.h
index 70a5ef23..9244da73 100644
--- a/src/resources/mapreader.h
+++ b/src/resources/mapreader.h
@@ -27,46 +27,23 @@
 #include <string>
 
 class Map;
-class Properties;
-class Tileset;
 
 /**
  * Reader for XML map files (*.tmx)
  */
 class MapReader
 {
-    public:
-        /**
-         * Read an XML map from a file.
-         */
-        static Map *readMap(const std::string &filename);
-
-        /**
-         * Read an XML map from a parsed XML tree. The path is used to find the
-         * location of referenced tileset images.
-         */
-        static Map *readMap(xmlNodePtr node, const std::string &path);
-
-    private:
-        /**
-         * Reads the properties element.
-         *
-         * @param node  The <code>properties</code> element.
-         * @param props The Properties instance to which the properties will
-         *              be assigned.
-         */
-        static void readProperties(xmlNodePtr node, Properties* props);
-
-        /**
-         * Reads a map layer and adds it to the given map.
-         */
-        static void readLayer(xmlNodePtr node, Map *map);
-
-        /**
-         * Reads a tile set.
-         */
-        static Tileset *readTileset(xmlNodePtr node, const std::string &path,
-                                    Map *map);
+public:
+    /**
+     * Read an XML map from a file.
+     */
+    static Map *readMap(const std::string &filename);
+
+    /**
+     * Read an XML map from a parsed XML tree. The path is used to find the
+     * location of referenced tileset images.
+     */
+    static Map *readMap(xmlNodePtr node, const std::string &path);
 };
 
-#endif
+#endif // MAPREADER_H
diff --git a/src/rotationalparticle.cpp b/src/rotationalparticle.cpp
index c7c70974..47aa8f78 100644
--- a/src/rotationalparticle.cpp
+++ b/src/rotationalparticle.cpp
@@ -24,22 +24,20 @@
 
 #define PI 3.14159265
 
-RotationalParticle::RotationalParticle(Map *map, Animation *animation):
+RotationalParticle::RotationalParticle(Map *map, Animation animation):
     ImageParticle(map, nullptr),
-    mAnimation(new SimpleAnimation(animation))
-{
-}
+    mAnimation(std::move(animation))
+{}
 
 RotationalParticle::RotationalParticle(Map *map, xmlNodePtr animationNode,
-                                       const std::string& dyePalettes):
+                                       const std::string &dyePalettes):
     ImageParticle(map, nullptr),
-    mAnimation(new SimpleAnimation(animationNode, dyePalettes))
-{
-}
+    mAnimation(animationNode, dyePalettes)
+{}
 
 RotationalParticle::~RotationalParticle()
 {
-    delete mAnimation;
+    // Prevent ImageParticle from decreasing the reference count of the image
     mImage = nullptr;
 }
 
@@ -50,13 +48,13 @@ bool RotationalParticle::update()
     float rad = atan2(mVelocity.x, mVelocity.y);
     if (rad < 0)
         rad = PI + (PI + rad);
-    int size = mAnimation->getLength();
+    int size = mAnimation.getLength();
     float range = PI / size;
 
     // Determines which frame the particle should play
     if (rad < range || rad > ((PI*2) - range))
     {
-        mAnimation->setFrame(0);
+        mAnimation.setFrame(0);
     }
     else
     {
@@ -64,13 +62,13 @@ bool RotationalParticle::update()
         {
             if (((c * (2 * range)) - range) < rad && rad < ((c * (2 * range)) + range))
             {
-                mAnimation->setFrame(c);
+                mAnimation.setFrame(c);
                 break;
             }
         }
     }
 
-    mImage = mAnimation->getCurrentImage();
+    mImage = mAnimation.getCurrentImage();
 
     return Particle::update();
 }
diff --git a/src/rotationalparticle.h b/src/rotationalparticle.h
index 465be989..cf068835 100644
--- a/src/rotationalparticle.h
+++ b/src/rotationalparticle.h
@@ -23,6 +23,7 @@
 #define ROTATIONAL_PARTICLE_H
 
 #include "imageparticle.h"
+#include "simpleanimation.h"
 
 #include <libxml/tree.h>
 
@@ -33,17 +34,17 @@ class SimpleAnimation;
 class RotationalParticle : public ImageParticle
 {
     public:
-        RotationalParticle(Map *map, Animation *animation);
+        RotationalParticle(Map *map, Animation animation);
 
         RotationalParticle(Map *map, xmlNodePtr animationNode,
-                           const std::string& dyePalettes = std::string());
+                           const std::string &dyePalettes = std::string());
 
         ~RotationalParticle() override;
 
         bool update() override;
 
     private:
-        SimpleAnimation *mAnimation; /**< Used animation for this particle */
+        SimpleAnimation mAnimation; /**< Used animation for this particle */
 };
 
 #endif
diff --git a/src/simpleanimation.cpp b/src/simpleanimation.cpp
index 72045ca9..8496f3b1 100644
--- a/src/simpleanimation.cpp
+++ b/src/simpleanimation.cpp
@@ -31,29 +31,18 @@
 #include "resources/imageset.h"
 #include "resources/resourcemanager.h"
 
-SimpleAnimation::SimpleAnimation(Animation *animation):
-    mAnimation(animation),
-    mAnimationTime(0),
-    mAnimationPhase(0),
-    mCurrentFrame(mAnimation->getFrame(0)),
+SimpleAnimation::SimpleAnimation(Animation animation):
+    mAnimation(std::move(animation)),
+    mCurrentFrame(mAnimation.getFrame(0)),
     mInitialized(true)
 {
 }
 
 SimpleAnimation::SimpleAnimation(xmlNodePtr animationNode,
-                                 const std::string& dyePalettes):
-    mAnimation(new Animation),
-    mAnimationTime(0),
-    mAnimationPhase(0),
-    mInitialized(false)
+                                 const std::string &dyePalettes)
 {
     initializeAnimation(animationNode, dyePalettes);
-    mCurrentFrame = mAnimation->getFrame(0);
-}
-
-SimpleAnimation::~SimpleAnimation()
-{
-    delete mAnimation;
+    mCurrentFrame = mAnimation.getFrame(0);
 }
 
 bool SimpleAnimation::draw(Graphics *graphics, int posX, int posY) const
@@ -76,10 +65,10 @@ void SimpleAnimation::setFrame(int frame)
 {
     if (frame < 0)
         frame = 0;
-    if (frame >= mAnimation->getLength())
-        frame = mAnimation->getLength() - 1;
+    if (frame >= mAnimation.getLength())
+        frame = mAnimation.getLength() - 1;
     mAnimationPhase = frame;
-    mCurrentFrame = mAnimation->getFrame(mAnimationPhase);
+    mCurrentFrame = mAnimation.getFrame(mAnimationPhase);
 }
 
 void SimpleAnimation::update(int timePassed)
@@ -93,35 +82,29 @@ void SimpleAnimation::update(int timePassed)
             mAnimationTime -= mCurrentFrame->delay;
             mAnimationPhase++;
 
-            if (mAnimationPhase >= mAnimation->getLength())
+            if (mAnimationPhase >= mAnimation.getLength())
                 mAnimationPhase = 0;
 
-            mCurrentFrame = mAnimation->getFrame(mAnimationPhase);
+            mCurrentFrame = mAnimation.getFrame(mAnimationPhase);
         }
     }
 }
 
 int SimpleAnimation::getLength() const
 {
-    if (mAnimation)
-        return mAnimation->getLength();
-    else
-        return 0;
+    return mAnimation.getLength();
 }
 
 Image *SimpleAnimation::getCurrentImage() const
 {
     if (mCurrentFrame)
         return mCurrentFrame->image;
-    else
-        return nullptr;
+    return nullptr;
 }
 
 void SimpleAnimation::initializeAnimation(xmlNodePtr animationNode,
                                           const std::string& dyePalettes)
 {
-    mInitialized = false;
-
     if (!animationNode)
         return;
 
@@ -174,7 +157,7 @@ void SimpleAnimation::initializeAnimation(xmlNodePtr animationNode,
                 continue;
             }
 
-            mAnimation->addFrame(img, delay, offsetX, offsetY);
+            mAnimation.addFrame(img, delay, offsetX, offsetY);
         }
         else if (xmlStrEqual(frameNode->name, BAD_CAST "sequence"))
         {
@@ -197,13 +180,13 @@ void SimpleAnimation::initializeAnimation(xmlNodePtr animationNode,
                     continue;
                 }
 
-                mAnimation->addFrame(img, delay, offsetX, offsetY);
+                mAnimation.addFrame(img, delay, offsetX, offsetY);
                 start++;
             }
         }
         else if (xmlStrEqual(frameNode->name, BAD_CAST "end"))
         {
-            mAnimation->addTerminator();
+            mAnimation.addTerminator();
         }
     }
 
diff --git a/src/simpleanimation.h b/src/simpleanimation.h
index 558e59ce..a8fb4cba 100644
--- a/src/simpleanimation.h
+++ b/src/simpleanimation.h
@@ -22,33 +22,30 @@
 #ifndef SIMPLEANIMAION_H
 #define SIMPLEANIMAION_H
 
+#include "resources/animation.h"
+
 #include "utils/xml.h"
 
-class Animation;
-class Frame;
 class Graphics;
-class Image;
 
 /**
  * This class is a leightweight alternative to the AnimatedSprite class.
  * It hosts a looping animation without actions and directions.
  */
-class SimpleAnimation
+class SimpleAnimation final
 {
     public:
         /**
          * Creates a simple animation with an already created \a animation.
          * Takes ownership over the given animation.
          */
-        SimpleAnimation(Animation *animation);
+        SimpleAnimation(Animation animation);
 
         /**
          * Creates a simple animation that creates its animation from XML Data.
          */
         SimpleAnimation(xmlNodePtr animationNode,
-                        const std::string& dyePalettes = std::string());
-
-        ~SimpleAnimation();
+                        const std::string &dyePalettes = std::string());
 
         void setFrame(int frame);
 
@@ -70,19 +67,19 @@ class SimpleAnimation
                                 const std::string& dyePalettes = std::string());
 
         /** The hosted animation. */
-        Animation *mAnimation;
+        Animation mAnimation;
 
         /** Time in game ticks the current frame is shown. */
-        int mAnimationTime;
+        int mAnimationTime = 0;
 
         /** Index of current animation phase. */
-        int mAnimationPhase;
+        int mAnimationPhase = 0;
 
         /** Current animation phase. */
-        Frame *mCurrentFrame;
+        Frame *mCurrentFrame = nullptr;
 
         /**  Tell whether the animation is ready */
-        bool mInitialized;
+        bool mInitialized = false;
 };
 
 #endif