Skip to content

feat: scene building, domain-reload survival, Unity 6 compat, bridge audit - #9

Open
isekream wants to merge 16 commits into
mainfrom
feat/scene-building-and-reliability
Open

feat: scene building, domain-reload survival, Unity 6 compat, bridge audit#9
isekream wants to merge 16 commits into
mainfrom
feat/scene-building-and-reliability

Conversation

@isekream

@isekream isekream commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

Supersedes #8. This branch reconciles the threading/headless server rewrite with the static tool-registry work, then layers reliability fixes, scene-building capabilities, Unity 6 compatibility, and Node bridge audit fixes.

Changes

Reliability (P0)

Scene building (P0)

  • create_primitive — Cube/Sphere/Capsule/Cylinder/Plane/Quad with mesh + collider
  • primitive on createGameObject — same path via existing MCP tool
  • Edit-mode modifyComponentSerializedObject property setter (vectors, colors, enums, object refs by name/id/asset path)
  • Node schemasscene.createPrimitive tool + primitive enum on scene.createGameObject

Compatibility & correctness (P1)

  • Unity 6 compile guardsPhysicMaterial/PhysicsMaterial, SceneManager.loadedSceneCount, symlinkSources
  • instanceId serialization — returns GetInstanceID() int via UnityCompat.GetObjectId
  • Playmode routing fix — accept snake_case actions injected by NormalizeAction (get_state, inspect_game_object, etc.)

Infrastructure

  • Shared McpEditorHelpers + UnityCompat utilities
  • Node bridge schema/logging alignment
  • Updated smoke test script

Test plan

  • Node tsc build passes
  • Unity Editor smoke test (server not reachable on :8090 during PR creation):
    • test ping
    • scene.createPrimitive → Capsule
    • scene.modifyComponent add Rigidbody + modify mass
    • scene.query — verify instanceId is int, not {}
    • Domain reload — confirm server auto-restarts with Auto Start enabled

Supersedes

Closes #8 — all threading/headless changes are merged and extended here.

Anzeit and others added 11 commits June 8, 2026 08:39
…pendent

The editor MCP server had coupled defects causing requests to hang or
return empty bodies:

1. Serial request handling: HandleRequests processed each request inline
   on the listener thread, blocking the accept loop for up to
   requestTimeout seconds, so subsequent requests sat unanswered until
   the current one finished. Each request is now serviced on the
   ThreadPool so GetContext() loops immediately.

2. Cross-thread EditorApplication.delayCall: tool execution was dispatched
   to the main thread via `delayCall +=` from the background listener
   thread. delayCall is a non-thread-safe static delegate; concurrent
   subscription drops callbacks, so the completion flag never flips and
   the request times out with no body. Replaced with a thread-safe
   ConcurrentQueue<Action> drained by a single persistent
   EditorApplication.update pump.

3. Window-coupled registry + static-ctor crash: tools lived on the
   EditorWindow instance, so a closed window made every call fail with
   "window not open"; and LoadPreferences() called EditorPrefs.* directly
   in the static constructor, which Unity forbids ("GetInt is not allowed
   to be called from a ScriptableObject constructor"). The registry is
   now static and initialized on server start; preference loading is
   deferred to EditorApplication.delayCall.

Adds volatile completion state for correct cross-thread visibility.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
UnityCompat: reflection-based shims for Unity 6 API renames (GetEntityId,
Rigidbody linearVelocity/linearDamping). McpEditorHelpers: shared GameObject
lookup, asset-path normalization, component-type resolution, JSON vector/color
extraction used across tools.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Route GameObject lookup, asset-path handling, and component resolution through
McpEditorHelpers; return UnityCompat.GetObjectId() (int) for instanceId so it no
longer serializes as {} and objects can be referenced by id.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…reload survival

Combines PR #8's concurrent, window-independent HTTP server (ThreadPool per
request + persistent main-thread pump) with the working-tree static tool
registry, unifying on EnsureToolsInitialized().

Adds domain-reload survival: a SessionState 'was running' flag is set on start
and captured in AssemblyReloadEvents.beforeAssemblyReload (which also releases
the HTTP port via ShutdownListener). The deferred static-ctor startup restarts
the listener when that flag is set or auto-start is enabled, so script recompiles
and Play Mode (with Reload Domain) no longer drop the server. User-initiated
StopServer clears the flag so it won't auto-restart after a later reload.

