Skip to content

Commit 4b6bed6

Browse files
committed
FIX: Corrected a bug where suppression flag was tied to action map instead of per action.
1 parent d9e1603 commit 4b6bed6

5 files changed

Lines changed: 119 additions & 16 deletions

File tree

Assets/Tests/InputSystem/CoreTests_Actions_Rebinding.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,8 +1192,8 @@ public void Actions_InteractiveRebinding_CanSuppressEventsWhileListening(InputEv
11921192
// - SuppressStateUpdates: the event is discarded before state updates, so the
11931193
// action never observes the press at all.
11941194
// - SuppressActionEventNotifications: state propagates (device sees the press)
1195-
// but InputAction.WasPressedThisFrame() is gated by IsSuppressed and returns
1196-
// false (see InputAction.cs WasPressedThisFrame).
1195+
// but InputAction.WasPressedThisFrame() is gated by the per-action suppressed
1196+
// flag (TriggerState.isSuppressed) and returns false.
11971197
Assert.That(observerAction.WasPressedThisFrame(), Is.False);
11981198
}
11991199
}

Assets/Tests/InputSystem/CoreTests_Events.cs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,67 @@ public void Events_AllWasXxxThisFrameAPIsRespectEventSuppression()
16071607
Assert.That(gamepad.buttonSouth.isPressed, Is.False);
16081608
}
16091609

