Back in the wild and wooly days of Puzzle Pirates development, we determined
that we needed to be able to tell if one game ended and another one had started so we introduced GameObject.roundId which was a value that was incremented every time the game started. Thus if you had a timer that expired and you wanted to make sure that somehow the game hadn't ended and then been started anew while you were away, you could save the roundId and check it when you woke up. That was all fine and good, except for the poor choice of variable name. Then we came to implement Whirled and wanted to provide an abstraction of "rounds" which were subunits into which a single game is divided. We had this lovely variable already lying around called roundId and we succumbed to the temptation to overload its meaning and have games that use round increment the round id multiple times during a game which preserves the monotonically increasing nature of roundId as expected by the Parlor code and seemed to do no harm. Then we discovered a pesky wrinkle, which is that GameManager sets GameObject.roundId in gameWillStart() and then goes on to set GameObject.state after that. We were naturally listening for roundId to change and triggering a call to roundDidStart() at that time, but this resulted in a strange sequencing of callback methods that went: roundDidStart(), gameDidStart(), roundDidEnd(), roundDidStart(), ..., roundDidEnd(), gameDidEnd(). The premature roundDidStart() was irksome, and I looked into what would be needed to remedy it. It turned out that a lot of code on the server-side of things depended on the Parlor semantics of roundId and wanted roundId to be set to the current round's value in gameWillStart() and similarly in gameDidStart(). However, there's no gameWillStart() on the client and it didn't appear to me that anyone much cared about roundId in gameDidStart(), so I opted for some hackery in the name of expedience that preserved the server-side semantics but changed the client to see ROUND_ID change after STATE changed. As you might expect given the verbosity of this explanation, that turned out to be a new bad idea stacked on the previous bad idea of reusing roundId which was stacked on the bad choice of name for roundId. It turns out some puzzles in Yohoho did expect roundId to be already set in gameDidStart() which was no longer the case and the inevitable digital mayhem ensued. The time has come to undo the various bad decisions and replace them with new decisions that we hope aren't bad. Those decisions are: - rename roundId in GameObject to sessionId and restore its original semantics which are that it represents a monotonically increasing integer that is incremented (and published to the client) in gameWillStart() and thus represents the current game session from the very first to the very last; - add WhirledGameObject.roundId for handling rounds in Whirled games and give it the semantics we desire which are for it to be set to 1 in gameDidStart() and then incremented only when a new round is started, and reset to 1 when a new game is started (this change is in another commit). This will temporarily break some builds as I need to go rename some things in Yohoho and then I need to look at Bang! Howdy which if I recall correctly also naughtily uses roundId for nefarious ulterior purposes and needs also to be cured of its wayward habits. git-svn-id: svn+ssh://src.earth.threerings.net/vilya/trunk@611 c613c5cb-e716-0410-b11b-feb51c14d237
This commit is contained in:
@@ -508,9 +508,9 @@ public abstract class PuzzleManager extends GameManager
|
||||
protected abstract BoardSummary newBoardSummary (Board board);
|
||||
|
||||
// documentation inherited from interface PuzzleGameProvider
|
||||
public void updateProgress (ClientObject caller, int roundId, int[] events)
|
||||
public void updateProgress (ClientObject caller, int sessionId, int[] events)
|
||||
{
|
||||
updateProgressSync(caller, roundId, events, null);
|
||||
updateProgressSync(caller, sessionId, events, null);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -518,42 +518,37 @@ public abstract class PuzzleManager extends GameManager
|
||||
* checks to make sure that the progress update is valid and the
|
||||
* puzzle is still in play and then applies the updates via {@link #applyProgressEvents}.
|
||||
*/
|
||||
public void updateProgressSync (ClientObject caller, int roundId, int[] events, Board[] states)
|
||||
public void updateProgressSync (ClientObject caller, int sessionId, int[] events, Board[] states)
|
||||
{
|
||||
// bail if the progress update isn't for the current round
|
||||
if (roundId != _puzobj.roundId) {
|
||||
// only warn if this isn't a straggling update from the
|
||||
// previous round
|
||||
if (roundId != _puzobj.roundId-1) {
|
||||
log.warning("Received progress update for invalid round, " +
|
||||
"not applying [game=" + _puzobj.which() +
|
||||
", invalidRoundId=" + roundId +
|
||||
", roundId=" + _puzobj.roundId + "].");
|
||||
// bail if the progress update isn't for the current session
|
||||
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 " +
|
||||
"[game=" + _puzobj.which() + ", invalidSessionId=" + 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() +
|
||||
", events=" + StringUtil.toString(events) + "].");
|
||||
log.debug("Ignoring straggling 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() + ", who=" + caller.who() +
|
||||
", ploids=" + StringUtil.toString(_playerOids) + "].");
|
||||
log.warning("Received progress update for non-player?!", "game", _puzobj.which(),
|
||||
"who", caller.who(), "ploids", _playerOids);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
// Log.info("Handling progress events [game=" + _puzobj.which() +
|
||||
// ", pidx=" + pidx + ", roundId=" + roundId +
|
||||
// ", pidx=" + pidx + ", sessionId=" + sessionId +
|
||||
// ", count=" + events.length + "].");
|
||||
|
||||
// note that we received a progress update from this player
|
||||
|
||||
Reference in New Issue
Block a user