diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 34cfa9299d..9f266e9e81 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -3748,6 +3748,13 @@ public void Actions_ResettingDevice_CancelsOngoingActionsThatAreDrivenByIt() // does not cause the action to start back up. For pass-through actions, that is different // as *any* value change performs the action. So here, we see *both* a cancellation and then // immediately a performing of the action. + // + // ISXB-1097 DESIGN NOTE: Whether pass-through actions should emit Performed(0f) on reset is + // debatable. Emitting it is consistent with the pass-through contract ("any value + // change performs"). Suppressing it would be consistent with button/value actions and + // the idea that resets aren't real user input. If we decide to suppress, the synthetic + // reset event in ResetDevice should be explicitly marked as handled rather than + // relying on the old eventId=-1 sentinel side-effect (see InputManager.cs:1656). Assert.That(passThroughActionTrace, Canceled(passThroughAction).AndThen(Performed(passThroughAction, value: 0f))); } } diff --git a/Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs b/Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs index 2df0feaefa..5d89b4d3af 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs @@ -1135,22 +1135,34 @@ public void Actions_InteractiveRebinding_IfDeviceHasMultipleUsages_UsagesAreAppl } } - // It can be desirable to not let the event through that we're rebinding from. This, for example, prevents the event - // from triggering UI actions. Note, however, that it also prevents the state of the device from updating correctly. - // - // NOTE: Hopefully, when we have a system in place that allows coordinating event consumption between actions, we have - // have a more elegant solution at our hand for solving the problem here. - [Test] + // ISXB-1097: Verifies event suppression behavior during interactive rebinding under both + // event handled policies. Under SuppressStateUpdates, handled events are discarded entirely + // (device state not updated). Under SuppressActionEventNotifications, handled events still + // propagate state but suppress action-level notifications (e.g. WasPressedThisFrame). +#pragma warning disable CS0618 // Type or member is obsolete + [TestCase(InputEventHandledPolicy.SuppressStateUpdates)] +#pragma warning restore CS0618 // Type or member is obsolete + [TestCase(InputEventHandledPolicy.SuppressActionEventNotifications)] [Category("Actions")] - public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening() + public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening(InputEventHandledPolicy policy) { + InputSystem.manager.inputEventHandledPolicy = policy; +#pragma warning disable CS0618 // Type or member is obsolete + var stateUpdatesAreSuppressed = policy == InputEventHandledPolicy.SuppressStateUpdates; +#pragma warning restore CS0618 // Type or member is obsolete + var gamepad = InputSystem.AddDevice(); var mouse = InputSystem.AddDevice(); - var action = new InputAction(binding: "/buttonNorth"); + var rebindAction = new InputAction(binding: "/buttonNorth"); + + // Separate action to verify WasPressedThisFrame suppression behavior. + var observerAction = new InputAction(name: "observer", type: InputActionType.Button, + binding: "/buttonSouth"); + observerAction.Enable(); using (new InputActionRebindingExtensions.RebindingOperation() - .WithAction(action) + .WithAction(rebindAction) .WithControlsExcluding("/position") .WithControlsExcluding("/press") .WithControlsExcluding("/leftButton") @@ -1159,17 +1171,30 @@ public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening() .Start() ) { - // Non-bindable controls should not be suppressed and continue working as normal + // Non-bindable controls should not be suppressed and continue working as normal. Set(mouse.position, new Vector2(123, 234)); Press(mouse.leftButton); Press(gamepad.buttonEast); Press(gamepad.buttonSouth); - Assert.That(action.bindings[0].overridePath, Is.EqualTo("/buttonSouth")); + Assert.That(rebindAction.bindings[0].overridePath, Is.EqualTo("/buttonSouth")); Assert.That(mouse.leftButton.isPressed, Is.True); - Assert.That(gamepad.buttonSouth.isPressed, Is.False); Assert.That(gamepad.buttonEast.isPressed, Is.True); Assert.That(mouse.position.ReadValue(), Is.EqualTo(new Vector2(123, 234)).Using(Vector2EqualityComparer.Instance)); + + // ISXB-1097: Under SuppressStateUpdates, the handled event is discarded so device + // state is not updated. Under SuppressActionEventNotifications, state propagates + // normally — only action notifications are suppressed. + Assert.That(gamepad.buttonSouth.isPressed, Is.EqualTo(!stateUpdatesAreSuppressed)); + + // ISXB-1097: WasPressedThisFrame is an action-level pull API and should return + // false under both policies, but for different reasons: + // - SuppressStateUpdates: the event is discarded before state updates, so the + // action never observes the press at all. + // - SuppressActionEventNotifications: state propagates (device sees the press) + // but InputAction.WasPressedThisFrame() is gated by the per-action suppressed + // flag (TriggerState.isSuppressed) and returns false. + Assert.That(observerAction.WasPressedThisFrame(), Is.False); } } diff --git a/Assets/Tests/InputSystem/CoreTests_Events.cs b/Assets/Tests/InputSystem/CoreTests_Events.cs index e3a22fd97d..c55132f135 100644 --- a/Assets/Tests/InputSystem/CoreTests_Events.cs +++ b/Assets/Tests/InputSystem/CoreTests_Events.cs @@ -1318,10 +1318,18 @@ public void Events_HandledFlagIsResetWhenEventIsQueued() Assert.That(wasHandled, Is.False); } + // ISXB-1097: This test verifies the deprecated SuppressStateUpdates behavior where handled + // events are discarded entirely, preventing device state from updating. This policy is + // deprecated because it desynchronizes the Input System's state from the source, but the + // behavior is preserved for backward compatibility when explicitly opted in. [Test] [Category("Events")] public void Events_CanPreventEventsFromBeingProcessed() { +#pragma warning disable CS0618 // Type or member is obsolete + InputSystem.manager.inputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates; +#pragma warning restore CS0618 // Type or member is obsolete + InputSystem.onEvent += (inputEvent, _) => { @@ -1343,20 +1351,24 @@ public void Events_CanPreventEventsFromBeingProcessed() public void EventHandledPolicy_ShouldReflectUserSetting() { // Assert default setting - Assert.That(InputSystem.manager.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates)); + Assert.That(InputSystem.manager.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.Default)); // Assert policy can be changed InputSystem.manager.inputEventHandledPolicy = InputEventHandledPolicy.SuppressActionEventNotifications; Assert.That(InputSystem.manager.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressActionEventNotifications)); // Assert policy can be changed back +#pragma warning disable CS0618 // Type or member is obsolete InputSystem.manager.inputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates; Assert.That(InputSystem.manager.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates)); +#pragma warning restore CS0618 // Type or member is obsolete // Assert setting property to an invalid value throws exception and do not have side-effects Assert.Throws(() => InputSystem.manager.inputEventHandledPolicy = (InputEventHandledPolicy)123456); +#pragma warning disable CS0618 // Type or member is obsolete Assert.That(InputSystem.manager.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates)); +#pragma warning restore CS0618 // Type or member is obsolete } class SuppressedActionEventData @@ -1375,7 +1387,9 @@ class SuppressedActionEventData // Step 4: Press gamepad north and stick. // Press event is detected in step 2 (false positive) with default interaction +#pragma warning disable CS0618 // Type or member is obsolete [TestCase(InputEventHandledPolicy.SuppressStateUpdates, // policy +#pragma warning restore CS0618 // Type or member is obsolete null, // interactions new int[] { 0, 0, 1, 1, 2}, // started new int[] { 0, 0, 1, 1, 2}, // performed @@ -1387,7 +1401,9 @@ class SuppressedActionEventData new int[] { 0, 0, 0, 0, 1}, new int[] {0, 0, 0, 1, 1})] // Press event is detected in step 2 (false positive) with explicit press interaction +#pragma warning disable CS0618 // Type or member is obsolete [TestCase(InputEventHandledPolicy.SuppressStateUpdates, +#pragma warning restore CS0618 // Type or member is obsolete "press", new int[] { 0, 0, 1, 1, 2}, new int[] { 0, 0, 1, 1, 2}, @@ -1518,6 +1534,222 @@ public void Events_ShouldRespectHandledPolicyUponUpdateAndSuppressedPressTransit Assert.That(Gamepad.current.buttonNorth.wasReleasedThisFrame, Is.False); } + [Test] + [Category("Events")] + [Description("ISXB-1097 Marking events as handled should prevent actions from triggering when switching devices")] + public void Events_HandledEventsShouldNotTriggerActionsWhenSwitchingDevices() + { + // Regression test for ISXB-1097: Under the old SuppressStateUpdates policy, marking + // events as handled discarded them entirely, desynchronizing device state. When the + // user then switched to a different device, the non-handled event from that device + // would arrive while the first device still had stale state, causing spurious action + // triggers. The fix (SuppressActionEventNotifications) ensures state always propagates + // so that no desynchronization occurs. + var gamepad = InputSystem.AddDevice(); + var keyboard = InputSystem.AddDevice(); + + // Action bound to both devices, mimicking a typical "Jump" binding. + var action = new InputAction(type: InputActionType.Button); + action.AddBinding("/buttonSouth"); + action.AddBinding("/space"); + action.Enable(); + + var performedCount = 0; + action.performed += _ => ++ performedCount; + + // Suppress all events via onEvent listener (user scenario from the bug report). + InputSystem.onEvent += (eventPtr, _) => { eventPtr.handled = true; }; + + // Step 1: Press on keyboard (handled — should not trigger action). + Press(keyboard.spaceKey); + Assert.That(performedCount, Is.EqualTo(0), "Action should not trigger from handled keyboard event"); + Assert.That(action.WasPressedThisFrame(), Is.False); + + // Step 2: Switch to gamepad (also handled — should not trigger action). + // Under the old policy this was the problematic transition: the keyboard press was + // never recorded in state, so the gamepad press appeared as a "new" actuation. + Press(gamepad.buttonSouth); + Assert.That(performedCount, Is.EqualTo(0), "Action should not trigger from handled gamepad event after device switch"); + Assert.That(action.WasPressedThisFrame(), Is.False); + + // Step 3: Release and press again on gamepad (still handled). + Release(gamepad.buttonSouth); + Press(gamepad.buttonSouth); + Assert.That(performedCount, Is.EqualTo(0), "Action should not trigger from repeated handled gamepad events"); + + // Step 4: Verify state is synchronized despite suppression — device state should + // reflect the press even though action notifications were suppressed. + Assert.That(gamepad.buttonSouth.isPressed, Is.True, "Device state should be updated even for handled events"); + Assert.That(keyboard.spaceKey.isPressed, Is.True, "Keyboard state should reflect the handled press (state propagates)"); + } + + [Test] + [Category("Events")] + [Description("ISXB-1097 Multiple events per frame from a high-frequency device (e.g. DualSense at" + + " 600 Hz) should not trigger actions if the press-edge event is handled")] + public void Events_HandledPressEdgeInMultiEventFrameShouldNotTriggerActions() + { + // Regression test for a scenario where a high-frequency device queues multiple state + // events in a single frame. If the first event contains the button press edge and is + // marked as handled, subsequent events in the same frame that carry the same pressed + // state must not cause a spurious action trigger — even though from the action system's + // perspective the button transitions from "not pressed" to "pressed" on those events. + var gamepad = InputSystem.AddDevice(); + + var action = new InputAction(type: InputActionType.Button, binding: "/buttonSouth"); + action.Enable(); + + var performedCount = 0; + action.performed += _ => ++ performedCount; + + // Mark only the first event in each update as handled (simulating selective suppression + // of the press-edge event while allowing subsequent state updates through). + var handleNextEvent = true; + InputSystem.onEvent += (eventPtr, _) => + { + if (handleNextEvent) + { + eventPtr.handled = true; + handleNextEvent = false; + } + }; + + // Queue multiple events in a single frame, as a high-frequency device would. + // Event 1: button press edge (will be handled). + // Event 2: same button still pressed + slight stick drift (not handled). + // Event 3: same button still pressed + more stick drift (not handled). + InputSystem.QueueStateEvent(gamepad, + new GamepadState().WithButton(GamepadButton.South)); + InputSystem.QueueStateEvent(gamepad, + new GamepadState { leftStick = new Vector2(0.01f, 0f) }.WithButton(GamepadButton.South)); + InputSystem.QueueStateEvent(gamepad, + new GamepadState { leftStick = new Vector2(0.02f, 0f) }.WithButton(GamepadButton.South)); + InputSystem.Update(); + + // ISXB-1097: The first event (press edge) was handled, so the action should not have + // triggered. The subsequent events carry the same pressed state but since state was + // already updated by the handled event, they do not represent a new press transition + // and should not trigger the action either. + Assert.That(performedCount, Is.EqualTo(0), + "Action should not trigger when press-edge event is handled, even with subsequent same-state events"); + Assert.That(action.WasPressedThisFrame(), Is.False); + Assert.That(gamepad.buttonSouth.isPressed, Is.True, + "Device state should still reflect the press from the handled event"); + + // Next frame: verify a genuine new press (unhanded) does trigger normally. + handleNextEvent = false; + InputSystem.QueueStateEvent(gamepad, new GamepadState()); // release + InputSystem.Update(); + InputSystem.QueueStateEvent(gamepad, + new GamepadState().WithButton(GamepadButton.South)); // new press + InputSystem.Update(); + + Assert.That(performedCount, Is.EqualTo(1), + "Action should trigger normally for non-handled press events"); + } + + [Test] + [Category("Events")] + [Description("ISXB-1097 All WasXxxThisFrame polling APIs should return false when events are" + + " suppressed via SuppressActionEventNotifications")] + public void Events_AllWasXxxThisFrameAPIsRespectEventSuppression() + { + // ISXB-1097: Verifies that all WasXxxThisFrame (and DynamicUpdate variants) consistently + // return false when the underlying event is handled under SuppressActionEventNotifications. + // Previously WasReleasedThisFrame and WasCompletedThisFrame were not gated by IsSuppressed. + var gamepad = InputSystem.AddDevice(); + + var buttonAction = new InputAction(name: "button", type: InputActionType.Button, + binding: "/buttonSouth"); + buttonAction.Enable(); + + // Suppress all events. + InputSystem.onEvent += (eventPtr, _) => { eventPtr.handled = true; }; + + // Press: should suppress WasPressedThisFrame and WasPerformedThisFrame. + InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South)); + InputSystem.Update(); + + Assert.That(buttonAction.WasPressedThisFrame(), Is.False, "WasPressedThisFrame should be suppressed"); + Assert.That(buttonAction.WasPressedThisDynamicUpdate(), Is.False, "WasPressedThisDynamicUpdate should be suppressed"); + Assert.That(buttonAction.WasPerformedThisFrame(), Is.False, "WasPerformedThisFrame should be suppressed"); + Assert.That(buttonAction.WasPerformedThisDynamicUpdate(), Is.False, "WasPerformedThisDynamicUpdate should be suppressed"); + // Device state should still reflect the press. + Assert.That(gamepad.buttonSouth.isPressed, Is.True); + + // Release: should suppress WasReleasedThisFrame and WasCompletedThisFrame. + InputSystem.QueueStateEvent(gamepad, new GamepadState()); + InputSystem.Update(); + + Assert.That(buttonAction.WasReleasedThisFrame(), Is.False, "WasReleasedThisFrame should be suppressed"); + Assert.That(buttonAction.WasReleasedThisDynamicUpdate(), Is.False, "WasReleasedThisDynamicUpdate should be suppressed"); + Assert.That(buttonAction.WasCompletedThisFrame(), Is.False, "WasCompletedThisFrame should be suppressed"); + Assert.That(buttonAction.WasCompletedThisDynamicUpdate(), Is.False, "WasCompletedThisDynamicUpdate should be suppressed"); + // Device state should reflect the release. + Assert.That(gamepad.buttonSouth.isPressed, Is.False); + } + + [Test] + [Category("Events")] + [Description("ISXB-1097 Per-action suppression: mixed handled/unhandled events in the same" + + " frame should only suppress the actions affected by the handled event")] + public void Events_PerActionSuppressionWithMixedHandledEvents() + { + // ISXB-1097: When multiple events arrive in the same frame and only some are handled, + // the polling APIs should return correct results per-action. An action triggered by an + // unhandled event should not be affected by a different handled event in the same frame. + var gamepad = InputSystem.AddDevice(); + + var southAction = new InputAction(name: "south", type: InputActionType.Button, + binding: "/buttonSouth"); + var northAction = new InputAction(name: "north", type: InputActionType.Button, + binding: "/buttonNorth"); + southAction.Enable(); + northAction.Enable(); + + // Handle events that press buttonSouth, but let buttonNorth events through. + InputSystem.onEvent += (eventPtr, device) => + { + // We can't selectively handle per-control within a single event, so we use + // two separate events: one for south (handled) and one for north (not handled). + }; + + // Event 1: Press south only — mark as handled. + var handleNext = true; + InputSystem.onEvent += (eventPtr, _) => + { + if (handleNext) + { + eventPtr.handled = true; + handleNext = false; + } + }; + + // Queue two events: first presses south (will be handled), second presses north + // (will not be handled). Both arrive in the same frame. + InputSystem.QueueStateEvent(gamepad, + new GamepadState().WithButton(GamepadButton.South)); + InputSystem.QueueStateEvent(gamepad, + new GamepadState().WithButton(GamepadButton.South).WithButton(GamepadButton.North)); + InputSystem.Update(); + + // South was pressed by the handled event — its polling APIs should be suppressed. + Assert.That(southAction.WasPressedThisFrame(), Is.False, + "South action triggered by handled event should be suppressed"); + Assert.That(southAction.WasPerformedThisFrame(), Is.False, + "South action triggered by handled event should be suppressed"); + + // North was pressed by the unhandled event — its polling APIs should report normally. + Assert.That(northAction.WasPressedThisFrame(), Is.True, + "North action triggered by unhandled event should NOT be suppressed"); + Assert.That(northAction.WasPerformedThisFrame(), Is.True, + "North action triggered by unhandled event should NOT be suppressed"); + + // Both buttons should reflect actual device state regardless of suppression. + Assert.That(gamepad.buttonSouth.isPressed, Is.True); + Assert.That(gamepad.buttonNorth.isPressed, Is.True); + } + [StructLayout(LayoutKind.Explicit, Size = 2)] struct StateWith2Bytes : IInputStateTypeInfo { diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index b242446471..1b67cab397 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- Fixed `InputEventPtr.handled` not preventing actions from triggering when switching devices. The default event handled policy has been changed from `SuppressStateUpdates` (now deprecated) to `SuppressActionEventNotifications`, which keeps device state synchronized while suppressing action callbacks for handled events. [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097) +- Fixed all `InputAction.WasXxxThisFrame()` and `WasXxxThisDynamicUpdate()` polling APIs to use per-action suppression tracking instead of a map-wide flag. Previously, when multiple events arrived in the same frame with mixed handled/unhandled states, the last event's suppression state could incorrectly affect all actions in the map. [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097) - Fixed a `NullReferenceException` thrown when removing all action maps [UUM-137116](https://jira.unity3d.com/browse/UUM-137116) - Simplified default setting messaging by consolidating repetitive messages into a single HelpBox. - Fixed a `NullPointerReferenceException` thrown in `InputManagerStateMonitors.FireStateChangeNotifications` logging by adding validation [UUM-136095]. diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputAction.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputAction.cs index d7cb1eb6aa..42449d937d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputAction.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputAction.cs @@ -1260,9 +1260,11 @@ private int ExpectedFrame() public unsafe bool WasPressedThisFrame() { var state = GetOrCreateActionMap().m_State; - if (state != null && !state.IsSuppressed) + if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; var currentUpdateStep = InputUpdate.s_UpdateStepCount; return actionStatePtr->pressedInUpdate == currentUpdateStep && currentUpdateStep != default; } @@ -1303,6 +1305,8 @@ public unsafe bool WasPressedThisDynamicUpdate() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; return actionStatePtr->framePressed == ExpectedFrame(); } @@ -1353,6 +1357,8 @@ public unsafe bool WasReleasedThisFrame() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; var currentUpdateStep = InputUpdate.s_UpdateStepCount; return actionStatePtr->releasedInUpdate == currentUpdateStep && currentUpdateStep != default; } @@ -1394,6 +1400,8 @@ public unsafe bool WasReleasedThisDynamicUpdate() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; return actionStatePtr->frameReleased == ExpectedFrame(); } @@ -1451,9 +1459,11 @@ public unsafe bool WasPerformedThisFrame() { var state = GetOrCreateActionMap().m_State; - if (state != null && !state.IsSuppressed) + if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; var currentUpdateStep = InputUpdate.s_UpdateStepCount; return actionStatePtr->lastPerformedInUpdate == currentUpdateStep && currentUpdateStep != default; } @@ -1493,6 +1503,8 @@ public unsafe bool WasPerformedThisDynamicUpdate() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; return actionStatePtr->framePerformed == ExpectedFrame(); } @@ -1574,6 +1586,8 @@ public unsafe bool WasCompletedThisFrame() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; var currentUpdateStep = InputUpdate.s_UpdateStepCount; return actionStatePtr->lastCompletedInUpdate == currentUpdateStep && currentUpdateStep != default; } @@ -1615,6 +1629,8 @@ public unsafe bool WasCompletedThisDynamicUpdate() if (state != null) { var actionStatePtr = &state.actionStates[m_ActionIndexInState]; + if (actionStatePtr->isSuppressed) + return false; return actionStatePtr->frameCompleted == ExpectedFrame(); } diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs index 26dc558ffe..b8342ecb81 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionRebindingExtensions.cs @@ -2119,7 +2119,9 @@ public RebindingOperation WithActionEventNotificationsBeingSuppressed(bool value ThrowIfRebindInProgress(); m_TargetInputEventHandledPolicy = value ? InputEventHandledPolicy.SuppressActionEventNotifications +#pragma warning disable CS0618 // Type or member is obsolete : InputEventHandledPolicy.SuppressStateUpdates; +#pragma warning restore CS0618 // Type or member is obsolete return this; } @@ -2147,8 +2149,12 @@ public RebindingOperation Start() m_StartTime = InputState.currentTime; + // ISXB-1097: Only override the global policy if the user made an explicit + // choice via WithActionEventNotificationsBeingSuppressed(). Otherwise, + // preserve whatever policy is currently configured via InputSettings. m_SavedInputEventHandledPolicy = InputSystem.manager.inputEventHandledPolicy; - InputSystem.manager.inputEventHandledPolicy = m_TargetInputEventHandledPolicy; + if (m_TargetInputEventHandledPolicy.HasValue) + InputSystem.manager.inputEventHandledPolicy = m_TargetInputEventHandledPolicy.Value; if (m_WaitSecondsAfterMatch > 0 || m_Timeout > 0) { @@ -2705,7 +2711,10 @@ private string GeneratePathForControl(InputControl control) private float m_Timeout; private float m_WaitSecondsAfterMatch; private InputEventHandledPolicy m_SavedInputEventHandledPolicy; - private InputEventHandledPolicy m_TargetInputEventHandledPolicy; + // ISXB-1097: Nullable so that Start() only overrides the global policy when the + // user made an explicit choice via WithActionEventNotificationsBeingSuppressed(). + // When null, the current InputSystem.manager.inputEventHandledPolicy is preserved. + private InputEventHandledPolicy? m_TargetInputEventHandledPolicy; private InputControlList m_Candidates; private Action m_OnComplete; private Action m_OnCancel; diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs index 32b0354752..5756e0a960 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs @@ -368,11 +368,6 @@ private bool CanUseDevice(InputDevice device) return false; } - /// - /// Check whether the state is currently reflecting a suppressed state. - /// - public bool IsSuppressed => m_Suppressed; - /// /// Check whether the state has any actions that are currently enabled. /// @@ -2540,6 +2535,12 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt newState.startTime = newState.time; *actionState = newState; + // ISXB-1097: Stamp per-action suppression flag so polling APIs (WasPressedThisFrame, + // etc.) can check it after the update completes. This captures the suppression state + // of the specific event that changed this action, avoiding the last-event-wins issue + // with the map-wide m_Suppressed flag. + actionState->isSuppressed = m_Suppressed; + // Let listeners know. var map = maps[trigger.mapIndex]; Debug.Assert(actionIndex >= mapIndices[trigger.mapIndex].actionStartIndex, @@ -3998,6 +3999,18 @@ public bool inProcessing } } + public bool isSuppressed + { + get => (flags & Flags.Suppressed) != 0; + set + { + if (value) + flags |= Flags.Suppressed; + else + flags &= ~Flags.Suppressed; + } + } + public Flags flags { get => (Flags)m_Flags; @@ -4045,6 +4058,19 @@ public enum Flags Button = 1 << 5, Pressed = 1 << 6, + + /// + /// Whether the last event that changed this action's state was suppressed + /// (handled event under ). + /// + /// + /// ISXB-1097: This per-action flag is stamped when the action's phase changes, + /// capturing the suppression state of the specific event that caused the change. + /// Used by polling APIs (WasPressedThisFrame, etc.) instead of the map-wide + /// m_Suppressed flag which suffers from last-event-wins issues when multiple + /// events with different handled states arrive in the same frame. + /// + Suppressed = 1 << 7, } } diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Events/InputEventHandledPolicy.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Events/InputEventHandledPolicy.cs index fc36bd1805..84c42692ba 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Events/InputEventHandledPolicy.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Events/InputEventHandledPolicy.cs @@ -1,3 +1,5 @@ +using System; + namespace UnityEngine.InputSystem.LowLevel { /// @@ -9,12 +11,18 @@ internal enum InputEventHandledPolicy /// /// Input events will be discarded directly and not propagate for state changes. /// + [Obsolete("Use SuppressActionEventNotifications instead. SuppressStateUpdates desynchronizes Input System state from source state, leading to undefined behavior.", error: false)] SuppressStateUpdates, /// /// Input events will be processed for state updates and input action interaction updates but interaction /// event notifications will be suppressed. /// - SuppressActionEventNotifications + SuppressActionEventNotifications, + + /// + /// The default input event handling policy. + /// + Default = SuppressActionEventNotifications } } diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs index 4be52778df..1dfe2ef001 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs @@ -338,9 +338,11 @@ internal InputEventHandledPolicy inputEventHandledPolicy switch (value) { case InputEventHandledPolicy.SuppressActionEventNotifications: +#pragma warning disable CS0618 // Type or member is obsolete case InputEventHandledPolicy.SuppressStateUpdates: m_InputEventHandledPolicy = value; break; +#pragma warning restore CS0618 // Type or member is obsolete default: throw new ArgumentOutOfRangeException( $"Unsupported input event handling policy: {value}"); @@ -1643,7 +1645,13 @@ public unsafe void ResetDevice(InputDevice device, bool alsoResetDontResetContro stateEventPtr->baseEvent.sizeInBytes = InputEvent.kBaseEventSize + sizeof(int) + deviceStateBlockSize; stateEventPtr->baseEvent.time = currentTime; stateEventPtr->baseEvent.deviceId = device.deviceId; - stateEventPtr->baseEvent.eventId = -1; + // ISXB-1097: Using InvalidEventId (0) rather than -1 here. Setting eventId to -1 + // (0xFFFFFFFF) accidentally sets the handled bit (bit 31, kHandledMask) which + // causes SuppressActionEventNotifications policy to suppress action callbacks + // from this synthetic reset event. Whether reset events *should* suppress + // pass-through Performed(0) notifications is a separate design question — but + // it should be an intentional choice, not a side-effect of a sentinel value. + stateEventPtr->baseEvent.eventId = InputEvent.InvalidEventId; stateEventPtr->stateFormat = device.m_StateBlock.format; // Decide whether we perform a soft reset or a hard reset. @@ -1988,7 +1996,7 @@ internal void InitializeData() #endif // Default input event handled policy. - m_InputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates; + m_InputEventHandledPolicy = InputEventHandledPolicy.Default; // Register layouts. // NOTE: Base layouts must be registered before their derived layouts @@ -3427,12 +3435,14 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven new InputEventPtr(currentEventReadPtr), device, k_InputOnEventMarker, "InputSystem.onEvent"); // If a listener marks the event as handled, we don't process it further. +#pragma warning disable CS0618 // Type or member is obsolete if (m_InputEventHandledPolicy == InputEventHandledPolicy.SuppressStateUpdates && currentEventReadPtr->handled) { m_InputEventStream.Advance(false); continue; } +#pragma warning restore CS0618 // Type or member is obsolete } // Update metrics.