fix(shadergraph): restore ShaderGraph tools on Unity 6.3 / SG 17.3.0 (#18 follow-up)#23
Open
mrooney wants to merge 3 commits into
AnkleBreaker-Studio:mainAnkleBreaker-Studio/unity-mcp-plugin:mainfrom
atxlan:fix/shadergraph-addnode-arityatxlan/unity-mcp-plugin:fix/shadergraph-addnode-arityCopy head branch name to clipboard
Open
fix(shadergraph): restore ShaderGraph tools on Unity 6.3 / SG 17.3.0 (#18 follow-up)#23mrooney wants to merge 3 commits intoAnkleBreaker-Studio:mainAnkleBreaker-Studio/unity-mcp-plugin:mainfrom atxlan:fix/shadergraph-addnode-arityatxlan/unity-mcp-plugin:fix/shadergraph-addnode-arityCopy head branch name to clipboard
mrooney wants to merge 3 commits into
AnkleBreaker-Studio:mainAnkleBreaker-Studio/unity-mcp-plugin:mainfrom
atxlan:fix/shadergraph-addnode-arityatxlan/unity-mcp-plugin:fix/shadergraph-addnode-arityCopy head branch name to clipboard
Conversation
…haderGraph 17.x MCPShaderGraphApi resolved only the 1-arg GraphData.AddNode(AbstractMaterialNode) overload. On ShaderGraph 17.3.0 (Unity 6000.3.15f1) the only instance overloads are AddNode(AbstractMaterialNode, bool usePreviewPref) and AddNodeNoValidate(node), so the lookup returned null, the version-mismatch guard tripped, and Initialize() marked the whole API unavailable. Effect on 6.3: every write-path shadergraph tool (create / add_node / connect / disconnect / remove_node / set_node_property) failed with "ShaderGraph real-API access unavailable: One or more ShaderGraph API methods not found (version mismatch)". It failed closed rather than corrupting anything, but the fixes for AnkleBreaker-Studio#18 were unreachable. Probed all 15 required members on 17.3.0: AddNode was the only null one. Resolve either overload and match the arity at the call site; usePreviewPref: true is the parameter's declared default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ParseEdgesFromJson matches "m_OutputSlot".*?"m_Id" without RegexOptions.Singleline, so `.` never crosses a newline. Real .shadergraph files — including everything the new GraphData/MultiJson writer emits — put one field per line, so every match failed and shadergraph/get-edges reported edges with empty node ids and slot 0. Read-only path, so nothing was corrupted; the on-disk edges were correct the whole time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ound The GraphData rewrite kept AddNode type-only, so shadergraph/add-node with nodeType "Property" still wrote a node with m_Property.m_Id "" and m_Slots []. On the next import PropertyNode.OnEnable → AddOutputSlot NREs and the whole .shadergraph fails to import — AnkleBreaker-Studio#18 bug 3, unchanged in 2.39.4. (_propertyNodeT was resolved in Initialize but never used.) Accept propertyId (alias: property) = the objectId of a blackboard property on the graph, and assign PropertyNode.property after the node is added, which rebuilds the output slot from the property's concrete type. A Property node with no/unknown propertyId now errors before the graph is saved, so the asset is never left in the unimportable state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR restores functionality of the ShaderGraph MCP tooling on Unity 6.3 / ShaderGraph 17.3 by updating reflected API bindings and fixing edge parsing so graphs can be read/modified safely via ShaderGraph’s real GraphData model again.
Changes:
- Update reflected
GraphData.AddNodebinding to support the 2-parameter overload used by ShaderGraph 17.x and match the invocation arity at runtime. - Fix
get_edges/edge parsing for multi-line.shadergraphJSON by enabling regex singleline matching. - Add support for binding
PropertyNodeinstances to an existing blackboard property viapropertyId/property.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Editor/MCPShaderGraphCommands.cs | Passes propertyId/property through on add_node and fixes multi-line edge parsing. |
| Editor/MCPShaderGraphApi.cs | Updates reflection initialization and AddNode to handle 17.x overloads and bind PropertyNode to a blackboard property. |
Comments suppressed due to low confidence (1)
Editor/MCPShaderGraphApi.cs:234
AddNodecalls_propertyNodeT.IsInstanceOfType(node)unconditionally. If_propertyNodeTis null (e.g., type not present/renamed), this will throw aNullReferenceExceptioneven when adding non-Property nodes, turning a version mismatch into a confusing runtime failure. Guard against null and error explicitly if Property binding is requested but thepropertymember couldn’t be reflected.
if (_propertyNodeT.IsInstanceOfType(node))
{
if (string.IsNullOrEmpty(propertyId))
throw new InvalidOperationException(
"A Property node needs 'propertyId' — the objectId of a blackboard property on this graph (see shadergraph/get-nodes).");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+82
to
+84
| _graphPropertiesProp = _graphDataT.GetProperty("properties", PIF) ?? throw new InvalidOperationException("properties"); | ||
| _jsonObjectIdProp = Req(sg, "UnityEditor.ShaderGraph.Serialization.JsonObject").GetProperty("objectId", PIF) ?? throw new InvalidOperationException("JsonObject.objectId"); | ||
| _propertyNodePropertyProp = _propertyNodeT?.GetProperty("property", PIF) ?? throw new InvalidOperationException("PropertyNode.property"); |
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
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.
Follow-up to #18, verified against v2.39.4 on Unity 6000.3.15f1 / com.unity.shadergraph 17.3.0 / URP 17.3.0 (Windows).
I rolled back to v2.32.0 to confirm all four original bugs still reproduced, then upgraded to v2.39.4 to check the fixes. Two of the four are genuinely fixed by the
GraphData/MultiJsonrewrite — but on this ShaderGraph version the whole API refuses to initialize, so nothing was reachable. Three commits here, each independent.1.
GraphData.AddNodearity — the whole ShaderGraph tool suite is disabled on 6.3MCPShaderGraphApi.Initializeresolves only the 1-arg overload:On ShaderGraph 17.3.0 the only instance overloads are:
So
_addNodeis null, the version-mismatch guard throws, andAvailableis false. Every write-path tool (create/add_node/connect/disconnect/remove_node/set_node_property) returns:It fails closed and corrupts nothing — that part works exactly as designed — but the #18 fixes are unreachable on Unity 6.3. I probed all 15 required members on 17.3.0;
AddNodewas the only null one. Fixed by accepting either overload and matching the arity at the call site (usePreviewPref: trueis the parameter's declared default).2.
get_edgesreturns blank ids on multi-line edgesParseEdgesFromJsonuses"m_OutputSlot".*?"m_Id"withoutRegexOptions.Singleline, so.never crosses a newline. Every real.shadergraph— including everything the new writer emits — puts one field per line, so all four matches fail and each edge comes back as{"outputNodeId": "", "outputSlotId": 0, "inputNodeId": "", "inputSlotId": 0}. Read-only path, nothing corrupted; the on-disk edges were correct. Looks superficially like #18 bug 2b, which is how I noticed it.3. #18 bug 3 (PropertyNode) is still open
The rewrite kept
AddNodetype-only —_propertyNodeTis resolved inInitializebut never used, and nopropertyIdis read from the args.add_nodewithnodeType: "Property"still writes"m_Property": {"m_Id": ""}/"m_Slots": [], and the next import throws inPropertyNode.AddOutputSlotand fails the whole asset. Fixed by acceptingpropertyId(aliasproperty) and assigningPropertyNode.propertyafter the node is added, which rebuilds the output slot from the property's concrete type. A Property node with a missing or unknownpropertyIdnow errors before the graph is saved, so the asset is never left unimportable.Verification on 6.3 (17.3.0), all live
urp_lit+urp_unlitboth 809 B, noUniversalTarget, noBlockNodeedgeCount: 0m_SlotId: 1preserved, compilesAsset import failedAsset import failedm_Position (0,0)(150,250)and(400,250)as passedAlso confirmed the rejected-Property-node call leaves the target graph importable and untouched.
Happy to split these into three PRs if you'd rather take them separately.