Use ResourceRef<Image> in more places

Automatic reference counting of images is now used by Item, Icon,
AnimatedSprite, ImageSprite, ParticleEmitter, Minimap, Desktop and
Emote.

Since ResourceManager::get automatically adds a reference, it needs to
be explicitly subtracted when the resource is managed by ResourceRef.
This is taken care of by the new ResourceManager::getImageRef.

Also removed the apprently unused and duplicate "mDrawImage" from Item
(which also didn't get decRef called on it).

Fixes cleanup of emote ImageSet and ImageSprite instances, as well as
particle images.

6 files changed, 68 insertions, 26 deletions

diff --git a/src/resources/emotedb.cpp b/src/resources/emotedb.cpp
index e1dd8f4a..b0536a0e 100644
--- a/src/resources/emotedb.cpp
+++ b/src/resources/emotedb.cpp
@@ -44,8 +44,8 @@ void EmoteDB::init()
 
     mUnknown.name = "unknown";
     mUnknown.effectId = -1;
-    mUnknown.sprite = new ImageSprite(
-        ResourceManager::getInstance()->getImage("graphics/sprites/error.png"));
+    mUnknown.sprite = std::make_unique<ImageSprite>(
+        ResourceManager::getInstance()->getImageRef("graphics/sprites/error.png"));
 }
 
 void EmoteDB::readEmoteNode(xmlNodePtr node, const std::string &filename)
@@ -57,16 +57,16 @@ void EmoteDB::readEmoteNode(xmlNodePtr node, const std::string &filename)
         return;
     }
 
-    Emote currentEmote;
+    Emote emote;
 
-    currentEmote.id = id;
-    currentEmote.name = XML::getProperty(node, "name", "unknown");
-    currentEmote.effectId = XML::getProperty(node, "effectid", -1);
+    emote.id = id;
+    emote.name = XML::getProperty(node, "name", "unknown");
+    emote.effectId = XML::getProperty(node, "effectid", -1);
 
-    if (currentEmote.effectId == -1)
+    if (emote.effectId == -1)
     {
         logger->log("Emote Database: Warning: Emote %s has no attached effect in %s!",
-                    currentEmote.name.c_str(), filename.c_str());
+                    emote.name.c_str(), filename.c_str());
         return;
     }
 
@@ -77,25 +77,26 @@ void EmoteDB::readEmoteNode(xmlNodePtr node, const std::string &filename)
     if (imageName.empty() || width <= 0 || height <= 0)
     {
         logger->log("Emote Database: Warning: Emote %s has bad imageset values in %s",
-                    currentEmote.name.c_str(), filename.c_str());
+                    emote.name.c_str(), filename.c_str());
         return;
     }
 
-    ImageSet *is = ResourceManager::getInstance()->getImageSet(imageName,
-                                                               width,
-                                                               height);
-    if (!is || is->size() == 0)
+    emote.is = ResourceManager::getInstance()->getImageSet(imageName,
+                                                           width,
+                                                           height);
+    emote.is->decRef(); // clear automatic reference
+
+    if (!emote.is || emote.is->size() == 0)
     {
         logger->log("Emote Database: Error loading imageset for emote %s in %s",
-                    currentEmote.name.c_str(), filename.c_str());
-        delete is;
+                    emote.name.c_str(), filename.c_str());
         return;
     }
 
     // For now we just use the first image in the animation
-    currentEmote.sprite = new ImageSprite(is->get(0));
+    emote.sprite = std::make_unique<ImageSprite>(emote.is->get(0));
 
-    mEmotes.push_back(std::move(currentEmote));
+    mEmotes.push_back(std::move(emote));
 }
 
 void EmoteDB::checkStatus()
@@ -105,7 +106,9 @@ void EmoteDB::checkStatus()
 
 void EmoteDB::unload()
 {
-    // todo: don't we need to delete the sprites?
+    for (auto &emote : mEmotes)
+        emote.sprite->releaseImageRef();
+
     mEmotes.clear();
     mLoaded = false;
 }
diff --git a/src/resources/emotedb.h b/src/resources/emotedb.h
index 1ed86a05..a6f93211 100644
--- a/src/resources/emotedb.h
+++ b/src/resources/emotedb.h
@@ -22,10 +22,14 @@
 #ifndef EMOTE_DB_H
 #define EMOTE_DB_H
 
+#include <memory>
 #include <string>
 
+#include "resources/resource.h"
+
 #include "utils/xml.h"
 
+class ImageSet;
 class ImageSprite;
 
 struct Emote
