feat(manage_gameobject): add is_static parameter to modify action#1005
Conversation
manage_gameobject already serializes isStatic in read output (GameObjectSerializer, GameObjectResource) but the modify action had no parameter to write it. This adds is_static (bool) across all three layers: - C#: GameObjectModify.cs calls GameObjectUtility.SetStaticEditorFlags() - Python MCP tool: manage_gameobject.py with coerce_bool normalization - Python CLI: gameobject.py with --static/--no-static flag When true, all static flags are set; when false, all are cleared. Omitting the parameter is a no-op (fully backwards compatible). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests cover: is_static=True, is_static=False, string coercion
("true" → True), and omission (isStatic excluded from params).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdds an Sequence diagram for manage_gameobject modify with is_static propagationsequenceDiagram
actor AIClient
participant ManageGameobjectTool as ManageGameobjectTool_Python
participant UnityMCP as UnityMCP_Server
participant GameObjectModify as GameObjectModify_CSharp
AIClient->>ManageGameobjectTool: manage_gameobject(action=modify, target, is_static)
ManageGameobjectTool->>ManageGameobjectTool: coerce_bool(is_static)
ManageGameobjectTool->>UnityMCP: POST manage_gameobject params including isStatic
UnityMCP->>GameObjectModify: Handle(params)
GameObjectModify->>GameObjectModify: read isStatic from params
GameObjectModify->>GameObjectModify: SetStaticEditorFlags(targetGo, flags)
GameObjectModify-->>UnityMCP: modified GameObject
UnityMCP-->>ManageGameobjectTool: response
ManageGameobjectTool-->>AIClient: updated GameObject data including isStatic
Class diagram for manage_gameobject and GameObjectModify is_static handlingclassDiagram
class GameObjectCLICommand {
+modify(target str, search_method str, tag str, layer str, active bool, static bool, add_components str, remove_components str)
}
class ManageGameobjectTool {
+manage_gameobject(action str, target str, search_method str, prefab_name str, prefab_folder str, save_as_prefab bool, set_active bool, layer str, is_static bool, components_to_remove list~str~, component_properties dict~str, dict~str, any~~, world_space bool, position any, rotation any, scale any)
-coerce_bool(value any, default bool)
}
class GameObjectModify {
+Handle(params JObject, targetToken JToken, searchMethod string) object
-ApplyLayer(targetGo GameObject, layer string)
-ApplyActive(targetGo GameObject, setActive bool)
-ApplyStatic(targetGo GameObject, isStatic bool)
}
GameObjectCLICommand --> ManageGameobjectTool : builds params
ManageGameobjectTool --> GameObjectModify : passes isStatic in params
GameObjectModify ..> GameObject : modifies isStatic and StaticEditorFlags
GameObjectModify ..> StaticEditorFlags : uses for SetStaticEditorFlags
GameObjectModify ..> GameObjectUtility : calls SetStaticEditorFlags
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds optional static-flag support end-to-end: a CLI Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Service
participant Unity
CLI->>Service: modify command with params (including isStatic)
Service->>Service: coerce is_static -> boolean or remove if None
Service->>Unity: send command params (action, target, isStatic?)
Unity->>Unity: GameObjectModify.Handle parses params
Unity->>Unity: compute desired StaticEditorFlags
Unity->>Unity: if changed -> SetStaticEditorFlags(target, desiredFlags)
Unity-->>Service: response (success/data)
Service-->>CLI: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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🧪 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 |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Server/src/services/tools/manage_gameobject.py" line_range="186-188" />
<code_context>
"prefabFolder": prefab_folder,
"setActive": set_active,
"layer": layer,
+ "isStatic": is_static,
"componentsToRemove": components_to_remove,
"componentProperties": component_properties,
</code_context>
<issue_to_address>
**suggestion:** Avoid sending `"isStatic": None` in the payload when no static change was requested.
`is_static` is currently coerced and left as `None` when unspecified, but the payload always includes:
```python
"isStatic": is_static,
```
so the receiver gets `"isStatic": null`. Even though the C# side handles nullable values, it’s clearer and safer to omit the key when no change is requested. Consider only adding it when `is_static is not None`:
```python
payload = {
"prefabFolder": prefab_folder,
"setActive": set_active,
"layer": layer,
# ...
}
if is_static is not None:
payload["isStatic"] = is_static
```
This matches how other optional fields are handled and avoids relying on `null` semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Server/tests/integration/test_manage_gameobject_is_static.py (1)
61-85: Add one more coercion test for"false"You already cover
"true"string coercion; adding"false"would lock in the symmetric parse path and prevent regressions in false-value serialization.🧪 Suggested additional test
+@pytest.mark.asyncio +async def test_manage_gameobject_is_static_string_false_coercion(monkeypatch): + captured = {} + + async def fake_send(cmd, params, **kwargs): + captured["params"] = params + return {"success": True, "data": {}} + + monkeypatch.setattr( + manage_go_mod, + "async_send_command_with_retry", + fake_send, + ) + + resp = await manage_go_mod.manage_gameobject( + ctx=DummyContext(), + action="modify", + target="Ground", + is_static="false", + ) + + assert resp.get("success") is True + assert captured["params"]["isStatic"] is FalseAlso applies to: 87-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/tests/integration/test_manage_gameobject_is_static.py` around lines 61 - 85, Add a mirrored test to assert string "false" is coerced to boolean False: duplicate the existing test_manage_gameobject_is_static_string_coercion pattern but call manage_go_mod.manage_gameobject with is_static="false", patch manage_go_mod.async_send_command_with_retry (the fake_send that captures params), await the response and assert resp.get("success") is True and captured["params"]["isStatic"] is False to ensure the false-value serialization path is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.cs`:
- Around line 154-160: The comparison using targetGo.isStatic is wrong for
partial static objects; instead obtain the current flags via
GameObjectUtility.GetStaticEditorFlags(targetGo) and compare to the desired
flags (use desired = isStatic.Value ? (StaticEditorFlags)~0 : 0) and only call
GameObjectUtility.SetStaticEditorFlags(targetGo, desired) if currentFlags !=
desired; update the block referencing isStatic, targetGo.isStatic,
StaticEditorFlags and GameObjectUtility.SetStaticEditorFlags to use
GetStaticEditorFlags for the comparison.
---
Nitpick comments:
In `@Server/tests/integration/test_manage_gameobject_is_static.py`:
- Around line 61-85: Add a mirrored test to assert string "false" is coerced to
boolean False: duplicate the existing
test_manage_gameobject_is_static_string_coercion pattern but call
manage_go_mod.manage_gameobject with is_static="false", patch
manage_go_mod.async_send_command_with_retry (the fake_send that captures
params), await the response and assert resp.get("success") is True and
captured["params"]["isStatic"] is False to ensure the false-value serialization
path is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e496cd9-1751-44da-9f2e-f80103e3679d
📒 Files selected for processing (4)
MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.csServer/src/cli/commands/gameobject.pyServer/src/services/tools/manage_gameobject.pyServer/tests/integration/test_manage_gameobject_is_static.py
…ects targetGo.isStatic returns true when *any* flag is set, so a partially static object (e.g. only Navigation) would skip the update when is_static=true was requested. Compare GetStaticEditorFlags() against the desired flags instead. Also adds a "false" string coercion test per review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for this PR, very helpful! |
Summary
manage_gameobjectserializesisStaticin read output but themodifyaction has no parameter to write it. AI clients can see whether a GameObject is static, but can't change it.This adds
is_static(bool) to themodifyaction, closing the read/write gap.Motivation
Setting up scenes programmatically requires marking objects as static for lightmap baking, navigation, occlusion culling, and batching. Without this parameter, the only workaround is a custom Editor script invoked via
execute_menu_item.Usage
Omitting
is_staticleaves the flag unchanged (fully backwards compatible).Changes
GameObjectModify.cs):isStaticparameter handling after the existinglayerblockmanage_gameobject.py):is_staticparameter with bool coerciongameobject.py):--static/--no-staticflag onmodifytest_manage_gameobject_is_static.py): 4 integration testsTest plan
is_static=True/Falseforwarded correctly"true"→True)Documentation Updates
Summary by Sourcery
Add support for modifying a GameObject's static flag through the manage_gameobject tool, Unity handler, and CLI.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Tests