1610+
[Test]
1611+
[Category("Events")]
1612+
[Description("ISXB-1097 Per-action suppression: mixed handled/unhandled events in the same" +
1613+
" frame should only suppress the actions affected by the handled event")]
1614+
public void Events_PerActionSuppressionWithMixedHandledEvents()
1615+
{
1616+
// ISXB-1097: When multiple events arrive in the same frame and only some are handled,
1617+
// the polling APIs should return correct results per-action. An action triggered by an
1618+
// unhandled event should not be affected by a different handled event in the same frame.
1619+
var gamepad = InputSystem.AddDevice<Gamepad>();
1620+
1621+
var southAction = new InputAction(name: "south", type: InputActionType.Button,
1622+
binding: "<Gamepad>/buttonSouth");
1623+
var northAction = new InputAction(name: "north", type: InputActionType.Button,
1624+
binding: "<Gamepad>/buttonNorth");
1625+
southAction.Enable();
1626+
northAction.Enable();
1627+
1628+
// Handle events that press buttonSouth, but let buttonNorth events through.
1629+
InputSystem.onEvent += (eventPtr, device) =>
1630+
{
1631+
// We can't selectively handle per-control within a single event, so we use
1632+
// two separate events: one for south (handled) and one for north (not handled).
1633+
};
1634+
1635+
// Event 1: Press south only — mark as handled.
1636+
var handleNext = true;
1637+
InputSystem.onEvent += (eventPtr, _) =>
1638+
{
1639+
if (handleNext)
1640+
{
1641+
eventPtr.handled = true;
1642+
handleNext = false;
1643+
}
1644+
};
1645+
1646+
// Queue two events: first presses south (will be handled), second presses north
1647+
// (will not be handled). Both arrive in the same frame.
1648+
InputSystem.QueueStateEvent(gamepad,
1649+
new GamepadState().WithButton(GamepadButton.South));
1650+
InputSystem.QueueStateEvent(gamepad,
1651+
new GamepadState().WithButton(GamepadButton.South).WithButton(GamepadButton.North));
1652+
InputSystem.Update();
1653+
1654+
// South was pressed by the handled event — its polling APIs should be suppressed.
1655+
Assert.That(southAction.WasPressedThisFrame(), Is.False,
1656+
"South action triggered by handled event should be suppressed");
1657+
Assert.That(southAction.WasPerformedThisFrame(), Is.False,
1658+
"South action triggered by handled event should be suppressed");
1659+
1660+
// North was pressed by the unhandled event — its polling APIs should report normally.
1661+
Assert.That(northAction.WasPressedThisFrame(), Is.True,
1662+
"North action triggered by unhandled event should NOT be suppressed");
1663+
Assert.That(northAction.WasPerformedThisFrame(), Is.True,
1664+
"North action triggered by unhandled event should NOT be suppressed");
1665+
1666+
// Both buttons should reflect actual device state regardless of suppression.
1667+
Assert.That(gamepad.buttonSouth.isPressed, Is.True);
1668+
Assert.That(gamepad.buttonNorth.isPressed, Is.True);
1669+
}
1670+
16101671
[StructLayout(LayoutKind.Explicit, Size = 2)]
16111672
struct StateWith2Bytes : IInputStateTypeInfo
16121673
{

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1010
### Fixed
1111

1212
- 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)
13-
- Fixed `InputAction.WasReleasedThisFrame()`, `WasReleasedThisDynamicUpdate()`, `WasCompletedThisFrame()`, and `WasCompletedThisDynamicUpdate()` not respecting event suppression, making them inconsistent with `WasPressedThisFrame()` and `WasPerformedThisFrame()`. [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097)
13+
- 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)
1414
- Fixed a `NullReferenceException` thrown when removing all action maps [UUM-137116](https://jira.unity3d.com/browse/UUM-137116)
1515
- Simplified default setting messaging by consolidating repetitive messages into a single HelpBox.
1616
- Fixed a `NullPointerReferenceException` thrown in `InputManagerStateMonitors.FireStateChangeNotifications` logging by adding validation [UUM-136095].

Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputAction.cs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,9 +1260,11 @@ private int ExpectedFrame()
12601260
public unsafe bool WasPressedThisFrame()
12611261
{
12621262
var state = GetOrCreateActionMap().m_State;
1263-
if (state != null && !state.IsSuppressed)
1263+
if (state != null)
12641264
{
12651265
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1266+
if (actionStatePtr->isSuppressed)
1267+
return false;
12661268
var currentUpdateStep = InputUpdate.s_UpdateStepCount;
12671269
return actionStatePtr->pressedInUpdate == currentUpdateStep && currentUpdateStep != default;
12681270
}
@@ -1300,9 +1302,11 @@ public unsafe bool WasPressedThisFrame()
13001302
public unsafe bool WasPressedThisDynamicUpdate()
13011303
{
13021304
var state = GetOrCreateActionMap().m_State;
1303-
if (state != null && !state.IsSuppressed)
1305+
if (state != null)
13041306
{
13051307
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1308+
if (actionStatePtr->isSuppressed)
1309+
return false;
13061310
return actionStatePtr->framePressed == ExpectedFrame();
13071311
}
13081312

@@ -1350,9 +1354,11 @@ public unsafe bool WasPressedThisDynamicUpdate()
13501354
public unsafe bool WasReleasedThisFrame()
13511355
{
13521356
var state = GetOrCreateActionMap().m_State;
1353-
if (state != null && !state.IsSuppressed)
1357+
if (state != null)
13541358
{
13551359
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1360+
if (actionStatePtr->isSuppressed)
1361+
return false;
13561362
var currentUpdateStep = InputUpdate.s_UpdateStepCount;
13571363
return actionStatePtr->releasedInUpdate == currentUpdateStep && currentUpdateStep != default;
13581364
}
@@ -1391,9 +1397,11 @@ public unsafe bool WasReleasedThisFrame()
13911397
public unsafe bool WasReleasedThisDynamicUpdate()
13921398
{
13931399
var state = GetOrCreateActionMap().m_State;
1394-
if (state != null && !state.IsSuppressed)
1400+
if (state != null)
13951401
{
13961402
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1403+
if (actionStatePtr->isSuppressed)
1404+
return false;
13971405
return actionStatePtr->frameReleased == ExpectedFrame();
13981406
}
13991407

@@ -1451,9 +1459,11 @@ public unsafe bool WasPerformedThisFrame()
14511459
{
14521460
var state = GetOrCreateActionMap().m_State;
14531461

1454-
if (state != null && !state.IsSuppressed)
1462+
if (state != null)
14551463
{
14561464
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1465+
if (actionStatePtr->isSuppressed)
1466+
return false;
14571467
var currentUpdateStep = InputUpdate.s_UpdateStepCount;
14581468
return actionStatePtr->lastPerformedInUpdate == currentUpdateStep && currentUpdateStep != default;
14591469
}
@@ -1490,9 +1500,11 @@ public unsafe bool WasPerformedThisDynamicUpdate()
14901500
{
14911501
var state = GetOrCreateActionMap().m_State;
14921502

1493-
if (state != null && !state.IsSuppressed)
1503+
if (state != null)
14941504
{
14951505
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1506+
if (actionStatePtr->isSuppressed)
1507+
return false;
14961508
return actionStatePtr->framePerformed == ExpectedFrame();
14971509
}
14981510

@@ -1571,9 +1583,11 @@ public unsafe bool WasCompletedThisFrame()
15711583
{
15721584
var state = GetOrCreateActionMap().m_State;
15731585

1574-
if (state != null && !state.IsSuppressed)
1586+
if (state != null)
15751587
{
15761588
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1589+
if (actionStatePtr->isSuppressed)
1590+
return false;
15771591
var currentUpdateStep = InputUpdate.s_UpdateStepCount;
15781592
return actionStatePtr->lastCompletedInUpdate == currentUpdateStep && currentUpdateStep != default;
15791593
}
@@ -1612,9 +1626,11 @@ public unsafe bool WasCompletedThisDynamicUpdate()
16121626
{
16131627
var state = GetOrCreateActionMap().m_State;
16141628

1615-
if (state != null && !state.IsSuppressed)
1629+
if (state != null)
16161630
{
16171631
var actionStatePtr = &state.actionStates[m_ActionIndexInState];
1632+
if (actionStatePtr->isSuppressed)
1633+
return false;
16181634
return actionStatePtr->frameCompleted == ExpectedFrame();
16191635
}
16201636

Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,11 +368,6 @@ private bool CanUseDevice(InputDevice device)
368368
return false;
369369
}
370370

371-
/// <summary>
372-
/// Check whether the state is currently reflecting a suppressed state.
373-
/// </summary>
374-
public bool IsSuppressed => m_Suppressed;
375-
376371
/// <summary>
377372
/// Check whether the state has any actions that are currently enabled.
378373
/// </summary>
@@ -2540,6 +2535,12 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt
25402535
newState.startTime = newState.time;
25412536
*actionState = newState;
25422537

2538+
// ISXB-1097: Stamp per-action suppression flag so polling APIs (WasPressedThisFrame,
2539+
// etc.) can check it after the update completes. This captures the suppression state
2540+
// of the specific event that changed this action, avoiding the last-event-wins issue
2541+
// with the map-wide m_Suppressed flag.
2542+
actionState->isSuppressed = m_Suppressed;
2543+
25432544
// Let listeners know.
25442545
var map = maps[trigger.mapIndex];
25452546
Debug.Assert(actionIndex >= mapIndices[trigger.mapIndex].actionStartIndex,
@@ -3998,6 +3999,18 @@ public bool inProcessing
39983999
}
39994000
}
40004001

4002+
public bool isSuppressed
4003+
{
4004+
get => (flags & Flags.Suppressed) != 0;
4005+
set
4006+
{
4007+
if (value)
4008+
flags |= Flags.Suppressed;
4009+
else
4010+
flags &= ~Flags.Suppressed;
4011+
}
4012+
}
4013+
40014014
public Flags flags
40024015
{
40034016
get => (Flags)m_Flags;
@@ -4045,6 +4058,19 @@ public enum Flags
40454058
Button = 1 << 5,
40464059

40474060
Pressed = 1 << 6,
4061+
4062+
/// <summary>
4063+
/// Whether the last event that changed this action's state was suppressed
4064+
/// (handled event under <see cref="InputEventHandledPolicy.SuppressActionEventNotifications"/>).
4065+
/// </summary>
4066+
/// <remarks>
4067+
/// ISXB-1097: This per-action flag is stamped when the action's phase changes,
4068+
/// capturing the suppression state of the specific event that caused the change.
4069+
/// Used by polling APIs (WasPressedThisFrame, etc.) instead of the map-wide
4070+
/// m_Suppressed flag which suffers from last-event-wins issues when multiple
4071+
/// events with different handled states arrive in the same frame.
4072+
/// </remarks>
4073+
Suppressed = 1 << 7,
40484074
}
40494075
}
40504076

0 commit comments

Comments
 (0)