Android: Add selection dialog (drop down/combo box)

[srifqi]

Goal of the PR
This PR adds selection dialog on Android that will be shown when a player taps a drop down (combo box).

Videos

Minetest-Android-selection-dialog.mp4

Minetest-Android-selection-dialog-in-game.mp4

How does the PR work?
In GUIFormSpecMenu::getAndroidUIInput() and GUIModalMenu::preprocessEvent(), combo boxes are also processed.

This PR also refactors and simplifies interaction between the Java (Android) part and the C++ part (see porting_android.cpp/h). See the commit message for more technical information.

Does this relate to a goal in the roadmap?
Yes, this PR tries to improve user experiences.

If not a bug fix, why is this PR needed? What usecases does it solve?
The Irrlicht's combo box (drop down) is small and hard to navigate on Android (touch-screen devices). Hard to scroll worsen this issue.

To do

This PR is a Ready for Review.

How to test

To test the change in the main menu,

1. Run Minetest on a touch-screen device.
2. Open any dialog/setting that has selections/drop downs/combo boxes.
3. Tap it.
4. A selection dialog should show up.
5. Tap a different selection.
6. The formspec should update with new data.
7. Repeat step 3 to 5, but with the same selection (as given).
8. The formspec should stay the same.
9. Repeat step 3 to 4.
10. Tap outside the dialog.
11. The formspec should stay the same.

To test the change inside a world,

1. Run Minetest on a touch-screen device.
2. Create a world (optional) and use a mod that has selections/drop downs/combo boxes.
3. Enter the world.
4. Open the dialog from the mod.
5. Try the same as testing in the main menu above.

Android: APK builds
Included in these builds is the Mail mod. Type /mail in the chat to open the formspec. These builds use minetest/irrlicht#241 for their IrrlichtMT library.

• armeabi-v7a
• arm64-v8a
• x86
• x86_64

Old builds (without the PR in the IrrlichtMT)

• armeabi-v7a
• arm64-v8a
• x86
• x86_64

[grorp]

I tested your arm64 APK. Most of the time, it worked fine, but a few times, I somehow accidentally broke it. Here's a video:

1000034919.mp4

First, the Android dialog didn't show and I was shown a standard Minetest dropdown instead. Then, I tried to select something in that dropdown and the Android dialog opened. When I closed the Android dialog, the whole settings dialog closed with it.

[srifqi]

(video)
First, the Android dialog didn't show and I was shown a standard Minetest dropdown instead. Then, I tried to select something in that dropdown and the Android dialog opened. When I closed the Android dialog, the whole settings dialog closed with it.

This is caused by Irrlicht opening the combo box if we clicked/tapped outside the formspec area while the combo box is still focused (also happens on desktop). We can try to remove focus from the combo box by focusing on its parent element after selecting the item to prevent the issue.

diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp
index ea89dcdfa..bcda1f655 100644
--- a/src/gui/guiFormSpecMenu.cpp
+++ b/src/gui/guiFormSpecMenu.cpp
@@ -3503,6 +3503,7 @@ bool GUIFormSpecMenu::getAndroidUIInput()
				change.GUIEvent.Element = 0;
				change.GUIEvent.EventType = EGET_COMBO_BOX_CHANGED;
				dropdown->getParent()->OnEvent(change);
+				Environment->setFocus(dropdown->getParent());
			}
		}

The dialog close is caused by passing an Esc keyboard event to the combo box, but since the combo box is already closed, the event is passed to the dialog and closes it. There is no method to close a combo box list (or at least check its status).

[oong819]

Minetest formspec on Android are very unTouchscreenFriendly and somewhat small (made for cursor)

Should we consider create a functiom that read formspec and create Android widget accordingly?

[srifqi]

Should we consider create a functiom that read formspec and create Android widget accordingly?

The problem is that a new element is created by calling the Environment class, so we will also need to edit the Irrlicht either way (which I try to not do it because it is harder).

minetest/src/gui/guiFormSpecMenu.cpp

Lines 1351 to 1353 in e36b222

	//now really show list
	gui::IGUIComboBox *e = Environment->addComboBox(rect, data->current_parent,
	spec.fid);

---

By the way, is the patch above enough to prevent the issue? If it is not, should we try to edit the Irrlicht's IGUIComboBox and add a new method to open/close a list?

[oong819]

oh i have no problem with the PR, its work fine. i just got reminded of irrlicht scrollview X_X

[grorp]

I don't think we can get away without modifying IrrlichtMt here. Instead of unfocusing the combobox to hide the issue, I think we should add a way to reliably detect when the user opens a combobox.

I also think that we should prevent comboboxes from opening their dropdown on Android, otherwise you see the Irrlicht dropdown behind the transparent background of the Android dialog.

Here's my proposal for the Irrlicht changes: minetest/irrlicht#241

And here are the Minetest changes for the Irrlicht changes (quick and dirty, view with whitespace changes hidden): srifqi/minetest@android_selection_dialog...grorp:minetest:sel-dialog-proposal

[srifqi]

Here's my proposal [...]

Thank you for your work! I did not know much about IrrlichtMT to make those changes. The code works.

---

EDIT: The build failure is caused by the differences in the IrrlichtMT library. New APKs have been added to the first post.

[srifqi]

I rebased and squashed this PR.

The grorp's PR has been merged on IrrlichtMT, too.

