Add delete_child command for prefab manage_contents action#980
Open
darkdread wants to merge 1 commit intoCoplayDev:betafrom
Open
Add delete_child command for prefab manage_contents action#980darkdread wants to merge 1 commit intoCoplayDev:betafrom
darkdread wants to merge 1 commit intoCoplayDev:betafrom
Conversation
Also updated unity-mcp prefab cli command to include modify command
Contributor
Reviewer's GuideAdds a new headless Sequence diagram for prefab modify delete_child flowsequenceDiagram
actor Developer
participant CLI_prefab_modify as CLI_prefab_modify
participant manage_prefabs_tool as manage_prefabs_tool
participant Unity_ManagePrefabs as Unity_ManagePrefabs
participant RemoveChildren as RemoveChildren
Developer->>CLI_prefab_modify: unity-mcp prefab modify path --delete-child Child1 --delete-child Turret/Barrel
CLI_prefab_modify->>CLI_prefab_modify: _parse_vector3 / _parse_property (if used)
CLI_prefab_modify->>CLI_prefab_modify: build params with action=modify_contents
CLI_prefab_modify->>CLI_prefab_modify: params[deleteChild] = ["Child1","Turret/Barrel"]
CLI_prefab_modify->>manage_prefabs_tool: run_command manage_prefabs(params)
manage_prefabs_tool->>manage_prefabs_tool: manage_prefabs(..., delete_child)
manage_prefabs_tool->>manage_prefabs_tool: if delete_child is not None
manage_prefabs_tool->>manage_prefabs_tool: params[deleteChild] = delete_child
manage_prefabs_tool->>Unity_ManagePrefabs: async_send_command_with_retry("manage_prefabs", params)
Unity_ManagePrefabs->>Unity_ManagePrefabs: ApplyModificationsToPrefabObject(@params, targetGo, prefabRoot)
Unity_ManagePrefabs->>Unity_ManagePrefabs: read deleteChildToken = @params[deleteChild] or @params[delete_child]
Unity_ManagePrefabs->>RemoveChildren: RemoveChildren(deleteChildToken, targetGo, prefabRoot)
alt error while removing
RemoveChildren-->>Unity_ManagePrefabs: (removedCount, error)
Unity_ManagePrefabs-->>manage_prefabs_tool: ErrorResponse
manage_prefabs_tool-->>CLI_prefab_modify: error result
CLI_prefab_modify-->>Developer: formatted error output
else removed one or more children
RemoveChildren-->>Unity_ManagePrefabs: (removedCount>0, null)
Unity_ManagePrefabs->>Unity_ManagePrefabs: modified = true
Unity_ManagePrefabs-->>manage_prefabs_tool: success result
manage_prefabs_tool-->>CLI_prefab_modify: success result
CLI_prefab_modify-->>Developer: "Modified prefab: path"
end
Sequence diagram for set_property mapping to componentPropertiessequenceDiagram
actor Developer
participant CLI_prefab_modify as CLI_prefab_modify
participant manage_prefabs_tool as manage_prefabs_tool
participant Unity_ManagePrefabs as Unity_ManagePrefabs
Developer->>CLI_prefab_modify: unity-mcp prefab modify path --set-property "Rigidbody.mass=5"
CLI_prefab_modify->>CLI_prefab_modify: _parse_property("Rigidbody.mass=5") -> ("Rigidbody","mass",5)
CLI_prefab_modify->>CLI_prefab_modify: build component_properties = {Rigidbody: {mass: 5}}
CLI_prefab_modify->>manage_prefabs_tool: run_command manage_prefabs(params with componentProperties)
manage_prefabs_tool->>Unity_ManagePrefabs: send manage_prefabs with componentProperties
Unity_ManagePrefabs->>Unity_ManagePrefabs: ApplyModificationsToPrefabObject reads componentProperties
Unity_ManagePrefabs->>Unity_ManagePrefabs: set serialized fields on existing components
Unity_ManagePrefabs-->>manage_prefabs_tool: success result
manage_prefabs_tool-->>CLI_prefab_modify: success result
CLI_prefab_modify-->>Developer: "Modified prefab: path"
Class diagram for updated ManagePrefabs delete_child supportclassDiagram
class PrefabCLICommands {
+modify(path, target, position, rotation, scale, name, tag, layer, active, parent, add_component, remove_component, set_property, delete_child, create_child)
-_parse_vector3(value)
-_parse_property(prop_str)
}
class ManagePrefabsTool {
+async manage_prefabs(action, prefab_path, target, include_inactive, unlink, source_prefab_path, source_scene_path, components_to_add, components_to_remove, create_child, delete_child, component_properties)
+normalize_child_params(child, index)
}
class ManagePrefabs {
+ApplyModificationsToPrefabObject(@params, targetGo, prefabRoot) (bool modified, ErrorResponse error)
+CreateSingleChildInPrefab(childParams, targetGo, prefabRoot) (bool created, ErrorResponse error)
-RemoveChildren(deleteChildToken, targetGo, prefabRoot) (int removedCount, ErrorResponse error)
}
PrefabCLICommands --> ManagePrefabsTool : uses manage_prefabs via run_command
ManagePrefabsTool --> ManagePrefabs : sends params to manage_prefabs command
ManagePrefabs ..> RemoveChildren : calls for deleteChild processing
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as prefab modify<br/>(CLI Command)
participant Server as manage_prefabs<br/>(Service)
participant Editor as ManagePrefabs<br/>(Unity Editor)
participant Scene as Scene/Prefab<br/>(GameObject)
User->>CLI: prefab modify --delete-child "ChildName"
CLI->>CLI: Parse --delete-child flags<br/>into deleteChild array
CLI->>Server: run_command(action="modify_contents",<br/>deleteChild=[...])
Server->>Server: Append deleteChild to<br/>command payload
Server->>Editor: HandleCommand(action="modify_contents",<br/>deleteChild=[...])
Editor->>Editor: RemoveChildren(deleteChild,<br/>targetGo, prefabRoot)
Editor->>Editor: Normalize deleteChild to array<br/>Resolve each child path
Editor->>Scene: DestroyImmediate(child)
Scene-->>Editor: Child destroyed
Editor->>Editor: Set modified = true
Editor-->>Server: ErrorResponse (success)
Server-->>CLI: Response
CLI-->>User: Command result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 the CLI
_parse_vector3()helper, consider catchingValueErrorand re-raising aclick.BadParameterwith a clearer message so malformed numeric input (e.g.--position 1,foo,3) produces a friendly CLI error instead of a stack trace. - In
RemoveChildrenthe error message mentions'delete_child'while the code accepts bothdeleteChildanddelete_child; it may be less confusing to align the wording with the accepted parameter names or explicitly mention both.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the CLI `_parse_vector3()` helper, consider catching `ValueError` and re-raising a `click.BadParameter` with a clearer message so malformed numeric input (e.g. `--position 1,foo,3`) produces a friendly CLI error instead of a stack trace.
- In `RemoveChildren` the error message mentions `'delete_child'` while the code accepts both `deleteChild` and `delete_child`; it may be less confusing to align the wording with the accepted parameter names or explicitly mention both.
## Individual Comments
### Comment 1
<location path="Server/src/cli/commands/prefab.py" line_range="252-257" />
<code_context>
print_success(f"Created prefab: {path}")
+
+
+def _parse_vector3(value: str) -> list[float]:
+ """Parse 'x,y,z' string to list of floats."""
+ parts = value.split(",")
+ if len(parts) != 3:
+ raise click.BadParameter("Must be 'x,y,z' format")
+ return [float(p.strip()) for p in parts]
+
+
</code_context>
<issue_to_address>
**issue:** Handle non-numeric vector components with a clearer error instead of propagating ValueError.
If any component in `value` isn’t numeric, `float(p.strip())` raises `ValueError`, which Click will show with its default messaging and stack trace. Consider wrapping the comprehension in try/except and raising `click.BadParameter` with a clearer message (e.g. including the original `value` and stating that all three components must be numeric).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+252
to
+257
| def _parse_vector3(value: str) -> list[float]: | ||
| """Parse 'x,y,z' string to list of floats.""" | ||
| parts = value.split(",") | ||
| if len(parts) != 3: | ||
| raise click.BadParameter("Must be 'x,y,z' format") | ||
| return [float(p.strip()) for p in parts] |
Contributor
There was a problem hiding this comment.
issue: Handle non-numeric vector components with a clearer error instead of propagating ValueError.
If any component in value isn’t numeric, float(p.strip()) raises ValueError, which Click will show with its default messaging and stack trace. Consider wrapping the comprehension in try/except and raising click.BadParameter with a clearer message (e.g. including the original value and stating that all three components must be numeric).
![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
🧹 Nitpick comments (1)
Server/src/cli/commands/prefab.py (1)
361-365: Chain the exception for better debugging.Per Python best practices, use
raise ... from eto preserve the exception chain, which helps with debugging by showing both the original JSON error and the re-raisedBadParameter.♻️ Proposed fix
if create_child: try: params["createChild"] = json.loads(create_child) except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON for --create-child: {e}") + raise click.BadParameter(f"Invalid JSON for --create-child: {e}") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/cli/commands/prefab.py` around lines 361 - 365, The exception handling for JSON parsing of create_child should preserve the original JSONDecodeError; in the except block catching json.JSONDecodeError as e (around the code that sets params["createChild"] from json.loads(create_child)), re-raise the click.BadParameter using exception chaining (i.e., raise click.BadParameter(f"Invalid JSON for --create-child: {e}") from e) so the original error context is retained for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Server/src/services/tools/manage_prefabs.py`:
- Around line 32-34: The docstring describing the delete_child parameter in
manage_prefabs.py has a missing closing parenthesis; update the string that
contains "Use delete_child parameter to remove child GameObjects from the prefab
(single name/path or array of paths for batch deletion. Example:
delete_child=[\"Child1\", \"Child2/Grandchild\"]). " to include the missing
closing parenthesis (so the parentheses around the phrase are balanced) — locate
the delete_child description in the module or function docstring and append the
closing ")" where the sentence ends.
---
Nitpick comments:
In `@Server/src/cli/commands/prefab.py`:
- Around line 361-365: The exception handling for JSON parsing of create_child
should preserve the original JSONDecodeError; in the except block catching
json.JSONDecodeError as e (around the code that sets params["createChild"] from
json.loads(create_child)), re-raise the click.BadParameter using exception
chaining (i.e., raise click.BadParameter(f"Invalid JSON for --create-child:
{e}") from e) so the original error context is retained for debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16438862-91de-4ce2-823b-6e80b1729dd8
📒 Files selected for processing (5)
MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.csServer/src/cli/commands/prefab.pyServer/src/services/tools/manage_prefabs.pyServer/tests/test_cli.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs
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
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.
Regarding documentation, I don't see
tools/UPDATE_DOCS.md. Also I couldn't findtools/check_docs_sync.pymentioned inUPDATE_DOCS_PROMPT.md.Description
Added
delete_childparameter tomanage_prefabstool and CLI command for batch removal of child GameObjects from prefabs.Type of Change
Changes Made
CLI (
Server/src/cli/commands/prefab.py)prefab modifycommand for headless prefab editing (+121 lines)_parse_vector3(),_parse_property()for CLI argument parsing--target,--position,--rotation,--scale,--name,--tag,--layer,--active/--inactive,--parent,--add-component,--remove-component,--set-property,--delete-child,--create-childPython MCP Tool (
Server/src/services/tools/manage_prefabs.py)delete_child: str | list[str]parameterC# Implementation (
MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs)deleteChild/delete_childparameter handling inApplyModificationsToPrefabObject()RemoveChildren()method (+41 lines) usingTransform.Find()for child lookupTests
Python CLI tests (+8):
test_prefab_modify_*covering delete_child, transform, set_property, components, create_child, active_stateUnity tests (+4):
ModifyContents_DeleteChild_*for single, nested, multiple, and nonexistent child scenariosTesting/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
Fixes #979
Additional Notes
Summary by Sourcery
Add support for headless modification of prefab contents, including deleting child GameObjects, via both the CLI and the manage_prefabs tool.
New Features:
prefab modifyCLI command to edit prefab contents without opening the Unity editor, including transforms, components, properties, child creation, and child deletion.delete_childparameter for removing child GameObjects by name or path from prefabs.Tests:
Summary by CodeRabbit
New Features
prefab modifyCLI command, supporting single or multiple child removals by name or path.Tests