GNOME Bugzilla – Bug 779598
When playing time-bound game, wrong player wins
Last modified: 2018-05-22 12:26:54 UTC
Created attachment 347241 [details] Screenshot of the match with wrong result. I experienced on a time-bound match that at the end of the game the wrong person is said to have won it. I tried it a couple of times but cannot reproduce it again. See attached screenshot.
I'll try reproducing this, Could you attach the saved game pgn file if you still have it?
Unfortunately not. When I come across it again I'll save it.
Do you remember the clock type that was being used, or any settings from the preferences dialog?
Related bug found while trying to reproduce this issue: https://bugzilla.gnome.org/show_bug.cgi?id=779937 (Already fixed).
Looking into all possible combinations of clock types and black/white winning to try to reproduce this.
Created attachment 349046 [details] Preferences set to Simple clock
Created attachment 349047 [details] Simple clock & white wins the game
Created attachment 349048 [details] Simple clock & black wins the game
Created attachment 349049 [details] Preferences set to Fischer clock
Created attachment 349050 [details] Fischer clock & white wins the game
Created attachment 349051 [details] Fischer clock & black wins the game
Created attachment 349052 [details] Preferences set to Bronstein clock
Created attachment 349055 [details] Bronstein clock & black wins the game
Created attachment 349056 [details] Bronstein clock & white wins the game
The problem doesn't seem to be happening in the current state as it looks like from my testing. Alessandro, Can you please update to the latest release if not already there? [1] I'm resolving this for now, please reopen if you happen to hit it again and attach to the bug: - Saved game PGN file - gnome-chess -> "Preferences" - Version from gnome-chess -> "About" [1]: https://git.gnome.org/browse/gnome-chess/refs/
Hi all, I would like to reopen this bug as I found a way to consistently reproduce it after looking at your code. Specifically, the method clock_expired_cb in lib/chess-game.vala, line 314. When the clock expires, it checks who is the current player and assigns the victory to the opponent. However if the player whose clock is going to expire makes a move in the last second, the other player will be the current player when the clock expires, hence the victory will be reported incorrectly. You can reproduce this consistently by: 1. starting a time-limited game 2. letting one player's time almost expire 3. on the last second, making a move the player whose clock still has plenty of time will be marked as loser. Proposed solution: instead of checking whose turn it is when the clock expires, check which player has no time left. I hope this helps.
Thanks for your report, I'll take a look at it now.
Created attachment 350032 [details] Move on the last second consistently shows the bug
I'm wrapping up a proposal for a talk for GUADEC, I'll look at this first thing after I finish that.
Created attachment 350033 [details] [review] Should fix the bug
Hi Sahil, it wasn't my intention to put pressure, rather I wanted to document the fix :) I just attached a patch, seems to work on my machine. Best, Alessandro
Review of attachment 350033 [details] [review]: ::: lib/chess-game.vala @@ +284,3 @@ private void clock_expired_cb (ChessClock clock) { + if (clock.white_remaining_seconds == 0) Please check for less-or-equal zero. I do think that with the callbacks there are potential timing issues that would allow this to become negative at least by one. Haven't checked the other code so if there is something that already checks for this case and prevents it, please ignore.
Created attachment 350059 [details] [review] Updated patch
Review of attachment 350059 [details] [review]: ::: lib/chess-game.vala @@ +284,3 @@ private void clock_expired_cb (ChessClock clock) { + if (clock.white_remaining_seconds == 0) Please check for less-or-equal zero. I do think that with the callbacks there are potential timing issues that would allow this to become negative at least by one. Haven't checked the other code so if there is something that already checks for this case and prevents it, please ignore.
Created attachment 350072 [details] [review] Updated patch Sorry I thought I had updated it instead it was the same file as before.
Comment on attachment 350072 [details] [review] Updated patch Thanks for the patch! :) You should probably also need to take care of white_extra_seconds and black_extra_seconds which are used for Fischer and Bronstein clocks. Can you test play a game with custom clocks instead of the simple clock please?
Comment on attachment 350072 [details] [review] Updated patch Looks like Sahil left some review comments that still need to be addressed.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-chess/issues/25.