Use a map to quickly find items and monsters by their name

Introduced a template class NameMap, which provides a nice API for
mapping any custom types by their name.

This change also makes any duplicate item or monster definitions be
complete ignored, rather than being merged into the first definition.

Reviewed-by: Philipp Sehmisch
Reviewed-by: Yohann Ferreira

4 files changed, 54 insertions, 58 deletions

diff --git a/src/game-server/itemmanager.cpp b/src/game-server/itemmanager.cpp
index 02729684..b9ac2feb 100644
--- a/src/game-server/itemmanager.cpp
+++ b/src/game-server/itemmanager.cpp
@@ -27,7 +27,6 @@
 #include "game-server/skillmanager.h"
 #include "scripting/script.h"
 #include "utils/logger.h"
-#include "utils/string.h"
 
 #include <map>
 #include <set>
@@ -53,6 +52,7 @@ void ItemManager::deinitialize()
         delete i->second;
     }
     mItemClasses.clear();
+    mItemClassesByName.clear();
 }
 
 ItemClass *ItemManager::getItem(int itemId) const
@@ -61,18 +61,9 @@ ItemClass *ItemManager::getItem(int itemId) const
     return i != mItemClasses.end() ? i->second : 0;
 }
 
-ItemClass *ItemManager::getItemByName(std::string name) const
+ItemClass *ItemManager::getItemByName(const std::string &name) const
 {
-    name = utils::toLower(name);
-    for (ItemClasses::const_iterator i = mItemClasses.begin(),
-         i_end = mItemClasses.end(); i != i_end; ++i)
-    {
-        if (utils::toLower(i->second->getName()) == name)
-        {
-            return i->second;
-        }
-    }
-    return 0;
+    return mItemClassesByName.find(name);
 }
 
 unsigned int ItemManager::getDatabaseVersion() const
@@ -229,15 +220,20 @@ void ItemManager::readItemNode(xmlNodePtr itemNode)
         return;
     }
 
-    // Type is mostly unused, but still serves for
-    // hairsheets and race sheets.
-    std::string sItemType = XML::getProperty(itemNode, "type", std::string());
-    if (sItemType == "hairsprite" || sItemType == "racesprite")
+    // Type is mostly unused, but still serves for hairsheets and race sheets
+    const std::string type = XML::getProperty(itemNode, "type", std::string());
+    if (type == "hairsprite" || type == "racesprite")
         return;
 
-    ItemClass *item;
     ItemClasses::iterator i = mItemClasses.find(id);
 
+    if (i != mItemClasses.end())
+    {
+        LOG_WARN("Item Manager: Ignoring duplicate definition of item '" << id
+                 << "'!");
+        return;
+    }
+
     unsigned int maxPerSlot = XML::getProperty(itemNode, "max-per-slot", 0);
     if (!maxPerSlot)
     {
@@ -246,19 +242,19 @@ void ItemManager::readItemNode(xmlNodePtr itemNode)
         maxPerSlot = 1;
     }
 
-    if (i == mItemClasses.end())
-    {
-        item = new ItemClass(id, maxPerSlot);
-        mItemClasses[id] = item;
-    }
-    else
+    ItemClass *item = new ItemClass(id, maxPerSlot);
+    mItemClasses.insert(std::make_pair(id, item));
+
+    const std::string name = XML::getProperty(itemNode, "name", std::string());
+    if (!name.empty())
     {
-        LOG_WARN("Multiple defintions of item '" << id << "'!");
-        item = i->second;
-    }
+        item->setName(name);
 
-    std::string name = XML::getProperty(itemNode, "name", "unnamed");
-    item->setName(name);
+        if (mItemClassesByName.contains(name))
+            LOG_WARN("Item Manager: Name not unique for item " << id);
+        else
+            mItemClassesByName.insert(name, item);
+    }
 
     int value = XML::getProperty(itemNode, "value", 0);
     // Should have multiple value definitions for multiple currencies?
diff --git a/src/game-server/itemmanager.h b/src/game-server/itemmanager.h
index fa5ade54..c310df44 100644
--- a/src/game-server/itemmanager.h
+++ b/src/game-server/itemmanager.h
@@ -22,6 +22,7 @@
 #define ITEMMANAGER_H
 
 #include "utils/xml.h"
+#include "utils/string.h"
 
 #include <string>
 #include <map>
@@ -65,7 +66,7 @@ class ItemManager
          * Returns null when there is no item with such
          * a name.
          */
-        ItemClass *getItemByName(std::string name) const;
+        ItemClass *getItemByName(const std::string &name) const;
 
         /**
          * Gets the version of the loaded item database.
@@ -99,6 +100,7 @@ class ItemManager
         typedef std::vector< unsigned int > VisibleEquipSlots;
 
         ItemClasses mItemClasses; /**< Item reference */
