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

Android: Keyboard modifier and arrow key support for text edit #40398

Merged

Conversation

thebestnom
Copy link
Contributor

Needed for #36776, also I think it's a welconiing change anyway

thinking changing InputEventScreenTouch and InputEventScreenDrag to inherit from InputEventWithModifiers instead so they could also get benefits from this PR

(if someone have better option for what I did on onKeyDown in GodotEditText I would be glad to hear)

Tested on Android Editor and https://github.com/thebestnom/Android-Tester

@thebestnom thebestnom changed the title Android: Keyboard modifier and arrow key support Android: Keyboard modifier and arrow key support for text edit Jul 15, 2020
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

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

I've just checked the code and haven't tested yet. The DisplayServer part looks fine (apart from the comment by @akien-mga), and for now I don't clearly understand the java side.

I assume this change is only meant to be used for an actual connected keyboard? It's hard to tell how this might affect the case with virtual keyboard so it needs proper testing.

@thebestnom
Copy link
Contributor Author

Also I just saw that EditText didn't handle keyUp event, so I'm adding that

@thebestnom thebestnom force-pushed the android_keyboard_modifiers_and_arrows branch from c121de5 to 803c913 Compare July 15, 2020 12:53
@thebestnom
Copy link
Contributor Author

also tested with virtual keyboard and it doesn't seem to interfere

@thebestnom
Copy link
Contributor Author

thinking changing InputEventScreenTouch and InputEventScreenDrag to inherit from InputEventWithModifiers instead so they could also get benefits from this PR

asking again, any comment on this?

@pouleyKetchoupp
Copy link
Contributor

thinking changing InputEventScreenTouch and InputEventScreenDrag to inherit from InputEventWithModifiers instead so they could also get benefits from this PR

It would be probably ok to change if needed, but is there an actual use case for an editor or game feature that could benefit from it? Pressing keyboard keys along with touch events doesn't seem user-friendly.

@thebestnom
Copy link
Contributor Author

It would be probably ok to change if needed, but is there an actual use case for an editor or game feature that could benefit from it? Pressing keyboard keys along with touch events doesn't seem user-friendly.

I'm comming from the point of view of Android Editor where there it's pretty handy, but for other pojects most I can think of is testing your game on device/playing on chromeOS/playing on tablet without special touch control and in those cases a lot of time you can use drag or touch with one hand

@pouleyKetchoupp
Copy link
Contributor

I'm comming from the point of view of Android Editor where there it's pretty handy, but for other pojects most I can think of is testing your game on device/playing on chromeOS/playing on tablet without special touch control and in those cases a lot of time you can use drag or touch with one hand

I understand it's physically possible in some cases but I would be for adding support for it only when there's a specific use case scenario that requires it.

@thebestnom thebestnom force-pushed the android_keyboard_modifiers_and_arrows branch 5 times, most recently from d0c5112 to b234513 Compare July 15, 2020 17:25
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Most of the logic looks good; just a couple of comments to address.

@thebestnom thebestnom force-pushed the android_keyboard_modifiers_and_arrows branch 2 times, most recently from 65e4ae2 to ba71c80 Compare July 18, 2020 21:09
@thebestnom thebestnom force-pushed the android_keyboard_modifiers_and_arrows branch from ba71c80 to cb06d90 Compare July 19, 2020 19:23
@akien-mga
Copy link
Member

Needs a rebase to fix CI builds (at least Linux - not sure why macOS fail but a new build will likely let it pass).

@thebestnom thebestnom force-pushed the android_keyboard_modifiers_and_arrows branch from cb06d90 to 166103c Compare July 21, 2020 19:15
@thebestnom
Copy link
Contributor Author

@akien-mga rebased :)

@akien-mga akien-mga merged commit 25d59a5 into godotengine:master Jul 21, 2020
@akien-mga
Copy link
Member

Thanks!

@thebestnom thebestnom deleted the android_keyboard_modifiers_and_arrows branch July 21, 2020 20:01
@thebestnom thebestnom mentioned this pull request Jul 21, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants