Skip to content

Added shift+M without having to press space first (Issue #49)#53

Open
MimmyJau wants to merge 1 commit intompereira:masterfrom
MimmyJau:feat-spaceless-shift-M
Open

Added shift+M without having to press space first (Issue #49)#53
MimmyJau wants to merge 1 commit intompereira:masterfrom
MimmyJau:feat-spaceless-shift-M

Conversation

@MimmyJau
Copy link
Contributor

Can now <Shift+M> without having to press space first. I was trying to prioritize adding as little code as possible, so solution might not be the cleanest. I find the feature useful when I play personally.

Very open to any and all feedback. Addresses Issue #49.

@mpereira
Copy link
Owner

Hi @MimmyJau,

Thanks for this feature, I think it makes a lot of sense!

As you mentioned, the current implementation for it might not be in its simplest form. The existing game implementation itself doesn't help, since it expected movements to start by marking cards with the spacebar.

I'll try to think about a cleaner way to implement this during this weekend. It might require refactoring the existing keyboard handling a bit to better accommodate card movements not starting with the spacebar. If you have ideas feel free to share them too.

In addition to that, when we get Shift-m selecting all cards in a stack, we should also get Shift-n unselecting all cards in a stack, and possibly even get m and n selecting/unselecting all the way to the first/last card as well. It doesn't have to be in the context of this PR though!

@MimmyJau
Copy link
Contributor Author

Thanks @mpereira, I'll spend some time thinking about it too. If you come up with something and want some help implementing it, let me know.

And agree on having an unselect all cards stack. As I was playing the game with this patch, I realized Shift-m could also be a useful unselect all cards button (can give it that functionality if all cards are selected / marked when it's pressed).

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.

2 participants