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

Restore pre-5.9.0-dev behavior of touch_use_crosshair=false shootline #14389

Merged
merged 2 commits into from Feb 24, 2024

Conversation

grorp
Copy link
Member

@grorp grorp commented Feb 19, 2024

On Android with touch_use_crosshair=false, if you move the camera, release your finger again and then put your finger on any on-screen button or the joystick, the shootline will be updated according to the new position of your finger. This happens because Android reuses pointer IDs.

This bug was introduced by #14087. This PR fixes it by storing the position of the pointer used for the shootline separately.

We can't just use a zero-length shootline after the pointer is released because that breaks short-tapping.

To do

This PR is a Ready for Review.

How to test

As described above:

  1. Play Minetest on Android with touch_use_crosshair=false
  2. Start moving the camera
  3. Release your finger
  4. Put your finger on the joystick

before this PR: the shootline is updated to start behind the joystick
after this PR: the shootline stays the same

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

Not tested but seems good

@grorp
Copy link
Member Author

grorp commented Feb 20, 2024

Another case to consider is where the shootline should point before the player first touches the screen.

In 5.8.0 and earlier, the shootline wasn't set at all in this case. This results in Irrlicht's line3d default (0,0,0 to 1,1,1) being used, irrespective of the camera position.

line3d default shootline

In 5.9.0-dev (and also with this PR), the pointer position is initialized as (0,0), so the shootline starts in the upper-left corner of the screen.

Both behaviors are clearly stupid, the question is what the right behavior would be.

…r=false

This happened because Android reuses pointer IDs.
Also includes a refactor to merge "m_known_ids" and "m_pointer_pos".
@grorp
Copy link
Member Author

grorp commented Feb 21, 2024

I've decided to simply restore the behavior before 5.9.0-dev: The shootline is initially at (0,0,0) -> (1,1,1) until you first move the camera/dig/place. After releasing the finger used to move the camera/dig/place, the shootline stays at it's current in-world position until it's needed again. #14188 is of course still fixed.

This way, it will all work like it used to work and nobody will complain. You could put more work into this to find a perfect solution, but it really doesn't matter very much.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

Still not tested but still seems good

@grorp grorp changed the title Fix incorrect shootline after releasing pointer if touch_use_crosshair=false Restore pre-5.9.0-dev behavior of touch_use_crosshair=false shootline Feb 24, 2024
@grorp grorp merged commit 57de599 into minetest:master Feb 24, 2024
13 checks passed
@grorp grorp deleted the touch-fix-id-reuse branch February 24, 2024 12:12
appgurueu pushed a commit to y5nw/minetest that referenced this pull request Mar 4, 2024
…minetest#14389)

* Fix incorrect shootline after releasing pointer if touch_use_crosshair=false

This happened because Android reuses pointer IDs.
Also includes a refactor to merge "m_known_ids" and "m_pointer_pos".

* Restore pre-5.9.0-dev behavior of shootline when !m_has_move_id
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.

None yet

3 participants