Add create_child parameter to manage_prefabs modify_contents #646
Conversation
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
Enables adding child GameObjects to existing prefabs via headless editing. Supports single object or array for batch creation in one save operation. Features: - Create children with primitive types (Cube, Sphere, etc.) - Set position, rotation, scale on new children - Add components to children - Specify parent within prefab hierarchy for nested children - Set tag, layer, and active state Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Reviewer's GuideAdds a new create_child parameter to the manage_prefabs modify_contents flow, wiring Python-side request handling through to Unity editor code to create one or more child GameObjects in a prefab with configurable transform, components, and metadata. Sequence diagram for manage_prefabs modify_contents with create_childsequenceDiagram
actor User
participant PythonTool as manage_prefabs
participant UnityBridge as send_with_unity_instance
participant UnityEditor as ManagePrefabs_Editor
participant PrefabRoot as prefabRoot
User->>PythonTool: Call manage_prefabs(action=modify_contents, create_child)
PythonTool->>PythonTool: Normalize create_child (position, rotation, scale)
PythonTool->>UnityBridge: params including createChild
UnityBridge->>UnityEditor: ApplyModificationsToPrefabObject(params, targetGo, prefabRoot)
UnityEditor->>UnityEditor: Read createChild from params
alt createChild is array
loop For each childToken in createChild
UnityEditor->>UnityEditor: CreateSingleChildInPrefab(childToken, targetGo, prefabRoot)
alt ErrorResponse returned
UnityEditor-->>UnityBridge: error (success false)
UnityBridge-->>PythonTool: error
PythonTool-->>User: Failure response
else Child created
UnityEditor->>PrefabRoot: Add new child GameObject
end
end
else createChild is single object
UnityEditor->>UnityEditor: CreateSingleChildInPrefab(createChild, targetGo, prefabRoot)
alt ErrorResponse returned
UnityEditor-->>UnityBridge: error (success false)
UnityBridge-->>PythonTool: error
PythonTool-->>User: Failure response
else Child created
UnityEditor->>PrefabRoot: Add new child GameObject
end
end
UnityEditor-->>UnityBridge: modified true, no error
UnityBridge-->>PythonTool: Success response
PythonTool-->>User: Success (children created in prefab)
Class diagram for Unity ManagePrefabs create_child implementationclassDiagram
class ManagePrefabs
ManagePrefabs : +static ApplyModificationsToPrefabObject(JObject @params, GameObject targetGo, GameObject prefabRoot) bool ErrorResponse
ManagePrefabs : -static CreateSingleChildInPrefab(JToken createChildToken, GameObject defaultParent, GameObject prefabRoot) bool ErrorResponse
ManagePrefabs : -static FindInPrefabContents(GameObject prefabRoot, string name) GameObject
class ErrorResponse
ErrorResponse : +string message
ErrorResponse : +ErrorResponse(string message)
class GameObject
GameObject : +string name
GameObject : +Transform transform
GameObject : +int layer
GameObject : +string tag
GameObject : +void SetActive(bool value)
GameObject : +static GameObject CreatePrimitive(PrimitiveType type)
class Transform
Transform : +Vector3 localPosition
Transform : +Vector3 localEulerAngles
Transform : +Vector3 localScale
Transform : +Transform parent
Transform : +void SetParent(Transform parent, bool worldPositionStays)
class VectorParsing
VectorParsing : +static Vector3~nullable~ ParseVector3(JToken token)
class ComponentResolver
ComponentResolver : +static bool TryResolve(string typeName, Type componentType, string error)
class McpLog
McpLog : +static void Info(string message)
McpLog : +static void Warn(string message)
class LayerMask
LayerMask : +static int NameToLayer(string layerName)
class PrimitiveType
ManagePrefabs --> ErrorResponse
ManagePrefabs --> GameObject
ManagePrefabs --> Transform
ManagePrefabs --> VectorParsing
ManagePrefabs --> ComponentResolver
ManagePrefabs --> McpLog
ManagePrefabs --> LayerMask
GameObject --> Transform
GameObject --> PrimitiveType
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds headless prefab child-creation: the Python service accepts and normalizes a new create_child parameter and forwards it to Unity; the C# editor implements child GameObject creation, transform application, component attachment, tag/layer setting, and error handling during prefab modification. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant PythonServer as Python Server
participant UnityEditor as C# Editor
participant Prefab as Prefab Asset
Client->>PythonServer: manage_prefabs(create_child=[...])
PythonServer->>PythonServer: Validate & normalize vectors
PythonServer->>UnityEditor: Send command with params.createChild
UnityEditor->>UnityEditor: ApplyModificationsToPrefabObject()
loop For each child in params.createChild
UnityEditor->>UnityEditor: Resolve parent GameObject
UnityEditor->>UnityEditor: Create GameObject / primitive
UnityEditor->>UnityEditor: Apply position/rotation/scale
UnityEditor->>UnityEditor: Attach components (cleanup on failure)
UnityEditor->>UnityEditor: Set tag & layer
UnityEditor->>UnityEditor: Set active state
end
UnityEditor->>Prefab: Persist changes
UnityEditor->>PythonServer: Return success response
PythonServer->>Client: Return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
normalize_child_paramsthe return type is annotated astuple[dict, str | None]but the function can return(None, err)on failure; consider either adjusting the type hint to allowdict | Noneor returning an empty dict to keep the annotation accurate. - In
normalize_child_params,child_params = dict(child)assumeschildis a mapping and will raise a generic error for bad input; it may be more robust to explicitly check that eachchildis a dict and return a clear error message if not. - When iterating
componentsToAddinCreateSingleChildInPrefab, non-string tokens that are not objects with atypeNamefield are silently ignored; consider either validating and erroring on unexpected shapes or logging a warning so mis-specified component entries are easier to detect.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `normalize_child_params` the return type is annotated as `tuple[dict, str | None]` but the function can return `(None, err)` on failure; consider either adjusting the type hint to allow `dict | None` or returning an empty dict to keep the annotation accurate.
- In `normalize_child_params`, `child_params = dict(child)` assumes `child` is a mapping and will raise a generic error for bad input; it may be more robust to explicitly check that each `child` is a dict and return a clear error message if not.
- When iterating `componentsToAdd` in `CreateSingleChildInPrefab`, non-string tokens that are not objects with a `typeName` field are silently ignored; consider either validating and erroring on unexpected shapes or logging a warning so mis-specified component entries are easier to detect.
## Individual Comments
### Comment 1
<location> `Server/src/services/tools/manage_prefabs.py:164-168` </location>
<code_context>
+ child_params[vec_field] = vec_val
+ return child_params, None
+
+ if isinstance(create_child, list):
+ # Array of children
+ normalized_children = []
+ for i, child in enumerate(create_child):
+ child_params, err = normalize_child_params(child, i)
+ if err:
+ return {"success": False, "message": err}
</code_context>
<issue_to_address>
**issue (bug_risk):** Add validation or clearer error handling when list elements in `create_child` are not dictionaries.
In the list branch, `normalize_child_params` assumes each `child` is dict-like and does `child_params = dict(child)`. If a non-dict is passed (e.g. string, list), this may raise or behave unexpectedly. Consider explicitly checking `isinstance(child, dict)` and returning a clear error (including the index and expected structure) when the input is invalid.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs`:
- Around line 864-891: The child tag/layer assignment in ManagePrefabs.cs
currently only logs warnings (references: childParams, childName, newChild.tag
assignment and LayerMask.NameToLayer), causing silent successes when invalid
values are provided; change this to match the main target behavior by returning
an ErrorResponse on invalid tag or invalid layer for children (instead of only
McpLog.Warn), i.e., validate the tag and layer the same way the main target does
and propagate an error response back to the caller when validation fails so the
overall operation fails consistently.
- Fix type hint to `tuple[dict | None, str | None]` to match actual returns - Add explicit dict validation with clear error message including actual type - Error on invalid component entries instead of silently ignoring them - Return ErrorResponse for invalid tag/layer instead of just logging warnings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests cover: - Single child with primitive type - Empty GameObject (no primitive_type) - Multiple children from array (batch creation) - Nested parenting within prefab - Error handling for invalid inputs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
create_childparameter tomanage_prefabsmodify_contentsactionFeatures
primitive_type) or primitives (Cube, Sphere, Capsule, Cylinder, Plane, Quad)components_to_addExample Usage
Test plan
🤖 Generated with Claude Code