** Cleanup in ObjectAccessor/HashMapHolder:

* Correctly lock the HashMapHolder on access.
* Use ObjectAccessor wrappers (GetPlayers, GetCreatures, GetGameObjects) instead of raw HashMapHolder<Type>::GetContainer.
* General code style cleanup.

--HG--
branch : trunk

1 files changed, 54 insertions, 49 deletions

diff --git a/src/game/ObjectAccessor.h b/src/game/ObjectAccessor.h
index 9667686bb40..9134dd730f7 100644
--- a/src/game/ObjectAccessor.h
+++ b/src/game/ObjectAccessor.h
@@ -48,9 +48,9 @@ class HashMapHolder
 {
     public:
 
-        typedef UNORDERED_MAP< uint64, T* >   MapType;
+        typedef UNORDERED_MAP<uint64, T*> MapType;
         typedef ACE_Thread_Mutex LockType;
-        typedef Trinity::GeneralLock<LockType > Guard;
+        typedef Trinity::GeneralLock<LockType> Guard;
 
         static void Insert(T* o)
         {
@@ -74,6 +74,7 @@ class HashMapHolder
         static MapType& GetContainer() { return m_objectMap; }
 
         static LockType* GetLock() { return &i_lock; }
+
     private:
 
         //Non instanceable only static
@@ -85,15 +86,15 @@ class HashMapHolder
 
 class ObjectAccessor : public Trinity::Singleton<ObjectAccessor, Trinity::ClassLevelLockable<ObjectAccessor, ACE_Thread_Mutex> >
 {
-
     friend class Trinity::OperatorNew<ObjectAccessor>;
     ObjectAccessor();
     ~ObjectAccessor();
-    ObjectAccessor(const ObjectAccessor &);
-    ObjectAccessor& operator=(const ObjectAccessor &);
+    ObjectAccessor(const ObjectAccessor&);
+    ObjectAccessor& operator=(const ObjectAccessor&);
 
     public:
-        typedef UNORDERED_MAP<uint64, Corpse* >      Player2CorpsesMapType;
+
+        typedef UNORDERED_MAP<uint64, Corpse*> Player2CorpsesMapType;
         typedef UNORDERED_MAP<Player*, UpdateData>::value_type UpdateDataValueType;
 
         template<class T> static T* GetObjectInWorld(uint64 guid, T* /*fake*/)
@@ -103,19 +104,19 @@ class ObjectAccessor : public Trinity::Singleton<ObjectAccessor, Trinity::ClassL
 
         static Unit* GetObjectInWorld(uint64 guid, Unit* /*fake*/)
         {
-            if(!guid)
+            if (!guid)
                 return NULL;
 
             if (IS_PLAYER_GUID(guid))
             {
-                Unit * u = (Unit*)HashMapHolder<Player>::Find(guid);
-                if(!u || !u->IsInWorld())
+                Unit* u = (Unit*)HashMapHolder<Player>::Find(guid);
+                if (!u || !u->IsInWorld())
                     return NULL;
 
                 return u;
             }
 
-            if(IS_PET_GUID(guid))
+            if (IS_PET_GUID(guid))
                 return (Unit*)HashMapHolder<Pet>::Find(guid);
 
             return (Unit*)HashMapHolder<Creature>::Find(guid);
@@ -123,34 +124,36 @@ class ObjectAccessor : public Trinity::Singleton<ObjectAccessor, Trinity::ClassL
 
         static Unit* GetUnitInOrOutOfWorld(uint64 guid, Unit* /*fake*/)
         {
-            if(!guid)
+            if (!guid)
                 return NULL;
 
             if (IS_PLAYER_GUID(guid))
             {
-                Unit * u = (Unit*)HashMapHolder<Player>::Find(guid);
+                Unit* u = (Unit*)HashMapHolder<Player>::Find(guid);
                 if(!u)
                     return NULL;
 
                 return u;
             }
+
             // Other object types than player are unloaded while out of world
-            return GetObjectInWorld(guid, ((Unit*)NULL));
+            return GetObjectInWorld(guid, (Unit*)NULL);
         }
 
         template<class T> static T* GetObjectInWorld(uint32 mapid, float x, float y, uint64 guid, T* /*fake*/)
         {
             T* obj = HashMapHolder<T>::Find(guid);
-            if(!obj || obj->GetMapId() != mapid) return NULL;
+            if (!obj || obj->GetMapId() != mapid)
+                return NULL;
 
-            CellPair p = Trinity::ComputeCellPair(x,y);
-            if(p.x_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP || p.y_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP )
+            CellPair p = Trinity::ComputeCellPair(x, y);
+            if (p.x_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP || p.y_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP )
             {
                 sLog.outError("ObjectAccessor::GetObjectInWorld: invalid coordinates supplied X:%f Y:%f grid cell [%u:%u]", x, y, p.x_coord, p.y_coord);
                 return NULL;
             }
 
-            CellPair q = Trinity::ComputeCellPair(obj->GetPositionX(),obj->GetPositionY());
+            CellPair q = Trinity::ComputeCellPair(obj->GetPositionX(), obj->GetPositionY());
             if(q.x_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP || q.y_coord >= TOTAL_NUMBER_OF_CELLS_PER_MAP )
             {
                 sLog.outError("ObjectAccessor::GetObjecInWorld: object (GUID: %u TypeId: %u) has invalid coordinates X:%f Y:%f grid cell [%u:%u]", obj->GetGUIDLow(), obj->GetTypeId(), obj->GetPositionX(), obj->GetPositionY(), q.x_coord, q.y_coord);
@@ -160,65 +163,68 @@ class ObjectAccessor : public Trinity::Singleton<ObjectAccessor, Trinity::ClassL
             int32 dx = int32(p.x_coord) - int32(q.x_coord);
             int32 dy = int32(p.y_coord) - int32(q.y_coord);
 
-            if (dx > -2 && dx < 2 && dy > -2 && dy < 2) return obj;
-            else return NULL;
+            if (dx > -2 && dx < 2 && dy > -2 && dy < 2)
+                return obj;
+            else
+                return NULL;
         }
 
-        static WorldObject* GetWorldObject(WorldObject const &, uint64);
-        static Object*   GetObjectByTypeMask(WorldObject const &, uint64, uint32 typemask);
-        static Creature* GetCreatureOrPetOrVehicle(WorldObject const &, uint64);
-        static Unit* GetUnit(WorldObject const &, uint64 guid) { return GetObjectInWorld(guid, (Unit*)NULL); }
-        static Unit* GetUnitInOrOutOfWorld(WorldObject const &, uint64 guid) { return GetUnitInOrOutOfWorld(guid, (Unit*)NULL); }
-        static Pet* GetPet(Unit const &, uint64 guid) { return GetPet(guid); }
-        static Player* GetPlayer(Unit const &, uint64 guid) { return FindPlayer(guid); }
-        static Corpse* GetCorpse(WorldObject const &u, uint64 guid);
+        static WorldObject* GetWorldObject(WorldObject const&, uint64);
+        static Object* GetObjectByTypeMask(WorldObject const&, uint64, uint32 typemask);
+        static Creature* GetCreatureOrPetOrVehicle(WorldObject const&, uint64);
+        static Unit* GetUnit(WorldObject const&, uint64 guid) { return GetObjectInWorld(guid, (Unit*)NULL); }
+        static Unit* GetUnitInOrOutOfWorld(WorldObject const&, uint64 guid) { return GetUnitInOrOutOfWorld(guid, (Unit*)NULL); }
+        static Pet* GetPet(Unit const&, uint64 guid) { return GetPet(guid); }
+        static Player* GetPlayer(Unit const&, uint64 guid) { return FindPlayer(guid); }
+        static Corpse* GetCorpse(WorldObject const& u, uint64 guid);
         static Pet* GetPet(uint64 guid);
         static Player* FindPlayer(uint64);
 
-        Player* FindPlayerByName(const char *name) ;
+        Player* FindPlayerByName(const char* name) ;
 
+        // when using this, you must use the hashmapholder's lock
         HashMapHolder<Player>::MapType& GetPlayers()
         {
             return HashMapHolder<Player>::GetContainer();
         }
 
+        // when using this, you must use the hashmapholder's lock
         HashMapHolder<Creature>::MapType& GetCreatures()
         {
             return HashMapHolder<Creature>::GetContainer();
         }
 
+        // when using this, you must use the hashmapholder's lock
         HashMapHolder<GameObject>::MapType& GetGameObjects()
         {
             return HashMapHolder<GameObject>::GetContainer();
         }
 
-        template<class T> void AddObject(T *object)
+        template<class T> void AddObject(T* object)
         {
             HashMapHolder<T>::Insert(object);
         }
 
-        template<class T> void RemoveObject(T *object)
+        template<class T> void RemoveObject(T* object)
         {
             HashMapHolder<T>::Remove(object);
         }
 
-        void RemoveObject(Player *pl)
+        void RemoveObject(Player* pl)
         {
             HashMapHolder<Player>::Remove(pl);
-
-            Guard guard(i_updateGuard);
-            i_objects.erase((Object *)pl);
+            RemoveUpdateObject((Object*)pl);
         }
 
         void SaveAllPlayers();
 
-        void AddUpdateObject(Object *obj)
+        void AddUpdateObject(Object* obj)
         {
             Guard guard(i_updateGuard);
             i_objects.insert(obj);
         }
 
-        void RemoveUpdateObject(Object *obj)
+        void RemoveUpdateObject(Object* obj)
         {
             Guard guard(i_updateGuard);
             i_objects.erase( obj );
@@ -227,26 +233,25 @@ class ObjectAccessor : public Trinity::Singleton<ObjectAccessor, Trinity::ClassL
         void Update(uint32 diff);
 
         Corpse* GetCorpseForPlayerGUID(uint64 guid);
-        void RemoveCorpse(Corpse *corpse);
-        void AddCorpse(Corpse *corpse);
-        void AddCorpsesToGrid(GridPair const& gridpair,GridType& grid,Map* map);
+        void RemoveCorpse(Corpse* corpse);
+        void AddCorpse(Corpse* corpse);
+        void AddCorpsesToGrid(GridPair const& gridpair, GridType& grid, Map* map);
         Corpse* ConvertCorpseForPlayer(uint64 player_guid, bool insignia = false);
 
-        //static void UpdateVisibilityForPlayer(Player* player);
+        typedef ACE_Thread_Mutex LockType;
+        typedef Trinity::GeneralLock<LockType> Guard;
+
     private:
 
-        Player2CorpsesMapType   i_player2corpse;
+        Player2CorpsesMapType i_player2corpse;
 
-        typedef ACE_Thread_Mutex LockType;
-        typedef Trinity::GeneralLock<LockType > Guard;
+        static void _buildChangeObjectForPlayer(WorldObject*, UpdateDataMapType&);
+        static void _buildPacket(Player*, Object*, UpdateDataMapType&);
+        void _update();
+
+        std::set<Object*> i_objects;
 
-        static void _buildChangeObjectForPlayer(WorldObject *, UpdateDataMapType &);
-        static void _buildPacket(Player *, Object *, UpdateDataMapType &);
-        void _update(void);
-        std::set<Object *> i_objects;
-        LockType i_playerGuard;
         LockType i_updateGuard;
         LockType i_corpseGuard;
-        LockType i_petGuard;
 };
 #endif