Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 49 additions & 7 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,45 @@ public void Actions_ValueActionsEnabledInOnEvent_DoNotReactToCurrentStateOfContr
}
}

// Regression test for UUM-100125.
[Test]
[Category("Actions")]
public void Actions_InitialStateCheckAfterConfigurationChange_DoesNotTriggerForInactiveTouch()
{
var touchscreen = InputSystem.AddDevice<Touchscreen>();
var action = new InputAction(type: InputActionType.Value, binding: "<Touchscreen>/primaryTouch/position");
action.Enable();

// Run the first initial state check from enabling the action.
InputSystem.Update();

using (var trace = new InputActionTrace(action))
{
BeginTouch(1, new Vector2(123, 234));
EndTouch(1, new Vector2(345, 456));

Assert.That(touchscreen.primaryTouch.isInProgress, Is.False);
Assert.That(touchscreen.primaryTouch.position.ReadValue(), Is.Not.EqualTo(default(Vector2)));

trace.Clear();

// Configuration change causes full re-resolve and schedules initial state check.
InputSystem.QueueConfigChangeEvent(touchscreen);
InputSystem.Update();
InputSystem.Update();

// Full re-resolve may cancel the current action state. What must NOT happen is a synthetic
// Started/Performed pair from persisted inactive touch state.
Assert.AreEqual(1, trace.count);
foreach (var eventPtr in trace)
{
// The trace should only contain a Canceled event for the action.
Assert.AreEqual(InputActionPhase.Canceled, eventPtr.phase,
$"inactive touch state should not produce action callbacks, but received {eventPtr.phase}.");
}
}
}

// https://fogbugz.unity3d.com/f/cases/1192972/
[Test]
[Category("Actions")]
Expand Down Expand Up @@ -12468,17 +12507,20 @@ public void Actions_WithMultipleCompositeBindings_WithoutEvaluateMagnitude_Works

// Now when enabling actionMap ..
actionMap.Enable();
// On the following update we will trigger OnBeforeUpdate which will rise started/performed
// from InputActionState.OnBeforeInitialUpdate as controls are "actuated"
// Inactive touches (ended before action was enabled) must NOT produce started/performed from
// OnBeforeInitialUpdate. Their persisted state (position, touchId) is non-default due to
// dontReset, but only TouchControl.isInProgress should be considered for initial-state check.
// Related to UUM-100125 and Actions_InitialStateCheckAfterConfigurationChange_DoesNotTriggerForInactiveTouch.
InputSystem.Update();
Assert.That(values.Count, Is.EqualTo(prepopulateTouchesBeforeEnablingAction ? 2 : 0)); // started+performed arrive from OnBeforeUpdate
Assert.That(values.Count, Is.EqualTo(0));
values.Clear();

// Now subsequent touches should not be ignored
BeginTouch(200, new Vector2(1, 1));
Assert.That(values.Count, Is.EqualTo(1));
Assert.That(values[0].InputId, Is.EqualTo(200));
Assert.That(values[0].Position, Is.EqualTo(new Vector2(1, 1)));
// If prepopulated, action was never actuated (synthetic initial-check is suppressed),
// so BeginTouch fires started+performed (2 events).
Assert.That(values.Count, Is.EqualTo(prepopulateTouchesBeforeEnablingAction ? 2 : 1));
Assert.That(values[values.Count - 1].InputId, Is.EqualTo(200));
Assert.That(values[values.Count - 1].Position, Is.EqualTo(new Vector2(1, 1)));
}

// FIX: This test is currently checking if shortcut support is enabled by testing that the unwanted behaviour exists.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,9 @@
if (IsActiveControl(bindingIndex, controlIndex))
continue;

if (ShouldSkipInitialStateCheck(control))
continue;

if (!control.CheckStateIsAtDefault())
{
// Update press times.
Expand Down Expand Up @@ -1348,6 +1351,22 @@
k_InputInitialActionStateCheckMarker.End();
}

private static bool ShouldSkipInitialStateCheck(InputControl control)
{
// UUM-100125
// Touch controls intentionally preserve state such as position even when no touch is currently active.
// During binding re-resolution this can make inactive touches look actuated and cause invalid triggers.
for (var current = control; current != null; current = current.parent)
{

Check warning on line 1360 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs#L1360

Added line #L1360 was not covered by tests
if (current is TouchControl touchControl)
{
return !touchControl.isInProgress;
}
}

return false;
}

// Called from InputManager when one of our state change monitors has fired.
// Tells us the time of the change *according to the state events coming in*.
// Also tells us which control of the controls we are binding to triggered the
Expand Down