Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow user to undo single step #17

Merged
merged 12 commits into from
Dec 23, 2017
Merged

Allow user to undo single step #17

merged 12 commits into from
Dec 23, 2017

Conversation

tuankiet65
Copy link
Contributor

@tuankiet65 tuankiet65 commented Dec 20, 2017

This pull request adds a button to allow user to undo a single step.

The button is initially disabled after a new game. After a move is made the button is enabled, and after user undoes a move, the button is disabled again. User can undo even after encountered a Game Over.

This pull request implements #9.

This commit adds fPreviousBoard, which saves the board state before making a move,
to allow for undoing a move via calling Game::undoMove()
User can undo by pressing 'u' on the terminal
The new name is more appropriate as H2048_BOARD_CHANGED covers both move made and undos.
How it works:
* canUndo is included with every H2048_BOARD_CHANGED messages
* GameBoard::boardChanged is modified to include the canUndo passed from the H2048_BOARD_CHANGED messages
* The GUI can now know whether to enable/disable the undo button
Copy link
Member

@humdingerb humdingerb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the "u" key for "Undo" in the WindowBoard, too?

(Not sure how the commandline version is supposed to work.)

Game.cpp Outdated
@@ -8,6 +8,7 @@

#include <algorithm>
#include <cstdlib>
#include <cstring>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I intended to use std::memcpy to copy the tables, but then I switched to a for-loop and I forgot to remove that header. My mistakes.

WindowBoard.cpp Outdated
fSending = true;
}

void
WindowBoard::gameEnded()
{
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray tab

@scottmc
Copy link
Member

scottmc commented Dec 20, 2017

Instead of adding an undo button, I would add it as an option on a menu of the menubar. If you put an undo button in front of the user, they are more likely to use it more often to cheat. But I'll let @humdingerb make that call.

@humdingerb
Copy link
Member

Instead of adding an undo button, I would add it as an option on a menu of the menubar.

How about making that another GCI task?
If we go for that, we should clean up the look in general. Put the score in the window title and move the "New game" and "Undo last move" into the menu (together with a "Quit" and "About" item).

@tuankiet65
Copy link
Contributor Author

tuankiet65 commented Dec 20, 2017

(Not sure how the commandline version is supposed to work.)

I've added a 'u' key for undo in the command-line version, not the GUI though. I'll work on it tomorrow since it's already midnight in my place.

@humdingerb
Copy link
Member

I've added a 'u' key for undo in the command-line version, not the GUI though. I'll work on it tomorrow since it's already midnight in my place.

Can you add the shortcut to the GUI, too?
As you play the game with the keyboard, having a shortcut is vital, or you have to hassle with grabbing the mouse and point and click on the button.

@tuankiet65
Copy link
Contributor Author

I've added it in 2042969.

@humdingerb
Copy link
Member

Works, thanks!
I'll wait with the merge if some better-than-me-coder has some input.

@scottmc scottmc merged commit 94a7d7f into HaikuArchives:master Dec 23, 2017
@humdingerb
Copy link
Member

Undoing a step doesn't decrease the score accordingly. Sorry I didn't notice sooner...
Can you try to fix that, @tuankiet65 ?

@tuankiet65
Copy link
Contributor Author

tuankiet65 commented Dec 23, 2017

Hmm that's weird, it works on my side (at least on my branch). I can't test master because it doesn't compile currently (#18, last comment)

@humdingerb
Copy link
Member

You right. It does work... Probably didn't relize those were "non-scoring" moves that - of course - don't change the score when reverted. Sorry... :)

@tuankiet65
Copy link
Contributor Author

No problem :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants