Skip to content

Commit 5701067

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

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
@@ -1689,6 +1689,67 @@ public void Events_AllWasXxxThisFrameAPIsRespectEventSuppression()
16891689
Assert.That(gamepad.buttonSouth.isPressed, Is.False);
16901690
}
16911691

1692+
[Test]
1693+
[Category("Events")]
1694+
[Description("ISXB-1097 Per-action suppression: mixed handled/unhandled events in the same" +
1695+
" frame should only suppress the actions affected by the handled event")]
1696+
public void Events_PerActionSuppressionWithMixedHandledEvents()
1697+
{
1698+
// ISXB-1097: When multiple events arrive in the same frame and only some are handled,
1699+
// the polling APIs should return correct results per-action. An action triggered by an
1700+
// unhandled event should not be affected by a different handled event in the same frame.
1701+
var gamepad = InputSystem.AddDevice<Gamepad>();
1702+
1703+
var southAction = new InputAction(name: "south", type: InputActionType.Button,
1704+
binding: "<Gamepad>/buttonSouth");
1705+
var northAction = new InputAction(name: "north", type: InputActionType.Button,
1706+
binding: "<Gamepad>/buttonNorth");
1707+
southAction.Enable();
1708+
northAction.Enable();
1709+
1710+
// Handle events that press buttonSouth, but let buttonNorth events through.
1711+
InputSystem.onEvent += (eventPtr, device) =>
1712+
{
1713+
// We can't selectively handle per-control within a single event, so we use
1714+
// two separate events: one for south (handled) and one for north (not handled).
1715+
};
1716+
1717+
// Event 1: Press south only — mark as handled.
1718+
var handleNext = true;
1719+
InputSystem.onEvent += (eventPtr, _) =>
1720+
{
1721+
if (handleNext)
1722+
{
1723+
eventPtr.handled = true;
1724+
handleNext = false;
1725+
}
1726+
};
1727+
1728+
// Queue two events: first presses south (will be handled), second presses north
1729+
// (will not be handled). Both arrive in the same frame.
1730+
InputSystem.QueueStateEvent(gamepad,
1731+
new GamepadState().WithButton(GamepadButton.South));
1732+
InputSystem.QueueStateEvent(gamepad,
1733+
new GamepadState().WithButton(GamepadButton.South).WithButton(GamepadButton.North));
1734+
InputSystem.Update();
1735+
1736+
// South was pressed by the handled event — its polling APIs should be suppressed.
1737+
Assert.That(southAction.WasPressedThisFrame(), Is.False,
1738+
"South action triggered by handled event should be suppressed");
1739+
Assert.That(southAction.WasPerformedThisFrame(), Is.False,
1740+
"South action triggered by handled event should be suppressed");
1741+
1742+
// North was pressed by the unhandled event — its polling APIs should report normally.
1743+
Assert.That(northAction.WasPressedThisFrame(), Is.True,
1744+
"North action triggered by unhandled event should NOT be suppressed");
1745+
Assert.That(northAction.WasPerformedThisFrame(), Is.True,
1746+
"North action triggered by unhandled event should NOT be suppressed");
1747+
1748+
// Both buttons should reflect actual device state regardless of suppression.
1749+
Assert.That(gamepad.buttonSouth.isPressed, Is.True);
1750+
Assert.That(gamepad.buttonNorth.isPressed, Is.True);
1751+
}
1752+
16921753
[StructLayout(LayoutKind.Explicit, Size = 2)]
16931754
struct StateWith2Bytes : IInputStateTypeInfo
16941755
{

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)