Skip to content

Commit 145da6d

Browse files
ekcohu-pr[bot]
andauthored
FIX: ISXB-1097 Changed default event handled policy and deprecated event handling policy SuppressStateUpdates since broken by design. (#2412)
Co-authored-by: u-pr[bot] <205906871+u-pr[bot]@users.noreply.github.com>
1 parent 81a1535 commit 145da6d

9 files changed

Lines changed: 360 additions & 25 deletions

File tree

Assets/Tests/InputSystem/CoreTests_Actions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3748,6 +3748,13 @@ public void Actions_ResettingDevice_CancelsOngoingActionsThatAreDrivenByIt()
37483748
// does not cause the action to start back up. For pass-through actions, that is different
37493749
// as *any* value change performs the action. So here, we see *both* a cancellation and then
37503750
// immediately a performing of the action.
3751+
//
3752+
// ISXB-1097 DESIGN NOTE: Whether pass-through actions should emit Performed(0f) on reset is
3753+
// debatable. Emitting it is consistent with the pass-through contract ("any value
3754+
// change performs"). Suppressing it would be consistent with button/value actions and
3755+
// the idea that resets aren't real user input. If we decide to suppress, the synthetic
3756+
// reset event in ResetDevice should be explicitly marked as handled rather than
3757+
// relying on the old eventId=-1 sentinel side-effect (see InputManager.cs:1656).
37513758
Assert.That(passThroughActionTrace, Canceled(passThroughAction).AndThen(Performed(passThroughAction, value: 0f)));
37523759
}
37533760
}

Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,22 +1135,34 @@ public void Actions_InteractiveRebinding_IfDeviceHasMultipleUsages_UsagesAreAppl
11351135
}
11361136
}
11371137

1138-
// It can be desirable to not let the event through that we're rebinding from. This, for example, prevents the event
1139-
// from triggering UI actions. Note, however, that it also prevents the state of the device from updating correctly.
1140-
//
1141-
// NOTE: Hopefully, when we have a system in place that allows coordinating event consumption between actions, we have
1142-
// have a more elegant solution at our hand for solving the problem here.
1143-
[Test]
1138+
// ISXB-1097: Verifies event suppression behavior during interactive rebinding under both
1139+
// event handled policies. Under SuppressStateUpdates, handled events are discarded entirely
1140+
// (device state not updated). Under SuppressActionEventNotifications, handled events still
1141+
// propagate state but suppress action-level notifications (e.g. WasPressedThisFrame).
1142+
#pragma warning disable CS0618 // Type or member is obsolete
1143+
[TestCase(InputEventHandledPolicy.SuppressStateUpdates)]
1144+
#pragma warning restore CS0618 // Type or member is obsolete
1145+
[TestCase(InputEventHandledPolicy.SuppressActionEventNotifications)]
11441146
[Category("Actions")]
1145-
public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening()
1147+
public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening(InputEventHandledPolicy policy)
11461148
{
1149+
InputSystem.manager.inputEventHandledPolicy = policy;
1150+
#pragma warning disable CS0618 // Type or member is obsolete
1151+
var stateUpdatesAreSuppressed = policy == InputEventHandledPolicy.SuppressStateUpdates;
1152+
#pragma warning restore CS0618 // Type or member is obsolete
1153+
11471154
var gamepad = InputSystem.AddDevice<Gamepad>();
11481155
var mouse = InputSystem.AddDevice<Mouse>();
11491156

1150-
var action = new InputAction(binding: "<Gamepad>/buttonNorth");
1157+
var rebindAction = new InputAction(binding: "<Gamepad>/buttonNorth");
1158+
1159+
// Separate action to verify WasPressedThisFrame suppression behavior.
1160+
var observerAction = new InputAction(name: "observer", type: InputActionType.Button,
1161+
binding: "<Gamepad>/buttonSouth");
1162+
observerAction.Enable();
11511163

11521164
using (new InputActionRebindingExtensions.RebindingOperation()
1153-
.WithAction(action)
1165+
.WithAction(rebindAction)
11541166
.WithControlsExcluding("<Pointer>/position")
11551167
.WithControlsExcluding("<Pointer>/press")
11561168
.WithControlsExcluding("<Mouse>/leftButton")
@@ -1159,17 +1171,30 @@ public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening()
11591171
.Start()
11601172
)
11611173
{
1162-
// Non-bindable controls should not be suppressed and continue working as normal
1174+
// Non-bindable controls should not be suppressed and continue working as normal.
11631175
Set(mouse.position, new Vector2(123, 234));
11641176
Press(mouse.leftButton);
11651177
Press(gamepad.buttonEast);
11661178

11671179
Press(gamepad.buttonSouth);
1168-
Assert.That(action.bindings[0].overridePath, Is.EqualTo("<Gamepad>/buttonSouth"));
1180+
Assert.That(rebindAction.bindings[0].overridePath, Is.EqualTo("<Gamepad>/buttonSouth"));
11691181
Assert.That(mouse.leftButton.isPressed, Is.True);
1170-
Assert.That(gamepad.buttonSouth.isPressed, Is.False);
11711182
Assert.That(gamepad.buttonEast.isPressed, Is.True);
11721183
Assert.That(mouse.position.ReadValue(), Is.EqualTo(new Vector2(123, 234)).Using(Vector2EqualityComparer.Instance));
1184+
1185+
// ISXB-1097: Under SuppressStateUpdates, the handled event is discarded so device
1186+
// state is not updated. Under SuppressActionEventNotifications, state propagates
1187+
// normally — only action notifications are suppressed.
1188+
Assert.That(gamepad.buttonSouth.isPressed, Is.EqualTo(!stateUpdatesAreSuppressed));
1189+
1190+
// ISXB-1097: WasPressedThisFrame is an action-level pull API and should return
1191+
// false under both policies, but for different reasons:
1192+
// - SuppressStateUpdates: the event is discarded before state updates, so the
1193+
// action never observes the press at all.
1194+
// - SuppressActionEventNotifications: state propagates (device sees the press)
1195+
// but InputAction.WasPressedThisFrame() is gated by the per-action suppressed
1196+
// flag (TriggerState.isSuppressed) and returns false.
1197+
Assert.That(observerAction.WasPressedThisFrame(), Is.False);
11731198
}
11741199
}
11751200

0 commit comments

Comments
 (0)