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
8 changes: 2 additions & 6 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ _Please describe the testing already done by you and what testing you request/re

_Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any._

- Complexity:
- Halo Effect:
- Complexity:
- Halo Effect:

### Comments to reviewers

Expand Down Expand Up @@ -45,7 +45,3 @@ During merge:
- `DOCS: ___`.
- `CHANGE: ___`.
- `RELEASE: 1.1.0-preview.3`.

After merge:

- [ ] Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.
54 changes: 54 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,60 @@ public void Actions_DoNotGetTriggeredByEditorUpdates()
}
}

[Test]
[Category("Actions")]
[Description("Tests that that only the latest event after focus is regained is able to trigger the action." +
"Depends on background behavior. (ISXB-1671)")]
[TestCase(InputSettings.BackgroundBehavior.IgnoreFocus)]
[TestCase(InputSettings.BackgroundBehavior.ResetAndDisableNonBackgroundDevices)]
[TestCase(InputSettings.BackgroundBehavior.ResetAndDisableAllDevices)]
public void Actions_DoNotGetTriggeredByOutOfFocusEventInEditor(InputSettings.BackgroundBehavior backgroundBehavior)
{
InputSystem.settings.backgroundBehavior = backgroundBehavior;

var mouse = InputSystem.AddDevice<Mouse>();
var mousePointAction = new InputAction(binding: "<Mouse>/position", type: InputActionType.PassThrough);
mousePointAction.Enable();

using (var trace = new InputActionTrace(mousePointAction))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend letting the time step parameter also be an input parameter to the test case. It should still be valid for timeStep = 0.0f, since our logic need to handle event order as the determining factor when time isn't sufficient to tell them apart.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I would recommend adding that test as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, this would not work. Time needs to move; otherwise, we can't know what events have occurred before and after the focus. This solution relies on event timestamps. Similar to what we do for transitions between Edit Mode and Entering Play Mode, see the conditions in ShouldDiscardEditModeTransitionEvent(). Let me know if I'm missing something.

In theory, if we had focus events in the queues, we wouldn't need to know about timestamps. However, as we discussed, there is a higher risk in introducing such a change. We should still conduct an exploration of the implementation, but I would prefer to land first a less risky solution for this particular bug.

I'm pappy to prototype a solution together if you like, after this fix. We can sync on Slack.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use timestamps than what we had before and in practise it's likely not a problem. Still safe to land as is though and guess we need it in the queue to have a strictly accurate implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can also add an inline comment to this test case that with a strictly defined event queue containing all relevant inputs without downsampling the test case should be expanded to be time-agnostic for a fixed timestamp?

{
// Note: We currently test against timestamps otherwise the test fails. But, ideally, we wouldn't need to.
// If we ever reach a point of having all relevant input events in the queue (including focus events) we
// could just rely on order of event. Which means this test work for a fixed timestamp and it should
// changed accordingly.
currentTime += 1.0f;
runtime.PlayerFocusLost();
currentTime += 1.0f;
// Queuing an event like it would be in the editor when the GameView is out of focus.
Set(mouse.position, new Vector2(0.234f, 0.345f) , queueEventOnly: true);
currentTime += 1.0f;
// Gaining focus like it would happen in the editor when the GameView regains focus.
runtime.PlayerFocusGained();
currentTime += 1.0f;
// This emulates a device sync that happens when the player regains focus through an IOCTL command.
// That's why it also has it's time incremented.
Set(mouse.position, new Vector2(1.0f, 2.0f), queueEventOnly: true);
currentTime += 1.0f;
// This update should not trigger any ction as it's an editor update.
InputSystem.Update(InputUpdateType.Editor);
currentTime += 1.0f;

var actions = trace.ToArray();
Assert.That(actions, Has.Length.EqualTo(0));
// This update should trigger an action with regards to the event queued after focus was regained.
// The one queued while out of focus should have been ignored and we should expect only one action triggered.
// Unless background behavior is set to IgnoreFocus in which case both events should trigger the action.
InputSystem.Update(InputUpdateType.Dynamic);

actions = trace.ToArray();
Assert.That(actions, Has.Length.EqualTo(backgroundBehavior == InputSettings.BackgroundBehavior.IgnoreFocus ? 2 : 1));
Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
Vector2Control control = (Vector2Control)actions[0].control;
// Make sure the value is from the event after focus was regained.
Assert.That(control.value, Is.EqualTo(new Vector2(1.0f, 2.0f)).Using(Vector2EqualityComparer.Instance));
}
}

