Skip to content

Commit eeb7e1c

Browse files
fixed failing tests, processed feedback
1 parent eb257b0 commit eeb7e1c

9 files changed

Lines changed: 62 additions & 51 deletions

File tree

Assets/Tests/InputSystem/Plugins/EnhancedTouchTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,11 @@ public void EnhancedTouch_SupportsEditorUpdates(InputSettings.UpdateMode updateM
172172
// Switch back to player.
173173
ScheduleFocusEvent(true);
174174

175-
// We have to schedule the specific update type that the player is configured to process events in,
176-
// otherwise we will end up using defaultUpdateType, which due to the fact we do not have focus in pre-update yet,
177-
// will be Editor, which means that we will end up swapping buffers to the editor buffer, and retreiving the wrong active touch
178-
// The only way to properly fix this, is to remove defaultUpdateType, and split the player/editor update loops into separate methods, which would be a breaking change.
179-
// Until then, we have to make sure to schedule the correct update type here.
175+
// Explicitly schedule the player's configured update type rather than relying on the default.
176+
// Without explicit scheduling, defaultUpdateType would be Editor (since focus has not yet been
177+
// gained during update), causing the editor buffer to be used instead of the player buffer,
178+
// which would retrieve the wrong active touch. A proper fix would require removing defaultUpdateType
179+
// and splitting player/editor update loops into separate methods.
180180
switch (updateMode)
181181
{
182182
case InputSettings.UpdateMode.ProcessEventsInDynamicUpdate:

Assets/Tests/InputSystem/Plugins/UserTests.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,14 +1268,19 @@ public void Users_DoNotReactToEditorInput()
12681268
++InputUser.listenForUnpairedDeviceActivity;
12691269
InputUser.onUnpairedDeviceUsed += (control, eventPtr) => Assert.Fail("Should not react!");
12701270

1271-
// We now have to run a dynamic update before the press, to make sure the focus state is up to date.
1272-
// This is due to scheduled focus events are not being processed in pre-update, and not running an update before the press would result in the incorrect state.
1273-
// When calling an empty Update(), it will use the default defaultUpdateType to determine which update type to use.
1274-
// However in pre-update the focus will not have switched to false yet if not running a dynamic update before, so the focus check in defaultUpdateType is no longer correct.
1275-
// which would cause it to schedule a dynamic update instead. We solve this by first processing the focus event before pressing the button,
1276-
// which will then make it correctly swap to an editor update type when doing the button press.
1277-
// Another way to make this test pass, is to queue the button press and then explicitly call an editor update and process both events
1278-
// The way to solve this is to remove defaultUpdateType and split the editor/player loops or to make sure we do not call any Update() without update type, so we do not use defaultUpdateType.
1271+
// Process the focus event before pressing the button to ensure correct update type selection.
1272+
//
1273+
// Issue: When Update() is called without an update type, it uses defaultUpdateType which checks
1274+
// focus state. However, scheduled focus events aren't processed until an update runs, so the
1275+
// focus check sees stale state and selects the wrong update type.
1276+
//
1277+
// Workaround: Run a dynamic update first to process the focus event, ensuring the subsequent
1278+
// button press correctly uses editor update type.
1279+
//
1280+
// Alternative: Queue the button press and explicitly call an editor update to process both events.
1281+
//
1282+
// Proper fix: Remove defaultUpdateType and split editor/player loops, or always specify the
1283+
// update type explicitly when calling Update().
12791284
ScheduleFocusEvent(false);
12801285
InputSystem.Update(InputUpdateType.Dynamic);
12811286
Press(gamepad.buttonSouth);

Packages/com.unity.inputsystem/InputSystem/LegacyInputManager.cs renamed to Packages/com.unity.inputsystem/InputSystem/InputManager.LegacyFocusHandling.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal enum FocusFlags : ushort
2020
None = 0,
2121

2222
/// <summary>
23-
/// The application has focus.
23+
/// In editor this means the GameView has focus. In a built player this means the player has focus.
2424
/// </summary>
2525
ApplicationFocus = (1 << 0)
2626
};

Packages/com.unity.inputsystem/InputSystem/InputManager.LegacyFocusHandling.cs.meta

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ public bool runPlayerUpdatesInEditMode
508508
#else
509509
true;
510510
#endif
511-
private bool applicationHasFocus => (focusState & FocusFlags.ApplicationFocus) != 0;
511+
private bool applicationHasFocus => (focusState & FocusFlags.ApplicationFocus) != FocusFlags.None;
512512

513513
private bool gameHasFocus =>
514514
#if UNITY_EDITOR
@@ -1973,10 +1973,12 @@ internal void InitializeData()
19731973
// can manually turn off one of them to optimize operation.
19741974
m_UpdateMask = InputUpdateType.Dynamic | InputUpdateType.Fixed;
19751975
#if !UNITY_INPUTSYSTEM_SUPPORTS_FOCUS_EVENTS
1976-
m_FocusState = FocusFlags.None;
1976+
m_FocusState = Application.isFocused
1977+
? m_FocusState | FocusFlags.ApplicationFocus
1978+
: m_FocusState & ~FocusFlags.ApplicationFocus;
19771979
#endif
19781980

1979-
#if UNITY_EDITOR
1981+
#if UNITY_EDITOR
19801982
m_EditorIsActive = true;
19811983
m_UpdateMask |= InputUpdateType.Editor;
19821984
#endif
@@ -2221,7 +2223,7 @@ internal void InstallRuntime(IInputRuntime runtime)
22212223
#endif
22222224
m_Runtime.pollingFrequency = pollingFrequency;
22232225

2224-
focusState = Application.isFocused
2226+
focusState = m_Runtime.isPlayerFocused
22252227
? focusState | FocusFlags.ApplicationFocus
22262228
: focusState & ~FocusFlags.ApplicationFocus;
22272229

@@ -2380,7 +2382,7 @@ internal struct AvailableDevice
23802382
private bool m_NativeBeforeUpdateHooked;
23812383
private bool m_HaveDevicesWithStateCallbackReceivers;
23822384
#if !UNITY_INPUTSYSTEM_SUPPORTS_FOCUS_EVENTS
2383-
private FocusFlags m_FocusState;
2385+
private FocusFlags m_FocusState = FocusFlags.ApplicationFocus;
23842386
private bool m_DiscardOutOfFocusEvents;
23852387
private double m_FocusRegainedTime;
23862388
#endif
@@ -2829,13 +2831,13 @@ private void InstallBeforeUpdateHookIfNecessary()
28292831

28302832
private void RestoreDevicesAfterDomainReloadIfNecessary()
28312833
{
2832-
#if UNITY_EDITOR && !ENABLE_CORECLR
2834+
#if UNITY_EDITOR && !ENABLE_CORECLR
28332835
if (m_SavedDeviceStates != null)
28342836
RestoreDevicesAfterDomainReload();
28352837
#endif
28362838
}
28372839

2838-
#if UNITY_EDITOR
2840+
#if UNITY_EDITOR
28392841
private void SyncAllDevicesWhenEditorIsActivated()
28402842
{
28412843
var isActive = m_Runtime.isEditorActive;
@@ -2868,7 +2870,7 @@ internal void SyncAllDevicesAfterEnteringPlayMode()
28682870
SyncAllDevices();
28692871
}
28702872

2871-
#endif // UNITY_EDITOR
2873+
#endif // UNITY_EDITOR
28722874

28732875
private void WarnAboutDevicesFailingToRecreateAfterDomainReload()
28742876
{
@@ -2888,7 +2890,7 @@ private void WarnAboutDevicesFailingToRecreateAfterDomainReload()
28882890
// At this point, we throw the device states away and forget about
28892891
// what we had before the domain reload.
28902892
m_SavedDeviceStates = null;
2891-
#endif // UNITY_EDITOR
2893+
#endif // UNITY_EDITOR
28922894
}
28932895

28942896
private void OnBeforeUpdate(InputUpdateType updateType)
@@ -3105,7 +3107,7 @@ private bool ShouldRunDeviceInBackground(InputDevice device)
31053107
device.canRunInBackground;
31063108
}
31073109

