Some idle widening & vararg logging

git-svn-id: svn+ssh://src.earth.threerings.net/vilya/trunk@801 c613c5cb-e716-0410-b11b-feb51c14d237
2009-02-14 06:25:29 +00:00
parent 295393ce2b
commit 8fcdcea137
32 changed files with 703 additions and 944 deletions
@@ -100,8 +100,7 @@ public class PlayerStatusView extends JPanel
    @Override
    public String toString ()
    {
-        return "[user=" + _username + ", pidx=" + _pidx +
+        return "[user=" + _username + ", pidx=" + _pidx + ", status=" + _status + "]";
            ", status=" + _status + "]";
    }
    /** The game object associated with this view. */
@@ -86,10 +86,9 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Provides the board view with a reference to its controller so that
+     * Provides the board view with a reference to its controller so that it may communicate
-     * it may communicate directly rather than by posting actions up the
+     * directly rather than by posting actions up the interface hierarchy which sometimes fails if
-     * interface hierarchy which sometimes fails if the puzzle board view
+     * the puzzle board view is hidden before we get a chance to post our actions.
     * is hidden before we get a chance to post our actions.
     */
    public void setController (PuzzleController pctrl)
    {
@@ -105,9 +104,8 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Set whether this puzzle is paused or not.
+     * Set whether this puzzle is paused or not. If paused, a label will be displayed with the
-     * If paused, a label will be displayed with the component's font,
+     * component's font, which may be set with setFont().
     * which may be set with setFont().
     */
    @Override
    public void setPaused (boolean paused)
@@ -118,8 +116,7 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
                PuzzleCodes.PUZZLE_MESSAGE_BUNDLE).xlate(pmsg);
            // create a label using our component's standard font
            _pauseLabel = new Label(pmsg, Label.BOLD | Label.OUTLINE,
-                                    Color.WHITE, Color.BLACK,
+                                    Color.WHITE, Color.BLACK, getFont());
                                    getFont());
            _pauseLabel.setTargetWidth(_bounds.width);
            _pauseLabel.layout(this);
        } else {
@@ -129,11 +126,10 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Adds the given animation to the set of animations currently present
+     * Adds the given animation to the set of animations currently present on the board. The
-     * on the board.  The animation will be added to a list of action
+     * animation will be added to a list of action animations whose count can be queried with
-     * animations whose count can be queried with {@link
+     * {@link #getActionAnimationCount}. The animation will automatically be removed from the
-     * #getActionAnimationCount}. The animation will automatically be
+     * action list when it completes.
     * removed from the action list when it completes.
     */
    public void addActionAnimation (Animation anim)
    {
@@ -172,11 +168,10 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Adds the given sprite to the set of sprites currently present on
+     * Adds the given sprite to the set of sprites currently present on the board. The sprite will
-     * the board.  The sprite will be added to a list of action sprites
+     * be added to a list of action sprites whose count can be queried with
-     * whose count can be queried with {@link #getActionSpriteCount}.  Callers
+     * {@link #getActionSpriteCount}. Callers should be sure to remove the sprite when their work
-     * should be sure to remove the sprite when their work with it is done
+     * with it is done via {@link #removeSprite}.
     * via {@link #removeSprite}.
     */
    public void addActionSprite (Sprite sprite)
    {
@@ -253,15 +248,16 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
                return StringUtil.shortClassName(obj);
            }
        };
-        log.info("Board contents [board=" + StringUtil.shortClassName(this) +
+        log.info("Board contents",
-                 ", sprites=" + StringUtil.listToString(_actionSprites, fmt) +
+            "board", StringUtil.shortClassName(this),
-                 ", anims=" + StringUtil.listToString(_actionAnims, fmt) +
+            "sprites", StringUtil.listToString(_actionSprites, fmt),
-                 "].");
+            "anims", StringUtil.listToString(_actionAnims, fmt));
    }
    /**
-     * Creates and returns an animation displaying the given string with
+     * Creates and returns an animation displaying the given string with the specified parameters,
-     * the specified parameters, floating it a short distance up the view.
+     * floating it a short distance up the view.
     *
     * @param score the score text to display.
     * @param color the color of the text.
@@ -276,9 +272,8 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Creates a score animation, allowing derived classes to use custom
+     * Creates a score animation, allowing derived classes to use custom animations that are
-     * animations that are customized following a call to
+     * customized following a call to {@link #createScoreAnimation(String,Color,Font,int,int)}.
     * {@link #createScoreAnimation(String,Color,Font,int,int)}.
     */
    protected ScoreAnimation createScoreAnimation (Label label, int x, int y)
    {
@@ -286,10 +281,9 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Positions the supplied animation so as to avoid any active
+     * Positions the supplied animation so as to avoid any active animations previously registered
-     * animations previously registered with this method, and adds the
+     * with this method, and adds the animation to the list of animations to be avoided by any
-     * animation to the list of animations to be avoided by any future
+     * future avoid animations.
     * avoid animations.
     */
    public void trackAvoidAnimation (Animation anim)
    {
@@ -341,8 +335,8 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Fires a {@link #ACTION_CLEARED} command iff we have no remaining
+     * Fires a {@link #ACTION_CLEARED} command if we have no remaining interesting sprites or
-     * interesting sprites or animations.
+     * animations.
     */
    protected void maybeFireCleared ()
    {
@@ -351,10 +345,9 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
                     getActionCount() + ":" + isShowing());
        }
        if (getActionCount() == 0) {
-            // we're probably in the middle of a tick() in an
+            // we're probably in the middle of a tick() in an animationDidFinish() call and we want
-            // animationDidFinish() call and we want everyone to finish
+            // everyone to finish processing their business before we go clearing the action, so we
-            // processing their business before we go clearing the action,
+            // queue this up to be run after the tick is complete
            // so we queue this up to be run after the tick is complete
            _ctx.getClient().getRunQueue().postRunnable(new Runnable() {
                public void run () {
                    _pctrl.boardActionCleared();
@@ -364,9 +357,8 @@ public abstract class PuzzleBoardView extends VirtualMediaPanel
    }
    /**
-     * Renders the board contents to the given graphics context.
+     * Renders the board contents to the given graphics context. Sub-classes should implement this
-     * Sub-classes should implement this method to draw all of their
+     * method to draw all of their game-specific business.
     * game-specific business.
     */
    protected abstract void renderBoard (Graphics2D gfx, Rectangle dirty);
@@ -130,16 +130,14 @@ public abstract class PuzzleController extends GameController
    {
        super.setGameOver(gameOver);
-        // clear the action if we're informed that the game is over early
+        // clear the action if we're informed that the game is over early by the client
        // by the client
        if (gameOver) {
            clearAction();
        }
    }
    /**
-     * Returns true if the puzzle has action, false if the action is
+     * Returns true if the puzzle has action, false if the action is cleared or it is suspended.
     * cleared or it is suspended.
     */
    public boolean hasAction ()
    {
@@ -209,9 +207,8 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Derived classes should override this and return false if their
+     * Derived classes should override this and return false if their action should not be paused
-     * action should not be paused when the user switches control to the
+     * when the user switches control to the chat area.
     * chat area.
     */
    protected boolean supportsActionPause ()
    {
@@ -297,9 +294,9 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Puzzles that do not have "action" that starts and stops (via {@link
+     * Puzzles that do not have "action" that starts and stops (via {@link #startAction} and
-     * #startAction} and {@link #clearAction}) when the puzzle starts and
+     * {@link #clearAction}) when the puzzle starts and stops can override this method and return
-     * stops can override this method and return false.
+     * false.
     */
    protected boolean isActionPuzzle ()
    {
@@ -307,11 +304,10 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Indicates whether the action should start immediately as a result
+     * Indicates whether the action should start immediately as a result of
-     * of {@link GameController#gameDidStart} being called. If a puzzle wishes to do
+     * {@link GameController#gameDidStart} being called. If a puzzle wishes to do some beginning
-     * some beginning of the game fun stuff, like display a tutorial
+     * of the game fun stuff, like display a tutorial screen, they can veto the action start and
-     * screen, they can veto the action start and then start it themselves
+     * then start it themselves later.
     * later.
     */
    protected boolean startActionImmediately ()
    {
@@ -327,28 +323,25 @@ public abstract class PuzzleController extends GameController
        if (name.equals(PuzzleObject.STATE)) {
            switch (event.getIntValue()) {
            case PuzzleObject.IN_PLAY:
-                // we have to postpone all game starting activity until the
+                // we have to postpone all game starting activity until the current action has
-                // current action has ended; only after all the animations have
+                // ended; only after all the animations have been completed will everything be in a
-                // been completed will everything be in a state fit for
+                // state fit for starting back up again
                // starting back up again
                fireWhenActionCleared(new ClearPender() {
                    public int actionCleared () {
                        // do the standard game did start business
                        gameDidStart();
                        // we don't always start the action immediately
-                        return startActionImmediately() ?
+                        return startActionImmediately() ? RESTART_ACTION : CARE_NOT;
                            RESTART_ACTION : CARE_NOT;
                    }
                });
                break;
            case PuzzleObject.GAME_OVER:
-                // similarly we haev to postpone game ending activity until
+                // similarly we haev to postpone game ending activity until the current action has
-                // the current action has ended
+                // ended clean up and clear out
                // clean up and clear out
                clearAction();
-                // wait until the action is cleared before we roll down to our
+                // wait until the action is cleared before we roll down to our delegates and do all
-                // delegates and do all that business
+                // that business
                fireWhenActionCleared(new ClearPender() {
                    public int actionCleared () {
                        gameDidEnd();
@@ -390,12 +383,10 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Derived classes should override this method and do whatever is
+     * Derived classes should override this method and do whatever is necessary to start up the
-     * necessary to start up the action for their puzzle. This could be
+     * action for their puzzle. This could be called when the user is already in the "room" and
-     * called when the user is already in the "room" and the game starts,
+     * the game starts, or immediately upon entering the room if the game is already started (for
-     * or immediately upon entering the room if the game is already
+     * example if they disconnected and reconnected to a game already in progress).
     * started (for example if they disconnected and reconnected to a game
     * already in progress).
     */
    protected void startAction ()
    {
@@ -413,23 +404,22 @@ public abstract class PuzzleController extends GameController
        // refuse to start the action if it's already going
        if (_astate != ACTION_CLEARED) {
-            log.warning("Action state inappropriate for startAction() " +
+            log.warning("Action state inappropriate for startAction()", "astate", _astate);
                        "[astate=" + _astate + "].");
            Thread.dumpStack();
            return;
        }
        if (isChatting() && supportsActionPause()) {
-            log.info("Not starting action, player is chatting in a puzzle " +
+            log.info(
-                     "that supports pausing the action.");
+                "Not starting action, player is chatting in a puzzle that supports pausing the action.");
            return;
        }
        log.debug("Starting puzzle action.");
-        // register the game progress updater; it may already be updated
+        // register the game progress updater; it may already be updated because we can cycle
-        // because we can cycle through clearing the action and starting
+        // through clearing the action and starting it again before the updater gets a chance to
-        // it again before the updater gets a chance to unregister itself
+        // unregister itself
        if (!_pctx.getFrameManager().isRegisteredFrameParticipant(_updater)) {
            _pctx.getFrameManager().registerFrameParticipant(_updater);
        }
@@ -440,8 +430,8 @@ public abstract class PuzzleController extends GameController
        // let our panel know what's up
        _panel.startAction();
-        // and if we're not currently chatting, set the puzzle to grab
+        // and if we're not currently chatting, set the puzzle to grab keys and for the chatbox to
-        // keys and for the chatbox to look disabled
+        // look disabled
        if (!isChatting()) {
            _panel.setPuzzleGrabsKeys(true);
        }
@@ -456,11 +446,10 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * If it is not known whether the puzzle board view has finished
+     * If it is not known whether the puzzle board view has finished animating its final bits
-     * animating its final bits after a previous call to {@link
+     * after a previous call to {@link #clearAction}, this method should be used instead of
-     * #clearAction}, this method should be used instead of {@link
+     * {@link #startAction} as it will wait until the action is confirmedly over before starting
-     * #startAction} as it will wait until the action is confirmedly over
+     * it anew.
     * before starting it anew.
     */
    protected void safeStartAction ()
    {
@@ -477,14 +466,12 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Called when the game has ended or when it is going to reset and
+     * Called when the game has ended or when it is going to reset and when the client leaves the
-     * when the client leaves the game "room". This method does not always
+     * game "room". This method does not always immediately clear the action, but may mark the
-     * immediately clear the action, but may mark the clear as pending if
+     * clear as pending if the action cannot yet be cleared (as indicated by
-     * the action cannot yet be cleared (as indicated by {@link
+     * {@link #canClearAction}). The action will eventually be cleared which will result in a call
-     * #canClearAction}). The action will eventually be cleared which will
+     * to {@link #actuallyClearAction} which is what derived classes should override to do their
-     * result in a call to {@link #actuallyClearAction} which is what
+     * action clearing business.
     * derived classes should override to do their action clearing
     * business.
     */
    protected void clearAction ()
    {
@@ -500,35 +487,31 @@ public abstract class PuzzleController extends GameController
        log.debug("Attempting to clear puzzle action.");
-        // put ourselves into a pending clear state and attempt to clear
+        // put ourselves into a pending clear state and attempt to clear the action
        // the action
        _astate = CLEAR_PENDING;
        maybeClearAction();
    }
    /**
-     * This method is called by the {@link PuzzleBoardView} when all
+     * This method is called by the {@link PuzzleBoardView} when all action on the board has
-     * action on the board has finished.
+     * finished.
     */
    protected void boardActionCleared ()
    {
-        // if we have a clear pending, this could be the trigger that
+        // if we have a clear pending, this could be the trigger that allows us to clear our action
        // allows us to clear our action
        maybeClearAction();
    }
    /**
-     * Queues up code to be invoked when the action is completely cleared
+     * Queues up code to be invoked when the action is completely cleared (including all remaining
-     * (including all remaining interesting sprites and animations on the
+     * interesting sprites and animations on the puzzle board).
     * puzzle board).
     */
    protected void fireWhenActionCleared (ClearPender pender)
    {
        // if the action is already ended, fire this pender immediately
        if (_astate == ACTION_CLEARED) {
            if (pender.actionCleared() == ClearPender.RESTART_ACTION) {
-                log.debug("Restarting action at behest of pender " +
+                log.debug("Restarting action at behest of pender " + pender + ".");
                          pender + ".");
                startAction();
            }
@@ -539,14 +522,12 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Returns whether or not it is safe to clear the action. The default
+     * Returns whether or not it is safe to clear the action. The default behavior is to not allow
-     * behavior is to not allow the action to be cleared until all
+     * the action to be cleared until all interesting sprites and animations in the board view
-     * interesting sprites and animations in the board view have finished.
+     * have finished. If derived classes or delegates wish to postpone the clearing of the action,
-     * If derived classes or delegates wish to postpone the clearing of
+     * they can return false from this method, but they must then be sure to call
-     * the action, they can return false from this method, but they must
+     * {@link #maybeClearAction} when whatever condition that caused them to desire to postpone
-     * then be sure to call {@link #maybeClearAction} when whatever
+     * action clearing has finally been satisfied.
     * condition that caused them to desire to postpone action clearing
     * has finally been satisfied.
     */
    protected boolean canClearAction ()
    {
@@ -561,8 +542,7 @@ public abstract class PuzzleController extends GameController
        applyToDelegates(new DelegateOp(PuzzleControllerDelegate.class) {
            @Override
            public void apply (PlaceControllerDelegate delegate) {
-                canClear[0] = canClear[0] &&
+                canClear[0] = canClear[0] && ((PuzzleControllerDelegate)delegate).canClearAction();
                    ((PuzzleControllerDelegate)delegate).canClearAction();
            }
        });
@@ -570,27 +550,23 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Called to effect the actual clearing of our action if we've
+     * Called to effect the actual clearing of our action if we've received some asynchronous
-     * received some asynchronous trigger that indicates that it may well
+     * trigger that indicates that it may well be safe now to clear the action.
     * be safe now to clear the action.
     */
    protected void maybeClearAction ()
    {
        if (_astate == CLEAR_PENDING && canClearAction()) {
            actuallyClearAction();
-//         } else {
+//        } else {
-//             Log.info("Not clearing action [astate=" + _astate +
+//            log.info("Not clearing action", "astate", _astate, "canClear", canClearAction());
 //                      ", canClear=" + canClearAction() + "].");
        }
    }
    /**
-     * Performs the actual process of clearing the action for this puzzle.
+     * Performs the actual process of clearing the action for this puzzle. This is only called
-     * This is only called after it is known to be safe to clear the
+     * after it is known to be safe to clear the action. Derived classes can override this method
-     * action. Derived classes can override this method and clear out
+     * and clear out anything that is not needed while the puzzle's "action" is not going (timers,
-     * anything that is not needed while the puzzle's "action" is not
+     * etc.). Anything that is cleared out here should be recreated in {@link #startAction}.
     * going (timers, etc.). Anything that is cleared out here should be
     * recreated in {@link #startAction}.
     */
    protected void actuallyClearAction ()
    {
@@ -635,16 +611,15 @@ public abstract class PuzzleController extends GameController
        });
        _clearPenders.clear();
-        // if there are no refusals and at least one restart request, go
+        // if there are no refusals and at least one restart request, go ahead and restart the
-        // ahead and restart the action now
+        // action now
        if (results[1] == 0 && results[0] > 0) {
            startAction();
        }
    }
    /**
-     * Called when the action was actually cleared, but before the action
+     * Called when the action was actually cleared, but before the action observers are notified.
     * obsevers are notified.
     */
    protected void actionWasCleared ()
    {
@@ -665,10 +640,9 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Returns the delay in milliseconds between sending each progress
+     * Returns the delay in milliseconds between sending each progress update event to the server.
-     * update event to the server.  Derived classes may wish to override
+     * Derived classes may wish to override this to send their progress updates more or less
-     * this to send their progress updates more or less frequently than
+     * frequently than the default.
     * the default.
     */
    protected long getProgressInterval ()
    {
@@ -680,8 +654,8 @@ public abstract class PuzzleController extends GameController
     */
    protected void generateNewBoard ()
    {
-        // wait for any animations or sprites in the board to finish their
+        // wait for any animations or sprites in the board to finish their business before setting
-        // business before setting the board into place
+        // the board into place
        fireWhenActionCleared(new ClearPender() {
            public int actionCleared () {
                // update the player board
@@ -709,8 +683,8 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Returns the number of progress events currently queued up for
+     * Returns the number of progress events currently queued up for sending to the server with
-     * sending to the server with the next progress update.
+     * the next progress update.
     */
    public int getEventCount ()
    {
@@ -718,8 +692,8 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Are we syncing boards for this puzzle?
+     * Are we syncing boards for this puzzle? By default, we defer to the PuzzlePanel and its
-     * By default, we defer to the PuzzlePanel and its runtime config.
+     * runtime config.
     */
    protected boolean isSyncingBoards ()
    {
@@ -727,17 +701,15 @@ public abstract class PuzzleController extends GameController
    }
    /**
-     * Adds the given progress event and a snapshot of the supplied board
+     * Adds the given progress event and a snapshot of the supplied board state to the set of
-     * state to the set of progress events and associated board states for
+     * progress events and associated board states for later transmission to the server.
     * later transmission to the server.
     */
    public void addProgressEvent (int event, Board board)
    {
        // make sure they don't queue things up at strange times
        if (_puzobj.state != PuzzleObject.IN_PLAY) {
-            log.warning("Rejecting progress event; game not in play " +
+            log.warning("Rejecting progress event; game not in play",
-                        "[puzobj=" + _puzobj.which() +
+                "puzobj", _puzobj.which(), "event", event);
                        ", event=" + event + "].");
            return;
        }
@@ -745,16 +717,16 @@ public abstract class PuzzleController extends GameController
        if (isSyncingBoards()) {
            _states.add((board == null) ? null : (Board)board.clone());
            if (board == null) {
-                log.warning("Added progress event with no associated board " +
+                log.warning(
-                            "state, server will not be able to ensure " +
+                    "Added progress event with no associated board state, " +
-                            "board state synchronization.");
+                    "server will not be able to ensure board state synchronization.");
            }
        }
    }
    /**
-     * Sends the server a game progress update with the list of events, as
+     * Sends the server a game progress update with the list of events, as well as board states if
-     * well as board states if {@link PuzzlePanel#isSyncingBoards} is true.
+     * {@link PuzzlePanel#isSyncingBoards} is true.
     */
    public void sendProgressUpdate ()
    {
@@ -768,11 +740,11 @@ public abstract class PuzzleController extends GameController
        int[] events = CollectionUtil.toIntArray(_events);
        _events.clear();
-//        Log.info("Sending progress [session=" + _puzobj.sessionId +
+//        log.info("Sending progress", "session", _puzobj.sessionId,
-//                 ", events=" + StringUtil.toString(events) + "].");
+//            "events", StringUtil.toString(events));
-        // create an array of the board states that correspond with those
+        // create an array of the board states that correspond with those events (if state syncing
-        // events (if state syncing is enabled)
+        // is enabled)
        int numStates = _states.size();
        if (numStates == size) { // ie, if we have a board to match every event
            Board[] states = new Board[numStates];
@@ -812,8 +784,7 @@ public abstract class PuzzleController extends GameController
     */
    class Unpauser extends MouseHijacker
    {
-        public Unpauser (PuzzlePanel panel)
+        public Unpauser (PuzzlePanel panel) {
        {
            super(panel.getBoardView());
            _panel = panel;
            panel.addMouseListener(_clicker);
@@ -821,8 +792,7 @@ public abstract class PuzzleController extends GameController
        }
        @Override
-        public Component release ()
+        public Component release () {
        {
            _panel.removeMouseListener(_clicker);
            _panel.getBoardView().removeMouseListener(_clicker);
            return super.release();
@@ -838,22 +808,21 @@ public abstract class PuzzleController extends GameController
        protected PuzzlePanel _panel;
    }
-    /** A special frame participant that handles the sending of puzzle
+    /**
-     * progress updates. We can't just
+     * A special frame participant that handles the sending of puzzle progress updates. We can't
-     * register an interval for this because sometimes the clock goes
+     * just register an interval for this because sometimes the clock goes backwards in time in
-     * backwards in time in windows and our intervals don't get called for
+     * windows and our intervals don't get called for a long period of time which causes the
-     * a long period of time which causes the server to think the client
+     * server to think the client is disconnected or cheating and resign them from the puzzle. God
-     * is disconnected or cheating and resign them from the puzzle. God
+     * bless you, Microsoft.
-     * bless you, Microsoft. */
+     */
    protected class Updater implements FrameParticipant
    {
        public void tick (long tickStamp) {
            if (_astate == ACTION_CLEARED) {
-                // remove ourselves as the action is now cleared; we can't
+                // remove ourselves as the action is now cleared; we can't do this in
-                // do this in actuallyClearAction() because that might get
+                // actuallyClearAction() because that might get called during the PuzzlePanel's
-                // called during the PuzzlePanel's frame tick and it's
+                // frame tick and it's only safe to remove yourself during a tick(), not another
-                // only safe to remove yourself during a tick(), not
+                // frame participant
                // another frame participant
                _pctx.getFrameManager().removeFrameParticipant(_updater);
            } else if (tickStamp - _lastProgressTick > getProgressInterval()) {
@@ -952,8 +921,7 @@ public abstract class PuzzleController extends GameController
            if (keycode == KeyEvent.VK_ESCAPE || keycode == KeyEvent.VK_PAUSE) {
                setChatting(!isChatting());
-            // pressing P also to pause (but not unpause),
+            // pressing P also to pause (but not unpause), and only if it has not been reassigned
            // and only if it has not been reassigned
            } else if (keycode == KeyEvent.VK_P && !isChatting() &&
                    !_panel._xlate.hasCommand(KeyEvent.VK_P)) {
                setChatting(true);
@@ -31,10 +31,9 @@ import com.threerings.puzzle.data.PuzzleGameCodes;
 import com.threerings.puzzle.data.PuzzleObject;
 /**
- * A base class for puzzle controller delegates. Provides access to some
+ * A base class for puzzle controller delegates. Provides access to some delegated puzzle
- * delegated puzzle controller methods ({@link #startAction}, {@link
+ * controller methods ({@link #startAction}, {@link #clearAction}, etc.) and provides a casted
- * #clearAction}, etc.) and provides a casted reference to the puzzle
+ * reference to the puzzle object.
 * object.
 */
 public class PuzzleControllerDelegate extends GameControllerDelegate
    implements PuzzleCodes, PuzzleGameCodes
@@ -90,23 +89,20 @@ public class PuzzleControllerDelegate extends GameControllerDelegate
    }
    /**
-     * Derived classes should override this method and do whatever is
+     * Derived classes should override this method and do whatever is necessary to start up the
-     * necessary to start up the action for their puzzle. This could be
+     * action for their puzzle. This could be called when the user is already in the "room" and
-     * called when the user is already in the "room" and the game starts,
+     * the game starts, or immediately upon entering the room if the game is already started (for
-     * or immediately upon entering the room if the game is already
+     * example if they disconnected and reconnected to a game already in progress).
     * started (for example if they disconnected and reconnected to a game
     * already in progress).
     */
    protected void startAction ()
    {
    }
    /**
-     * Delegates that wish to postpone action clearing can override this
+     * Delegates that wish to postpone action clearing can override this method to return false
-     * method to return false until such time as the action can be
+     * until such time as the action can be cleared. They must, however, call
-     * cleared. They must, however, call {@link #maybeClearAction} when
+     * {@link #maybeClearAction} when conditions become such that they would once again allow
-     * conditions become such that they would once again allow action to
+     * action to be cleared.
     * be cleared.
     */
    protected boolean canClearAction ()
    {
@@ -114,9 +110,8 @@ public class PuzzleControllerDelegate extends GameControllerDelegate
    }
    /**
-     * Calls {@link PuzzleController#maybeClearAction}, preserving its
+     * Calls {@link PuzzleController#maybeClearAction}, preserving its protected access but making
-     * protected access but making the method available to all
+     * the method available to all PuzzleControllerDelegate derivations.
     * PuzzleControllerDelegate derivations.
     */
    protected void maybeClearAction ()
    {
@@ -124,20 +119,17 @@ public class PuzzleControllerDelegate extends GameControllerDelegate
    }
    /**
-     * Puzzles should override this method and clear out any action on the
+     * Puzzles should override this method and clear out any action on the board and generally
-     * board and generally clean up anything that was going on because the
+     * clean up anything that was going on because the game was in play. This is called when the
-     * game was in play. This is called when the game has ended or when it
+     * game has ended or when it is going to reset and when the client leaves the game "room".
-     * is going to reset and when the client leaves the game "room".
+     * Anything that is cleared out here should be recreated in {@link #startAction}.
     * Anything that is cleared out here should be recreated in {@link
     * #startAction}.
     */
    protected void clearAction ()
    {
    }
    /**
-     * Called when the puzzle controller sets up a new board for the
+     * Called when the puzzle controller sets up a new board for the player.
     * player.
     *
     * @param board the newly initialized and ready-to-go board.
     */
@@ -45,10 +45,9 @@ import com.threerings.puzzle.util.PuzzleContext;
 import static com.threerings.puzzle.Log.log;
 /**
- * The puzzle panel class should be extended by classes that provide a
+ * The puzzle panel class should be extended by classes that provide a view for a puzzle game. The
- * view for a puzzle game.  The {@link PuzzleController} calls these
+ * {@link PuzzleController} calls these methods as necessary to perform its duties in managing the
- * methods as necessary to perform its duties in managing the logical
+ * logical actions of a puzzle game.
 * actions of a puzzle game.
 */
 public abstract class PuzzlePanel extends JPanel
    implements PlaceView, ControllerProvider, PuzzleCodes, PuzzleGameCodes
@@ -83,8 +82,7 @@ public abstract class PuzzlePanel extends JPanel
    {
        super.addNotify();
-        // leave the keyboard manager disabled to start, and set things up
+        // leave the keyboard manager disabled to start, and set things up for chatting
        // for chatting
        setPuzzleGrabsKeys(false);
    }
@@ -98,20 +96,18 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Temporarily replaces the puzzle board display with the supplied
+     * Temporarily replaces the puzzle board display with the supplied overlay panel. The panel
-     * overlay panel.  The panel can be removed and the board display
+     * can be removed and the board display restored by calling {@link #popOverlayPanel}.
     * restored by calling {@link #popOverlayPanel}.
     *
-     * @return true if the specified panel will be displayed, false if it
+     * @return true if the specified panel will be displayed, false if it was not able to be
-     * was not able to be pushed because another overlay panel is already
+     * pushed because another overlay panel is already pushed onto the primary panel.
     * pushed onto the primary panel.
     */
    public boolean pushOverlayPanel (JPanel opanel)
    {
        // bail if we've already got an overlay
        if (_opanel != null) {
-            log.info("Refusing to push overlay panel, we've already got one " +
+            log.info("Refusing to push overlay panel, we've already got one",
-                     "[opanel=" + _opanel + ", npanel=" + opanel + "].");
+                "opanel", _opanel, "npanel", opanel);
            return false;
        }
@@ -150,8 +146,8 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Initializes the puzzle panel with the puzzle config of the puzzle
+     * Initializes the puzzle panel with the puzzle config of the puzzle whose user interface is
-     * whose user interface is being displayed by the panel
+     * being displayed by the panel
     */
    public void init (GameConfig config)
    {
@@ -160,8 +156,7 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Sets whether this panel receives events periodically from a robot
+     * Sets whether this panel receives events periodically from a robot player.
     * player.
     */
    public void setRobotPlayer (boolean isrobot)
    {
@@ -175,8 +170,7 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Sets whether the puzzle grabs keys or if they should go to the chat
+     * Sets whether the puzzle grabs keys or if they should go to the chat window.
     * window.
     */
    public void setPuzzleGrabsKeys (boolean puzgrabs)
    {
@@ -211,9 +205,8 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Creates a robot player using the default RobotPlayer.  Derived
+     * Creates a robot player using the default RobotPlayer. Derived classes can override to make
-     * classes can override to make use of more advanced robots adapted to
+     * use of more advanced robots adapted to specific puzzles.
     * specific puzzles.
     */
    protected RobotPlayer createRobotPlayer ()
    {
@@ -221,25 +214,23 @@ public abstract class PuzzlePanel extends JPanel
    }
    /**
-     * Creates the puzzle board view that will be used to display the main
+     * Creates the puzzle board view that will be used to display the main puzzle interface. This
-     * puzzle interface. This is called when the puzzle panel is
+     * is called when the puzzle panel is constructed. The derived panel will still be responsible
-     * constructed. The derived panel will still be responsible for adding
+     * for adding the board to the interface hierarchy.
     * the board to the interface hierarchy.
     */
    protected abstract PuzzleBoardView createBoardView (PuzzleContext ctx);
    /**
-     * Creates the main panel used to display the puzzle and its various
+     * Creates the main panel used to display the puzzle and its various in-game accoutrements
-     * in-game accoutrements (next block views, player status displays,
+     * (next block views, player status displays, etc.) This is called when the puzzle panel is
-     * etc.)  This is called when the puzzle panel is constructed.  The
+     * constructed. The derived panel is responsible for making sure that the board view is
     * derived panel is responsible for making sure that the board view is
     * present in the board panel.
     */
    protected abstract JPanel createBoardPanel (PuzzleContext ctx);
    /**
-     * Returns a key translator with the desired key to controller command
+     * Returns a key translator with the desired key to controller command mappings desired for
-     * mappings desired for this puzzle.
+     * this puzzle.
     */
    protected abstract KeyTranslator getKeyTranslator ();
@@ -37,8 +37,8 @@ public abstract class Board
    public abstract void dump ();
    /**
-     * Outputs a string representation of the board contents, interlaced
+     * Outputs a string representation of the board contents, interlaced with the supplied
-     * with the supplied comparison board.
+     * comparison board.
     */
    public abstract void dumpAndCompare (Board other);
@@ -60,8 +60,7 @@ public abstract class Board
    }
    /**
-     * Sets the seed in the board's random number generator and calls
+     * Sets the seed in the board's random number generator and calls {@link #populate}.
     * {@link #populate}.
     */
    public void initializeSeed (long seed)
    {
@@ -70,8 +69,8 @@ public abstract class Board
    }
    /**
-     * Returns the random number generator used by the board to generate
+     * Returns the random number generator used by the board to generate random numbers for our
-     * random numbers for our puzzles.
+     * puzzles.
     */
    public Random getRandom ()
    {
@@ -79,8 +78,8 @@ public abstract class Board
    }
    /**
-     * Called after the seed is set in the board to give derived classes a
+     * Called after the seed is set in the board to give derived classes a chance to do things
-     * chance to do things like populating the board with random pieces.
+     * like populating the board with random pieces.
     */
    protected void populate ()
    {
@@ -90,29 +89,25 @@ public abstract class Board
    protected static class BoardRandom extends Random
        implements Cloneable
    {
-        public BoardRandom (long seed)
+        public BoardRandom (long seed) {
        {
            super(0L);
            setSeed(seed);
        }
        @Override
-        public synchronized void setSeed (long seed)
+        public synchronized void setSeed (long seed) {
        {
            _seed = (seed ^ multiplier) & mask;
        }
        @Override
-        synchronized protected int next (int bits)
+        synchronized protected int next (int bits) {
        {
            long nextseed = (_seed * multiplier + addend) & mask;
            _seed = nextseed;
            return (int)(nextseed >>> (48 - bits));
        }
        @Override
-        public void nextBytes (byte[] bytes)
+        public void nextBytes (byte[] bytes) {
        {
            unimplemented();
        }
@@ -123,8 +118,7 @@ public abstract class Board
        // nextFloat()
        @Override
-        public int nextInt (int n)
+        public int nextInt (int n) {
        {
            if (n <= 0) {
                throw new IllegalArgumentException("n must be positive");
            }
@@ -142,15 +136,13 @@ public abstract class Board
        }
        @Override
-        public double nextDouble ()
+        public double nextDouble () {
        {
            long l = ((long)(next(26)) << 27) + next(27);
            return l / (double)(1L << 53);
        }
        @Override
-        public synchronized double nextGaussian ()
+        public synchronized double nextGaussian () {
        {
            if (_haveNextNextGaussian) {
                _haveNextNextGaussian = false;
                return _nextNextGaussian;
@@ -170,8 +162,7 @@ public abstract class Board
        }
        @Override
-        public Object clone ()
+        public Object clone () {
        {
            try {
                return super.clone();
            } catch (CloneNotSupportedException cnse) {
@@ -180,14 +171,12 @@ public abstract class Board
        }
        /**
-         * I suppose I could copy all the methods from Random, then we
+         * I suppose I could copy all the methods from Random, then we wouldn't need this..
         * wouldn't need this..
         */
        private final void unimplemented ()
        {
            throw new RuntimeException(
-                "The Random method you attempted to call " +
+                "The Random method you attempted to call has not been implemented by BoardRandom.");
                "has not been implemented by BoardRandom.");
        }
        /** The internal state related to generating random numbers. */
@@ -24,15 +24,14 @@ package com.threerings.puzzle.data;
 import com.threerings.io.SimpleStreamableObject;
 /**
- * Provides summarized data representing a player's board in a puzzle
+ * Provides summarized data representing a player's board in a puzzle game. Board summaries are
- * game.  Board summaries are maintained by the puzzle server and are
+ * maintained by the puzzle server and are periodically sent to the clients to give them a view
- * periodically sent to the clients to give them a view into how well
+ * into how well their opponent(s) are doing. The data required to marshal a board summary object
- * their opponent(s) are doing.  The data required to marshal a board
+ * should be notably smaller in size than what would be required to marshal the entire associated
- * summary object should be notably smaller in size than what would be
+ * {@link Board}.
 * required to marshal the entire associated {@link Board}.
 *
- * <p> Note all non-transient members of this and derived classes will
+ * <p> Note all non-transient members of this and derived classes will automatically be serialized
- * automatically be serialized when the summary is sent over the wire.
+ * when the summary is sent over the wire.
 */
 public abstract class BoardSummary extends SimpleStreamableObject
 {
@@ -45,8 +44,8 @@ public abstract class BoardSummary extends SimpleStreamableObject
    }
    /**
-     * Constructs a board summary that retrieves full board information
+     * Constructs a board summary that retrieves full board information from the supplied board
-     * from the supplied board when summarizing.
+     * when summarizing.
     */
    public BoardSummary (Board board)
    {
@@ -54,8 +53,8 @@ public abstract class BoardSummary extends SimpleStreamableObject
    }
    /**
-     * Sets the board associated with this board summary, causing
+     * Sets the board associated with this board summary, causing an immediate update to this
-     * an immediate update to this summary.
+     * summary.
     */
    public void setBoard (Board board)
    {
@@ -64,14 +63,12 @@ public abstract class BoardSummary extends SimpleStreamableObject
    }
    /**
-     * Called by the {@link
+     * Called by the {@link com.threerings.puzzle.server.PuzzleManager} to refresh the board
-     * com.threerings.puzzle.server.PuzzleManager} to refresh the
+     * summary information by studying the associated board contents.
     * board summary information by studying the associated board
     * contents.
     */
    public abstract void summarize ();
-    /** The board that we're summarizing.  This is only valid on the
+    /** The board that we're summarizing.  This is only valid on the server, and on the client only
-     * server, and on the client only for the actual player's board. */
+     * for the actual player's board. */
    protected transient Board _board;
 }
@@ -34,10 +34,9 @@ public interface PuzzleCodes extends InvocationCodes
    /** The default puzzle difficulty level. */
    public static final int DEFAULT_DIFFICULTY = 2;
-    /** Whether to enable debug logging and assertions for puzzles. Note
+    /** Whether to enable debug logging and assertions for puzzles. Note that enabling this may
-     * that enabling this may result in the server or client exiting
+     * result in the server or client exiting unexpectedly if certain error conditions arise in
-     * unexpectedly if certain error conditions arise in order to
+     * order to facilitate debugging, and so this should never be enabled in any environment even
-     * facilitate debugging, and so this should never be enabled in any
+     * remotely resembling production. */
     * environment even remotely resembling production. */
    public static final boolean DEBUG_PUZZLE = false;
 }
@@ -22,9 +22,9 @@
 package com.threerings.puzzle.data;
 /**
- * Contains codes used by the puzzle game client implementations. This
+ * Contains codes used by the puzzle game client implementations. This differs from
- * differs from {@link PuzzleCodes} as that is related to the puzzle
+ * {@link PuzzleCodes} as that is related to the puzzle services which span the client and the
- * services which span the client and the server.
+ * server.
 */
 public interface PuzzleGameCodes
 {
@@ -24,10 +24,9 @@ package com.threerings.puzzle.data;
 import com.threerings.parlor.game.data.GameObject;
 /**
- * Extends the basic {@link GameObject} to add individual player
+ * Extends the basic {@link GameObject} to add individual player status. Puzzle games typically
- * status. Puzzle games typically contain numerous players that may be
+ * contain numerous players that may be knocked out of the game while the overall game continues
- * knocked out of the game while the overall game continues on, thereby
+ * on, thereby necessitating this second level of game status.
 * necessitating this second level of game status.
 */
 public class PuzzleObject extends GameObject
    implements PuzzleCodes
@@ -52,8 +51,8 @@ public class PuzzleObject extends GameObject
    /** The puzzle difficulty level. */
    public int difficulty;
-    /** Summaries of the boards of all players in this puzzle (may be null
+    /** Summaries of the boards of all players in this puzzle (may be null if the puzzle doesn't
-     * if the puzzle doesn't support individual player boards). */
+     * support individual player boards). */
    public BoardSummary[] summaries;
    /** The seed used to germinate the boards. */
@@ -24,11 +24,10 @@ package com.threerings.puzzle.drop.client;
 import java.awt.Rectangle;
 /**
- * The drop block sprite represents a block of multiple pieces that can be
+ * The drop block sprite represents a block of multiple pieces that can be rotated to any of the
- * rotated to any of the four cardinal compass directions.  As such, it
+ * four cardinal compass directions. As such, it may span multiple columns or rows depending on
- * may span multiple columns or rows depending on its orientation.  The
+ * its orientation. The block has a "central" piece around which it rotates, with the other pieces
- * block has a "central" piece around which it rotates, with the other
+ * referred to as "external" pieces.
 * pieces referred to as "external" pieces.
 */
 public class DropBlockSprite extends DropSprite
 {
@@ -41,8 +40,7 @@ public class DropBlockSprite extends DropSprite
     * @param orient the orientation of the sprite.
     * @param pieces the pieces displayed by the sprite.
     */
-    public DropBlockSprite (
+    public DropBlockSprite (DropBoardView view, int col, int row, int orient, int[] pieces)
        DropBoardView view, int col, int row, int orient, int[] pieces)
    {
        super(view, col, row, pieces, 0);
@@ -60,8 +58,7 @@ public class DropBlockSprite extends DropSprite
     * @param renderOrder the rendering order of the sprite.
     */
    public DropBlockSprite (
-        DropBoardView view, int col, int row, int orient, int[] pieces,
+        DropBoardView view, int col, int row, int orient, int[] pieces, int renderOrder)
        int renderOrder)
    {
        super(view, col, row, pieces, 0, renderOrder);
@@ -77,10 +74,9 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Returns an array of the row numbers containing the block pieces.
+     * Returns an array of the row numbers containing the block pieces. The first index is the row
-     * The first index is the row of the central piece.  The array is
+     * of the central piece. The array is cached and re-used internally and so the caller should
-     * cached and re-used internally and so the caller should make their
+     * make their own copy if they care to modify it.
     * own copy if they care to modify it.
     */
    public int[] getRows ()
    {
@@ -88,10 +84,9 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Returns an array of the column numbers containing the block pieces.
+     * Returns an array of the column numbers containing the block pieces. The first index is the
-     * The first index is the column of the central piece.  The array is
+     * column of the central piece. The array is cached and re-used internally and so the caller
-     * cached and re-used internally and so the caller should make their
+     * should make their own copy if they care to modify it.
     * own copy if they care to modify it.
     */
    public int[] getColumns ()
    {
@@ -99,9 +94,9 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Returns the bounds of the block in board piece coordinates.  The
+     * Returns the bounds of the block in board piece coordinates. The bounds rectangle is cached
-     * bounds rectangle is cached and re-used internally and so the caller
+     * and re-used internally and so the caller should make their own copy if they care to modify
-     * should make their own copy if they care to modify it.
+     * it.
     */
    public Rectangle getBoardBounds ()
    {
@@ -146,8 +141,8 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Updates the sprite image offset to reflect the direction in which
+     * Updates the sprite image offset to reflect the direction in which the external piece is
-     * the external piece is hanging.
+     * hanging.
     */
    @Override
    public void setOrientation (int orient)
@@ -194,8 +189,7 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Re-calculates the external piece position and bounds of the drop
+     * Re-calculates the external piece position and bounds of the drop block.
     * block.
     */
    protected void updateDropInfo ()
    {
@@ -227,8 +221,8 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Returns the row the external piece is located in based on the
+     * Returns the row the external piece is located in based on the current central piece
-     * current central piece location and sprite orientation.
+     * location and sprite orientation.
     */
    protected int calculateExternalRow ()
    {
@@ -242,8 +236,8 @@ public class DropBlockSprite extends DropSprite
    }
    /**
-     * Returns the column the external piece is located in based on the
+     * Returns the column the external piece is located in based on the current central piece
-     * current central piece location and sprite orientation.
+     * location and sprite orientation.
     */
    protected int calculateExternalColumn ()
    {
@@ -256,8 +250,7 @@ public class DropBlockSprite extends DropSprite
        }
    }
-    /** How many times this sprite can be popped-up a row in a forgiving
+    /** How many times this sprite can be popped-up a row in a forgiving rotation. */
     * rotation. */
    protected byte _popups = 2;
    /** The drop block bounds in board coordinates. */
@@ -108,11 +108,9 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Called by the {@link DropSprite} to populate <code>pos</code> with
+     * Called by the {@link DropSprite} to populate <code>pos</code> with the screen coordinates
-     * the screen coordinates in pixels at which a piece at <code>(col,
+     * in pixels at which a piece at <code>(col, row)</code> in the board should be drawn. Derived
-     * row)</code> in the board should be drawn.  Derived classes may wish
+     * classes may wish to override this method to allow specialised positioning of sprites.
     * to override this method to allow specialised positioning of
     * sprites.
     */
    public void getPiecePosition (int col, int row, Point pos)
    {
@@ -120,10 +118,9 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Called by the {@link DropSprite} to get the dimensions of the area
+     * Called by the {@link DropSprite} to get the dimensions of the area that will be occupied by
-     * that will be occupied by rendering a piece segment of the given
+     * rendering a piece segment of the given orientation and length whose bottom-leftmost corner
-     * orientation and length whose bottom-leftmost corner is at
+     * is at <code>(col, row)</code>.
     * <code>(col, row)</code>.
     */
    public Dimension getPieceSegmentSize (int col, int row, int orient, int len)
    {
@@ -141,8 +138,7 @@ public abstract class DropBoardView extends PuzzleBoardView
    public void createPiece (int piece, int sx, int sy)
    {
        if (sx < 0 || sy < 0 || sx >= _bwid || sy >= _bhei) {
-            log.warning("Requested to create piece in invalid location " +
+            log.warning("Requested to create piece in invalid location", "sx", sx, "sy", sy);
                        "[sx=" + sx + ", sy=" + sy + "].");
            Thread.dumpStack();
            return;
        }
@@ -150,11 +146,10 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Refreshes the piece sprite at the specified location, if no sprite
+     * Refreshes the piece sprite at the specified location, if no sprite exists at the location,
-     * exists at the location, one will be created. <em>Note:</em> this
+     * one will be created. <em>Note:</em> this method assumes the default {@link ImageSprite} is
-     * method assumes the default {@link ImageSprite} is being used to
+     * being used to display pieces. If {@link #createPieceSprite} is overridden to return a
-     * display pieces. If {@link #createPieceSprite} is overridden to
+     * non-ImageSprite, this method must also be customized.
     * return a non-ImageSprite, this method must also be customized.
     */
    public void updatePiece (int sx, int sy)
    {
@@ -162,17 +157,15 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Updates the piece sprite at the specified location, if no sprite
+     * Updates the piece sprite at the specified location, if no sprite exists at the location,
-     * exists at the location, one will be created. <em>Note:</em> this
+     * one will be created. <em>Note:</em> this method assumes the default {@link ImageSprite} is
-     * method assumes the default {@link ImageSprite} is being used to
+     * being used to display pieces. If {@link #createPieceSprite} is overridden to return a
-     * display pieces. If {@link #createPieceSprite} is overridden to
+     * non-ImageSprite, this method must also be customized.
     * return a non-ImageSprite, this method must also be customized.
     */
    public void updatePiece (int piece, int sx, int sy)
    {
        if (sx < 0 || sy < 0 || sx >= _bwid || sy >= _bhei) {
-            log.warning("Requested to update piece in invalid location " +
+            log.warning("Requested to update piece in invalid location", "sx", sx, "sy", sy);
                        "[sx=" + sx + ", sy=" + sy + "].");
            Thread.dumpStack();
            return;
        }
@@ -192,8 +185,8 @@ public abstract class DropBoardView extends PuzzleBoardView
                             long duration)
    {
        if (tx < 0 || ty < 0 || tx >= _bwid || ty >= _bhei) {
-            log.warning("Requested to create and move piece to invalid " +
+            log.warning("Requested to create and move piece to invalid location",
-                        "location [tx=" + tx + ", ty=" + ty + "].");
+                "tx", tx, "ty", ty);
            Thread.dumpStack();
            return;
        }
@@ -211,11 +204,10 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Instructs the view to move the piece at the specified starting
+     * Instructs the view to move the piece at the specified starting position to the specified
-     * position to the specified destination position. There must be a
+     * destination position. There must be a sprite at the starting position, if there is a sprite
-     * sprite at the starting position, if there is a sprite at the
+     * at the destination position, it must also be moved immediately following this call (as in
-     * destination position, it must also be moved immediately following
+     * the case of a swap) to avoid badness.
     * this call (as in the case of a swap) to avoid badness.
     *
     * @return the piece sprite that is being moved.
     */
@@ -224,8 +216,7 @@ public abstract class DropBoardView extends PuzzleBoardView
        int spos = sy * _bwid + sx;
        Sprite piece = _pieces[spos];
        if (piece == null) {
-            log.warning("Missing source sprite for drop [sx=" + sx +
+            log.warning("Missing source sprite for drop", "sx", sx, "sy", sy, "tx", tx, "ty", ty);
                        ", sy=" + sy + ", tx=" + tx + ", ty=" + ty + "].");
            return null;
        }
        _pieces[spos] = null;
@@ -236,8 +227,8 @@ public abstract class DropBoardView extends PuzzleBoardView
    /**
     * A helper function for moving pieces into place.
     */
-    protected void movePiece (Sprite piece, final int sx, final int sy,
+    protected void movePiece (
-                              final int tx, final int ty, long duration)
+        Sprite piece, final int sx, final int sy, final int tx, final int ty, long duration)
    {
        final Exception where = new Exception();
@@ -247,9 +238,8 @@ public abstract class DropBoardView extends PuzzleBoardView
        if (sx == tx && sy == ty) {
            int tpos = ty * _bwid + tx;
            if (_pieces[tpos] != null) {
-                log.warning("Zoiks! Asked to add a piece where we already " +
+                log.warning("Zoiks! Asked to add a piece where we already have one",
-                            "have one [sx=" + sx + ", sy=" + sy +
+                    "sx", sx, "sy", sy, "tx", tx, "ty", ty, where);
                            ", tx=" + tx + ", ty=" + ty + "].", where);
            }
            _pieces[tpos] = piece;
            piece.setLocation(start.x, start.y);
@@ -268,9 +258,8 @@ public abstract class DropBoardView extends PuzzleBoardView
            public void pathCompleted (Sprite sprite, Path path, long when) {
                sprite.removeSpriteObserver(this);
                if (_pieces[tpos] != null) {
-                    log.warning("Oh god, we're dropping onto another piece " +
+                    log.warning("Oh god, we're dropping onto another piece",
-                                "[sx=" + sx + ", sy=" + sy +
+                        "sx", sx, "sy", sy, "tx", tx + "ty", ty, where);
                                ", tx=" + tx + ", ty=" + ty + "].", where);
                    return;
                }
                _pieces[tpos] = sprite;
@@ -285,9 +274,8 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Called when a piece is finished moving into its requested position.
+     * Called when a piece is finished moving into its requested position. Derived classes may
-     * Derived classes may wish to take this opportunity to play a sound
+     * wish to take this opportunity to play a sound or whatnot.
     * or whatnot.
     */
    protected void pieceArrived (long tickStamp, Sprite sprite, int px, int py)
    {
@@ -295,9 +283,9 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Returns true if the piece that is supposed to be at the specified
+     * Returns true if the piece that is supposed to be at the specified coordinates is not yet
-     * coordinates is not yet there but is actually en route to that
+     * there but is actually en route to that location (due to a call to
-     * location (due to a call to {@link #movePiece(Sprite,int,int,int,int,long)}).
+     * {@link #movePiece(Sprite,int,int,int,int,long)}).
     */
    protected boolean isMoving (int px, int py)
    {
@@ -305,10 +293,9 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Returns the image used to display the given piece at coordinates
+     * Returns the image used to display the given piece at coordinates <code>(0, 0)</code> with
-     * <code>(0, 0)</code> with an orientation of {@link #NORTH}.  This
+     * an orientation of {@link #NORTH}. This serves as a convenience routine for those puzzles
-     * serves as a convenience routine for those puzzles that don't bother
+     * that don't bother rendering their pieces differently when placed at different board
     * rendering their pieces differently when placed at different board
     * coordinates or in different orientations.
     */
    public Mirage getPieceImage (int piece)
@@ -317,11 +304,10 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Returns the image used to display the given piece at the specified
+     * Returns the image used to display the given piece at the specified column and row with the
-     * column and row with the given orientation.
+     * given orientation.
     */
-    public abstract Mirage getPieceImage (
+    public abstract Mirage getPieceImage (int piece, int col, int row, int orient);
        int piece, int col, int row, int orient);
    @Override
    public void setBoard (Board board)
@@ -356,8 +342,7 @@ public abstract class DropBoardView extends PuzzleBoardView
        _pieces = new Sprite[width * height];
        for (int yy = 0; yy < height; yy++) {
            for (int xx = 0; xx < width; xx++) {
-                Sprite piece = createPieceSprite(
+                Sprite piece = createPieceSprite(_dboard.getPiece(xx, yy), xx, yy);
                    _dboard.getPiece(xx, yy), xx, yy);
                if (piece != null) {
                    int ppos = yy * width + xx;
                    getPiecePosition(xx, yy, spos);
@@ -403,8 +388,7 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Creates a new drop sprite used to animate the given pieces falling
+     * Creates a new drop sprite used to animate the given pieces falling in the specified column.
     * in the specified column.
     */
    public DropSprite createPieces (int col, int row, int[] pieces, int dist)
    {
@@ -412,9 +396,8 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Dirties the rectangle encompassing the segment with the given
+     * Dirties the rectangle encompassing the segment with the given direction and length whose
-     * direction and length whose bottom-leftmost corner is at <code>(col,
+     * bottom-leftmost corner is at <code>(col, row)</code>.
     * row)</code>.
     */
    public void dirtySegment (int dir, int col, int row, int len)
    {
@@ -425,9 +408,8 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Creates and returns an animation showing the specified score
+     * Creates and returns an animation showing the specified score floating up the view, with the
-     * floating up the view, with the label initially centered within the
+     * label initially centered within the view.
     * view.
     *
     * @param score the score text to display.
     * @param color the color of the text.
@@ -436,63 +418,57 @@ public abstract class DropBoardView extends PuzzleBoardView
    public ScoreAnimation createScoreAnimation (
        String score, Color color, Font font)
    {
-        return createScoreAnimation(
+        return createScoreAnimation(score, color, font, 0, _bhei - 1, _bwid, _bhei);
            score, color, font, 0, _bhei - 1, _bwid, _bhei);
    }
    /**
-     * Creates and returns an animation showing the specified score
+     * Creates and returns an animation showing the specified score floating up the view.
     * floating up the view.
     *
     * @param score the score text to display.
     * @param color the color of the text.
     * @param font the font to use.
-     * @param x the left coordinate in board coordinates of the rectangle
+     * @param x the left coordinate in board coordinates of the rectangle within which the score
-     * within which the score is to be centered.
+     * is to be centered.
-     * @param y the bottom coordinate in board coordinates of the
+     * @param y the bottom coordinate in board coordinates of the rectangle within which the score
-     * rectangle within which the score is to be centered.
+     * is to be centered.
-     * @param width the width in board coordinates of the rectangle within
+     * @param width the width in board coordinates of the rectangle within which the score is to
-     * which the score is to be centered.
+     * be centered.
-     * @param height the height in board coordinates of the rectangle
+     * @param height the height in board coordinates of the rectangle within which the score is to
-     * within which the score is to be centered.
+     * be centered.
     */
-    public ScoreAnimation createScoreAnimation (String score, Color color,
+    public ScoreAnimation createScoreAnimation (
-                                                Font font, int x, int y,
+        String score, Color color, Font font, int x, int y, int width, int height)
                                                int width, int height)
    {
        // create the score animation
-        ScoreAnimation anim =
+        ScoreAnimation anim = createScoreAnimation(score, color, font, x, y);
            createScoreAnimation(score, color, font, x, y);
        // position the label within the specified rectangle
        Dimension lsize = anim.getLabel().getSize();
        Point pos = new Point();
-        centerRectInBoardRect(
+        centerRectInBoardRect(x, y, width, height, lsize.width, lsize.height, pos);
            x, y, width, height, lsize.width, lsize.height, pos);
        anim.setLocation(pos.x, pos.y);
        return anim;
    }
    /**
-     * Creates the sprite that is used to display the specified piece. If
+     * Creates the sprite that is used to display the specified piece. If the piece represents no
-     * the piece represents no piece, this method should return null.
+     * piece, this method should return null.
     */
    protected Sprite createPieceSprite (int piece, int px, int py)
    {
        if (piece == PIECE_NONE) {
            return null;
        }
-        ImageSprite sprite = new ImageSprite(
+        ImageSprite sprite = new ImageSprite(getPieceImage(piece, px, py, NORTH));
            getPieceImage(piece, px, py, NORTH));
        sprite.setRenderOrder(-1);
        return sprite;
    }
    /**
-     * Populates <code>pos</code> with the most appropriate screen
+     * Populates <code>pos</code> with the most appropriate screen coordinates to center a
-     * coordinates to center a rectangle of the given width and height (in
+     * rectangle of the given width and height (in pixels) within the specified rectangle (in
-     * pixels) within the specified rectangle (in board coordinates).
+     * board coordinates).
     *
     * @param bx the bounding rectangle's left board coordinate.
     * @param by the bounding rectangle's bottom board coordinate.
@@ -500,8 +476,7 @@ public abstract class DropBoardView extends PuzzleBoardView
     * @param bhei the bounding rectangle's height in board coordinates.
     * @param rwid the width of the rectangle to position in pixels.
     * @param rhei the height of the rectangle to position in pixels.
-     * @param pos the point to populate with the rectangle's final
+     * @param pos the point to populate with the rectangle's final position.
     * position.
     */
    protected void centerRectInBoardRect (
        int bx, int by, int bwid, int bhei, int rwid, int rhei, Point pos)
@@ -515,10 +490,9 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Rotates the given drop block sprite to the specified orientation,
+     * Rotates the given drop block sprite to the specified orientation, updating the image as
-     * updating the image as necessary.  Derived classes that make use of
+     * necessary. Derived classes that make use of block dropping functionality should override
-     * block dropping functionality should override this method to do the
+     * this method to do the right thing.
     * right thing.
     */
    public void rotateDropBlock (DropBlockSprite sprite, int orient)
    {
@@ -543,9 +517,8 @@ public abstract class DropBoardView extends PuzzleBoardView
    }
    /**
-     * Renders the row of rising pieces to the given graphics context.
+     * Renders the row of rising pieces to the given graphics context. Sub-classes that make use
-     * Sub-classes that make use of board rising functionality should
+     * of board rising functionality should override this method to draw the rising piece row.
     * override this method to draw the rising piece row.
     */
    protected void renderRisingPieces (Graphics2D gfx, Rectangle dirtyRect)
    {
@@ -53,14 +53,13 @@ import com.threerings.puzzle.util.PuzzleContext;
 import static com.threerings.puzzle.Log.log;
 /**
- * Games that wish to make use of the drop puzzle services will need to
+ * Games that wish to make use of the drop puzzle services will need to create an extension of
- * create an extension of this delegate class, customizing it for their
+ * this delegate class, customizing it for their particular game and then adding it via
- * particular game and then adding it via {@link PlaceController#addDelegate}.
+ * {@link PlaceController#addDelegate}.
 *
- * <p> It handles logical actions for a puzzle game that generally
+ * <p> It handles logical actions for a puzzle game that generally consists of a two-dimensional
- * consists of a two-dimensional board containing pieces, with new pieces
+ * board containing pieces, with new pieces either falling into the board as a "drop block", or
- * either falling into the board as a "drop block", or rising into the
+ * rising into the bottom of the board in new piece rows.
 * bottom of the board in new piece rows.
 *
 * <p> Derived classes must implement {@link #getPieceVelocity} and {@link #evolveBoard}.
 *
@@ -95,8 +94,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    public static final String RAISE_ROW = "raise_row";
    /**
-     * Creates a delegate with the specified drop game logic and
+     * Creates a delegate with the specified drop game logic and controller.
     * controller.
     */
    public DropControllerDelegate (PuzzleController ctrl, DropLogic logic)
    {
@@ -141,8 +139,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Returns the speed with which the next board row should rise into
+     * Returns the speed with which the next board row should rise into place, in pixels per
-     * place, in pixels per millisecond.
+     * millisecond.
     */
    protected float getRiseVelocity ()
    {
@@ -166,7 +164,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    {
        super.startAction();
-//         Log.info("Starting drop action");
+//         log.info("Starting drop action");
        // add ourselves as a frame participant
        _ctx.getFrameManager().registerFrameParticipant(this);
@@ -181,17 +179,15 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        // it on its merry way once more
        if (_blocksprite != null) {
            long delta = _dview.getTimeStamp() - _blockStamp;
-            log.info("Restarting drop sprite [delta=" + delta + "].");
+            log.info("Restarting drop sprite", "delta", delta);
            _blocksprite.fastForward(delta);
            _blockStamp = 0L;
            _dview.addSprite(_blocksprite);
-            // if we cleared the action while the drop sprite was
+            // if we cleared the action while the drop sprite was bouncing, we need to land the
-            // bouncing, we need to land the block to get things going
+            // block to get things going again
            // again
            if (_blocksprite.isBouncing()) {
-                log.info("Ended on a bounce, landing the block and " +
+                log.info("Ended on a bounce, landing the block and starting things up.");
                         "starting things up.");
                checkBlockLanded("bounced", true, true);
            }
        }
@@ -210,16 +206,15 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Clears out all of the action in the board; removes any drop block
+     * Clears out all of the action in the board; removes any drop block sprites, any pieces
-     * sprites, any pieces rising in the board, and resets the animation
+     * rising in the board, and resets the animation timestamps.
     * timestamps.
     */
    @Override
    protected void clearAction ()
    {
        super.clearAction();
-//         Log.info("Clearing drop action.");
+//         log.info("Clearing drop action.");
        // do away with the bounce interval
        _bounceStamp = 0;
@@ -310,8 +305,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        }
        if (handled && _blocksprite != null) {
-            // land the block if it's been placed on something solid as a
+            // land the block if it's been placed on something solid as a result of one of the
-            // result of one of the above actions
+            // above actions
            String source = "fiddled [cmd=" + cmd + "]";
            if (checkBlockLanded(source, false, false)) {
                startBounceTimer(source);
@@ -335,19 +330,17 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        int row = _blocksprite.getRow(), col = _blocksprite.getColumn();
        int dx = (dir == LEFT) ? -1 : 1;
-        // if the sprite has made it to the bottom of the board then we
+        // if the sprite has made it to the bottom of the board then we don't want to allow it to
-        // don't want to allow it to "virtually" fall any further because
+        // "virtually" fall any further because of the bounce interval
        // of the bounce interval
         float pctdone = (row >= (_bhei - 1)) ? 0 :
             _blocksprite.getPercentDone(_dview.getTimeStamp());
        // get the drop block position resulting from the move
-        Point pos = _dboard.getForgivingMove(
+        Point pos = _dboard.getForgivingMove(bb.x, bb.y, bb.width, bb.height, dx, 0, pctdone);
            bb.x, bb.y, bb.width, bb.height, dx, 0, pctdone);
        if (pos != null) {
            int frow = row + (pos.y - bb.y);
            int fcol = col + (pos.x - bb.x);
-            // Log.info("Valid move [row=" + frow + ", col=" + col + "].");
+            // log.info("Valid move", "row", frow, "col", col);
            _blocksprite.setBoardLocation(frow, fcol);
        }
    }
@@ -364,9 +357,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        // gather information regarding the attempted rotation
        int[] rows = _blocksprite.getRows();
        int[] cols = _blocksprite.getColumns();
-        // if the sprite has made it to the bottom of the board then we
+        // if the sprite has made it to the bottom of the board then we don't want to allow it to
-        // don't want to allow it to "virtually" fall any further because
+        // "virtually" fall any further because of the bounce interval
        // of the bounce interval
        float pctdone = (rows[0] >= (_bhei - 1)) ? 0 :
            _blocksprite.getPercentDone(_dview.getTimeStamp());
@@ -375,10 +367,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
            rows, cols, _blocksprite.getOrientation(), dir,
            getRotationType(), pctdone, _blocksprite.canPopup());
        if (info != null) {
-//             Log.info("Found valid rotation " +
+//             log.info("Found valid rotation",
-//                      "[orient=" + DirectionUtil.toShortString(info[0]) +
+//                 "orient", DirectionUtil.toShortString(info[0]), "col", info[1], "row", info[2],
-//                      ", col=" + info[1] + ", row=" + info[2] +
+//                 "blocksprite", _blocksprite);
 //                      ", blocksprite=" + _blocksprite + "].");
            // update the piece image
            _dview.rotateDropBlock(_blocksprite, info[0]);
@@ -396,16 +387,16 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called when the drop block has rotated in the specified direction
+     * Called when the drop block has rotated in the specified direction to allow derived classes
-     * to allow derived classes to engage in any game-specific antics.
+     * to engage in any game-specific antics.
     */
    protected void blockDidRotate (int dir)
    {
    }
    /**
-     * Returns the rotation type used by this drop game. Either {@link
+     * Returns the rotation type used by this drop game. Either {@link DropBoard#RADIAL_ROTATION}
-     * DropBoard#RADIAL_ROTATION} or {@link DropBoard#INPLACE_ROTATION}.
+     * or {@link DropBoard#INPLACE_ROTATION}.
     */
    protected int getRotationType ()
    {
@@ -421,7 +412,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        // only allow changing the piece velocity if we're not bouncing
        if (_blocksprite != null && _bounceStamp == 0) {
-            // Log.info("Updating drop block velocity [fast=" + fast + "].");
+            // log.info("Updating drop block velocity", "fast", fast);
            _blocksprite.setVelocity(getPieceVelocity(fast));
        }
    }
@@ -437,13 +428,11 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    protected void dropNextBlock ()
    {
        if (_blocksprite != null || !_ctrl.hasAction()) {
-            log.info("Not dropping block [bs=" + (_blocksprite != null) +
+            log.info("Not dropping block", "bs", _blocksprite != null, "action", _ctrl.hasAction());
                     ", action=" + _ctrl.hasAction() + "].");
            return;
        }
-        // determine whether or not the game should be ended because we
+        // determine whether or not the game should be ended because we can't drop the next block
        // can't drop the next block
        if (checkDropEndsGame()) {
            return;
        }
@@ -476,12 +465,10 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called by {@link #dropNextBlock} to determine whether the game
+     * Called by {@link #dropNextBlock} to determine whether the game should be ended rather than
-     * should be ended rather than dropping the next block because the
+     * dropping the next block because the board is filled and a new block cannot enter. If true
-     * board is filled and a new block cannot enter. If true is returned,
+     * is returned, the drop controller assumes that the derived class will have ended or reset
-     * the drop controller assumes that the derived class will have ended
+     * the game as appropriate and will simply abandon its attempt to drop the next block.
     * or reset the game as appropriate and will simply abandon its
     * attempt to drop the next block.
     */
    protected boolean checkDropEndsGame ()
    {
@@ -489,9 +476,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called only for block-dropping puzzles when it's time to create the
+     * Called only for block-dropping puzzles when it's time to create the next drop block.
-     * next drop block.  Returns the drop block sprite if it was
+     * Returns the drop block sprite if it was successfully created, or <code>null</code> if it
-     * successfully created, or <code>null</code> if it was not.
+     * was not.
     */
    protected DropBlockSprite createNextBlock ()
    {
@@ -510,8 +497,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    /**
     * Called when a drop sprite posts a piece moved event.
     */
-    protected void handleDropSpriteMoved (
+    protected void handleDropSpriteMoved (DropSprite sprite, long when, int col, int row)
        DropSprite sprite, long when, int col, int row)
    {
        if (sprite instanceof DropBlockSprite) {
            if (checkBlockLanded("piece-moved", false, true)) {
@@ -538,9 +524,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Applies the pieces in the given sprite to the specified column and
+     * Applies the pieces in the given sprite to the specified column and row in the board. Called
-     * row in the board.  Called when a drop sprite has finished
+     * when a drop sprite has finished traversing its entire distance.
     * traversing its entire distance.
     */
    protected void applyDropSprite (DropSprite sprite, int col, int row)
    {
@@ -551,17 +536,16 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        for (int dy = 0; dy < pieces.length; dy++) {
            // pieces outside the board are discarded
            if (row - dy >= 0) {
-                // note: vertical segments are applied counting downwards
+                // note: vertical segments are applied counting downwards from the starting row
-                // from the starting row toward row zero
+                // toward row zero
                _dview.createPiece(pieces[dy], col, row - dy);
            }
        }
    }
    /**
-     * Called to determine whether it is safe to evolve the board. The
+     * Called to determine whether it is safe to evolve the board. The default implementation does
-     * default implementation does not allow board evolution if there are
+     * not allow board evolution if there are sprites or animations active on the board.
     * sprites or animations active on the board.
     */
    protected boolean canEvolveBoard ()
    {
@@ -569,24 +553,21 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Evolves the board to an unchanging state. If the board is in a
+     * Evolves the board to an unchanging state. If the board is in a state where pieces should
-     * state where pieces should react with one another to cause changes
+     * react with one another to cause changes to the board state (such as piece dropping via
-     * to the board state (such as piece dropping via {@link #dropPieces},
+     * {@link #dropPieces}, piece destruction, and/or piece joining), this is where that process
-     * piece destruction, and/or piece joining), this is where that
+     * should be effected.
     * process should be effected.
     *
-     * <p> When no further evolution is possible and the board has
+     * <p> When no further evolution is possible and the board has stabilized this method should
-     * stabilized this method should return false to indicate that such
+     * return false to indicate that such action should be taken. That will result in a follow-up
-     * action should be taken. That will result in a follow-up call to
+     * call to {@link #boardDidStabilize} (assuming that the action was not cleared prior to the
-     * {@link #boardDidStabilize} (assuming that the action was not
+     * final stabilization of the board).
     * cleared prior to the final stabilization of the board).
     */
    protected abstract boolean evolveBoard ();
    /**
-     * Derived classes should call this method whenever they change some
+     * Derived classes should call this method whenever they change some board state that will
-     * board state that will require board evolution to restabilize the
+     * require board evolution to restabilize the board.
     * board.
     */
    protected void unstabilizeBoard ()
    {
@@ -594,11 +575,10 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called when the board has been fully evolved and is once again
+     * Called when the board has been fully evolved and is once again stable. The default
-     * stable. The default implementation updates the player's local board
+     * implementation updates the player's local board summary and drops the next block into the
-     * summary and drops the next block into the board, but derived
+     * board, but derived classes may wish to perform custom actions if they don't use drop blocks
-     * classes may wish to perform custom actions if they don't use drop
+     * or have other requirements.
     * blocks or have other requirements.
     */
    protected void boardDidStabilize ()
    {
@@ -607,10 +587,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Updates the player's own local board summary to reflect the local
+     * Updates the player's own local board summary to reflect the local copy of the player's
-     * copy of the player's board which is likely to be more up to date
+     * board which is likely to be more up to date than the server-side board from which all other
-     * than the server-side board from which all other player board
+     * player board summaries originate.
     * summaries originate.
     */
    public void updateSelfSummary ()
    {
@@ -624,9 +603,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called when an animation finishes doing its business.  Derived
+     * Called when an animation finishes doing its business. Derived classes may wish to override
-     * classes may wish to override this method but should be sure to call
+     * this method but should be sure to call <code>super.animationDidFinish()</code>.
     * <code>super.animationDidFinish()</code>.
     */
    protected void animationDidFinish (Animation anim)
    {
@@ -634,17 +612,16 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Checks whether the drop block can continue dropping and lands its
+     * Checks whether the drop block can continue dropping and lands its pieces if not. Returns
-     * pieces if not.  Returns whether at least one piece of the block has
+     * whether at least one piece of the block has landed; note that the other piece may need
-     * landed; note that the other piece may need subsequent dropping.
+     * subsequent dropping.
     *
-     * @param commit if true, the block landing is committed, if false, it
+     * @param commit if true, the block landing is committed, if false, it is only checked, not
-     * is only checked, not committed.
+     * committed.
-     * @param atTop whether the block sprite is to be treated as being at
+     * @param atTop whether the block sprite is to be treated as being at the top of its current
-     * the top of its current row.
+     * row.
     */
-    protected boolean checkBlockLanded (
+    protected boolean checkBlockLanded (String source, boolean commit, boolean atTop)
        String source, boolean commit, boolean atTop)
    {
        if (_blocksprite == null) {
            return true;
@@ -659,25 +636,20 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        float pctdone = (atTop) ? 0.0f :
            _blocksprite.getPercentDone(_dview.getTimeStamp());
-//         Log.info("Checking landed [source=" + source +
+//        log.info("Checking landed",
-//                  ", bounceRow=" + _bounceRow +
+//            "source", source, "bounceRow", _bounceRow, "rows", rows, "cols", cols,
-//                  ", rows=" + StringUtil.toString(rows) +
+//            "orient", DirectionUtil.toShortString(_blocksprite.getOrientation()),
-//                  ", cols=" + StringUtil.toString(cols) +
+//            "commit", commit, "pctdone", pctdone);
 //                  ", orient=" + DirectionUtil.toShortString(
 //                      _blocksprite.getOrientation()) +
 //                  ", commit=" + commit + ", pctdone=" + pctdone + "].");
        if (_dboard.isValidDrop(rows, cols, pctdone)) {
            if (commit) {
-                log.info("Not valid drop [source=" + source +
+                log.info("Not valid drop",
-                         ", commit=" + commit + ", atTop=" + atTop +
+                    "source", source, "commit", commit, "atTop", atTop, "pctdone", pctdone);
                         ", pctdone=" + pctdone + "].");
            }
            return false;
        }
-        // if we're committing the landing, remove the sprite and update
+        // if we're committing the landing, remove the sprite and update the board and all that
        // the board and all that
        if (commit) {
            // give sub-classes a chance to do any pre-landing business
            blockWillLand();
@@ -689,23 +661,18 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
                if (rows[ii] >= 0) {
                    int col = cols[ii], row = rows[ii];
-                    // sanity-check the block to make sure it's located in
+                    // sanity-check the block to make sure it's located in a valid position, and
-                    // a valid position, and that we aren't somehow
+                    // that we aren't somehow overwriting an existing piece
                    // overwriting an existing piece
                    if (col < 0 || col >= _bwid || row >= _bhei) {
-                        log.warning("Placing drop block piece outside board " +
+                        log.warning("Placing drop block piece outside board bounds!?",
-                                    "bounds!? [x=" + col + ", y=" + row +
+                            "x", col, "y", row, "pidx", ii, "blocksprite", _blocksprite);
                                    ", pidx=" + ii +
                                    ", blocksprite=" + _blocksprite + "].");
                        error = true;
                    } else {
                        int cpiece = _dboard.getPiece(col, row);
                        if (cpiece != PIECE_NONE) {
-                            log.warning("Placing drop block piece onto " +
+                            log.warning("Placing drop block piece onto occupied board position!?",
-                                        "occupied board position!? [x=" + col +
+                                "x", col, "y", row, "pidx", ii, "blocksprite", _blocksprite);
                                        ", y=" + row + ", pidx=" + ii +
                                        ", blocksprite=" + _blocksprite + "].");
                            error = true;
                        }
                    }
@@ -736,10 +703,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called only for block-dropping puzzles when the drop block is about
+     * Called only for block-dropping puzzles when the drop block is about to land on something.
-     * to land on something.  Derived classes may wish to override this
+     * Derived classes may wish to override this method to perform game-specific actions such as
-     * method to perform game-specific actions such as queueing up a
+     * queueing up a "block placed" progress event.
     * "block placed" progress event.
     */
    protected void blockWillLand ()
    {
@@ -747,9 +713,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called only for block-dropping puzzles when the drop block lands on
+     * Called only for block-dropping puzzles when the drop block lands on something. Derived
-     * something.  Derived classes may wish to override this method to
+     * classes may wish to override this method to perform any game-specific actions.
     * perform any game-specific actions.
     */
    protected void blockDidLand ()
    {
@@ -757,19 +722,16 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called when a block lands. We give the user a smidgen of time to
+     * Called when a block lands. We give the user a smidgen of time to continue to fiddle with
-     * continue to fiddle with the block before we actually land it. If
+     * the block before we actually land it. If the block is still landed when the bounce timer
-     * the block is still landed when the bounce timer expires, we commit
+     * expires, we commit the landing, otherwise we let the block keep falling.
     * the landing, otherwise we let the block keep falling.
     */
    protected void startBounceTimer (String source)
    {
        int bounceRow = IntListUtil.getMaxValue(_blocksprite.getRows());
-//         Log.info("startBounceTimer [source=" + source +
+//         log.info("startBounceTimer",
-//                  ", bounceStamp=" + _bounceStamp +
+//             "source", source, "bounceStamp", _bounceStamp, "time", _dview.getTimeStamp(),
-//                  ", time=" + _dview.getTimeStamp() +
+//             "bounceRow", _bounceRow, "nbounceRow", bounceRow);
 //                  ", bounceRow=" + _bounceRow +
 //                  ", nbounceRow=" + bounceRow + "].");
        // forcibly land the block if we bounce twice at the same row
        if (_bounceStamp == 0 && _bounceRow == bounceRow) {
@@ -779,26 +741,23 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
            return;
        }
-        // if the bounce "timer" is already started, the user probably did
+        // if the bounce "timer" is already started, the user probably did something like rotate
-        // something like rotate the piece while it was bouncing (which is
+        // the piece while it was bouncing (which is why we give them the bounce interval), so we
-        // why we give them the bounce interval), so we don't reset
+        // don't reset
        if (_bounceStamp == 0) {
-            // slow the piece down so that it doesn't fly past the
+            // slow the piece down so that it doesn't fly past the coordinates at which it's
-            // coordinates at which it's potentially landing; we have to
+            // potentially landing; we have to do this before we tell the sprite that it's bouncing
-            // do this before we tell the sprite that it's bouncing
+            // because changing the velocity fiddles with the rowstamp and we're going to reset the
-            // because changing the velocity fiddles with the rowstamp and
+            // rowstamp when we tell the sprite that it's bouncing
            // we're going to reset the rowstamp when we tell the sprite
            // that it's bouncing
            _blocksprite.setVelocity(getPieceVelocity(false));
-            // set up our bounce interval (it depends on the current piece
+            // set up our bounce interval (it depends on the current piece velocity and so must be
-            // velocity and so must be set at the time we bounce)
+            // set at the time we bounce)
            _bounceInterval = (int)
                ((_dview.getPieceHeight() * BOUNCE_FRACTION) /
                 getPieceVelocity(false));
-//             Log.info("bounceInterval=" + _bounceInterval +
+//             log.info("bounce", "bounceInterval", _bounceInterval,
-//                      ", phei=" + _dview.getPieceHeight() +
+//                 "phei", _dview.getPieceHeight(), "vel", getPieceVelocity(false));
 //                      ", vel=" + getPieceVelocity(false));
            // make a note of the time we started bouncing
            _bounceStamp = _dview.getTimeStamp();
@@ -812,15 +771,13 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called when the bounce timer expires. Herein we either commit the
+     * Called when the bounce timer expires. Herein we either commit the landing of a block if it
-     * landing of a block if it is still landed or let it keep falling if
+     * is still landed or let it keep falling if it is no longer landed.
-     * it is no longer landed.
+     */
    */
    protected void bounceTimerExpired ()
    {
-//         Log.info("bounceTimerExpired [bounceStamp=" + _bounceStamp +
+//         log.info("bounceTimerExpired",
-//                  ", time=" + _dview.getTimeStamp() +
+//             "bounceStamp", _bounceStamp, "time", _dview.getTimeStamp(), "bounceRow", _bounceRow);
 //                  ", bounceRow=" + _bounceRow + "].");
        // make sure we weren't cancelled for some reason
        if (_bounceStamp != 0) {
@@ -836,11 +793,10 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Drops any pieces that need dropping and returns whether any pieces
+     * Drops any pieces that need dropping and returns whether any pieces were dropped. Derived
-     * were dropped.  Derived classes that would like to drop their pieces
+     * classes that would like to drop their pieces should include a call to this method in their
-     * should include a call to this method in their {@link #evolveBoard}
+     * {@link #evolveBoard} implementation, and must also override {@link #getPieceDropLogic} to
-     * implementation, and must also override {@link #getPieceDropLogic}
+     * provide their game-specific piece dropper implementation.
     * to provide their game-specific piece dropper implementation.
     */
    protected boolean dropPieces ()
    {
@@ -848,8 +804,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
            public void pieceDropped (
                int piece, int sx, int sy, int dx, int dy) {
                float vel = getPieceVelocity(true) * 1.5f;
-                long duration = (long)(_dview.getPieceHeight() *
+                long duration = (long)(_dview.getPieceHeight() * Math.abs(dy-sy) / vel);
                                       Math.abs(dy-sy) / vel);
                if (sy < 0) {
                    _dview.createPiece(piece, sx, sy, dx, dy, duration);
                } else {
@@ -861,10 +816,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Returns the piece drop logic used to drop any pieces that need
+     * Returns the piece drop logic used to drop any pieces that need dropping in the board.
-     * dropping in the board.  Derived classes that intend to make use of
+     * Derived classes that intend to make use of {@link #dropPieces} must implement this method
-     * {@link #dropPieces} must implement this method and return a
+     * and return a reference to their game-specific piece dropper implementation.
     * reference to their game-specific piece dropper implementation.
     */
    protected PieceDropLogic getPieceDropLogic ()
    {
@@ -893,24 +847,24 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
            bounceTimerExpired();
        }
-        // if we can't evolve the board because it doesn't need evolving
+        // if we can't evolve the board because it doesn't need evolving or things are going on,
-        // or things are going on, we stop here
+        // we stop here
        if (_stable || !canEvolveBoard()) {
            return;
        }
-        // if we do not evolve the board in any way, let the derived class
+        // if we do not evolve the board in any way, let the derived class know that the board
-        // know that the board stabilized so that they can drop in a new
+        // stabilized so that they can drop in a new piece if they like or take whatever other
-        // piece if they like or take whatever other action is appropriate
+        // action is appropriate
        boolean evolving = evolveBoard();
        boolean debug = false;
        if (debug) {
-            log.info("Evolved board [evolving=" + evolving + "].");
+            log.info("Evolved board", "evolving", evolving);
        }
-        // if we're no longer evolving and the action has not ended, go
+        // if we're no longer evolving and the action has not ended, go ahead and let our derived
-        // ahead and let our derived class know that the board has
+        // class know that the board has stabilized so that it can drop in the next piece or
-        // stabilized so that it can drop in the next piece or somesuch
+        // somesuch
        if (!evolving) {
            // no evolving again until someone destabilizes the board
            _stable = true;
@@ -921,8 +875,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
            }
            boardDidStabilize();
-            // ensure that if we have been postponing action due to board
+            // ensure that if we have been postponing action due to board evolution, that it will
-            // evolution, that it will now be cleared
+            // now be cleared
            if (!_ctrl.hasAction()) {
                if (debug) {
                    log.info("Maybe clearing action.");
@@ -971,8 +925,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    {
        // don't overwrite an existing zip
        if (_zipstamp == 0) {
-            // if we're paused, inherit the pause time, otherwise use the
+            // if we're paused, inherit the pause time, otherwise use the current time
            // current time
            if (_rpstamp != 0) {
                _zipstamp = _rpstamp;
            } else {
@@ -982,8 +935,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called periodically on the frame tick.  Raises the board row based
+     * Called periodically on the frame tick. Raises the board row based on the time since the
-     * on the time since the current row traversal began.
+     * current row traversal began.
     */
    /*
    protected void raiseBoard (long tickStamp)
@@ -1040,10 +993,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        _dview.setRiseOffset(ypos);
        if (rose) {
-            // check to see if this means doom and defeat (even though the
+            // check to see if this means doom and defeat (even though the game might be over, we
-            // game might be over, we still want to advance the piece
+            // still want to advance the piece packet one last time and do the last rise so that
-            // packet one last time and do the last rise so that the
+            // the server can tell that we kicked the proverbial bucket)
            // server can tell that we kicked the proverbial bucket)
            boolean canRise = checkCanRise();
            // apply the rising row pieces to the board
@@ -1062,23 +1014,21 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
                unstabilizeBoard();
            } else {
-                log.debug("Sticking fork in it [risers=" +
+                log.debug("Sticking fork in it", "risers",  pieces);
                          StringUtil.toString(pieces) + ".");
                // let the controller know that we're done for
                _ctrl.resetGame();
            }
        }
-//         Log.info("Board rise [msecs=" + msecs + ", roff=" + ypos +
+//         log.info("Board rise",
-//                  ", pctdone=" + pctdone + ", zipsecs=" + zipsecs + "].");
+//             "msecs", msecs, "roff", ypos, "pctdone", pctdone, "zipsecs", zipsecs);
    }
    */
    /**
-     * Called to determine whether or not rising a new row into the board
+     * Called to determine whether or not rising a new row into the board is legal. The default
-     * is legal. The default implementation will return false if the top
+     * implementation will return false if the top row of the board contains any pieces.
     * row of the board contains any pieces.
     */
    protected boolean checkCanRise ()
    {
@@ -1086,10 +1036,9 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called only for board-rising puzzles before effecting the rising of
+     * Called only for board-rising puzzles before effecting the rising of the board by one row.
-     * the board by one row.  Derived classes may wish to override this
+     * Derived classes may wish to override this method to add any desired behaviour, but should
-     * method to add any desired behaviour, but should be sure to call
+     * be sure to call <code>super.boardWillRise()</code>.
     * <code>super.boardWillRise()</code>.
     */
    protected void boardWillRise ()
    {
@@ -1097,17 +1046,16 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    }
    /**
-     * Called only for board-rising puzzles when the board has finished
+     * Called only for board-rising puzzles when the board has finished rising one row. Derived
-     * rising one row.  Derived classes may wish to override this method
+     * classes may wish to override this method to add any desired behaviour, but should be sure
-     * to add any desired behaviour, but should be sure to call
+     * to call <code>super.boardDidRise()</code>.
     * <code>super.boardDidRise()</code>.
     */
    protected void boardDidRise ()
    {
        // nothing for now
    }
-    /** The yohoho context. */
+    /** The puzzle context. */
    protected PuzzleContext _ctx;
    /** Our puzzle controller. */
@@ -1143,8 +1091,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    /** The drop block sprite associated with the landing block, if any. */
    protected DropBlockSprite _blocksprite;
-    /** The piece dropper used to drop pieces in the board if the puzzle
+    /** The piece dropper used to drop pieces in the board if the puzzle chooses to make use of
-     * chooses to make use of piece dropping functionality. */
+     * piece dropping functionality. */
    protected PieceDropper _dropper;
    /** The time at which the board rise was paused. */
@@ -1168,8 +1116,8 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    /** The row at which we last bounced, or {@link Integer#MIN_VALUE}. */
    protected int _bounceRow;
-    /** The timestamp used to keep track of when the drop block was
+    /** The timestamp used to keep track of when the drop block was removed so that we can
-     * removed so that we can fast-forward it when restored. */
+     * fast-forward it when restored. */
    protected long _blockStamp;
    /** Whether the drop blocks are currently dropping quickly. */
@@ -1191,8 +1139,7 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
        }
    };
-    /** A piece operation that will update piece sprites as board
+    /** A piece operation that will update piece sprites as board positions are updated. */
     * positions are updated. */
    protected DropBoard.PieceOperation _updateBoardOp =
        new DropBoard.PieceOperation() {
        public boolean execute (DropBoard board, int col, int row) {
@@ -1210,7 +1157,6 @@ public abstract class DropControllerDelegate extends PuzzleControllerDelegate
    /** The delay in milliseconds between board rising intervals. */
    protected static final long RISE_INTERVAL = 50L;
-    /** Defines the distance of a piece that we allow to bounce before we
+    /** Defines the distance of a piece that we allow to bounce before we land it. */
     * land it. */
    protected static final float BOUNCE_FRACTION = 0.125f;
 }
@@ -24,8 +24,8 @@ package com.threerings.puzzle.drop.client;
 import com.threerings.puzzle.data.BoardSummary;
 /**
- * Puzzles using the drop services need implement this interface to
+ * Puzzles using the drop services need implement this interface to display drop puzzle related
- * display drop puzzle related information.
+ * information.
 */
 public interface DropPanel
 {
@@ -34,8 +34,8 @@ import com.threerings.media.image.Mirage;
 import com.threerings.media.sprite.Sprite;
 /**
- * The drop sprite is a sprite that displays one or more pieces falling
+ * The drop sprite is a sprite that displays one or more pieces falling toward the bottom of the
- * toward the bottom of the board.
+ * board.
 */
 public class DropSprite extends Sprite
 {
@@ -48,8 +48,7 @@ public class DropSprite extends Sprite
     * @param pieces the pieces displayed by the sprite.
     * @param dist the distance the sprite is to drop in rows.
     */
-    public DropSprite (
+    public DropSprite (DropBoardView view, int col, int row, int[] pieces, int dist)
        DropBoardView view, int col, int row, int[] pieces, int dist)
    {
        this(view, col, row, pieces, dist, -1);
    }
@@ -65,8 +64,7 @@ public class DropSprite extends Sprite
     * @param renderOrder the render order.
     */
    public DropSprite (
-        DropBoardView view, int col, int row, int[] pieces, int dist,
+        DropBoardView view, int col, int row, int[] pieces, int dist, int renderOrder)
        int renderOrder)
    {
        _view = view;
        _col = col;
@@ -188,11 +186,9 @@ public class DropSprite extends Sprite
    }
    /**
-     * Sets the velocity of this sprite.  The time at which the current
+     * Sets the velocity of this sprite. The time at which the current row was entered is modified
-     * row was entered is modified so that the sprite position will remain
+     * so that the sprite position will remain the same when calculated using the new velocity
-     * the same when calculated using the new velocity since the piece
+     * since the piece sprite may have its velocity modified in the middle of a row traversal.
     * sprite may have its velocity modified in the middle of a row
     * traversal.
     */
    public void setVelocity (float velocity)
    {
@@ -235,15 +231,14 @@ public class DropSprite extends Sprite
            _stopstamp = 0;
        } else {
-            // we're continuing a previous drop, so make use of any
+            // we're continuing a previous drop, so make use of any previously existing time
            // previously existing time
            _rowstamp = _endstamp;
        }
    }
    /**
-     * Returns true if this drop sprite is dropping, false if it has been
+     * Returns true if this drop sprite is dropping, false if it has been {@link #stop}ped or has
-     * {@link #stop}ped or has not yet been {@link #drop}ped.
+     * not yet been {@link #drop}ped.
     */
    public boolean isDropping ()
    {
@@ -257,33 +252,29 @@ public class DropSprite extends Sprite
    {
        if (_stopstamp == 0) {
            _stopstamp = _view.getTimeStamp();
-            // Log.info("Stopped piece [piece=" + this + "].");
+            // log.info("Stopped piece", "piece", this);
        }
    }
    /**
-     * Puts the drop sprite into (or takes it out of) bouncing
+     * Puts the drop sprite into (or takes it out of) bouncing mode. Bouncing mode is used to put
-     * mode. Bouncing mode is used to put the sprite into limbo after it
+     * the sprite into limbo after it lands but before we commit the landing, giving the user a
-     * lands but before we commit the landing, giving the user a last
+     * last moment change move or rotate the piece. While the sprite is "bouncing" it will be
-     * moment change move or rotate the piece. While the sprite is
+     * rendered one pixel below it's at rest state.
     * "bouncing" it will be rendered one pixel below it's at rest state.
     */
    public void setBouncing (boolean bouncing)
    {
        _bouncing = bouncing;
        if (_bouncing) {
-            // if we've activated bouncing, shift the sprite slightly to
+            // if we've activated bouncing, shift the sprite slightly to illustrate its new state
            // illustrate its new state
            shiftForBounce();
-            // to prevent funny business in the event that we were a long
+            // to prevent funny business in the event that we were a long ways past the end of the
-            // ways past the end of the row when we landed, we warp the
+            // row when we landed, we warp the sprite back to the exact point of landing for the
-            // sprite back to the exact point of landing for the purposes
+            // purposes of the bounce and any subsequent antics
            // of the bounce and any subsequent antics
            _endstamp = _rowstamp = _view.getTimeStamp();
-//             Log.info("Adjusted rowstap due to bounce " +
+//             log.info("Adjusted rowstap due to bounce", "time", _endstamp);
 //                      "[time=" + _endstamp + "].");
        }
    }
@@ -296,8 +287,7 @@ public class DropSprite extends Sprite
    }
    /**
-     * Updates the sprite's location to illustrate that it is currently in
+     * Updates the sprite's location to illustrate that it is currently in the "bouncing" state.
     * the "bouncing" state.
     */
    protected void shiftForBounce ()
    {
@@ -311,15 +301,13 @@ public class DropSprite extends Sprite
    }
    /**
-     * Returns a value between <code>0.0</code> and <code>1.0</code>
+     * Returns a value between <code>0.0</code> and <code>1.0</code> representing how far the
-     * representing how far the piece has moved toward the next row
+     * piece has moved toward the next row as of the given time stamp.
     * as of the given time stamp.
     */
    public float getPercentDone (long timestamp)
    {
-        // if we've never been ticked and so haven't yet initialized our
+        // if we've never been ticked and so haven't yet initialized our row start timestamp, just
-        // row start timestamp, just let the caller know that we've not
+        // let the caller know that we've not traversed our row at all
        // traversed our row at all
        if (_rowstamp == 0) {
            return 0.0f;
        }
@@ -328,10 +316,9 @@ public class DropSprite extends Sprite
        float travpix = msecs * _vel;
        float pctdone = (travpix / _unit);
-//         Log.info("getPercentDone [timestamp=" + timestamp +
+//         log.info("getPercentDone",
-//                  ", rowstamp=" + _rowstamp + ", msecs=" + msecs +
+//             "timestamp", timestamp, "rowstamp", _rowstamp, "msecs", msecs, "travpix", travpix,
-//                  ", travpix=" + travpix + ", pctdone=" + pctdone +
+//             "pctdone", pctdone, "vel", _vel);
 //                  ", vel=" + _vel + "].");
        return pctdone;
    }
@@ -352,8 +339,7 @@ public class DropSprite extends Sprite
            // ask the board for the render position of this piece
            _view.getPiecePosition(pcol, prow, _renderPos);
            // draw the piece image
-            paintPieceImage(gfx, ii, pcol, prow, _orient,
+            paintPieceImage(gfx, ii, pcol, prow, _orient, _renderPos.x + dx, _renderPos.y + dy);
                            _renderPos.x + dx, _renderPos.y + dy);
            // increment the target column and row
            pcol += incx;
            prow += incy;
@@ -363,8 +349,8 @@ public class DropSprite extends Sprite
    /**
     * Paints the specified piece with the supplied parameters.
     */
-    protected void paintPieceImage (Graphics2D gfx, int pieceidx,
+    protected void paintPieceImage (
-                                    int col, int row, int orient, int x, int y)
+        Graphics2D gfx, int pieceidx, int col, int row, int orient, int x, int y)
    {
        Mirage image = _view.getPieceImage(_pieces[pieceidx], col, row, orient);
        image.paint(gfx, x, y);
@@ -415,9 +401,8 @@ public class DropSprite extends Sprite
        int nx = _srcPos.x + (int)((_destPos.x - _srcPos.x) * pctdone);
        int ny = _srcPos.y + (int)((_destPos.y - _srcPos.y) * pctdone);
-//         Log.info("Drop sprite tick [dist=" + _dist + ", pctdone=" + pctdone +
+//        log.info("Drop sprite tick",
-//                  ", row=" + _row + ", col=" + _col +
+//            "dist", _dist, "pctdone", pctdone, "row", _row, "col", _col, "nx", nx, "ny", ny);
 //                  ", nx=" + nx + ", ny=" + ny + "].");
        // only update the sprite's location if it actually moved
        if (_ox != nx || _oy != ny) {
@@ -431,8 +416,8 @@ public class DropSprite extends Sprite
    }
    /**
-     * Called when the sprite has finished traversing its current row to
+     * Called when the sprite has finished traversing its current row to advance its board
-     * advance its board coordinates to the next row.
+     * coordinates to the next row.
     */
    protected void advancePosition ()
    {
@@ -480,21 +465,18 @@ public class DropSprite extends Sprite
    }
    /**
-     * Updates the bounds for this sprite based on the sprite display
+     * Updates the bounds for this sprite based on the sprite display dimensions in the view.
     * dimensions in the view.
     */
    protected void updateBounds ()
    {
-        Dimension size = _view.getPieceSegmentSize(
+        Dimension size = _view.getPieceSegmentSize(_col, _row, _orient, _pieces.length);
            _col, _row, _orient, _pieces.length);
        _bounds.width = size.width;
        _bounds.height = size.height;
    }
    /**
-     * Adjusts our render origin such that our location is not in the
+     * Adjusts our render origin such that our location is not in the upper left of the sprite's
-     * upper left of the sprite's rendered image but is in fact offset by
+     * rendered image but is in fact offset by some number of rows and columns.
     * some number of rows and columns.
     */
    protected void updateRenderOffset ()
    {
@@ -505,19 +487,16 @@ public class DropSprite extends Sprite
    /** Used to dispatch {@link DropSpriteObserver#pieceMoved}. */
    protected static class PieceMovedOp implements ObserverList.ObserverOp<Object>
    {
-        public PieceMovedOp (DropSprite sprite, long when, int col, int row)
+        public PieceMovedOp (DropSprite sprite, long when, int col, int row) {
        {
            _sprite = sprite;
            _when = when;
            _col = col;
            _row = row;
        }
-        public boolean apply (Object observer)
+        public boolean apply (Object observer) {
        {
            if (observer instanceof DropSpriteObserver) {
-                ((DropSpriteObserver)observer).pieceMoved(
+                ((DropSpriteObserver)observer).pieceMoved(_sprite, _when, _col, _row);
                    _sprite, _when, _col, _row);
            }
            return true;
        }
@@ -545,16 +524,14 @@ public class DropSprite extends Sprite
    /** The unit distance the sprite moves to reach the next row. */
    protected int _unit;
-    /** The screen coordinates of the top-left of the row currently
+    /** The screen coordinates of the top-left of the row currently occupied by the sprite. */
     * occupied by the sprite. */
    protected Point _srcPos = new Point();
-    /** The screen coordinates of the top-left of the row toward which the
+    /** The screen coordinates of the top-left of the row toward which the sprite is falling. */
     * sprite is falling. */
    protected Point _destPos = new Point();
-    /** The piece render position; used as working data when determining
+    /** The piece render position; used as working data when determining where to render each piece
-     * where to render each piece in the sprite. */
+     * in the sprite. */
    protected Point _renderPos = new Point();
    /** The number of rows remaining to drop. */
@@ -572,8 +549,7 @@ public class DropSprite extends Sprite
    /** The pieces this sprite is displaying. */
    protected int[] _pieces;
-    /** Indicates that the drop sprite is bouncing; see {@link
+    /** Indicates that the drop sprite is bouncing; see {@link #setBouncing}. */
     * #setBouncing}. */
    protected boolean _bouncing;
    // used to compute the column and row increment while rendering the
@@ -27,8 +27,7 @@ package com.threerings.puzzle.drop.client;
 public interface DropSpriteObserver
 {
    /**
-     * Called when the drop sprite has moved completely to the specified
+     * Called when the drop sprite has moved completely to the specified board coordinates.
     * board coordinates.
     */
    public void pieceMoved (DropSprite sprite, long when, int col, int row);
 }
@@ -32,8 +32,7 @@ import com.threerings.util.DirectionCodes;
 import com.threerings.media.image.Mirage;
 /**
- * The next block view displays an image representing the next drop block
+ * The next block view displays an image representing the next drop block to appear in the game.
 * to appear in the game.
 */
 public class NextBlockView extends JComponent
    implements DirectionCodes
@@ -74,8 +73,8 @@ public class NextBlockView extends JComponent
            int xpos = (_orient == VERTICAL) ? 0 : (size.width - _pwid);
            int ypos = (_orient == VERTICAL) ? (size.height - _phei) : 0;
-            for (int ii = 0; ii < _pieces.length; ii++) {
+            for (int _piece : _pieces) {
-                Mirage image = _view.getPieceImage(_pieces[ii]);
+                Mirage image = _view.getPieceImage(_piece);
                image.paint(gfx, xpos, ypos);
                if (_orient == VERTICAL) {
                    ypos -= _phei;
@@ -103,7 +102,6 @@ public class NextBlockView extends JComponent
    /** The piece dimensions in pixels. */
    protected int _pwid, _phei;
-    /** The view orientation; one of {@link #HORIZONTAL} or {@link
+    /** The view orientation; one of {@link #HORIZONTAL} or {@link #VERTICAL}. */
     * #VERTICAL}. */
    protected int _orient;
 }
@@ -34,8 +34,8 @@ import com.threerings.media.util.Path;
 import com.threerings.puzzle.drop.data.DropBoard;
 /**
- * Animates all the pieces on a puzzle board doing some sort of global
+ * Animates all the pieces on a puzzle board doing some sort of global effect like all flying into
- * effect like all flying into place or out into the ether.
+ * place or out into the ether.
 */
 public abstract class PieceGroupAnimation extends Animation
    implements PathObserver
@@ -80,8 +80,7 @@ public abstract class PieceGroupAnimation extends Animation
    {
        super.willStart(tickStamp);
-        // create an image sprite for every piece on the board and set
+        // create an image sprite for every piece on the board and set them on their paths
        // them on their paths
        int width = _board.getWidth(), height = _board.getHeight();
        _sprites = new Sprite[width * height];
        for (int yy = 0; yy < height; yy++) {
@@ -98,9 +97,8 @@ public abstract class PieceGroupAnimation extends Animation
    }
    /**
-     * An animation must override this method to configure each sprite
+     * An animation must override this method to configure each sprite with a path, potentially a
-     * with a path, potentially a render order, and whatever other
+     * render order, and whatever other configurations are needed.
     * configurations are needed.
     */
    protected abstract void configureSprite (Sprite sprite, int xx, int yy);
@@ -257,8 +257,7 @@ public class DropBoard extends Board
    {
        int px = cols[0], py = rows[0];
-//         Log.info("Starting rotation [px=" + px + ", py=" + py +
+//         log.info("Starting rotation", "px", px, "py", py, "orient", orient, "pctdone", pctdone);
 //                  ", orient=" + orient + ", pctdone=" + pctdone + "].");
        // try rotating the block in the given direction through all four possible orientations
        for (int ii = 0; ii < 4; ii++) {
@@ -285,16 +284,12 @@ public class DropBoard extends Board
            }
            // try each of three coercions: nothing, one left, one right
-            for (int c = 0; c < COERCE_DX.length; c++) {
+            for (int cx : COERCE_DX) {
                int cx = COERCE_DX[c];
                // check if our hypothetical new coordinates are empty
                if (isBlockEmpty(ox + cx, oy, ORIENT_WIDTHS[oidx], ORIENT_HEIGHTS[oidx])) {
-//                     Log.info(
+//                    log.info("Block is empty", "ox", ox + "cx", cx, "oy", oy, "oidx", oidx,
-//                         "Block is empty [ox=" + ox + ", cx=" + cx +
+//                        "orient", DirectionUtil.toShortString(orient), "owid", ORIENT_WIDTHS[oidx],
-//                         ", oy=" + oy + ", oidx=" + oidx +
+//                        "ohei", ORIENT_HEIGHTS[oidx]);
 //                         ", orient=" + DirectionUtil.toShortString(orient) +
 //                         ", owid=" + ORIENT_WIDTHS[oidx] +
 //                         ", ohei=" + ORIENT_HEIGHTS[oidx] + "].");
                    return new int[] { orient, px + cx, py, 0 };
                }
            }
@@ -304,13 +299,10 @@ public class DropBoard extends Board
            if (canPopup && rtype == RADIAL_ROTATION && orient == SOUTH) {
                // check if our hypothetical new coordinates are empty
                if (isBlockEmpty(ox, oy - 1, ORIENT_WIDTHS[oidx], ORIENT_HEIGHTS[oidx])) {
-//                     Log.info(
+//                    log.info("Popped-up block is empty",
-//                         "Popped-up block is empty [ox=" + ox +
+//                        "ox", ox, "oy", (oy - 1), "oidx", oidx,
-//                         ", oy=" + (oy - 1) + ", oidx=" + oidx +
+ //                       "orient", DirectionUtil.toShortString(orient), "owid", ORIENT_WIDTHS[oidx],
-//                         ", orient=" + DirectionUtil.toShortString(orient) +
+  //                      "ohei", ORIENT_HEIGHTS[oidx], "bhei", _bhei);
 //                         ", owid=" + ORIENT_WIDTHS[oidx] +
 //                         ", ohei=" + ORIENT_HEIGHTS[oidx] +
 //                         ", bhei=" + _bhei + "].");
                    return new int[] { orient, px, py - 1, 1 };
                }
            }
@@ -325,8 +317,9 @@ public class DropBoard extends Board
    /**
     * Returns a {@link Point} object containing the coordinates to place the bottom-left of the
-     * given block at after moving it the given distance on the x- and y-axes, or <code>null</code>
+     * given block at after moving it the given distance on the x- and y-axes, or
-     * if the move is not valid.  Note that only the final block position is checked.
+     * <code>null</code> if the move is not valid. Note that only the final block position is
     * checked.
     *
     * @param col the leftmost column of the block.
     * @param row the bottommost row of the block.
@@ -424,8 +417,8 @@ public class DropBoard extends Board
            return true;
        } else {
-            log.warning("Attempt to set piece outside board bounds " +
+            log.warning("Attempt to set piece outside board bounds",
-                        "[col=" + col + ", row=" + row + ", p=" + piece + "].");
+                "col", col, "row", row, "p", piece);
            return false;
        }
    }
@@ -451,8 +444,8 @@ public class DropBoard extends Board
     * @param len the length of the segment in pieces.
     * @param piece the piece to set in the segment.
     *
-     * @return false if the segment was only partially applied because some pieces were outside the
+     * @return false if the segment was only partially applied because some pieces were outside
-     * bounds of the board, true if it was completely applied.
+     * the bounds of the board, true if it was completely applied.
     */
    public boolean setSegment (int dir, int col, int row, int len, int piece)
    {
@@ -492,8 +485,8 @@ public class DropBoard extends Board
    }
    /**
-     * Applies a specified {@link PieceOperation} to all pieces in a row or column segment starting
+     * Applies a specified {@link PieceOperation} to all pieces in a row or column segment
-     * at the specified coordinates and of the specified length in the board.
+     * starting at the specified coordinates and of the specified length in the board.
     *
     * @param dir the direction to iterate in; one of {@link #HORIZONTAL} or {@link #VERTICAL}.
     * @param col the starting leftmost column of the segment.
@@ -685,8 +678,8 @@ public class DropBoard extends Board
    {
        int size = (_bwid*_bhei);
        if (board.length < size) {
-            log.warning("Attempt to set board with invalid data size " +
+            log.warning("Attempt to set board with invalid data size",
-                        "[len=" + board.length + ", expected=" + size + "].");
+                "len", board.length, "expected", size);
            return;
        }
@@ -724,16 +717,14 @@ public class DropBoard extends Board
        /**
         * Sets the array of pieces to be placed in the board segment.
         */
-        public void init (int dir, int[] pieces)
+        public void init (int dir, int[] pieces) {
        {
            _dir = dir;
            _pieces = pieces;
            _idx = (dir == HORIZONTAL) ? _pieces.length - 1 : 0;
        }
        // documentation inherited
-        public boolean execute (DropBoard board, int col, int row)
+        public boolean execute (DropBoard board, int col, int row) {
        {
            if (_dir == HORIZONTAL) {
                board.setPiece(col, row, _pieces[_idx--]);
            } else {
@@ -758,8 +749,7 @@ public class DropBoard extends Board
        /**
         * Sets the piece to be placed in the board segment.
         */
-        public void init (int piece)
+        public void init (int piece) {
        {
            _piece = piece;
            _error = false;
        }
@@ -768,14 +758,12 @@ public class DropBoard extends Board
         * Returns true if we attempted to set a piece outside the bounds of the board during the
         * course of our operation.
         */
-        public boolean getError ()
+        public boolean getError () {
        {
            return _error;
        }
        // documentation inherited
-        public boolean execute (DropBoard board, int col, int row)
+        public boolean execute (DropBoard board, int col, int row) {
        {
            if (!board.setPiece(col, row, _piece)) {
                _error = true;
            }
@@ -41,8 +41,8 @@ public class DropBoardSummary extends BoardSummary
    }
    /**
-     * Constructs a drop board summary that retrieves board information
+     * Constructs a drop board summary that retrieves board information from the supplied board
-     * from the supplied board when summarizing.
+     * when summarizing.
     */
    public DropBoardSummary (Board board)
    {
@@ -50,8 +50,8 @@ public class DropBoardSummary extends BoardSummary
    }
    /**
-     * Returns the column number of the column within the given column
+     * Returns the column number of the column within the given column range that contains the
-     * range that contains the most pieces.
+     * most pieces.
     */
    public int getHighestColumn (int startx, int endx)
    {
@@ -22,8 +22,7 @@
 package com.threerings.puzzle.drop.data;
 /**
- * Provides access to the configuration information for a drop puzzle
+ * Provides access to the configuration information for a drop puzzle game.
 * game.
 */
 public interface DropConfig
 {
@@ -22,20 +22,18 @@
 package com.threerings.puzzle.drop.data;
 /**
- * Describes the features and configuration desired for a given drop
+ * Describes the features and configuration desired for a given drop puzzle game.
 * puzzle game.
 */
 public interface DropLogic
 {
    /**
-     * Returns whether the puzzle game would like to make use of the
+     * Returns whether the puzzle game would like to make use of the manipulable block dropping
-     * manipulable block dropping functionality.
+     * functionality.
     */
    public boolean useBlockDropping ();
    /**
-     * Returns whether the puzzle game would like to make use of the
+     * Returns whether the puzzle game would like to make use of the rising board functionality.
     * rising board functionality.
     */
    public boolean useBoardRising ();
 }
@@ -24,8 +24,7 @@ package com.threerings.puzzle.drop.data;
 import com.threerings.util.DirectionCodes;
 /**
- * The drop piece codes interface contains constants common to the drop
+ * The drop piece codes interface contains constants common to the drop game package.
 * game package.
 */
 public interface DropPieceCodes extends DirectionCodes
 {
@@ -39,25 +39,21 @@ import com.threerings.puzzle.server.PuzzleManagerDelegate;
 import static com.threerings.puzzle.Log.log;
 /**
- * Provides the necessary support for a puzzle game that involves a
+ * Provides the necessary support for a puzzle game that involves a two-dimensional board
- * two-dimensional board containing pieces, with new pieces either falling
+ * containing pieces, with new pieces either falling into the board as a "drop block", or rising
- * into the board as a "drop block", or rising into the bottom of the
+ * into the bottom of the board in new piece rows, groups of blocks can be "broken" and garbage
- * board in new piece rows, groups of blocks can be "broken" and garbage
+ * can be sent to other players' boards as a result. This is implemented as a delegate so that the
- * can be sent to other players' boards as a result. This is implemented
+ * natural hierarchy need not be twisted to differentiate between puzzles that use piece dropping
- * as a delegate so that the natural hierarchy need not be twisted to
+ * and those that don't. Because we have need to structure our hierarchy around things like
- * differentiate between puzzles that use piece dropping and those that
+ * whether a puzzle is a duty puzzle, this becomes necessary.
 * don't. Because we have need to structure our hierarchy around things
 * like whether a puzzle is a duty puzzle, this becomes necessary.
 *
- * <p> A puzzle game using these services will then need to extend this
+ * <p> A puzzle game using these services will then need to extend this delegate, implementing the
- * delegate, implementing the necessary methods to customize it for the
+ * necessary methods to customize it for the particulars of their game and then register it with
- * particulars of their game and then register it with their game manager
+ * their game manager via {@link PlaceManager#addDelegate}.
 * via {@link PlaceManager#addDelegate}.
 *
- * <p> It also keeps track of, for each player, board level information,
+ * <p> It also keeps track of, for each player, board level information, and player game status.
- * and player game status.  Miscellaneous utility routines are provided
+ * Miscellaneous utility routines are provided for checking things like whether the game is over,
- * for checking things like whether the game is over, whether a player is
+ * whether a player is still active in the game, and so forth.
 * still active in the game, and so forth.
 *
 * <p> Derived classes are likely to want to override {@link #getPieceDropLogic}.
 */
@@ -82,9 +78,8 @@ public abstract class DropManagerDelegate extends PuzzleManagerDelegate
        _usedrop = logic.useBlockDropping();
        _userise = logic.useBoardRising();
        if (_usedrop && _userise) {
-            log.warning("Can't use dropping blocks and board rising "+
+            log.warning("Can't use dropping blocks and board rising functionality simultaneously " +
-                        "functionality simultaneously in a drop puzzle game! " +
+                        "in a drop puzzle game! Falling back to straight dropping.");
                        "Falling back to straight dropping.");
            _userise = false;
        }
    }
@@ -129,8 +124,8 @@ public abstract class DropManagerDelegate extends PuzzleManagerDelegate
    }
    /**
-     * Drops any pieces that need dropping on the given player's board and
+     * Drops any pieces that need dropping on the given player's board and returns whether any
-     * returns whether any pieces were dropped.
+     * pieces were dropped.
     */
    protected boolean dropPieces (DropBoard board)
    {
@@ -153,10 +148,9 @@ public abstract class DropManagerDelegate extends PuzzleManagerDelegate
        return new PieceDropper(logic);
    }
    /**
-     * This method should be called by derived classes whenever the player
+     * This method should be called by derived classes whenever the player successfully places a
-     * successfully places a drop block.
+     * drop block.
     */
    protected void placedBlock (int pidx)
    {
@@ -177,11 +171,11 @@ public abstract class DropManagerDelegate extends PuzzleManagerDelegate
    /** The board dimensions in pieces. */
    protected int _bwid, _bhei;
-    /** The piece dropper used to drop pieces in the board if the puzzle
+    /** The piece dropper used to drop pieces in the board if the puzzle chooses to make use of
-     * chooses to make use of piece dropping functionality. */
+     * piece dropping functionality. */
    protected PieceDropper _dropper;
-    /** Used to limit the maximum number of board update loops permitted
+    /** Used to limit the maximum number of board update loops permitted before assuming something's
-     * before assuming something's gone horribly awry and aborting. */
+     *  gone horribly awry and aborting. */
    protected static final int MAX_UPDATE_LOOPS = 100;
 }
@@ -27,12 +27,11 @@ public class DropBoardUtil
    implements DirectionCodes
 {
    /**
-     * Returns the orientation resulting from rotating the block in the
+     * Returns the orientation resulting from rotating the block in the given direction the
-     * given direction the specified number of times.
+     * specified number of times.
     *
     * @param orient the current orientation.
-     * @param dir the direction to rotate in; one of <code>CW</code> or
+     * @param dir the direction to rotate in; one of <code>CW</code> or <code>CCW</code>.
     * <code>CCW</code>.
     * @param count the number of rotations to perform.
     *
     * @return the rotated orientation.
@@ -46,12 +45,10 @@ public class DropBoardUtil
    }
    /**
-     * Returns the orientation resulting from rotating the block in
+     * Returns the orientation resulting from rotating the block in the given direction.
     * the given direction.
     *
     * @param orient the current orientation.
-     * @param dir the direction to rotate in; one of <code>CW</code> or
+     * @param dir the direction to rotate in; one of <code>CW</code> or <code>CCW</code>.
     * <code>CCW</code>.
     *
     * @return the rotated orientation.
     */
@@ -34,8 +34,7 @@ import com.threerings.puzzle.util.PuzzleGameUtil;
 public class DropGameUtil
 {
    /**
-     * Returns a key translator configured with mappings suitable for a
+     * Returns a key translator configured with mappings suitable for a drop puzzle game.
     * drop puzzle game.
     */
    public static KeyTranslatorImpl getKeyTranslator ()
    {
@@ -44,21 +43,15 @@ public class DropGameUtil
        // add all press key mappings
        xlate.addPressCommand(KeyEvent.VK_LEFT,
-                              DropControllerDelegate.MOVE_BLOCK_LEFT,
+                              DropControllerDelegate.MOVE_BLOCK_LEFT, MOVE_RATE, MOVE_DELAY);
                              MOVE_RATE, MOVE_DELAY);
        xlate.addPressCommand(KeyEvent.VK_RIGHT,
-                              DropControllerDelegate.MOVE_BLOCK_RIGHT,
+                              DropControllerDelegate.MOVE_BLOCK_RIGHT, MOVE_RATE, MOVE_DELAY);
-                              MOVE_RATE, MOVE_DELAY);
+        xlate.addPressCommand(KeyEvent.VK_UP, DropControllerDelegate.ROTATE_BLOCK_CCW, 0);
-        xlate.addPressCommand(KeyEvent.VK_UP,
+        xlate.addPressCommand(KeyEvent.VK_DOWN, DropControllerDelegate.ROTATE_BLOCK_CW, 0);
-                              DropControllerDelegate.ROTATE_BLOCK_CCW, 0);
+        xlate.addPressCommand(KeyEvent.VK_SPACE, DropControllerDelegate.START_DROP_BLOCK, 0);
        xlate.addPressCommand(KeyEvent.VK_DOWN,
                              DropControllerDelegate.ROTATE_BLOCK_CW, 0);
        xlate.addPressCommand(KeyEvent.VK_SPACE,
                              DropControllerDelegate.START_DROP_BLOCK, 0);
        // add all release key mappings
-        xlate.addReleaseCommand(KeyEvent.VK_SPACE,
+        xlate.addReleaseCommand(KeyEvent.VK_SPACE, DropControllerDelegate.END_DROP_BLOCK);
                                DropControllerDelegate.END_DROP_BLOCK);
        return xlate;
    }
@@ -37,29 +37,26 @@ public class PieceDestroyer
    implements DropPieceCodes
 {
    /**
-     * An interface to be implemented by specific puzzles to detail the
+     * An interface to be implemented by specific puzzles to detail the parameters and methodology
-     * parameters and methodology by which pieces are destroyed in the
+     * by which pieces are destroyed in the puzzle board.
     * puzzle board.
     */
    public interface DestroyLogic
    {
        /**
-         * Returns the minimum length of a contiguously piece segment that
+         * Returns the minimum length of a contiguously piece segment that should be destroyed.
         * should be destroyed.
         */
        public int getMinimumLength ();
        /**
-         * Returns whether piece <code>a</code> is equivalent to piece
+         * Returns whether piece <code>a</code> is equivalent to piece <code>b</code> for the
-         * <code>b</code> for the purposes of including it in a contiguous
+         * purposes of including it in a contiguous piece segment to be destroyed.
         * piece segment to be destroyed.
         */
        public boolean isEquivalent (int a, int b);
    }
    /**
-     * Constructs a piece destroyer that destroys pieces as specified by
+     * Constructs a piece destroyer that destroys pieces as specified by the supplied destroy
-     * the supplied destroy logic.
+     * logic.
     */
    public PieceDestroyer (DestroyLogic logic)
    {
@@ -67,13 +64,11 @@ public class PieceDestroyer
    }
    /**
-     * Destroys all pieces in the given board that are in contiguous rows
+     * Destroys all pieces in the given board that are in contiguous rows or columns of pieces,
-     * or columns of pieces, returning a list of {@link SegmentInfo}
+     * returning a list of {@link SegmentInfo} objects detailing the destroyed piece segments.
-     * objects detailing the destroyed piece segments.  Note that a single
+     * Note that a single list is used internally to gather the segment info, and so callers that
-     * list is used internally to gather the segment info, and so callers
+     * care to modify the list should create their own copy; also, the pieces in the segments may
-     * that care to modify the list should create their own copy; also,
+     * overlap, i.e., two segments may contain the same piece.
     * the pieces in the segments may overlap, i.e., two segments may
     * contain the same piece.
     */
    public List<SegmentInfo> destroyPieces (DropBoard board, PieceOperation destroyOp)
    {
@@ -108,9 +103,8 @@ public class PieceDestroyer
    }
    /**
-     * Searches for a contiguously colored piece segment with the
+     * Searches for a contiguously colored piece segment with the specified orientation and root
-     * specified orientation and root coordinates in the supplied board
+     * coordinates in the supplied board and returns the length of the segment traversed.
     * and returns the length of the segment traversed.
     */
    protected int findSegment (DropBoard board, int dir, int x, int y)
    {
@@ -124,8 +118,8 @@ public class PieceDestroyer
    }
    /**
-     * A piece operation that calculates the length of the contiguous
+     * A piece operation that calculates the length of the contiguous piece segment to which it is
-     * piece segment to which it is applied.
+     * applied.
     */
    protected class SegmentLengthOperation
        implements PieceOperation
@@ -133,22 +127,19 @@ public class PieceDestroyer
        /**
         * Resets the operation for application to a new piece segment.
         */
-        public void reset ()
+        public void reset () {
        {
            _len = 0;
        }
        /**
         * Returns the length of the contiguous piece segment.
         */
-        public int getLength ()
+        public int getLength () {
        {
            return _len;
        }
        // documentation inherited
-        public boolean execute (DropBoard board, int col, int row)
+        public boolean execute (DropBoard board, int col, int row) {
        {
            int piece = board.getPiece(col, row);
            if (_len == 0) {
                _len = 1;
@@ -26,8 +26,8 @@ import com.threerings.util.DirectionCodes;
 import com.threerings.puzzle.drop.data.DropBoard;
 /**
- * An interface to be implemented by games that would like to be able to
+ * An interface to be implemented by games that would like to be able to drop their pieces during
- * drop their pieces during game play.
+ * game play.
 */
 public interface PieceDropLogic
 {
@@ -49,28 +49,28 @@ public interface PieceDropLogic
    public boolean isConstrainedPiece (int piece);
    /**
-     * Returns whether the given piece terminates a column climb when
+     * Returns whether the given piece terminates a column climb when determining the height of a
-     * determining the height of a piece column to be dropped.
+     * piece column to be dropped.
     *
-     * @param allowConst whether to allow dropping constrained pieces
+     * @param allowConst whether to allow dropping constrained pieces (though only in the first
-     * (though only in the first encountered constrained block.)
+     * encountered constrained block.)
     * @param piece the piece to consider.
-     * @param pre whether the climbability check is being performed
+     * @param pre whether the climbability check is being performed before the height is
-     * before the height is incremented, or after.
+     * incremented, or after.
     */
    public boolean isClimbablePiece (boolean allowConst, int piece, boolean pre);
    /**
     * Returns the x-axis coordinate of the specified edge of the given constrained piece.
     *
-     * <p> TODO: This should go away once the sword and sail games have standardized on 
+     * <p> TODO: This should go away once the sword and sail games have standardized on WEST/EAST
-     * WEST/EAST or BLOCK_LEFT/BLOCK_RIGHT to reference block edges.
+     * or BLOCK_LEFT/BLOCK_RIGHT to reference block edges.
     *
     * @param board the board to search.
     * @param col the column of the constrained piece.
     * @param row the row of the constrained piece.
-     * @param dir the edge direction to find; one of {@link
+     * @param dir the edge direction to find; one of {@link DirectionCodes#LEFT} or
-     * DirectionCodes#LEFT} or {@link DirectionCodes#RIGHT}.
+     * {@link DirectionCodes#RIGHT}.
     */
    public int getConstrainedEdge (DropBoard board, int col, int row, int dir);
 }
@@ -54,16 +54,14 @@ public class PieceDropper
        /**
         * Constructs a piece drop info object.
         */
-        public PieceDropInfo (int col, int row, int dist)
+        public PieceDropInfo (int col, int row, int dist) {
        {
            this.col = col;
            this.row = row;
            this.dist = dist;
        }
        @Override
-        public String toString ()
+        public String toString () {
        {
            return StringUtil.fieldsToString(this);
        }
    }
@@ -78,8 +76,8 @@ public class PieceDropper
    }
    /**
-     * Constructs a piece dropper that uses the supplied piece drop logic
+     * Constructs a piece dropper that uses the supplied piece drop logic to specialize itself for
-     * to specialise itself for a particular puzzle.
+     * a particular puzzle.
     */
    public PieceDropper (PieceDropLogic logic)
    {
@@ -87,8 +85,8 @@ public class PieceDropper
    }
    /**
-     * Effects any drops possible on the supplied board (modifying the
+     * Effects any drops possible on the supplied board (modifying the board in the progress) and
-     * board in the progress) and notifying the supplied drop observer of those drops.
+     * notifying the supplied drop observer of those drops.
     *
     * @return the number of pieces dropped.
     */
@@ -135,8 +133,8 @@ public class PieceDropper
    }
    /**
-     * Computes and effects the drop for the specified piece and any
+     * Computes and effects the drop for the specified piece and any associated attached pieces.
-     * associated attached pieces. The supplied observer is notified of all drops.
+     * The supplied observer is notified of all drops.
     */
    protected int dropPieces (DropBoard board, int xx, int yy, DropObserver drobs)
    {
@@ -153,8 +151,8 @@ public class PieceDropper
            int end = _logic.getConstrainedEdge(board, xx, yy, RIGHT);
            int bwid = board.getWidth();
            if (start < 0 || end >= bwid) {
-                log.warning("Board reported bogus constrained edge " +
+                log.warning("Board reported bogus constrained edge",
-                            "[x=" + xx + ", y=" + yy + ", start=" + start + ", end=" + end + "].");
+                    "x", xx, "y", yy, "start", start, "end", end);
                board.dump();
                start = Math.max(start, 0);
                end = Math.min(end, bwid);
@@ -169,8 +167,7 @@ public class PieceDropper
                return 0;
            }
-            // scoot along the bottom edge of the block, noting the drop
+            // scoot along the bottom edge of the block, noting the drop for each column
            // for each column
            for (int xpos = start; xpos <= end; xpos++) {
                piece = board.getPiece(xpos, yy);
                drop(board, piece, xpos, yy, yy + dist, drobs);
@@ -191,8 +188,8 @@ public class PieceDropper
    }
    /** Helpy helper function. */
-    protected final void drop (DropBoard board, int piece,
+    protected final void drop (
-                               int xx, int yy, int ty, DropObserver drobs)
+        DropBoard board, int piece, int xx, int yy, int ty, DropObserver drobs)
    {
        // don't try to clear things out if we're filling in from off-board
        if (yy >= 0) {
@@ -57,13 +57,12 @@ public abstract class PuzzleManager extends GameManager
     */
    public BoardSummary getBoardSummary (int pidx)
    {
-        return (_puzobj == null || _puzobj.summaries == null) ? null :
+        return (_puzobj == null || _puzobj.summaries == null) ? null : _puzobj.summaries[pidx];
            _puzobj.summaries[pidx];
    }
    /**
     * Returns whether this puzzle cares to make use of per-player board summaries that are sent
-     * periodically to all users in the puzzle via {@link #sendStatusUpdate}.  The default
+     * periodically to all users in the puzzle via {@link #sendStatusUpdate}. The default
     * implementation returns <code>false</code>.
     */
    public boolean needsBoardSummaries ()
@@ -81,7 +80,7 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Handles the server and client states being out of sync when in debug mode.  The default
+     * Handles the server and client states being out of sync when in debug mode. The default
     * implementation halts the server.
     */
    protected void handleBoardNotEqual ()
@@ -104,7 +103,7 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Applies updateBoardSummary on all the players' boards.  AI board summaries should be updated
+     * Applies updateBoardSummary on all the players' boards. AI board summaries should be updated
     * by the AI logic.
     */
    public void updateBoardSummaries ()
@@ -207,9 +206,10 @@ public abstract class PuzzleManager extends GameManager
    /**
     * Returns the frequency with which puzzle status updates are broadcast to the players (which
-     * is accomplished via a call to {@link #sendStatusUpdate} which in turn calls {@link
+     * is accomplished via a call to {@link #sendStatusUpdate} which in turn calls
-     * #updateStatus} wherein derived classes can participate in the status update).  Returning
+     * {@link #updateStatus} wherein derived classes can participate in the status update).
-     * <code>O</code> (the default) indicates that a periodic status update is not desired.
+     * Returning <code>O</code> (the default) indicates that a periodic status update is not
     * desired.
     */
    protected long getStatusInterval ()
    {
@@ -246,7 +246,7 @@ public abstract class PuzzleManager extends GameManager
    {
        _puzobj.startTransaction();
        try {
-            // Log.info("Updating status [game=" + _puzobj.which() + "].");
+            // log.info("Updating status", "game", _puzobj.which());
            updateStatus();
        } finally {
            _puzobj.commitTransaction();
@@ -255,9 +255,9 @@ public abstract class PuzzleManager extends GameManager
    /**
     * A puzzle periodically (default of once every 5 seconds but configurable by puzzle) updates
-     * status information that is visible to the user. Derived classes can override this method and
+     * status information that is visible to the user. Derived classes can override this method
-     * effect their updates by generating events on the puzzle object and they will be packaged
+     * and effect their updates by generating events on the puzzle object and they will be
-     * into the update transaction.
+     * packaged into the update transaction.
     */
    protected void updateStatus ()
    {
@@ -289,8 +289,8 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Creates and initializes boards and board summaries (if desired per {@link
+     * Creates and initializes boards and board summaries (if desired per
-     * #needsBoardSummaries}) for each player.
+     * {@link #needsBoardSummaries}) for each player.
     */
    protected void initBoards ()
    {
@@ -314,8 +314,8 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Returns whether this puzzle needs a board for the given player index.  The default
+     * Returns whether this puzzle needs a board for the given player index. The default
-     * implementation only creates boards for occupied player slots.  Derived classes may wish to
+     * implementation only creates boards for occupied player slots. Derived classes may wish to
     * override this method if they have specialized board needs, e.g., they need only a single
     * board for all players.
     */
@@ -356,8 +356,8 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Applies progress updates received from the client. If puzzle debugging is enabled, this also
+     * Applies progress updates received from the client. If puzzle debugging is enabled, this
-     * compares the client board dumps provided along with each puzzle event.
+     * also compares the client board dumps provided along with each puzzle event.
     */
    protected void applyProgressEvents (int pidx, int[] gevents, Board[] states)
    {
@@ -376,8 +376,7 @@ public abstract class PuzzleManager extends GameManager
            // apply the event to the player's board
            if (!applyProgressEvent(pidx, gevent, cboard)) {
-                log.warning("Unknown event [puzzle=" + where() + ", pidx=" + pidx +
+                log.warning("Unknown event", "puzzle", where(), "pidx", pidx, "event", gevent);
                            ", event=" + gevent + "].");
            }
            // maybe we are comparing boards afterwards
@@ -393,8 +392,8 @@ public abstract class PuzzleManager extends GameManager
    protected void compareBoards (int pidx, Board boardstate, int gevent, boolean before)
    {
        if (DEBUG_PUZZLE) {
-            log.info((before ? "About to apply " : "Just applied ") + "[game=" + _puzobj.which() +
+            log.info((before ? "About to apply " : "Just applied "),
-                     ", pidx=" + pidx + ", event=" + gevent + "].");
+                "game", _puzobj.which(), "pidx", pidx, "event", gevent);
        }
        if (boardstate == null) {
            if (DEBUG_PUZZLE) {
@@ -404,8 +403,8 @@ public abstract class PuzzleManager extends GameManager
        }
        boolean equal = _boards[pidx].equals(boardstate);
        if (!equal) {
-            log.warning("Client and server board states not equal! [game=" + _puzobj.which() +
+            log.warning("Client and server board states not equal!",
-                        ", type=" + _puzobj.getClass().getName() + "].");
+                "game", _puzobj.which(), "type", _puzobj.getClass().getName());
        }
        if (DEBUG_PUZZLE) {
            // if we're debugging, dump the board state every time we're about to apply an event
@@ -424,14 +423,14 @@ public abstract class PuzzleManager extends GameManager
    /**
     * Called by {@link #updateProgress} to give the server a chance to apply each game event
     * received from the client to the respective player's server-side board and, someday, confirm
-     * their validity.  Derived classes that make use of the progress updating functionality should
+     * their validity. Derived classes that make use of the progress updating functionality should
-     * be sure to override this method to perform their game-specific event application
+     * be sure to override this method to perform their game-specific event application antics.
-     * antics. They should first perform a call to super() to see if the event is handled there.
+     * They should first perform a call to super() to see if the event is handled there.
     *
     * @param pidx the player index that submitted the progress event.
     * @param gevent the progress event itself.
-     * @param cboard a snapshot of the board on the client iff the client has board syncing enabled
+     * @param cboard a snapshot of the board on the client iff the client has board syncing
-     * (which is only enabled when debugging).
+     * enabled (which is only enabled when debugging).
     *
     * @return true to indicate that the event was handled.
     */
@@ -441,7 +440,7 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Overrides the game manager implementation to mark all active players as winners.  Derived
+     * Overrides the game manager implementation to mark all active players as winners. Derived
     * classes may wish to override this method in order to customize the winning conditions.
     */
    @Override
@@ -458,7 +457,7 @@ public abstract class PuzzleManager extends GameManager
    protected abstract Board newBoard (int pidx);
    /**
-     * Creates and returns a new board summary for the given board.  Puzzles that do not make use
+     * Creates and returns a new board summary for the given board. Puzzles that do not make use
     * of board summaries should implement this method and return <code>null</code>.
     */
    protected abstract BoardSummary newBoardSummary (Board board);
@@ -480,31 +479,30 @@ public abstract class PuzzleManager extends GameManager
        if (sessionId != _puzobj.sessionId) {
            // only warn if this isn't a straggling update from the previous session
            if (sessionId != _puzobj.sessionId-1) {
-                log.warning("Received progress update for invalid session, not applying " +
+                log.warning("Received progress update for invalid session, not applying",
-                            "[game=" + _puzobj.which() + ", invalidSessionId=" + sessionId +
+                    "game", _puzobj.which(), "invalidSessionId", sessionId,
-                            ", sessionId=" + _puzobj.sessionId + "].");
+                    "sessionId", _puzobj.sessionId);
            }
            return;
        }
        // if the game is over, we wing straggling updates
        if (!_puzobj.isInPlay()) {
-            log.debug("Ignoring straggling events", "game", _puzobj.which(), "who", caller.who(),
+            log.debug("Ignoring straggling events",
-                      "events", events);
+                "game", _puzobj.which(), "who", caller.who(), "events", events);
            return;
        }
        // determine the caller's player index in the game
        int pidx = IntListUtil.indexOf(_playerOids, caller.getOid());
        if (pidx == -1) {
-            log.warning("Received progress update for non-player?!", "game", _puzobj.which(),
+            log.warning("Received progress update for non-player?!",
-                        "who", caller.who(), "ploids", _playerOids);
+                "game", _puzobj.which(), "who", caller.who(), "ploids", _playerOids);
            return;
        }
-//         Log.info("Handling progress events [game=" + _puzobj.which() +
+//        log.info("Handling progress events", "game", _puzobj.which(),
-//                  ", pidx=" + pidx + ", sessionId=" + sessionId +
+//            "pidx", pidx + "sessionId", sessionId, "count", events.length);
 //                  ", count=" + events.length + "].");
        // note that we received a progress update from this player
        _lastProgress[pidx] = System.currentTimeMillis();
@@ -530,8 +528,8 @@ public abstract class PuzzleManager extends GameManager
    }
    /**
-     * Returns whether {@link #checkPlayerActivity} should be called periodically while the game is
+     * Returns whether {@link #checkPlayerActivity} should be called periodically while the game
-     * in play to make sure players are still active.
+     * is in play to make sure players are still active.
     */
    protected boolean checkForInactivity ()
    {
@@ -540,7 +538,7 @@ public abstract class PuzzleManager extends GameManager
    /**
     * Called periodically for each human player to give puzzles a chance to make sure all such
-     * players are engaging in reasonable levels of activity.  The default implementation does
+     * players are engaging in reasonable levels of activity. The default implementation does
     * naught.
     */
    protected void checkPlayerActivity (long tickStamp, int pidx)
@@ -26,8 +26,8 @@ import com.threerings.crowd.data.PlaceConfig;
 import com.threerings.parlor.game.server.GameManagerDelegate;
 /**
- * Extends the {@link GameManagerDelegate} mechanism with puzzle manager specific methods (of which
+ * Extends the {@link GameManagerDelegate} mechanism with puzzle manager specific methods (of
- * there are currently none).
+ * which there are currently none).
 */
 public class PuzzleManagerDelegate extends GameManagerDelegate
 {
@@ -28,14 +28,13 @@ import java.awt.Point;
 import static com.threerings.puzzle.Log.log;
 /**
- * The point set class provides an efficient implementation of a set
+ * The point set class provides an efficient implementation of a set containing two-dimensional
- * containing two-dimensional point values as <code>(x, y)</code>.
+ * point values as <code>(x, y)</code>.
 */
 public class PointSet
 {
    /**
-     * Creates a point set that can contain points within the given range
+     * Creates a point set that can contain points within the given range of values.
     * of values.
     *
     * @param rangeX the maximum x-axis range.
     * @param rangeY the maximum y-axis range.
@@ -48,8 +47,7 @@ public class PointSet
    }
    /**
-     * Adds a point to the set and returns whether the point was already
+     * Adds a point to the set and returns whether the point was already present in the set.
     * present in the set.
     *
     * @param x the point x-coordinate.
     * @param y the point y-coordinate.
@@ -122,10 +120,9 @@ public class PointSet
    }
    /**
-     * Returns an iterator that iterates over the points in this set,
+     * Returns an iterator that iterates over the points in this set, returning them as
-     * returning them as {@link Point} objects.  Note that the iterator
+     * {@link Point} objects. Note that the iterator uses a single point object internally, and so
-     * uses a single point object internally, and so callers should create
+     * callers should create their own copy of the point if they plan to do something fancy with
     * their own copy of the point if they plan to do something fancy with
     * it.
     *
     * @return the iterator over the set's points.
@@ -136,8 +133,7 @@ public class PointSet
    }
    /**
-     * Removes the given point from the set and returns whether the point
+     * Removes the given point from the set and returns whether the point was present in the set.
     * was present in the set.
     *
     * @param x the point x-coordinate.
     * @param y the point y-coordinate.