Refactor|Client: Simplified handling of remote mobjs

The so called "clmobjs" are now handled like regular mobjs. The only
difference is that they have a NetworkState present in their private
data.

Cleaned up the way mobjs are deleted, since clmobjs no longer need
special handling.

Map uses observers to remove mobjs from the client mobj hash.

doomsday/client/include/client/cl_mobj.h

@@ -45,12 +45,6 @@ ClientMobjThinkerData::NetworkState *ClMobj_GetInfo(mobj_t *mo);
  */
 dd_bool ClMobj_Reveal(mobj_t *cmo);
 
-/**
- * Unlinks the mobj from sectorlinks and if the object is solid,
- * the blockmap.
- */
-void ClMobj_Unlink(mobj_t *cmo); // needed?
-
 /**
  * Links the mobj into sectorlinks and if the object is solid, the
  * blockmap. Linking to sectorlinks makes the mobj visible and linking

doomsday/client/include/world/map.h

@@ -872,12 +872,6 @@ class Map
      */
     mobj_t *clMobjFor(thid_t id, bool canCreate = false) const;
 
-    /**
-     * Destroys the client mobj. Before this is called, the client mobj should be
-     * unlinked from the thinker list by calling <code>thinkers().remove()</code>.
-     */
-    void deleteClMobj(mobj_t *mo);
-
     /**
      * Iterate all client mobjs, making a callback for each. Iteration ends if a
      * callback returns a non-zero value.

doomsday/client/src/client/cl_mobj.cpp

@@ -67,11 +67,6 @@ mobj_t *ClMobjInfo::mobj()
 }
 #endif
 
-void ClMobj_Unlink(mobj_t *mo)
-{
-    Mobj_Unlink(mo);
-}
-
 void ClMobj_Link(mobj_t *mo)
 {
     ClientMobjThinkerData::NetworkState *info = ClMobj_GetInfo(mo);
@@ -216,8 +211,7 @@ void Cl_UpdateRealPlayerMobj(mobj_t *localMobj, mobj_t *remoteClientMobj,
 
 dd_bool Cl_IsClientMobj(mobj_t const *mo)
 {
-    if(ClientMobjThinkerData *data = reinterpret_cast<Thinker::IData *>(mo->thinker.d)
-            ->maybeAs<ClientMobjThinkerData>())
+    if(ClientMobjThinkerData *data = THINKER_DATA_MAYBE(mo->thinker, ClientMobjThinkerData))
     {
         return data->hasNetworkState();
     }
@@ -247,8 +241,7 @@ ClientMobjThinkerData::NetworkState *ClMobj_GetInfo(mobj_t *mo)
 {
     if(!mo) return 0;
 
-    if(ClientMobjThinkerData *data = reinterpret_cast<Thinker::IData *>(mo->thinker.d)
-            ->maybeAs<ClientMobjThinkerData>())
+    if(ClientMobjThinkerData *data = THINKER_DATA_MAYBE(mo->thinker, ClientMobjThinkerData))
     {
         if(!data->hasNetworkState()) return 0;
         return &data->networkState();
@@ -397,7 +390,7 @@ void ClMobj_ReadDelta()
        !justCreated && !d->dPlayer)
     {
         needsLinking = true;
-        ClMobj_Unlink(mo);
+        Mobj_Unlink(mo);
     }
 
     // Remember where the mobj used to be in case we need to cancel a move.
@@ -561,7 +554,7 @@ void ClMobj_ReadDelta()
             if(ClMobj_IsStuckInsideLocalPlayer(mo))
             {
                 // Oopsie, on second thought we shouldn't do this move.
-                ClMobj_Unlink(mo);
+                Mobj_Unlink(mo);
                 V3d_Copy(mo->origin, oldState->origin);
                 mo->floorZ = oldState->floorZ;
                 mo->ceilingZ = oldState->ceilingZ;
@@ -606,7 +599,7 @@ void ClMobj_ReadNullDelta()
     // Get rid of this mobj.
     if(!mo->dPlayer)
     {
-        ClMobj_Unlink(mo);
+        Mobj_Unlink(mo);
     }
     else
     {

doomsday/client/src/client/cl_player.cpp

@@ -340,7 +340,7 @@ void ClPlayer_ReadDelta()
             {
                 // The client mobj is already known to us.
                 // Unlink it (not interactive or visible).
-                ClMobj_Unlink(clmo);
+                Mobj_Unlink(clmo);
             }
 
             clmo->dPlayer = ddpl;

doomsday/client/src/world/map.cpp

@@ -108,6 +108,9 @@ struct EditableElements
 
 DENG2_PIMPL(Map)
 , DENG2_OBSERVES(bsp::Partitioner, UnclosedSectorFound)
+#ifdef __CLIENT__
+, DENG2_OBSERVES(ThinkerData, Deletion)
+#endif
 {
     bool editingEnabled;
     EditableElements editable;
@@ -330,6 +333,12 @@ DENG2_PIMPL(Map)
         {
             Z_Free(clPolyMovers.takeFirst());
         }
+
+        // Stop observing client mobjs.
+        foreach(mobj_t *mo, clMobjHash)
+        {
+            THINKER_DATA(mo->thinker, ThinkerData).audienceForDeletion() -= this;
+        }
 #endif
 
         /// @todo fixme: Free all memory we have ownership of.
@@ -1588,6 +1597,11 @@ DENG2_PIMPL(Map)
             }
         }
     }
+
+    void thinkerBeingDeleted(thinker_s &th)
+    {
+        clMobjHash.remove(th.id);
+    }
 #endif // __CLIENT__
 };
 
@@ -2039,55 +2053,36 @@ mobj_t *Map::clMobjFor(thid_t id, bool canCreate) const
     // Create a new client mobj. This is a regular mobj that has network state
     // associated with it.
 
-    /*
-    void *data       = Z_Malloc(sizeof(ClMobjInfo) + MOBJ_SIZE, PU_MAP, 0);
-    ClMobjInfo *info = new (data) ClMobjInfo;
-    DENG2_UNUSED(info);
-    mobj_t *mo       = (mobj_t *) ((char *)data + sizeof(ClMobjInfo));
-
-    std::memset(mo, 0, MOBJ_SIZE);
-    mo->ddFlags = DDMF_REMOTE;*/
-
     MobjThinker mo(Thinker::AllocateMemoryZone);
