feat: tool robustness + usability (INVALID_PARAM, uniform response envelope, execute_code skip_refresh, screenshot auto-file)#34
Conversation
…velope, execute_code skip_refresh, screenshot auto-file)
Reliability:
- Malformed tool arguments no longer silently coerce to default(T)/Vector3.zero and run
with a wrong value. Unparseable values return a structured INVALID_PARAM naming the
parameter, provided value, and expected format -- in the reflection-based invoker AND in
set_transform/create_primitive's own vector parsing (create_primitive validates before
creating the object, leaving no half-created primitive on a bad value).
- Several Profiler/memory tools reported error/precondition states (get_object_memory empty
target, memory_list_snapshots with none, get_frame_timing unavailable, frame_debugger_get_events
with none) as bare strings on the success channel, so IsError() and callers treated the
failure as success. Now structured {success:false,code} errors.
- Every tool response is uniformly parseable as {success,...}: legacy bare-string successes are
wrapped in {success:true,message} by the serializer, while image data URIs and existing
{success:...} envelopes pass through untouched (screenshots still render; no double-wrap).
Usability:
- execute_code skip_refresh option bypasses the pre-compile AssetDatabase.Refresh + wait-for-ready
for read-only snippets or a live Play Mode session you must not disturb (the default refresh can
trigger a reload that wipes Play state, especially in a shared editor).
- Screenshot captures auto-fall back to a file above ~768KB instead of emitting an oversized
base64 payload that drops the client socket; returns {path,bytes,fell_back_to_file}. Small
captures still inline; save_to_file remains an explicit override.
Verified live in a Unity 6000.3.13f1 project (embedded package): compiles clean; malformed
create_primitive position -> INVALID_PARAM with no object created; get_object_memory empty target
-> TARGET_REQUIRED envelope; legacy string successes now returned as envelopes; execute_code
skip_refresh=true runs; FinishCapture(800KB) -> fell_back_to_file=true, (16B) -> inline.
(The CLAUDE.md/AGENTS.md managed-block change is split into its own PR.)
… (adversarial-audit follow-ups) - ConvertValue: an explicit empty string for a non-string param, an unrecognized bool, and a numeric/out-of-range enum value no longer silently coerce to default(T)/false/(T)N -- they now throw -> INVALID_PARAM like every other malformed value, closing the last holes in the contract. - Screenshot auto-fallback threshold measured on RAW png bytes (512KB) so the ~1.33x base64 expansion stays under the socket-drop point; capture_multiview's default inline path now also spills all frames to files when their combined size is too large (it previously bypassed the guard).
|
Thanks @dehuaichendragonplus. I landed this via commit 5e7dadf on main. The five core improvements are retained: strict typed-argument parsing, structured Profiler precondition errors, uniform success envelopes, Validation completed:
You are credited as co-author on the landing commit. Closing this PR because the work was incorporated on main with the additional compatibility and test coverage. |
Why this change is needed
Four independent reliability / ergonomics problems an AI agent driving Unity through this MCP hits in practice:
FunctionInvokerController.ConvertValuewraps all parsing intry { ... } catch { return default(T); }, andGameObjectFunctions.ParseVector3returnsVector3.zerowhen the string doesn't split into 3 parts. Soset_transform(position:"1,2")silently moves the object to the origin and reports success; a non-numeric int silently becomes0. The caller gets a success response for an operation that did the wrong thing, with no signal. This is the most dangerous class — it produces incorrect state invisibly."target is required.","No snapshots found.", ...). The plugin's ownToolResultFormatter.IsError()only recognizes failure by parsing{success:false}, so these read as success — a polling caller can loop forever or mis-report a failure as data.{success:false,code}JSON on failure, so a client cannot uniformly branch on asuccessfield — the success path is unparseable prose.execute_codealways runsAssetDatabase.Refresh+ wait-for-ready before compiling, which in a shared editor (or during Play Mode) can trigger a domain reload that wipes the runtime state you were inspecting. And high-resolution screenshots emit a multi-MB base64 payload that reliably drops the MCP client socket, putting the burden on the caller to know to passsave_to_file.Change approach
INVALID_PARAMnaming the parameter, provided value, and expected format — both in the reflection invoker (aToolArgumentExceptioncarrying context, thrown fromBuildArgumentsand mapped toINVALID_PARAM) and inset_transform/create_primitive's own vector parsing (TryParseVector3;create_primitivevalidates before creating the object, so a bad value leaves no half-created primitive).ProfilerFunctionsbecomeToolResultFormatter.Error(code, { hint })(TARGET_REQUIRED,NO_SNAPSHOTS,FRAME_TIMING_UNAVAILABLE,NO_FRAME_DEBUGGER_EVENTS).SerializeResultwraps a legacy tool's bare success string in{success:true,message}— but passes through (a) imagedata:URIs (still rendered as image content downstream) and (b) strings already in a{success:...}envelope (no double-wrap). Result: every tool response is uniformly{success,...}.execute_codegains an optionalskip_refresh(defaultfalse) that bypasses the pre-compile refresh.FinishCaptureauto-falls back to a file when the PNG exceeds ~768 KB (returns{path, bytes, fell_back_to_file:true}); small captures still inline;save_to_fileremains an explicit override.Benefits
IsError) can reliably detect failures across the whole tool surface.{success,...}— removes a whole class of client-side parse fragility — while images and existing envelopes are untouched.skip_refreshmakes read-only inspection and live Play-Mode debugging safe in a shared editor.Test results
Verified live in a Unity 6000.3.13f1 project (embedded package, compiles with zero errors):
create_primitive(position:"1,2")→{success:false, code:"INVALID_PARAM", data:{param:"position", provided:"1,2", expected:"Vector3 'x,y,z'", detail:"...got 2"}}, andget_hierarchyconfirms no object was created ✔get_object_memory(target:"")→{success:false, code:"TARGET_REQUIRED"}(was a bare string) ✔get_compilation_errors/memory_list_snapshotssuccess now returned as{success:true,message:"..."}envelopes ✔execute_code(skip_refresh:true)runs a snippet without a refresh ✔FinishCapturevia reflection: 800 KB payload →{..., fell_back_to_file:true}saved to file; 16 B → inlinedata:URI ✔; live captures under the threshold still return inline images ✔Compatibility testing
initializehandshake, and tool schemas are untouched → compatible with all MCP clients (Claude Code, Cursor, LM Studio, VS Code, Trae, Kiro, Codex).skip_refreshis a new optional parameter (defaultfalse) → existing callers unaffected.{success:true,message}JSON: still a valid text content block for every client, now additionally machine-parseable; imagedata:URIs are preserved so image rendering is unchanged; error envelopes are never double-wrapped.Checklist
Funplay > MCP Serveropens and starts correctly.idea/or.DS_StoreCHANGELOG.md