Ghosting/refCount bug/enhancement
by Anthony Lovell · in Torque Game Engine · 09/10/2006 (1:50 pm) · 3 replies
I fixed two small issues which fall somewhere between bug and feature enhancements.
Issue 1: Object::decRef() does the wrong thing if mRefCount == 0
If a user inappropriately calls decRef() on an Object he never called incRef() on, no error message at all will alert him to this difficult to diagnose error. I think a TNLAssert should alert those running in debug mode to their mistake:
tnlNetBase.h
On the heels of this, I notice that GhostConnection.cpp deletes local ghosts without checking to see if their mRefCount > 0. This, I feel, is a tragedy to those who might wish to keep ghosts around even after the connection they arrived over goes away. One can, I feel, greatly extend the utility of this feature by incRef()ing newly created ghosts, and replacing the 2 places where they might simply be deleted with calls to decRef(). This change would allow the TNL user to keep the ghosted objects around for further use simply by assigning RefPtrs to them.
The fix is super-simple, occuring in 3 lines in GhostConnection.cpp. The first two occur within GhostConnection::readPacket():
and, lower in readPacket():
and one more place in GhostConnection::deleteLocalGhosts():
I suppose it is debatable what one should do with the call to onGhostRemove, in both cases, as it will no longer be inextricably bound to the object's imminent disappearance... it only means that the ghosted reference is going away and that the user may yet retain a copy which has been severed from it's "symbolic link" to a copy maintained on the server.
tone
Issue 1: Object::decRef() does the wrong thing if mRefCount == 0
If a user inappropriately calls decRef() on an Object he never called incRef() on, no error message at all will alert him to this difficult to diagnose error. I think a TNLAssert should alert those running in debug mode to their mistake:
tnlNetBase.h
void decRef()
{
#ifdef TONE_FIX
TNLAssert(mRefCount != 0, "decRef() called when mRefCount == 0");
#endif
mRefCount--;
if(!mRefCount)
destroySelf();
}On the heels of this, I notice that GhostConnection.cpp deletes local ghosts without checking to see if their mRefCount > 0. This, I feel, is a tragedy to those who might wish to keep ghosts around even after the connection they arrived over goes away. One can, I feel, greatly extend the utility of this feature by incRef()ing newly created ghosts, and replacing the 2 places where they might simply be deleted with calls to decRef(). This change would allow the TNL user to keep the ghosted objects around for further use simply by assigning RefPtrs to them.
The fix is super-simple, occuring in 3 lines in GhostConnection.cpp. The first two occur within GhostConnection::readPacket():
if(mLocalGhosts[index])
{
mLocalGhosts[index]->onGhostRemove();
#ifdef TONE_FIX
mLocalGhosts[index]->decRef();
#else
delete mLocalGhosts[index];
#endif
mLocalGhosts[index] = NULL;
} and, lower in readPacket():
// object gets initial update before adding to the manager
obj->mNetIndex = index;
mLocalGhosts[index] = obj;
#ifdef TONE_FIX
// increase its reference count by virtue of its presence in mLocalGhosts
obj->incRef();
#endif
NetObject::mIsInitialUpdate = true;
mLocalGhosts[index]->unpackUpdate(this, bstream);and one more place in GhostConnection::deleteLocalGhosts():
void GhostConnection::deleteLocalGhosts()
{
if(!mLocalGhosts)
return;
// just delete all the local ghosts,
// and delete all the ghosts in the current save list
for(S32 i = 0; i < MaxGhostCount; i++)
{
if(mLocalGhosts[i])
{
mLocalGhosts[i]->onGhostRemove();
#ifdef TONE_FIX
mLocalGhosts[i]->decRef();
#else
delete mLocalGhosts[i];
#endif
mLocalGhosts[i] = NULL;
}
}
}I suppose it is debatable what one should do with the call to onGhostRemove, in both cases, as it will no longer be inextricably bound to the object's imminent disappearance... it only means that the ghosted reference is going away and that the user may yet retain a copy which has been severed from it's "symbolic link" to a copy maintained on the server.
tone
#2
I think the point I'd stress is: RefPtrs are useful in application-land, but their stock TNL treatment basically makes their power unavailable to the TNL user, as assertions will fire off when he disconnects.
tone
09/11/2006 (12:55 pm)
You're right that the second step all depends on just what one is mirroring, and what one's application is. In my case, I am connecting to an authentication server which maintains basic privilege information for my player. It ghosts this over to the player client, who binds it into place with a RefPtr. It then disconnects from the authentication server and connects to a game server, maintaining its copy of the data received from the authentication server. Of course, it is up to my application to know that, should the player client wish to contact the authentication server a second time, it is up to the application to know that attempts to reference the OLD copy by its ghost ID will be "a bad thing".I think the point I'd stress is: RefPtrs are useful in application-land, but their stock TNL treatment basically makes their power unavailable to the TNL user, as assertions will fire off when he disconnects.
tone
#3
09/11/2006 (1:00 pm)
Ok, you found a use case that I didn't, and yes, it does make sense in this case :) 
Torque 3D Owner Stephen Zepp
Now, if your client code detects and automatically converts these ghosted objects into client side only objects or something similar, then you'd be good to go (although I don't see a use case where this would actually be too beneficial), but as far as I can tell you are simply stranding the objects client side, and they won't have any authoritative ownership by any simulation (server gone, client is still treating them as a ghosted object).