3108-
#if UNITY_EDITOR
3110+
#if UNITY_EDITOR
31093111
internal void LeavePlayMode()
31103112
{
31113113
// Reenable all devices and reset their play mode state.
@@ -3133,7 +3135,7 @@ private void OnPlayerLoopInitialization()
31333135
InputStateBuffers.SwitchTo(m_StateBuffers, InputUpdate.s_LatestUpdateType);
31343136
}
31353137

3136-
#endif // UNITY_EDITOR
3138+
#endif // UNITY_EDITOR
31373139

31383140
internal bool ShouldRunUpdate(InputUpdateType updateType)
31393141
{
@@ -3144,7 +3146,7 @@ internal bool ShouldRunUpdate(InputUpdateType updateType)
31443146

31453147
var mask = m_UpdateMask;
31463148

3147-
#if UNITY_EDITOR
3149+
#if UNITY_EDITOR
31483150
// If the player isn't running, the only thing we run is editor updates, except if
31493151
// explicitly overriden via `runUpdatesInEditMode`.
31503152
// NOTE: This means that in edit mode (outside of play mode) we *never* switch to player
@@ -3153,7 +3155,7 @@ internal bool ShouldRunUpdate(InputUpdateType updateType)
31533155
// it will see gamepad inputs going to the editor and respond to them.
31543156
if (!gameIsPlaying && updateType != InputUpdateType.Editor && !runPlayerUpdatesInEditMode)
31553157
return false;
3156-
#endif // UNITY_EDITOR
3158+
#endif // UNITY_EDITOR
31573159

31583160
return (updateType & mask) != 0;
31593161
}
@@ -3314,7 +3316,7 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33143316
}
33153317
#endif
33163318
#else
3317-
#if UNITY_EDITOR
3319+
#if UNITY_EDITOR
33183320
if (dropStatusEvents)
33193321
{
33203322
// If the type here is a status event, ask advance not to leave the event in the buffer. Otherwise, leave it there.
@@ -3332,7 +3334,7 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33323334
m_InputEventStream.Advance(false);
33333335
continue;
33343336
}
3335-
#endif
3337+
#endif
33363338
#endif
33373339
// If we're timeslicing, check if the event time is within limits.
33383340
if (timesliceEvents && currentEventTimeInternal >= currentTime)
@@ -3346,7 +3348,7 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33463348
device = TryGetDeviceById(currentEventReadPtr->deviceId);
33473349
if (device == null && currentEventType != focusEventType)
33483350
{
3349-
#if UNITY_EDITOR
3351+
#if UNITY_EDITOR
33503352
////TODO: see if this is a device we haven't created and if so, just ignore
33513353
m_Diagnostics?.OnCannotFindDeviceForEvent(new InputEventPtr(currentEventReadPtr));
33523354
#endif
@@ -3355,11 +3357,11 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33553357
continue;
33563358
}
33573359

