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

Fix broken fixed_virtual_joystick #12814

Merged
merged 1 commit into from Nov 26, 2022

Conversation

srifqi
Copy link
Member

@srifqi srifqi commented Sep 27, 2022

Goal of the PR
Restore the behaviour of fixed virtual joystick as originally implemented.

How does the PR work?
This PR fixes some C++ implementation issues of the behaviour:

  • The usage of m_screensize as u32 produced underflow when it is used as a subtrahend.
  • Virtual joystick's movement control calculation did not run when the position of the tap does not change.

Does it resolve any reported issue?
Yes, this PR tries to fixes #12002.

Does this relate to a goal in the roadmap?
Probably, this PR tries to fix gameplay/control-related bug.

To do

This PR is Ready for Review.

How to test

  1. Use a device that uses touch screen.
  2. Enable fixed_virtual_joystick.
  3. Play any Minetest world.
  4. Tap outside (especially above/below) the virtual joystick.
  5. That tap should control the camera and should not control the movement (the middle "button" should not moved to the tap's location).
  6. Tap inside the virtual joystick.
  7. That tap should control the movement immediately (no need to move finger).

If still uses u32, m_screensize will yield a big value (underflow) when used as a subtrahend.
ETIE_MOVED is allowed to be run if joystick's ID is available and virtual joystick is fixed.
Add .0f for some float values.
@sfan5 sfan5 changed the title Virtual joystick: Use s32 when using m_screensize as a subtrahend Fix broken fixed_virtual_joystick Sep 27, 2022
@sfan5 sfan5 added Android Bugfix 🐛 PRs that fix a bug labels Sep 27, 2022
@rubenwardy rubenwardy added this to the 5.7.0 milestone Sep 27, 2022
@rubenwardy rubenwardy self-requested a review September 29, 2022 19:55
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TurkeyMcMac TurkeyMcMac 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 and works on Android.

@srifqi
Copy link
Member Author

srifqi commented Nov 26, 2022

Thank you for the merge!

@srifqi srifqi deleted the fix_fixed_virtual_joystick branch November 26, 2022 17:00
appgurueu pushed a commit to appgurueu/minetest that referenced this pull request May 2, 2023
…netest#12814)

If still uses u32, m_screensize will yield a big value (underflow) when used as a subtrahend.
ETIE_MOVED is allowed to be run if joystick's ID is available and virtual joystick is fixed.
Add .0f for some float values.
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.

Enabling fixed_virtual_joystick doesn't fix virtual joystick
5 participants