+        utils::NameMap<ItemClass*> mItemClassesByName;
         EquipSlots mEquipSlots;
         VisibleEquipSlots mVisibleEquipSlots;
 
diff --git a/src/game-server/monstermanager.cpp b/src/game-server/monstermanager.cpp
index a98126e0..5fb0b675 100644
--- a/src/game-server/monstermanager.cpp
+++ b/src/game-server/monstermanager.cpp
@@ -25,7 +25,6 @@
 #include "game-server/itemmanager.h"
 #include "game-server/monster.h"
 #include "utils/logger.h"
-#include "utils/string.h"
 #include "utils/xml.h"
 
 #define MAX_MUTATION 99
@@ -86,28 +85,36 @@ void MonsterManager::reload()
             continue;
 
         int id = XML::getProperty(node, "id", 0);
-        std::string name = XML::getProperty(node, "name", "unnamed");
+        std::string name = XML::getProperty(node, "name", std::string());
 
         if (id < 1)
         {
-            LOG_WARN("Monster Manager: There is a monster ("
+            LOG_WARN("Monster Manager: Ignoring monster ("
                      << name << ") without Id in "
                      << mMonsterReferenceFile << "! It has been ignored.");
             continue;
         }
 
-        MonsterClass *monster;
         MonsterClasses::iterator i = mMonsterClasses.find(id);
-        if (i == mMonsterClasses.end())
+        if (i != mMonsterClasses.end())
         {
-            monster = new MonsterClass(id);
-            mMonsterClasses[id] = monster;
+            LOG_WARN("Monster Manager: Ignoring duplicate definition of "
+                     "monster '" << id << "'!");
+            continue;
         }
-        else
+
+        MonsterClass *monster = new MonsterClass(id);
+        mMonsterClasses[id] = monster;
+
+        if (!name.empty())
         {
-            monster = i->second;
+            monster->setName(name);
+
+            if (mMonsterClassesByName.contains(name))
+                LOG_WARN("Monster Manager: Name not unique for monster " << id);
+            else
+                mMonsterClassesByName.insert(name, monster);
         }
-        monster->setName(name);
 
         MonsterDrops drops;
         bool attributesSet = false;
@@ -345,28 +352,15 @@ void MonsterManager::deinitialize()
         delete i->second;
     }
     mMonsterClasses.clear();
+    mMonsterClassesByName.clear();
 }
 
-MonsterClass *MonsterManager::getMonsterByName(std::string name) const
+MonsterClass *MonsterManager::getMonsterByName(const std::string &name) const
 {
-    // this function is not very fast but neither does it need to be
-    // because it is only used by the @spawn command. It would be
-    // possible to speed it up by caching the lowercase_name/MonsterClass
-    // mapping in a std::map during MonsterManager::reload, should the
-    // need arise.
-    name = utils::toLower(name);
-    for (MonsterClasses::const_iterator i = mMonsterClasses.begin(),
-         i_end = mMonsterClasses.end(); i != i_end; ++i)
-    {
-        if(utils::toLower(i->second->getName()) == name)
-        {
-            return i->second;
-        }
-    }
-    return 0;
+    return mMonsterClassesByName.find(name);
 }
 
-MonsterClass *MonsterManager::getMonster(int id)
+MonsterClass *MonsterManager::getMonster(int id) const
 {
     MonsterClasses::const_iterator i = mMonsterClasses.find(id);
     return i != mMonsterClasses.end() ? i->second : 0;
diff --git a/src/game-server/monstermanager.h b/src/game-server/monstermanager.h
index acdb9cad..f04a5733 100644
--- a/src/game-server/monstermanager.h
+++ b/src/game-server/monstermanager.h
@@ -23,7 +23,10 @@
 
 #include <string>
 #include <map>
+#include "utils/string.h"
+
 class MonsterClass;
+
 class MonsterManager
 {
     public:
@@ -47,7 +50,7 @@ class MonsterManager
         /**
          * Gets the MonsterClass having the given ID.
          */
-        MonsterClass *getMonster(int id);
+        MonsterClass *getMonster(int id) const;
 
         /**
          * Gets the first monster type with a specific name.
@@ -55,12 +58,13 @@ class MonsterManager
          * Returns null when there is no monster with such
          * a name.
          */
-        MonsterClass *getMonsterByName(std::string name) const;
+        MonsterClass *getMonsterByName(const std::string &name) const;
 
     private:
 
         typedef std::map< int, MonsterClass * > MonsterClasses;
         MonsterClasses mMonsterClasses; /**< Monster reference */
+        utils::NameMap<MonsterClass*> mMonsterClassesByName;
 
         std::string mMonsterReferenceFile;
 };