GNOME Bugzilla – Bug 702157
Trying to start a new chess game after using resign functionality has inconsistent behavior
Last modified: 2013-10-30 10:57:07 UTC
User can notice that trying to start a new game after resigning, he will get a confirmation dialogue asking him to save/abandon the game. The game is over since player resigned there is no need to save/abandon the game. Steps : 1. Boot System 2. Open Chess 3. Play a little 4. Click Game - Resign 5. Try to start a new game 6. Observe the "Save this game" pop-up Expected outcome Since game is over no save needed Actual outcome Save this game dialogue box appears after the game ended
Created attachment 247170 [details] Screenshot showing the save game prompt after resigning a game
Created attachment 247172 [details] [review] Don't prompt to abandon game after player resigns This prevents the abandon/save game dialog after the user has resigned from a game, as this doesn't make contextual sense since the game is over.
This probably makes sense. You might want to save the completed game after finishing it, if you save records of your games, but you can do this yourself if you want to -- I don't see the need to prompt the user here. (And the message is confusing, anyway.)
Created attachment 247256 [details] [review] Don't prompt to abandon/save game after it has ended I didn't realise that this abandon/save game prompt was displayed after *every* end game, not just when the player resigns. With this in mind, I'd suggest that it would be more consistent to remove the prompt for all end-game scenarios. An alternative approach would be to re-word the "Abandon game" option so that it still makes contextual sense after a game has ended; that way the user still gets the prompt with the option to save a record of the game as Michael pointed out.
Created attachment 247257 [details] [review] Make new game prompt text conditional on game status As an alternative approach to the problem, this patch would mean that the user is always prompted with the option of saving a record of the game, but the 'Abandon game' label text is changed to 'New game' in the case where a game has ended (Michael, you may want to weigh-in on whether there's a better approach for tracking the status of a game, I'm not all that familiar with the codebase). I guess it's a matter of taste on what to do after a game has ended, but hopefully there'll be something between these two patches :-)
Created attachment 247563 [details] [review] Open save dialog rather than autosaving on new game prompt Saying that you want to save the game, then not getting any save prompt, is confusing and unexpected. The user should never have any choice in autosaving anyway; by definition that's something that should happen without his input.
Created attachment 247564 [details] [review] Game needs saving when the result is added E.g. if you resign, the game wants to be saved to reflect that resignation
Created attachment 247565 [details] [review] Use better text when prompting to save a completed game Otherwise it seems like you can keep playing the game
Created attachment 247566 [details] [review] Game does not need saving immediately after it is saved Moreover, we should wait until after the save is successful before making this determination.
Review of attachment 247256 [details] [review]: Glad you noticed that this had to be handled for every endgame, not just resigns. But let's adopt your other approach of just changing the text, that way it's not possible to lose data without a prompt.
Review of attachment 247257 [details] [review]: Looks like it should work, but it's better to just do a check like `game.result == ChessResult.IN_PROGRESS`
Chris, thanks for helping with this one! My patch 247565 is pretty much the same as your 247257, except it uses game.result for the check and changes a bit more of the text. The other three patches correct a few other issues I noticed with saving/autosaving. I think everything is working properly now, but if you want to, could you do a quick sanity check of these, to make sure I didn't mess anything up before I push them? You don't have to, I'd just feel dumb if you point out something wrong right after I push them. I impressively attached them to bugzilla in the reverse order, so 247566 is the first patch, 247565 is the second patch, and 247563 is the fourth and last patch.
Review of attachment 247565 [details] [review]: ::: src/gnome-chess.vala @@ +1025,3 @@ + else + { + dialog.add_button (_("_Abandon game"), Gtk.ResponseType.NO); As a matter of taste shouldn't this be "Discard game", rather than "Discard game log"? The result of pressing that button is that the whole game is discarded, of which the log is just one component.
Hi Michael, that all checks out, the patch-set looks good to me (save for that one minor bikeshed). It appears that I managed to post my review comment twice (and attached to the wrong line!) - man, this really is the Bugzilla learning zone around here :-)
Created attachment 247690 [details] [review] Use better text when prompting to save a completed game "Discard game log" and "save game log" -> "discard game" and "save game"
I think we're done here, thanks Chirs for the assistance and Ewan for the report!