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

TouchScreenGUI: Fix only 9 hotbar slots being usable #13698

Merged
merged 3 commits into from Aug 24, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Jul 28, 2023

This PR fixes #13691.

Before this PR, TouchScreenGUI simply emitted a keypress event for one of the number keys when a hotbar item was pressed. After the 9th hotbar item (the last one for which there is a number key), it started pressing "random" keys.

After this PR, all hotbar items are selectable (except if they are hidden behind other controls, but that's a different issue).

To do

This PR is a Ready for Review.

How to test

Install the CI-built APK on your Android device. Start a Devtest world and execute /hotbar 20. Verify that you can select all hotbar slots. Verify that you cannot use the 19th hotbar slot to change your camera mode.

Because I rebased this PR, you'll experience #13743 while testing.

@srifqi
Copy link
Member

srifqi commented Aug 2, 2023

Thank you for this fix. I was not sure about the hotbar when refactoring TouchScreenGUI, so I just left it as it is. I also just compiled it using Android Studio, so I have not checked it with other compilers.

Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

I tested the fix. It does work and its implementation looks good, except for a few lines.

src/client/hud.cpp Outdated Show resolved Hide resolved
src/gui/touchscreengui.cpp Outdated Show resolved Hide resolved
@srifqi
Copy link
Member

srifqi commented Aug 15, 2023

The merge conflict is caused by the same TouchScreenGUI compilation error fix that this PR proposes because the fix has been merged separately (7f9de5d).

@srifqi srifqi added Rebase needed The PR needs to be rebased by its author. Action / change needed Code still needs changes (PR) / more information requested (Issues) labels Aug 15, 2023
grorp and others added 2 commits August 21, 2023 13:05
Co-authored-by: Muhammad Rifqi Priyo Susanto <muhammadrifqipriyosusanto@gmail.com>
Co-authored-by: Muhammad Rifqi Priyo Susanto <muhammadrifqipriyosusanto@gmail.com>
@grorp grorp requested a review from srifqi August 21, 2023 12:09
@Zughy Zughy removed Rebase needed The PR needs to be rebased by its author. Action / change needed Code still needs changes (PR) / more information requested (Issues) labels Aug 21, 2023
Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

This PR looks good to me.

@grorp grorp merged commit 92b6ff4 into minetest:master Aug 24, 2023
13 checks passed
@grorp grorp deleted the touchscreengui-hotbar branch December 18, 2023 16:38
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.

Only hotbar slots 1-9 are selectable on Android.
3 participants