3358-
#if UNITY_EDITOR
3360+
#if UNITY_EDITOR
33593361
// In the editor, route keyboard/pointer events between Editor/Player updates if required.
33603362
if (ShouldDeferEventBetweenEditorAndPlayerUpdates(updateType, currentEventType, device))
33613363
continue;
3362-
#endif // UNITY_EDITOR
3364+
#endif // UNITY_EDITOR
33633365

33643366
// If device is disabled, we let the event through only in certain cases.
33653367
// Removal and configuration change events should always be processed.
@@ -3369,12 +3371,12 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33693371
(device.m_DeviceFlags & (InputDevice.DeviceFlags.DisabledInRuntime |
33703372
InputDevice.DeviceFlags.DisabledWhileInBackground)) != 0)
33713373
{
3372-
#if UNITY_EDITOR
3374+
#if UNITY_EDITOR
33733375
// If the device is disabled in the backend, getting events for them
33743376
// is something that indicates a problem in the backend so diagnose.
33753377
if ((device.m_DeviceFlags & InputDevice.DeviceFlags.DisabledInRuntime) != 0)
33763378
m_Diagnostics?.OnEventForDisabledDevice(currentEventReadPtr, device);
3377-
#endif
3379+
#endif
33783380

33793381
m_InputEventStream.Advance(false);
33803382
continue;
@@ -3390,16 +3392,16 @@ private unsafe void ProcessEventBuffer(InputUpdateType updateType, ref InputEven
33903392
// Give the device a chance to do something with data before we propagate it to event listeners.
33913393
if (device != null && device.hasEventPreProcessor)
33923394
{
3393-
#if UNITY_EDITOR
3395+
#if UNITY_EDITOR
33943396
var eventSizeBeforePreProcessor = currentEventReadPtr->sizeInBytes;
3395-
#endif
3397+
#endif
33963398
var shouldProcess = ((IEventPreProcessor)device).PreProcessEvent(currentEventReadPtr);
3397-
#if UNITY_EDITOR
3399+
#if UNITY_EDITOR
33983400
if (currentEventReadPtr->sizeInBytes > eventSizeBeforePreProcessor)
33993401
{
34003402
throw new AccessViolationException($"'{device}'.PreProcessEvent tries to grow an event from {eventSizeBeforePreProcessor} bytes to {currentEventReadPtr->sizeInBytes} bytes, this will potentially corrupt events after the current event and/or cause out-of-bounds memory access.");
34013403
}
3402-
#endif
3404+
#endif
34033405
if (!shouldProcess)
34043406
{
34053407
// Skip event if PreProcessEvent considers it to be irrelevant.
@@ -3750,9 +3752,9 @@ private unsafe void ProcessStateEvent(InputDevice device, InputUpdateType update
37503752
// If the state format doesn't match, ignore the event.
37513753
if (device.stateBlock.format != eventPtr.stateFormat)
37523754
{
3753-
#if UNITY_EDITOR
3755+
#if UNITY_EDITOR
37543756
m_Diagnostics?.OnEventFormatMismatch(currentEventReadPtr, device);
3755-
#endif
3757+
#endif
37563758
return;
37573759
}
37583760

@@ -3767,9 +3769,9 @@ private unsafe void ProcessStateEvent(InputDevice device, InputUpdateType update
37673769
// Only events should. If running play mode updates in editor, we want to defer to the play mode
37683770
// callbacks to set the last update time to avoid dropping events only processed by the editor state.
37693771
if (device.m_LastUpdateTimeInternal <= eventPtr.internalTime
3770-
#if UNITY_EDITOR
3772+
#if UNITY_EDITOR
37713773
&& !(updateType == InputUpdateType.Editor && runPlayerUpdatesInEditMode)
3772-
#endif
3774+
#endif
37733775
)
37743776
device.m_LastUpdateTimeInternal = eventPtr.internalTime;
37753777

@@ -4382,7 +4384,7 @@ private bool FlipBuffersForDeviceIfNecessary(InputDevice device, InputUpdateType
43824384
return false;
43834385
}
43844386

4385-
#if UNITY_EDITOR
4387+
#if UNITY_EDITOR
43864388
////REVIEW: should this use the editor update ticks as quasi-frame-boundaries?
43874389
// Updates go to the editor only if the game isn't playing or does not have focus.
43884390
// Otherwise we fall through to the logic that flips for the *next* dynamic and
@@ -4396,7 +4398,7 @@ private bool FlipBuffersForDeviceIfNecessary(InputDevice device, InputUpdateType
43964398
m_StateBuffers.m_EditorStateBuffers.SwapBuffers(device.m_DeviceIndex);
43974399
return true;
43984400
}
4399-
#endif
4401+
#endif
44004402

44014403
// Flip buffers if we haven't already for this frame.
44024404
if (device.m_CurrentUpdateStepCount != InputUpdate.s_UpdateStepCount)
@@ -4414,7 +4416,7 @@ private bool FlipBuffersForDeviceIfNecessary(InputDevice device, InputUpdateType
44144416

44154417
// Stuff everything that we want to survive a domain reload into
44164418
// a m_SerializedState.
4417-
#if UNITY_EDITOR || DEVELOPMENT_BUILD
4419+
#if UNITY_EDITOR || DEVELOPMENT_BUILD
44184420
[Serializable]
44194421
internal struct DeviceState
44204422
{

Packages/com.unity.inputsystem/InputSystem/LegacyInputManager.cs.meta

Lines changed: 0 additions & 2 deletions
This file was deleted.

Packages/com.unity.inputsystem/InputSystem/NativeInputRuntime.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ public FocusFlags focusState
234234
get => m_FocusState;
235235
set => m_FocusState = value;
236236
}
237-
public bool isPlayerFocused => (m_FocusState & FocusFlags.ApplicationFocus) != 0;
237+
public bool isPlayerFocused => (m_FocusState & FocusFlags.ApplicationFocus) != FocusFlags.None;
238238

239239
public float pollingFrequency
240240
{

Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,7 @@ public void Trigger(InputAction action)
844844
/// <summary>
845845
/// Utility function for manually scheduling an InputFocusEvent.
846846
/// This is useful for testing how the system reacts to focus changes.
847+
/// <param name = "focus">The focus state to be scheduled.</param>
847848
/// </summary>
848849
public unsafe void ScheduleFocusEvent(bool focus)
849850
{

Packages/com.unity.inputsystem/Tests/TestFixture/InputTestRuntime.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,9 @@ public unsafe long DeviceCommand(int deviceId, InputDeviceCommand* commandPtr)
236236

237237
public void InvokePlayerFocusChanged(bool newFocusState)
238238
{
239-
focusState = newFocusState ? FocusFlags.ApplicationFocus : FocusFlags.None;
239+
m_FocusState = newFocusState
240+
? m_FocusState | FocusFlags.ApplicationFocus
241+
: m_FocusState & ~FocusFlags.ApplicationFocus;
240242
onPlayerFocusChanged?.Invoke(newFocusState);
241243
}
242244

@@ -349,8 +351,8 @@ public struct PairedUser
349351
public Action<int, string> onDeviceDiscovered { get; set; }
350352
public Action onShutdown { get; set; }
351353
public Action<bool> onPlayerFocusChanged { get; set; }
352-
public FocusFlags focusState { get; set; }
353-
public bool isPlayerFocused => (focusState & FocusFlags.ApplicationFocus) != 0;
354+
public FocusFlags focusState { get { return m_FocusState; } set { m_FocusState = value; } }
355+
public bool isPlayerFocused => (m_FocusState & FocusFlags.ApplicationFocus) != 0;
354356
public float pollingFrequency { get; set; } = 60.0f; // At least 60 Hz by default
355357
public double currentTime { get; set; }
356358
public double currentTimeForFixedUpdate { get; set; }
@@ -424,6 +426,7 @@ public void SetUnityRemoteGyroUpdateInterval(float interval)
424426

425427
internal const int kDefaultEventBufferSize = 1024 * 512;
426428

429+
private FocusFlags m_FocusState = FocusFlags.ApplicationFocus;
427430
private int m_NextDeviceId = 1;
428431
private int m_NextEventId = 1;
429432
internal int m_EventCount;

0 commit comments

Comments
 (0)