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.

Sign up for GitHub

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

Jump to bottom

Add crosshair support for Android #7865

Merged
merged 7 commits into from
Sep 29, 2022
Merged

Add crosshair support for Android #7865

merged 7 commits into from
Sep 29, 2022

Conversation

srifqi
Copy link
Member

@srifqi srifqi commented Nov 14, 2018 •
edited

If touch_use_crosshair is enabled, a crosshair will be shown to select the object. This will give Android (or any touch screen interface) player a way to play like they play on desktop (with mouse).

For third-person camera mode, it is same as the desktop version: on third-person back camera mode, player is forced to use crosshair; while on third-person front camera mode, player is unable to select anything.

Possibly fixes #7645 and fixes #6181.

YouTube video (1)
YouTube video (2)

@paramat paramat added Android Feature ✨ PRs that add or enhance a feature labels Nov 14, 2018
@stujones11
Copy link
Contributor

stujones11 commented Nov 14, 2018 •
edited

I do welcome this as an option, however, it only really fixes #6181 if this is enforced in third person view. I don't see any code that is doing this.

@srifqi
Copy link
Member Author

srifqi commented Nov 21, 2018

I do welcome this as an option, however, it only really fixes #6181 if this is enforced in third person view. I don't see any code that is doing this.

Just added. Thanks for remind me.

@stujones11
Copy link
Contributor

Tested and working. 👍

The code also seems fine except maybe some questionable styling, however this file clearly must be whitelisted, anyway :D

@ghost
Copy link

ghost commented Dec 1, 2018

When you slide your finger while digging, does the camera rotate? If not, this pull request doesn't fix issue #7645. The YouTube video doesn't show what the case is.

@SmallJoker SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Dec 2, 2018
nerzhul
nerzhul previously requested changes Dec 3, 2018
View reviewed changes
src/gui/touchscreengui.cpp Outdated Show resolved Hide resolved
@srifqi
Copy link
Member Author

srifqi commented Dec 23, 2018

When you slide your finger while digging, does the camera rotate? If not, this pull request doesn't fix issue #7645. The YouTube video doesn't show what the case is.

Yes. The camera still rotates and the player is still digging. Watch my new video.

@srifqi
Copy link
Member Author

srifqi commented Dec 25, 2018 •
edited

I just realized that something doesn't seem right on third-person back camera mode. You can't select as far as when you're playing on desktop.
@stujones11, can you try please?

@SmallJoker SmallJoker removed the Rebase needed The PR needs to be rebased by its author. label Dec 26, 2018
@stujones11
Copy link
Contributor

stujones11 commented Dec 26, 2018 •
edited

@srifqi I have noticed that 'rear-view' crosshair dig/place is rather awkward on my phone due to its very wide aspect screen, although my tablet seems much more like the windowed desktop version.

The only other reason I can think why selection distance might be different for android would be this commit: 2c450ed Are you comparing this in creative or survival mode?

Edit: At a guess, it may be that the shootline is taken from the camera position and not the player's. That would explain why selection distance (from the player) would be reduced in back camera mode.

Note to devs: This is technically a bugfix since #6181 is labelled as such. I don't mean to be pedantic but this could matter if there is a feature freeze :)

@ghost ghost mentioned this pull request Jan 5, 2019
Closed
@srifqi
Copy link
Member Author

srifqi commented Jan 7, 2019 •
edited

Are you comparing this in creative or survival mode?

Yes. It has lower distance in survival mode. I cannot find any line that make it differs. I just tested and selection distance bug also happens on desktop.

I'm still searching for camera offset thing-y. Maybe, you have ideas where to search? I think that selection distance issue can be in another PR.

I agree that this is a bugfix for #6181 and a feature for #7645. This is important to be part of 5.0.0 because many player will be forced to update to 5.0.0 and trick described in #6181 cannot be used. But, if this will be merged after 5.0.0, some player may not update and use trick described in #6181.