Supersedes #8.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…link)

- Read Physics.defaultMaterial as UnityEngine.Object (PhysicMaterial was renamed
  PhysicsMaterial in Unity 6; only .name is needed) to avoid the removed type name.
- Route loadedSceneCount/GetSceneAt through runtime SceneManager (stable on both
  2022.3 and Unity 6) instead of the deprecated EditorSceneManager members.
- Guard symlinkLibraries -> symlinkSources behind UNITY_6000_0_OR_NEWER while
  keeping the JSON output key stable for the Node bridge.

Preserves the declared 2022.3 minimum (no raw renames).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
- create_primitive action and primitive param on create_object
- SerializedObject-based property setting for add/modify component actions
- Supports vectors, colors, enums, and object references by name/id/path
McpUnityServer injects normalized snake_case actions (e.g. get_state,
inspect_game_object) but PlayModeTool only matched camelCase. Accept both
forms so all playmode.* MCP tools route correctly.
…ject

Expose primitive creation in MCP schemas to match Unity scene_manipulate
support and documented TOOLS_REFERENCE examples.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6527010523

ℹ️ 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".

Comment thread Unity/Editor/UnityCompat.cs Outdated

if (GetEntityIdMethod != null)
{
return (int)GetEntityIdMethod.Invoke(obj, null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Convert EntityId before returning an int

On Unity versions where Object.GetEntityId() exists but returns the newer EntityId value instead of an int, this reflection call returns a boxed EntityId; casting that boxed value directly to int throws InvalidCastException. Any tool response that includes an object id (for example create/query/instantiate paths) will fail at runtime in those Unity 6 environments instead of falling back to GetInstanceID() or converting via the supported API.

Useful? React with 👍 / 👎.

Comment on lines +153 to +155
var subAction = "create";
if (rawParams is JObject jObj)
subAction = (jObj.Value<string>("action") ?? "create").Trim().ToLowerInvariant();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve prefab sub-actions after routing

When assets.managePrefabs is called through the normal dotted method path, ProcessMcpRequest routes it to asset_manage via InjectAction, which converts the params to a Dictionary<string, object>. In that context this JObject check is false, so subAction stays at the default create; requests with action: "instantiate" are incorrectly handled as creates and fail with gameObjectName is required instead of instantiating the prefab.

Useful? React with 👍 / 👎.


var material = new Material(shader) { name = args.name };

if (rawParams is JObject jObj && jObj["properties"] is JObject props)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Read material properties from routed params

For normal assets.createMaterial calls the router injects the action by serializing params into a Dictionary<string, object>, so rawParams is JObject is false here. As a result, requested properties.color, metallic, and smoothness are silently ignored and the material is created with defaults despite the server schema advertising those fields.

Useful? React with 👍 / 👎.

return CreateErrorResponse($"Script '{scriptName}' already exists. Set overwrite=true to replace.");

var baseClass = args.baseClass;
if (!string.IsNullOrWhiteSpace(args.template) && string.IsNullOrWhiteSpace(args.baseClass))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor the ScriptableObject template default

Because baseClass defaults to "MonoBehaviour", this condition is false whenever callers omit baseClass, so template: "ScriptableObject" never gets its template default applied. A code.createScript request for a ScriptableObject therefore falls through to BuildTemplate with baseClass == "MonoBehaviour" and generates a MonoBehaviour script instead.

Useful? React with 👍 / 👎.

isekream added 4 commits June 8, 2026 13:28
- Resolve SerializedProperty.type as string when wiring object references
- Guard AssetDatabase.CopyAsset bool vs string return across Unity versions
- Remove duplicate organize switch arm in AssetManagerTool
GetEntityId() returns an EntityId struct in Unity 6, which cannot be cast
to int and broke scene.createGameObject/createPrimitive responses. Also fix
.meta YAML formatting by cloning the working MonoImporter template.
delayCall subscribed from HTTP worker threads can run on the wrong thread
in Unity 6 (SetSceneRepaintDirty errors). Use a ConcurrentQueue drained by a
delayCall pump chain scheduled from the editor main thread, plus immediate
execution when already on the main thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants