FIX: Re-inject player loop hook after user-driven loop reset (UUM-140343)#2419
FIX: Re-inject player loop hook after user-driven loop reset (UUM-140343)#2419LeoUnity wants to merge 4 commits into
Conversation
Adds a [UnityTest] integration test that resets the player loop via PlayerLoop.SetPlayerLoop(PlayerLoop.GetDefaultPlayerLoop()) and asserts that mouse button state is still observable in FixedUpdate. Currently fails on develop, reproducing the user-reported bug: input state freezes in FixedUpdate after a user-initiated player-loop reset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pdate Fixes UUM-140343 with Option B from the design spec: relocate the post-editor-update buffer restore (previously in OnPlayerLoopInitialization, fired from an InputSystemPlayerLoopRunnerInitializationSystem we inject at PlayerLoop.Initialization) into the InputManager.OnUpdate exit paths. PlayerLoop.SetPlayerLoop(GetDefaultPlayerLoop()) wipes that injection; NativeInputSystem.onUpdate (which drives OnUpdate) is fired by Unity's built-in player-loop subsystems and survives a user-driven loop reset. The restore is wired through three OnUpdate exit paths: - FinalizeUpdate (normal exit) - The eventCount==0 early-return (UNITY_INPUTSYSTEM_SUPPORTS_FOCUS_EVENTS) - LegacyEarlyOutFromEventProcessing (older Unity versions) A m_ManualUpdateDepth counter suppresses the auto-restore for manual InputSystem.Update() calls, preserving the contract observed by tests like Devices_CanHandleFocusChanges that read state immediately after a manual editor update. This commit preserves Option B in history. A subsequent commit replaces this with Option 2 (EditorApplication.update watchdog scoped to play mode), which is a smaller and cleaner fix once the no-default-loop-extension constraint is understood from Unity source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…into OnUpdate" This reverts commit 12a5921.
Fixes UUM-140343. When user code calls PlayerLoop.SetPlayerLoop(PlayerLoop.GetDefaultPlayerLoop()) mid-play, the InputSystem's editor-only InputSystemPlayerLoopRunnerInitializationSystem is wiped from PlayerLoop.Initialization. Without it OnPlayerLoopInitialization never fires, so InputStateBuffers are never switched back to player mode after an editor update -- FixedUpdate (and other reads between editor updates) see stale editor-buffer state. Subscribe a watchdog to EditorApplication.update that re-injects when missing. EditorApplication.update is invoked by SceneTracker (Editor/Src/Selection/SceneInspector.cpp) and is not part of the player loop, so a user-initiated SetPlayerLoop call cannot wipe it. The watchdog is gated on EditorApplication.isPlaying so it does nothing in edit mode; while playing the cost is one PlayerLoopSystem-array scan per editor tick, returning immediately when the injection is still in place (the common case). Unity exposes no callback for SetPlayerLoop, no engine API to add to the default player loop, and no per-frame hook inside the player loop that could not be wiped the same way; the watchdog is the cleanest workaround until the engine gains one of those. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2419 +/- ##
===========================================
+ Coverage 78.13% 78.93% +0.80%
===========================================
Files 483 761 +278
Lines 98770 139018 +40248
===========================================
+ Hits 77169 109732 +32563
- Misses 21601 29286 +7685
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| EnsurePlayerLoopInjection(); | ||
|
|
||
| // Watchdog for UUM-140343: user code calling | ||
| // PlayerLoop.SetPlayerLoop(GetDefaultPlayerLoop()) wipes our injection, |
There was a problem hiding this comment.
Is this only affecting the editor hook? We do a number of player loop hooks currently here: https://github.cds.internal.unity3d.com/unity/unity/blob/trunk/Modules/Input/Private/InputModuleRegistration.cpp#L26-L39
There was a problem hiding this comment.
We discussed this offline, but adding here just for context.
GetDefaultPlayerLoop only contains systems registered in Native, the problem is that we have one(?) system today that is registered in managed, so if users SetPlayerLoop using GetDefaultPlayerLoop, those managed registered systems gets removed.
If we were to somehow move all (one? InputSystemPlayerLoopRunnerInitializationSystem) systems registrations to native it would solve the problem.
| // Watchdog tick. Fires from EditorApplication.update (independent of the player loop) and | ||
| // re-injects only when actually needed. In edit mode we don't care; OnPlayerLoopInitialization | ||
| // is gated on gameIsPlaying anyway, so a wiped injection while not playing is harmless. | ||
| private void EnsurePlayerLoopInjectionIfPlaying() |
There was a problem hiding this comment.
This is a lot of work to do on every tick and currently also triggering LINQ it seems - another idea - let me know what you think, do an active watchdog check instead, e.g. on each editor tick where the player hook is actually called we store the current timestamp realTImeSinceStartup, then this tick could just check e.g. WasPlayerloopHookCalledLastXXX or similar. That would imply that we never run anything heavier than a single if-statement for the happy case and the loop injection just triggers after XXX passes without an editor cycle executed?
There was a problem hiding this comment.
Completely agree, I don't think this PR should move forward, it was just to illustrate the issue and a (bad) fix. I don't think we should invest on a watchdog at all, unless there is something I am missing, we should just aim to make GetDefaultPlayerLoop to contain all the hooks we need always.
2 participants
Summary
Fixes UUM-140343 —
PlayerLoop.SetPlayerLoop(PlayerLoop.GetDefaultPlayerLoop())called from user code mid-play wipes the InputSystem's editor-onlyInputSystemPlayerLoopRunnerInitializationSystem, causing input state to freeze inFixedUpdateand other reads that occur between editor updates.Root cause
The InputSystem injects an editor-only subsystem into
PlayerLoop.Initializationto driveOnPlayerLoopInitialization, which switches input state buffers back to player mode after an editor update. PR #1424 introduced this as a self-described hot-fix (see comment atNativeInputRuntime.cs:147-148). When the user resets the loop to Unity's default, that injection is dropped — the default loop only contains Unity's native subsystems — and the buffer-switch never fires again.Fix
Subscribe a watchdog to
EditorApplication.updatethat re-injects the subsystem if it's missing.EditorApplication.updatefires fromSceneTracker::Update()(Editor/Src/Selection/SceneInspector.cpp:742), independently of the player loop, so a userSetPlayerLoopcannot wipe it. The watchdog is gated onEditorApplication.isPlaying:PlayerLoopSystem-array scan per editor tick. Common case (injection present) returns immediately.The original
OnPlayerLoopInitializationmechanism is left untouched — the watchdog only re-establishes the injection when needed.Why not other approaches
Investigated and ruled out (Unity source-confirmed):
s_defaultLoopis built from C++PLAYER_LOOP_INJECT(...)macros (Runtime/Misc/PlayerLoop.cpp:270-292).SetPlayerLoopevent: no callback fires (PlayerLoop.cpp:368-389— buffer swap, no events).OnUpdate: tried (commit12a5921f7), preserved in branch history. Works but breaks the contract that manualInputSystem.Update(Editor)callers observes_LatestUpdateType == Editorimmediately after; required am_ManualUpdateDepthcounter to gate the restore. Reverted in favor of this approach.Test plan
[UnityTest]Integration_InputUpdatesContinue_AfterResettingPlayerLoopToDefaultreproduces the bug pre-fix and passes post-fix.Devices_CanHandleFocusChanges(parametrized variants) still pass — no manual-update contract changes.Unity.InputSystem.IntegrationTestsassembly via Yamato.Branch history
The branch keeps Option B (
OnUpdate-based restore) committed and reverted (12a5921f7+209331a5b) so reviewers can compare both approaches.🤖 Generated with Claude Code