[Test]
[Category("Actions")]
public void Actions_TimeoutsDoNotGetTriggeredInEditorUpdates()
Expand Down
66 changes: 66 additions & 0 deletions Assets/Tests/InputSystem/Plugins/InputForUITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,72 @@ public void ActionWithDifferentExpectedControlType_ShouldGenerateWarning(string
LogAssert.NoUnexpectedReceived();
}

[Test]
[Category("Actions")]
[Description("Tests that that only the latest event after focus is regained is able to trigger the action." +
"Depends on background behavior. " +
"Similar to CoreTests_Actions.Actions_DoNotGetTriggeredByOutOfFocusEventInEditor but with InputForUI nuances.")]
[TestCase(InputSettings.BackgroundBehavior.IgnoreFocus)]
[TestCase(InputSettings.BackgroundBehavior.ResetAndDisableNonBackgroundDevices)]
[TestCase(InputSettings.BackgroundBehavior.ResetAndDisableAllDevices)]
public void UIActions_DoNotGetTriggeredByOutOfFocusEventInEditor(InputSettings.BackgroundBehavior backgroundBehavior)
{
InputSystem.settings.backgroundBehavior = backgroundBehavior;
var mouse = InputSystem.AddDevice<Mouse>();

Vector2 focusPosition = new Vector2(800f, 600f);
Vector2 outOfFocusPosition = new Vector2(100f, 500f);

// Simulate moving the mouse, losing focus, moving out of focus, regaining focus, and moving again to emulate
// a device sync that happens when regaining focus in play mode.
Update();
currentTime += 1.0f;
Set(mouse.position, new Vector2(1.0f, 1.0f), queueEventOnly: true);
currentTime += 1.0f;
Update();
currentTime += 1.0f;
runtime.PlayerFocusLost();
currentTime += 1.0f;
Set(mouse.position, outOfFocusPosition , queueEventOnly: true);
currentTime += 1.0f;
runtime.PlayerFocusGained();
currentTime += 1.0f;
Set(mouse.position, focusPosition, queueEventOnly: true);
currentTime += 1.0f;

// We call specific updates to simulate editor behavior when regaining focus.
InputSystem.Update(InputUpdateType.Editor);
Assert.AreEqual(0, m_InputForUIEvents.Count);
InputSystem.Update();
// Calling the event provider update after we call InputSystem updates so that we trigger InputForUI events
EventProvider.NotifyUpdate();

// Convert the input coordinates to UI panel space. We only assume 1 display for the tests.
var focusPositionInUI = InputSystemProvider.ScreenBottomLeftToPanelPosition(focusPosition, 0);
var outOfFocusPositionInUI = InputSystemProvider.ScreenBottomLeftToPanelPosition(outOfFocusPosition, 0);


// If we don't ignore focus, we only expect one event (the last one after focus is regained).
Assert.AreEqual(backgroundBehavior != InputSettings.BackgroundBehavior.IgnoreFocus ? 1 : 2, m_InputForUIEvents.Count);

// There will be an out of focus event only if we are ignoring focus. Validate its position.
if (backgroundBehavior == InputSettings.BackgroundBehavior.IgnoreFocus)
{
Assert.That(GetNextRecordedUIEvent() is
{
type: Event.Type.PointerEvent,
asPointerEvent: { type: PointerEvent.Type.PointerMoved, eventSource: EventSource.Mouse, position: var outOfFocusVector2 },
} && Mathf.Approximately(outOfFocusVector2.y, outOfFocusPositionInUI.y) && Mathf.Approximately(outOfFocusVector2.x, outOfFocusPositionInUI.x));
}

// Validate that we only we get the event for the position after focus is regained. Make sure its position is correct.
Assert.That(GetNextRecordedUIEvent() is
{
type: Event.Type.PointerEvent,
asPointerEvent: { type: PointerEvent.Type.PointerMoved, eventSource: EventSource.Mouse, position: var focusPositionVector2 },
} && Mathf.Approximately(focusPositionVector2.y, focusPositionInUI.y) && Mathf.Approximately(focusPositionVector2.x, focusPositionInUI.x));
}

#endif // UNITY_EDITOR

[Test]
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ however, it has to be formatted properly to pass verification tests.
- Fixed an issue where the action icon would shrink or disappear from UI when an action has a very long name. [ISXB-1650](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1650).
- Fixed upgrading input actions containing multiple processors [ISXB-1674](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1674).
- Fixed an issue that led to non-deterministic behaviour during `.inputaction` asset imports that could lead to corrupted assets or Unity hanging during asset import when "Generate C# Scripts" was active in importer settings. [ISXB-1746](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1746)
- An issue where a UITK MouseEvent was triggered when changing from Scene View to Game View in the Editor has been fixed. [ISXB-1671](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1671)

## [1.15.0] - 2025-10-03

Expand Down
Loading