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

Rotate keybind #162

Merged
merged 14 commits into from
Dec 9, 2022
Merged

Rotate keybind #162

merged 14 commits into from
Dec 9, 2022

Conversation

elmodor
Copy link
Contributor

@elmodor elmodor commented Dec 3, 2022

Fixes/Solves
Fixes #113

Default key is T. Please let me know if I should change this to a different key.

Since I included mouseovers this includes/depends on #154 . Will update this once #154 is merged.

@elmodor elmodor marked this pull request as ready for review December 9, 2022 14:06
@elmodor elmodor marked this pull request as draft December 9, 2022 14:10
@GrimPixel
Copy link

GrimPixel commented Dec 9, 2022

I assume this “rotate” means yaw+. I first thought Q and E (on QWERTY layout), then I realize they might be better for the camera's rotation. For the object, if there are also keys of their movements like WSAD, then the corresponding QE are more suitable.
Maybe Caps Lock can be utilised: when it is on, WSAD and QE control the object; when it is off, WSAD and QE control the camera.

@elmodor
Copy link
Contributor Author

elmodor commented Dec 9, 2022

I assume this “rotate” means yaw+. I first thought Q and E (on QWERTY layout), then I realize they might be better for the camera's rotation. For the object, if there are also keys of their movements like WSAD, then the corresponding QE are more suitable. Maybe Caps Lock can be utilised: when it is on, WSAD and QE control the object; when it is off, WSAD and QE control the camera.

If @drwhut wants such an extensive object and camera control this should be done in a separate PR.
This implements the basic functionality to rotate an object via keybinding.

Also I don't have a capslock since no game ever uses that. But there could be a difference if shift(or capslock) is active to differentiate between camera and object - if needed.

@elmodor elmodor marked this pull request as ready for review December 9, 2022 14:24
@elmodor
Copy link
Contributor Author

elmodor commented Dec 9, 2022

This PR introduces a keybind to rotate an object. Rotation is disabled for cards in hand.

The code for the tooltips (at least the other section) needs some kind of refactoring. This is getting messy in there to figure out all possible/different scenarios and what should be displayed.

Copy link
Owner

@drwhut drwhut left a comment

Choose a reason for hiding this comment

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

Looks good, will test before merging!

@drwhut
Copy link
Owner

drwhut commented Dec 9, 2022

I assume this “rotate” means yaw+. I first thought Q and E (on QWERTY layout), then I realize they might be better for the camera's rotation. For the object, if there are also keys of their movements like WSAD, then the corresponding QE are more suitable. Maybe Caps Lock can be utilised: when it is on, WSAD and QE control the object; when it is off, WSAD and QE control the camera.

This seems more complicated than it needs to be - having one button + scroll wheel to rotate an object is fine.

The code for the tooltips (at least the other section) needs some kind of refactoring. This is getting messy in there to figure out all possible/different scenarios and what should be displayed.

Yeah, I agree. I think it could be a case of adding more auxiliary functions, like you did for checking if a piece is modifiable. Feel free to open an issue about this if you want.

@drwhut drwhut merged commit 73c04bd into drwhut:master Dec 9, 2022
@elmodor elmodor deleted the rotate_keybind branch December 9, 2022 16:00
@elmodor elmodor restored the rotate_keybind branch December 9, 2022 17:25
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.

Feature: Rotate keyboard shortcut
3 participants