Upgraded Skill Map loading.

- Added a new 'default' boolean parameter in mana-skills.xml.
  If set to true, unknown weapon types will be defaulted
  to the given value.
- Added better checks on skill id and names and improved error
  reporting.
- Corrected minor typos, and made small cleanups.

Reviewed-by: jaxad0127

4 files changed, 151 insertions, 19 deletions

diff --git a/src/game-server/itemmanager.cpp b/src/game-server/itemmanager.cpp
index c175044..f296225 100644
--- a/src/game-server/itemmanager.cpp
+++ b/src/game-server/itemmanager.cpp
@@ -97,7 +97,7 @@ void ItemManager::reload()
         {
             LOG_WARN(itemReferenceFile << ": Unknown item type \"" << sItemType
                      << "\" for item #" << id <<
-                     " - treating it as \"generic\"");
+                     " - treating it as \"generic\".");
             itemType = ITEM_UNUSABLE;
         }
 
@@ -133,12 +133,13 @@ void ItemManager::reload()
             std::string strWeaponType = XML::getProperty(node, "weapon-type", "");
             if (strWeaponType == "")
             {
-                LOG_WARN(itemReferenceFile << ": Unknown weapon type \""
+                LOG_WARN(itemReferenceFile << ": Empty weapon type \""
                          << "\" for item #" << id <<
-                         " - treating it as generic item");
-            } else {
-                weaponType = SkillManager::getIdFromString(strWeaponType);
+                         " - treating it as generic item.");
             }
+            else
+                weaponType = SkillManager::getIdFromString(strWeaponType);
+
             modifiers.setValue(MOD_WEAPON_TYPE, weaponType);
             modifiers.setValue(MOD_WEAPON_RANGE, XML::getProperty(node, "range",   0));
             modifiers.setValue(MOD_ELEMENT_TYPE, XML::getProperty(node, "element", 0));
diff --git a/src/game-server/skillmanager.cpp b/src/game-server/skillmanager.cpp
index 4d262a4..dd9ee51 100644
--- a/src/game-server/skillmanager.cpp
+++ b/src/game-server/skillmanager.cpp
@@ -30,6 +30,7 @@
 typedef std::map< std::string, int > SkillMap;
 static SkillMap skillMap;
 static std::string skillReferenceFile;
