fix: trust the pipeline flag when a domain reload is deferred (#1276) - #1291
Merged
Scriptwonder merged 1 commit intoJul 28, 2026
Merged
Conversation
…Dev#1276) EditorApplication.isCompiling conflates three states: actually compiling, compilation queued, and finished-but-reload-deferred. A project holding EditorApplication.LockReloadAssemblies sits in the third state for as long as the lock is held, with no compilation running, and isCompiling stays true the whole time. Eight call sites gated on that raw flag, so they refused work indefinitely: the stdio bridge would not start, unity_reflect and manage_scriptable_object returned "Unity is compiling", refresh_unity reported the wrong resulting state and never completed its wait, the stdio reload handler deferred its resume, and TestJobManager both mis-attributed its init timeout and reported a bogus "compiling" block reason. Route all eight through EditorStateCache.GetActualIsCompiling(), which falls back to the event-tracked CompilationPipeline flag, and drop the isPlaying gate that previously limited the workaround to play mode. Verified live: with LockReloadAssemblies held after RequestScriptCompilation, EditorApplication.isCompiling is true while the pipeline flag is false, so CompilationPipeline.compilationFinished does fire while the reload is held.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughChangesThe PR centralizes Unity compilation detection through Compilation-state gating
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Unity Editor readiness/“busy: compiling” false-positive that occurs when compilation has finished but the assembly reload is deferred (e.g., via EditorApplication.LockReloadAssemblies). It centralizes “actual compiling” detection in EditorStateCache.GetActualIsCompiling() and routes functional gates through it so tools and the stdio bridge don’t remain blocked indefinitely.
Changes:
- Updates
EditorStateCache.GetActualIsCompiling()to rely on event-trackedCompilationPipelinestart/finish state wheneverEditorApplication.isCompilingis true (covering both play-mode and deferred-reload scenarios). - Replaces direct
EditorApplication.isCompilingreads (and dead reflection probes) across key call sites (bridge startup/resume, tools, test-job gating). - Aligns readiness reporting in
refresh_unityand related wait logic with the new “actual compiling” signal.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| MCPForUnity/Editor/Services/EditorStateCache.cs | Broadens GetActualIsCompiling() semantics beyond play mode and documents the deferred-reload cases. |
| MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs | Uses GetActualIsCompiling() to avoid delaying bridge resume when a domain reload is deferred. |
| MCPForUnity/Editor/Services/TestJobManager.cs | Uses GetActualIsCompiling() to avoid misattributing init timeouts / blocked reasons to “compiling”. |
| MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs | Gates stdio bridge startup on GetActualIsCompiling() (removes reflection probe). |
| MCPForUnity/Editor/Tools/ManageScriptableObject.cs | Prevents rejecting calls purely due to deferred-reload isCompiling false positives. |
| MCPForUnity/Editor/Tools/RefreshUnity.cs | Ensures wait_for_ready and resulting_state don’t stick on “compiling” during deferred reload. |
| MCPForUnity/Editor/Tools/UnityReflect.cs | Avoids blocking reflection due to deferred-reload isCompiling false positives. |
Comments suppressed due to low confidence (1)
MCPForUnity/Editor/Services/EditorStateCache.cs:557
- GetActualIsCompiling() now drives multiple functional gates (bridge startup, tool readiness, test job timeouts) but there’s no automated regression test for the #1276 scenario (LockReloadAssemblies held after compilation finished) to ensure this stays fixed across Unity versions. Consider adding an EditMode test that holds LockReloadAssemblies(), triggers a compile (or simulates the post-compile deferred-reload window), and asserts GetActualIsCompiling() becomes false while EditorApplication.isCompiling remains true, then releases the lock in tear-down to avoid contaminating other tests.
internal static bool GetActualIsCompiling()
{
// If EditorApplication.isCompiling is false, Unity is definitely not compiling
if (!EditorApplication.isCompiling)
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1276.
The bug
EditorApplication.isCompilingconflates three distinct states:A project holding
EditorApplication.LockReloadAssembliessits in state 3 for as long as the lock is held. No compilation is running, butisCompilingstaystruethe entire time — and any code gated on it refuses work indefinitely.Eight call sites read the raw flag:
StdioBridgeHost.IsCompilingStdioBridgeReloadHandlerdelayCallinstead of running immediatelyTestJobManager(init timeout)TestJobManager(blocked reason)"compiling"blockManageScriptableObjectUnityReflectRefreshUnity(wait tick)wait_for_readynever completesRefreshUnity(resulting state)"compiling"when idleThe fix
Route all eight through
EditorStateCache.GetActualIsCompiling(), which short-circuits tofalsewhenisCompilingisfalseand otherwise trusts the event-trackedCompilationPipelineflag.Also drops the
isPlayinggate that previously limited this workaround to play mode — the same false positive occurs outside play mode, which is exactly what #1276 reports. The existing #549 (Recompile-After-Finished-Playing) case is unaffected and still covered.Two dead reflection probes that could never be reached are deleted rather than left in place, per the repo's delete-don't-deprecate policy.
The load-bearing assumption, verified
The fix only works if
CompilationPipeline.compilationFinishedactually fires whileLockReloadAssembliesis held. It does. Reproduced live in the Editor:Then confirmed end-to-end per call site: with a reload held,
unity_reflectpreviously returned "Cannot reflect while Unity is compiling" and now reaches parameter validation ('class_name' parameter is required), andrefresh_unitypreviously reportedresulting_state: "compiling"and now reportsidle.An earlier draft of this fix added a separate
_pipelineCompilationFinishedflag. That turned out to be unnecessary given the above, and would have left the flag set after a failed compile — dropped.Tests
Full EditMode suite on 2021.3.45f2:
1168 total / 1094 passed / 0 failed / 74 skipped.Known remaining sites, deliberately not touched
Three display-only reads still use the raw flag and are cosmetic rather than functional:
McpConnectionSection.cs:382,McpConnectionSection.cs:640,ManagePackages.cs:603.The EditMode harness itself is also affected —
TestUtilities.cs:65spins onwhile (EditorApplication.isCompiling || EditorApplication.isUpdating), so a held reload makes everySetUpburn its 180s timeout. That is a test-infrastructure change and belongs in its own PR rather than riding along here.Summary by CodeRabbit