Implement an editor to customize the touchscreen controls #14933
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grorp
added
Android
Feature ✨
grorp
force-pushed
the
touch-layout-editor
branch
from
5f174a8 to
2b12121
Compare
grorp
removed
the
Waiting (on dependency)
This comment was marked as duplicate.
This comment was marked as duplicate.
Member
Author
No, but I might add one in a future PR. See the "Features that would be cool, but are not planned for this initial PR" section. |
Contributor
|
I get the following crash when dragging a button after adding it from "Add" in the editor |
rubenwardy
added
the
Action / change needed
grorp
force-pushed
the
touch-layout-editor
branch
from
2b12121 to
22e005b
Compare
Member
Author
Cannot reproduce, but I assume it's this assertion failing: Replaced it with some printing for debugging (22e005b), can you tell me what it outputs? And more importantly, does it work now? If it does, I'll just remove the assertion. |
grorp
removed
the
Action / change needed
Member
|
FWIW: Yesterday I tested this PR on Android 6 and could not reproduce any such crash. Works on my side. |
Contributor
Works now |
grorp
force-pushed
the
touch-layout-editor
branch
from
22e005b to
ece32a9
Compare
Contributor
|
something for a follow up PR but might be nice to be able to move HUD elements like the minimap and hotbar as part of this: #15021 |
grorp
force-pushed
the
touch-layout-editor
branch
from
ece32a9 to
ff072c7
Compare
grorp
added
Rebase needed
grorp
force-pushed
the
touch-layout-editor
branch
from
ff072c7 to
ee6b6d8
Compare
grorp
force-pushed
the
touch-layout-editor
branch
from
9a1fe10 to
da80a40
Compare
grorp
force-pushed
the
touch-layout-editor
branch
from
da80a40 to
0d04cb6
Compare
Member
|
@grorp I would've done that if I had an Android build system set up at all. Currently I am testing the ARM v7a builds from the buildbot and sign them manually. |
Member
Author
would happen with enable_touch=true on desktop platforms
"anchor" is used by formspec with a different meaning, "position" is used by HUD and formspec with the same meaning. This change thus avoids confusion.
grorp
force-pushed
the
touch-layout-editor
branch
from
01415b8 to
d8ff60e
Compare
SmallJoker
approved these changes
Nov 19, 2024
Member
SmallJoker
left a comment
SmallJoker
left a comment
There was a problem hiding this comment.
☑️ The layout is reset to default if any Json::RuntimeError is thrown. The user does not see any error. Luanti continues to work.
☑️ Move/add/remove buttons from the Settings menu
A few minor comments regarding code quality. Works well as-is.
At some point I tested with SDL_HINT_MOUSE_TOUCH_EVENTS=1 to be able to test the touch controls with a mouse on desktop. That resulted in the touch controls working, but mouse events arriving twice in GUIs: - once directly via mouse - once via mouse -> touch (converted by SDL) -> mouse (converted by Minetest, regular conversion that's necessary for menus to work on touchscreens at all) This resulted in a crash here because the code had more side effects iirc. Now it seems fine, and having a workaround for this specific case is questionable anyway.
Member
|
Checked the diff since my last review. Still LGTM. Thanks for the update. 👍 |
This reverts commit d8ff60e.
Contributor
|
Just tested this out on Android, this is such a massive improvement! Tysm for implementing it, this fixes multiple gripes I've had with the mobile experience all at once, and is so good for accessability and customizability! Just being able to remove the zoom button and add the inventory button in the controls is huge for me lol By the way, do you have any thoughts about how #13229 could be implemented, adding arbitrary internal keybindings as buttons through your new add-buttons prompt? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #14456
This PR implements a touchscreen layout editor. Features:
editor.mp4
The UI design is inspired by Minecraft's control editor.
P.S. I have used "ChatGPT" while working on an early version of the touch control editor (but this PR doesn't include code generated by it)
To do
This PR is a Ready for Review.
Revert commit "Temp: Unbreak mainmenu security" before merging
Features that would be cool, but are not planned for this initial PR:
How to test
Play Minetest on Android. Use the touchscreen layout editor (accessible via the pause menu or the settings menu).
APKs for testing (signed with a debug keystore)
app-armeabi-v7a-release.zip
app-arm64-v8a-release.zip
app-x86-release.zip
app-x86_64-release.zip
The APKs are inside ZIP archives, so you have to extract the ZIP first. You have to delete any existing Minetest installation including data beforehand for installation to work.