Sync fork beta with upstream CoplayDev/beta (9.7.2-beta.9)#17
Conversation
- ToolDiscoveryService: add AppDomain fallback scan for [McpForUnityTool] types - StdioBridgeHost: include project_scoped_tools flag in heartbeat JSON - McpToolsSection: update tooltip and default to reflect stdio support - models.py: add project_scoped_tools field to UnityInstanceInfo - port_discovery.py: read project_scoped_tools from status JSON - main.py: enable project-scoped tools when Unity instance requests it
…nd surface inner errors
The RoslynInstaller downloads only 4 NuGet packages but Microsoft.CodeAnalysis 4.12.0
on netstandard2.0 also references System.Runtime.CompilerServices.Unsafe v6.0.0.0,
which is NOT what Unity ships (Unity bundles v4.x). The reference is unresolved at
runtime, so Roslyn's StringTable static cctor throws FileNotFoundException, which
in turn poisons CSharpSyntaxTree's cctor: every parse / compile attempt then
throws TypeInitializationException.
The error is invisible because RoslynCompiler.Compile's catch block only logs
e.Message, and for TargetInvocationException that string is the generic
"Exception has been thrown by the target of an invocation." — the real cause
in InnerException is silently dropped.
Repro:
1. Trigger Tools > MCP for Unity > Install Roslyn on a fresh project.
2. Ask the MCP execute_code tool to compile any Roslyn-only snippet.
3. Observe: "Compilation failed: Roslyn compilation error: Exception has been
thrown by the target of an invocation." — with no further detail.
4. Drilling via reflection reveals:
TypeInitializationException for CSharpSyntaxTree
└─ TypeInitializationException for Roslyn.Utilities.StringTable
└─ FileNotFoundException: Could not load file or assembly
'System.Runtime.CompilerServices.Unsafe, Version=6.0.0.0'
Fix:
* RoslynInstaller.NuGetEntries: add the missing dependency so a fresh install
drops all 5 DLLs into Assets/Plugins/Roslyn/.
* RoslynCompiler.Compile catch: walk the InnerException chain so a future
bootstrap regression surfaces the actual cause (type + message) instead of
the generic invocation wrapper.
Verified locally: after dropping System.Runtime.CompilerServices.Unsafe.dll v6
into the plugins folder and forcing a domain reload, the execute_code tool
compiles C# 7+ snippets via the Roslyn backend successfully.
Address PR review: file-existence alone treats a stale v4 of System.Runtime.CompilerServices.Unsafe (or any other downgraded plugin) as "installed" and silently masks the bootstrap failure this PR is meant to fix. Read each on-disk DLL's manifest via AssemblyName.GetAssemblyName(...).Version and compare to the NuGet entry's declared version; if the file is older than declared or its manifest can't be read, return false so Install() rewrites it. Applied uniformly to all entries (defense-in-depth, per the optional suggestion in the review). Notes: - System.Version comparison handles "6.0.0" (Revision=-1) vs "6.0.0.0" (Revision=0) correctly: the higher-revision actual passes; a stale v4 fails. - AssemblyName.GetAssemblyName only reads the manifest, no assembly load into the AppDomain, so this is cheap and side-effect-free. - Validated locally: with the v6 DLL in Plugins/Roslyn/, IsInstalled returns true; manually swapping in a v4 placeholder causes it to return false and Install() proceeds.
MCPForUnity.Runtime uses Texture2D.EncodeToPNG, Physics2D, and ScreenCapture APIs. Declaring their built-in modules prevents Unity projects that do not already enable those modules from failing script compilation. Co-authored-by: Codex <noreply@openai.com>
Readme-cleanup
ScreenshotUtility.CaptureComposited called ScreenCapture.CaptureScreenshotAsTexture inline, before the next frame had been rendered and presented. UI Toolkit overlays are composited at end-of-frame, so the captured texture contained an unwritten backbuffer and the saved PNG was blank. Route the play-mode editor path through a transient MonoBehaviour that yields WaitForEndOfFrame before calling ScreenCapture, then advance the player loop with EditorApplication.Step() until the coroutine completes. The single-call API is preserved; total cost is bounded (5 steps, ~80ms at 60fps). Complements CoplayDev#1040, which switched to the correct API but kept the synchronous invocation that caused the empty capture.
…ale state Two robustness tweaks to CompositedFrameCapturer / CaptureCompositedAfterFrame flagged by CodeRabbit on the parent commit: 1. On exception inside the WaitForEndOfFrame coroutine, mark s_pendingCompositedDone = true so CaptureCompositedAfterFrame's spin loop exits immediately. The null texture already signals failure to the caller, which falls through to the camera-based fallback. Previously the loop would spin for the full timeout despite the coroutine being effectively done. 2. Reset s_pendingCompositedTex / s_pendingCompositedDone unconditionally on entry. A coroutine from a previous call that timed out can complete asynchronously and leave a stale texture / done flag behind; clearing on entry prevents the next call from picking up that stale capture.
Removing the if (s_pendingCompositedStarted) guard in the previous commit
left the field with only assignments and no reads, which Unity surfaces as
CS0414 ("assigned but its value is never used"). Drop the field and its
three write sites.
No behavior change.
Reflect the new onboarding wizard, multi-client one-click flow, Claude Desktop transport auto-coercion, and startup config rewrite in the README. Update the Claude Desktop section of the configurator guide to drop the stale "throw on HTTP" language.
fix: enable project-scoped custom tools in stdio mode
…eta.3-26412591059 chore: update Unity package to beta version 9.7.2-beta.3
…enshot-wait-for-end-of-frame
…enshot-wait-for-end-of-frame # Conflicts: # MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs
…shot-wait-for-end-of-frame fix(screenshot): wait for end-of-frame before composited capture
…eta.4-26430668594 chore: update Unity package to beta version 9.7.2-beta.4
…esource fix(prefabs): wire missing C# handler for editor/prefab-stage resource
…eta.5-26438040267 chore: update Unity package to beta version 9.7.2-beta.5
…sion-dependency fix: declare required Unity module dependencies
…eta.6-26439496903 chore: update Unity package to beta version 9.7.2-beta.6
…arations (CoplayDev#1122) Since CoplayDev#1122 declared the Physics 2D, Screen Capture, Image Conversion, etc. built-in modules as required deps in package.json, Unity Package Manager now keeps them enabled while MCP for Unity is installed. Two defensive layers that protected against the now-impossible "module disabled" case become dead code, plus one sub-floor preprocessor gate that was always-true on our declared `unity: 2021.3` floor. Tier 1A — UnityPhysicsCompat.cs - Revert `Type.GetType("UnityEngine.Physics2D, UnityEngine.Physics2DModule")` to `typeof(Physics2D)`. The property-level reflection (for `autoSyncTransforms` deprecation in Unity 6.x) stays — that's a different concern. Tier 1B — TrailControl.cs - Strip `#if UNITY_2021_1_OR_NEWER` gate. Package floor is 2021.3 per package.json, so the `#else` branch ("AddPosition requires Unity 2021.1+") is unreachable. Tier 2 — ScreenshotUtility.cs + callers - Remove `IsScreenCaptureModuleAvailable` property, `ScreenCaptureModuleNotAvailableError` constant, `InvokeCaptureScreenshotAsTexture` helper, and the `s_captureScreenshotMethod` / `s_captureScreenshotAsTextureMethod` / `s_screenCaptureModuleAvailable` reflection caches. - Replace reflective `MethodInfo.Invoke` calls with direct `ScreenCapture.X` calls in `CaptureToProjectFolder`, `CaptureComposited`, and the `ScreenshotCapturer` MonoBehaviour. - Camera-based fallback path (`FindAvailableCamera` + `CaptureFromCameraToProjectFolder`) is preserved — it still handles transient null returns from `ScreenCapture` inside `CaptureComposited`. - Drop pre-flight `if (!IsScreenCaptureModuleAvailable)` gates in `ManageUI.cs` (UI render) and `ManageScene.cs` (screenshot path). - `ProjectInfo` resource: `screenCapture` field is now an invariant `true` (preserves API shape for `mcpforunity://project/info` consumers). Net: 144 lines removed, 15 added. No new tests — the changes are pure removal of code paths that cannot fire with the current deps declared. Related: CoplayDev#1160, CoplayDev#1122.
…post-module-deps refactor: drop defensive scaffolding made obsolete by module dep declarations
…oplayDev#1144) `CodeDomCompile` in `MCPForUnity/Editor/Tools/ExecuteCode.cs` pushed every filtered assembly path into `CompilerParameters.ReferencedAssemblies`. Mono's `CSharpCodeCompiler.BuildArgs` (verified at mcs/class/System/Microsoft.CSharp/CSharpCodeCompiler.cs:388-392) turns each reference into a literal `/r:"<absolute_path>"` flag and concatenates them inline on the `mono.exe csc ...` command line. Projects with ~100+ asmdefs (a perfectly normal large Unity project) overflow Windows' 32 KB CreateProcess argument limit and `Process.Start` throws Win32 `ERROR_FILENAME_EXCED_RANGE`, which Mono surfaces as: SystemException: Error running …mono.exe: The filename or extension is too long. …exactly the failure reported in CoplayDev#1144 at ExecuteCode.cs:115 on Windows 10/11 + Unity 2022.3.62f2 + MCP for Unity 9.6.9-beta.8. Fix: write all `/r:"…"` lines to a GUID-named temp response file and pass `@"<path>"` via `CompilerParameters.CompilerOptions` (which `BuildArgs` appends verbatim, confirmed at line 396 of the same Mono source). One short argument regardless of reference count — the 32 KB ceiling is no longer reachable. Both legacy mcs and Roslyn csc accept `@responsefile`, so the change is cross-platform: macOS/Linux Mono behaves identically, and the path doesn't have a 32 KB limit there to begin with. Response file is cleaned up in a `finally` block (best-effort; OS reaps temp on its own otherwise). Tests: added two EditMode regression tests in `ExecuteCodeTests.cs` that exercise the codedom backend end-to-end: - `Execute_CodedomBackend_CompilesAndRuns` — basic compile + execute. - `Execute_CodedomBackend_ResolvesUnityTypes` — verifies Unity references resolve through the response file. Could not reproduce the exact 32 KB failure on macOS (Mono on POSIX doesn't hit the limit), but the response-file path is the only path in the new code, so the fix is mechanically equivalent for any reference-set size on any platform. Sources: - Mono CSharpCodeCompiler.cs — BuildArgs converts ReferencedAssemblies to `/r:"…"` inline and appends CompilerOptions verbatim. - C# compiler — ResponseFiles option (`@responsefile` syntax).
…eta.7-26441731642 chore: update Unity package to beta version 9.7.2-beta.7
…dedom-arg-length fix(execute_code): route CodeDom references through a response file (CoplayDev#1144)
…eta.8-26442775926 chore: update Unity package to beta version 9.7.2-beta.8
feat: add Kimi Code CLI client configurator
…eta.9-26492235542 chore: update Unity package to beta version 9.7.2-beta.9
Syncs the fork with the latest upstream beta (up to 2d2bdc5, package 9.7.2-beta.9). 156 upstream commits; 5 conflicts resolved: - MCPForUnity.Editor.asmdef: union (kept our versionDefines + upstream UNITY_VFX_GRAPH) - StdioBridgeHost.cs: kept our structured eviction notice (#10; evictionBytes is required by the following foreach) - ScreenshotUtility.cs: kept our screen-capture-module availability guard (used at lines ~331/346) - README.md: took upstream's docs-site restructure (no The1Studio-specific content lost) - website/docs/architecture/STUDIO_TOOLS.md: kept ours (upstream has no version) Fork features preserved: #16 UXML basePath fix, #10 eviction notice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Review summary
This is a large fork-sync (156 commits, 222 files). I spot-checked the conflict-resolved files where fork features and upstream changes intersect, since those are the highest-risk areas. Two of the four 'kept ours' resolutions don't match what's actually in the merged tree — please verify before merging.
🔴 Blocking concerns
-
ScreenshotUtility.csmay have lost the fork's screen-capture-module guard. The PR description says the resolution was 'Kept ours — screen-capture-module availability guard (used at ~L331/L346)', but the merged file is only 50 lines with no guard aroundScreenCapture.CaptureScreenshotAsTexture(). Lines 331/346 do not exist. Either the fork feature was dropped or the description refers to the wrong file. Inline comment added. -
StdioBridgeHost.csmay have lost the fork's PR #10 structured eviction notice. The description says it was kept along with anevictionBytesvariable required by a followingforeach, but neitherevictionnorevictionBytesappears in the merged file. General comment added requesting verification.
Given the PR description explicitly lists 'Fork features preserved: #10 — structured stdio eviction notice', if PR #10's code really isn't in StdioBridgeHost.cs, that's a regression. Please confirm whether the feature was relocated, refactored away upstream, or accidentally dropped.
🟡 Non-blocking but worth a decision
.github/workflows/sync-releases.yml— new upstream workflow that force-pushesmaintorelease/<major.minor>on every push. Will fire on this fork too. Recommend either gating withif: github.repository == 'CoplayDev/unity-mcp'or removing the workflow if the fork doesn't use this release strategy. Inline comment added.
✅ What looked good on spot-check
MCPForUnity.Editor.asmdefunion resolution — fork'sversionDefinesretained,UNITY_VFX_GRAPHfrom upstream added correctly- New configurators (
KimiCodeConfigurator,AntigravityIdeConfigurator) follow the existingIMcpClientConfiguratorpattern cleanly IMcpClientConfiguratorinterface additions (UsesCliConfiguration, etc.) are non-breaking for existing implementors via the new propertiesunity-tests.ymlchange is benign (free-disk-space + matrix expansion)
Suggested next steps before merging
- Diff the pre-merge fork copies of
ScreenshotUtility.csandStdioBridgeHost.csagainst what's currently in this branch, and confirm the fork features are actually present (or document that upstream replaced/superseded them). - Complete the two items in the PR's own 'Verification needed' checklist (Unity compile + Window opens), since 222 files / +31k lines is far beyond what a code review alone can validate.
- Decide on
sync-releases.yml.
There was a problem hiding this comment.
🤖 Claude Code Review
Decision: ❌ REQUEST CHANGES
Summary
Review summary
This is a large fork-sync (156 commits, 222 files). I spot-checked the conflict-resolved files where fork features and upstream changes intersect, since those are the highest-risk areas. Two of the four 'kept ours' resolutions don't match what's actually in the merged tree — please verify before merging.
🔴 Blocking concerns
-
ScreenshotUtility.csmay have lost the fork's screen-capture-module guard. The PR description says the resolution was 'Kept ours — screen-capture-module availability guard (used at ~L331/L346)', but the merged file is only 50 lines with no guard aroundScreenCapture.CaptureScreenshotAsTexture(). Lines 331/346 do not exist. Either the fork feature was dropped or the description refers to the wrong file. Inline comment added. -
StdioBridgeHost.csmay have lost the fork's PR #10 structured eviction notice. The description says it was kept along with anevictionBytesvariable required by a followingforeach, but neitherevictionnorevictionBytesappears in the merged file. General comment added requesting verification.
Given the PR description explicitly lists 'Fork features preserved: #10 — structured stdio eviction notice', if PR #10's code really isn't in StdioBridgeHost.cs, that's a regression. Please confirm whether the feature was relocated, refactored away upstream, or accidentally dropped.
🟡 Non-blocking but worth a decision
.github/workflows/sync-releases.yml— new upstream workflow that force-pushesmaintorelease/<major.minor>on every push. Will fire on this fork too. Recommend either gating withif: github.repository == 'CoplayDev/unity-mcp'or removing the workflow if the fork doesn't use this release strategy. Inline comment added.
✅ What looked good on spot-check
MCPForUnity.Editor.asmdefunion resolution — fork'sversionDefinesretained,UNITY_VFX_GRAPHfrom upstream added correctly- New configurators (
KimiCodeConfigurator,AntigravityIdeConfigurator) follow the existingIMcpClientConfiguratorpattern cleanly IMcpClientConfiguratorinterface additions (UsesCliConfiguration, etc.) are non-breaking for existing implementors via the new propertiesunity-tests.ymlchange is benign (free-disk-space + matrix expansion)
Suggested next steps before merging
- Diff the pre-merge fork copies of
ScreenshotUtility.csandStdioBridgeHost.csagainst what's currently in this branch, and confirm the fork features are actually present (or document that upstream replaced/superseded them). - Complete the two items in the PR's own 'Verification needed' checklist (Unity compile + Window opens), since 222 files / +31k lines is far beyond what a code review alone can validate.
- Decide on
sync-releases.yml.
…c (fixes CS0103) The upstream-sync merge (PR #17) resolved the ScreenshotUtility.cs conflict with 'git checkout --ours', which took our entire fork copy and discarded ALL of upstream's additions to that file — including the ScreenshotCapturer MonoBehaviour (upstream §747). ManageUI.cs (auto-merged to upstream) then referenced an undefined ScreenshotCapturer → CS0103 on first compile. Re-resolved as a proper 3-way merge: keep our screen-capture-module guard (IsScreenCaptureModuleAvailable / ScreenCaptureModuleNotAvailableError, still referenced by ManageScene/ProjectInfo/ManageUI) AND restore upstream's ScreenshotCapturer + capture refactor. Verified: every method/class defined once, no leftover conflict markers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Syncs The1Studio/unity-mcp
betawith the latest upstreamCoplayDev/unity-mcp@beta(up to2d2bdc55, package 9.7.2-beta.9).Conflicts resolved (5)
MCPForUnity.Editor.asmdefversionDefines+ added upstreamUNITY_VFX_GRAPHStdioBridgeHost.csevictionBytesis required by the followingforeachScreenshotUtility.csREADME.mdwebsite/docs/architecture/STUDIO_TOOLS.mdFork features preserved
#16— UXML basePath fix under thefile:.../MCPForUnitysubmodule layout#10— structured stdio eviction noticeVerification needed before merge
Window > MCP for Unityopens (UXML loads) and Start Session works🤖 Generated with Claude Code