FIX: inactive touch driving invalid synthetic click on state reset

[MorganHoarau]

Description

Addresses UUM-100125 (phantom click after device rotation).

Follow up PR about a UITKInputModule failing test locally: #2367

Issue:

• After rotation, InputSystem queue a device configuration event which causes InputActionState to run initial-state catch-up in OnBeforeInitialUpdate. (See Comments To Reviewers section for overview)
• Touch position state can still be non-default after touch end (because position is dontReset = true). So the stale state (touch position being non-default) was treated as fresh actuation and trigger unexpected callbacks.

Changes

• Added a guard in initial-state check to skip replay for inactive touch state.
• Added regression test: Actions_InitialStateCheckAfterConfigurationChange_DoesNotTriggerForInactiveTouch
• Updated existing test Actions_WithMultipleCompositeBindings_WithoutEvaluateMagnitude_Works(true) to align the new expected behaviour. The test still preserves the original ISXB-98 guarantee: after enabling, real subsequent touches must still trigger the composite action correctly.

TODO:

• Check existing documentation around the topic (Touch, device configuration event, dontReset) - need update?

Also, unlike both Button and PassThrough actions, Value actions perform what's called "initial state check" on the first input update after the action was enabled. What this does is check controls bound to the action and if they are already actuated (that is, at non-default value), the action will immediately be started and performed. What this means in practice is that when a value action is bound to, say, the left stick on a gamepad and the stick is already moved out of its resting position, then the action will immediately trigger instead of first requiring the stick to be moved slightly.
From InputActionType documentation.

• Changelog

Testing status & QA

• Reproduced issue in repro project.
• Added new regression test for this bug path.
• Ran existing tests and identified conflict with: Actions_WithMultipleCompositeBindings_WithoutEvaluateMagnitude_Works(true)

Overall Product Risks

• Complexity: Low
• Halo Effect: Medium - affects TouchControl that supports initial state check.

Comments to reviewers

Tracing the whole flow can help understand the broader context. DeviceConfigurationEvent flow diagram:

flowchart TD
    subgraph Native["Native / Backend"]
        A[Device rotates]
        B[Queue DeviceConfigurationEvent]
    end

    subgraph IM["InputManager"]
        C[Process DeviceConfigurationEvent]
        D[NotifyConfigurationChanged]
        E[InputActionState.OnDeviceChange ConfigurationChanged]
    end

    subgraph IASResolve["InputActionState.Resolve phase"]
        F[Full binding re-resolve]
        G[Set initialStateCheckPending = true]
    end

    subgraph IASInitial["InputActionState.OnBeforeInitialUpdate"]
        H[Loop bindings and controls]
        I{Control belongs to TouchControl?}
        J{touch.isInProgress?}
        K[Default CheckStateIsAtDefault path]
    end

    subgraph OldPath["Old behavior"]
        L[SignalStateChangeMonitor for inactive touch]
        M[Started/Performed from stale touch state]
        N[Phantom UI click]
    end

    subgraph NewPath["Current proposed fix"]
        O[Skip inactive touch control]
        P[No synthetic Started/Performed]
        Q[No phantom UI click]
    end

    A --> B
    B --> C
    C --> D
    D --> E
    E --> F
    F --> G
    G --> H
    H --> I
    I -- no --> K
    I -- yes --> J
    J -- yes --> K
    J -- no old --> L
    L --> M
    M --> N
    J -- no new --> O
    O --> P
    P --> Q

Loading

Checklist

Before review:

• Changelog entry added.
  • Explains the change in Changed, Fixed, Added sections.
  • For API change contains an example snippet and/or migration example.
  • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
  • Jira port for the next release set as "Resolved".
• Tests added/changed, if applicable.
  • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
  • Performance tests.
  • Integration tests.
• Docs for new/changed API's.
  • Xmldoc cross references are set correctly.
  • Added explanation how the API works.
  • Usage code examples added.
  • The manual is updated, if needed.

During merge:

• Commit message for squash-merge is prefixed with one of the list:
  • NEW: ___.
  • FIX: ___.
  • DOCS: ___.
  • CHANGE: ___.
  • RELEASE: 1.1.0-preview.3.

