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 675532 - Undoing causes Odd number of tiles
Undoing causes Odd number of tiles
Status: VERIFIED FIXED
Product: gnome-mahjongg
Classification: Applications
Component: general
git master
Other Linux
: Normal major
: ---
Assigned To: gnome-games-general-maint
GNOME Games maintainers
: 708947 709701 728862 731184 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-05-05 20:18 UTC by Hb
Modified: 2014-06-04 16:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screen shot with 9 tiles. (52.26 KB, image/png)
2012-05-05 20:18 UTC, Hb
  Details
ScreenshotThreeFree.jpg (150.92 KB, image/jpeg)
2012-07-16 05:25 UTC, Hb
  Details
Andreas's patch from comment 5 as file (926 bytes, patch)
2014-04-25 10:39 UTC, Hb
none Details | Review

Description Hb 2012-05-05 20:18:49 UTC
Created attachment 213526 [details]
Screen shot with 9 tiles.

After some undoing and going forward again an odd number of tiles (=9) was on the screen. It was impossible to finish the game. Layout was Confounding Cross.
Comment 1 Robert Ancell 2012-07-15 02:31:28 UTC
What versions of mahjongg is this?
Comment 2 Hb 2012-07-16 05:25:48 UTC
Created attachment 218878 [details]
ScreenshotThreeFree.jpg

Version 3.4.1 under Ubuntu 12.04.

Steps to reproduce:

1. Shuffle as long as you have a new game with three equal stones on free positions. See stones T1, T2 and T3 in ScreenShotThreeFree.jpg.

2. Pair stones T1 with T2.

3. Undo with Strg+Z

4. Pair stones T1 with T3.

5. Undo with Strg+Z.

6. Redo with Shift+Strg+Z. All three stones T1, T2 and T3 are removed with only this one step.

This fault happens in the middle of a game too. Reshuffling in this demonstration is only made to get a reproducible situation.
Comment 3 Andreas König 2013-05-01 07:38:54 UTC
I could reproduce the bug with current git://git.gnome.org/archive/gnome-games (revision GNOME_GAMES_3_6_0-31-g0f1e468)

The general case of this bug seems to be that tiles keep a record of the move number without reset. The general case can be described as

1. loop for any number of pairs: pick a pair , then undo

2. redo. All stones that have been removed during the loop are removed in this one single redo. So if there was a triple involved, the number of remaining stones becomes odd.

A fix would probably be to unset the move_number for all tiles that occupy the current move_number.
Comment 4 Andreas König 2013-05-01 08:04:50 UTC
Tentative fix:

--- a/gnome-mahjongg/src/game.vala
+++ b/gnome-mahjongg/src/game.vala
@@ -365,7 +365,7 @@ public class Game
 
         /* You lose your re-do queue when you make a move */
         foreach (var tile in tiles)
-            if (tile.move_number >= move_number)
+            if (tile.move_number >= move_number || tile.move_number == move_number-1 && tile.number != tile0.number && tile.number != tile1.number)
                 tile.move_number = 0;
 
         if (complete)
Comment 5 Andreas König 2013-05-09 05:13:52 UTC
I think I have a more convincing fix now. While the above fix seemed to work correctly, it is ugly. The following patch simple changes the order of setting and unsetting. The patch is longer but changes less:

diff --git a/gnome-mahjongg/src/game.vala b/gnome-mahjongg/src/game.vala
index ba54b7d..9e8e49c 100644
--- a/gnome-mahjongg/src/game.vala
+++ b/gnome-mahjongg/src/game.vala
@@ -353,6 +353,11 @@ public class Game
         selected_tile = null;
         set_hint (null, null);
 
+        /* You lose your re-do queue when you make a move */
+        foreach (var tile in tiles)
+            if (tile.move_number >= move_number)
+                tile.move_number = 0;
+
         tile0.visible = false;
         tile0.move_number = move_number;
         tile1.visible = false;
@@ -363,11 +368,6 @@ public class Game
         redraw_tile (tile0);
         redraw_tile (tile1);
 
-        /* You lose your re-do queue when you make a move */
-        foreach (var tile in tiles)
-            if (tile.move_number >= move_number)
-                tile.move_number = 0;
-
         if (complete)
             stop_clock ();
         else
Comment 6 Michael Catanzaro 2013-09-06 21:00:00 UTC
Thanks for the fix, Andreas -- it works great and seems correct.  If you submit another patch in the future, go ahead and upload it as an attachment so that it shows up in Bugzilla's patch tracking system.
Comment 7 Michael Catanzaro 2013-09-28 03:20:57 UTC
*** Bug 708947 has been marked as a duplicate of this bug. ***
Comment 8 Michael Catanzaro 2013-10-13 14:06:08 UTC
*** Bug 709701 has been marked as a duplicate of this bug. ***
Comment 9 Michael Catanzaro 2014-04-24 21:02:22 UTC
*** Bug 728862 has been marked as a duplicate of this bug. ***
Comment 10 Hb 2014-04-25 10:39:03 UTC
Created attachment 275119 [details] [review]
Andreas's patch from comment 5 as file

(In reply to comment #6)

Is this bug really FIXED? I can't find a commit, see https://git.gnome.org/browse/gnome-mahjongg/log/?qt=grep&q=675532
Comment 11 Michael Catanzaro 2014-04-25 13:11:43 UTC
Yes, it's fixed in 3.8.1 and all newer stable versions by https://git.gnome.org/browse/gnome-mahjongg/commit/src?id=31bf4f2187c7f981713a48303e4c8cdf5f7ecca2

Sorry that the bug number didn't make it into the commit.
Comment 12 Hb 2014-04-25 22:26:49 UTC
Works in Ubuntu 14.04 now.
Comment 13 Michael Catanzaro 2014-06-04 16:17:50 UTC
*** Bug 731184 has been marked as a duplicate of this bug. ***