Some cleanup of the SkinLoader

Mainly stopped it from parsing the skin XML files over and over again,
but also fixed Skin leaking sticky button images and made some
optimizations.

The Skin class still has an 'instances' member which could be used for
cleanup in theory, but is unused at the moment.

5 files changed, 156 insertions, 152 deletions

diff --git a/src/configlistener.h b/src/configlistener.h
index ec7d6a2c..0fbdfea9 100644
--- a/src/configlistener.h
+++ b/src/configlistener.h
@@ -36,7 +36,7 @@ class ConfigListener
         /**
          * Destructor.
          */
-        virtual ~ConfigListener() {};
+        virtual ~ConfigListener() {}
 
         /**
          * Called when an option changed. The config listener will have to be
diff --git a/src/gui/buddywindow.cpp b/src/gui/buddywindow.cpp
index 80a34848..8af4dd6e 100644
--- a/src/gui/buddywindow.cpp
+++ b/src/gui/buddywindow.cpp
@@ -35,7 +35,7 @@ BuddyWindow::BuddyWindow():
     Window(_("Buddy"))
 {
     setVisible(false);
-    setWindowName("Buddy Window");
+    setWindowName("BuddyWindow");
     setCaption(_("Buddy List"));
     setResizable(true);
     setCloseButton(true);
diff --git a/src/gui/skin.cpp b/src/gui/skin.cpp
index 0130ad71..6fcbb0e2 100644
--- a/src/gui/skin.cpp
+++ b/src/gui/skin.cpp
@@ -2,6 +2,7 @@
  *  Gui Skinning
  *  Copyright (C) 2008  The Legend of Mazzeroth Development Team
  *  Copyright (C) 2009  Aethyra Development Team
+ *  Copyright (C) 2009  The Mana World Development Team
  *
  *  This file is part of The Mana World.
  *
@@ -20,43 +21,58 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include <algorithm>
+#include "gui/skin.h"
+
+#include "configuration.h"
+#include "configlistener.h"
+#include "log.h"
 
-#include "skin.h"
+#include "resources/image.h"
+#include "resources/resourcemanager.h"
 
-#include "../configuration.h"
-#include "../configlistener.h"
-#include "../log.h"
+#include "utils/dtor.h"
+#include "utils/strprintf.h"
+#include "utils/xml.h"
 
-#include "../resources/image.h"
-#include "../resources/resourcemanager.h"
+#include <algorithm>
 
-#include "../utils/dtor.h"
-#include "../utils/strprintf.h"
-#include "../utils/xml.h"
+SkinLoader *skinLoader = 0;
 
-SkinLoader *skinLoader = NULL;
-ConfigListener *SkinLoader::skinConfigListener = NULL;
 
 class SkinConfigListener : public ConfigListener
 {
-    void optionChanged(const std::string &)
-    {
-        if (skinLoader)
-            skinLoader->updateAlpha();
-    }
+    public:
+        SkinConfigListener(SkinLoader *skinLoader)
+            : mSkinLoader(skinLoader)
+        {
+            config.addListener("guialpha", this);
+        }
+
+        ~SkinConfigListener()
+        {
+            config.removeListener("guialpha", this);
+        }
+
+        void optionChanged(const std::string &)
+        {
+            mSkinLoader->updateAlpha();
+        }
+
+    private:
+        SkinLoader *mSkinLoader;
 };
 
+
 Skin::Skin(ImageRect skin, Image *close, Image *stickyUp, Image *stickyDown,
            const std::string &filePath,
            const std::string &name):
     instances(0),
     mFilePath(filePath),
     mName(name),
-    border(skin),
-    closeImage(close),
-    stickyImageUp(stickyUp),
-    stickyImageDown(stickyDown)
+    mBorder(skin),
+    mCloseImage(close),
+    mStickyImageUp(stickyUp),
+    mStickyImageDown(stickyDown)
 {
 }
 
@@ -64,114 +80,145 @@ Skin::~Skin()
 {
     // Clean up static resources
     for (int i = 0; i < 9; i++)
-    {
-        delete border.grid[i];
-        border.grid[i] = NULL;
-    }
+        delete mBorder.grid[i];
 
-    closeImage->decRef();
+    mCloseImage->decRef();
+    delete mStickyImageUp;
+    delete mStickyImageDown;
 }
 
 void Skin::updateAlpha()
 {
     const float alpha = config.getValue("guialpha", 0.8);
 
-    for_each(border.grid, border.grid + 9,
+    for_each(mBorder.grid, mBorder.grid + 9,
              std::bind2nd(std::mem_fun(&Image::setAlpha), alpha));
-    closeImage->setAlpha(alpha);
+
+    mCloseImage->setAlpha(alpha);
+    mStickyImageUp->setAlpha(alpha);
+    mStickyImageDown->setAlpha(alpha);
 }
 
 int Skin::getMinWidth() const
 {
-    return (border.grid[0]->getWidth() + border.grid[1]->getWidth()) +
-            border.grid[2]->getWidth();
+    return (mBorder.grid[ImageRect::UPPER_LEFT]->getWidth() +
+            mBorder.grid[ImageRect::UPPER_CENTER]->getWidth()) +
+            mBorder.grid[ImageRect::UPPER_RIGHT]->getWidth();
 }
 
 int Skin::getMinHeight() const
 {
-    return (border.grid[0]->getHeight() + border.grid[3]->getHeight()) +
-            border.grid[6]->getHeight();
+    return (mBorder.grid[ImageRect::UPPER_LEFT]->getHeight() +
+            mBorder.grid[ImageRect::LEFT]->getHeight()) +
+            mBorder.grid[ImageRect::LOWER_LEFT]->getHeight();
 }
 
-Skin *SkinLoader::load(const std::string &filename,
-                       const std::string &defaultPath)
-{
-    std::string filePath = filename;
 
-    ResourceManager *resman = ResourceManager::getInstance();
+SkinLoader::SkinLoader()
+    : mSkinConfigListener(new SkinConfigListener(this))
+{
+}
 
-    logger->log("Loading Skin '%s'.", filename.c_str());
+SkinLoader::~SkinLoader()
+{
+    delete_all(mSkins);
+    delete mSkinConfigListener;
+}
 
-    if (filename.empty() && defaultPath.empty())
-        logger->error("SkinLoader::load(): Invalid File Name.");
+Skin *SkinLoader::load(const std::string &filename,
+                       const std::string &defaultPath)
+{
+    // Check if this skin was already loaded
+    SkinIterator skinIterator = mSkins.find(filename);
+    if (mSkins.end() != skinIterator)
+    {
+        skinIterator->second->instances++;
+        return skinIterator->second;
+    }
 
-    XML::Document *doc = new XML::Document(filePath);
-    xmlNodePtr rootNode = doc->rootNode();
+    Skin *skin = readSkin(filename);
 
-    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "skinset"))
+    if (!skin)
     {
-        filePath = defaultPath;
-
-        logger->log("Widget Skinning error. Loading '%s' instead.",
-                    filePath.c_str());
+        // Try falling back on the defaultPath if this makes sense
+        if (filename != defaultPath)
+        {
+            logger->log("Error loading skin '%s', falling back on default.",
+                        filename.c_str());
 
-        delete doc;
+            skin = readSkin(defaultPath);
+        }
 
-        doc = new XML::Document(filePath);
-        rootNode = doc->rootNode();
-        if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "skinset"))
+        if (!skin)
         {
-            logger->error(strprintf("Skinning failed. Check this skin file "
-                                    "to make sure it's valid: %s",
-                                    filePath.c_str()));
+            logger->error(strprintf("Error: Loading default skin '%s' failed. "
+                                    "Make sure the skin file is valid.",
+                                    defaultPath.c_str()));
         }
     }
 
-    SkinIterator skinIterator = mSkins.find(filePath);
+    // Add the skin to the loaded skins
+    mSkins[filename] = skin;
 
-    if (mSkins.end() != skinIterator)
-    {
-        skinIterator->second->instances++;
-        return skinIterator->second;
-    }
+    return skin;
+}
 
-    std::string skinSetImage;
-    skinSetImage = XML::getProperty(rootNode, "image", "");
-    Image *dBorders = NULL;
-    ImageRect border;
+void SkinLoader::updateAlpha()
+{
+    for (SkinIterator iter = mSkins.begin(); iter != mSkins.end(); ++iter)
+        iter->second->updateAlpha();
+}
 
-    if (!skinSetImage.empty())
-    {
-        logger->log("SkinLoader::load(): <skinset> defines "
-                    "'%s' as a skin image.", skinSetImage.c_str());
-        dBorders = resman->getImage("graphics/gui/" + skinSetImage);
-    }
-    else
+Skin *SkinLoader::readSkin(const std::string &filename)
+{
+    if (filename.empty())
+        return 0;
+
+    logger->log("Loading skin '%s'.", filename.c_str());
+
+    XML::Document doc(filename);
+    xmlNodePtr rootNode = doc.rootNode();
+
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "skinset"))
+        return 0;
+
+    const std::string skinSetImage = XML::getProperty(rootNode, "image", "");
+
+    if (skinSetImage.empty())
     {
-        logger->error("SkinLoader::load(): Skinset does not define an image!");
+        logger->log("SkinLoader::readSkin(): Skinset does not define an "
+                    "image!");
+        return 0;
     }
 
-    //iterate <widget>'s
+    logger->log("SkinLoader::load(): <skinset> defines "
+                "'%s' as a skin image.", skinSetImage.c_str());
+
+    ResourceManager *resman = ResourceManager::getInstance();
+    Image *dBorders = resman->getImage("graphics/gui/" + skinSetImage);
+    ImageRect border;
+
+    // iterate <widget>'s
     for_each_xml_child_node(widgetNode, rootNode)
     {
         if (!xmlStrEqual(widgetNode->name, BAD_CAST "widget"))
             continue;
 
-        std::string widgetType;
-        widgetType = XML::getProperty(widgetNode, "type", "unknown");
+        const std::string widgetType =
+                XML::getProperty(widgetNode, "type", "unknown");
         if (widgetType == "Window")
         {
             // Iterate through <part>'s
             // LEEOR / TODO:
-            // We need to make provisions to load in a CloseButton image. For 
+            // We need to make provisions to load in a CloseButton image. For
             // now it can just be hard-coded.
             for_each_xml_child_node(partNode, widgetNode)
             {
                 if (!xmlStrEqual(partNode->name, BAD_CAST "part"))
                     continue;
 
-                std::string partType;
-                partType = XML::getProperty(partNode, "type", "unknown");
+                const std::string partType =
+                        XML::getProperty(partNode, "type", "unknown");
                 // TOP ROW
                 const int xPos = XML::getProperty(partNode, "xpos", 0);
                 const int yPos = XML::getProperty(partNode, "ypos", 0);
@@ -201,63 +248,31 @@ Skin *SkinLoader::load(const std::string &filename,
                 else if (partType == "bottom-right-corner")
                     border.grid[8] = dBorders->getSubImage(xPos, yPos, width, height);
 
-                // Part is of an uknown type.
                 else
-                    logger->log("SkinLoader::load(): Unknown Part Type '%s'", partType.c_str());
+                    logger->log("SkinLoader::readSkin(): Unknown part type '%s'",
+                                partType.c_str());
             }
         }
-        // Widget is of an uknown type.
         else
         {
-            logger->log("SkinLoader::load(): Unknown Widget Type '%s'", widgetType.c_str());
+            logger->log("SkinLoader::readSkin(): Unknown widget type '%s'",
+                        widgetType.c_str());
         }
     }
-    dBorders->decRef();
 
-    logger->log("Finished loading Skin.");
+    dBorders->decRef();
 
-    delete doc;
+    logger->log("Finished loading skin.");
 
-    // Hard-coded for now until we update the above code to look for window buttons.
+    // Hard-coded for now until we update the above code to look for window buttons
     Image *closeImage = resman->getImage("graphics/gui/close_button.png");
-
     Image *sticky = resman->getImage("graphics/gui/sticky_button.png");
     Image *stickyImageUp = sticky->getSubImage(0, 0, 15, 15);
     Image *stickyImageDown = sticky->getSubImage(15, 0, 15, 15);
-
     sticky->decRef();
 
     Skin *skin = new Skin(border, closeImage, stickyImageUp, stickyImageDown,
                           filename);
-
-    mSkins[filename] = skin;
-
-    updateAlpha();
-
+    skin->updateAlpha();
     return skin;
 }
-
-SkinLoader::SkinLoader()
-{
-    skinConfigListener = new SkinConfigListener;
-    // Send GUI alpha changed for initialization
-    skinConfigListener->optionChanged("guialpha");
-    config.addListener("guialpha", skinConfigListener);
-}
-
-SkinLoader::~SkinLoader()
-{
-    delete_all(mSkins);
-    config.removeListener("guialpha", skinConfigListener);
-    delete skinConfigListener;
-    skinConfigListener = NULL;
-}
-
-void SkinLoader::updateAlpha()
-{
-    for (SkinIterator iter = mSkins.begin(); iter != mSkins.end(); ++iter)
-    {
-        iter->second->updateAlpha();
-    }
-}
-
diff --git a/src/gui/skin.h b/src/gui/skin.h
index 30f7f360..25a9f0e4 100644
--- a/src/gui/skin.h
+++ b/src/gui/skin.h
@@ -2,6 +2,7 @@
  *  Gui Skinning
  *  Copyright (C) 2008  The Legend of Mazzeroth Development Team
  *  Copyright (C) 2009  Aethyra Development Team
+ *  Copyright (C) 2009  The Mana World Development Team
  *
  *  This file is part of The Mana World.
  *
@@ -23,11 +24,11 @@
 #ifndef SKIN_H
 #define SKIN_H
 
+#include "graphics.h"
+
 #include <map>
 #include <string>
 
-#include "../graphics.h"
-
 class ConfigListener;
 class Image;
 
@@ -45,33 +46,28 @@ class Skin
          * name if a dialog for skin selection for a specific window type is
          * done.
          */
