-
Notifications
You must be signed in to change notification settings - Fork 328
FIX: MouseDownEvent is triggered when changing from Scene view to Game view (ISXB-1671) #2234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
6157498
300896c
f78e947
f1ed615
9023b74
510730e
ac75141
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -642,6 +642,55 @@ public void Actions_DoNotGetTriggeredByEditorUpdates() | |
} | ||
} | ||
|
||
[Test] | ||
[Category("Actions")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still need to add a InputForUI specific test for this. |
||
[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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I would recommend adding that test as well. |
||
{ | ||
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() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ however, it has to be formatted properly to pass verification tests. | |
- Fixed the compilation warnings when used with Unity 6.4 (ISX-2349). | ||
- 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick. Double space before ISXB number. |
||
|
||
## [1.14.2] - 2025-08-05 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2244,6 +2244,8 @@ | |
private bool m_NativeBeforeUpdateHooked; | ||
private bool m_HaveDevicesWithStateCallbackReceivers; | ||
private bool m_HasFocus; | ||
private bool m_DiscardOutOfFocusEvents; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit of a corner case, but what happens with these if there is a domain reload while in playmode? Will they be reinitialised to the same values they had before the domain reload was initiated? |
||
private double m_FocusRegainedTime; | ||
private InputEventStream m_InputEventStream; | ||
|
||
// We want to sync devices when the editor comes back into focus. Unfortunately, there's no | ||
|
@@ -3032,6 +3034,8 @@ | |
} | ||
else | ||
{ | ||
m_DiscardOutOfFocusEvents = true; | ||
m_FocusRegainedTime = m_Runtime.currentTime; | ||
// On focus gain, reenable and sync devices. | ||
for (var i = 0; i < m_DevicesCount; ++i) | ||
{ | ||
|
@@ -3201,50 +3205,18 @@ | |
var timesliceEvents = (updateType == InputUpdateType.Fixed || updateType == InputUpdateType.BeforeRender) && | ||
InputSystem.settings.updateMode == InputSettings.UpdateMode.ProcessEventsInFixedUpdate; | ||
|
||
// Figure out if we can just flush the buffer and early out. | ||
var canFlushBuffer = | ||
false | ||
#if UNITY_EDITOR | ||
// If out of focus and runInBackground is off and ExactlyAsInPlayer is on, discard input. | ||
|| (!gameHasFocus && m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDeviceInputAlwaysGoesToGameView && | ||
(!m_Runtime.runInBackground || | ||
m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.ResetAndDisableAllDevices)) | ||
#else | ||
|| (!gameHasFocus && !m_Runtime.runInBackground) | ||
#endif | ||
; | ||
var canEarlyOut = | ||
// Early out if there's no events to process. | ||
eventBuffer.eventCount == 0 | ||
|| canFlushBuffer | ||
|
||
#if UNITY_EDITOR | ||
// If we're in the background and not supposed to process events in this update (but somehow | ||
// still ended up here), we're done. | ||
|| ((!gameHasFocus || gameShouldGetInputRegardlessOfFocus) && | ||
((m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.ResetAndDisableAllDevices && updateType != InputUpdateType.Editor) | ||
|| (m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDevicesRespectGameViewFocus && updateType != InputUpdateType.Editor) | ||
|| (m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.IgnoreFocus && m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDeviceInputAlwaysGoesToGameView && updateType == InputUpdateType.Editor) | ||
) | ||
// When the game is playing and has focus, we never process input in editor updates. All we | ||
// do is just switch to editor state buffers and then exit. | ||
|| (gameIsPlaying && gameHasFocus && updateType == InputUpdateType.Editor)) | ||
#endif | ||
; | ||
|
||
// Determine if we should flush the event buffer which would imply we exit early and do not process | ||
// any of those events, ever. | ||
var shouldFlushEventBuffer = ShouldFlushEventBuffer(); | ||
// When we exit early, we may or may not flush the event buffer. It depends if we want to process events | ||
// later once this method is called. | ||
var shouldExitEarly = ShouldExitEarlyFromEventProcessing(eventBuffer, shouldFlushEventBuffer, updateType); | ||
|
||
#if UNITY_EDITOR | ||
var dropStatusEvents = false; | ||
if (!gameIsPlaying && gameShouldGetInputRegardlessOfFocus && (eventBuffer.sizeInBytes > (100 * 1024))) | ||
{ | ||
// If the game is not playing but we're sending all input events to the game, the buffer can just grow unbounded. | ||
// So, in that case, set a flag to say we'd like to drop status events, and do not early out. | ||
canEarlyOut = false; | ||
dropStatusEvents = true; | ||
} | ||
var dropStatusEvents = ShouldDropStatusEvents(eventBuffer, ref shouldExitEarly); | ||
#endif | ||
|
||
if (canEarlyOut) | ||
if (shouldExitEarly) | ||
{ | ||
// Normally, we process action timeouts after first processing all events. If we have no | ||
// events, we still need to check timeouts. | ||
|
@@ -3253,7 +3225,7 @@ | |
|
||
k_InputUpdateProfilerMarker.End(); | ||
InvokeAfterUpdateCallback(updateType); | ||
if (canFlushBuffer) | ||
if (shouldFlushEventBuffer) | ||
eventBuffer.Reset(); | ||
m_CurrentUpdate = default; | ||
return; | ||
|
@@ -3317,25 +3289,9 @@ | |
|
||
continue; | ||
} | ||
#endif | ||
|
||
// In the editor, we discard all input events that occur in-between exiting edit mode and having | ||
// entered play mode as otherwise we'll spill a bunch of UI events that have occurred while the | ||
// UI was sort of neither in this mode nor in that mode. This would usually lead to the game receiving | ||
// an accumulation of spurious inputs right in one of its first updates. | ||
// | ||
// NOTE: There's a chance the solution here will prove inadequate on the long run. We may do things | ||
// here such as throwing partial touches away and then letting the rest of a touch go through. | ||
// Could be that ultimately we need to issue a full reset of all devices at the beginning of | ||
// play mode in the editor. | ||
#if UNITY_EDITOR | ||
if ((currentEventType == StateEvent.Type || | ||
currentEventType == DeltaStateEvent.Type) && | ||
(updateType & InputUpdateType.Editor) == 0 && | ||
InputSystem.s_SystemObject.exitEditModeTime > 0 && | ||
currentEventTimeInternal >= InputSystem.s_SystemObject.exitEditModeTime && | ||
(currentEventTimeInternal < InputSystem.s_SystemObject.enterPlayModeTime || | ||
InputSystem.s_SystemObject.enterPlayModeTime == 0)) | ||
// Decide to skip events based on timing or focus state | ||
if (ShouldDiscardEventInEditor(currentEventType, currentEventTimeInternal, updateType)) | ||
{ | ||
m_InputEventStream.Advance(false); | ||
continue; | ||
|
@@ -3695,6 +3651,8 @@ | |
throw; | ||
} | ||
|
||
m_DiscardOutOfFocusEvents = false; | ||
|
||
if (shouldProcessActionTimeouts) | ||
ProcessStateChangeMonitorTimeouts(); | ||
|
||
|
@@ -3706,6 +3664,160 @@ | |
m_CurrentUpdate = default; | ||
} | ||
|
||
/// <summary> | ||
/// Determines if the event buffer should be flushed without processing events. | ||
/// </summary> | ||
/// <returns>True if the buffer should be flushed, false otherwise.</returns> | ||
private bool ShouldFlushEventBuffer() | ||
{ | ||
#if UNITY_EDITOR | ||
// If out of focus and runInBackground is off and ExactlyAsInPlayer is on, discard input. | ||
if (!gameHasFocus && | ||
m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDeviceInputAlwaysGoesToGameView | ||
&& | ||
(!m_Runtime.runInBackground || m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.ResetAndDisableAllDevices)) | ||
return true; | ||
#else | ||
// In player builds, flush if out of focus and not running in background | ||
if (!gameHasFocus && !m_Runtime.runInBackground) | ||
return true; | ||
#endif | ||
return false; | ||
} | ||
|
||
/// <summary> | ||
/// Determines if we should exit early from event processing without handling events. | ||
/// </summary> | ||
/// <param name="eventBuffer">The current event buffer</param> | ||
/// <param name="canFlushBuffer">Whether the buffer can be flushed</param> | ||
/// <param name="updateType">The current update type</param> | ||
/// <returns>True if we should exit early, false otherwise.</returns> | ||
private bool ShouldExitEarlyFromEventProcessing(InputEventBuffer eventBuffer, bool canFlushBuffer, InputUpdateType updateType) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer that we do not pass canFlushBuffer to this function, nor eventBuffer. Those two is the obvious things to check in the outer scope. E.g. var shouldExitEarly = eventBuffer.eventCount == 0 || canFlushBuffer || ShouldExitEarlyFromEventProcessing(updateType); |
||
{ | ||
// Early out if there are no events to process | ||
if (eventBuffer.eventCount == 0) | ||
return true; | ||
|
||
// Early out if we can flush the buffer | ||
if (canFlushBuffer) | ||
return true; | ||
|
||
#if UNITY_EDITOR | ||
// Check various PlayMode specific early exit conditions | ||
if (ShouldExitEarlyInEditor(updateType)) | ||
return true; | ||
|
||
// When the game is playing and has focus, we never process input in editor updates. | ||
// All we do is just switch to editor state buffers and then exit. | ||
if ((gameIsPlaying && gameHasFocus && updateType == InputUpdateType.Editor)) | ||
return true; | ||
#endif | ||
|
||
return false; | ||
} | ||
|
||
#if UNITY_EDITOR | ||
/// <summary> | ||
/// Checks editor-specific conditions for early exit from event processing. | ||
/// </summary> | ||
/// <param name="updateType">The current update type</param> | ||
/// <returns>True if we should exit early in editor context, false otherwise.</returns> | ||
/// <remarks> | ||
/// Whenever this method returns true, it usually means that events are left in the buffer and should be | ||
/// processed in a next update call. | ||
/// </remarks> | ||
private bool ShouldExitEarlyInEditor(InputUpdateType updateType) | ||
{ | ||
// In Play Mode, if we're in the background and not supposed to process events in this update | ||
if ((!gameHasFocus || gameShouldGetInputRegardlessOfFocus) && updateType != InputUpdateType.Editor) | ||
{ | ||
if (m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.ResetAndDisableAllDevices || | ||
m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDevicesRespectGameViewFocus) | ||
return true; | ||
} | ||
|
||
// Special case for IgnoreFocus behavior with AllDeviceInputAlwaysGoesToGameView in editor updates | ||
if ((!gameHasFocus || gameShouldGetInputRegardlessOfFocus) && | ||
m_Settings.backgroundBehavior == InputSettings.BackgroundBehavior.IgnoreFocus && | ||
m_Settings.editorInputBehaviorInPlayMode == InputSettings.EditorInputBehaviorInPlayMode.AllDeviceInputAlwaysGoesToGameView && | ||
updateType == InputUpdateType.Editor) | ||
return true; | ||
|
||
return false; | ||
} | ||
|
||
/// <summary> | ||
/// Determines if status events should be dropped and modifies early exit behavior accordingly. | ||
/// </summary> | ||
/// <param name="eventBuffer">The current event buffer</param> | ||
/// <param name="canEarlyOut">Reference to the early exit flag that may be modified</param> | ||
/// <returns>True if status events should be dropped, false otherwise.</returns> | ||
private bool ShouldDropStatusEvents(InputEventBuffer eventBuffer, ref bool canEarlyOut) | ||
{ | ||
// If the game is not playing but we're sending all input events to the game, | ||
// the buffer can just grow unbounded. So, in that case, set a flag to say we'd | ||
// like to drop status events, and do not early out. | ||
if (!gameIsPlaying && gameShouldGetInputRegardlessOfFocus && (eventBuffer.sizeInBytes > (100 * 1024))) | ||
{ | ||
canEarlyOut = false; | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
/// <summary> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for documenting complex internal functions. I think its helpful. |
||
/// Determines if an event should be discarded based on timing or focus state. | ||
/// </summary> | ||
/// <param name="eventType">The type of the current event</param> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: I would advise against using the term "current" here. It is just an event being evaluated right? |
||
/// <param name="eventTime">The internal time of the current event</param> | ||
/// <param name="updateType">The current update type</param> | ||
/// <returns>True if the event should be discarded, false otherwise.</returns> | ||
private bool ShouldDiscardEventInEditor(FourCC eventType, double eventTime, InputUpdateType updateType) | ||
{ | ||
// Check if this is an event that occurred during edit mode transition | ||
if (ShouldDiscardEditModeTransitionEvent(eventType, eventTime, updateType)) | ||
return true; | ||
|
||
// Check if this is an out-of-focus event that should be discarded | ||
if (ShouldDiscardOutOfFocusEvent(eventTime)) | ||
return true; | ||
|
||
return false; | ||
} | ||
|
||
/// <summary> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally this makes sense, but have you tested that the following scenario works. |
||
/// In the editor, we discard all input events that occur in-between exiting edit mode and having | ||
/// entered play mode as otherwise we'll spill a bunch of UI events that have occurred while the | ||
/// UI was sort of neither in this mode nor in that mode. This would usually lead to the game receiving | ||
/// an accumulation of spurious inputs right in one of its first updates. | ||
/// | ||
/// NOTE: There's a chance the solution here will prove inadequate on the long run. We may do things | ||
/// here such as throwing partial touches away and then letting the rest of a touch go through. | ||
/// Could be that ultimately we need to issue a full reset of all devices at the beginning of | ||
/// play mode in the editor. | ||
/// </summary> | ||
private bool ShouldDiscardEditModeTransitionEvent(FourCC eventType, double eventTime, InputUpdateType updateType) | ||
{ | ||
return (eventType == StateEvent.Type || eventType == DeltaStateEvent.Type) && | ||
(updateType & InputUpdateType.Editor) == 0 && | ||
InputSystem.s_SystemObject.exitEditModeTime > 0 && | ||
eventTime >= InputSystem.s_SystemObject.exitEditModeTime && | ||
(eventTime < InputSystem.s_SystemObject.enterPlayModeTime || | ||
InputSystem.s_SystemObject.enterPlayModeTime == 0); | ||
} | ||
|
||
/// <summary> | ||
/// Checks if an event should be discarded because it occurred while out of focus, under specific settings. | ||
/// </summary> | ||
private bool ShouldDiscardOutOfFocusEvent(double eventTime) | ||
{ | ||
if (gameHasFocus && m_Settings.backgroundBehavior != InputSettings.BackgroundBehavior.IgnoreFocus) | ||
return m_DiscardOutOfFocusEvents && eventTime < m_FocusRegainedTime; | ||
return false; | ||
} | ||
|
||
#endif | ||
|
||
bool AreMaximumEventBytesPerUpdateExceeded(uint totalEventBytesProcessed) | ||
{ | ||
if (m_Settings.maxEventBytesPerUpdate > 0 && | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this PR modifying the template?