@@ -33,7 +37,8 @@ struct Emote
     int id;
     int effectId;
     std::string name;
-    ImageSprite *sprite;
+    ResourceRef<ImageSet> is;
+    std::unique_ptr<ImageSprite> sprite;
 };
 
 /**
diff --git a/src/resources/mapreader.cpp b/src/resources/mapreader.cpp
index 2136ed58..b49195fc 100644
--- a/src/resources/mapreader.cpp
+++ b/src/resources/mapreader.cpp
@@ -529,13 +529,12 @@ static Tileset *readTileset(xmlNodePtr node, const std::string &path,
                 std::string sourceStr = resolveRelativePath(pathDir, source);
 
                 ResourceManager *resman = ResourceManager::getInstance();
-                Image *tilebmp = resman->getImage(sourceStr);
+                auto tilebmp = resman->getImageRef(sourceStr);
 
                 if (tilebmp)
                 {
                     set = new Tileset(tilebmp, tw, th, firstGid, margin,
                                       spacing);
-                    tilebmp->decRef();
                 }
                 else
                 {
diff --git a/src/resources/resource.h b/src/resources/resource.h
index 9fe00f3d..e1f37d73 100644
--- a/src/resources/resource.h
+++ b/src/resources/resource.h
@@ -120,6 +120,20 @@ public:
         return *this;
     }
 
+    // Move assignment operator
+    ResourceRef &operator=(ResourceRef &&other)
+    {
+        if (this != &other)
+        {
+            if (mResource)
+                mResource->decRef();
+
+            mResource = other.mResource;
+            other.mResource = nullptr;
+        }
+        return *this;
+    }
+
     // Allow dereferencing
     RESOURCE *operator->() const
     { return mResource; }
@@ -131,6 +145,15 @@ public:
     operator RESOURCE *() const
     { return mResource; }
 
+    /**
+     * Releases the resource without decrementing the reference count!
+     *
+     * This is currently necessary to avoid calls to decRef on instances of
+     * SubImage, which are not reference counted resources.
+     */
+    void release()
+    { mResource = nullptr; }
+
 private:
     RESOURCE *mResource;
 };
diff --git a/src/resources/resourcemanager.cpp b/src/resources/resourcemanager.cpp
index f43aea41..e6600236 100644
--- a/src/resources/resourcemanager.cpp
+++ b/src/resources/resourcemanager.cpp
@@ -310,6 +310,13 @@ Image *ResourceManager::getImage(const std::string &idPath)
     }));
 }
 
+ResourceRef<Image> ResourceManager::getImageRef(const std::string &idPath)
+{
+    ResourceRef<Image> img = getImage(idPath);
+    img->decRef();  // remove ref added by ResourceManager::get
+    return img;
+}
+
 ImageSet *ResourceManager::getImageSet(const std::string &imagePath,
                                        int w, int h)
 {
@@ -317,13 +324,11 @@ ImageSet *ResourceManager::getImageSet(const std::string &imagePath,
     ss << imagePath << "[" << w << "x" << h << "]";
 
     return static_cast<ImageSet*>(get(ss.str(), [&] () -> Resource * {
-        Image *img = getImage(imagePath);
+        auto img = getImageRef(imagePath);
         if (!img)
             return nullptr;
 
-        auto *res = new ImageSet(img, w, h);
-        img->decRef();
-        return res;
+        return new ImageSet(img, w, h);
     }));
 }
 
diff --git a/src/resources/resourcemanager.h b/src/resources/resourcemanager.h
index 6694321c..d1c32d8c 100644
--- a/src/resources/resourcemanager.h
+++ b/src/resources/resourcemanager.h
@@ -22,6 +22,8 @@
 #ifndef RESOURCE_MANAGER_H
 #define RESOURCE_MANAGER_H
 
+#include "resources/resource.h"
+
 #include <ctime>
 #include <functional>
 #include <map>
@@ -31,7 +33,6 @@
 class Image;
 class ImageSet;
 class Music;
-class Resource;
 class SoundEffect;
 class SpriteDef;
 
@@ -144,6 +145,12 @@ class ResourceManager
 
         /**
          * Convenience wrapper around ResourceManager::get for loading
+         * images. Returns an automatically reference-counted resource.
+         */
+        ResourceRef<Image> getImageRef(const std::string &idPath);
+
+        /**
+         * Convenience wrapper around ResourceManager::get for loading
          * songs.
          */
         Music *getMusic(const std::string &idPath);