@paramat paramat added this to the 5.0.0 milestone Jan 10, 2019
@rubenwardy rubenwardy removed this from the 5.0.0 milestone Feb 2, 2019
@srifqi srifqi mentioned this pull request Feb 16, 2019
Closed
@rubenwardy rubenwardy force-pushed the master branch 2 times, most recently from 0e3b135 to 39c54e1 Compare June 21, 2019 00:46
@SmallJoker SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Jul 20, 2019
ClobberXD
ClobberXD reviewed Sep 1, 2019
View reviewed changes
src/client/game.cpp Outdated Show resolved Hide resolved
@SmallJoker SmallJoker removed the Rebase needed The PR needs to be rebased by its author. label Sep 1, 2019
sfan5
sfan5 reviewed Jul 14, 2022
View reviewed changes
builtin/settingtypes.txt Outdated Show resolved Hide resolved
src/client/game.cpp Outdated Show resolved Hide resolved
src/client/game.cpp Outdated Show resolved Hide resolved
src/gui/touchscreengui.cpp Outdated Show resolved Hide resolved
src/gui/touchscreengui.cpp Outdated Show resolved Hide resolved
@Zughy Zughy added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Aug 2, 2022
@srifqi
Copy link
Member Author

srifqi commented Aug 3, 2022

I changed some things from the reviews:

  • The crosshair calculation (in determining enable/disable) is moved into game.cpp.
  • The setting use_crosshair is now touch_use_crosshair.
  • The "touch target" is removed completely.
  • The crosshair support is not just for Android, but also for all that uses touch screen. (This is already done since rollerozxa's commit but I explain it more in settingtypes.txt etc.)

@Zughy Zughy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Aug 3, 2022
x2048
x2048 reviewed Aug 17, 2022
View reviewed changes
src/client/game.cpp Outdated Show resolved Hide resolved
src/gui/touchscreengui.cpp Outdated Show resolved Hide resolved
@sfan5 sfan5 self-requested a review August 23, 2022 20:16
@Dawimpy Dawimpy mentioned this pull request Sep 11, 2022
Closed
@Zughy
Copy link
Member

Zughy commented Sep 18, 2022

@srifqi please have a look at 2048's review, just one appproval missing :)

@Zughy Zughy added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Sep 18, 2022
@srifqi
Copy link
Member Author

srifqi commented Sep 21, 2022

I followed up the review and rebased the PR.

@Zughy Zughy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Sep 25, 2022
x2048
x2048 approved these changes Sep 27, 2022
View reviewed changes
Copy link
Contributor

@x2048 x2048 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.

src/client/game.cpp Outdated Show resolved Hide resolved
srifqi and others added 7 commits September 28, 2022 09:00
@srifqi
Add crosshair support for Android
3da73ac 
If enabled, a crosshair will be shown to select object.
This will give Android players a way to play like they play on desktop.
On third-person back camera mode, player is forced to use crosshair.
On third-person front camera mode, player is unable to select anything.
@rollerozxa @srifqi
Fix building when HAVE_TOUCHSCREENGUI=1 with android crosshair PR app…
445066e 
…lied
@rubenwardy @srifqi
Android: Fix selection mesh not updating when crosshair moves
7b59a5b
@srifqi
Move crosshair drawing calculation into game.cpp
ac6d75d 
Crosshair drawing calculation is the same as non-touch control, except when in first-person camera.
Rename setting: use_crosshair -> touch_use_crosshair
@srifqi
Remove touch target setting
0ea601d 
Crosshair is for all touch screen GUI (not necessarily Android).
Rename variable: m_android_use_crosshair -> m_touch_use_crosshair
@srifqi
Add Game::isNoCrosshairAllowed()
46aa095 
The rule about allowing no crosshair is moved into a method.
The (X, Y) calculation for updating shootline when using crosshair is removed for a specific case.
@srifqi
Use !isNoCrosshairAllowed() instead
97ec893 
The case of third-person front camera is already covered in other places.
@x2048 x2048 merged commit 3978b9b into minetest:master Sep 29, 2022
@srifqi
Copy link
Member Author

srifqi commented Sep 30, 2022

Thank you for the merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClobberXD ClobberXD ClobberXD left review comments

@rubenwardy rubenwardy rubenwardy approved these changes

@x2048 x2048 x2048 approved these changes

@nerzhul nerzhul nerzhul left review comments

@sfan5 sfan5 Awaiting requested review from sfan5

Assignees
No one assigned
Labels
Android @ Client / Controls / Input Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature >= Two approvals ✅ ✅
Projects
None yet
Milestone
No milestone