Merged Björn Steinbrinks improvements at the xml parsing from trunk version.

8 files changed, 84 insertions, 104 deletions

diff --git a/ChangeLog b/ChangeLog
index 92b37960..74b6b931 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2007-04-22  Björn Steinbrink  <B.Steinbrink@gmx.de>
+
+	* src/configuration.cpp, src/utils/xml.h, src/resources/mapreader.cpp,
+	src/resources/spritedef.cpp, src/resources/monsterdb.cpp,
+	src/resources/itemdb.cpp, src/resources/equipmentdb.cpp: Added a macro
+	for XML child node iterations to make the code more terse and easier
+	to read.
+	* src/resources/itemdb.cpp: Made the parameter checks more terse and
+	manageable.
+
 2007-04-15  Bjørn Lindeijer  <bjorn@lindeijer.nl>
 
 	* src/openglgraphics.cpp: Rely on default 0 value for z axis.
diff --git a/src/configuration.cpp b/src/configuration.cpp
index 53f6785e..d33df386 100644
--- a/src/configuration.cpp
+++ b/src/configuration.cpp
@@ -30,6 +30,7 @@
 #include "log.h"
 
 #include "utils/tostring.h"
+#include "utils/xml.h"
 
 void Configuration::init(const std::string &filename)
 {
@@ -48,14 +49,14 @@ void Configuration::init(const std::string &filename)
 
     if (!doc) return;
 
-    xmlNodePtr node = xmlDocGetRootElement(doc);
+    xmlNodePtr rootNode = xmlDocGetRootElement(doc);
 
-    if (!node || !xmlStrEqual(node->name, BAD_CAST "configuration")) {
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "configuration")) {
         logger->log("Warning: No configuration file (%s)", filename.c_str());
         return;
     }
 
