After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705002 - Loading completed games is broken
Loading completed games is broken
Status: RESOLVED FIXED
Product: gnome-chess
Classification: Applications
Component: General
3.9.x
Other Linux
: High major
: ---
Assigned To: Sahil Sareen
gnome-chess-maint
: 742020 (view as bug list)
Depends on:
Blocks: 726466
 
 
Reported: 2013-07-27 18:53 UTC by Michael Catanzaro
Modified: 2014-12-28 17:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Completed game (540 bytes, application/octet-stream)
2013-10-13 16:58 UTC, Michael Catanzaro
  Details
Load completed saved games properly (2.20 KB, patch)
2014-12-28 13:47 UTC, Sahil Sareen
reviewed Details | Review
Load completed saved games properly (1.85 KB, patch)
2014-12-28 15:30 UTC, Sahil Sareen
none Details | Review
Load completed saved games properly (1.88 KB, patch)
2014-12-28 16:12 UTC, Sahil Sareen
committed Details | Review

Description Michael Catanzaro 2013-07-27 18:53:21 UTC
GNOME Chess shouldn't allow continuing a game if a Result is specified in the PGN.  Rather, the infobar should indicate the winner.  The purpose of loading a completed game would be to go through the history.
Comment 1 Michael Catanzaro 2013-10-13 16:58:56 UTC
Created attachment 257180 [details]
Completed game

This game is completed, but no indication of this is given.  The white player can continue to move and undo, but black will never play.

There's an error in the pgn parser: ** (glchess:3108): CRITICAL **: chess_game_remove_hold: assertion '_tmp0_ > 0' failed
Comment 2 Michael Catanzaro 2013-10-13 17:02:20 UTC
(In reply to comment #1)
> This game is completed, but no indication of this is given.  The white player
> can continue to move and undo, but black will never play.

Answering my own complaint:

1) the AI will only play if indicated in the pgn; that's a GNOME extension and it's working as intended.
2) the game is not completed; the Black Queen can move.
Comment 3 Michael Catanzaro 2014-12-27 15:57:04 UTC
*** Bug 742020 has been marked as a duplicate of this bug. ***
Comment 4 Sahil Sareen 2014-12-27 18:42:08 UTC
Oops! I missed this BUG.
I'll take it up!
Comment 5 Sahil Sareen 2014-12-28 13:47:47 UTC
Created attachment 293399 [details] [review]
Load completed saved games properly

Fixes the game_start() function to take care when loading completed games:
1. Update game.result from pgn-result.
2. If loaded a completed game - Display winner and stop timer.
Comment 6 Michael Catanzaro 2014-12-28 14:22:11 UTC
Review of attachment 293399 [details] [review]:

Thanks. Just a few trivial comments:

::: src/gnome-chess.vala
@@ -470,3 @@
                 var white_seconds = int.parse (pgn_game.white_time_left);
                 var black_seconds = int.parse (pgn_game.black_time_left);
-

No need to remove this line. I guess this snuck in by accident.

@@ +573,3 @@
         game.start ();
+
+	if (pgn_game.result != PGNGame.RESULT_IN_PROGRESS)

The outer conditional here isn't doing anything and can be removed.

@@ +615,3 @@
+	if (game.result != ChessResult.IN_PROGRESS)
+	{
+	     game.stop (game.result, game.rule);

You accidentally indented five spaces here, instead of two.
Comment 7 Michael Catanzaro 2014-12-28 14:25:13 UTC
Most importantly, this means I get to delete the PGN that's been sitting at the top of my home directory for over a year, as a reminder to fix this bug. :)
Comment 8 Sahil Sareen 2014-12-28 15:30:04 UTC
Created attachment 293400 [details] [review]
Load completed saved games properly

Thanks for the quick review Michael!
Fixed your review comments.

Note: I'll address the timer problem in BUG741966, which I intend to fix soon.
Comment 9 Sahil Sareen 2014-12-28 15:35:18 UTC
I missed removing the outer conditional, adding a patch with that shortly.
Comment 10 Sahil Sareen 2014-12-28 16:12:36 UTC
Created attachment 293401 [details] [review]
Load completed saved games properly

Fixed all review comments.
PS: Whitespace convention for indentation in this file is 4.
Comment 11 Michael Catanzaro 2014-12-28 17:08:35 UTC
Attachment 293401 [details] pushed as 4a3ccef - Load completed saved games properly