Bit the bullet and implemented invocation service groups so that our various

MetaSOY clients (Swiftly, World, and soon Admin Dashboard) don't have to know about a bunch of unrelated crap. Fricking complexity++, grumble. git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@4551 542714f4-19e9-0310-aa3c-eee0fc999fb1
2007-02-11 00:42:35 +00:00
parent d1ff63d62d
commit 9afcc526a0
6 changed files with 514 additions and 609 deletions
@@ -21,6 +21,8 @@
 package com.threerings.presents.client;
 import java.util.HashSet;
 import com.samskivert.util.Interval;
 import com.samskivert.util.ObserverList;
 import com.samskivert.util.RunAnywhere;
@@ -36,9 +38,9 @@ import com.threerings.presents.net.PingRequest;
 import com.threerings.presents.net.PongResponse;
 /**
- * Through the client object, a connection to the system is established
+ * Through the client object, a connection to the system is established and maintained. The client
- * and maintained. The client object maintains two separate threads (a
+ * object maintains two separate threads (a reader and a writer) by which all network traffic is
- * reader and a writer) by which all network traffic is managed.
+ * managed.
 */
 public class Client
 {
@@ -47,22 +49,17 @@ public class Client
    public static final int[] DEFAULT_SERVER_PORTS = { 47624 };
    /**
-     * Constructs a client object with the supplied credentials and
+     * Constructs a client object with the supplied credentials and RunQueue. The creds will be
-     * RunQueue. The creds will be used to authenticate with any server to
+     * used to authenticate with any server to which this client attempts to connect. The RunQueue
-     * which this client attempts to connect. The RunQueue is used to
+     * is used to operate the distributed object event dispatch mechanism. To allow the dobj event
-     * operate the distributed object event dispatch mechanism. To allow
+     * dispatch to coexist with threads like the AWT thread, the client will request that the
-     * the dobj event dispatch to coexist with threads like the AWT
+     * RunQueue queue up a runnable whenever there are distributed object events that need to be
-     * thread, the client will request that the RunQueue queue up a
+     * processed. The RunQueue can then queue that runnable up on the AWT thread if it is so
-     * runnable whenever there are distributed object events that need to
+     * inclined to make life simpler for the rest of the application.
     * be processed. The RunQueue can then queue that runnable up on the
     * AWT thread if it is so inclined to make life simpler for the rest
     * of the application.
     *
-     * @param creds the credentials to use when logging on to the server.
+     * @param creds the credentials to use when logging on to the server.  These can be null, but
-     * These can be null, but <code>setCredentials</code> must then be
+     * <code>setCredentials</code> must then be called before any call to <code>logon</code>.
-     * called before any call to <code>logon</code>.
+     * @param runQueue a RunQueue that can be used to process incoming events.
     * @param runQueue a RunQueue that can be used to process incoming
     * events.
     */
    public Client (Credentials creds, RunQueue runQueue)
    {
@@ -71,10 +68,9 @@ public class Client
    }
    /**
-     * Registers the supplied observer with this client. While registered
+     * Registers the supplied observer with this client. While registered the observer will receive
-     * the observer will receive notifications of state changes within the
+     * notifications of state changes within the client. The function will refuse to register an
-     * client. The function will refuse to register an already registered
+     * already registered observer.
     * observer.
     *
     * @see ClientObserver
     * @see SessionObserver
@@ -87,9 +83,8 @@ public class Client
    }
    /**
-     * Unregisters the supplied observer. Upon return of this function,
+     * Unregisters the supplied observer. Upon return of this function, the observer will no longer
-     * the observer will no longer receive notifications of state changes
+     * receive notifications of state changes within the client.
     * within the client.
     */
    public void removeClientObserver (SessionObserver observer)
    {
@@ -105,7 +100,7 @@ public class Client
    {
        _standalone = standalone;
    }
-    
+
    /**
     * Checks whether or not this client is operating in a standalone mode.
     */
@@ -113,10 +108,10 @@ public class Client
    {
        return _standalone;
    }
-    
+
    /**
-     * Configures the client to communicate with the server on the supplied
+     * Configures the client to communicate with the server on the supplied hostname and set of
-     * hostname and set of ports (which will be tried in succession).
+     * ports (which will be tried in succession).
     *
     * @see #logon
     */
@@ -127,8 +122,8 @@ public class Client
    }
    /**
-     * Returns the RunQueue in use by this client. This can be used to
+     * Returns the RunQueue in use by this client. This can be used to queue up event dispatching
-     * queue up event dispatching stints.
+     * stints.
     */
    public RunQueue getRunQueue ()
    {
@@ -136,8 +131,7 @@ public class Client
    }
    /**
-     * Returns the hostname of the server to which this client is
+     * Returns the hostname of the server to which this client is currently configured to connect.
     * currently configured to connect.
     */
    public String getHostname ()
    {
@@ -145,8 +139,7 @@ public class Client
    }
    /**
-     * Returns the port on which this client is currently configured to
+     * Returns the port on which this client is currently configured to connect to the server.
     * connect to the server.
     */
    public int[] getPorts ()
    {
@@ -154,8 +147,8 @@ public class Client
    }
    /**
-     * Returns the credentials with which this client is currently
+     * Returns the credentials with which this client is currently configured to connect to the
-     * configured to connect to the server.
+     * server.
     */
    public Credentials getCredentials ()
    {
@@ -163,9 +156,8 @@ public class Client
    }
    /**
-     * Sets the credentials that will be used by this client to
+     * Sets the credentials that will be used by this client to authenticate with the server. This
-     * authenticate with the server. This should be done before any call
+     * should be done before any call to <code>logon</code>.
     * to <code>logon</code>.
     */
    public void setCredentials (Credentials creds)
    {
@@ -181,9 +173,8 @@ public class Client
    }
    /**
-     * Sets the version string reported to the server during
+     * Sets the version string reported to the server during authentication. Some server
-     * authentication. Some server implementations may wish to refuse
+     * implementations may wish to refuse connections by old or invalid client versions.
     * connections by old or invalid client versions.
     */
    public void setVersion (String version)
    {
@@ -191,8 +182,8 @@ public class Client
    }
    /**
-     * Configures the client with a custom class loader which will be used
+     * Configures the client with a custom class loader which will be used when reading objects off
-     * when reading objects off of the network.
+     * of the network.
     */
    public void setClassLoader (ClassLoader loader)
    {
@@ -203,10 +194,19 @@ public class Client
    }
    /**
-     * Returns the data associated with our authentication response. Users
+     * Marks this client as interested in the specified bootstrap services group. Any services
-     * of the Presents system may wish to communicate authentication
+     * registered as bootstrap services with the supplied group name will be included in this
-     * related information to their client by extending and augmenting
+     * clients bootstrap services set. This must be called before {@link #logon}.
-     * {@link AuthResponseData}.
+     */
    public void addBootstrapGroup (String group)
    {
        _bootGroups.add(group);
    }
    /**
     * Returns the data associated with our authentication response. Users of the Presents system
     * may wish to communicate authentication related information to their client by extending and
     * augmenting {@link AuthResponseData}.
     */
    public AuthResponseData getAuthResponseData ()
    {
@@ -214,13 +214,11 @@ public class Client
    }
    /**
-     * Returns the distributed object manager associated with this
+     * Returns the distributed object manager associated with this session. This reference is only
-     * session. This reference is only valid for the duration of the
+     * valid for the duration of the session and a new reference must be obtained if the client
     * session and a new reference must be obtained if the client
     * disconnects and reconnects to the server.
     *
-     * @return the dobjmgr in effect or null if we have no established
+     * @return the dobjmgr in effect or null if we have no established connection to the server.
     * connection to the server.
     */
    public DObjectManager getDObjectManager ()
    {
@@ -228,17 +226,15 @@ public class Client
    }
    /**
-     * Instructs the distributed object manager associated with this
+     * Instructs the distributed object manager associated with this client to allow objects of the
-     * client to allow objects of the specified class to linger around the
+     * specified class to linger around the specified number of milliseconds after their last
-     * specified number of milliseconds after their last subscriber has
+     * subscriber has been removed before the client finally removes its object proxy and flushes
-     * been removed before the client finally removes its object proxy and
+     * the object. Normally, objects are flushed immediately following the removal of their last
-     * flushes the object. Normally, objects are flushed immediately
+     * subscriber.
     * following the removal of their last subscriber.
     *
-     * <p><em>Note:</em> the delay will be applied to derived classes as
+     * <p><em>Note:</em> the delay will be applied to derived classes as well as exact
-     * well as exact matches. <em>Note also:</em> this method cannot be
+     * matches. <em>Note also:</em> this method cannot be called until after the client has
-     * called until after the client has established a connection with the
+     * established a connection with the server and the distributed object manager is available.
     * server and the distributed object manager is available.
     */
    public void registerFlushDelay (Class objclass, long delay)
    {
@@ -247,8 +243,8 @@ public class Client
    }
    /**
-     * Returns the oid of the client object associated with this session.
+     * Returns the oid of the client object associated with this session.  It is only valid for the
-     * It is only valid for the duration of the session.
+     * duration of the session.
     */
    public int getClientOid ()
    {
@@ -256,8 +252,8 @@ public class Client
    }
    /**
-     * Returns a reference to the client object associated with this
+     * Returns a reference to the client object associated with this session. It is only valid for
-     * session. It is only valid for the duration of the session.
+     * the duration of the session.
     */
    public ClientObject getClientObject ()
    {
@@ -265,8 +261,8 @@ public class Client
    }
    /**
-     * Returns the invocation director associated with this session. This
+     * Returns the invocation director associated with this session. This reference is only valid
-     * reference is only valid for the duration of the session.
+     * for the duration of the session.
     */
    public InvocationDirector getInvocationDirector ()
    {
@@ -274,9 +270,9 @@ public class Client
    }
    /**
-     * Returns the first bootstrap service that could be located that
+     * Returns the first bootstrap service that could be located that implements the supplied
-     * implements the supplied {@link InvocationService} derivation.
+     * {@link InvocationService} derivation.  <code>null</code> is returned if no such service
-     * <code>null</code> is returned if no such service could be found.
+     * could be found.
     */
    public InvocationService getService (Class sclass)
    {
@@ -295,10 +291,9 @@ public class Client
    }
    /**
-     * Like {@link #getService} except that a {@link RuntimeException} is
+     * Like {@link #getService} except that a {@link RuntimeException} is thrown if the service is
-     * thrown if the service is not available. Useful to avoid redundant
+     * not available. Useful to avoid redundant error checking when you know that the shit will hit
-     * error checking when you know that the shit will hit the fan if a
+     * the fan if a particular invocation service is not available.
     * particular invocation service is not available.
     */
    public InvocationService requireService (Class sclass)
    {
@@ -312,8 +307,7 @@ public class Client
    }
    /**
-     * Returns a reference to the bootstrap data provided to this client
+     * Returns a reference to the bootstrap data provided to this client at logon time.
     * at logon time.
     */
    public BootstrapData getBootstrapData ()
    {
@@ -321,30 +315,28 @@ public class Client
    }
    /**
-     * Converts a server time stamp to a value comparable to client clock
+     * Converts a server time stamp to a value comparable to client clock readings.
     * readings.
     */
    public long fromServerTime (long stamp)
    {
-        // when we calcuated our time delta, we did it such that: C - S =
+        // when we calcuated our time delta, we did it such that: C - S = dT, thus to convert
-        // dT, thus to convert server to client time we do: C = S + dT
+        // server to client time we do: C = S + dT
        return stamp + _serverDelta;
    }
    /**
-     * Converts a client clock reading to a value comparable to a server
+     * Converts a client clock reading to a value comparable to a server time stamp.
     * time stamp.
     */
    public long toServerTime (long stamp)
    {
-        // when we calcuated our time delta, we did it such that: C - S =
+        // when we calcuated our time delta, we did it such that: C - S = dT, thus to convert
-        // dT, thus to convert server to client time we do: S = C - dT
+        // server to client time we do: S = C - dT
        return stamp - _serverDelta;
    }
    /**
-     * Returns true if we are in active communication (we may not yet be logged
+     * Returns true if we are in active communication (we may not yet be logged on, but we could be
-     * on, but we could be trying to log on).
+     * trying to log on).
     */
    public synchronized boolean isActive ()
    {
@@ -357,17 +349,16 @@ public class Client
     */
    public synchronized boolean isLoggedOn ()
    {
-        // we're not "logged on" until we're fully done with the
+        // we're not "logged on" until we're fully done with the procedure, meaning we have a
-        // procedure, meaning we have a client object reference
+        // client object reference
        return (_clobj != null);
    }
    /**
-     * Requests that this client connect and logon to the server with
+     * Requests that this client connect and logon to the server with which it was previously
-     * which it was previously configured.
+     * configured.
     *
-     * @return false if we're already logged on, true if a logon attempt
+     * @return false if we're already logged on, true if a logon attempt was initiated.
     * was initiated.
     */
    public synchronized boolean logon ()
    {
@@ -376,14 +367,14 @@ public class Client
            return false;
        }
-        // otherwise create a new communicator instance and start it up.
+        // otherwise create a new communicator instance and start it up.  this will initiate the
-        // this will initiate the logon process
+        // logon process
        _comm = new Communicator(this);
        _comm.setClassLoader(_loader);
        _comm.logon();
-        // register an interval that we'll use to keep the clock synced
+        // register an interval that we'll use to keep the clock synced and to send pings when
-        // and to send pings when appropriate
+        // appropriate
        if (_tickInterval == null) {
            _tickInterval = new Interval() {
                public void expired () {
@@ -397,28 +388,23 @@ public class Client
    }
    /**
-     * Requests that the client log off of the server to which it is
+     * Requests that the client log off of the server to which it is connected.
     * connected.
     *
-     * @param abortable If true, the client will call
+     * @param abortable If true, the client will call <code>clientWillDisconnect</code> on all of
-     * <code>clientWillDisconnect</code> on all of the client observers
+     * the client observers and abort the logoff process if any of them return false. If false,
     * and abort the logoff process if any of them return false. If false,
     * <code>clientWillDisconnect</code> will not be called at all.
     *
-     * @return true if the logoff succeeded, false if it failed due to a
+     * @return true if the logoff succeeded, false if it failed due to a disagreeable observer.
     * disagreeable observer.
     */
    public boolean logoff (boolean abortable)
    {
        // if we have no communicator, we're not logged on anyway
        if (_comm == null) {
-            Log.warning("Ignoring request to logoff because we're not " +
+            Log.warning("Ignoring request to logoff because we're not logged on.");
                        "logged on.");
            return true;
        }
-        // if the request is abortable, let's run it past the observers
+        // if the request is abortable, let's run it past the observers before we act upon it
        // before we act upon it
        if (abortable && notifyObservers(CLIENT_WILL_LOGOFF, null)) {
            return false;
        }
@@ -427,29 +413,25 @@ public class Client
        _tickInterval.cancel();
        _tickInterval = null;
-        // ask the communicator to send a logoff message and disconnect
+        // ask the communicator to send a logoff message and disconnect from the server
        // from the server
        _comm.logoff();
        return true;
    }
    /**
-     * For standalone mode, this notifies observers that the client has logged
+     * For standalone mode, this notifies observers that the client has logged off and cleans up.
     * off and cleans up.
     */
    public void standaloneLogoff ()
    {
        notifyObservers(CLIENT_DID_LOGOFF, null);
        cleanup(null);
    }
-    
+
    /**
-     * Called by the {@link ClientDObjectMgr} when our bootstrap
+     * Called by the {@link ClientDObjectMgr} when our bootstrap notification arrives. If the
-     * notification arrives. If the client and server are being run in
+     * client and server are being run in "merged" mode in a single JVM, this is how the client is
-     * "merged" mode in a single JVM, this is how the client is configured
+     * configured with the server's distributed object manager and provided with bootstrap data.
     * with the server's distributed object manager and provided with
     * bootstrap data.
     */
    public void gotBootstrap (BootstrapData data, DObjectManager omgr)
    {
@@ -465,20 +447,18 @@ public class Client
        // initialize our invocation director
        _invdir.init(omgr, _cloid, this);
-        // send a few pings to the server to establish the clock offset
+        // send a few pings to the server to establish the clock offset between this client and
-        // between this client and server standard time
+        // server standard time
        establishClockDelta(System.currentTimeMillis());
-        // we can't quite call initialization completed at this point
+        // we can't quite call initialization completed at this point because we need for the
-        // because we need for the invocation director to fully initialize
+        // invocation director to fully initialize (which requires a round trip to the server)
-        // (which requires a round trip to the server) before turning the
+        // before turning the client loose to do things like request invocation services
        // client loose to do things like request invocation services
    }
    /**
-     * Called every five seconds; ensures that we ping the server if we
+     * Called every five seconds; ensures that we ping the server if we haven't communicated in a
-     * haven't communicated in a long while and periodically resyncs the
+     * long while and periodically resyncs the client and server clock deltas.
     * client and server clock deltas.
     */
    protected void tick ()
    {
@@ -501,8 +481,8 @@ public class Client
            }
        } else if (now - _comm.getLastWrite() > PingRequest.PING_INTERVAL) {
-            // if we haven't sent anything over the network in a while, we
+            // if we haven't sent anything over the network in a while, we ping the server to let
-            // ping the server to let it know that we're still alive
+            // it know that we're still alive
            _comm.postMessage(new PingRequest());
        } else if (now - _lastSync > CLOCK_SYNC_INTERVAL) {
@@ -512,10 +492,9 @@ public class Client
    }
    /**
-     * Called during initialization to initiate a sequence of ping/pong
+     * Called during initialization to initiate a sequence of ping/pong messages which will be used
-     * messages which will be used to determine (with "good enough"
+     * to determine (with "good enough" accuracy) the difference between the client clock and the
-     * accuracy) the difference between the client clock and the server
+     * server clock so that we can later interpret server timestamps.
     * clock so that we can later interpret server timestamps.
     */
    protected void establishClockDelta (long now)
    {
@@ -530,8 +509,8 @@ public class Client
    }
    /**
-     * Called by the {@link Communicator} if it is experiencing trouble logging
+     * Called by the {@link Communicator} if it is experiencing trouble logging on but is still
-     * on but is still trying fallback strategies.
+     * trying fallback strategies.
     */
    protected void reportLogonTribulations (final LogonException cause)
    {
@@ -543,8 +522,8 @@ public class Client
    }
    /**
-     * Called by the invocation director when it successfully subscribes
+     * Called by the invocation director when it successfully subscribes to the client object
-     * to the client object immediately following logon.
+     * immediately following logon.
     */
    protected void gotClientObject (ClientObject clobj)
    {
@@ -556,8 +535,7 @@ public class Client
    }
    /**
-     * Called by the invocation director if it fails to subscribe to the
+     * Called by the invocation director if it fails to subscribe to the client object after logon.
     * client object after logon.
     */
    protected void getClientObjectFailed (Exception cause)
    {
@@ -566,8 +544,7 @@ public class Client
    }
    /**
-     * Called by the invocation director when it discovers that the client
+     * Called by the invocation director when it discovers that the client object has changed.
     * object has changed.
     */
    protected void clientObjectDidChange (ClientObject clobj)
    {
@@ -589,17 +566,16 @@ public class Client
            }
        };
-        // we need to run immediately if this is WILL_LOGOFF or if we have
+        // we need to run immediately if this is WILL_LOGOFF or if we have no RunQueue (which
-        // no RunQueue (which currently only happens in some really obscure
+        // currently only happens in some really obscure circumstances where we're using a Client
-        // circumstances where we're using a Client instance on the server
+        // instance on the server so that we can sort of pretend to be a real client)
        // so that we can sort of pretend to be a real client)
        if (code == CLIENT_WILL_LOGOFF || _runQueue == null) {
            unit.run();
            return noty.getRejected();
        } else {
-            // otherwise we can queue this notification up with our
+            // otherwise we can queue this notification up with our RunQueue and ensure that it's
-            // RunQueue and ensure that it's run on the proper thread
+            // run on the proper thread
            _runQueue.postRunnable(unit);
            return false;
        }
@@ -607,12 +583,10 @@ public class Client
    synchronized void cleanup (final Exception logonError)
    {
-        // we know that prior to the call to this method, the observers
+        // we know that prior to the call to this method, the observers were notified with
-        // were notified with CLIENT_DID_LOGOFF; that may not have been
+        // CLIENT_DID_LOGOFF; that may not have been invoked yet, so we don't want to clear out our
-        // invoked yet, so we don't want to clear out our communicator
+        // communicator reference immediately; instead we queue up a runnable unit to do so to
-        // reference immediately; instead we queue up a runnable unit to
+        // ensure that it won't happen until CLIENT_DID_LOGOFF was dispatched
        // do so to ensure that it won't happen until CLIENT_DID_LOGOFF
        // was dispatched
        _runQueue.postRunnable(new Runnable() {
            public void run () {
                // clear out our references
@@ -625,11 +599,10 @@ public class Client
                // and let our invocation director know we're logged off
                _invdir.cleanup();
-                // if we were cleaned up due to a failure to logon, we can
+                // if we were cleaned up due to a failure to logon, we can report the logon error
-                // report the logon error now that the communicator is
+                // now that the communicator is cleaned up; this allows a logon failure listener to
-                // cleaned up; this allows a logon failure listener to
+                // immediately try another logon (hopefully with something changed like the server
-                // immediately try another logon (hopefully with something
+                // or port)
                // changed like the server or port)
                if (logonError != null) {
                    notifyObservers(CLIENT_FAILED_TO_LOGON, logonError);
                } else {
@@ -640,18 +613,15 @@ public class Client
    }
    /**
-     * Called when we receive a pong packet. We may be in the process of
+     * Called when we receive a pong packet. We may be in the process of calculating the client/
-     * calculating the client/server time differential, or we may have
+     * server time differential, or we may have already done that at which point we ignore pongs.
     * already done that at which point we ignore pongs.
     */
    void gotPong (PongResponse pong)
    {
-        // if we're not currently calculating our client/server delta, then
+        // if we're not currently calculating our delta, then we can throw away the pong
        // we can throw away the pong
        if (_dcalc != null) {
-            // we update the delta after every receipt so as to immediately
+            // we update the delta after every receipt so as to immediately obtain an estimate of
-            // obtain an estimate of the clock delta and then refine it as
+            // the clock delta and then refine it as more packets come in
            // more packets come in
            _dcalc.gotPong(pong);
            _serverDelta = _dcalc.getTimeDelta();
        }
@@ -684,8 +654,7 @@ public class Client
            case CLIENT_FAILED_TO_LOGON:
                if (obs instanceof ClientObserver) {
-                    ((ClientObserver)obs).clientFailedToLogon(
+                    ((ClientObserver)obs).clientFailedToLogon(Client.this, _cause);
                        Client.this, _cause);
                }
                break;
@@ -697,8 +666,7 @@ public class Client
            case CLIENT_CONNECTION_FAILED:
                if (obs instanceof ClientObserver) {
-                    ((ClientObserver)obs).clientConnectionFailed(
+                    ((ClientObserver)obs).clientConnectionFailed(Client.this, _cause);
                        Client.this, _cause);
                }
                break;
@@ -721,8 +689,7 @@ public class Client
                break;
            default:
-                throw new RuntimeException("Invalid code supplied to " +
+                throw new RuntimeException("Invalid code supplied to notifyObservers: " + _code);
                                           "notifyObservers: " + _code);
            }
            return true;
@@ -739,8 +706,7 @@ public class Client
    /** The version string reported to the server at auth time. */
    protected String _version = "";
-    /** An entity that gives us the ability to process events on the main
+    /** An entity that gives us the ability to process events on the main client thread. */
     * client thread. */
    protected RunQueue _runQueue;
    /** The distributed object manager we're using during this session. */
@@ -757,7 +723,7 @@ public class Client
    /** Whether or not this client is operating in a standalone mode. */
    protected boolean _standalone;
-    
+
    /** The game server host. */
    protected String _hostname;
@@ -771,22 +737,23 @@ public class Client
    /** The entity that manages our network communications. */
    protected Communicator _comm;
-    /** A custom class loader used to load objects that come in over the
+    /** A custom class loader used to load objects that come in over the network. */
     * network. */
    protected ClassLoader _loader = getClass().getClassLoader();
    /** General startup information provided by the server. */
    protected BootstrapData _bstrap;
    /** The set of bootstrap service groups this client cares about. */
    protected HashSet<String> _bootGroups = new HashSet<String>();
    /** Manages invocation services. */
    protected InvocationDirector _invdir = new InvocationDirector();
-    /** The difference between the server clock and the client clock
+    /** The difference between the server clock and the client clock (estimated immediately after
-     * (estimated immediately after logging on). */
+     * logging on). */
    protected long _serverDelta;
-    /** Used when establishing our clock delta between the client and
+    /** Used when establishing our clock delta between the client and server. */
     * server. */
    protected DeltaCalculator _dcalc;
    /** The last time at which we synced our clock with the server. */
@@ -54,9 +54,8 @@ import com.threerings.presents.net.LogoffRequest;
 import com.threerings.presents.net.UpstreamMessage;
 /**
- * The client performs all network I/O on separate threads (one for
+ * The client performs all network I/O on separate threads (one for reading and one for
- * reading and one for writing). The communicator class encapsulates that
+ * writing). The communicator class encapsulates that functionality.
 * functionality.
 *
 * <pre>
 * Logon synopsis:
@@ -79,8 +78,7 @@ import com.threerings.presents.net.UpstreamMessage;
 public class Communicator
 {
    /**
-     * Creates a new communicator instance which is associated with the
+     * Creates a new communicator instance which is associated with the supplied client.
     * supplied client.
     */
    public Communicator (Client client)
    {
@@ -88,8 +86,7 @@ public class Communicator
    }
    /**
-     * Logs on to the server and initiates our full-duplex message
+     * Logs on to the server and initiates our full-duplex message exchange.
     * exchange.
     */
    public void logon ()
    {
@@ -98,21 +95,19 @@ public class Communicator
            throw new RuntimeException("Communicator already started.");
        }
-        // start up the reader thread. it will connect to the server and
+        // start up the reader thread. it will connect to the server and start up the writer thread
-        // start up the writer thread if everything went successfully
+        // if everything went successfully
        _reader = new Reader();
        _reader.start();
    }
    /**
-     * Delivers a logoff notification to the server and shuts down the
+     * Delivers a logoff notification to the server and shuts down the network connection. Also
-     * network connection. Also causes all communication threads to
+     * causes all communication threads to terminate.
     * terminate.
     */
    public synchronized void logoff ()
    {
-        // if our socket is already closed, we've already taken care of
+        // if our socket is already closed, we've already taken care of this business
        // this business
        if (_channel == null) {
            return;
        }
@@ -122,27 +117,22 @@ public class Communicator
        // let our reader and writer know that it's time to go
        if (_reader != null) {
-            // if logoff() is being called by the client as part of a
+            // if logoff() is being called by the client as part of a normal shutdown, this will
-            // normal shutdown, this will cause the reader thread to be
+            // cause the reader thread to be interrupted and shutdown gracefully. if logoff is
-            // interrupted and shutdown gracefully. if logoff is being
+            // being called by the reader thread as a result of a failed socket, it won't interrupt
-            // called by the reader thread as a result of a failed socket,
+            // itself as it is already shutting down gracefully. if the JVM is buggy and calling
-            // it won't interrupt itself as it is already shutting down
+            // interrupt() on a thread that is blocked on a socket doesn't wake it up, then when we
-            // gracefully. if the JVM is buggy and calling interrupt() on
+            // close() the socket a bit further down, we have another chance that the reader thread
-            // a thread that is blocked on a socket doesn't wake it up,
+            // will wake up; this time slightly less gracefully because it will think there's a
-            // then when we close() the socket a bit further down, we have
+            // network error when in fact we're just shutting down, but at least it will cleanly
-            // another chance that the reader thread will wake up; this
+            // exit
            // time slightly less gracefully because it will think there's
            // a network error when in fact we're just shutting down, but
            // at least it will cleanly exit
            _reader.shutdown();
        }
        if (_writer != null) {
-            // shutting down the writer thread is simpler because we can
+            // shutting down the writer thread is simpler because we can post a termination message
-            // post a termination message on the queue and be sure that it
+            // on the queue and be sure that it will receive it. when the writer thread has
-            // will receive it. when the writer thread has delivered our
+            // delivered our logoff request and exited, we will complete the logoff process by
-            // logoff request and exited, we will complete the logoff
+            // closing our socket and invoking the clientDidLogoff callback
            // process by closing our socket and invoking the
            // clientDidLogoff callback
            _writer.shutdown();
        }
    }
@@ -157,8 +147,8 @@ public class Communicator
    }
    /**
-     * Configures this communicator with a custom class loader to be used
+     * Configures this communicator with a custom class loader to be used when reading and writing
-     * when reading and writing objects over the network.
+     * objects over the network.
     */
    public void setClassLoader (ClassLoader loader)
    {
@@ -169,10 +159,9 @@ public class Communicator
    }
    /**
-     * Callback called by the reader when the authentication process
+     * Callback called by the reader when the authentication process completes successfully. Here
-     * completes successfully. Here we extract the bootstrap information
+     * we extract the bootstrap information for the client and start up the writer thread to manage
-     * for the client and start up the writer thread to manage the other
+     * the other half of our bi-directional message stream.
     * half of our bi-directional message stream.
     */
    protected synchronized void logonSucceeded (AuthResponseData data)
    {
@@ -188,22 +177,21 @@ public class Communicator
        _writer = new Writer();
        _writer.start();
-        // fill the auth data into the client's local field so that it can
+        // fill the auth data into the client's local field so that it can be requested by external
-        // be requested by external entities
+        // entities
        _client._authData = data;
-        // wait for the bootstrap notification before we claim that we're
+        // wait for the bootstrap notification before we claim that we're actually logged on
        // actually logged on
    }
    /**
-     * Callback called by the reader or writer thread when something goes
+     * Callback called by the reader or writer thread when something goes awry with our socket
-     * awry with our socket connection to the server.
+     * connection to the server.
     */
    protected synchronized void connectionFailed (IOException ioe)
    {
-        // make sure the socket isn't already closed down (meaning we've
+        // make sure the socket isn't already closed down (meaning we've already dealt with the
-        // already dealt with the failed connection)
+        // failed connection)
        if (_channel == null) {
            return;
        }
@@ -219,13 +207,12 @@ public class Communicator
    }
    /**
-     * Callback called by the reader if the server closes the other end of
+     * Callback called by the reader if the server closes the other end of the connection.
     * the connection.
     */
    protected synchronized void connectionClosed ()
    {
-        // make sure the socket isn't already closed down (meaning we've
+        // make sure the socket isn't already closed down (meaning we've already dealt with the
-        // already dealt with the closed connection)
+        // closed connection)
        if (_channel == null) {
            return;
        }
@@ -244,8 +231,8 @@ public class Communicator
        _reader = null;
        if (_writer == null) {
-            // there's no writer during authentication, so we may be
+            // there's no writer during authentication, so we may be responsible for closing the
-            // responsible for closing the socket channel
+            // socket channel
            closeChannel();
            // let the client know when we finally go away
@@ -267,9 +254,8 @@ public class Communicator
        // let the client observers know that we're logged off
        _client.notifyObservers(Client.CLIENT_DID_LOGOFF, null);
-        // now that the writer thread has gone away, we can safely close
+        // now that the writer thread has gone away, we can safely close our socket and let the
-        // our socket and let the client know that the logoff process has
+        // client know that the logoff process has completed
        // completed
        closeChannel();
        // let the client know when we finally go away
@@ -279,9 +265,8 @@ public class Communicator
    }
    /**
-     * Closes the socket channel that we have open to the server. Called
+     * Closes the socket channel that we have open to the server. Called by either {@link
-     * by either {@link #readerDidExit} or {@link #writerDidExit}
+     * #readerDidExit} or {@link #writerDidExit} whichever is called last.
     * whichever is called last.
     */
    protected void closeChannel ()
    {
@@ -319,16 +304,13 @@ public class Communicator
        try {
            ByteBuffer buffer = _fout.frameAndReturnBuffer();
            if (buffer.limit() > 4096) {
-                String txt = StringUtil.truncate(
+                String txt = StringUtil.truncate(String.valueOf(msg), 80, "...");
-                    String.valueOf(msg), 80, "...");
+                Log.info("Whoa, writin' a big one [msg=" + txt + ", size=" + buffer.limit() + "].");
                Log.info("Whoa, writin' a big one [msg=" + txt +
                         ", size=" + buffer.limit() + "].");
            }
            int wrote = _channel.write(buffer);
            if (wrote != buffer.limit()) {
                Log.warning("Aiya! Couldn't write entire message [msg=" + msg +
-                            ", size=" + buffer.limit() +
+                            ", size=" + buffer.limit() + ", wrote=" + wrote + "].");
                            ", wrote=" + wrote + "].");
 //             } else {
 //                 Log.info("Wrote " + wrote + " bytes.");
            }
@@ -350,8 +332,7 @@ public class Communicator
    }
    /**
-     * Makes a note of the time at which we last communicated with the
+     * Makes a note of the time at which we last communicated with the server.
     * server.
     */
    protected synchronized void updateWriteStamp ()
    {
@@ -359,17 +340,15 @@ public class Communicator
    }
    /**
-     * Reads a new message from the socket (blocking until a message has
+     * Reads a new message from the socket (blocking until a message has arrived).
     * arrived).
     */
    protected DownstreamMessage receiveMessage ()
        throws IOException
    {
-        // read in the next message frame (readFrame() can return false
+        // read in the next message frame (readFrame() can return false meaning it only read part
-        // meaning it only read part of the frame from the network, in
+        // of the frame from the network, in which case we simply call it again because we can't do
-        // which case we simply call it again because we can't do anything
+        // anything until it has a whole frame; it will throw an exception if it hits EOF or if
-        // until it has a whole frame; it will throw an exception if it
+        // something goes awry)
        // hits EOF or if something goes awry)
        while (!_fin.readFrame(_channel));
        try {
@@ -386,8 +365,8 @@ public class Communicator
    }
    /**
-     * Callback called by the reader thread when it has parsed a new
+     * Callback called by the reader thread when it has parsed a new message from the socket and
-     * message from the socket and wishes to have it processed.
+     * wishes to have it processed.
     */
    protected void processMessage (DownstreamMessage msg)
    {
@@ -396,9 +375,8 @@ public class Communicator
    }
    /**
-     * Cancels our preferred port saving interval. This method is called from
+     * Cancels our preferred port saving interval. This method is called from the communication
-     * the communication reader thread and the interval thread and must thus be
+     * reader thread and the interval thread and must thus be synchronized.
     * synchronized.
     */
    protected synchronized boolean clearPPI (boolean cancel)
    {
@@ -414,10 +392,9 @@ public class Communicator
    }
    /**
-     * The reader encapsulates the authentication and message reading
+     * The reader encapsulates the authentication and message reading process. It calls back to the
-     * process. It calls back to the <code>Communicator</code> class to do
+     * {@link Communicator} class to do things, but the general flow of the reader thread is
-     * things, but the general flow of the reader thread is encapsulated
+     * encapsulated in this class.
     * in this class.
     */
    protected class Reader extends LoopingThread
    {
@@ -452,8 +429,8 @@ public class Communicator
            // look up the address of the target server
            InetAddress host = InetAddress.getByName(_client.getHostname());
-            // obtain the list of available ports on which to attempt our
+            // obtain the list of available ports on which to attempt our client connection and
-            // client connection and determine our preferred port
+            // determine our preferred port
            String pportKey = _client.getHostname() + ".preferred_port";
            int[] ports = _client.getPorts();
            int pport = PresentsPrefs.config.getValue(pportKey, ports[0]);
@@ -468,18 +445,16 @@ public class Communicator
                try {
                    synchronized (Communicator.this) {
                        clearPPI(true);
-                        _prefPortInterval =
+                        _prefPortInterval = new PrefPortInterval(pportKey, port, nextPort);
                            new PrefPortInterval(pportKey, port, nextPort);
                        _channel = SocketChannel.open(addr);
                        _prefPortInterval.schedule(PREF_PORT_DELAY);
                    }
                    break;
                } catch (IOException ioe) {
-                    if (ioe instanceof ConnectException &&
+                    if (ioe instanceof ConnectException && ii < (ports.length-1)) {
                        ii < (ports.length-1)) {
                        _client.reportLogonTribulations(
-                            new LogonException(
+                            new LogonException(AuthCodes.TRYING_NEXT_PORT, true));
                                AuthCodes.TRYING_NEXT_PORT, true));
                        continue; // try the next port
                    }
                    throw ioe;
@@ -488,8 +463,8 @@ public class Communicator
            _channel.configureBlocking(true);
-            // our messages are framed (preceded by their length), so we
+            // our messages are framed (preceded by their length), so we use these helper streams
-            // use these helper streams to manage the framing
+            // to manage the framing
            _fin = new FramedInputStream();
            _fout = new FramingOutputStream();
@@ -503,8 +478,8 @@ public class Communicator
            throws IOException, LogonException
        {
            // construct an auth request and send it
-            AuthRequest req = new AuthRequest(_client.getCredentials(),
+            AuthRequest req = new AuthRequest(
-                                              _client.getVersion());
+                _client.getCredentials(), _client.getVersion(), _client._bootGroups);
            sendMessage(req);
            // now wait for the auth response
@@ -513,8 +488,8 @@ public class Communicator
            AuthResponseData data = rsp.getData();
            Log.debug("Got auth response: " + data);
-            // if the auth request failed, we want to let the communicator
+            // if the auth request failed, we want to let the communicator know by throwing a logon
-            // know by throwing a logon exception
+            // exception
            if (!data.code.equals(AuthResponseData.SUCCESS)) {
                throw new LogonException(data.code);
            }
@@ -523,9 +498,8 @@ public class Communicator
            logonSucceeded(data);
        }
-        // now that we're authenticated, we manage the reading
+        // now that we're authenticated, we manage the reading half of things by continuously
-        // half of things by continuously reading messages from
+        // reading messages from the socket and processing them
        // the socket and processing them
        protected void iterate ()
        {
            DownstreamMessage msg = null;
@@ -538,14 +512,12 @@ public class Communicator
                processMessage(msg);
            } catch (InterruptedIOException iioe) {
-                // somebody set up us the bomb! we've been interrupted
+                // somebody set up us the bomb! we've been interrupted which means that we're being
-                // which means that we're being shut down, so we just
+                // shut down, so we just report it and return from iterate() like a good monkey
                // report it and return from iterate() like a good monkey
                Log.debug("Reader thread woken up in time to die.");
            } catch (EOFException eofe) {
-                // let the communicator know that our connection was
+                // let the communicator know that our connection was closed
                // closed
                connectionClosed();
                // and shut ourselves down
                shutdown();
@@ -557,8 +529,7 @@ public class Communicator
                shutdown();
            } catch (Exception e) {
-                Log.warning("Error processing message [msg=" + msg +
+                Log.warning("Error processing message [msg=" + msg + ", error=" + e + "].");
                            ", error=" + e + "].");
            }
        }
@@ -570,8 +541,8 @@ public class Communicator
        protected void didShutdown ()
        {
-            // If we haven't recorded a preferred port yet, instead do the
+            // if we haven't recorded a preferred port yet, instead do the failure action since we
-            // failure action since we didn't stay connected long enough.
+            // didn't stay connected long enough
            clearPPI(true);
            // let the communicator know when we finally go away
@@ -580,17 +551,16 @@ public class Communicator
        protected void kick ()
        {
-            // we want to interrupt the reader thread as it may be blocked
+            // we want to interrupt the reader thread as it may be blocked listening to the socket;
-            // listening to the socket; this is only called if the reader
+            // this is only called if the reader thread doesn't shut itself down
            // thread doesn't shut itself down
 //             interrupt();
        }
    }
    /**
-     * The writer encapsulates the message writing process. It calls back
+     * The writer encapsulates the message writing process. It calls back to the {@link
-     * to the <code>Communicator</code> class to do things, but the
+     * Communicator} class to do things, but the general flow of the writer thread is encapsulated
-     * general flow of the writer thread is encapsulated in this class.
+     * in this class.
     */
    protected class Writer extends LoopingThread
    {
@@ -599,9 +569,8 @@ public class Communicator
            // fetch the next message from the queue
            UpstreamMessage msg = _msgq.get();
-            // if this is a termination message, we're being
+            // if this is a termination message, we're being requested to exit, so we want to bail
-            // requested to exit, so we want to bail now rather
+            // now rather than continuing
            // than continuing
            if (msg instanceof TerminationMessage) {
                return;
            }
@@ -611,8 +580,7 @@ public class Communicator
                sendMessage(msg);
            } catch (IOException ioe) {
-                // let the communicator know if we have any
+                // let the communicator know if we have any problems
                // problems
                connectionFailed(ioe);
                // and bail
                shutdown();
@@ -632,8 +600,8 @@ public class Communicator
        protected void kick ()
        {
-            // post a bogus message to the outgoing queue to ensure that
+            // post a bogus message to the outgoing queue to ensure that the writer thread notices
-            // the writer thread notices that it's time to go
+            // that it's time to go
            postMessage(new TerminationMessage());
        }
    }
@@ -643,12 +611,11 @@ public class Communicator
    {
    }
-    /** Used to save our preferred port once we know our connection is not
+    /** Used to save our preferred port once we know our connection is not going to be
-     * going to be unceremoniously closed by Windows Connection Sharing. */
+     * unceremoniously closed by Windows Connection Sharing. */
    protected class PrefPortInterval extends Interval
    {
-        public PrefPortInterval (String key, int thisPort, int nextPort)
+        public PrefPortInterval (String key, int thisPort, int nextPort) {
        {
            super();
            _key = key;
            _thisPort = thisPort;
@@ -691,8 +658,7 @@ public class Communicator
    protected ClientDObjectMgr _omgr;
    protected ClassLoader _loader;
-    /** We use this interval to record the preferred port if it stays connected
+    /** We use this interval to record the preferred port if it stays connected long enough. */
     * long enough. */
    protected PrefPortInterval _prefPortInterval;
    /** Time a port must remain connected before we mark it as preferred. */
@@ -700,8 +666,7 @@ public class Communicator
    /** Used to control low-level message logging. */
    protected static RuntimeAdjust.BooleanAdjust _logMessages =
-        new RuntimeAdjust.BooleanAdjust(
+        new RuntimeAdjust.BooleanAdjust("Toggles whether or not all sent and received low-level " +
-            "Toggles whether or not all sent and received low-level " +
+                                        "network events are logged.", "narya.presents.log_events",
-            "network events are logged.", "narya.presents.log_events",
+                                        PresentsPrefs.config, false);
            PresentsPrefs.config, false);
 }
@@ -21,8 +21,12 @@
 package com.threerings.presents.net;
 import java.util.HashSet;
 import java.util.TimeZone;
 /**
 * Used to authenticate with the server.
 */
 public class AuthRequest extends UpstreamMessage
 {
    /**
@@ -34,14 +38,14 @@ public class AuthRequest extends UpstreamMessage
    }
    /**
-     * Constructs a auth request with the supplied credentials and client
+     * Constructs a auth request with the supplied credentials and client version information.
     * version information.
     */
-    public AuthRequest (Credentials creds, String version)
+    public AuthRequest (Credentials creds, String version, HashSet<String> bootGroups)
    {
        _creds = creds;
        _version = version;
        _zone = TimeZone.getDefault().getID();
        _bootGroups = bootGroups.toArray(new String[bootGroups.size()]);
    }
    /**
@@ -53,8 +57,7 @@ public class AuthRequest extends UpstreamMessage
    }
    /**
-     * Returns a reference to the version information provided with this
+     * Returns a reference to the version information provided with this request.
     * request.
     */
    public String getVersion ()
    {
@@ -69,6 +72,14 @@ public class AuthRequest extends UpstreamMessage
        return TimeZone.getTimeZone(_zone);
    }
    /**
     * Returns the set of bootstrap service groups in which this client is interested.
     */
    public String[] getBootGroups ()
    {
        return _bootGroups;
    }
    /**
     * Generates a string representation of this instance.
     */
@@ -86,4 +97,7 @@ public class AuthRequest extends UpstreamMessage
    /** The timezone in which this client is operating. */
    protected String _zone;
    /** The set of bootstrap service groups this client is interested in. */
    protected String[] _bootGroups;
 }
@@ -24,6 +24,8 @@ package com.threerings.presents.net;
 import com.threerings.io.SimpleStreamableObject;
 import com.threerings.util.StreamableArrayList;
 import com.threerings.presents.data.InvocationMarshaller;
 /**
 * A <code>BootstrapData</code> object is communicated back to the client
 * after authentication has succeeded and after the server is fully
@@ -36,5 +38,5 @@ public class BootstrapData extends SimpleStreamableObject
    public int clientOid;
    /** A list of handles to invocation services. */
-    public StreamableArrayList services;
+    public StreamableArrayList<InvocationMarshaller> services;
 }
@@ -40,40 +40,40 @@ import com.threerings.presents.dobj.InvocationRequestEvent;
 import com.threerings.presents.dobj.ObjectAccessException;
 import com.threerings.presents.dobj.RootDObjectManager;
 import java.util.HashMap;
 /**
- * The invocation services provide client to server invocations (service
+ * The invocation services provide client to server invocations (service requests) and server to
- * requests) and server to client invocations (responses and
+ * client invocations (responses and notifications). Via this mechanism, the client can make
- * notifications). Via this mechanism, the client can make requests of the
+ * requests of the server, be notified of its response and the server can asynchronously invoke
- * server, be notified of its response and the server can asynchronously
+ * code on the client.
 * invoke code on the client.
 *
- * <p> Invocations are like remote procedure calls in that they are named
+ * <p> Invocations are like remote procedure calls in that they are named and take arguments. All
- * and take arguments. All arguments must be {@link Streamable} objects,
+ * arguments must be {@link Streamable} objects, primitive types, or String objects. All arguments
- * primitive types, or String objects. All arguments are passed by value
+ * are passed by value (by serializing and unserializing the arguments); there is no special
- * (by serializing and unserializing the arguments); there is no special
+ * facility provided for referencing non-local objects (it is assumed that the distributed object
- * facility provided for referencing non-local objects (it is assumed that
+ * facility will already be in use for any objects that should be shared).
 * the distributed object facility will already be in use for any objects
 * that should be shared).
 *
- * <p> The server invocation manager listens for invocation requests from
+ * <p> The server invocation manager listens for invocation requests from the client and passes
- * the client and passes them on to the invocation provider registered for
+ * them on to the invocation provider registered for the requested invocation module. It also
- * the requested invocation module. It also provides a mechanism by which
+ * provides a mechanism by which responses and asynchronous notification invocations can be
- * responses and asynchronous notification invocations can be delivered to
+ * delivered to the client.
 * the client.
 */
 public class InvocationManager
    implements EventListener
 {
-    /** The list of services that are to be provided to clients at boot
+    /** Defines the name of the global bootstrap group. */
-     * time. Don't mess with this list! */
+    public static final String GLOBAL_BOOTSTRAP_GROUP = "global";
-    public StreamableArrayList<InvocationMarshaller> bootlist =
+
-        new StreamableArrayList<InvocationMarshaller>();
+    /** A mapping from bootstrap group to lists of services that are to be provided to clients at
     * boot time. Don't mess with these lists! */
    public HashMap<String,StreamableArrayList<InvocationMarshaller>> bootlists =
        new HashMap<String,StreamableArrayList<InvocationMarshaller>>();
    /**
-     * Constructs an invocation manager which will use the supplied
+     * Constructs an invocation manager which will use the supplied distributed object manager to
-     * distributed object manager to operate its invocation
+     * operate its invocation services. Generally only one invocation manager should be operational
-     * services. Generally only one invocation manager should be
+     * in a particular system.
     * operational in a particular system.
     */
    public InvocationManager (RootDObjectManager omgr)
    {
@@ -96,17 +96,30 @@ public class InvocationManager
    }
    /**
-     * Registers the supplied invocation dispatcher, returning a
+     * Registers the supplied invocation dispatcher, returning a marshaller that can be used to
-     * marshaller that can be used to send requests to the provider for
+     * send requests to the provider for whom the dispatcher is proxying.
     * whom the dispatcher is proxying.
     *
     * @param dispatcher the dispatcher to be registered.
-     * @param bootstrap if true, the service instance will be added to the
+     * @param bootstrap if true, the service instance will be added to the list of invocation
-     * list of invocation service objects provided to the client in the
+     * service objects provided to the client in the bootstrap data.
     * bootstrap data.
     */
    public InvocationMarshaller registerDispatcher (
        InvocationDispatcher dispatcher, boolean bootstrap)
    {
        return registerDispatcher(dispatcher, bootstrap ? GLOBAL_BOOTSTRAP_GROUP : null);
    }
    /**
     * Registers the supplied invocation dispatcher, returning a marshaller that can be used to
     * send requests to the provider for whom the dispatcher is proxying.
     *
     * @param dispatcher the dispatcher to be registered.
     * @param group the bootstrap group in which this marshaller is to be registered, or null if it
     * is not a bootstrap service. <em>Do not:</em> register a dispatcher with multiple boot
     * groups. You must collect shared dispatchers into as fine grained a set of groups as
     * necessary and have different types of clients specify the list of groups they need.
     */
    public InvocationMarshaller registerDispatcher (InvocationDispatcher dispatcher, String group)
    {
        // get the next invocation code
        int invCode = nextInvCode();
@@ -119,8 +132,12 @@ public class InvocationManager
        _dispatchers.put(invCode, dispatcher);
        // if it's a bootstrap service, slap it in the list
-        if (bootstrap) {
+        if (group != null) {
-            bootlist.add(marsh);
+            StreamableArrayList<InvocationMarshaller> list = bootlists.get(group);
            if (list == null) {
                bootlists.put(group, list = new StreamableArrayList<InvocationMarshaller>());
            }
            list.add(marsh);
        }
        _recentRegServices.put(Integer.valueOf(invCode),
@@ -28,12 +28,14 @@ import java.util.Iterator;
 import java.util.TimeZone;
 import com.samskivert.util.HashIntMap;
 import com.samskivert.util.Throttle;
 import com.samskivert.util.ResultListener;
 import com.samskivert.util.Throttle;
 import com.threerings.util.Name;
 import com.threerings.util.StreamableArrayList;
 import com.threerings.presents.Log;
 import com.threerings.presents.data.ClientObject;
 import com.threerings.presents.data.InvocationMarshaller;
 import com.threerings.presents.dobj.DEvent;
 import com.threerings.presents.dobj.DObject;
@@ -63,18 +65,15 @@ import com.threerings.presents.server.net.Connection;
 import com.threerings.presents.server.net.MessageHandler;
 /**
- * A client object represents a client session in the server. It is
+ * Represents a client session in the server. It is associated with a connection instance (while
- * associated with a connection instance (while the client is connected)
+ * the client is connected) and acts as the intermediary for the remote client in terms of passing
- * and acts as the intermediary for the remote client in terms of passing
+ * along events forwarded by the client, ensuring that subscriptions are maintained on behalf of
- * along events forwarded by the client, ensuring that subscriptions are
+ * the client and that events are forwarded to the client.
 * maintained on behalf of the client and that events are forwarded to the
 * client.
 *
- * <p><em>A note on synchronization:</em> the client object is structured
+ * <p><em>A note on synchronization:</em> the client object is structured so that its
- * so that its <code>Subscriber</code> implementation (which is called
+ * <code>Subscriber</code> implementation (which is called from the dobjmgr thread) can proceed
- * from the dobjmgr thread) can proceed without synchronization. This does
+ * without synchronization. This does not overlap with its other client duties which are called
- * not overlap with its other client duties which are called from the
+ * from the conmgr thread and therefore also need not be synchronized.
 * conmgr thread and therefore also need not be synchronized.
 */
 public class PresentsClient
    implements ProxySubscriber, MessageHandler, ClientResolutionListener
@@ -82,23 +81,19 @@ public class PresentsClient
    /** Used by {@link #setUsername} to report success or failure. */
    public static interface UserChangeListener
    {
-        /** Called when the new client object has been resolved and the
+        /** Called when the new client object has been resolved and the new client object reported
-         * new client object reported to the client, but the old one has
+         * to the client, but the old one has not yet been destroyed. Any events delivered on this
-         * not yet been destroyed. Any events delivered on this callback
+         * callback to the old client object will be delivered.
         * to the old client object will be delivered.
         *
-         * @param rl when this method is finished with its business and
+         * @param rl when this method is finished with its business and the old client object can
-         * the old client object can be destroyed, the result listener
+         * be destroyed, the result listener should be called. */
         * should be called.
         * */
        public void changeReported (ClientObject newObji, ResultListener rl);
-        /** Called when the user change is completed, the old client
+        /** Called when the user change is completed, the old client object is destroyed and all
-         * object is destroyed and all updates are committed. */
+         * updates are committed. */
        public void changeCompleted (ClientObject newObj);
-        /** Called if some failure occurs during the user change
+        /** Called if some failure occurs during the user change process. */
         * process. */
        public void changeFailed (Exception cause);
    }
@@ -119,8 +114,8 @@ public class PresentsClient
    }
    /**
-     * Returns true if this client has been disconnected for sufficiently
+     * Returns true if this client has been disconnected for sufficiently long that its session
-     * long that its session should be forcibly ended.
+     * should be forcibly ended.
     */
    public boolean checkExpired (long now)
    {
@@ -128,8 +123,7 @@ public class PresentsClient
    }
    /**
-     * Returns the time at which this client started their network
+     * Returns the time at which this client started their network session.
     * session.
     */
    public long getSessionStamp ()
    {
@@ -137,8 +131,7 @@ public class PresentsClient
    }
    /**
-     * Returns the time at which this client most recently connected or
+     * Returns the time at which this client most recently connected or disconnected.
     * disconnected.
     */
    public long getNetworkStamp ()
    {
@@ -154,8 +147,7 @@ public class PresentsClient
    }
    /**
-     * Returns the address of the connected client or null if this client
+     * Returns the address of the connected client or null if this client is not connected.
     * is not connected.
     */
    public InetAddress getInetAddress ()
    {
@@ -164,8 +156,8 @@ public class PresentsClient
    }
    /**
-     * Configures this client with a custom class loader that will be used
+     * Configures this client with a custom class loader that will be used when unserializing
-     * when unserializing classes from the network.
+     * classes from the network.
     */
    public void setClassLoader (ClassLoader loader)
    {
@@ -177,38 +169,32 @@ public class PresentsClient
    }
    /**
-     * <em>Danger:</em> this method is not for general consumption. This
+     * <em>Danger:</em> this method is not for general consumption. This changes the username of
-     * changes the username of the client, but should only be done very
+     * the client, but should only be done very early in a user's session, when you know that no
-     * early in a user's session, when you know that no one has mapped the
+     * one has mapped the user based on their username or has in any other way made use of their
-     * user based on their username or has in any other way made use of
+     * username in a way that will break. However, it should not be done <em>too</em> early in the
-     * their username in a way that will break. However, it should not be
+     * session. The client must be fully resolved.
     * done <em>too</em> early in the session. The client must be fully
     * resolved.
     *
-     * <p> It exists to support systems wherein a user logs in with an
+     * <p> It exists to support systems wherein a user logs in with an account username and then
-     * account username and then chooses a "screen name" by which they
+     * chooses a "screen name" by which they will play (often from a small set of available
-     * will play (often from a small set of available "characters"
+     * "characters" available per account). This will take care of remapping the username to client
-     * available per account). This will take care of remapping the
+     * object mappings that were made by the Presents services when the user logs on, but anything
-     * username to client object mappings that were made by the Presents
+     * else that has had its grubby mits on the username will be left to its own devices, hence the
-     * services when the user logs on, but anything else that has had its
+     * care that must be exercised when using this method.
     * grubby mits on the username will be left to its own devices, hence
     * the care that must be exercised when using this method.
     *
-     * @param ucl an entity that will (optionally) be notified when the
+     * @param ucl an entity that will (optionally) be notified when the username conversion process
-     * username conversion process is complete.
+     * is complete.
     */
    public void setUsername (Name username, final UserChangeListener ucl)
    {
        ClientResolutionListener clr = new ClientResolutionListener() {
-            public void clientResolved (final Name username,
+            public void clientResolved (final Name username, final ClientObject clobj)
                    final ClientObject clobj)
            {
-                // if they old client object is gone by now, they ended
+                // if they old client object is gone by now, they ended their session while we were
-                // their session while we were switching, so freak out
+                // switching, so freak out
                if (_clobj == null) {
-                    Log.warning("Client disappeared before we could " +
+                    Log.warning("Client disappeared before we could complete the switch to a " +
-                                "complete the switch to a new client " +
+                                "new client object [ousername=" + _username +
                                "object [ousername=" + _username +
                                ", nusername=" + username + "].");
                    _cmgr.releaseClientObject(username);
                    Exception error = new Exception("Early withdrawal");
@@ -216,10 +202,8 @@ public class PresentsClient
                    return;
                }
-                // let the client know that the rug has been yanked out
+                // let the client know that the rug has been yanked out from under their ass
-                // from under their ass
+                Object[] args = new Object[] { Integer.valueOf(clobj.getOid()) };
                Object[] args = new Object[] {
                    Integer.valueOf(clobj.getOid()) };
                _clobj.postMessage(ClientObject.CLOBJ_CHANGED, args);
                // call down to any derived classes
@@ -231,7 +215,6 @@ public class PresentsClient
                        public void requestCompleted (Object result) {
                            finishResolved(username, clobj);
                        }
                        public void requestFailed (Exception cause) {
                            finishResolved(username, clobj);
                        }
@@ -263,9 +246,8 @@ public class PresentsClient
            }
            public void resolutionFailed (Name username, Exception reason) {
-                Log.warning("Unable to resolve new client object " +
+                Log.warning("Unable to resolve new client object [oldname=" + _username +
-                            "[oldname=" + _username + ", newname=" + username +
+                            ", newname=" + username + ", reason=" + reason + "].");
                            ", reason=" + reason + "].");
                Log.logStackTrace(reason);
                // let our listener know we're hosed
@@ -288,25 +270,21 @@ public class PresentsClient
    }
    /**
-     * Forcibly terminates a client's session. This must be called from
+     * Forcibly terminates a client's session. This must be called from the dobjmgr thread.
     * the dobjmgr thread.
     */
    public void endSession ()
    {
-        // queue up a request for our connection to be closed (if we have
+        // queue up a request for our connection to be closed (if we have a connection, that is)
        // a connection, that is)
        Connection conn = getConnection();
        if (conn != null) {
-            // go ahead and clear out our connection now to prevent
+            // go ahead and clear out our connection now to prevent funniness
            // funniness
            setConnection(null);
-            // have the connection manager close our connection when it is
+            // have the connection manager close our connection when it is next convenient
            // next convenient
            PresentsServer.conmgr.closeConnection(conn);
        }
-        // if we don't have a client object, we failed to resolve in the
+        // if we don't have a client object, we failed to resolve in the first place, in which case
-        // first place, in which case we have to cope as best we can
+        // we have to cope as best we can
        if (_clobj != null) {
            // and clean up after ourselves
            try {
@@ -328,17 +306,15 @@ public class PresentsClient
    }
    /**
-     * This is called when the server is shut down in the middle of a
+     * This is called when the server is shut down in the middle of a client session. In this
-     * client session. In this circumstance, {@link #endSession} will
+     * circumstance, {@link #endSession} will <em>not</em> be called and so any persistent data
-     * <em>not</em> be called and so any persistent data that might
+     * that might normally be flushed at the end of a client's session should likely be flushed
-     * normally be flushed at the end of a client's session should likely
+     * here.
     * be flushed here.
     */
    public void shutdown  ()
    {
-        // if the client is connected, we need to fake the computation of
+        // if the client is connected, we need to fake the computation of their final connect time
-        // their final connect time because we won't be closing their
+        // because we won't be closing their socket normally
        // socket normally
        if (getConnection() != null) {
            long now = System.currentTimeMillis();
            _connectTime += ((now - _networkStamp) / 1000);
@@ -346,10 +322,9 @@ public class PresentsClient
    }
    /**
-     * Makes a note that this client is subscribed to this object so that
+     * Makes a note that this client is subscribed to this object so that we can clean up after
-     * we can clean up after ourselves if and when the client goes
+     * ourselves if and when the client goes away. This is called by the client internals and
-     * away. This is called by the client internals and needn't be called
+     * needn't be called by code outside the client.
     * by code outside the client.
     */
    public synchronized void mapSubscrip (DObject object)
    {
@@ -357,10 +332,9 @@ public class PresentsClient
    }
    /**
-     * Makes a note that this client is no longer subscribed to this
+     * Makes a note that this client is no longer subscribed to this object. The subscription map
-     * object. The subscription map is used to clean up after the client
+     * is used to clean up after the client when it goes away. This is called by the client
-     * when it goes away. This is called by the client internals and
+     * internals and needn't be called by code outside the client.
     * needn't be called by code outside the client.
     */
    public synchronized void unmapSubscrip (int oid)
    {
@@ -368,8 +342,7 @@ public class PresentsClient
        if (object != null) {
            object.removeSubscriber(this);
        } else {
-            Log.warning("Requested to unmap non-existent subscription " +
+            Log.warning("Requested to unmap non-existent subscription [oid=" + oid + "].");
                        "[oid=" + oid + "].");
        }
    }
@@ -403,24 +376,23 @@ public class PresentsClient
    {
        _messagesIn++; // count 'em up!
-        // if the client has been getting crazy with the cheeze whiz,
+        // if the client has been getting crazy with the cheeze whiz, stick a fork in them; the
-        // stick a fork in them; the first time through we end our
+        // first time through we end our session, subsequently _throttle is null and we just drop
-        // session, subsequently _throttle is null and we just drop any
+        // any messages that come in until we've fully shutdown
        // messages that come in until we've fully shutdown
        if (_throttle == null) {
-//             Log.info("Dropping message from force-quit client " +
+//             Log.info("Dropping message from force-quit client [conn=" + _conn +
 //                      "[conn=" + _conn +
 //                      ", msg=" + message + "].");
            return;
        } else if (_throttle.throttleOp(message.received)) {
-            Log.warning("Client sent more than 100 messages in 10 seconds, " +
+            Log.warning("Client sent more than 100 messages in 10 seconds, forcing " +
-                        "forcing disconnect " + this + ".");
+                        "disconnect " + this + ".");
            safeEndSession();
            _throttle = null;
        }
-        // we dispatch to a message dispatcher that is specialized for the
+        // we dispatch to a message dispatcher that is specialized for the particular class of
-        // particular class of message that we received
+        // message that we received
        MessageDispatcher disp = _disps.get(message.getClass());
        if (disp == null) {
            Log.warning("No dispatcher for message [msg=" + message + "].");
@@ -453,8 +425,7 @@ public class PresentsClient
    public void eventReceived (DEvent event)
    {
        if (event instanceof PresentsDObjectMgr.AccessObjectEvent) {
-            Log.warning("Ignoring event that shouldn't be forwarded " +
+            Log.warning("Ignoring event that shouldn't be forwarded " + event + ".");
                        event + ".");
            Thread.dumpStack();
        } else {
            postMessage(new EventNotification(event));
@@ -462,10 +433,9 @@ public class PresentsClient
    }
    /**
-     * Called when {@link #setUsername} has been called and the new client
+     * Called when {@link #setUsername} has been called and the new client object is about to be
-     * object is about to be applied to this client. The old client object
+     * applied to this client. The old client object will not yet have been destroyed, so any final
-     * will not yet have been destroyed, so any final events can be sent
+     * events can be sent along prior to the new object being put into effect.
     * along prior to the new object being put into effect.
     */
    protected void clientObjectWillChange (
        ClientObject oldClobj, ClientObject newClobj)
@@ -473,16 +443,16 @@ public class PresentsClient
    }
    /**
-     * Called after the new client object has been committed to this
+     * Called after the new client object has been committed to this client due to a call to {@link
-     * client due to a call to {@link #setUsername}.
+     * #setUsername}.
     */
    protected void clientObjectDidChange (ClientObject newClobj)
    {
    }
    /**
-     * Initializes this client instance with the specified username, connection
+     * Initializes this client instance with the specified username, connection instance and client
-     * instance and client object and begins a client session.
+     * object and begins a client session.
     */
    protected void startSession (
        ClientManager cmgr, AuthRequest req, Connection conn, Object authdata)
@@ -503,12 +473,10 @@ public class PresentsClient
    }
    /**
-     * This is factored out to allow derived classes to use a different
+     * This is factored out to allow derived classes to use a different starting username than the
-     * starting username than the one supplied in the user's credentials.
+     * one supplied in the user's credentials.  Generally one only wants to munge the starting
-     * Generally one only wants to munge the starting username if the user
+     * username if the user will subsequently choose a "screen name" and it is desirable to avoid
-     * will subsequently choose a "screen name" and it is desirable to
+     * collision between the authentication user namespace and the screen namespace.
     * avoid collision between the authentication user namespace and the
     * screen namespace.
     */
    protected void assignStartingUsername ()
    {
@@ -516,80 +484,67 @@ public class PresentsClient
    }
    /**
-     * Called by the client manager when a new connection arrives that
+     * Called by the client manager when a new connection arrives that authenticates as this
-     * authenticates as this already established client. This must only be
+     * already established client. This must only be called from the congmr thread.
     * called from the congmr thread.
     */
    protected void resumeSession (Connection conn)
    {
-        // check to see if we've already got a connection object, in which
+        // check to see if we've already got a connection object, in which case it's probably stale
        // case it's probably stale
        Connection oldconn = getConnection();
        if (oldconn != null && !oldconn.isClosed()) {
-            Log.info("Closing stale connection [old=" + oldconn +
+            Log.info("Closing stale connection [old=" + oldconn + ", new=" + conn + "].");
-                     ", new=" + conn + "].");
+            // close the old connection (which results in everything being properly unregistered)
            // close the old connection (which results in everything being
            // properly unregistered)
            oldconn.close();
        }
        // start using the new connection
        setConnection(conn);
-        // if a client connects, drops the connection and reconnects
+        // if a client connects, drops the connection and reconnects within the span of a very
-        // within the span of a very short period of time, we'll find
+        // short period of time, we'll find ourselves in resumeSession() before their client object
-        // ourselves in resumeSession() before their client object was
+        // was resolved from the initial connection; in such a case, we can simply bail out here
-        // resolved from the initial connection; in such a case, we can
+        // and let the original session establishment code take care of initializing this resumed
-        // simply bail out here and let the original session establishment
+        // session
        // code take care of initializing this resumed session
        if (_clobj == null) {
-            Log.warning("Rapid-fire reconnect caused us to arrive in " +
+            Log.warning("Rapid-fire reconnect caused us to arrive in resumeSession() before the " +
-                        "resumeSession() before the original session " +
+                        "original session resolved its client object? " + this + ".");
                        "resolved its client object? " + this + ".");
            return;
        }
-        // we need to get onto the distributed object thread so that we
+        // we need to get onto the dobj thread so that we can finalize resumption of the session
        // can finalize the resumption of the session.
        PresentsServer.omgr.postRunnable(new Runnable() {
            public void run () {
-                // now that we're on the dobjmgr thread we can resume our
+                // now that we're on the dobjmgr thread we can resume our session resumption
                // session resumption
                finishResumeSession();
            }
        });
    }
    /**
-     * This is called from the dobjmgr thread to complete the session
+     * This is called from the dobjmgr thread to complete the session resumption. We call some call
-     * resumption. We call some call backs and send the bootstrap info to
+     * backs and send the bootstrap info to the client.
     * the client.
     */
    protected void finishResumeSession ()
    {
        // let derived classes do any session resuming
        sessionWillResume();
-        // send off a bootstrap notification immediately because we've
+        // send off a bootstrap notification immediately as we've already got our client object
        // already got our client object
        sendBootstrap();
        Log.info("Session resumed " + this + ".");
    }
    /**
-     * Queues up a runnable on the object manager thread where we can
+     * Queues up a runnable on the object manager thread where we can safely end the session.
     * safely end the session.
     */
    protected void safeEndSession ()
    {
        PresentsServer.omgr.postRunnable(new Runnable() {
            public void run () {
                if (getClientObject() == null) {
-                    // refuse to end the session unless the client is
+                    // refuse to end the session unless the client is fully resolved
-                    // fully resolved
+                    Log.warning("Refusing logoff from still-resolving client " + this + ".");
                    Log.warning("Refusing logoff request from " +
                                "still-resolving client " + this + ".");
                } else {
                    // end the session in a civilized manner
                    endSession();
@@ -599,8 +554,8 @@ public class PresentsClient
    }
    /**
-     * Clears out the tracked client subscriptions. Called when the client
+     * Clears out the tracked client subscriptions. Called when the client goes away and shouldn't
-     * goes away and shouldn't be called otherwise.
+     * be called otherwise.
     */
    protected void clearSubscrips (boolean verbose)
    {
@@ -615,58 +570,50 @@ public class PresentsClient
    }
    /**
-     * Called when the client session is first started. The client object
+     * Called when the client session is first started. The client object has been created at this
-     * has been created at this point and after this method is executed,
+     * point and after this method is executed, the bootstrap information will be sent to the
-     * the bootstrap information will be sent to the client which will
+     * client which will trigger the start of the session. Derived classes that override this
-     * trigger the start of the session. Derived classes that override
+     * method should be sure to call <code>super.sessionWillStart</code>.
     * this method should be sure to call
     * <code>super.sessionWillStart</code>.
     *
-     * <p><em>Note:</em> This function will be called on the dobjmgr
+     * <p><em>Note:</em> This function will be called on the dobjmgr thread which means that object
-     * thread which means that object manipulations are OK, but client
+     * manipulations are OK, but client instance manipulations must done carefully.
     * instance manipulations must done carefully.
     */
    protected void sessionWillStart ()
    {
    }
    /**
-     * Called when the client resumes a session (after having disconnected
+     * Called when the client resumes a session (after having disconnected and reconnected). After
-     * and reconnected). After this method is executed, the bootstrap
+     * this method is executed, the bootstrap information will be sent to the client which will
-     * information will be sent to the client which will trigger the
+     * trigger the resumption of the session. Derived classes that override this method should be
-     * resumption of the session. Derived classes that override this
+     * sure to call <code>super.sessionWillResume</code>.
     * method should be sure to call <code>super.sessionWillResume</code>.
     *
-     * <p><em>Note:</em> This function will be called on the dobjmgr
+     * <p><em>Note:</em> This function will be called on the dobjmgr thread which means that object
-     * thread which means that object manipulations are OK, but client
+     * manipulations are OK, but client instance manipulations must done carefully.
     * instance manipulations must done carefully.
     */
    protected void sessionWillResume ()
    {
    }
    /**
-     * Called when the client session ends (either because the client
+     * Called when the client session ends (either because the client logged off or because the
-     * logged off or because the server forcibly terminated the session).
+     * server forcibly terminated the session).  Derived classes that override this method should
-     * Derived classes that override this method should be sure to call
+     * be sure to call <code>super.sessionDidEnd</code>.
     * <code>super.sessionDidEnd</code>.
     *
-     * <p><em>Note:</em> This function will be called on the dobjmgr
+     * <p><em>Note:</em> This function will be called on the dobjmgr thread which means that object
-     * thread which means that object manipulations are OK, but client
+     * manipulations are OK, but client instance manipulations must done carefully.
     * instance manipulations must done carefully.
     */
    protected void sessionDidEnd ()
    {
-        // clear out our subscriptions so that we don't get a complaint
+        // clear out our subscriptions so that we don't get a complaint about inability to forward
-        // about inability to forward the object destroyed event we're
+        // the object destroyed event we're about to generate
        // about to generate
        clearSubscrips(false);
    }
    /**
-     * This is called once we have a handle on the client distributed
+     * This is called once we have a handle on the client distributed object. It sends a bootstrap
-     * object. It sends a bootstrap notification to the client with all
+     * notification to the client with all the information it will need to interact with the
-     * the information it will need to interact with the server.
+     * server.
     */
    protected void sendBootstrap ()
    {
@@ -681,9 +628,8 @@ public class PresentsClient
    }
    /**
-     * Derived client classes can override this member to create derived
+     * Derived client classes can override this member to create derived bootstrap data classes
-     * bootstrap data classes that contain extra bootstrap information, if
+     * that contain extra bootstrap information, if desired.
     * desired.
     */
    protected BootstrapData createBootstrapData ()
    {
@@ -691,14 +637,12 @@ public class PresentsClient
    }
    /**
-     * Derived client classes can override this member to populate the
+     * Derived client classes can override this member to populate the bootstrap data with
-     * bootstrap data with additional information. They should be sure to
+     * additional information. They should be sure to call <code>super.populateBootstrapData</code>
-     * call <code>super.populateBootstrapData</code> before doing their
+     * before doing their own populating, however.
     * own populating, however.
     *
-     * <p><em>Note:</em> This function will be called on the dobjmgr
+     * <p><em>Note:</em> This function will be called on the dobjmgr thread which means that object
-     * thread which means that object manipulations are OK, but client
+     * manipulations are OK, but client instance manipulations must be done carefully.
     * instance manipulations must be done carefully.
     */
    protected void populateBootstrapData (BootstrapData data)
    {
@@ -706,28 +650,36 @@ public class PresentsClient
        data.clientOid = _clobj.getOid();
        // fill in the list of bootstrap services
-        data.services = PresentsServer.invmgr.bootlist;
+        data.services = new StreamableArrayList<InvocationMarshaller>();
        StreamableArrayList<InvocationMarshaller> list = 
            PresentsServer.invmgr.bootlists.get(InvocationManager.GLOBAL_BOOTSTRAP_GROUP);
        if (list != null) {
            data.services.addAll(list);
        }
        for (String group : _areq.getBootGroups()) {
            list = PresentsServer.invmgr.bootlists.get(group);
            if (list != null) {
                data.services.addAll(list);
            }
        }
    }
    /**
-     * Called by the connection manager when this client's connection is
+     * Called by the connection manager when this client's connection is unmapped. That may be
-     * unmapped. That may be because of a connection failure (in which
+     * because of a connection failure (in which case this call will be followed up by a call to
-     * case this call will be followed up by a call to
+     * <code>connectionFailed</code>) or it may be because of an orderly closing of the
-     * <code>connectionFailed</code>) or it may be because of an orderly
+     * connection. In either case, the client can deal with its lack of a connection in this
-     * closing of the connection. In either case, the client can deal with
+     * method. This is invoked by the conmgr thread and should behave accordingly.
     * its lack of a connection in this method. This is invoked by the
     * conmgr thread and should behave accordingly.
     */
    protected void wasUnmapped ()
    {
        // clear out our connection reference
        setConnection(null);
-        // clear out our subscriptions. we need to do this on the dobjmgr
+        // clear out our subscriptions. we need to do this on the dobjmgr thread. it is important
-        // thread. it is important that we do this *after* we clear out
+        // that we do this *after* we clear out our connection reference. once the connection ref
-        // our connection reference. once the connection ref is null, no
+        // is null, no more subscriptions will be processed (even those that were queued up before
-        // more subscriptions will be processed (even those that were
+        // the connection went away)
        // queued up before the connection went away)
        PresentsServer.omgr.postRunnable(new Runnable() {
            public void run () {
                sessionConnectionClosed();
@@ -736,38 +688,33 @@ public class PresentsClient
    }
    /**
-     * Called on the dobjmgr thread when the connection associated with
+     * Called on the dobjmgr thread when the connection associated with this session has been
-     * this session has been closed and unmapped. If the user logged off
+     * closed and unmapped. If the user logged off before closing their connection, this will be
-     * before closing their connection, this will be preceded by a call to
+     * preceded by a call to {@link #sessionDidEnd}.
     * {@link #sessionDidEnd}.
     */
    protected void sessionConnectionClosed ()
    {
-        // clear out our dobj subscriptions in case they weren't cleared
+        // clear out our dobj subscriptions in case they weren't cleared by a call to sessionDidEnd
        // by a call to sessionDidEnd
        clearSubscrips(false);
    }
    /**
-     * Called by the connection manager when this client's connection
+     * Called by the connection manager when this client's connection fails. This is invoked on the
-     * fails. This is invoked on the conmgr thread and should behave
+     * conmgr thread and should behave accordingly.
     * accordingly.
     */
    protected void connectionFailed (IOException fault)
    {
-        // nothing to do here presently. the client manager already
+        // nothing to do here, the client manager already complained about the failed connection
        // complained about the failed connection
    }
    /**
-     * Sets our connection reference in a thread safe way. Also
+     * Sets our connection reference in a thread safe way. Also establishes the back reference to
-     * establishes the back reference to us as the connection's message
+     * us as the connection's message handler.
     * handler.
     */
    protected synchronized void setConnection (Connection conn)
    {
-        // if our connection is being cleared out, record the amount of
+        // if our connection is being cleared out, record the amount of time we were connected to
-        // time we were connected to our total connected time
+        // our total connected time
        long now = System.currentTimeMillis();
        if (_conn != null && conn == null) {
            _connectTime += ((now - _networkStamp) / 1000);
@@ -776,8 +723,8 @@ public class PresentsClient
        // keep a handle to the new connection
        _conn = conn;
-        // tell the connection to pass messages on to us (if we're setting
+        // tell the connection to pass messages on to us (if we're setting a connection rather than
-        // a connection rather than clearing one out)
+        // clearing one out)
        if (_conn != null) {
            _conn.setMessageHandler(this);
@@ -792,20 +739,19 @@ public class PresentsClient
    }
    /**
-     * The connection instance must be accessed via this member function
+     * The connection instance must be accessed via this member function because it is read from
-     * because it is read from both the dobjmgr and conmgr threads and is
+     * both the dobjmgr and conmgr threads and is modified by the conmgr thread.
     * modified by the conmgr thread.
     *
-     * @return The connection instance associated with this client or null
+     * @return The connection instance associated with this client or null if the client is not
-     * if the client is not currently connected.
+     * currently connected.
     */
    protected synchronized Connection getConnection ()
    {
        return _conn;
    }
-    /** Callable from non-dobjmgr thread, this queues up a runnable on the
+    /** Callable from non-dobjmgr thread, this queues up a runnable on the dobjmgr thread to post
-     * dobjmgr thread to post the supplied message to this client. */
+     * the supplied message to this client. */
    protected final void safePostMessage (final DownstreamMessage msg)
    {
        PresentsServer.omgr.postRunnable(new Runnable() {
@@ -825,9 +771,8 @@ public class PresentsClient
            return true;
        }
-        // don't log dropped messages unless we're dropping a lot of them
+        // don't log dropped messages unless we're dropping a lot of them (meaning something is
-        // (meaning something is still queueing messages up for this dead
+        // still queueing messages up for this dead client even though it shouldn't be)
        // client even though it shouldn't be)
        if (++_messagesDropped % 50 == 0) {
            Log.warning("Dropping many messages? [client=" + this +
                        ", count=" + _messagesDropped + "].");
@@ -859,17 +804,15 @@ public class PresentsClient
    {
        buf.append("username=").append(_username);
        buf.append(", conn=").append(_conn);
-        buf.append(", cloid=").append(
+        buf.append(", cloid=").append((_clobj == null) ? -1 : _clobj.getOid());
            (_clobj == null) ? -1 : _clobj.getOid());
        buf.append(", in=").append(_messagesIn);
        buf.append(", out=").append(_messagesOut);
    }
    /**
-     * Message dispatchers are used to dispatch each different type of
+     * Message dispatchers are used to dispatch each different type of upstream message. We can
-     * upstream message. We can look the dispatcher up in a table and
+     * look the dispatcher up in a table and invoke it through an overloaded member which is faster
-     * invoke it through an overloaded member which is faster (so we
+     * (so we think) than doing a bunch of instanceofs.
     * think) than doing a bunch of instanceofs.
     */
    protected static interface MessageDispatcher
    {
@@ -887,8 +830,7 @@ public class PresentsClient
        public void dispatch (PresentsClient client, UpstreamMessage msg)
        {
            SubscribeRequest req = (SubscribeRequest)msg;
-//              Log.info("Subscribing [client=" + client +
+//             Log.info("Subscribing [client=" + client + ", oid=" + req.getOid() + "].");
 //                       ", oid=" + req.getOid() + "].");
            // forward the subscribe request to the omgr for processing
            PresentsServer.omgr.subscribeToObject(req.getOid(), client);
@@ -911,8 +853,8 @@ public class PresentsClient
            // update our subscription tracking table
            client.unmapSubscrip(oid);
-            // post a response to the client letting them know that we
+            // post a response to the client letting them know that we will no longer send them
-            // will no longer send them events regarding this object
+            // events regarding this object
            client.safePostMessage(new UnsubscribeResponse(oid));
        }
    }
@@ -930,8 +872,7 @@ public class PresentsClient
            // fill in the proper source oid
            fevt.setSourceOid(client.getClientObject().getOid());
-//              Log.info("Forwarding event [client=" + client +
+//             Log.info("Forwarding event [client=" + client + ", event=" + fevt + "].");
 //                       ", event=" + fevt + "].");
            // forward the event to the omgr for processing
            PresentsServer.omgr.postEvent(fevt);
@@ -975,16 +916,15 @@ public class PresentsClient
    /** The time at which this client started their session. */
    protected long _sessionStamp;
-    /** The time at which this client most recently connected or
+    /** The time at which this client most recently connected or disconnected. */
     * disconnected. */
    protected long _networkStamp;
-    /** The total number of seconds for which the user was connected to
+    /** The total number of seconds for which the user was connected to the server in this
-     * the server in this session. */
+     * session. */
    protected int _connectTime;
-    /** Prevent the client from sending too many messages too frequently.
+    /** Prevent the client from sending too many messages too frequently.  100 messages in 10
-     * 100 messages in 10 seconds and you're audi. */
+     * seconds and you're audi like 5000. */
    protected Throttle _throttle = new Throttle(100, 10 * 1000L);
    // keep these for kicks and giggles
@@ -996,8 +936,8 @@ public class PresentsClient
    protected static HashMap<Class<?>,MessageDispatcher> _disps =
        new HashMap<Class<?>,MessageDispatcher>();
-    /** The amount of time after disconnection a user is allowed before
+    /** The amount of time after disconnection a user is allowed before their session is forcibly
-     * their session is forcibly ended. */
+     * ended. */
    protected static final long FLUSH_TIME = 7 * 60 * 1000L;
    // register our message dispatchers