FIX: (ISXB-1524) Rebinding issues related to incorrect event suppression (WIP)

[ekcoh]

Description

Fix originates from users reporting issues with rebinding functionality and differences between Xbox gamepad and DualShock/DualSense. The problem surfaces when a control, e.g. button is targeted in rebinding that is already associated with an action doing something else outside the context of rebinding, but generally unintentional triggering of actions in relation to the rebinding process.

According to support engineers and customer, improved rebinding UI sample (commit which was improved to address this issue works well with Xbox gamepad but not with Sony gamepads. This have been verified on Windows. On macOS also Xbox gamepads have issues. It turns out the difference between Xbox and DualShock/DualSense has nothing to do with the gamepads but is a function of the underlying backend behaviour and/or filtering behaviour since lack of filtering will generate new samples solely based on analog control noise and repeat button states effectively bypassing current event suppression mechanism. This is also true if periodic state readings are being made without change filtering.

The problem The existing InputEvent.handled flags marks an event as being "handled" which blocks event/state propagation. This is problematic if the associated InputEvent is not representing an event but a state reading. The reason for this can be shown by the example below. Consider an initial state where a button is in state 0 (not pressed), this corresponds to some event 'a'. A press event 'b' changes the state of the button to 1, but this event is for some reason suppressed, for example during rebinding. This blocks the event from propagating as a state change which is according to documentation of InputEventPtr.handled. This leads to "event" 'c' which is just a periodic reading that doesn't reflect a change to be seen as a change since 'b' was suppressed but 'c' wasn't suppressed. This implies that event 'c' would evaluate to a "press" since it is a state change from the perspective of the action state, but not from the state of the button. This is illustrated in figure below.

-0----1------1----0----------------> time (Button state in reading)
-0----0------1----0----------------> time (Action state)
-a----b------c----d----------------> time (Events)
------s----------------------------> suppression
   
      ^      ^
      |      |
      |      Logical press detection happens here due to suppression
      |
      Actual press happens here

If suppression instead had been allowed to propagate to update state, but not fire notifications outside internal data representation, the scenario would have been different since event/reading 'c' would not imply a state change:

-0----1------1----0----------------> time (Button state in reading)
-0----1------1----0----------------> time (Action state)
-a----b------c----d----------------> time (Events)
------s----------------------------> suppression
   
      ^
      |
      |
      |
      Actual press happens here but notification is suppressed

Previous possible work around (Should not be used - will not work)

A slight improvement was suggested based on OnMatchWaitForAnother(x), which seems to solve the issue if time x is large enough (larger than a frame cycle). The reason this suggestion seem to work is due to input buffers being emptied/swapped on frame boundaries, typically leading to release by user before timeout. Since this functionality is time-based it do not provide a reliable solution and this workaround should not be used in production code. This is easily seen by. using this function on the branch and pressing a button in rebinding UI and never releasing it. As soon as suppression stops, the action bound to the rebind target will still fire.

Proposed solution

It's currently assumed that event suppression logic in Input System is incorrect and hence it is being investigated as part of this PR. Essentially event suppression should only suppress events from firing but still allow state propagation updates since otherwise it is just postponing/deferring the problem. It's difficult to change this behavior without risking changing behaviour with side-effects in existing projects. Hence the current working assumption is that this need to be a setting and/or runtime property of the system to allow switching modes. Such a solution could temporarily switch mode during rebinding to solve the reported problem. This have been implemented as InputSystem.inputEventHandledPolicy property to control how the system processes handled events and the RebindingUI sample have been updated to use this via the new fluent-API extensions WithSuppressedActionPropagation().

It has been proven that uncommenting suppression early-exit within InputManager.OnUpdate eliminates an action callback when e.g. holding a button during rebinding, even when using OnMatchWaitForAnother(x). This indicates that we need to allow interaction FSMs to update based on state changes but their output should be suppressed. This implies we either need to modify behavior of current suppress flag or introduce yet another suppress mechanism.

Open questions to reviewers

Should we change behavior of event suppression during rebinding or may it have side-effects? Current changes do not do this but instead allow users to opt-in via fluent API or by changing property on InputSystem proxy API.

Testing status & QA

The current fix has been tested on macOS and Windows 10 and Window 11 using Xbox gamepad, DualShock gamepad and DualSense gamepad.

Overall Product Risks

• Complexity: Medium
• Halo Effect: Large - Event suppression is central and will affect multiple features: rebinding, shortcuts, etc.

Comments to reviewers

Main effort should be on solving customer issue but also to look for regressed behavior due to changer in event suppression logic.

The easiest way I have found to test this is as follows:

1. Create a debug script to get observability of phases on some action, e.g.

// ActionDebug.cs
using UnityEngine;
using UnityEngine.InputSystem;

public class ActionDebug : MonoBehaviour
{
    public InputActionReference trigger;

    // Start is called once before the first execution of Update after the MonoBehaviour is created
    void Start()
    {
        trigger.action.performed += ActionOnPerformed;
        trigger.action.canceled += ActionOnCanceled;
        trigger.action.started += ActionOnStarted;
        trigger.action.Enable();
    }

    private void ActionOnStarted(InputAction.CallbackContext obj)
    {
        Debug.Log("Action Started");
    }

    private void ActionOnCanceled(InputAction.CallbackContext obj)
    {
        Debug.Log("Action Canceled");
    }

    private void ActionOnPerformed(InputAction.CallbackContext obj)
    {
        Debug.Log("Action Performed");
    }

    // Update is called once per frame
    void Update()
    {
        if (trigger.action.WasPerformedThisFrame())
            Debug.Log("Action Performed (Polled Event)");
    }
}

2. Update the input action asset in the RebindingUI sample to have an action "Test" that triggers on some gamepad button, e.g. Triangle on a DualSense. Add the script from step (1) to some game object within the Rebinding UI scene.

3. Run the Rebinding UI sample and rebind a button action to the same button as bound to Test action.

Observe behavior and compare to previous behavior. It's critical to test with both polled and event based gamepads, e.g. Xbox + Dualsense/Dualshock on Windows or any of them on macOS.

Checklist

Before review:

• [x] 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 %<ID>%). If it is a private issue, just add the case ID without a link.
  • Jira port for the next release set as "Resolved".
• [x] Tests added/changed, if applicable.
  • Functional tests EventHandledPolicy_ShouldReflectUserSetting, Events_ShouldRespectHandledPolicyUponUpdate.
  • Performance tests.
  • Integration tests.
• [x] 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.

After merge:

• [ ] Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

[ekcoh]

Tried running tests with this setting on all the time yields failures on: CoreTests.Actions_InteractiveRebinding_CanSuppressEventsWhileListening (Not part of PR but very related to fix) CoreTests.EventHandledPolicy_ShouldReflectUserSetting (Part of PR) CoreTests.Events_ShouldRespectHandledPolicyUponUpdate (Part of PR)

[codecov-github-com[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

@@             Coverage Diff             @@
##           develop    #2168      +/-   ##
===========================================
+ Coverage    68.10%   68.15%   +0.04%     
===========================================
  Files          367      367              
  Lines        53638    53662      +24     
===========================================
+ Hits         36529    36571      +42     
+ Misses       17109    17091      -18     

Flag	Coverage Δ
linux_2021.3_pkg	5.45% <0.00%> (-0.01%)	:arrow_down:
linux_2021.3_project	70.39% <100.00%> (+0.02%)	:arrow_up:
linux_2022.3_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
linux_2022.3_project	65.25% <100.00%> (+0.03%)	:arrow_up:
linux_6000.0_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
linux_6000.0_project	67.99% <100.00%> (-0.01%)	:arrow_down:
linux_6000.1_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
linux_6000.1_project	68.03% <100.00%> (+0.06%)	:arrow_up:
linux_6000.2_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
linux_6000.2_project	68.05% <100.00%> (+0.04%)	:arrow_up:
linux_trunk_pkg	5.24% <0.00%> (-0.01%)	:arrow_down:
linux_trunk_project	67.99% <100.00%> (-0.01%)	:arrow_down:
mac_2021.3_pkg	5.45% <0.00%> (-0.01%)	:arrow_down:
mac_2021.3_project	70.40% <100.00%> (+0.02%)	:arrow_up:
mac_2022.3_pkg	5.22% <0.00%> (-0.01%)	:arrow_down:
mac_2022.3_project	65.26% <100.00%> (+0.02%)	:arrow_up:
mac_6000.0_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
mac_6000.0_project	68.06% <100.00%> (+0.02%)	:arrow_up:
mac_6000.1_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
mac_6000.1_project	68.06% <100.00%> (+0.02%)	:arrow_up:
mac_6000.2_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
mac_6000.2_project	68.05% <100.00%> (+0.02%)	:arrow_up:
mac_trunk_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
mac_trunk_project	68.05% <100.00%> (+0.02%)	:arrow_up:
win_2021.3_pkg	5.45% <0.00%> (-0.01%)	:arrow_down:
win_2021.3_project	70.48% <100.00%> (+0.02%)	:arrow_up:
win_2022.3_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
win_2022.3_project	65.34% <100.00%> (+0.02%)	:arrow_up:
win_6000.0_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
win_6000.0_project	68.13% <100.00%> (+0.02%)	:arrow_up:
win_6000.1_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
win_6000.1_project	68.13% <100.00%> (+0.02%)	:arrow_up:
win_6000.2_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
win_6000.2_project	68.13% <100.00%> (+0.02%)	:arrow_up:
win_trunk_pkg	5.23% <0.00%> (-0.01%)	:arrow_down:
win_trunk_project	68.13% <100.00%> (+0.02%)	:arrow_up:

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

Files with missing lines	Coverage Δ
...ity.inputsystem/InputSystem/Actions/InputAction.cs	92.34% <100.00%> (+0.21%)	:arrow_up:
...utSystem/Actions/InputActionRebindingExtensions.cs	82.96% <100.00%> (+0.61%)	:arrow_up:
...nputsystem/InputSystem/Actions/InputActionState.cs	92.85% <100.00%> (+0.52%)	:arrow_up:
.../com.unity.inputsystem/InputSystem/InputManager.cs	88.77% <100.00%> (+0.04%)	:arrow_up:
...putsystem/InputSystem/InputManagerStateMonitors.cs	95.56% <100.00%> (+0.01%)	:arrow_up:

... and 1 file with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ekcoh]

Observations that might need to be addressed:

• Update: A 10 seconds timeout is now part of the sample (but not visualised) rebinding UI sample would benefit from a timeout if nothing can be detected during a "wait"
• The rebinding UI sample would benefit from control swaps when assigning a control that is already assigned. This have been asked for in discussion/forum.

[Pauliusd01]

Looks great, still looking into it but just one thing I noticed immediately. It seems like other types of input, other than expected, are not blocked when rebinding. I can still navigate the UI using WASD when rebinding the Look Vector2

https://github.com/user-attachments/assets/16b7eaa3-268e-40de-8d6f-56711c205bf8

[ekcoh]

Looks great, still looking into it but just one thing I noticed immediately. It seems like other types of inputs, other than expected, are not blocked when rebinding. I can still navigate the UI using WASD when rebinding the Look Vector2

26.06.2025.-.Unity.205.mp4

Thanks, I believe I noticed this myself but forgot about it. I guess the change I reverted from Jak that disabled the UI map prevented that but I will see if there is a better solution that blocks all without requiring users to do anything. Basically just marking any input event handled during this phase should be enough, will revisit this aspect when back at work. Great to file any quirks you find like this and we can sort them out. I also listed a few above.

[ekcoh]

Another issue/quirk that I noticed that needs fixing is that when rebinding there will be double submit required to enter rebinding mode again if the bound control is not the submit control. This is likely since we enter rebinding with submit, action state is freezes with the submit action state "high", which means that it will not go "low" just by exiting rebinding wait mode. The next press on submit control will transition low to high which means it essentially doesn't change anything. But when released we go low again and then subsequent press will work since we detect low->high. This might call for a more involved fix in InputActionState.

[Pauliusd01]

Two minor things so far:

• There's a typo in the Canvas - RebindMenu - Keyboard - Help object, "somthing" -> "something"
• Shouldn't UI Navigate be disabled during gameplay? It is currently active with both menu on and off

[ekcoh]

Two minor things so far:

• There's a typo in the Canvas - RebindMenu - Keyboard - Help object, "somthing" -> "something"

Typo fixed in https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/4T3829dad4624d3fc51d13d4303467959de153916

• Shouldn't UI Navigate be disabled during gameplay?

It is currently active with both menu on and off It doesn't serve any purpose at the movement, but my intent was to keep it on to fish for issues. Also if we add in-game UI as part of extending this to on-screen touch controls later we want it on. However if you @Pauliusd01 prefer it's disabled now I can make that fix quickly?

Update: I made the change, they are now disabled. Also made it a setting on the RebindUIGameManager so it can easily be changed later. See https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/5d40eb455ac35c725eaf6c8b6263f2704c8bd0f7

[Pauliusd01]

Two minor things so far:

• There's a typo in the Canvas - RebindMenu - Keyboard - Help object, "somthing" -> "something"

Typo fixed in https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/4T3829dad4624d3fc51d13d4303467959de153916

• Shouldn't UI Navigate be disabled during gameplay?

It is currently active with both menu on and off It doesn't serve any purpose at the movement, but my intent was to keep it on to fish for issues. Also if we add in-game UI as part of extending this to on-screen touch controls later we want it on. However if you @Pauliusd01 prefer it's disabled now I can make that fix quickly?

Update: I made the change, they are now disabled. Also made it a setting on the RebindUIGameManager so it can easily be changed later. See 5d40eb4

I'm fine with your judgment just figured it was worth mentioning if It's supposed to represent what people want/should do

[ekcoh]

I agree, now its off so it makes sense given the current feature set.

[ritamerkl]

I think copilot found some good points

[Pauliusd01]

Only got one thing, when shooting and loading a scene while the bullet still exists an error is thrown. This can be reproduced easily by opening the QA Tools -> Add All Core Scenes... -> Open Core Scene Loader -> Entering play mode -> Switching to rebinding sample -> Shooting -> Press Esc while the bullet is still on the screen -> Return to the scene loader -> error is thrown

I would be fine to merge this though as this is supposed to be a standalone scene and it might even be a problem with my thrown together scene loader. All this affects is internal testing right now.

Also, I did not notice any UI scaling issues on Webgl that you mentioned so that's good too.

[ekcoh]

@Pauliusd01 Good catch on the problem relating to the pooled projectiles. Will look into it and fix it. I see no reason why we shouldn't fix that before merge.

[ekcoh]

I think copilot found some good points

Yes, will address those.

[ekcoh]

First I looked at the sample, and it's excellent. I think it is great to see when which action is fired in the top panel. I just have three minor remarks:

Good to hear, yeah the indicator UI is basically there to reveal any issues and an attempt to increase understanding of what is going on a bit.

• I do like the UI action panel on the top right, which shows which UI action was triggered, but could lead to confusion on how that relates to rebinding (at least for the user) - also during rebinding the UI actions are still fired- maybe it's worth to add a bit of clarity here (maybe by only showing the UI Actions during a rebinding process / active menu?) I am not sure I understand what you consider confusing? UI actions during rebinding, e.g. a rebinding UI is support (since previously as well according to docs) and hence I also added the cancel button to the sample which allows UI driven action to cancel and show how to achieve this. If UI actions would be disabled during rebinding, that cancel button wouldn't work.

The reason the actions are always visible is to indicate that they only exist. Only their colours change to reflect their current state. Any suggestion on how to make things clearer? Missing descriptions from sample README maybe?

• the descriptive texts on the left and right are strictly seen not necessary IMO, since the example is pretty self-explaining

I reduced the original sample texts and kept what is still missing. I would suggest we delete them when we have addressed the following:

• Made sure you can actually rebind delta, this is currently not working.
• Add a "swap button" as suggested by the RHS text since we do not show how to do this.

• the look with Mouse works slow for me, maybe great to speed it up to make it a bit easier to handle

This is interesting since I tried it with 3 mice on macOS and Windows and attempted to find a decent middle ground. I believe we should make another PR adding mouse sensitivity setting to this sample for completeness and also showing why you need it (your comment is basically, the game doesn't feel good since there is no mouse sensitivity setting and delta is not consistent across OS and devices and user settings. Maybe its because I have my OS sensitivity close to max....

[ekcoh]

Pull Request Overview

This PR adds a new input-event suppression policy to allow state updates while still suppressing action callbacks during rebinding, and it overhauls the rebinding sample to better mimic real-game UI with game/menu modes and new helper components.

• Introduces InputEventHandledPolicy in InputManager and propagates it through the event loop and action state.
• Adds the fluent API WithActionEventNotificationsBeingSuppressed() and updates the sample to opt into it.
• Reworks the Rebinding UI sample: version bump, game/menu modes, dynamic labels, cancel buttons, timeout info, and new helper scripts.

Comments suppressed due to low confidence (4) Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs:374

• Public C# properties should use PascalCase. Rename isSuppressed to IsSuppressed for consistency with .NET naming conventions.

        public bool isSuppressed => m_Suppressed;

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs:1560

• [nitpick] The sentence is awkward and redundant. Consider rephrasing to "Note that not all input should necessarily be suppressed."

            /// Note that all input shouldn't necessarily should be suppressed. For example, it can be desirable to have UI that

Assets/Samples/RebindingUI/RebindActionUI.cs:199

• The variable bindingId is undefined in this scope (renamed to id). Update the log to reference id instead or reintroduce bindingId.

                Debug.LogError($"Cannot find binding with ID '{bindingId}' on '{action}'", this);

Assets/Samples/RebindingUI/RebindSaveLoad.cs:1

• Removed the using UnityEngine; and using UnityEngine.InputSystem; directives, causing MonoBehaviour and PlayerPrefs to be unresolved. Reintroduce the necessary using statements.

namespace UnityEngine.InputSystem.Samples.RebindUI

All of these comments have been addressed in commit https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/49eb6c8bfcc15ec350c627c84aa68b0e67088576

[ekcoh]

I think copilot found some good points

Addressed in https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/49eb6c8bfcc15ec350c627c84aa68b0e67088576

[ekcoh]

Only got one thing, when shooting and loading a scene while the bullet still exists an error is thrown. This can be reproduced easily by opening the QA Tools -> Add All Core Scenes... -> Open Core Scene Loader -> Entering play mode -> Switching to rebinding sample -> Shooting -> Press Esc while the bullet is still on the screen -> Return to the scene loader -> error is thrown

I would be fine to merge this though as this is supposed to be a standalone scene and it might even be a problem with my thrown together scene loader. All this affects is internal testing right now.

Also, I did not notice any UI scaling issues on Webgl that you mentioned so that's good too.

Not able to reproduce this currently on macOS, let me know if more information is available regarding stack trace or ways to reproduce.

[ritamerkl]

The reason the actions are always visible is to indicate that they only exist. Only their colours change to reflect their current state. Any suggestion on how to make things clearer? Missing descriptions from sample README maybe?

We discussed this in person, but a quick summary: it would be good to have the action indicators always in the front (top layer UI). Otherwise not suppressing the UI Actions makes a lot of sense to enable UI interactions also during the rebinding process. This example makes the usage pretty clear with the cancel button.

I will approve now, but it would be still good to get this small UI change in before landing.

[ekcoh]

The reason the actions are always visible is to indicate that they only exist. Only their colours change to reflect their current state. Any suggestion on how to make things clearer? Missing descriptions from sample README maybe?

We discussed this in person, but a quick summary: it would be good to have the action indicators always in the front (top layer UI). Otherwise not suppressing the UI Actions makes a lot of sense to enable UI interactions also during the rebinding process. This example makes the usage pretty clear with the cancel button.

I will approve now, but it would be still good to get this small UI change in before landing.

I addressed this by just making indicators be in front of other UI in the Z-order: https://github.com/Unity-Technologies/InputSystem/pull/2168/commits/c9043baaab4c51b6182cb56b48801abfa7bc8ca1

[ekcoh]

I had a look and the fixes look good to me. Also nice to have test coverage for it.

However, I think we could take a different approach with the sample, but I'll poke you @ekcoh on Slack

I interpret this as something not relevant for the PR then?

[ekcoh]

After discussing with @ekcoh, I'm approving this PR.

The main concern I had was with the goal and maintainability aspect of the code inside the Game folder. And also with the purpose of such a big scene. I was under the impression this was going to only be used for the rebinding scene.

However, going forward, we should reuse the Game folder code (which builds most of the scene) in other scenes, instead of reinventing the wheel for each sample and having scenes that don't share any code and just "spin a cube".

We agreed to move the "Game" folder out of the RebindingUI sample scene so that its intent is a little bit clearer to be reused by other Samples.

We also agreed to reuse this "main scene" to add something like Local Multiplayer logic, as part of some Multiplayer Sample scene, in the future. Ideally, we could potentially replace the existing scenes and centralize them using the Game code.

I have also filed an internal work ticket on doing this refactor after this has landed together with some additional improvements that can be made such as, switch stick button, delta control rebinding, rate-adoption fixes for haptic and light output etc.

[ekcoh]

Intend to land this without the fix for the QA-tool only issue that @Pauliusd01 found. Will file that as a separate internal ticket instead to be fixed after this has landed. There is no currently known way to trigger that issue when using the sample as a regular Unity scene.

[windxu88]

@ekcoh You can merge your PR if you want, the remaining job is for 6000.1 which is EOL now.

Edit: All jobs have passed.