+    mo.id = id;
+    mo.function = reinterpret_cast<thinkfunc_t>(gx.MobjThinker);
+
     ClientMobjThinkerData *data = new ClientMobjThinkerData(mo);
     data->networkState().flags = DDMF_REMOTE;
     mo.setData(data);
 
     d->clMobjHash.insert(id, mo);
+    data->audienceForDeletion() += d; // for removing from the hash
+
     d->thinkers->setMobjId(id); // Mark this ID as used.
 
     // Client mobjs are full-fludged game mobjs as well.
-    mo->thinker.function = reinterpret_cast<thinkfunc_t>(gx.MobjThinker);
     d->thinkers->add(*(thinker_t *)mo);
 
     return mo.take();
 }
 
-void Map::deleteClMobj(mobj_t *mo)
-{
-    LOG_AS("Map::deleteClMobj");
-
-    if(!mo) return;
-
-    LOG_NET_XVERBOSE("mobj %i being destroyed") << mo->thinker.id;
-
-    CL_ASSERT_CLMOBJ(mo);
-
-    // Stop any sounds originating from this mobj.
-    S_StopSound(0, mo);
-
-    // The ID is free once again.
-    d->thinkers->setMobjId(mo->thinker.id, false);
-    d->clMobjHash.remove(mo->thinker.id);
-    ClMobj_Unlink(mo); // from block/sector
-
-    MobjThinker::destroy(mo); // delete the mobj + private data
-}
-
 int Map::clMobjIterator(int (*callback)(mobj_t *, void *), void *context)
 {
-    DENG2_FOR_EACH_CONST(ClMobjHash, i, d->clMobjHash)
+    ClMobjHash::const_iterator next;
+    for(ClMobjHash::const_iterator i = d->clMobjHash.constBegin();
+        i != d->clMobjHash.constEnd(); i = next)
     {
+        next = i;
+        next++;
+
+        DENG2_ASSERT(THINKER_DATA(i.value()->thinker, ClientMobjThinkerData).hasNetworkState());
+
         // Callback returns zero to continue.
         if(int result = callback(i.value(), context))
             return result;

doomsday/client/src/world/thinkers.cpp

@@ -424,21 +424,8 @@ static int runThinker(thinker_t *th, void * /*context*/)
 
         if(th->id)
         {
-            mobj_t *mo = (mobj_t *) th;
-#ifdef __CLIENT__
-            if(!Cl_IsClientMobj(mo))
-            {
-                // It's a regular mobj: recycle for reduced allocation overhead.
-                P_MobjRecycle(mo);
-            }
-            else
-            {
-                // Delete the client mobj.
-                Mobj_Map(*mo).deleteClMobj(mo);
-            }
-#else
-            P_MobjRecycle(mo);
-#endif
+            // Recycle for reduced allocation overhead.
+            P_MobjRecycle((mobj_t *) th);
         }
         else
         {