fix: Prevent infinite compilation loop in Unity 6 when using wait_for_ready #559
Conversation
…_ready Skip WaitForUnityReadyAsync when compileRequested is true. The EditorApplication.update polling doesn't survive domain reloads properly in Unity 6, causing infinite compilation loops. When compilation is requested, return immediately and let the client poll editor_state resource instead. Fixes CoplayDev#557 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the Unity editor refresh command handling so that in Unity 6+ an explicit compile request with wait_for_ready=true no longer awaits WaitForUnityReadyAsync, preventing infinite compilation loops and aligning the hint messaging with the new behavior. Sequence diagram for refresh_unity with compile=request and wait_for_ready in Unity 6+sequenceDiagram
actor Client
participant MCPServer
participant RefreshUnity_HandleCommand as RefreshUnity_HandleCommand
participant WaitForUnityReadyAsync as WaitForUnityReadyAsync
participant EditorState as editor_state_resource
Client->>MCPServer: send refresh_unity(compile=request, wait_for_ready=true)
MCPServer->>RefreshUnity_HandleCommand: HandleCommand(params)
activate RefreshUnity_HandleCommand
RefreshUnity_HandleCommand-->>RefreshUnity_HandleCommand: perform asset refresh
RefreshUnity_HandleCommand-->>RefreshUnity_HandleCommand: set compileRequested = true
Note over RefreshUnity_HandleCommand: Unity 6+: shouldWaitForReady = waitForReady && !compileRequested
RefreshUnity_HandleCommand-->>RefreshUnity_HandleCommand: shouldWaitForReady = false
RefreshUnity_HandleCommand-->>MCPServer: return { resulting_state = compiling, hint = "If Unity enters compilation/domain reload, poll editor_state until ready_for_tools is true." }
deactivate RefreshUnity_HandleCommand
MCPServer-->>Client: response(resulting_state=compiling)
loop poll until ready
Client->>MCPServer: request editor_state
MCPServer->>EditorState: read editor_ready status
EditorState-->>MCPServer: ready_for_tools flag
MCPServer-->>Client: editor_state(ready_for_tools)
end
Client-->>Client: proceed when ready_for_tools is true
Flow diagram for shouldWaitForReady logic in RefreshUnity.HandleCommandflowchart TD
A[Start HandleCommand after refresh/compile setup] --> B{Unity version >= 6000.0?}
B -- Yes --> C[compute shouldWaitForReady = waitForReady && !compileRequested]
B -- No --> D[compute shouldWaitForReady = waitForReady]
C --> E{shouldWaitForReady?}
D --> E
E -- Yes --> F[await WaitForUnityReadyAsync]
F --> G[set resultingState = ready]
G --> H[set hint = Unity refresh completed; editor should be ready.]
H --> I[return response]
E -- No --> J[set resultingState = compiling or current state]
J --> K[set hint = If Unity enters compilation/domain reload, poll editor_state until ready_for_tools is true.]
K --> I[return response]
I --> L[End HandleCommand]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Unity-6+ conditional gate to the RefreshUnity wait logic so the code only waits for Unity readiness when Changes
Sequence Diagram(s)(omitted — change is a localized control-flow tweak and does not introduce a multi-component sequential feature) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-29T15:23:11.613ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83ddd69b21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (waitForReady && !compileRequested) | ||
| { |
There was a problem hiding this comment.
When compile="request" and wait_for_ready=true, this new guard skips WaitForUnityReadyAsync, but the response still sets the hint based on waitForReady alone. That means the tool will return immediately yet still claim “Unity refresh completed; editor should be ready,” which is false while compilation is still running. Clients that treat wait_for_ready (or the hint) as a readiness guarantee can proceed too early and hit tool failures. Consider basing the hint on waitForReady && !compileRequested (or otherwise signaling that polling is still required) to keep the response consistent with the new behavior.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Tools/RefreshUnity.cs (1)
129-131: Hint message is inconsistent when compile was requested withwait_for_ready=true.When
waitForReady=truebutcompileRequested=true, we now skip waiting (per the fix), yet the hint still says "Unity refresh completed; editor should be ready." This is misleading since we didn't actually wait.Proposed fix
- return new SuccessResponse("Refresh requested.", new + bool actuallyWaited = waitForReady && !compileRequested; + return new SuccessResponse("Refresh requested.", new { refresh_triggered = refreshTriggered, compile_requested = compileRequested, resulting_state = resultingState, - hint = waitForReady + hint = actuallyWaited ? "Unity refresh completed; editor should be ready." : "If Unity enters compilation/domain reload, poll editor_state until ready_for_tools is true." });
🧹 Nitpick comments (1)
MCPForUnity/Editor/Tools/RefreshUnity.cs (1)
101-103:allowNudgeparameter is now alwaystruein this context.Since we only enter this block when
!compileRequestedis true, the expressionallowNudge: !compileRequestedwill always evaluate totrue. This is harmless but slightly redundant. Consider simplifying toallowNudge: truefor clarity, or leave as-is if you anticipate the condition changing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MCPForUnity/Editor/Tools/RefreshUnity.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T15:23:11.613Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:11.613Z
Learning: In MCPForUnity, prefer relying on the established testing process to catch UI initialization issues instead of adding defensive null checks for UI elements in editor windows. This means during reviews, verify that tests cover UI initialization paths and that code avoids repetitive null-check boilerplate in Editor Windows. If a UI element can be null, ensure there is a well-tested fallback or that its initialization is guaranteed by design, rather than sprinkling null checks throughout editor code.
Applied to files:
MCPForUnity/Editor/Tools/RefreshUnity.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (1)
MCPForUnity/Editor/Tools/RefreshUnity.cs (1)
93-97: Well-documented fix for Unity 6 domain reload issue.The approach of skipping
WaitForUnityReadyAsyncwhen compilation is requested is sound. The comments clearly explain the root cause (EditorApplication.update callbacks not surviving domain reloads) and the rationale for the fix. Returning immediately withresulting_state="compiling"and letting the client poll is a pragmatic solution that maintains backward compatibility.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Address code review feedback: when compile was requested and we skip WaitForUnityReadyAsync, the hint should correctly indicate that the client needs to poll editor_state, not claim the editor is ready. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since the behavior change is specifically to address a Unity 6 issue, consider gating the
compileRequested+wait_for_readybypass behind a Unity version check (e.g., preprocessor symbol or runtime version check) so earlier Unity versions retain the previous behavior unless they exhibit the same problem. - The
actuallyWaitedvariable name could be clearer as something likeshouldWaitForReadyorwaitThisCall, since in this context it represents the decision to perform the wait rather than a result determined after the fact.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the behavior change is specifically to address a Unity 6 issue, consider gating the `compileRequested` + `wait_for_ready` bypass behind a Unity version check (e.g., preprocessor symbol or runtime version check) so earlier Unity versions retain the previous behavior unless they exhibit the same problem.
- The `actuallyWaited` variable name could be clearer as something like `shouldWaitForReady` or `waitThisCall`, since in this context it represents the decision to perform the wait rather than a result determined after the fact.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Address code review feedback: - Use UNITY_6000_0_OR_NEWER preprocessor directive to only apply the compilation wait bypass on Unity 6+, preserving original behavior for earlier versions - Rename actuallyWaited to shouldWaitForReady for clarity, as it represents the decision to wait rather than a post-hoc result Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The allowNudge parameter was attempting to fix Unity 6 compilation loops by skipping QueuePlayerLoopUpdate, but this was not the root cause. The actual issue (fixed by CoplayDev#559's Unity 6+ preprocessor guard) is that EditorApplication.update callbacks don't survive domain reloads properly in Unity 6+. Since CoplayDev#559 skips WaitForUnityReadyAsync entirely on Unity 6+ when compile is requested, the allowNudge guard is redundant and can be removed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The allowNudge parameter was attempting to fix Unity 6 compilation loops by skipping QueuePlayerLoopUpdate, but this was not the root cause. The actual issue (fixed by #559's Unity 6+ preprocessor guard) is that EditorApplication.update callbacks don't survive domain reloads properly in Unity 6+. Since #559 skips WaitForUnityReadyAsync entirely on Unity 6+ when compile is requested, the allowNudge guard is redundant and can be removed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@BlindsidedGames Thanks for the contribution and digging into the issue. I tested it and works fine for me. @Scriptwonder let us know if this fixes it for you too. (I reverted the earlier PR changes that weren't actually doing much). |
Upstream changes (v9.0.7 → v9.0.8): - fix: UIDocument serialization to prevent infinite loops (CoplayDev#586) - fix: Filter isCompiling false positives in Play mode (CoplayDev#582) - fix: search inactive objects when setActive=true (CoplayDev#581) - fix: Add Prefab Stage support for GameObject lookup (CoplayDev#573) - fix: Prevent infinite compilation loop in Unity 6 (CoplayDev#559) - fix: parse and validate read_console types (CoplayDev#565) - fix: Local HTTP server UI check (CoplayDev#556) - fix: Claude Code HTTP Remote UV path override detection - fix: ULF detection in Claude licensing (CoplayDev#569) - chore: Replace asmdef GUID references (CoplayDev#564) - docs: Streamline README for faster onboarding (CoplayDev#583) - Many new client configurators (VSCode, Cursor, Windsurf, etc.) Fork enhancements preserved: - "find" instruction handler in UnityTypeConverters.cs - MarkSceneOrPrefabDirty() helper for proper Prefab Stage support - IsInPrefabStage() and GetPrefabStageRoot() helpers - #main URL reference (no version tags) - TestProjects excluded Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Summary
Fixes #555 - Infinite compilation/refresh loop when using
refresh_unitywithcompile="request"andwait_for_ready=truein Unity 6.Root Cause
In Unity 6, the
WaitForUnityReadyAsyncmethod's use ofEditorApplication.update += Tickto poll for compilation completion causes infinite domain reload loops. The async Task + EditorApplication.update callback combination doesn't survive domain reloads properly, causing the editor to repeatedly recompile until timeout.Fix
Skip
WaitForUnityReadyAsyncwhencompileRequestedis true. Return immediately and let the client polleditor_stateresource instead.This is a minimal, targeted fix that:
compile="request"+wait_for_ready=trueresulting_state="compiling"with a hint to pollTesting
Environment:
Before fix:
refresh_unitywithcompile="request"andwait_for_ready=truecauses ~3 compilation loops before timing out.After fix: Single compilation, immediate return with
resulting_state="compiling".Notes
QueuePlayerLoopUpdatechanges in Guard refresh wait nudge during compile #558 were not the cause of the loopEditorApplication.updatecallback behavior across domain reloads in Unity 6🤖 Generated with Claude Code
Summary by Sourcery
Prevent infinite compilation loops in Unity 6 when using refresh commands that request compilation and wait for readiness.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.