fix: Filter EditorApplication.isCompiling false positives in Play mode #582
Conversation
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
EditorApplication.isCompiling can return true in Play mode even when Unity is not actually compiling, causing MCP tools to incorrectly return "busy/compiling" errors. This adds a GetActualIsCompiling() helper that double-checks with CompilationPipeline.isCompiling via reflection when in Play mode to filter out these false positives. Fixes CoplayDev#549
Contributor
Reviewer's GuideAdds a Play-mode-safe helper around Unity’s compilation state and replaces direct EditorApplication.isCompiling usages to avoid false positives that caused MCP tools to report spurious "busy/compiling" errors. Sequence diagram for MCP tool snapshot using GetActualIsCompilingsequenceDiagram
actor Developer
participant MCPToolClient
participant MCPToolService
participant EditorStateCache
participant EditorApplication
participant Type
participant PropertyInfo
participant CompilationPipeline
Developer->>MCPToolClient: Invoke MCP tool (find_gameobjects, manage_scene)
MCPToolClient->>MCPToolService: HTTP request
MCPToolService->>EditorStateCache: GetSnapshot()
activate EditorStateCache
EditorStateCache->>EditorStateCache: GetActualIsCompiling()
EditorStateCache->>EditorApplication: read isCompiling
alt isCompiling is false
EditorStateCache-->>EditorStateCache: return false
else isCompiling is true and isPlaying is true
EditorStateCache->>EditorApplication: read isPlaying
alt isPlaying is true
EditorStateCache->>Type: GetType(UnityEditor.Compilation.CompilationPipeline, UnityEditor)
Type-->>EditorStateCache: CompilationPipeline type
EditorStateCache->>CompilationPipeline: get isCompiling via PropertyInfo
CompilationPipeline-->>EditorStateCache: actual isCompiling
else not in Play mode
EditorStateCache-->>EditorStateCache: trust EditorApplication.isCompiling
end
end
EditorStateCache-->>MCPToolService: snapshot with is_compiling state
deactivate EditorStateCache
MCPToolService-->>MCPToolClient: response (no spurious busy/compiling)
MCPToolClient-->>Developer: Display tool result
Class diagram for updated EditorStateCache compilation handlingclassDiagram
class EditorStateCache {
- double _lastUpdateTimeSinceStartup
- double MinUpdateIntervalSeconds
- bool _lastIsCompiling
- long _sequence
- long _observedUnixMs
- long _lastCompileStartedUnixMs
- JObject _cached
+ void OnUpdate()
+ JObject BuildSnapshot(string reason)
+ JObject GetSnapshot()
- bool GetActualIsCompiling()
}
class EditorApplication {
<<static>>
+ bool isCompiling
+ bool isPlaying
}
class CompilationPipeline {
<<static>>
+ bool isCompiling
}
class Type {
+ static Type GetType(string typeName)
}
class PropertyInfo {
+ object GetValue(object obj)
}
EditorStateCache ..> EditorApplication : uses
EditorStateCache ..> CompilationPipeline : reflects_to
EditorStateCache ..> Type : uses_reflection
EditorStateCache ..> PropertyInfo : uses_reflection
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughFixes a false positive bug where Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
✨ Finishing touches
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider caching the reflected
CompilationPipelinetype andisCompilingPropertyInfoin static fields so you avoid paying the reflection cost on everyGetActualIsCompilingcall duringOnUpdate. - The catch block in
GetActualIsCompilingsilently swallows all exceptions; if this can fail in practice, it may be worth at least logging once (or using a guard flag) so unexpected reflection issues are diagnosable while still falling back gracefully.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider caching the reflected `CompilationPipeline` type and `isCompiling` `PropertyInfo` in static fields so you avoid paying the reflection cost on every `GetActualIsCompiling` call during `OnUpdate`.
- The catch block in `GetActualIsCompiling` silently swallows all exceptions; if this can fail in practice, it may be worth at least logging once (or using a guard flag) so unexpected reflection issues are diagnosable while still falling back gracefully.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Services/EditorStateCache.cs:458-460` </location>
<code_context>
+ return (bool)prop.GetValue(null);
+ }
+ }
+ catch
+ {
+ // If reflection fails, fall back to EditorApplication.isCompiling
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Catching all exceptions here may hide useful diagnostics when the reflection path breaks.
This bare `catch` will silently hide reflection issues (e.g., type renames, removed APIs), making it hard to detect when this path stops working due to Unity changes. Please at least emit a one-time warning when reflection fails (e.g., guarded by a static flag) so such breakages are visible without flooding the console.
Suggested implementation:
```csharp
}
catch (Exception ex)
{
// If reflection fails, fall back to EditorApplication.isCompiling, but emit a one-time warning
if (!_compilationReflectionWarningEmitted)
{
_compilationReflectionWarningEmitted = true;
UnityEngine.Debug.LogWarning(
"[EditorStateCache] Failed to query UnityEditor.Compilation.CompilationPipeline.isCompiling via reflection. " +
"Falling back to EditorApplication.isCompiling. This may indicate a Unity API change.\n" +
ex);
}
}
}
```
1. Add a static flag field to the containing class (likely `EditorStateCache`):
```csharp
private static bool _compilationReflectionWarningEmitted;
```
Place it with the other fields in `EditorStateCache`.
2. Ensure the file has the necessary `using` directives at the top if they are not already present:
```csharp
using System;
using System.Reflection;
using UnityEngine;
```
3. If your codebase uses a different logging abstraction (e.g., a custom logger instead of `UnityEngine.Debug`), replace `UnityEngine.Debug.LogWarning(...)` with the project-standard logging method.
</issue_to_address>
### Comment 2
<location> `MCPForUnity/Editor/Services/EditorStateCache.cs:464-465` </location>
<code_context>
+ }
+ }
+
+ // Outside Play mode or if reflection failed, trust EditorApplication.isCompiling
+ return true;
+ }
}
</code_context>
<issue_to_address>
**nitpick:** Final return value relies on earlier guard, making the comment slightly misleading.
The comment says we "trust EditorApplication.isCompiling", but this branch always returns `true`. That only matches the intent because the earlier guard already returned `false` when `EditorApplication.isCompiling` was `false`. Consider either returning `EditorApplication.isCompiling` here, or rephrasing the comment to state that at this point `EditorApplication.isCompiling` is guaranteed to be `true`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+458
to
+460
| catch | ||
| { | ||
| // If reflection fails, fall back to EditorApplication.isCompiling |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Catching all exceptions here may hide useful diagnostics when the reflection path breaks.
This bare catch will silently hide reflection issues (e.g., type renames, removed APIs), making it hard to detect when this path stops working due to Unity changes. Please at least emit a one-time warning when reflection fails (e.g., guarded by a static flag) so such breakages are visible without flooding the console.
Suggested implementation:
}
catch (Exception ex)
{
// If reflection fails, fall back to EditorApplication.isCompiling, but emit a one-time warning
if (!_compilationReflectionWarningEmitted)
{
_compilationReflectionWarningEmitted = true;
UnityEngine.Debug.LogWarning(
"[EditorStateCache] Failed to query UnityEditor.Compilation.CompilationPipeline.isCompiling via reflection. " +
"Falling back to EditorApplication.isCompiling. This may indicate a Unity API change.\n" +
ex);
}
}
}-
Add a static flag field to the containing class (likely
EditorStateCache):private static bool _compilationReflectionWarningEmitted;
Place it with the other fields in
EditorStateCache. -
Ensure the file has the necessary
usingdirectives at the top if they are not already present:using System; using System.Reflection; using UnityEngine;
-
If your codebase uses a different logging abstraction (e.g., a custom logger instead of
UnityEngine.Debug), replaceUnityEngine.Debug.LogWarning(...)with the project-standard logging method.
Comment on lines
+464
to
+465
| // Outside Play mode or if reflection failed, trust EditorApplication.isCompiling | ||
| return true; |
Contributor
There was a problem hiding this comment.
nitpick: Final return value relies on earlier guard, making the comment slightly misleading.
The comment says we "trust EditorApplication.isCompiling", but this branch always returns true. That only matches the intent because the earlier guard already returned false when EditorApplication.isCompiling was false. Consider either returning EditorApplication.isCompiling here, or rephrasing the comment to state that at this point EditorApplication.isCompiling is guaranteed to be true.
vbucc
added a commit
to Studio-Pronto/unity-mcp
that referenced
this pull request
Jan 19, 2026
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes #549
EditorApplication.isCompilingcan returntruein Play mode even when Unity is not actually compiling. This causes MCP tools to incorrectly return"error": "busy", "reason": "compiling"errors intermittently.Changes
Added
GetActualIsCompiling()helper method inEditorStateCache.csthat:falseimmediately ifEditorApplication.isCompilingisfalseEditorApplication.isCompilingistrue, double-checks withCompilationPipeline.isCompilingvia reflectionCompilationPipeline.isCompilingreturnsfalse, treats it as a false positiveEditorApplication.isCompilingoutside Play mode or if reflection failsUpdated both usages in
EditorStateCache.csto use the new helperTest plan
find_gameobjects,manage_scene, etc.)editor_stateresource showsis_compiling: falsewhen not actually compilingSummary by Sourcery
Bug Fixes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.