-
Notifications
You must be signed in to change notification settings - Fork 324
NEW: (ISXB-1524, ISX-2325, revisiting ISXB-1396) Fix for rebinding issues related to incorrect event suppression + Changed Rebinding Sample #2168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Tried running tests with this setting on all the time yields failures on: |
…nologies/InputSystem into ekcoh/rebinding-issues-events
Observations that might need to be addressed:
|
…. failing to press a valid binding control
…nologies/InputSystem into ekcoh/rebinding-issues-events
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 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. |
…g when the rebinding screen has no UI. Added missing exception checks for attempting to reconfigure rebinding during rebinding.
…nologies/InputSystem into ekcoh/rebinding-issues-events
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. |
…t might not be needed. Added proper UI disable state to RebindMenu since RebindOverlay is essentially another "window" grabbing input context. Hence the sample should behave like this since its the most correct way instead of suppressing UI input.
…ing overlay with a gamepad
Description
Fix originates from users reporting issues with rebinding functionality and differences between Xbox gamepad and
DualShock/DualSense (ISXB-1396 - which this PR basically undo changes from: https://github.com/Unity-Technologies/InputSystem/pull/2137/files). 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. This problem was filed as ISXB-1524. Additionally the current rebinding sample hides these issues due to poor visibility and little resemblance with rebinding menus in real games. This was filed as ISX-2325.
The problem
The existing
InputEvent.handled
(currently used to suppress events during "listening") 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 ofInputEventPtr.handled
. This leads to "event" 'c' which is just a periodic reading that doesn't reflect a change at all 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.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:
Previous possible work around (Should not be used - will not work)
A slight improvement was suggested to affected users based on
OnMatchWaitForAnother(x)
, which seems to solvethe 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 device agnostic solution and this workaround should not be used in production code. This is easily seen by using this function 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 which is what has been implemented on this PR. This have been implemented as
InputManager.inputEventHandledPolicy
(internal) property to control how the system processes handled events and the RebindingUI sample have been updated to use this via the new fluent-API extensionsWithSuppressedActionPropagation()
which is the only public API introduced by this PR at this point. This is now used in the sample which has also been reworked to give better visibility related to actions firing and now uses a "game mode" and a "menu mode" for mimic a real game better.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 usingOnMatchWaitForAnother(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. On this PR the rebinding suppress actions only while listening to avoid breaking existing games using the existing solution. Hence theWithSuppressedActionPropagation
is an opt-in.Note that this PR adds 1 new API and hence also updates minor version of the Input System to v1.15.0.
Updated rebinding sample
The rebinding sample has been updated and should be tested as part of this PR (It was required to properly investigate and fix this bug and sample code. Note that screenshots may not be up to date with final solution. Apart from using the new solution to avoid side-effects during rebinding the sample now also has a "game mode" allowing you to move, rotate (look), change color (interact) and change shape (use) to mimic trivial game like features. When pressing default ESC or Start an in-game menu is shown allowing you to do rebinding. Also note that UI colours and navigation links where fixed to allow running the sample without a mouse or keyboard present.
Rebinding sample game mode:

Rebinding sample rebinding menu mode:

Note that look (mouse delta) have been added for completeness even if a user may decide to not include it and there is typically no other delta controls available with common desktop equipment.
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. Additionally a rather complex parameterised test
Events_ShouldRespectHandledPolicyUponUpdateAndSuppressedPressTransition
have been added testing expected outcomes for bothInputEventSuppressPolicy.SuppressEvents
andInputEventSuppressPolicy.SuppressActions
for press and release interactions, default interactions as well as action polled event evaluations for press and release. Note that polled state events will not be affected by this setting.Overall Product Risks
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:
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.
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.
The above approach can be used on both previous and current PR.
Also note that for test/verification that is not comparative, the reworked rebinding samples is recommended to test this through.
Checklist
Before review:
Changed
,Fixed
,Added
sections.EventHandledPolicy_ShouldReflectUserSetting
,Events_ShouldRespectHandledPolicyUponUpdate
.During merge:
NEW: ___
.FIX: ___
.DOCS: ___
.CHANGE: ___
.RELEASE: 1.1.0-preview.3
.After merge: