Finishing resource manager ability to clean up resources. Still work to do on

resources other than images.

10 files changed, 162 insertions, 161 deletions

diff --git a/src/resources/image.cpp b/src/resources/image.cpp
index c34ba42f..1abad8ec 100644
--- a/src/resources/image.cpp
+++ b/src/resources/image.cpp
@@ -155,8 +155,6 @@ Image* Image::load(void* buffer, unsigned int bufferSize)
         return NULL;
     }
 
-    logger->log("Alpha: %d, Magic pink: %d", hasAlpha, hasPink);
-
     return new Image(image);
 
 #else
@@ -416,8 +414,7 @@ SubImage::~SubImage()
 #ifndef USE_OPENGL
     image = NULL;
 #endif
-    // TODO: Enable when no longer a problem
-    //parent->decRef();
+    parent->decRef();
 }
 
 int SubImage::getWidth() const
diff --git a/src/resources/image.h b/src/resources/image.h
index 14aa4550..b41876d2 100644
--- a/src/resources/image.h
+++ b/src/resources/image.h
@@ -32,10 +32,6 @@
 #include <string>
 
 
-// Forward declarations
-class SubImage;
-class ScaledImage;
-
 /**
  * Defines a class for loading and storing images.
  */
@@ -45,7 +41,8 @@ class Image : public Resource
         /**
          * Destructor.
          */
-        virtual ~Image();
+        virtual
+        ~Image();
 
         /**
          * Loads an image from a buffer in memory.
@@ -56,22 +53,26 @@ class Image : public Resource
          * @return <code>NULL</code> if the an error occurred, a valid pointer
          *         otherwise.
          */
-        static Image *load(void* buffer, unsigned int bufferSize);
+        static Image*
+        load(void* buffer, unsigned int bufferSize);
 
         /**
          * Frees the resources created by SDL.
          */
-        virtual void unload();
+        virtual void
+        unload();
 
         /**
          * Returns the width of the image.
          */
-        virtual int getWidth() const;
+        virtual int
+        getWidth() const;
 
         /**
          * Returns the height of the image.
          */
-        virtual int getHeight() const;
+        virtual int
+        getHeight() const;
 
         /**
          * Creates a new image with the desired clipping rectangle.
@@ -79,7 +80,8 @@ class Image : public Resource
          * @return <code>NULL</code> if creation failed and a valid
          *         object otherwise.
          */
-        virtual Image* getSubImage(int x, int y, int width, int height);
+        virtual Image*
+        getSubImage(int x, int y, int width, int height);
 
         /**
          * Blits the image onto the screen.
@@ -87,10 +89,11 @@ class Image : public Resource
          * @return <code>true</code> if the image was blitted properly
          *         <code>false</code> otherwise.
          */
-        virtual bool draw(SDL_Surface *screen,
-                int srcX, int srcY,
-                int dstX, int dstY,
-                int width, int height);
+        virtual bool
+        draw(SDL_Surface *screen,
+             int srcX, int srcY,
+             int dstX, int dstY,
+             int width, int height);
 
         /**
          * Blits the image onto the screen.
@@ -98,23 +101,26 @@ class Image : public Resource
          * @return <code>true</code> if the image was blitted properly
          *         <code>false</code> otherwise.
          */
-        virtual bool draw(SDL_Surface *screen, int x, int y);
+        virtual bool
+        draw(SDL_Surface *screen, int x, int y);
 
         /**
          * Does a pattern fill on the given area.
          */
-        virtual void drawPattern(
-                SDL_Surface *screen, int x, int y, int w, int h);
+        virtual void
+        drawPattern(SDL_Surface *screen, int x, int y, int w, int h);
 
         /**
          * Sets the alpha value of this image.
          */
-        void setAlpha(float alpha);
+        void
+        setAlpha(float alpha);
 
         /**
          * Returns the alpha value of this image.
          */
-        float getAlpha();
+        float
+        getAlpha();
 
 
     protected:
@@ -123,12 +129,14 @@ class Image : public Resource
          */
 #ifdef USE_OPENGL
         Image(GLuint image,
-                int width, int height,
-                int texWidth, int texHeight);
+              int width, int height,
+              int texWidth, int texHeight);
 #else
         Image(SDL_Surface *image);
 #endif
 
+        bool loaded;
+
 #ifdef USE_OPENGL
         GLuint image;
         int width, height;
@@ -150,10 +158,10 @@ class SubImage : public Image
          */
 #ifndef USE_OPENGL
         SubImage(Image *parent, SDL_Surface *image,
-                int x, int y, int width, int height);
+                 int x, int y, int width, int height);
 #else
         SubImage(Image *parent, GLuint image, int x, int y,
-                int width, int height, int texWidth, int textHeight);
+                 int width, int height, int texWidth, int textHeight);
 #endif
 
         /**
@@ -164,32 +172,41 @@ class SubImage : public Image
         /**
          * Returns the width of the image.
          */
-        int getWidth() const;
+        int
+        getWidth() const;
 
         /**
          * Returns the height of the image.
          */
-        int getHeight() const;
+        int
+        getHeight() const;
 
         /**
          * Creates a new image with the desired clipping rectangle.
+         *
          * @return <code>NULL</code> if creation failed and a valid
-         * object otherwise.
+         *         image otherwise.
          */
-        Image* getSubImage(int x, int y, int width, int height);
+        Image*
+        getSubImage(int x, int y, int width, int height);
 
         /**
          * Draws this image.
          */
-        bool draw(SDL_Surface *screen, int srcX, int srcY,
-                int dstX, int dstY, int width, int height);
+        bool
+        draw(SDL_Surface *screen,
+             int srcX, int srcY,
+             int dstX, int dstY,
+             int width, int height);
 
         /**
          * Draws the clipped image onto the screen.
+         *
          * @return <code>true</code> if drawing was succesful
-         * <code>false</code> otherwise.
+         *         <code>false</code> otherwise.
          */
-        bool draw(SDL_Surface *screen, int x, int y);
+        bool
+        draw(SDL_Surface *screen, int x, int y);
 
     private:
         Image *parent;
diff --git a/src/resources/music.cpp b/src/resources/music.cpp
index 7c353f03..6b41dd76 100644
--- a/src/resources/music.cpp
+++ b/src/resources/music.cpp
@@ -32,7 +32,9 @@ Music::Music(Mix_Chunk *music):
 
 Music::~Music()
 {
-    unload();
+    //Mix_FreeMusic(music);
+    Mix_FreeChunk(music);
+    music = NULL;
 }
 
 Music* Music::load(void* buffer, unsigned int bufferSize)
@@ -50,14 +52,6 @@ Music* Music::load(void* buffer, unsigned int bufferSize)
     return new Music(tmpMusic);
 }
 
-void Music::unload()
-{
-    //Mix_FreeMusic(music);
-    Mix_FreeChunk(music);
-    music = NULL;
-    loaded = false;
-}
-
 bool Music::play(int loops)
 {
     /*
diff --git a/src/resources/music.h b/src/resources/music.h
index ab440e4b..68f285e4 100644
--- a/src/resources/music.h
+++ b/src/resources/music.h
@@ -51,11 +51,6 @@ class Music : public Resource
         static Music *load(void* buffer, unsigned int bufferSize);
 
         /**
-         * Frees the resources created by SDL.
-         */
-        virtual void unload();
-
-        /**
          * Plays the music.
          *
          * @param loops     Number of times to repeat the playback.
diff --git a/src/resources/resource.cpp b/src/resources/resource.cpp
index b3aa80f6..3ff8064d 100644
--- a/src/resources/resource.cpp
+++ b/src/resources/resource.cpp
@@ -22,49 +22,45 @@
  */
 
 #include "resource.h"
+#include "resourcemanager.h"
 
 Resource::Resource():
-    referenceCount(0)
+    mRefCount(0)
 {
 }
 
 Resource::~Resource()
 {
-    // TODO: Notify resource manager about this resource being deleted
 }
 
-bool Resource::isLoaded() const
+void
+Resource::setIdPath(const std::string &idPath)
 {
-    return loaded;
+    mIdPath = idPath;
 }
 
-void Resource::incRef()
+void
+Resource::incRef()
 {
-    referenceCount++;
+    mRefCount++;
 }
 
-bool Resource::decRef()
+bool
+Resource::decRef()
 {
-    /* Warning: There is still a serious problem with the object deleting
-     * itself and that is that the resource manager doesn't know about it
-     * currently, causing it to crash while trying to clean up. Don't use
-     * this function until that is solved. Probably we'll have to make it
-     * so that decrementing count goes through resource manager too.
-     */
-    if (referenceCount > 0)
-    {
-        referenceCount--;
+    // Reference may not already have reached zero
+    assert(mRefCount != 0);
 
-        if (referenceCount == 0) {
-            //delete this;
-            return true;
-        }
-        else {
-            return false;
-        }
+    mRefCount--;
+
+    if (mRefCount == 0) {
+        // Make sure resource manager won't refer to deleted resource
+        ResourceManager *resman = ResourceManager::getInstance();
+        resman->release(mIdPath);
+        delete this;
+        return true;
     }
     else {
-        // Warning: Shouldn't get here!
-        return true;
+        return false;
     }
 }
diff --git a/src/resources/resource.h b/src/resources/resource.h
index 266b4198..09c12c90 100644
--- a/src/resources/resource.h
+++ b/src/resources/resource.h
@@ -24,6 +24,8 @@
 #ifndef _TMW_RESOURCE_H
 #define _TMW_RESOURCE_H
 
+#include <string>
+
 /**
  * A generic reference counted resource object.
  */
@@ -36,40 +38,38 @@ class Resource
         Resource();
 
         /**
-         * Destructor.
-         */
-        virtual ~Resource();
-
-        /**
-         * Frees this objects internal resources.
+         * Sets the id path of this resource. This path is used to notify the
+         * resource manager when this resource is deleted.
          */
-        virtual void unload() = 0;
-
-        /**
-         * Indicates whether or not a resource is loaded.
-         * @return <code>true</code> if resource is loaded 
-         * <code>false</code> otherwise.
-         */
-        virtual bool isLoaded() const;
+        void
+        setIdPath(const std::string &idPath);
 
         /**
          * Increments the internal reference count.
          */
-        void incRef();
+        void
+        incRef();
 
         /**
          * Decrements the reference count and deletes the object
          * if no references are left.
+         *
          * @return <code>true</code> if the object was deleted
-         * <code>false</code> otherwise.
+         *         <code>false</code> otherwise.
          */
-        bool decRef();
+        bool
+        decRef();
 
     protected:
-        bool loaded;
+        /**
+         * Destructor.
+         */
+        virtual
+        ~Resource();
 
     private:
-        unsigned int referenceCount;
+        unsigned int mRefCount;    /**< Reference count */
+        std::string mIdPath;       /**< Path identifying this resource */
 };
 
 #endif
diff --git a/src/resources/resourcemanager.cpp b/src/resources/resourcemanager.cpp
index 1e2f4f31..fc0c7f9f 100644
--- a/src/resources/resourcemanager.cpp
+++ b/src/resources/resourcemanager.cpp
@@ -36,11 +36,6 @@
 #include <dirent.h>
 #endif
 
-ResourceEntry::ResourceEntry():
-    resource(NULL)
-{
-}
-
 ResourceManager *ResourceManager::instance = NULL;
 
 ResourceManager::ResourceManager()
@@ -52,36 +47,36 @@ ResourceManager::ResourceManager()
 ResourceManager::~ResourceManager()
 {
     // Create our resource iterator.
-    std::map<std::string, ResourceEntry>::iterator iter = resources.begin();
+    std::map<std::string, Resource*>::iterator iter = resources.begin();
     int danglingResources = 0;
     int danglingReferences = 0;
 
     // Iterate through and release references until objects are deleted.
-    while (iter != resources.end()) {
-        Resource *res = iter->second.resource;
+    while (!resources.empty())
+    {
+        Resource *res = resources.begin()->second;
         danglingResources++;
-        danglingReferences++;
-        while (!res->decRef()) {
+
+        do {
             danglingReferences++;
         }
-        iter++;
+        while (!res->decRef());
     }
-    resources.clear();
 
     logger->log("ResourceManager::~ResourceManager() cleaned up %d references "
             "to %d resources", danglingReferences, danglingResources);
 }
 
-Resource* ResourceManager::get(const E_RESOURCE_TYPE &type,
-        const std::string &idPath)
+Resource*
+ResourceManager::get(const E_RESOURCE_TYPE &type, const std::string &idPath)
 {
     // Check if the id exists, and return the value if it does.
-    std::map<std::string, ResourceEntry>::iterator resIter =
+    std::map<std::string, Resource*>::iterator resIter =
         resources.find(idPath);
 
-    if (resIter != resources.end() && resIter->second.resource) {
-        resIter->second.resource->incRef();
-        return resIter->second.resource;
+    if (resIter != resources.end() && resIter->second) {
+        resIter->second->incRef();
+        return resIter->second;
     }
 
     logger->log("ResourceManager::get(%s)", idPath.c_str());
@@ -164,42 +159,57 @@ Resource* ResourceManager::get(const E_RESOURCE_TYPE &type,
 
     if (resource) {
         resource->incRef();
+        resource->setIdPath(idPath);
 
-        // Create the resource entry for this object.
-        ResourceEntry entry;
-        entry.filePath = idPath;
-        entry.resource = resource;
-
-        resources[idPath] = entry;
+        resources[idPath] = resource;
     }
 
     // Return NULL if the object could not be created.
     return resource;
 }
 
-Image *ResourceManager::getImage(const std::string &idPath)
+Image*
+ResourceManager::getImage(const std::string &idPath)
 {
     return (Image*)get(IMAGE, idPath);
 }
 
-Music *ResourceManager::getMusic(const std::string &idPath)
+Music*
+ResourceManager::getMusic(const std::string &idPath)
 {
-      return (Music*)get(MUSIC, idPath);
+    return (Music*)get(MUSIC, idPath);
 }
 
-SoundEffect *ResourceManager::getSoundEffect(const std::string &idPath)
+SoundEffect*
+ResourceManager::getSoundEffect(const std::string &idPath)
 {
-      return (SoundEffect*)get(SOUND_EFFECT, idPath);
+    return (SoundEffect*)get(SOUND_EFFECT, idPath);
+}
+
+void
+ResourceManager::release(const std::string &idPath)
+{
+    logger->log("ResourceManager::release(%s)", idPath.c_str());
+
+    std::map<std::string, Resource*>::iterator resIter =
+        resources.find(idPath);
+
+    // The resource has to exist
+    assert(resIter != resources.end() && resIter->second);
+
+    resources.erase(idPath);
 }
 
-ResourceManager* ResourceManager::getInstance()
+ResourceManager*
+ResourceManager::getInstance()
 {
     // Create a new instance if necessary.
     if (instance == NULL) instance = new ResourceManager();
     return instance;
 }
 
-void ResourceManager::deleteInstance()
+void
+ResourceManager::deleteInstance()
 {
     if (instance != NULL) {
         delete instance;
@@ -207,7 +217,8 @@ void ResourceManager::deleteInstance()
     }
 }
 
-void ResourceManager::searchAndAddZipFiles()
+void
+ResourceManager::searchAndAddZipFiles()
 {
     // Add the main data directory to our PhysicsFS search path
     PHYSFS_addToSearchPath("data", 1);
@@ -275,7 +286,8 @@ void ResourceManager::searchAndAddZipFiles()
 #endif
 }
 
-void *ResourceManager::loadFile(const std::string &fileName, int &fileSize)
+void*
+ResourceManager::loadFile(const std::string &fileName, int &fileSize)
 {
     // If the file doesn't exist indicate failure
     if (!PHYSFS_exists(fileName.c_str())) {
diff --git a/src/resources/resourcemanager.h b/src/resources/resourcemanager.h
index 2204d455..f95acadc 100644
--- a/src/resources/resourcemanager.h
+++ b/src/resources/resourcemanager.h
@@ -32,17 +32,6 @@
 #include "soundeffect.h"
 
 /**
- * A resource entry descriptor.
- */
-struct ResourceEntry
-{
-    ResourceEntry();
-
-    Resource *resource;
-    std::string filePath;
-};
-
-/**
  * A class for loading and managing resources.
  */
 class ResourceManager
@@ -81,27 +70,35 @@ class ResourceManager
          * @return A valid resource or <code>NULL</code> if the resource could
          *         not be loaded.
          */
-        Resource *get(
-                const E_RESOURCE_TYPE &type,
-                const std::string &idPath);
+        Resource*
+        get(const E_RESOURCE_TYPE &type, const std::string &idPath);
 
         /**
          * Convenience wrapper around ResourceManager::create for loading
          * images.
          */
-        Image *getImage(const std::string &idPath);
+        Image*
+        getImage(const std::string &idPath);
 
         /**
          * Convenience wrapper around ResourceManager::create for loading
          * songs.
          */
-        Music *getMusic(const std::string &idPath);
+        Music*
+        getMusic(const std::string &idPath);
 
         /**
          * Convenience wrapper around ResourceManager::create for loading
          * samples.
          */
-        SoundEffect *getSoundEffect(const std::string &idPath);
+        SoundEffect*
+        getSoundEffect(const std::string &idPath);
+
+        /**
+         * Releases a resource, removing it from the set of loaded resources.
+         */
+        void
+        release(const std::string &idPath);
 
         /**
          * Allocates data into a buffer pointer for raw data loading. The
@@ -113,28 +110,32 @@ class ResourceManager
          * @return An allocated byte array containing the data that was loaded,
          *         or <code>NULL</code> on fail.
          */
-        void *loadFile(const std::string &fileName, int &fileSize);
+        void*
+        loadFile(const std::string &fileName, int &fileSize);
 
         /**
          * Returns an instance of the class, creating one if it does not
          * already exist.
          */
-        static ResourceManager *getInstance();
+        static ResourceManager*
+        getInstance();
 
         /**
          * Deletes the class instance if it exists.
          */
-        static void deleteInstance();
+        static void
+        deleteInstance();
 
     private:
         /**
          * Searches for zip files and adds them to the PhysicsFS search path.
          */
-        void searchAndAddZipFiles();
+        void
+        searchAndAddZipFiles();
 
 
         static ResourceManager *instance;
-        std::map<std::string, ResourceEntry> resources;
+        std::map<std::string, Resource*> resources;
 };
 
 #endif
diff --git a/src/resources/soundeffect.cpp b/src/resources/soundeffect.cpp
index 5d51ef12..ba0af415 100644
--- a/src/resources/soundeffect.cpp
+++ b/src/resources/soundeffect.cpp
@@ -31,7 +31,8 @@ SoundEffect::SoundEffect(Mix_Chunk *soundEffect):
 
 SoundEffect::~SoundEffect()
 {
-    unload();
+    Mix_FreeChunk(soundEffect);
+    soundEffect = NULL;
 }
 
 SoundEffect* SoundEffect::load(void* buffer, unsigned int bufferSize)
@@ -48,13 +49,6 @@ SoundEffect* SoundEffect::load(void* buffer, unsigned int bufferSize)
     return new SoundEffect(tmpSoundEffect);
 }
 
-void SoundEffect::unload()
-{
-    Mix_FreeChunk(soundEffect);
-    soundEffect = NULL;
-    loaded = false;
-}
-
 bool SoundEffect::play(int loops, int volume)
 {
     Mix_VolumeChunk(soundEffect, volume);
diff --git a/src/resources/soundeffect.h b/src/resources/soundeffect.h
index 959a8e2e..2e56df9b 100644
--- a/src/resources/soundeffect.h
+++ b/src/resources/soundeffect.h
@@ -51,11 +51,6 @@ class SoundEffect : public Resource
         static SoundEffect *load(void* buffer, unsigned int bufferSize);
 
         /**
-         * Frees the resources created by SDL.
-         */
-        virtual void unload();
-
-        /**
          * Plays the sample.
          *
          * @param loops     Number of times to repeat the playback.