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 411360 - please add "undo" command for gtali
please add "undo" command for gtali
Status: RESOLVED FIXED
Product: gnome-games-superseded
Classification: Deprecated
Component: gtali
2.19.x
Other Linux
: Low enhancement
: gnome-2-22
Assigned To: Thomas Andersen
GNOME Games maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-23 20:53 UTC by Josselin Mouette
Modified: 2007-10-28 22:38 UTC
See Also:
GNOME target: ---
GNOME version: 2.19/2.20


Attachments
Adds undo button to Game menu (4.93 KB, patch)
2007-07-10 06:08 UTC, Geoff Buchan
none Details | Review
Updated undo/redo patch (11.53 KB, patch)
2007-07-14 14:11 UTC, Geoff Buchan
needs-work Details | Review
Simplified undo patch (12.17 KB, patch)
2007-10-27 01:00 UTC, Geoff Buchan
committed Details | Review
Disable Undo option when a computer is playing (1.30 KB, patch)
2007-10-28 22:05 UTC, Geoff Buchan
committed Details | Review

Description Josselin Mouette 2007-02-23 20:53:17 UTC
[ forwarded from http://bugs.debian.org/412008 ]

Please add an "Undo" command for the case where one clicks on the
wrong score slot by mistake.  Some way of showing the potential score
for each slot, perhaps by mousing over or clicking-without-releasing,
would also be nice.  Thanks for a fun game.
Comment 1 Andreas Røsdal 2007-02-25 16:06:09 UTC
Thanks for the suggestion. This is something someone can implement during the 2.19.x development cycle.
Comment 2 Geoff Buchan 2007-07-10 06:08:49 UTC
Created attachment 91529 [details] [review]
Adds undo button to Game menu

This is an initial implementation. This currently allows undo at any point, but I could see that perhaps we would only want to allow undo when the player hasn't made a roll on the current turn. This also only allows undo; would it make sense to allow a redo also ?
Comment 3 Geoff Buchan 2007-07-14 14:11:11 UTC
Created attachment 91772 [details] [review]
Updated undo/redo patch

This implementation supports both undo and redo.
Comment 4 Andreas Røsdal 2007-07-14 15:42:27 UTC
Thanks for the patch. There is currently a string and Ui announcement period, so this patch will have to be committed during the 2.21.x development cycle.
Comment 5 Andreas Røsdal 2007-09-20 15:03:46 UTC
This patch is now ready for inclusion. 

Thomas, would you like to commit it?
Comment 6 Thomas Andersen 2007-09-20 19:58:14 UTC
I have a few comments from trying it out.

1) Undo/redo remain sensitive in the menu after a new game has been started

2) roll some good dice (say 1,2,3,4,5)
   chose a score
   dice now show a new random roll (say 6,5,4,3,2)
   undo
   dice are back to 1,2,3,4,5 and score undone
   redo
   score is back, but dice now show 1,2,3,4,5

   This should come back to 6.5.4.3.2

This should be fixed before the patch is committed at least. I didn't have the time for a proper review of the code tonight. I'll look more closely at it during the weekend
Comment 7 Thomas Andersen 2007-09-25 11:34:14 UTC
I don't think that we should include redo at all. The reason to even have undo is if the user has chosen a wrong score by mistake. So just one level of undo and only if the dice have not been rolled after it. Also we want to remember the new roll so that this can't be used to cheat.

I looked at your implementation with many levels of undo/redo. It's nice work but unfortunately more than what is needed. 

You should also be sure to keep the same coding style as the code you are modifying. 

I'll probably make a new patch based on your work soon. Unless you beat me to it :)
Comment 8 Geoff Buchan 2007-10-27 01:00:39 UTC
Created attachment 97956 [details] [review]
Simplified undo patch

This is a simplified (and I hope improved) undo patch. Undo has the following characteristics:
1. Undo only refers to playing a score. You can't undo a decision to roll dice.
2. You can undo at most one human move.
3. Once the next human rolls once, you can no longer undo.
4. Any computer turns that are undone will be redone as they originally were when you play a new score.
5. The roll shown for the next human player will be restored when you play a new score. So you can't undo and redo to the same (or a different) score slot to generate a new roll for the next human player.
Comment 9 Geoff Buchan 2007-10-27 01:05:49 UTC
My last patch also doesn't display a redo option to the user. The code will still redo computer turns when appropriate, but the user can't "Redo". Instead, (s)he can just put the score back in the same category.
Comment 10 Thomas Andersen 2007-10-27 10:15:28 UTC
Perfect - and just in time to get in before 2.21.1 on monday. I'll review the patch tomorrow as I won't have the time before. But it sounds promising and your last work was good so I'm confident it'll get in. Good work Geoff!
Comment 11 Thomas Andersen 2007-10-28 15:47:09 UTC
I just tested it and it works nicely. I only have a few comments:

1) playing with computer delay turned on lets you undo while the computer is playing. This breaks the feature. I suggest we don't make the undo-option sensitive unless current player is human.

2) The comment in clist.c no longer describe the behavior.

3) Indentation. You use 4 spaces rather than 2 for the functions you add. We should use consistent indentation in the code.

All in all good work. I'll commit your patch with fixes for issues 2 and 3. Number 1 I'm leaving to you :) If you can find the time to fix it before the 2.21.1 release tomorrow night it would be great - if not we'll fix it for 2.21.2 instead.

And btw thanks for the various coding style fixes you included too. Keep up the good work :)
Comment 12 Thomas Andersen 2007-10-28 16:50:45 UTC
Committed. I'm keeping this bug open undtil the last issue has been fixed as well.
Comment 13 Geoff Buchan 2007-10-28 22:05:05 UTC
Created attachment 98049 [details] [review]
Disable Undo option when a computer is playing

This patch turns off the Undo option when a computer is playing.
Comment 14 Thomas Andersen 2007-10-28 22:38:19 UTC
Nice :) committed.