Formspec: Pass the second-touch event as is

[srifqi]

Goal of the PR
This PR tries to restore old touch behaviour on inventory management.

Minetest-Android-split-stack.mp4

Old videos

Minetest-Android-split-stack.mp4

Minetest-Android-split-stack.mp4

How does the PR work?
The current code only convert the second-touch to be a right click event. This would not work for the new inventory management.

This PR passes the second-touch event as a touch event and then uses it as a modifier of the first-touch event.

The current code prohibits multiple button click/press action in a single mouse, but the behaviour is different for touch-screen devices because mouse clicks are simulated from touches. This PR removes that limit only for touch-screen devices.

Does it resolve any reported issue?
Yes, this PR tries to fix #13822.

Does this relate to a goal in the roadmap?
Yes, this PR fixes a regression.

To do

This PR is Ready for Review.

How to test

Do as described in the videos above. (... and go wild[!] while testing.)

Simple test

1. Play a Minetest world in a touch-screen device.
2. Open the inventory dialog.
3. Tap and hold a stack of item.
4. Do 2nd tap using different finger/hand (different touch).
5. A single item should be placed on the first touch's position.
6. Move the first touch to another slot.
7. Do 2nd tap again (like step number 4).
8. Another single item should be placed on the new first touch's position.
9. Play a Minetest world in a non-touch-screen device.
10. All inventory-related action should work as usual.

Android: APK builds (old, before grorp's second review)

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

Older builds

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

[srifqi]

I rebased this PR and added APKs for testing.

[SmallJoker]

I found the following issue that might be worth having a look into:

1. Use the MTG infinite source creative inventory
2. Drag & drop an ItemStack from the creative inventory onto another ItemStack of identical types
3. The ItemStack count combines but the dragged ItemStack is not released
4. When tapping again into any other empty or ItemStack of identical types, another copy is taken from the creative inventory.
   Expected: release the dragged ItemStack when releasing the finger press.

Aside from that, the PR works as advised.
EDIT: I again used a manually signed APK from the GitHub actions. I cannot figure out why your build (armeabi-v7a) seems to load files from some location that seems to have leftover data from previous installs 🤷 .

[srifqi]

I found the following issue that might be worth having a look ....

That also happens in the master branch, even for non-touch devices. My guess is that hovering over the same item as the selected/picked item means adding one to the stack. That would not work for detached inventories. It might be useful for other types of detached inventories. I can not find other use cases, though.

[SmallJoker]

Works.

[tetsuo55]

This build solves every issue I have except that (cross)bows still can't be used in mineclone2

[grorp]

I rebased a582d7b on top of the latest master branch (4d2227c), together with the latest Irrlicht master branch (minetest/irrlicht@85081d6).

In my tests, this PR didn't work as expected. Here's a video:

recording.android.inventory.fix.review.mp4

1. When I tried to use the "while drag&dropping, tap the screen with another finger to put a single item into the hovered slot" feature, the entire stack was put into the hovered slot, not just a single item.

   The feature only worked correctly if the first slot I tried to put an item into was the source slot, or no slot at all (e.g. a border between two slots).

2. Since Inventory mouse shortcut improvements #13146, you can left-drag to distribute an itemstack evenly across multiple slots. If you tap the screen with another finger during this operation, the item counts get confused. The second tap should probably be ignored in this case, as it was before this PR.

[srifqi]

Thank you for the test. I must have missed that case. It seems like the problem is much deeper than I though since the inventory PR only considers one pointer.

I'm converting this PR as a draft until I found a better solution or another PR fixes the issue better (in that case, I'll close this PR).

[appgurueu]

This build solves every issue I have except that (cross)bows still can't be used in mineclone2

Could you elaborate on this? Since when can't crossbows be used in MCL2? This seems like a different issue.

[grorp]

Could you elaborate on this? Since when can't crossbows be used in MCL2? This seems like a different issue.

This is a known issue, you can't hold the right mouse button on Android, so bows don't work. I'm currently working on different approaches to fix this.

#8543

[srifqi]

This PR is ready for review.

Android: APK builds

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

EDIT: The Visual Studio builds failed due to vcpkg errors. It is now fixed by the recent commit.

[SmallJoker]

Thank you for the update. I tried splitting stacks, combining them and it works well enough.

There is still a difference to the desktop version but I do not know whether that's intentional or not:

1. Single tap to select an item stack of any count (recommended > 10)
2. Finger down on any slot. Drag on other slots without releasing
3. (the stacks split up equally as more slots are selected)
4. Finger up
5. There is a remaining item stack still selected that needs manual placement
   • Desktop version: the remaining item stack is put back to the source.

[srifqi]

Thank you for testing. Can you give more concrete example, e.g. the amount of items and slots?

EDIT: It sometimes happens when the first dragged slot is the same as the original/source slot. If the first dragged slot is different than the original slot, the remaining items will be put back to the original slot. It happens when the original/source slot is filled. If the original slot is not filled, the remaining items will be put back to the original slot.

EDIT (2): That issue also happens in desktop. The table below shows my tests. Numbers of items in a stack that do not fail are omitted. It seems like it happens when

1. there are odd number of items divided into 2 slots or
2. there are odd number of items divided into ceil(number of items / 2) slots.

Number of items in a stack	Number of slots that fails
3	2
5	2 and 3
7	2 and 4
9	2 and 5
11	2 and 6
13	2 and 7
15	2 and 8
17	2 and 9
19	2 and 10

EDIT (3): One of the solution is to add the remainder into the original slot directly. (branch)

Example diff

diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp
index a51a02669..def1c28e5 100644
--- a/src/gui/guiFormSpecMenu.cpp
+++ b/src/gui/guiFormSpecMenu.cpp
@@ -4643,6 +4643,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 			// The split amount will always at least one, because the number
 			// of slots will never be greater than the selected amount
 			u16 split_amount = m_left_drag_amount / m_left_drag_stacks.size();
+			u16 split_remaining = m_left_drag_amount % m_left_drag_stacks.size();
 
 			ItemStack stack_from = m_left_drag_stack;
 			m_selected_amount = m_left_drag_amount;
@@ -4653,7 +4654,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 
 				if (ds.first == *m_selected_item) {
 					// Adding to the source stack, just change the selected amount
-					m_selected_amount -= split_amount;
+					m_selected_amount -= split_amount + split_remaining;
 
 				} else {
 					// Reset the stack to its original state

[SmallJoker]

@srifqi I see. Thank you for looking that up. It really does affect desktop as well, thus not of a concern for this PR.

[grorp]

Nice work!

In addition to what I wrote below: You can trigger broken/inconsistent behavior by doing a second tap while you are "left-dragging to distribute evenly". Here's a video:

Record_2023-11-21-16-48-03_2f92d212d72044a7b2c4130128bd465a.mp4

The second tap should probably be ignored if you are currently "left-dragging to distribute evenly". (Ignore the error messages, I added them for debugging.)

EDIT: I just noticed that I already reported this issue for a previous version of this PR :-D

[srifqi]

Thank you for testing. The issues should be fixed now.

---

I just noticed that I already reported this issue for a previous version of this PR :-D

I must have missed that. Thank you for noticing it (again).

[grorp]

Works correctly in the cases I've tested 👍

One remaining question:

[grorp]

Works. Code also looks good. 👍

Thank you.

There's another annoyance which is offtopic for this PR: Since #13146, you can pick up itemstacks of the same type by moving over them while dragging&dropping. I often accidentally pick up the items that I have just put down with a "second touch".

[srifqi]

@rubenwardy, any objection?

EDIT: https://irc.minetest.net/minetest-dev/2023-11-26#i_6135461