-        std::string getName() const { return mName; }
+        const std::string &getName() const { return mName; }
 
         /**
          * Returns the skin's xml file path.
          */
-        std::string getFilePath() const { return mFilePath; }
+        const std::string &getFilePath() const { return mFilePath; }
 
         /**
          * Returns the background skin.
          */
-        ImageRect getBorder() const { return border; }
+        const ImageRect &getBorder() const { return mBorder; }
 
         /**
          * Returns the image used by a close button for this skin.
          */
-        Image *getCloseImage() const { return closeImage; }
+        Image *getCloseImage() const { return mCloseImage; }
 
         /**
          * Returns the image used by a sticky button for this skin.
          */
         Image *getStickyImage(bool state) const
-        { return state ? stickyImageDown : stickyImageUp; }
-
-        /**
-         * Returns the number of instances which use this skin.
-         */
-        int getNumberOfInstances() const { return instances; }
+        { return state ? mStickyImageDown : mStickyImageUp; }
 
         /**
          * Returns the minimum width which can be used with this skin.
@@ -93,44 +89,42 @@ class Skin
     private:
         std::string mFilePath;     /**< File name path for the skin */
         std::string mName;         /**< Name of the skin to use */
-        ImageRect border;          /**< The window border and background */
-        Image *closeImage;         /**< Close Button Image */
-        Image *stickyImageUp;      /**< Sticky Button Image */
-        Image *stickyImageDown;    /**< Sticky Button Image */
+        ImageRect mBorder;         /**< The window border and background */
+        Image *mCloseImage;        /**< Close Button Image */
+        Image *mStickyImageUp;     /**< Sticky Button Image */
+        Image *mStickyImageDown;   /**< Sticky Button Image */
 };
 