+static std::string defaultSkillKey = std::string();
 
 void SkillManager::initialize(const std::string &file)
 {
@@ -49,8 +50,9 @@ void SkillManager::reload()
 
     std::string absPathFile = ResourceManager::resolve(skillReferenceFile);
 
-    if (!data) {
-        LOG_ERROR("Item Manager: Could not find " << skillReferenceFile << "!");
+    if (!data)
+    {
+        LOG_ERROR("Skill Manager: Could not find " << skillReferenceFile << "!");
         free(data);
         return;
     }
@@ -84,38 +86,128 @@ void SkillManager::reload()
         {
             if (xmlStrEqual(skillnode->name, BAD_CAST "skill"))
             {
-                std::string name = XML::getProperty(skillnode, "name", std::string());
+                std::string name = XML::getProperty(skillnode, "name",
+                                                    std::string());
                 name = utils::toupper(name);
                 int id = XML::getProperty(skillnode, "id", 0);
                 if (id && !name.empty())
                 {
-                    skillMap[utils::toupper(name)] = id;
+                    bool duplicateKey = false;
+                    for (SkillMap::iterator i = skillMap.begin();
+                         i != skillMap.end(); i++)
+                    {
+                        if (id == i->second)
+                        {
+                            LOG_ERROR("SkillManager: The same id: " << id
+                            << " is given for skill names: " << i->first
+                            << " and " << name);
+                            LOG_ERROR("The skill reference: " << "'" << name
+                            << "': " << id << " will be ignored.");
+
+                            duplicateKey = true;
+                            break;
+                        }
+                    }
+
+                    if (!duplicateKey)
+                    {
+                        if (XML::getBoolProperty(skillnode, "default", false))
+                        {
+                            if (!defaultSkillKey.empty())
+                            {
+                                LOG_WARN("SkillManager: "
+                                "Default Skill Key already defined as "
+                                << defaultSkillKey
+                                << ". Redefinit it as: " << name);
+                            }
+                            else
+                            {
+                                LOG_INFO("SkillManager: Defining " << name
+                                << " as default weapon-type key.");
+                            }
+                            defaultSkillKey = name;
+                        }
+                        skillMap[name] = id;
+                    }
                 }
             }
         }
     }
 
-    LOG_DEBUG("skill map:");
-    for (SkillMap::iterator i = skillMap.begin(); i != skillMap.end(); i++)
+    if (::utils::Logger::mVerbosity >= ::utils::Logger::Debug)
     {
-        LOG_DEBUG("  " << i->first << " : " << i->second);
+        LOG_DEBUG("Skill map in " << skillReferenceFile << ":"
+                  << std::endl << "-----");
+        for (SkillMap::iterator i = skillMap.begin(); i != skillMap.end(); i++)
+        {
+            if (!defaultSkillKey.compare(i->first))
+            {
+                LOG_DEBUG("'" << i->first << "': " << i->second
+                          << " (Default)");
+            }
+            else
+            {
+                LOG_DEBUG("'" << i->first << "': " << i->second);
+            }
+        }
+        LOG_DEBUG("-----");
     }
+
+    if (defaultSkillKey.empty())
+        LOG_WARN("SkillManager: No default weapon-type id was given during "
+                 "Skill map loading. Defaults will fall back to id 0.");
+
+    LOG_INFO("Loaded " << skillMap.size() << " skill references from "
+             << absPathFile);
 }
 
 int SkillManager::getIdFromString(const std::string &name)
 {
-    //check if already an integer, if yes just return it
-    int val;
-    val = atoi(name.c_str());
-    if (val)
-        return val;
+    // Check if the name is an integer value.
+    if (utils::isNumeric(name))
+    {
+        int val = 0;
+        val = utils::stringToInt(name);
+        if (val)
+        {
+            for (SkillMap::iterator i = skillMap.begin(); i != skillMap.end(); i++)
+            {
+                if (i->second == val)
+                    return val;
+            }
+            LOG_WARN("SkillManager::getIdFromString(): Numeric weapon-type id "
+            << val << " not found into " << skillReferenceFile);
+
+            SkillMap::iterator i = skillMap.find(defaultSkillKey);
+            if (i != skillMap.end())
+            {
+                LOG_WARN("Id defaulted to " << defaultSkillKey << ": "
+                << i->second);
+                return i->second;
+            }
+            else
+            {
+                LOG_WARN("Id defaulted to 0.");
+                return 0;
+            }
+        }
+        else
+        {
+            LOG_WARN("SkillManager: Invalid skill id " << name);
+            return 0;
+        }
+    }
 
-    // convert to upper case for easier finding
+    // Convert to upper case for easier finding
     SkillMap::iterator i = skillMap.find(utils::toupper(name));
     if (i == skillMap.end())
     {
+        LOG_WARN("SkillManager: No weapon-type name corresponding to "
+                 << utils::toupper(name) << " into " << skillReferenceFile);
         return 0;
-    } else {
+    }
+    else
+    {
         return i->second;
     }
 }
diff --git a/src/utils/xml.cpp b/src/utils/xml.cpp
index 659f928..8bc9ebe 100644
--- a/src/utils/xml.cpp
+++ b/src/utils/xml.cpp
@@ -96,6 +96,35 @@ namespace XML
         return mDoc ? xmlDocGetRootElement(mDoc) : 0;
     }
 
+    bool hasProperty(xmlNodePtr node, const char *name)
+    {
+        xmlChar *prop = xmlGetProp(node, BAD_CAST name);
+        if (prop)
+        {
+            xmlFree(prop);
+            return true;
+        }
+
+        return false;
+    }
+
+    bool getBoolProperty(xmlNodePtr node, const char *name, bool def)
+    {
+        bool ret = def;
+        xmlChar *prop = xmlGetProp(node, BAD_CAST name);
+        if (prop)
+        {
+            if (xmlStrEqual(prop, BAD_CAST "true")
+                ||xmlStrEqual(prop, BAD_CAST "yes"))
+                ret = true;
+            if (xmlStrEqual(prop, BAD_CAST "false")
+                ||xmlStrEqual(prop, BAD_CAST "no"))
+                ret = false;
+            xmlFree(prop);
+        }
+        return ret;
+    }
+
     int getProperty(xmlNodePtr node, const char *name, int def)
     {
         int &ret = def;
diff --git a/src/utils/xml.hpp b/src/utils/xml.hpp
index ee37b48..eba88e5 100644
--- a/src/utils/xml.hpp
+++ b/src/utils/xml.hpp
@@ -70,6 +70,16 @@ namespace XML
     };
 
     /**
+     * Tells if a property from an xmlNodePtr exists.
+     */
+    bool hasProperty(xmlNodePtr node, const char *name);
+
+    /**
+     * Gets a boolean property from an xmlNodePtr.
+     */
+    bool getBoolProperty(xmlNodePtr node, const char *name, bool def);
+
+    /**
      * Gets an integer property from an xmlNodePtr.
      */
     int getProperty(xmlNodePtr node, const char *name, int def);