-    for (node = node->xmlChildrenNode; node != NULL; node = node->next)
+    for_each_xml_child_node(node, rootNode)
     {
         if (!xmlStrEqual(node->name, BAD_CAST "option"))
             continue;
@@ -79,6 +80,8 @@ void Configuration::write()
     // Do not attempt to write to file that cannot be opened for writing
     FILE *testFile = fopen(mConfigPath.c_str(), "w");
     if (!testFile) {
+        logger->log("Configuration::write() couldn't open %s for writing",
+                    mConfigPath.c_str());
         return;
     }
     else {
@@ -87,8 +90,12 @@ void Configuration::write()
 
     xmlTextWriterPtr writer = xmlNewTextWriterFilename(mConfigPath.c_str(), 0);
 
-    if (!writer)
+    if (!writer) {
+        logger->log("Configuration::write() error while creating writer");
         return;
+    }
+
+    logger->log("Configuration::write() writing configuration...");
 
     xmlTextWriterSetIndent(writer, 1);
     xmlTextWriterStartDocument(writer, NULL, NULL, NULL);
@@ -96,9 +103,6 @@ void Configuration::write()
 
     for (OptionIterator i = mOptions.begin(); i != mOptions.end(); i++)
     {
-        logger->log("Configuration::write(%s, \"%s\")",
-                i->first.c_str(), i->second.c_str());
-
         xmlTextWriterStartElement(writer, BAD_CAST "option");
         xmlTextWriterWriteAttribute(writer,
                 BAD_CAST "name", BAD_CAST i->first.c_str());
diff --git a/src/resources/equipmentdb.cpp b/src/resources/equipmentdb.cpp
index 52a9fbd3..64982ce3 100644
--- a/src/resources/equipmentdb.cpp
+++ b/src/resources/equipmentdb.cpp
@@ -18,7 +18,7 @@
  *  along with The Mana World; if not, write to the Free Software

  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

  *

- *  $Id:

+ *  $Id$

  */

 

 #include "equipmentdb.h"

@@ -71,11 +71,8 @@ EquipmentDB::load()
     }

 

     //iterate <equipment>s

-    for (   xmlNodePtr equipmentNode = rootNode->xmlChildrenNode;

-            equipmentNode != NULL;

-            equipmentNode = equipmentNode->next)

+    for_each_xml_child_node(equipmentNode, rootNode)

     {

-

         if (!xmlStrEqual(equipmentNode->name, BAD_CAST "equipment"))

         {

             continue;

@@ -86,9 +83,7 @@ EquipmentDB::load()
         currentInfo->setSlot (XML::getProperty(equipmentNode, "slot", 0));

 

         //iterate <sprite>s

-        for (   xmlNodePtr spriteNode = equipmentNode->xmlChildrenNode;

-                spriteNode != NULL;

-                spriteNode = spriteNode->next)

+        for_each_xml_child_node(spriteNode, equipmentNode)

         {

             if (!xmlStrEqual(spriteNode->name, BAD_CAST "sprite"))

             {

diff --git a/src/resources/itemdb.cpp b/src/resources/itemdb.cpp
index f914af47..70ead6ab 100644
--- a/src/resources/itemdb.cpp
+++ b/src/resources/itemdb.cpp
@@ -65,13 +65,13 @@ void ItemDB::load()
         logger->error("ItemDB: Error while parsing item database (items.xml)!");

     }

 

-    xmlNodePtr node = xmlDocGetRootElement(doc);

-    if (!node || !xmlStrEqual(node->name, BAD_CAST "items"))

+    xmlNodePtr rootNode = xmlDocGetRootElement(doc);

+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "items"))

     {

         logger->error("ItemDB: items.xml is not a valid database file!");

     }

 

-    for (node = node->xmlChildrenNode; node != NULL; node = node->next)

+    for_each_xml_child_node(node, rootNode)

     {

         if (!xmlStrEqual(node->name, BAD_CAST "item")) {

             continue;

@@ -102,55 +102,26 @@ void ItemDB::load()
             mItemInfos[id] = itemInfo;

         }

 

-

         if (id == 0)

         {

             logger->log("ItemDB: An item has no ID in items.xml!");

         }

-        if (name == "")

-        {

-            logger->log("ItemDB: Missing name for item %d!", id);

-        }

 

-        if (image == "")

-        {

-            logger->log("ItemDB: Missing image parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        /*

-        if (art == 0)

-        {

-            logger->log("Item Manager: Missing art parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        if (description == "")

-        {

-            logger->log("ItemDB: Missing description parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        if (effect == "")

-        {

-            logger->log("ItemDB: Missing effect parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        if (type == 0)

-        {

-            logger->log("Item Manager: Missing type parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        */

-        if (weight == 0)

-        {

-            logger->log("Item Manager: Missing weight parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        /*

-        if (slot == 0)

-        {

-            logger->log("Item Manager: Missing slot parameter for item: %i. %s",

-                    id, name.c_str());

-        }

-        */

+#define CHECK_PARAM(param, error_value) \

+        if (param == error_value) \

+            logger->log("ItemDB: Missing" #param " parameter for item %i! %s", \

+                    id, name.c_str())

+

+        CHECK_PARAM(name, "");

+        CHECK_PARAM(image, "");

+        // CHECK_PARAM(art, 0);

+        // CHECK_PARAM(description, "");

+        // CHECK_PARAM(effect, "");

+        // CHECK_PARAM(type, 0);

+        CHECK_PARAM(weight, 0);

+        // CHECK_PARAM(slot, 0);

+

+#undef CHECK_PARAM

     }

 

     xmlFreeDoc(doc);

diff --git a/src/resources/mapreader.cpp b/src/resources/mapreader.cpp
index 9d9aabca..fda8916d 100644
--- a/src/resources/mapreader.cpp
+++ b/src/resources/mapreader.cpp
@@ -181,7 +181,6 @@ MapReader::readMap(const std::string &filename)
 
         if (!node || !xmlStrEqual(node->name, BAD_CAST "map")) {
             logger->log("Error: Not a map file (%s)!", filename.c_str());
-            return NULL;
         }
         else
         {
@@ -213,24 +212,24 @@ MapReader::readMap(xmlNodePtr node, const std::string &path)
     int layerNr = 0;
     Map *map = new Map(w, h, tilew, tileh);
 
-    for (node = node->xmlChildrenNode; node != NULL; node = node->next)
+    for_each_xml_child_node(childNode, node)
     {
-        if (xmlStrEqual(node->name, BAD_CAST "tileset"))
+        if (xmlStrEqual(childNode->name, BAD_CAST "tileset"))
         {
-            Tileset *tileset = readTileset(node, pathDir, map);
+            Tileset *tileset = readTileset(childNode, pathDir, map);
             if (tileset) {
                 map->addTileset(tileset);
             }
         }
-        else if (xmlStrEqual(node->name, BAD_CAST "layer"))
+        else if (xmlStrEqual(childNode->name, BAD_CAST "layer"))
         {
             logger->log("- Loading layer %d", layerNr);
-            readLayer(node, map, layerNr);
+            readLayer(childNode, map, layerNr);
             layerNr++;
         }
-        else if (xmlStrEqual(node->name, BAD_CAST "properties"))
+        else if (xmlStrEqual(childNode->name, BAD_CAST "properties"))
         {
-            readProperties(node, map);
+            readProperties(childNode, map);
         }
     }
 
@@ -242,13 +241,14 @@ MapReader::readMap(xmlNodePtr node, const std::string &path)
 void
 MapReader::readProperties(xmlNodePtr node, Properties* props)
 {
-    for (node = node->xmlChildrenNode; node; node = node->next) {
-        if (!xmlStrEqual(node->name, BAD_CAST "property"))
+    for_each_xml_child_node(childNode, node)
+    {
+        if (!xmlStrEqual(childNode->name, BAD_CAST "property"))
             continue;
 
         // Example: <property name="name" value="value"/>
-        xmlChar *name = xmlGetProp(node, BAD_CAST "name");
-        xmlChar *value = xmlGetProp(node, BAD_CAST "value");
+        xmlChar *name = xmlGetProp(childNode, BAD_CAST "name");
+        xmlChar *value = xmlGetProp(childNode, BAD_CAST "value");
 
         if (name && value) {
             props->setProperty((const char*)name, (const char*)value);
@@ -269,12 +269,13 @@ MapReader::readLayer(xmlNodePtr node, Map *map, int layer)
 
     // Load the tile data. Layers are assumed to be map size, with (0,0) as
     // origin.
-    for (node = node->xmlChildrenNode; node; node = node->next) {
-        if (!xmlStrEqual(node->name, BAD_CAST "data"))
+    for_each_xml_child_node(childNode, node)
+    {
+        if (!xmlStrEqual(childNode->name, BAD_CAST "data"))
             continue;
 
-        xmlChar *encoding = xmlGetProp(node, BAD_CAST "encoding");
-        xmlChar *compression = xmlGetProp(node, BAD_CAST "compression");
+        xmlChar *encoding = xmlGetProp(childNode, BAD_CAST "encoding");
+        xmlChar *compression = xmlGetProp(childNode, BAD_CAST "compression");
 
         if (encoding && xmlStrEqual(encoding, BAD_CAST "base64"))
         {
@@ -287,7 +288,7 @@ MapReader::readLayer(xmlNodePtr node, Map *map, int layer)
             }
 
             // Read base64 encoded map file
-            xmlNodePtr dataChild = node->xmlChildrenNode;
+            xmlNodePtr dataChild = childNode->xmlChildrenNode;
             if (!dataChild)
                 continue;
 
@@ -351,11 +352,12 @@ MapReader::readLayer(xmlNodePtr node, Map *map, int layer)
         }
         else {
             // Read plain XML map file
-            for (xmlNodePtr n2 = node->xmlChildrenNode; n2; n2 = n2->next) {
-                if (!xmlStrEqual(n2->name, BAD_CAST "tile"))
+            for_each_xml_child_node(childNode2, childNode)
+            {
+                if (!xmlStrEqual(childNode2->name, BAD_CAST "tile"))
                     continue;
 
-                int gid = XML::getProperty(n2, "gid", -1);
+                int gid = XML::getProperty(childNode2, "gid", -1);
                 map->setTileWithGid(x, y, layer, gid);
 
                 x++;
@@ -392,11 +394,12 @@ MapReader::readTileset(xmlNodePtr node,
     int tw = XML::getProperty(node, "tilewidth", map->getTileWidth());
     int th = XML::getProperty(node, "tileheight", map->getTileHeight());
 
-    for (node = node->xmlChildrenNode; node; node = node->next) {
-        if (!xmlStrEqual(node->name, BAD_CAST "image"))
+    for_each_xml_child_node(childNode, node)
+    {
+        if (!xmlStrEqual(childNode->name, BAD_CAST "image"))
             continue;
 
-        xmlChar* source = xmlGetProp(node, BAD_CAST "source");
+        xmlChar* source = xmlGetProp(childNode, BAD_CAST "source");
 
         if (source)
         {
diff --git a/src/resources/monsterdb.cpp b/src/resources/monsterdb.cpp
index e4406f9c..ac3ac3bc 100644
--- a/src/resources/monsterdb.cpp
+++ b/src/resources/monsterdb.cpp
@@ -18,7 +18,7 @@
  *  along with The Mana World; if not, write to the Free Software

  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

  *

- *  $Id:

+ *  $Id$

  */

 

 #include "monsterdb.h"

@@ -72,11 +72,8 @@ MonsterDB::load()
     }

 

     //iterate <monster>s

-    for (   xmlNodePtr monsterNode = rootNode->xmlChildrenNode;

-            monsterNode != NULL;

-            monsterNode = monsterNode->next)

+    for_each_xml_child_node(monsterNode, rootNode)

     {

-

         if (!xmlStrEqual(monsterNode->name, BAD_CAST "monster"))

         {

             continue;

@@ -87,9 +84,7 @@ MonsterDB::load()
         currentInfo->setName (XML::getProperty(monsterNode, "name", "unnamed"));

 

         //iterate <sprite>s and <sound>s

-        for (   xmlNodePtr spriteNode = monsterNode->xmlChildrenNode;

-                spriteNode != NULL;

-                spriteNode = spriteNode->next)

+        for_each_xml_child_node(spriteNode, monsterNode)

         {

             if (xmlStrEqual(spriteNode->name, BAD_CAST "sprite"))

             {

diff --git a/src/resources/spritedef.cpp b/src/resources/spritedef.cpp
index d29bd847..24156be1 100644
--- a/src/resources/spritedef.cpp
+++ b/src/resources/spritedef.cpp
@@ -76,22 +76,22 @@ SpriteDef::load(const std::string &animationFile, int variant)
                 "Animation: Error while parsing animation definition file!");
     }
 
-    xmlNodePtr node = xmlDocGetRootElement(doc);
-    if (!node || !xmlStrEqual(node->name, BAD_CAST "sprite")) {
+    xmlNodePtr rootNode = xmlDocGetRootElement(doc);
+    if (!rootNode || !xmlStrEqual(rootNode->name, BAD_CAST "sprite")) {
         logger->error(
                 "Animation: this is not a valid animation definition file!");
     }
 
     // Get the variant
-    int variant_num = XML::getProperty(node, "variants", 0);
+    int variant_num = XML::getProperty(rootNode, "variants", 0);
     int variant_offset = 0;
 
     if (variant_num > 0 && variant < variant_num)
     {
-        variant_offset = variant * XML::getProperty(node, "variant_offset", 0);
+        variant_offset = variant * XML::getProperty(rootNode, "variant_offset", 0);
     }
 
-    for (node = node->xmlChildrenNode; node != NULL; node = node->next)
+    for_each_xml_child_node(node, rootNode)
     {
         if (xmlStrEqual(node->name, BAD_CAST "imageset"))
         {
@@ -177,9 +177,7 @@ SpriteDef::loadAction(xmlNodePtr node, int variant_offset)
     }
 
     // Load animations
-    for (xmlNodePtr animationNode = node->xmlChildrenNode;
-            animationNode != NULL;
-            animationNode = animationNode->next)
+    for_each_xml_child_node(animationNode, node)
     {
         if (xmlStrEqual(animationNode->name, BAD_CAST "animation"))
         {
@@ -208,9 +206,7 @@ SpriteDef::loadAnimation(xmlNodePtr animationNode,
     action->setAnimation(directionType, animation);
 
     // Get animation frames
-    for (xmlNodePtr frameNode = animationNode->xmlChildrenNode;
-            frameNode != NULL;
-            frameNode = frameNode->next)
+    for_each_xml_child_node(frameNode, animationNode)
     {
         int delay = XML::getProperty(frameNode, "delay", 0);
         int offsetX = XML::getProperty(frameNode, "offsetX", 0);
diff --git a/src/utils/xml.h b/src/utils/xml.h
index 54ed9951..db4c264a 100644
--- a/src/utils/xml.h
+++ b/src/utils/xml.h
@@ -28,6 +28,9 @@
 
 #include <string>
 
+/**
+ * XML helper functions.
+ */
 namespace XML
 {
     /**
@@ -43,4 +46,7 @@ namespace XML
     getProperty(xmlNodePtr node, const char *name, const std::string &def);
 }
 
+#define for_each_xml_child_node(var, parent) \
+    for (xmlNodePtr var = parent->xmlChildrenNode; var; var = var->next)
+
 #endif