-// Map containing all window skins
-typedef std::map<std::string, Skin*> Skins;
-
-// Iterator for window skins
-typedef Skins::iterator SkinIterator;
-
 class SkinLoader 
 {
     public:
-        friend class SkinConfigListener;
-
         SkinLoader();
         ~SkinLoader();
 
         /**
-         * Loads a skin
+         * Loads a skin.
          */
         Skin *load(const std::string &filename,
                    const std::string &defaultPath);
 
         /**
-         * Updates the alpha values of all of the skins
+         * Updates the alpha values of all of the skins.
          */
         void updateAlpha();
 
     private:
+        Skin *readSkin(const std::string &filename);
+
+        // Map containing all window skins
+        typedef std::map<std::string, Skin*> Skins;
+        typedef Skins::iterator SkinIterator;
+
         Skins mSkins;
 
         /**
          * The config listener that listens to changes relevant to all skins.
          */
-        static ConfigListener *skinConfigListener;
+        ConfigListener *mSkinConfigListener;
 };
 
 extern SkinLoader *skinLoader;
diff --git a/src/gui/widgets/windowcontainer.h b/src/gui/widgets/windowcontainer.h
index a2c285cb..a1a4b14c 100644
--- a/src/gui/widgets/windowcontainer.h
+++ b/src/gui/widgets/windowcontainer.h
@@ -45,11 +45,6 @@ class WindowContainer : public Container
          */
         void scheduleDelete(gcn::Widget *widget);
 
-        /**
-         * Get the number of widget instances
-         */
-        int getNumberOfInstances() { return mDeathList.size(); }
-
     private:
         /**
          * List of widgets that are scheduled to be deleted.