Repository navigation
Sync upstream CoplayDev/unity-mcp v10.2.0 - #43
Merged
Merged
Conversation
Mono's mcs compiler emits a stray U+FEFF BOM character on stdout during compilation. CodeDom's CSharpCodeProvider misinterprets this as a compiler error (no ErrorNumber, ErrorText is just the BOM), causing HasErrors to be true and results.CompiledAssembly to be null — even though mcs exits 0 and wrote the DLL successfully. Fix: - Compile to a temp DLL path (GenerateInMemory=false + OutputAssembly) instead of relying on results.CompiledAssembly. - Skip phantom errors where ErrorNumber is empty and ErrorText is only BOM/whitespace. - Load the produced DLL via Assembly.Load(File.ReadAllBytes(...)) when no real errors exist. - Clean up the temp DLL in a finally block. Fixes CoplayDev#1186
Measured four concurrent agents against one Editor over the HTTP transport (527 client calls, 1498 commands through the bridge at 97.9% success). The Editor stayed healthy throughout, so the notes below are about degradation modes, not failure. Guide gains a section on running several agents at once: a single Editor executes one command at a time, so reads stretched from ~5s to ~17s behind another agent's writes and batching bought nothing (~2-3s per call regardless of batch size). Also documents that 'Available: none' is a transient reconnect artifact rather than an absent Editor, and tabulates when a failed call is safe to retry. Architecture doc records two gaps found while measuring. Instance resolution runs before dispatch and hard-fails while the registry is briefly empty during a domain reload, which was roughly seven in ten client-visible errors. And a timed-out command is not cancelled in Unity: send_command pops the pending future, so a command already executing runs to completion and its result is discarded, reporting failure for an effect that landed. Commands still queued when the socket drops are lost cleanly instead. Also corrects the claim that throughput had not been measured; the numbers support the existing no-broker decision, since serialization comes from Unity's single-threaded main loop rather than the layer a broker would occupy.
…http-findings docs: record multi-agent HTTP behavior in routing docs
…to-beta-30766000799 chore: sync main (v10.1.2) into beta
Every PR to this repo comes from a fork -- including maintainers', who work from Scriptwonder/unity-mcp. GitHub withholds secrets from runs triggered by a fork's pull request, so unity-tests.yml skips and reports a green check having compiled nothing. Across the last 60 runs of that workflow: 51 pull_request (all skipped), 9 pull_request_target, and zero push/workflow_call. No PR has ever been compile-verified. Unity refuses to open a project without an activated license, so there is no way to compile via the Editor without secrets. This sidesteps that: it never launches Unity. It pulls the PUBLIC unityci/editor image purely to read reference assemblies out of it, and drives Roslyn -- Unity's own bundled csc -- directly. A compiler needs no license. Compiles MCPForUnity.Runtime and MCPForUnity.Editor for win, osx and linux. The existing Unity matrix is linux-only, so the UNITY_EDITOR_WIN and UNITY_EDITOR_OSX branches have never been compiled anywhere in CI. Runtime targets netstandard2.1 and Editor targets .NET 4.8, matching what Unity does; feeding both BCLs to one compile double-defines System.Object. Reference manifests and defines are captured from Unity's generated csprojs rather than globbed: Editor/Data holds the whole .NET 4.8 BCL plus vendored libraries Unity deliberately does not reference (ExCSS.Unity redefines System.Tuple, cscompmgd.dll redefines Microsoft.CSharp. CompilerError). Both need regenerating when defaultVersion changes. Verified locally against 2021.3.45f2: all six compiles pass, and a probe error behind #if UNITY_EDITOR_OSX fails the osx pass with exit 1 while win and linux stay green. This does not replace unity-tests.yml -- it compiles, it does not run tests.
ci: license-free compile check on every PR (incl. forks)
…e-bom-phantom-error fix(execute_code): skip phantom CodeDom BOM error from mcs compiler
…beta.2-30767776542 chore: update Unity package to beta version 10.1.3-beta.2
…ly-dedup fix: deduplicate CodeDom assembly references
…beta.3-30827275259 chore: update Unity package to beta version 10.1.3-beta.3
Long-running tool calls (asset imports, test runs, batched edits) were cut off ~30-90s into execution, so the task could never finish. On the stdio transport this was governed by hardcoded values on both hops: - Unity side: StdioBridgeHost.FrameIOTimeoutMs (30s const) capped every command's execution and frame I/O; on timeout the client reconnected and re-sent, which force-closed the prior client and made the bridge restart on a new port (the repeated "StdioBridgeHost started on port 6400/6402" churn). - Server side: ServerConfig.connection_timeout (30s socket recv) and command_total_timeout (90s cross-retry ceiling) cut the command off first. Unlike the WebSocket transport (WebSocketTransportClient reads a per-call timeout off the wire), the stdio bridge had no way to raise these. Make all three configurable with a 5-minute default: - FrameIOTimeoutMs: 30s -> 300s, env UNITY_MCP_STDIO_COMMAND_TIMEOUT_MS. ReceiveTimeout now scales with it (max(60s, timeout)). - connection_timeout: 30s -> 300s, env UNITY_MCP_CONNECTION_TIMEOUT. - command_total_timeout: 90s -> 600s, env UNITY_MCP_COMMAND_TOTAL_TIMEOUT. Invalid/non-positive env values fall back to the default so a bad override can't disable the timeout. Updates the config characterization test to the new defaults.
Address CodeRabbit review on CoplayDev#1320: - _env_float now requires math.isfinite(value): "inf"/"Infinity"/"1e309"/ "nan" are positive-or-parseable but would produce unusable socket/timeout behaviour, so they now fall back to the default like other invalid input. - test_config_default_values clears the two env vars first so ambient env can't mask the defaults. - Added coverage for valid overrides and for invalid/zero/negative/ non-finite values falling back to defaults.
…le-command-timeout fix: make stdio bridge command timeout configurable (default 5m)
…beta.4-31207888075 chore: update Unity package to beta version 10.1.3-beta.4
The GitHub stargazer API restrictions have broken the current star history charts in README.md and docs/i18n/README-zh.md. Point them at star-history.dera.page, whose SVG endpoint and frontend share the same domain and use an alternative data source that needs no API token.
The signature regex matches call sites as readily as declarations, and
`(\S+)` will capture punctuation as a "return type". That is normally
harmless because matches are non-overlapping: an ordinary declaration
consumes its own opening brace, so a call inside the body has no
preceding token left to match against.
Any call sitting at class-member depth escapes that, because there is no
enclosing declaration match to consume the token in front of it:
- field initializers -- `static bool X = ReadPref();` captures `=`;
- expression-bodied members -- `int Total => Count();` captures `=>`,
and `Current => _current ??= Build();` captures `??=`;
- the first statement of a body whose declaration the regex could not
match at all, i.e. an explicit interface implementation, since
`(\w+)` cannot span the dot in `IThing.Notify` -- the body's opening
brace is then captured instead.
All of these sit at class-member depth, so the brace-depth guard added
in "Prevent validator false positives from method-body calls" cannot
reject them, and valid code is reported as a duplicate. Zero-argument
methods are what bite in practice, since the key includes parameter
types and an empty list always matches.
This repository's own sources contain four instances (McpLog,
StdioBridgeHost, BridgeControlService, SecureKeyStore).
Rather than a third targeted skip after this and the `new` constructor
case, reject the class: a captured return type that is punctuation is
never a declaration. `(` is kept so tuple returns still validate.
Prevent manage_build from blocking on Unity's unsaved-scene dialog by calling AssetDatabase.SaveAssets and EditorSceneManager.SaveOpenScenes before BuildPipeline.BuildPlayer runs. Co-authored-by: Cursor <cursoragent@cursor.com>
Trae is a VS Code fork: its global MCP config is .../Trae/User/mcp.json, not the user data root. Since we have written one level up, it fails silently, because IsInstalled only stats the parent directory so the UI still reports "Configured". Verified on macOS against Trae 1.10; Windows and Linux paths are inferred from the same VS Code profile layout.
After a domain reload, StdioBridgeHost resumes its TCP listener through an editor-idle retry loop once the OS releases the port. That path bypasses TransportManager.StartAsync, so the manager's cached TransportState stays Disconnected while the bridge is actually serving requests. The MCP for Unity window reads that cached snapshot, so it reports 'Not connected' indefinitely (until a manual Verify), which is especially misleading during compile-heavy sessions where reloads are frequent. Fix in two layers: - StdioTransportClient.State now refreshes its snapshot from StdioBridgeHost.IsRunning whenever the two disagree, in both directions (late bind after a busy port, or a listener that died without StopAsync). - TransportManager.GetState(Stdio) reconciles its own cached state with the client's live connectivity before returning it, creating the client lazily so host auto-start (CI, [InitializeOnLoad]) is also covered. Adds EditMode tests pinning both reconcile directions.
A stop/start cycle between two state reads can rebind the listener to a different port (the 6400<->6401 busy-port fallback) while both cached snapshots stay 'connected', leaving a stale port in the window. Compare the live bound port as well as connectivity when deciding to refresh, in both StdioTransportClient.State and TransportManager's reconcile. Addresses CodeRabbit review on CoplayDev#1349; adds an EditMode test pinning the rebind case and docstrings on the new tests.
….json Claude Code gates every file-editing tool on Edit(path) rules and no longer matches MultiEdit(path). The MultiEdit(reports/**) entry is dead config and prints a warning on every launch: Permission allow rule (.claude\settings.json): MultiEdit(reports/**) is not matched by file permission checks — only Edit(path) rules are. Use Edit(reports/**) instead (Edit rules cover all file-editing tools). Edit(reports/**) is already present, so behavior is unchanged.
…permission-rule chore: drop unmatched MultiEdit permission rule from .claude/settings.json
…th-user-folder fix: correct Trae MCP config path
The constants never matched UnityEditor.ConsoleWindow.Mode. 1<<2 is Unity's Log bit but was labelled Warning, and every scripting bit was off by one (ScriptingError is 1<<8, not 1<<9). Logs surfaced as Warnings and Warnings as Errors; entries carrying only ScriptingError matched nothing in the table and were classified by InferTypeFromMessage finding "LogError" in the appended stack trace. Replaces the table with Unity's real values and rewrites GetLogTypeFromMode to check Exception and Assert before the error mask, since Unity sets the Error bit alongside both. Fixes CoplayDev#1348
DownscaleTexture called RenderTexture.GetTemporary without a RenderTextureReadWrite argument, so it defaulted to sRGB in a Linear colorspace project. Graphics.Blit then sampled the linear-flagged capture with no decode while the target applied the encode on store, turning 11 into 60 exactly as reported. The direct EncodeToPNG path never goes through DownscaleTexture, which is why the on-disk PNG was correct and only the inline preview was washed out. Fixes CoplayDev#1328
… call Every invocation emitted a fresh in-memory "MCPDynamic" assembly via Assembly.Load, and Mono cannot unload a non-collectible assembly, so the count only dropped at domain reload. The existing caches hold reference paths, not compiled output. Caches the compiled assembly keyed on compiler + wrapped source, cleared on domain reload alongside the path caches and capped at 64 entries so a stream of distinct snippets cannot become the leak itself. Distinct snippets still accumulate until the next reload; that is a Mono limitation. Fixes CoplayDev#1351
HandleCommand stripped any leading Assets/ and re-rooted the remainder under Application.dataPath, so Packages/com.foo/Samples/X.unity became Assets/Packages/com.foo/Samples/X.unity and could never resolve. Paths are now rooted at Assets or Packages, and the existence checks in LoadScene/LoadSceneAdditive move from File.Exists to the AssetDatabase -- the pattern ManageAsset.cs already documents, and the only one that works for packages resolved through Library/PackageCache. Fixes CoplayDev#1197
Two reported documentation gaps: - resources/read takes a server key separate from the mcpforunity:// URI. Codex exposes tools as mcp__unityMCP__* but wants server: "unityMCP" on a resource read, which the server instructions never mentioned. Distinct from the name-vs-URI mistake CoplayDev#1302 fixed. - com.unity.ai.assistant can livelock AssetDatabase::InitialRefresh on Unity 6000.5.x, before any MCP assembly loads, which presents as an MCP connection failure. Documents the packages-lock.json deletion people miss. Fixes CoplayDev#1220 Fixes CoplayDev#1219
The reference pages are generated from the Python annotations by tools/generate_docs_reference.py; the hand-edited row drifted from what the generator emits, failing the Docs -- Reference Drift Check.
Swapping File.Exists for an AssetDatabase lookup fixed Packages/ resolution but narrowed Assets/: the AssetDatabase does not know about a scene file until it is imported, so a scene written by an external tool before a refresh became unloadable where it previously opened. Accept either answer. The AssetDatabase is the only one that resolves Packages/... (embedded packages live in Library/PackageCache); File.Exists still covers the not-yet-imported case. The guard only exists to produce a clearer error than EditorSceneManager.OpenScene would, so accepting is the safe direction.
…beta.7-33447879193 chore: update Unity package to beta version 10.1.3-beta.7
…ity-screenshot-color-execute-code-cache fix: read_console severity, screenshot color space, execute_code assembly leak, manage_scene package paths
Both Unity workflows gated every step on a step-level `if:` reading an in-job license-detection output. A step-level `if:` yields step-conclusion `skipped`, which contributes nothing to the job conclusion -- so on any fork PR, where GitHub withholds the UNITY_* secrets, the jobs reported a green check having compiled nothing and booted no Editor. Both workflows already said so in their own comments and step summaries, but a step summary is not what the merge box shows, and reviewers read the tick. Hoists detection into a `license` gate job and gates the real job on it with a job-level `if:`. A job skipped that way reports "Skipped" in the merge box. Branch protection treats a skipped required check as satisfied, so this does not block merges -- it only stops the check claiming a pass it never earned. Also documents which checks are real signal on a fork PR, and drops the stale `safe-to-test` references from the contributor docs (that gate was retired in CoplayDev#1308).
AutoDiscoverCommands materialised every type in every non-dynamic loaded assembly and then called GetCustomAttribute on each one twice -- once for tools, once for resources. On large projects that measured at ~8.9s per domain reload, roughly half the reporter's total reload time (issue CoplayDev#1336), repeating work Unity had already done. TypeCache is Unity's precomputed attribute index, rebuilt once per domain reload. McpClientRegistry.BuildRegistry() already uses it for configurator discovery, so this follows an in-tree precedent rather than introducing a mechanism. It also removes the GetCustomAttribute calls that could hard-crash Mono inside the AssetImportWorker (issue CoplayDev#1134). The worker guard stays regardless, since the registry is unused there either way. HasAttributeSafe had no remaining callers and is deleted. Verified on Unity 6000.4.11f1: AutoDiscovery_RegistersAllBuiltInTools passes and discovery still reports 35 tools and 19 resources (54 handlers). Compile check passes on the 2021.3.45f2 floor. Fixes CoplayDev#1336
…beta.8-33527686174 chore: update Unity package to beta version 10.1.3-beta.8
From review feedback on this PR: - The unity-tests pull_request trigger is path-filtered, so 'Every PR gets a unity-tests status check' overstated it. A PR touching only Server/** or docs never creates the check. - The full-matrix section told contributors the label takes effect on the next pull_request OR pull_request_target event. unity-tests.yml deliberately has no pull_request_target trigger (line 31 says so), so half that sentence pointed at an event that never fires. - 'licence'/'Licence-free' -> 'license', matching UNITY_LICENSE and the rest of the guide.
RegisterCommandType lets a duplicate command name overwrite the previously registered handler, so registration order decides which type wins a collision. TypeCache does not document an order, while the assembly scan it replaces was stable within a build -- so a name collision could have resolved differently between domain reloads. Orders both passes by Type.FullName (ordinal). ~54 types, so the cost is noise against the ~8.9s this PR removes. Raised in review on this PR. Unity 6000.4.11f1: CommandRegistryTests 2/2 pass, discovery still reports 35 tools and 19 resources (54 handlers).
…mmand-discovery perf(discovery): use TypeCache instead of scanning every loaded assembly
…gnal ci: report Skipped instead of a green check when Unity tests cannot run
…beta.9-33544601521 chore: update Unity package to beta version 10.1.3-beta.9
MCPForUnity.Editor sets overrideReferences, so it only sees precompiled assemblies it names explicitly, and precompiledReferences listed only Newtonsoft.Json.dll. RoslynInstaller writes its DLLs into the consuming project's Assets/Plugins/Roslyn, outside the package, so defining USE_ROSLYN left ManageScript.cs unable to resolve its Microsoft.CodeAnalysis using directives and broke the build. Embedded installs happened to work; UPM installs could not. Adds the five DLL names RoslynInstaller installs. Unity ignores a precompiledReferences entry whose assembly is absent, so projects without Roslyn are unaffected — verified by the full EditMode suite compiling and passing with none of these DLLs present. The new test reads the DLL list off RoslynInstaller by reflection rather than duplicating it, so the two cannot drift apart. Closes CoplayDev#1295
…def-references fix: reference the Roslyn assemblies from MCPForUnity.Editor.asmdef
…beta.10-33552348896 chore: update Unity package to beta version 10.1.3-beta.10
chore: bump version to 10.2.0
Resolution notes (rerere replayed nothing; all conflicts resolved fresh): - TraeConfigurator, StdioBridgeHost, StdioTransportClient, e2e-bridge.yml: kept the fork's deletions; upstream's edits to them do not apply. - TransportManager: kept the fork's single-transport API; upstream's ReconciledStdioState() targets the removed TransportMode.Stdio branch. - ExecuteCode: fork's WrapUserCode(code, usings) plus upstream's compiled assembly cache; the wrapped source embeds the usings, so the key holds. - config.py: kept the fork's removal of the stdio socket settings, and dropped the _env_float helper that only those settings used. - Dropped upstream tests that target code the fork removed and would not compile or pass: 3 stdio reconciliation tests in TransportManagerTests, 2 timeout env-override tests in the core characterization suite. - ReadConsoleTests: kept the fork's SplitMessageAndStackTrace tests and upstream's GetLogTypeFromMode cases. - multi-instance.md: kept the fork's HTTP-only session line and adopted upstream's multi-agent concurrency section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
10 participants
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.
Merges upstream CoplayDev/unity-mcp v10.2.0 into the fork and applies the fork fixups (version
10.2.0-fork.1).Upstream highlights: TypeCache-based tool discovery;
execute_codecompiled-assembly cache and CodeDom BOM fix;read_consoleseverity bit table fix;manage_sceneacceptsPackages/paths; builds save dirty scenes first; screenshot downscale color space; Roslyn asmdef references;manage_scriptduplicate-signature false positive.Fork-side resolutions (details in the merge commit): kept the fork's deletions of the stdio transport files,
TraeConfiguratorande2e-bridge.yml; kept the single-transportTransportManagerand HTTP-onlyconfig.py; combinedExecuteCode(forkusings+ upstream cache); dropped upstream tests that target removed stdio code (3 inTransportManagerTests, 2 timeout env-override tests).Local gate: pytest 1533 passed, 3 skipped. Unity harness: EditMode leg PASS (1041/1109 passed), PlayMode leg PASS (0 tests).
Commits:
🤖 Generated with Claude Code