Avoid duplicate Button presses from touch-emulated mouse

[KurodaKayn]

Close #120439.
Touch input with mouse emulation enabled can make BaseButton handle both the generated mouse event and the original touch event, causing pressed to emit twice.

This marks mouse events generated from touch and makes BaseButton ignore them, including the ui_accept path.

Regular emulated mouse events still work. Regression tests were added for the duplicate press case and the emulated mouse boundary.

[AThousandShips]

Did you use AI tools to write any part of this code or the description for the PR? If so you need to disclose this, and significant amounts of generated code is not allowed

[KurodaKayn]

Did you use AI tools to write any part of this code or the description for the PR? If so you need to disclose this, and significant amounts of generated code is not allowed

Jesus, my bad—the PR title, PR description, and commit message were indeed written by AI.
However the code itself was written manually by me; I just had AI review it.

[AThousandShips]

Please rewrite the PR description, using AI to write it is not permitted as per our guidelines (see this)

[syntaxerror247]

The approach doesn't look good to me.

Why are you creating emulated_from_touch when there's already InputEvent::DEVICE_ID_EMULATION?

[KurodaKayn]

The approach doesn't look good to me.

Why are you creating emulated_from_touch when there's already InputEvent::DEVICE_ID_EMULATION?

I added emulated_from_touch because DEVICE_ID_EMULATION only says “synthetic”, not “made from touch”.

Goal was: ignore mouse events paired with real touch, but not block other emulated input. Broadly ignoring DEVICE_ID_EMULATION had caused regressions before (#119329).

Your point makes sense though. Extra flag not needed. I updated PR to use DEVICE_ID_EMULATION only in BaseButton mouse / motion / ui_accept paths, no early return.

[ace24713]

The approach doesn't look good to me.
Why are you creating emulated_from_touch when there's already InputEvent::DEVICE_ID_EMULATION?

I added emulated_from_touch because DEVICE_ID_EMULATION only says “synthetic”, not “made from touch”.

Goal was: ignore mouse events paired with real touch, but not block other emulated input. Broadly ignoring DEVICE_ID_EMULATION had caused regressions before (#119329).

Your point makes sense though. Extra flag not needed. I updated PR to use DEVICE_ID_EMULATION only in BaseButton mouse / motion / ui_accept paths, no early return.

Wait I don't understand how this doesn't cause a regression now, because it looks like you're just ignoring all emulated mouse events again. Is it not desirable to have user emulated mouse events trigger button presses? Because it looks like you added a test case to verify exactly the opposite of that.

[KurodaKayn]

Wait I don't understand how this doesn't cause a regression now, because it looks like you're just ignoring all emulated mouse events again. Is it not desirable to have user emulated mouse events trigger button presses? Because it looks like you added a test case to verify exactly the opposite of that.

Good catch. I replaced the broad DEVICE_ID_EMULATION check with a source-specific marker for mouse events generated from touch input, So regular emulated mouse events are allowed to trigger buttons again

[syntaxerror247]

I added emulated_from_touch because DEVICE_ID_EMULATION only says “synthetic”, not “made from touch”.

Yes, but when you combine it with input event type then it's the same.
for eg, mouse_button.is_valid() && p_event->get_device() != InputEvent::DEVICE_ID_EMULATION means it's an non-emulated mouse event.

Goal was: ignore mouse events paired with real touch, but not block other emulated input. Broadly ignoring DEVICE_ID_EMULATION had caused regressions before (#119329).

I'm not sure why simply returning for DEVICE_ID_EMULATION would cause any issue. Below logic look specific to mouse input so ignoring it for touch shouldn't be an issue. @Nintorch can you clarify here?

Edit: opened #120864, I haven't tested if it fixes the issue or not, but double handling of touch/drag event is indeed a problem here.

[KurodaKayn]

I added emulated_from_touch because DEVICE_ID_EMULATION only says “synthetic”, not “made from touch”.

Yes, but when you combine it with input event type then it's the same. for eg, mouse_button.is_valid() && p_event->get_device() != InputEvent::DEVICE_ID_EMULATION means it's an non-emulated mouse event.

Goal was: ignore mouse events paired with real touch, but not block other emulated input. Broadly ignoring DEVICE_ID_EMULATION had caused regressions before (#119329).

I'm not sure why simply returning for DEVICE_ID_EMULATION would cause any issue. Below logic look specific to mouse input so ignoring it for touch shouldn't be an issue. @Nintorch can you clarify here?

Edit: opened #120864, I haven't tested if it fixes the issue or not, but double handling of touch/drag event is indeed a problem here.

Thanks for checking.
I agree that mouse_button.is_valid() && device != DEVICE_ID_EMULATION identifies a non-emulated mouse event, but my concern is that DEVICE_ID_EMULATION mouse events are not necessarily only the mouse event paired with a real touch event.

The early return approach in the draft you opened(#120864) would also skip regular emulated mouse/button events before the mouse/ui_accept logic. That is the behavior covered by the added regression test: a DEVICE_ID_EMULATION mouse button mapped to ui_accept should still be able to press a Button.

The marker is only used for mouse events generated from emulate_mouse_from_touch, where the original ScreenTouch/ScreenDrag event is handled separately. If the intended behavior is now to ignore all emulated mouse events in BaseButton, I can simplify the PR, but that would intentionally drop this regular-emulated-mouse case.

[KurodaKayn]

I added emulated_from_touch because DEVICE_ID_EMULATION only says “synthetic”, not “made from touch”.

Yes, but when you combine it with input event type then it's the same. for eg, mouse_button.is_valid() && p_event->get_device() != InputEvent::DEVICE_ID_EMULATION means it's an non-emulated mouse event.

Goal was: ignore mouse events paired with real touch, but not block other emulated input. Broadly ignoring DEVICE_ID_EMULATION had caused regressions before (#119329).

I'm not sure why simply returning for DEVICE_ID_EMULATION would cause any issue. Below logic look specific to mouse input so ignoring it for touch shouldn't be an issue. @Nintorch can you clarify here?

Edit: opened #120864, I haven't tested if it fixes the issue or not, but double handling of touch/drag event is indeed a problem here.

Your draft solves this problem and is clearer than mine. However, it makes mouse and ui_accept of DEVICE_ID_EMULATION be skipped, too. It's not necessary, right?

So that's why I created emulated_from_touch.

• DEVICE_ID_EMULATION + emulated_from_touch == true:
  Godot generates corresponding mouse events from touch input, which should be ignored to avoid duplication.
• DEVICE_ID_EMULATION + emulated_from_touch == false:
  Other emulated mouse events should still be handled as mouse/ui_accept.

[syntaxerror247]

superseded by #120864