Blag! Both the ZoneDirector and the SpotSceneDirector needed to extend the

SceneDirector which, without multiple inheritance, would make it
impossible to use both in a single client. Of course, we need to use both
in Yohoho!

So I refactored the whole enchilada to delegate rather than extend, which
blows because various methods must now be made public and a particular
failure case for the moveTo request now requires that a special failure
handler interface be made available so that LocationDirector "extensions"
can inject themselves into the failure handling process.


git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@789 542714f4-19e9-0310-aa3c-eee0fc999fb1

src/java/com/threerings/crowd/client/LocationDirector.java

@@ -1,5 +1,5 @@
 //
-// $Id: LocationDirector.java,v 1.15 2001/12/13 06:34:40 mdb Exp $
+// $Id: LocationDirector.java,v 1.16 2001/12/16 05:18:20 mdb Exp $
 
 package com.threerings.crowd.client;
 
@@ -23,6 +23,24 @@ import com.threerings.crowd.util.CrowdContext;
 public class LocationDirector
     implements ClientObserver, Subscriber
 {
+    /**
+     * Used to recover from a moveTo request that was accepted but
+     * resulted in a failed attempt to fetch the place object to which we
+     * were moving.
+     */
+    public static interface FailureHandler
+    {
+        /**
+         * Should instruct the client to move to the last known working
+         * location (as well as clean up after the failed moveTo request).
+         */
+        public void recoverFailedMove (int placeId);
+    }
+
+    /**
+     * Constructs a location director which will configure itself for
+     * operation using the supplied context.
+     */
     public LocationDirector (CrowdContext ctx)
     {
         // keep this around for later
@@ -111,18 +129,18 @@ public class LocationDirector
     }
 
     /**
-     * This can be called by derived classes that need to coopt the moving
+     * This can be called by cooperating directors that need to coopt the
-     * process to extend it in some way or other. In such situations, they
+     * moving process to extend it in some way or other. In such
-     * should call this method before moving to a new location to check to
+     * situations, they should call this method before moving to a new
-     * be sure that all of the registered location observers are amenable
+     * location to check to be sure that all of the registered location
-     * to a location change.
+     * observers are amenable to a location change.
      *
      * @param placeId the place oid of our tentative new location.
      *
      * @return true if everyone is happy with the move, false if it was
      * vetoed by one of the location observers.
      */
-    protected boolean mayMoveTo (int placeId)
+    public boolean mayMoveTo (int placeId)
     {
         for (int i = 0; i < _observers.size(); i++) {
             LocationObserver obs = (LocationObserver)_observers.get(i);
@@ -137,16 +155,16 @@ public class LocationDirector
     }
 
     /**
-     * This can be called by derived classes that need to coopt the moving
+     * This can be called by cooperating directors that need to coopt the
-     * process to extend it in some way or other. In such situations, they
+     * moving process to extend it in some way or other. In such
-     * will be responsible for receiving the successful move response and
+     * situations, they will be responsible for receiving the successful
-     * they should let the location director know that the move has been
+     * move response and they should let the location director know that
-     * effected.
+     * the move has been effected.
      *
      * @param placeId the place oid of our new location.
      * @param config the configuration information for the new place.
      */
-    protected void didMoveTo (int placeId, PlaceConfig config)
+    public void didMoveTo (int placeId, PlaceConfig config)
     {
         DObjectManager omgr = _ctx.getDObjectManager();
 
@@ -191,15 +209,15 @@ public class LocationDirector
     }
 
     /**
-     * This can be called by derived classes that need to coopt the moving
+     * This can be called by cooperating directors that need to coopt the
-     * process to extend it in some way or other. If the coopted move
+     * moving process to extend it in some way or other. If the coopted
-     * request fails, this failure can be propagated to the location
+     * move request fails, this failure can be propagated to the location
      * observers if appropriate.
      *
      * @param placeId the place oid to which we failed to move.
      * @param reason the reason code given for failure.
      */
-    protected void failedToMoveTo (int placeId, String reason)
+    public void failedToMoveTo (int placeId, String reason)
     {
         // let our observers know what's up
         notifyFailure(placeId, reason);
@@ -281,6 +299,10 @@ public class LocationDirector
         notifyFailure(placeId, reason);
     }
 
+    /**
+     * Called when we receive the place object to which we subscribed
+     * after a successful moveTo request.
+     */
     public void objectAvailable (DObject object)
     {
         // yay, we have our new place object
@@ -304,6 +326,12 @@ public class LocationDirector
         }
     }
 
+    /**
+     * Called if we are unable to subscribe to the place object that was
+     * provided to us with our successful moveTo request. This is
+     * generally a bad scene and we do our best to recover by going back
+     * to the previously known location.
+     */
     public void requestFailed (int oid, ObjectAccessException cause)
     {
         // aiya! we were unable to fetch our new place object; something
@@ -324,24 +352,40 @@ public class LocationDirector
         // does whatever's necessary
 
         // try to return to our previous location
-        recoverFailedMove(placeId);
+        if (_failureHandler != null) {
+            _failureHandler.recoverFailedMove(placeId);
+
+        } else {
+            // if we were previously somewhere (and that somewhere isn't
+            // where we just tried to go), try going back to that happy
+            // place
+            if (_previousPlaceId != -1 && _previousPlaceId != placeId) {
+                moveTo(_previousPlaceId);
+            }
+        }
     }
 
     /**
-     * If a <code>moveTo</code> request fails because we are unable to
+     * Sets the failure handler which will recover from place object
-     * fetch our new place object, we need to do something to recover. By
+     * fetching failures. In the event that we are unable to fetch our
-     * default that means attempting to return to the last location we
+     * place object after making a successful moveTo request, we attempt
-     * occupied, but derived classes may need to do things differently.
+     * to rectify the failure by moving back to the last known working
-     *
+     * location. Because entites that cooperate with the location director
-     * @param placeId the place id that we tried to move to but that
+     * may need to become involved in this failure recovery, we provide
-     * failed.
+     * this interface whereby they can interject themseves into the
+     * failure recovery process and do their own failure recovery.
      */
-    protected void recoverFailedMove (int placeId)
+    public void setFailureHandler (FailureHandler handler)
     {
-        // if we were previously somewhere (and that somewhere isn't where
+        if (_failureHandler != null) {
-        // we just tried to go), try going back to that happy place
+            Log.warning("Requested to set failure handler, but we've " +
-        if (_previousPlaceId != -1 && _previousPlaceId != placeId) {
+                        "already got one. The conflicting entities will " +
-            moveTo(_previousPlaceId);
+                        "likely need to perform more sophisticated " +
+                        "coordination to deal with failures. " +
+                        "[old=" + _failureHandler + ", new=" + handler + "].");
+
+        } else {
+            _failureHandler = handler;
         }
     }
 
@@ -376,4 +420,8 @@ public class LocationDirector
 
     /** The oid of the place we previously occupied. */
     protected int _previousPlaceId = -1;
+
+    /** The entity that deals when we fail to subscribe to a place
+     * object. */
+    protected FailureHandler _failureHandler;
 }

src/java/com/threerings/whirled/client/SceneDirector.java

@@ -1,5 +1,5 @@
 //
-// $Id: SceneDirector.java,v 1.9 2001/12/14 01:51:45 mdb Exp $
+// $Id: SceneDirector.java,v 1.10 2001/12/16 05:18:20 mdb Exp $
 
 package com.threerings.whirled.client;
 
@@ -24,7 +24,7 @@ import com.threerings.whirled.util.WhirledContext;
  * The scene director is the client's interface to all things scene
  * related. It interfaces with the scene repository to ensure that scene
  * objects are available when the client enters a particular scene. It
- * handles moving from scene to scene (it extends and replaces the {@link
+ * handles moving from scene to scene (it coordinates with the {@link
  * LocationDirector} in order to do this).
  *
  * <p> Note that when the scene director is in use instead of the location
@@ -33,26 +33,31 @@ import com.threerings.whirled.util.WhirledContext;
  * LocationObserver#locationChangeFailed}.
  */
 public class SceneDirector
-    extends LocationDirector implements SceneCodes
+    implements SceneCodes, LocationDirector.FailureHandler
 {
     /**
      * Creates a new scene director with the specified context.
      *
      * @param ctx the active client context.
+     * @param locdir the location director in use on the client, with
+     * which the scene director will coordinate when changing location.
      * @param screp the entity from which the scene director will load
      * scene data from the local client scene storage.
      * @param dsfact the factory that knows which derivation of {@link
      * DisplayScene} to create for the current system.
      */
-    public SceneDirector (WhirledContext ctx, SceneRepository screp,
+    public SceneDirector (WhirledContext ctx, LocationDirector locdir,
-                          DisplaySceneFactory dsfact)
+                          SceneRepository screp, DisplaySceneFactory dsfact)
     {
-        super(ctx);
-
         // we'll need these for later
         _ctx = ctx;
+        _locdir = locdir;
         _screp = screp;
         _dsfact = dsfact;
+
+        // set ourselves up as a failure handler with the location
+        // director because we need to do special processing
+        _locdir.setFailureHandler(this);
     }
 
     /**
@@ -91,13 +96,14 @@ public class SceneDirector
     /**
      * Prepares to move to the requested scene. The location observers are
      * asked to ratify the move and our pending scene mode is loaded from
-     * the scene repository.
+     * the scene repository. This can be called by cooperating directors
+     * that need to coopt the moveTo process.
      */
-    protected boolean prepareMoveTo (int sceneId)
+    public boolean prepareMoveTo (int sceneId)
     {
         // first check to see if our observers are happy with this move
         // request
-        if (!mayMoveTo(sceneId)) {
+        if (!_locdir.mayMoveTo(sceneId)) {
             return false;
         }
 
@@ -125,6 +131,18 @@ public class SceneDirector
         return true;
     }
 
+    /**
+     * Returns the model loaded in preparation for a scene
+     * transition. This is made available only for cooperating directors
+     * which may need to coopt the scene transition process. The pending
+     * model is only valid immediately following a call to {@link
+     * #prepareMoveTo}.
+     */
+    public SceneModel getPendingModel ()
+    {
+        return _pendingModel;
+    }
+
     /**
      * Called in response to a successful <code>moveTo</code> request.
      */
@@ -133,7 +151,7 @@ public class SceneDirector
     {
         // our move request was successful, deal with subscribing to our
         // new place object
-        didMoveTo(placeId, config);
+        _locdir.didMoveTo(placeId, config);
 
         // since we're committed to moving to the new scene, we'll
         // parallelize and go ahead and load up the new scene now rather
@@ -199,14 +217,14 @@ public class SceneDirector
         _pendingSceneId = -1;
 
         // let our observers know that something has gone horribly awry
-        notifyFailure(sceneId, reason);
+        _locdir.failedToMoveTo(sceneId, reason);
     }
 
     /**
      * Called when something breaks down in the process of performing a
      * <code>moveTo</code> request.
      */
-    protected void recoverFailedMove (int placeId)
+    public void recoverFailedMove (int placeId)
     {
         // we'll need this momentarily
         int sceneId = _sceneId;
@@ -282,6 +300,9 @@ public class SceneDirector
     /** Access to general client services. */
     protected WhirledContext _ctx;
 
+    /** The client's active location director. */
+    protected LocationDirector _locdir;
+
     /** The entity via which we load scene data. */
     protected SceneRepository _screp;
 

src/java/com/threerings/whirled/spot/client/SpotSceneDirector.java

@@ -1,15 +1,13 @@
 //
-// $Id: SpotSceneDirector.java,v 1.3 2001/12/14 23:12:39 mdb Exp $
+// $Id: SpotSceneDirector.java,v 1.4 2001/12/16 05:18:20 mdb Exp $
 
 package com.threerings.whirled.spot.client;
 
 import java.util.Iterator;
 import com.samskivert.util.StringUtil;
 
-import com.threerings.whirled.client.DisplayScene;
-import com.threerings.whirled.client.DisplaySceneFactory;
 import com.threerings.whirled.client.SceneDirector;
-import com.threerings.whirled.client.persist.SceneRepository;
+import com.threerings.whirled.data.SceneModel;
 import com.threerings.whirled.util.WhirledContext;
 
 import com.threerings.whirled.spot.Log;
@@ -20,7 +18,7 @@ import com.threerings.whirled.spot.data.Portal;
  * Extends the standard scene director with facilities to move between
  * locations within a scene.
  */
-public class SpotSceneDirector extends SceneDirector
+public class SpotSceneDirector
     implements SpotCodes
 {
     /**
@@ -42,18 +40,16 @@ public class SpotSceneDirector extends SceneDirector
     }
 
     /**
-     * Creates a new spot scene director with the specified context.
+     * Creates a new spot scene director with the specified context and
+     * which will cooperate with the supplied scene director.
      *
      * @param ctx the active client context.
-     * @param screp the entity from which the scene director will load
+     * @param scdir the scene director with which we will be cooperating.
-     * scene data from the local client scene storage.
-     * @param dsfact the factory that knows which derivation of {@link
-     * DisplayScene} to create for the current system.
      */
-    public SpotSceneDirector (
+    public SpotSceneDirector (WhirledContext ctx, SceneDirector scdir)
-        WhirledContext ctx, SceneRepository screp, DisplaySceneFactory dsfact)
     {
-        super(ctx, screp, dsfact);
+        _ctx = ctx;
+        _scdir = scdir;
     }
 
     /**
@@ -65,7 +61,8 @@ public class SpotSceneDirector extends SceneDirector
     public void traversePortal (int portalId)
     {
         // look up the destination scene and location
-        if (_scene == null) {
+        DisplaySpotScene scene = (DisplaySpotScene)_scdir.getScene();
+        if (scene == null) {
             Log.warning("Requested to traverse portal when we have " +
                         "no scene [portalId=" + portalId + "].");
             return;
@@ -73,8 +70,7 @@ public class SpotSceneDirector extends SceneDirector
 
         // find the portal they're talking about
         int targetSceneId = -1, targetLocId = -1;
-        DisplaySpotScene ds = (DisplaySpotScene)_scene;
+        Iterator portals = scene.getPortals().iterator();
-        Iterator portals = ds.getPortals().iterator();
         while (portals.hasNext()) {
             Portal portal = (Portal)portals.next();
             if (portal.locationId == portalId) {
@@ -85,14 +81,14 @@ public class SpotSceneDirector extends SceneDirector
 
         // make sure we found the portal
         if (targetSceneId == -1) {
+            portals = scene.getPortals().iterator();
             Log.warning("Requested to traverse non-existent portal " +
                         "[portalId=" + portalId +
-                        ", portals=" +
+                        ", portals=" + StringUtil.toString(portals) + "].");
-                        StringUtil.toString(ds.getPortals().iterator()) + "].");
         }
 
         // prepare to move to this scene (sets up pending data)
-        if (!prepareMoveTo(targetSceneId)) {
+        if (!_scdir.prepareMoveTo(targetSceneId)) {
             return;
         }
 
@@ -100,13 +96,14 @@ public class SpotSceneDirector extends SceneDirector
         // we're requesting to move; if we were unable to load it, assume
         // a cached version of zero
         int sceneVer = 0;
-        if (_pendingModel != null) {
+        SceneModel pendingModel = _scdir.getPendingModel();
-            sceneVer = _pendingModel.version;
+        if (pendingModel != null) {
+            sceneVer = pendingModel.version;
         }
 
         // issue a traversePortal request
         SpotService.traversePortal(
-            _ctx.getClient(), _sceneId, portalId, sceneVer, this);
+            _ctx.getClient(), scene.getId(), portalId, sceneVer, _scdir);
     }
 
     /**
@@ -127,7 +124,8 @@ public class SpotSceneDirector extends SceneDirector
         }
 
         // make sure we're currently in a scene
-        if (_sceneId == -1) {
+        DisplaySpotScene scene = (DisplaySpotScene)_scdir.getScene();
+        if (scene == null) {
             Log.warning("Requested to change locations, but we're not " +
                         "currently in any scene [locId=" + locationId + "].");
             return;
@@ -135,8 +133,7 @@ public class SpotSceneDirector extends SceneDirector
 
         // make sure the specified location is in the current scene
         int locidx = -1;
-        DisplaySpotScene sscene = (DisplaySpotScene)_scene;
+        Iterator locs = scene.getLocations().iterator();
-        Iterator locs = sscene.getLocations().iterator();
         for (int i = 0; locs.hasNext(); i++) {
             Location loc = (Location)locs.next();
             if (loc.locationId == locationId) {
@@ -147,7 +144,7 @@ public class SpotSceneDirector extends SceneDirector
         if (locidx == -1) {
             Log.warning("Requested to change to a location that's not " +
                         "in the current scene [locs=" + StringUtil.toString(
-                            sscene.getLocations().iterator()) +
+                            scene.getLocations().iterator()) +
                         ", locId=" + locationId + "].");
             return;
         }
@@ -156,7 +153,8 @@ public class SpotSceneDirector extends SceneDirector
         _pendingLocId = locationId;
         _changeObserver = obs;
         // and send the location change request
-        SpotService.changeLoc(_ctx.getClient(), _sceneId, locationId, this);
+        SpotService.changeLoc(_ctx.getClient(), scene.getId(),
+                              locationId, this);
     }
 
     /**
@@ -195,6 +193,12 @@ public class SpotSceneDirector extends SceneDirector
         }
     }
 
+    /** The active client context. */
+    protected WhirledContext _ctx;
+
+    /** The scene director with which we are cooperating. */
+    protected SceneDirector _scdir;
+
     /** The location id on which we have an outstanding change location
      * request. */
     protected int _pendingLocId = -1;

src/java/com/threerings/whirled/spot/client/SpotService.java

@@ -1,11 +1,13 @@
 //
-// $Id: SpotService.java,v 1.2 2001/12/14 23:12:39 mdb Exp $
+// $Id: SpotService.java,v 1.3 2001/12/16 05:18:20 mdb Exp $
 
 package com.threerings.whirled.spot.client;
 
 import com.threerings.presents.client.Client;
 import com.threerings.presents.client.InvocationDirector;
 
+import com.threerings.whirled.client.SceneDirector;
+
 import com.threerings.whirled.spot.Log;
 
 /**
@@ -21,7 +23,7 @@ public class SpotService implements SpotCodes
      */
     public static void traversePortal (
         Client client, int sceneId, int portalId, int sceneVer,
-        SpotSceneDirector rsptarget)
+        SceneDirector rsptarget)
     {
         InvocationDirector invdir = client.getInvocationDirector();
         Object[] args = new Object[] {

src/java/com/threerings/whirled/zone/client/ZoneDirector.java

@@ -1,21 +1,18 @@
 //
-// $Id: ZoneDirector.java,v 1.1 2001/12/04 00:31:58 mdb Exp $
+// $Id: ZoneDirector.java,v 1.2 2001/12/16 05:18:20 mdb Exp $
 
 package com.threerings.whirled.zone.client;
 
 import com.threerings.crowd.data.PlaceConfig;
 
-import com.threerings.whirled.client.DisplaySceneFactory;
 import com.threerings.whirled.client.SceneDirector;
-import com.threerings.whirled.client.persist.SceneRepository;
-
 import com.threerings.whirled.data.SceneModel;
 import com.threerings.whirled.util.WhirledContext;
 
 import com.threerings.whirled.zone.data.ZoneSummary;
 
 /**
- * The zone director extends the scene director with the notion of zones.
+ * The zone director augments the scene services with the notion of zones.
  * Zones are self-contained, connected groups of scenes. The normal scene
  * director services can be used to move from scene to scene, but moving
  * to a new zone requires a special move request which can be accomplished
@@ -23,17 +20,18 @@ import com.threerings.whirled.zone.data.ZoneSummary;
  * summary which provides information on the zone which can be used to
  * generate an overview map or similar.
  */
-public class ZoneDirector extends SceneDirector
+public class ZoneDirector
 {
     /**
-     * Constructs a zone director with the supplied context, repository
+     * Constructs a zone director with the supplied context, and delegate
-     * and scene factory. A zone director is required on the client side
+     * scene director (which the zone director will coordinate with when
-     * for systems that wish to use the zone services.
+     * moving from scene to scene). A zone director is required on the
+     * client side for systems that wish to use the zone services.
      */
-    public ZoneDirector (WhirledContext ctx, SceneRepository screp,
+    public ZoneDirector (WhirledContext ctx, SceneDirector scdir)
-                         DisplaySceneFactory dsfact)
     {
-        super(ctx, screp, dsfact);
+        _ctx = ctx;
+        _scdir = scdir;
     }
 
     /**
@@ -56,11 +54,11 @@ public class ZoneDirector extends SceneDirector
         // if the requested zone is the same as our current zone, we just
         // want a regular old moveTo request
         if (_summary != null && zoneId == _summary.zoneId) {
-            moveTo(sceneId);
+            _scdir.moveTo(sceneId);
 
         } else { // otherwise, we make a zoned moveTo request
             // prepare to move to this scene (sets up pending data)
-            if (!prepareMoveTo(sceneId)) {
+            if (!_scdir.prepareMoveTo(sceneId)) {
                 return;
             }
 
@@ -68,8 +66,9 @@ public class ZoneDirector extends SceneDirector
             // we're requesting to move; if we were unable to load it, assume
             // a cached version of zero
             int sceneVers = 0;
-            if (_pendingModel != null) {
+            SceneModel pendingModel = _scdir.getPendingModel();
-                sceneVers = _pendingModel.version;
+            if (pendingModel != null) {
+                sceneVers = pendingModel.version;
             }
 
             // issue a moveTo request
@@ -89,7 +88,7 @@ public class ZoneDirector extends SceneDirector
         _summary = summary;
 
         // and pass the rest off to the standard scene transition code
-        handleMoveSucceeded(invid, placeId, config);
+        _scdir.handleMoveSucceeded(invid, placeId, config);
     }
 
     /**
@@ -105,9 +104,23 @@ public class ZoneDirector extends SceneDirector
         _summary = summary;
 
         // and pass the rest off to the standard scene transition code
-        handleMoveSucceededPlusUpdate(invid, placeId, config, model);
+        _scdir.handleMoveSucceededPlusUpdate(invid, placeId, config, model);
     }
 
+    /**
+     * Called in response to a failed zoned <code>moveTo</code> request.
+     */
+    public void handleMoveFailed (int invid, String reason)
+    {
+        _scdir.handleMoveFailed(invid, reason);
+    }
+
+    /** A reference to the active client context. */
+    protected WhirledContext _ctx;
+
+    /** A reference to the scene director with which we coordinate. */
+    protected SceneDirector _scdir;
+
     /** A reference to the zone summary for the currently occupied
      * zone. */
     protected ZoneSummary _summary;