[grorp]

This is still waiting for a new IrrrlichtMT revision.

[srifqi]

I rebased this PR to include the newest IrrlichtMT version.

[grorp]

This PR makes dropdowns much more usable on Android. Thank you!

While testing, I noticed one problem: You can no longer cancel the text input completely by pressing the back button.

Before this PR

Record_2023-12-07-19-46-46_2f92d212d72044a7b2c4130128bd465a.mp4

• If you press the "Done" button without changing the pre-filled text, the pre-filled text is submitted.
• If you press the back button, no submission happens.

After this PR

Record_2023-12-07-19-53-52_2f92d212d72044a7b2c4130128bd465a.mp4

• If you press the "Done" button without changing the pre-filled text, the pre-filled text is submitted.
• If you press the back button, the pre-filled text is submitted. (<- wrong!)

[srifqi]

While testing, I noticed one problem: You can no longer cancel the text input completely by pressing the back button.

Thank you for noticing it.

I added another enum to represent input dialog states:

1. is currently shown,
2. has its input sent, and
3. is cancelled/dismissed.

---

I also rebased this PR to current master and added/fixed some comments. I might squash these commits later.

[grorp]

The text input dialog sometimes doesn't properly set the value:

not.working.text.input.dialog.mp4

So far, I've only observed on the first try after starting Minetest.

Shouldn't getAndroidUIInput return void instead of bool? Its return value is never used as far as I can see, and it always returns false.

[srifqi]

I have rebased and squashed this PR.

The text input dialog sometimes doesn't properly set the value:
(video)

So far, I've only observed on the first try after starting Minetest.

Oh, that is true. The event propagation (to C++ side) seems to be late by one/two events. I will try to look into it.

Shouldn't getAndroidUIInput return void instead of bool? Its return value is never used as far as I can see, and it always returns false.

I agree.

[srifqi]

The text input dialog sometimes doesn't properly set the value:

I can not find the actual cause, but I found that delaying GameActivity::getDialogMessage() (making it longer to response) somehow fixes the issue (or at least making it rarer for an already not-often case). This does not explain the value being late by one/two events.

diff --git a/android/app/src/main/java/net/minetest/minetest/GameActivity.java b/android/app/src/main/java/net/minetest/minetest/GameActivity.java
index f2ff09f3d..d5b057ffa 100644
--- a/android/app/src/main/java/net/minetest/minetest/GameActivity.java
+++ b/android/app/src/main/java/net/minetest/minetest/GameActivity.java
@@ -180,6 +180,7 @@ public class GameActivity extends NativeActivity {
 
 	public String getDialogMessage() {
 		inputDialogState = DialogState.DIALOG_CANCELED;
+		for (int i = 0; i < 1e5;) i ++;
 		return messageReturnValue;
 	}
 

---

By the way, that issue also happens in the recent master branch. I still has not been able to reproduce the issue in the 5.8.0 version.

[grorp]

By the way, that issue also happens in the recent master branch. I still has not been able to reproduce the issue in the 5.8.0 version.

Hmmm, if you say it doesn't happen on 5.8.0 ... could you try whether it's fixed if you revert 00d9d96?

[srifqi]

could you try whether it's fixed if you revert 00d9d96?

The issue seems to be caused by that commit.

We need a better heuristic for shouldRender():

minetest/src/client/renderingengine.h

Lines 141 to 150 in 2c44620

	static bool shouldRender()
	{
	// On Android, pause rendering while the app is in background (generally not visible).
	// Don't do this on desktop because windows can be partially visible.
	#ifdef __ANDROID__
	return get_raw_device()->isWindowActive();
	#else
	return true;
	#endif
	};

... or just always render the GUI:

diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp
index 4bb7b041f..43ea4d2b2 100644
--- a/src/gui/guiEngine.cpp
+++ b/src/gui/guiEngine.cpp
@@ -267,3 +267,2 @@ void GUIEngine::run()
 	while (m_rendering_engine->run() && (!m_startgame) && (!m_kill)) {
-		if (RenderingEngine::shouldRender()) {
 		// check if we need to update the "upper left corner"-text
@@ -295,3 +294,2 @@ void GUIEngine::run()
 		driver->endScene();
-		}
 

[grorp]

The issue seems to be caused by that commit.

Alright, sorry for reporting it on your PR. Feel free to ignore the issue for this PR.

We need a better heuristic for shouldRender()

I have an idea for that: Perhaps checking for !Paused instead of Focused && !Paused in Irrlicht fixes the issue. I'll try tomorrow.

... or just always render the GUI:

diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp
index 4bb7b041f..43ea4d2b2 100644
--- a/src/gui/guiEngine.cpp
+++ b/src/gui/guiEngine.cpp
@@ -267,3 +267,2 @@ void GUIEngine::run()
 	while (m_rendering_engine->run() && (!m_startgame) && (!m_kill)) {
-		if (RenderingEngine::shouldRender()) {
 		// check if we need to update the "upper left corner"-text
@@ -295,3 +294,2 @@ void GUIEngine::run()
 		driver->endScene();
-		}
 

guiEngine.cpp is only for the mainmenu, so this won't affect in-game GUIs. guiEngine.cpp not the "GUI engine", but the "engine GUI".

[grorp]

Tested together with #14211. Works well.

Thank you!

[SmallJoker]

Tested on Android 6.0. Works.