Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions MCPForUnity/Editor/Services/EditorStateCache.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Reflection;
using MCPForUnity.Editor.Helpers;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
Expand Down Expand Up @@ -265,7 +266,7 @@ private static void OnUpdate()
if (now - _lastUpdateTimeSinceStartup < MinUpdateIntervalSeconds)
{
// Still update on compilation edge transitions to keep timestamps meaningful.
bool isCompiling = EditorApplication.isCompiling;
bool isCompiling = GetActualIsCompiling();
if (isCompiling == _lastIsCompiling)
{
return;
Expand All @@ -289,7 +290,7 @@ private static JObject BuildSnapshot(string reason)
_sequence++;
_observedUnixMs = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds();

bool isCompiling = EditorApplication.isCompiling;
bool isCompiling = GetActualIsCompiling();
if (isCompiling && !_lastIsCompiling)
{
_lastCompileStartedUnixMs = _observedUnixMs;
Expand Down Expand Up @@ -427,6 +428,42 @@ public static JObject GetSnapshot()
return (JObject)_cached.DeepClone();
}
}

/// <summary>
/// Returns the actual compilation state, working around a known Unity quirk where
/// EditorApplication.isCompiling can return false positives in Play mode.
/// See: https://github.com/CoplayDev/unity-mcp/issues/549
/// </summary>
private static bool GetActualIsCompiling()
{
// If EditorApplication.isCompiling is false, Unity is definitely not compiling
if (!EditorApplication.isCompiling)
{
return false;
}

// In Play mode, EditorApplication.isCompiling can have false positives.
// Double-check with CompilationPipeline.isCompiling via reflection.
if (EditorApplication.isPlaying)
{
try
{
Type pipeline = Type.GetType("UnityEditor.Compilation.CompilationPipeline, UnityEditor");
var prop = pipeline?.GetProperty("isCompiling", BindingFlags.Public | BindingFlags.Static);
if (prop != null)
{
return (bool)prop.GetValue(null);
}
}
catch
{
// If reflection fails, fall back to EditorApplication.isCompiling
Comment on lines +458 to +460
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
                    }
                }
            }
  1. Add a static flag field to the containing class (likely EditorStateCache):

    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:

    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.

}
}

// Outside Play mode or if reflection failed, trust EditorApplication.isCompiling
return true;
Comment on lines +464 to +465
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
}

Expand Down