GNOME Bugzilla – Bug 701578
Timer for chess does not stop in certain situations
Last modified: 2013-10-09 13:42:18 UTC
User can notice that the when playing a timed Chess game, the timer does not stop in certain key events : pressing new game does not stop the counter when the "Save this game before..." is present on screen Going into settings does not pause the game so user cannot make any modifications to the setup If player lost a game, the computer timer continues to count, upon reaching zero the dialogue chances from "check mate" to "opponent has run out of time" Steps : 1. Boot 2. Open Chess 3. Go to settings - preferences and select Game Duration : One minute 4. Click close and start a new game 5. Press on Game - new game notice timer does not stop 6. Go to settings - preferences notice game does not stop 7. Lose the game in less than 60 seconds, when the "check mate" dialogue is open wait 60 seconds for the computer timer to run out and observe that it changes from from "check mate" to "opponent has run out of time". Timer never pauses
(In reply to comment #0) > User can notice that the when playing a timed Chess game, the timer does not > stop in certain key events : > > pressing new game does not stop the counter when the "Save this game > before..." is present on screen > Going into settings does not pause the game so user cannot make any > modifications to the setup Indeed. Opening any dialog ought to pause the game. (It'd also be good to have a "pause game" button which stops the timers and hides the board until the game is resumed.) > If player lost a game, the computer timer continues to count, upon reaching > zero the dialogue chances from "check mate" to "opponent has run out of time" This one should already be fixed (it'll show up in 3.8.3).
+1 for the "pause game" button Michael suggested.
Created attachment 249181 [details] [review] Proposed patch for pause button functionality I've proposed a patch for the pause game button functionality
Review of attachment 249181 [details] [review]: Thanks for the patch! It looks mostly good; a few minor issues need cleaned up though: * The indentation is inconsistent; it should be four spaces with no tabs, braces on new lines and on the outside * Can you make pausing the game desensitize the move selection combo box at the bottom of the screen? Clicking on that currently unpauses the game unexpectedly. * stash_button_sesnsitivity () is misspelled * Trivial: I'd prefer you make a little private struct to hold the sensitivities of the buttons, or else use an enum for indexing. Rather than just manually associating e.g. "first_move_button" with index 3. * In chess_clock_toggle_paused you need to make sure the ChessClock is not null (as it will be in untimed games) before using it. That's causing critical warnings. Optional: For bonus points, shroud the pieces when paused, just like GNOME Mines.
Created attachment 250534 [details] [review] Patch with requested changes
(In reply to comment #4) > Review of attachment 249181 [details] [review]: > > Thanks for the patch! It looks mostly good; a few minor issues need cleaned up > though: > > * The indentation is inconsistent; it should be four spaces with no tabs, > braces on new lines and on the outside > * Can you make pausing the game desensitize the move selection combo box at the > bottom of the screen? Clicking on that currently unpauses the game > unexpectedly. > * stash_button_sesnsitivity () is misspelled > * Trivial: I'd prefer you make a little private struct to hold the > sensitivities of the buttons, or else use an enum for indexing. Rather than > just manually associating e.g. "first_move_button" with index 3. > * In chess_clock_toggle_paused you need to make sure the ChessClock is not null > (as it will be in untimed games) before using it. That's causing critical > warnings. > > Optional: For bonus points, shroud the pieces when paused, just like GNOME > Mines. Thanks for the review! I've re-uploaded the patch with requested changes.
Created attachment 250567 [details] [review] Same patch, fixing whitespace errors This is the same as your patch, but with trailing whitespace and hard tabs removed.
Created attachment 250577 [details] [review] pause_button should always be a Gtk.ToolButton
Created attachment 250578 [details] [review] Flip pause/fullscreen buttons, remove pause label
Created attachment 250579 [details] [review] Pause button should be insensitive in untimed games
Created attachment 250580 [details] [review] Don't change UI when focus lost Just stop counting down
Review of attachment 250580 [details] [review]: FYI this last patch isn't quite ready yet (probably I shouldn't have uploaded it at all) since losing focus when paused leads to a double-pause and get everything stuck insensitive. (An easy fix.) I've also seen a double-unpause, though I'm not sure how I managed that....
Created attachment 250622 [details] [review] Don't change UI when focus lost Just stop counting down
Created attachment 250640 [details] [review] Make ChessClock class harder to misuse This class has never really been safe. Pay off some technical debt.
Plamena, do you want to check out this patch series and see if everything is working as you expect? Thanks for your help; this should be ready to go into 3.9.90.
Thank you for your review and your additional patches! I've tested them and everything works as it should, so I think they are good to land.
Excellent. This fix (and the new pause feature) will show up in v3.9.90. Thanks again for the patch! Attachment 250577 [details] pushed as c6154e8 - pause_button should always be a Gtk.ToolButton Attachment 250578 [details] pushed as 46c62fa - Flip pause/fullscreen buttons, remove pause label Attachment 250579 [details] pushed as 882e93d - Pause button should be insensitive in untimed games Attachment 250622 [details] pushed as b30e47a - Don't change UI when focus lost Attachment 250640 [details] pushed as 8ee7af6 - Make ChessClock class harder to misuse
If it matters, it would help to also backport them to gnome-3-8 branch too But I fear they cant be applied without adaptation...
Adding a new feature (pause) is not allowed by GNOME in stable releases, sorry.