[u-pr]

PR Reviewer Guide 🔍

(Review updated until commit ea8b067)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪ Moderate effort: small but behavior-changing runtime logic in action initial-state processing plus new/updated tests that need careful validation for determinism and intended semantics.

🏅 Score: 88 Solid, targeted fix with regression coverage, but the change alters initial-state behavior for touch bindings and one new test assertion looks potentially brittle across internal action-state variations.

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Robustness The new regression test assumes exactly one trace event and that it is always Canceled after a configuration change; depending on whether the re-resolve actually results in a cancel for this action in a given runtime/path, this may be flaky (could be zero events or additional events). Consider asserting the absence of Started/Performed rather than requiring an exact count/phase, or otherwise tightening the setup to guarantee a cancel. // Configuration change causes full re-resolve and schedules initial state check. InputSystem.QueueConfigChangeEvent(touchscreen); InputSystem.Update(); InputSystem.Update(); // Full re-resolve may cancel the current action state. What must NOT happen is a synthetic // Started/Performed pair from persisted inactive touch state. Assert.AreEqual(1, trace.count); foreach (var eventPtr in trace) { // The trace should only contain a Canceled event for the action. Assert.AreEqual(InputActionPhase.Canceled, eventPtr.phase, $"inactive touch state should not produce action callbacks, but received {eventPtr.phase}."); } Behavior Change Skipping initial-state catch-up for any control under a TouchControl when isInProgress is false prevents replay from persisted dontReset state (desired), but it also changes semantics for value bindings that may intentionally want “last known touch position/touchId” even when not in progress; confirm this is acceptable for all touch sub-controls and doesn’t break existing user expectations beyond the intended fix. if (ShouldSkipInitialStateCheck(control)) continue; if (!control.CheckStateIsAtDefault()) { // Update press times. if (control.IsValueConsideredPressed(control.magnitude)) { // ReSharper disable once CompareOfFloatsByEqualityOperator if (bindingState.pressTime == default || bindingState.pressTime > time) bindingState.pressTime = time; } // For composites, any one actuated control will lead to the composite being // processed as a whole so we can stop here. This also ensures that we are // not triggering the composite repeatedly if there are multiple actuated // controls bound to its parts. if (isComposite && didFindControlToSignal) continue; manager.SignalStateChangeMonitor(control, this); didFindControlToSignal = true; } } } manager.FireStateChangeNotifications(); k_InputInitialActionStateCheckMarker.End(); } private static bool ShouldSkipInitialStateCheck(InputControl control) { // UUM-100125 // Touch controls intentionally preserve state such as position even when no touch is currently active. // During binding re-resolution this can make inactive touches look actuated and cause invalid triggers. for (var current = control; current != null; current = current.parent) { if (current is TouchControl touchControl) { return !touchControl.isInProgress; } } return false; } Performance/Scope The parent-walk in ShouldSkipInitialStateCheck runs for every bound control during initial-state checks; it’s likely fine given the frequency, but validate it doesn’t become a hot spot with many bindings/controls, and consider a cheaper predicate (e.g., fast-path for control.device is Touchscreen) if needed. private static bool ShouldSkipInitialStateCheck(InputControl control) { // UUM-100125 // Touch controls intentionally preserve state such as position even when no touch is currently active. // During binding re-resolution this can make inactive touches look actuated and cause invalid triggers. for (var current = control; current != null; current = current.parent) { if (current is TouchControl touchControl) { return !touchControl.isInProgress; } } return false; }

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[u-pr]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Skip check for nested touch children This only checks control.parent, so bindings to deeper children (e.g. /primaryTouch/position/x) can still trigger from persisted inactive touch state. Walk up the full parent chain until a TouchControl is found, and base the skip decision on that. Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs [1354-1370] private static bool ShouldSkipInitialStateCheck(InputControl control) { // UUM-100125 // Touch controls intentionally preserve state such as position even when no touch is currently active. // During binding re-resolution this can make inactive touches look actuated and cause invalid triggers. - if (control is TouchControl touchControl) + for (var current = control; current != null; current = current.parent) { - return !touchControl.isInProgress; - } - - if (control.parent is TouchControl parentTouchControl) - { - return !parentTouchControl.isInProgress; + if (current is TouchControl touchControl) + return !touchControl.isInProgress; } return false; } Suggestion importance[1-10]: 8 __ Why: The current implementation only checks the control and its direct parent for a TouchControl. For bindings to nested controls, such as <Touchscreen>/primaryTouch/position/x, the check would fail to find the TouchControl and incorrectly skip the suppression logic.	Medium
More suggestions

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[codecov-github-com]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@             Coverage Diff             @@
##           develop    #2349      +/-   ##
===========================================
- Coverage    77.90%   77.90%   -0.01%     
===========================================
  Files          476      481       +5     
  Lines        97613    97724     +111     
===========================================
+ Hits         76048    76131      +83     
- Misses       21565    21593      +28     

Flag	Coverage Δ
inputsystem_MacOS_2022.3	5.50% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_2022.3_project	75.42% <100.00%> (+0.02%)	⬆️
inputsystem_MacOS_6000.0	5.27% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_6000.0_project	77.32% <100.00%> (+0.02%)	⬆️
inputsystem_MacOS_6000.3	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_6000.3_project	77.32% <100.00%> (+0.02%)	⬆️
inputsystem_MacOS_6000.4	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_6000.4_project	77.33% <100.00%> (+0.02%)	⬆️
inputsystem_MacOS_6000.5	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_6000.5_project	77.32% <100.00%> (+0.02%)	⬆️
inputsystem_MacOS_6000.6	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_MacOS_6000.6_project	77.32% <100.00%> (+0.01%)	⬆️
inputsystem_Ubuntu_2022.3_project	75.21% <100.00%> (+0.02%)	⬆️
inputsystem_Ubuntu_6000.0	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Ubuntu_6000.0_project	77.12% <100.00%> (+0.02%)	⬆️
inputsystem_Ubuntu_6000.3	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Ubuntu_6000.3_project	77.12% <100.00%> (+0.01%)	⬆️
inputsystem_Ubuntu_6000.4	5.29% <0.00%> (-0.04%)	⬇️
inputsystem_Ubuntu_6000.4_project	77.13% <100.00%> (+0.02%)	⬆️
inputsystem_Ubuntu_6000.5	5.29% <0.00%> (-0.04%)	⬇️
inputsystem_Ubuntu_6000.5_project	77.13% <100.00%> (+0.01%)	⬆️
inputsystem_Ubuntu_6000.6	5.29% <0.00%> (-0.04%)	⬇️
inputsystem_Ubuntu_6000.6_project	77.12% <100.00%> (+0.02%)	⬆️
inputsystem_Windows_2022.3	5.50% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_2022.3_project	75.54% <100.00%> (+0.01%)	⬆️
inputsystem_Windows_6000.0	5.27% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_6000.0_project	77.44% <100.00%> (+0.02%)	⬆️
inputsystem_Windows_6000.3	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_6000.3_project	77.44% <100.00%> (+0.02%)	⬆️
inputsystem_Windows_6000.4	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_6000.4_project	77.45% <100.00%> (+0.02%)	⬆️
inputsystem_Windows_6000.5	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_6000.5_project	77.45% <100.00%> (+0.02%)	⬆️
inputsystem_Windows_6000.6	5.28% <0.00%> (-0.03%)	⬇️
inputsystem_Windows_6000.6_project	77.45% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
Assets/Tests/InputSystem/CoreTests_Actions.cs	98.41% <100.00%> (+0.26%)	⬆️
...nputsystem/InputSystem/Actions/InputActionState.cs	92.62% <100.00%> (-0.38%)	⬇️

... and 5 files with indirect coverage changes

[u-pr]

Persistent review updated to latest commit ea8b067

[u-pr]

PR Code Suggestions ✨

No code suggestions found for the PR.

[ekcoh]

I think this fix is cleanly executed and really like that tests have been added for this specific scenario. I still wonder if a touch-specific solution is really needed, but see no reason to not land this now. We can always revisit this together with later interaction robustness improvements if needed since all is internal. Good job on sorting this out @MorganHoarau!