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

Add shortcut for next phase with action #3548

Merged
merged 6 commits into from
Feb 3, 2019

Conversation

basicer
Copy link
Member

@basicer basicer commented Feb 2, 2019

Short roundup of the initial problem

You have to click a lot on the phase tool bar while playing.
A shortcut that moved you to the next phase and preformed that phases default action will streamline things.

What will change with this Pull Request?

  • Adds a new action (default shit+tab) to move to the next phase and preform the phase action.
  • Preforming the action in the last phase automatically passes the turn before moving phases.

basicer and others added 2 commits February 2, 2019 11:54
Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>
Copy link
Member

@ZeldaZach ZeldaZach left a comment

Choose a reason for hiding this comment

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

Works as expected, very nice! One more review please by someone else :)

@ebbit1q
Copy link
Member

ebbit1q commented Feb 3, 2019

I'd prefer to have it do the default action (drawing a card) on pressing shift tab the second time, not immediately after switching. I guess you can switch up using tab and shift tab. Overall I think this is a great addition. All good 👍

@ebbit1q
Copy link
Member

ebbit1q commented Feb 3, 2019

I don't know why you set those all to nullptr, they did nothing in the first place.

@ZeldaZach
Copy link
Member

@ebbit1q didn't know, just saw clang-lint said to change 0->nullptr. Prob should've looked at the specs. Thanks 4 the cleanup :)

Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>
@ctrlaltca ctrlaltca merged commit 2bf444e into Cockatrice:master Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants