Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Events.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,34 @@ public void Events_OnAnyButtonPressed_FiltersOutOtherControls()

Assert.That(callCount, Is.EqualTo(1));
}

[Test]
[Category("Events")]
public void Events_OnAnyButtonPressed_WorksWithTouchControls()
{
InputSystem.settings.defaultButtonPressPoint = 0.5f;

var touch = InputSystem.AddDevice<Touchscreen>();

var callCount = 0;

InputSystem.onAnyButtonPress
.Call(ctrl =>
{
Assert.That(ctrl, Is.SameAs(touch.touches[0].press));
++callCount;
});

Assert.That(callCount, Is.Zero);

InputSystem.Update();

SetTouch(0,TouchPhase.Began, new Vector2(12,12));

InputSystem.Update();

Assert.That(callCount, Is.EqualTo(1));
}

[Test]
[Category("Events")]
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fixed `buttonSouth` returning the state of the east button (and so on for all the compass named buttons) when using a Nintendo Switch Pro Controller on iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632)
- Fixed `aButton` returning the state of the east button (and so on for all the letter named buttons) when using a Nintendo Switch Pro Controller on Standalone & iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632)
- Fixed an issue where `UIToolkit` `ClickEvent` could be fired on Android after device rotation due to inactive touch state being replayed during action initial state checks [UUM-100125](https://jira.unity3d.com/browse/UUM-100125).
- Fixed InputSystem.onAnyButtonPress fails to trigger when the device receives a touch [UUM-137930](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-137930).

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@ public static InputControl GetFirstButtonPressOrNull(this InputEventPtr eventPtr

foreach (var control in eventPtr.EnumerateControls(Enumerate.IgnoreControlsInDefaultState, magnitudeThreshold: magnitude))
{
if (!control.HasValueChangeInEvent(eventPtr))
if (!control.HasValueChangeInEvent(eventPtr) && !(control.device is Touchscreen)) // touches can happen to not have a previous state to compare to after the touch started
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high
The condition !(control.device is Touchscreen) bypasses the value change check for all touchscreen controls. This causes GetFirstButtonPressOrNull to return a control even if its state hasn't changed in the current event (e.g., during a touch move event where the finger is already down).

Since InputSystem.onAnyButtonPress triggers based on the return value of this method, it will now fire repeatedly for every touch move event as long as the finger is held down, rather than only on the initial press. This breaks the expected behavior of a "press" event and differs from how other devices (like Gamepads or Keyboards) behave.

If the goal is to handle the case where HasValueChangeInEvent fails due to missing previous state, consider checking if the control is specifically transitioning to a pressed state or if the event represents a TouchPhase.Began equivalent.

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium
Hardcoding a specific device type check (is Touchscreen) in a generic extension method creates tight coupling. This approach also requires updating this method if other devices (e.g., Pens or specialized VR controllers) exhibit similar state-handling characteristics.

Have you considered moving this logic into HasValueChangeInEvent itself? Since the root issue is that touches may not reliably report a value change via memory comparison when starting, fixing it at the source would ensure consistent behavior for all consumers of the HasValueChangeInEvent API and keep this extension method device-agnostic.

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium
The type check control.device is Touchscreen is performed for every control in the loop where HasValueChangeInEvent is false. Since all controls in a state event belong to the same device, this check can be moved outside the loop for better performance.

Additionally, bypassing the HasValueChangeInEvent filter for all touchscreen controls means that during every move event, every non-default control on the touchscreen (including other active touches) will undergo magnitude and button checks. In multi-touch scenarios, this could introduce unnecessary overhead.

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low
The comment could be slightly more descriptive to explain why this check is necessary.

Suggested revision: // Touchscreen events may not have a previous state available for comparison (e.g., when a touch just started), making memory-based change detection unreliable.

🤖 Helpful? 👍/👎

continue;
if (buttonControlsOnly && !control.isButton)
continue;
Expand Down
Loading