Skip to content

FIX: handled events still triggering actions when switching PlayerInput device#2335

Closed
AswinRajGopal wants to merge 4 commits intoUnity-Technologies:developfrom
AswinRajGopal:isxb-1097/fix-handledevents-triggers
Closed

FIX: handled events still triggering actions when switching PlayerInput device#2335
AswinRajGopal wants to merge 4 commits intoUnity-Technologies:developfrom
AswinRajGopal:isxb-1097/fix-handledevents-triggers

Conversation

@AswinRajGopal
Copy link
Collaborator

@AswinRajGopal AswinRajGopal commented Jan 29, 2026

Description

Bug: https://issuetracker.unity3d.com/issues/setting-inputeventptr-dot-handled-to-true-does-not-prevent-actions-from-triggering-when-changing-devices
When InputUser.onUnpairedDeviceUsed marks eventPtr.handled = true, we expect no action to fire from that same
button press.
But on devices like DualSense, a single press can generate multiple input events. The handled flag only stops the
one event, and another event in the same press still triggers the action.

Root cause
The action system is driven by state change events. We can see different eventIds for the handled event vs the
action‑triggering event, so the action was coming from a separate input event.

Fix

  • While dispatching InputSystem.onEvent, stop calling remaining listeners once handled becomes true.
  • When an event is handled (policy = SuppressStateUpdates), temporarily suppress action processing for that device
    for the rest of the update (and next update).
  • In InputActionState, skip processing control changes if the device is suppressed.

Testing status & QA

Manually verified using repro project + dualsense controller.

Overall Product Risks

  • Complexity: Low
  • Halo Effect: Low

Comments to reviewers

  • Only applies when policy is SuppressStateUpdates and a listener explicitly sets handled = true.
  • Normal input flow remains unchanged.
  • This simply makes “handled” cover the entire physical press, even if it arrives as multiple events.

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.

@cla-assistant-unity
Copy link

cla-assistant-unity bot commented Jan 29, 2026

CLA assistant check
All committers have signed the CLA.

@u-pr
Copy link
Contributor

u-pr bot commented Jan 29, 2026

PR Reviewer Guide 🔍

(Review updated until commit b94a66b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

The PR modifies core event loop logic and input suppression mechanisms, requiring a careful check of the event processing flow and potential side effects on input timing and loss.
🏅 Score: 92

The changes effectively address the reported bug by introducing a targeted suppression mechanism for handled events. The logic is sound, side effects are minimized by the policy check, and a relevant reproduction test is included.
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unnecessary Type Casting

In ShouldSuppressActionsForDevice, m_SuppressActionsForDeviceUpdate is defined as uint[] and InputUpdate.s_UpdateStepCount is a uint. Casting these values to int before comparison is unnecessary. While the wrap-around arithmetic likely holds for both signed and unsigned integers in this context, maintaining uint is cleaner and avoids potential confusion regarding overflow behavior.

var lastUpdate = (int)m_SuppressActionsForDeviceUpdate[deviceIndex];
var currentUpdate = (int)InputUpdate.s_UpdateStepCount;
  • Update review

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

@u-pr
Copy link
Contributor

u-pr bot commented Jan 29, 2026

PR Code Suggestions ✨

Latest suggestions up to b94a66b
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ignore uninitialized zero values in check

The m_SuppressActionsForDeviceUpdate array entries default to 0. This causes
ShouldSuppressActionsForDevice to incorrectly return true for all uninitialized
entries during updates 0 and 1 (because 0 == 0 and 0 + 1 == 1), leading to incorrect
action suppression at startup. Treat 0 as an uninitialized value to avoid this
issue.

Packages/com.unity.inputsystem/InputSystem/InputManager.cs [3919-3921]

 var lastUpdate = (int)m_SuppressActionsForDeviceUpdate[deviceIndex];
+if (lastUpdate == 0)
+    return false;
+
 var currentUpdate = (int)InputUpdate.s_UpdateStepCount;
 return lastUpdate == currentUpdate || lastUpdate + 1 == currentUpdate;
Suggestion importance[1-10]: 7

__

Why: The default value of elements in m_SuppressActionsForDeviceUpdate is 0. At the start of the application (when s_UpdateStepCount is 0 or 1), this default value satisfies the suppression condition (0 == 0 or 0 + 1 == 1), causing actions from uninitialized devices to be suppressed incorrectly. Treating 0 as 'uninitialized' prevents this false positive.

Medium
  • More suggestions

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


Previous suggestions

Suggestions up to commit d1da337
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix unbalanced profiler marker calls

The marker.End() call is currently unreachable in the return false path and missing
in the return true path, which will cause profiler imbalances. Ensure marker.End()
is called before returning in both scenarios.

Packages/com.unity.inputsystem/InputSystem/Utilities/DelegateHelpers.cs [107-116]

             if (stopOnHandled && eventPtr.handled)
             {
                 callbacks.UnlockForChanges();
+                marker.End();
                 return true;
             }
         }
         callbacks.UnlockForChanges();
+        marker.End();
         return false;
-        marker.End();
     }
Suggestion importance[1-10]: 9

__

Why: The suggested fix addresses a critical issue where marker.End() is skipped when returning early, and the existing marker.End() call is unreachable code. This ensures proper profiler balancing.

High
General
Prevent incorrect action suppression on startup

The m_SuppressActionsForDeviceUpdate array initializes with zeros. If
s_UpdateStepCount is 0 or 1 at startup, this logic will mistakenly suppress actions
for all devices (since 0 or 0+1 will match). Ignore the default 0 value to prevent
this.

Packages/com.unity.inputsystem/InputSystem/InputManager.cs [3919-3921]

         var lastUpdate = (int)m_SuppressActionsForDeviceUpdate[deviceIndex];
+        if (lastUpdate == 0)
+            return false;
+
         var currentUpdate = (int)InputUpdate.s_UpdateStepCount;
         return lastUpdate == currentUpdate || lastUpdate + 1 == currentUpdate;
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a logic flaw where the default zero-initialization of the m_SuppressActionsForDeviceUpdate array could cause false positives for action suppression during the initial updates (steps 0 and 1).

Medium

@AswinRajGopal AswinRajGopal force-pushed the isxb-1097/fix-handledevents-triggers branch from d1da337 to b94a66b Compare January 30, 2026 09:05
@AswinRajGopal AswinRajGopal changed the title Fix handled events still triggering actions when switching PlayerInput device "FIX": handled events still triggering actions when switching PlayerInput device Jan 30, 2026
@AswinRajGopal AswinRajGopal changed the title "FIX": handled events still triggering actions when switching PlayerInput device FIX: handled events still triggering actions when switching PlayerInput device Jan 30, 2026
@AswinRajGopal
Copy link
Collaborator Author

/review

@u-pr
Copy link
Contributor

u-pr bot commented Feb 2, 2026

Persistent review updated to latest commit b94a66b

@AswinRajGopal AswinRajGopal marked this pull request as draft February 2, 2026 06:33
@AswinRajGopal AswinRajGopal marked this pull request as ready for review February 2, 2026 06:34
@u-pr
Copy link
Contributor

u-pr bot commented Feb 2, 2026

Persistent review updated to latest commit b94a66b

@AswinRajGopal
Copy link
Collaborator Author

Closing this fork. Created new PR from develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants