Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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.
49 changes: 49 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,55 @@ 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. ")]
[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?

{
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;
Assert.That(control.value, Is.EqualTo(new Vector2(1.0f, 2.0f)).Using(Vector2EqualityComparer.Instance));
}
}

[Test]
[Category("Actions")]
public void Actions_TimeoutsDoNotGetTriggeredInEditorUpdates()
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 @@ -26,6 +26,7 @@ however, it has to be formatted properly to pass verification tests.
- Fixed an issue in `GamepadIconExample` which resulted in icons for left and right triggers not being displayed after a rebind to the exact same controls. ISXB-1593.
- Fixed an issue where `InputSystemUIInputModule.localMultiPlayerRoot` could not be set to `null` when using `MultiplayerEventSystem`. [ISXB-1610](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1610)
- Fixed an issue in `Keyboard` where the sub-script operator would return a `null` key control for the deprecated key `Key.IMESelected`. Now, an aliased `KeyControl`mapping to the IMESelected bit is returned for compability reasons. It is still strongly advised to not rely on this key since `IMESelected` bit isn't strictly a key and will be removed from the `Key` enumeration type in a future major revision. [ISXB-1541](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1541).
- 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.14.2] - 2025-08-05

Expand Down
Loading