From 2401fde81f6a194f15e9cd7c4f19f360157263c4 Mon Sep 17 00:00:00 2001 From: David Sarno Date: Fri, 20 Feb 2026 19:45:05 -0800 Subject: [PATCH 1/3] feat: support setting component properties on prefab assets (#793) Add component_properties parameter to manage_prefabs modify_contents, enabling AI agents to set serialized fields on prefab components without opening the prefab editor. Reuses existing ComponentOps.SetProperty for reflection, SerializedProperty, and object reference resolution (by GUID, path, or instanceID). Co-Authored-By: Claude Opus 4.6 --- .../Editor/Tools/Prefabs/ManagePrefabs.cs | 44 ++++ Server/src/services/tools/manage_prefabs.py | 6 + Server/tests/test_manage_prefabs.py | 38 +++ Server/uv.lock | 2 +- .../EditMode/Tools/ManagePrefabsCrudTests.cs | 216 ++++++++++++++++++ .../UnityMCPTests/Packages/manifest.json | 2 +- 6 files changed, 306 insertions(+), 2 deletions(-) create mode 100644 Server/tests/test_manage_prefabs.py diff --git a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs index 6df9d47bf..2f580f348 100644 --- a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs +++ b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs @@ -758,6 +758,50 @@ private static (bool modified, ErrorResponse error) ApplyModificationsToPrefabOb } } + // Set properties on existing components + JObject componentProperties = @params["componentProperties"] as JObject ?? @params["component_properties"] as JObject; + if (componentProperties != null && componentProperties.Count > 0) + { + var errors = new List(); + + foreach (var entry in componentProperties.Properties()) + { + string typeName = entry.Name; + if (!ComponentResolver.TryResolve(typeName, out Type componentType, out string resolveError)) + { + return (false, new ErrorResponse($"Component type '{typeName}' not found: {resolveError}")); + } + + Component component = targetGo.GetComponent(componentType); + if (component == null) + { + return (false, new ErrorResponse($"Component '{typeName}' not found on '{targetGo.name}'.")); + } + + if (entry.Value is not JObject props || !props.HasValues) + { + continue; + } + + foreach (var prop in props.Properties()) + { + if (!ComponentOps.SetProperty(component, prop.Name, prop.Value, out string setError)) + { + errors.Add($"{typeName}.{prop.Name}: {setError}"); + } + else + { + modified = true; + } + } + } + + if (errors.Count > 0) + { + return (false, new ErrorResponse($"Failed to set some component properties: {string.Join("; ", errors)}")); + } + } + return (modified, null); } diff --git a/Server/src/services/tools/manage_prefabs.py b/Server/src/services/tools/manage_prefabs.py index 15df5f803..ac9866202 100644 --- a/Server/src/services/tools/manage_prefabs.py +++ b/Server/src/services/tools/manage_prefabs.py @@ -29,6 +29,9 @@ "(single object or array for batch creation in one save). " "Example: create_child=[{\"name\": \"Child1\", \"primitive_type\": \"Sphere\", \"position\": [1,0,0]}, " "{\"name\": \"Child2\", \"primitive_type\": \"Cube\", \"parent\": \"Child1\"}]. " + "Use component_properties with modify_contents to set serialized fields on existing components " + "(e.g. component_properties={\"Rigidbody\": {\"mass\": 5.0}, \"MyScript\": {\"health\": 100}}). " + "Supports object references via {\"guid\": \"...\"}, {\"path\": \"Assets/...\"}, or {\"instanceID\": 123}. " "Use manage_asset action=search filterType=Prefab to list prefabs." ), annotations=ToolAnnotations( @@ -64,6 +67,7 @@ async def manage_prefabs( components_to_add: Annotated[list[str], "Component types to add in modify_contents."] | None = None, components_to_remove: Annotated[list[str], "Component types to remove in modify_contents."] | None = None, create_child: Annotated[dict[str, Any] | list[dict[str, Any]], "Create child GameObject(s) in the prefab. Single object or array of objects, each with: name (required), parent (optional, defaults to target), primitive_type (optional: Cube, Sphere, Capsule, Cylinder, Plane, Quad), position, rotation, scale, components_to_add, tag, layer, set_active."] | None = None, + component_properties: Annotated[dict[str, dict[str, Any]], "Set properties on existing components in modify_contents. Keys are component type names, values are dicts of property name to value. Example: {\"Rigidbody\": {\"mass\": 5.0}, \"MyScript\": {\"health\": 100}}. Supports object references via {\"guid\": \"...\"}, {\"path\": \"Assets/...\"}, or {\"instanceID\": 123}."] | None = None, ) -> dict[str, Any]: # Back-compat: map 'name' → 'target' for create_from_gameobject (Unity accepts both) if action == "create_from_gameobject" and target is None and name is not None: @@ -148,6 +152,8 @@ async def manage_prefabs( params["componentsToAdd"] = components_to_add if components_to_remove is not None: params["componentsToRemove"] = components_to_remove + if component_properties is not None: + params["componentProperties"] = component_properties if create_child is not None: # Normalize vector fields within create_child (handles single object or array) def normalize_child_params(child: Any, index: int | None = None) -> tuple[dict | None, str | None]: diff --git a/Server/tests/test_manage_prefabs.py b/Server/tests/test_manage_prefabs.py new file mode 100644 index 000000000..e2305262a --- /dev/null +++ b/Server/tests/test_manage_prefabs.py @@ -0,0 +1,38 @@ +"""Tests for manage_prefabs tool - component_properties parameter.""" + +import inspect + +from services.tools.manage_prefabs import manage_prefabs + + +class TestManagePrefabsComponentProperties: + """Tests for the component_properties parameter on manage_prefabs.""" + + def test_component_properties_parameter_exists(self): + """The manage_prefabs tool should have a component_properties parameter.""" + sig = inspect.signature(manage_prefabs) + assert "component_properties" in sig.parameters + + def test_component_properties_parameter_is_optional(self): + """component_properties should default to None.""" + sig = inspect.signature(manage_prefabs) + param = sig.parameters["component_properties"] + assert param.default is None + + def test_tool_description_mentions_component_properties(self): + """The tool description should mention component_properties.""" + from services.registry import get_registered_tools + tools = get_registered_tools() + prefab_tool = next( + (t for t in tools if t["name"] == "manage_prefabs"), None + ) + assert prefab_tool is not None + # Description is stored at top level or in kwargs depending on how the decorator stores it + desc = prefab_tool.get("description") or prefab_tool["kwargs"].get("description", "") + assert "component_properties" in desc + + def test_required_params_include_modify_contents(self): + """modify_contents should be a valid action requiring prefab_path.""" + from services.tools.manage_prefabs import REQUIRED_PARAMS + assert "modify_contents" in REQUIRED_PARAMS + assert "prefab_path" in REQUIRED_PARAMS["modify_contents"] diff --git a/Server/uv.lock b/Server/uv.lock index bb38dc952..46e8a45a7 100644 --- a/Server/uv.lock +++ b/Server/uv.lock @@ -912,7 +912,7 @@ wheels = [ [[package]] name = "mcpforunityserver" -version = "9.4.0" +version = "9.4.6" source = { editable = "." } dependencies = [ { name = "click" }, diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs index b4949224d..365ad306a 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs @@ -722,6 +722,204 @@ public void ModifyContents_CreateChild_ReturnsErrorForInvalidInput() #endregion + #region Component Properties Tests + + [Test] + public void ModifyContents_ComponentProperties_SetsSimpleProperties() + { + string prefabPath = CreatePrefabWithComponents("CompPropSimple", typeof(Rigidbody)); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject + { + ["mass"] = 42f, + ["useGravity"] = false + } + } + })); + + Assert.IsTrue(result.Value("success"), $"Expected success but got: {result}"); + Assert.IsTrue(result["data"].Value("modified")); + + // Verify changes persisted + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + var rb = reloaded.GetComponent(); + Assert.IsNotNull(rb); + Assert.AreEqual(42f, rb.mass, 0.01f); + Assert.IsFalse(rb.useGravity); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + [Test] + public void ModifyContents_ComponentProperties_SetsMultipleComponents() + { + string prefabPath = CreatePrefabWithComponents("CompPropMulti", typeof(Rigidbody), typeof(Light)); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject { ["mass"] = 10f }, + ["Light"] = new JObject { ["intensity"] = 3.5f } + } + })); + + Assert.IsTrue(result.Value("success"), $"Expected success but got: {result}"); + + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + Assert.AreEqual(10f, reloaded.GetComponent().mass, 0.01f); + Assert.AreEqual(3.5f, reloaded.GetComponent().intensity, 0.01f); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + [Test] + public void ModifyContents_ComponentProperties_SetsOnChildTarget() + { + // Create a prefab with a child that has a Rigidbody + EnsureFolder(TempDirectory); + GameObject root = new GameObject("ChildTargetTest"); + GameObject child = new GameObject("Child1") { transform = { parent = root.transform } }; + child.AddComponent(); + + string prefabPath = Path.Combine(TempDirectory, "ChildTargetTest.prefab").Replace('\\', '/'); + PrefabUtility.SaveAsPrefabAsset(root, prefabPath, out bool success); + UnityEngine.Object.DestroyImmediate(root); + AssetDatabase.Refresh(); + Assert.IsTrue(success); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child1", + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject { ["mass"] = 99f, ["drag"] = 2.5f } + } + })); + + Assert.IsTrue(result.Value("success"), $"Expected success but got: {result}"); + + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + var childRb = reloaded.transform.Find("Child1").GetComponent(); + Assert.AreEqual(99f, childRb.mass, 0.01f); + Assert.AreEqual(2.5f, childRb.drag, 0.01f); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + [Test] + public void ModifyContents_ComponentProperties_ReturnsErrorForMissingComponent() + { + string prefabPath = CreateTestPrefab("CompPropMissing"); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject { ["mass"] = 5f } + } + })); + + Assert.IsFalse(result.Value("success")); + Assert.IsTrue(result.Value("error").Contains("not found"), + $"Expected 'not found' error but got: {result.Value("error")}"); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + [Test] + public void ModifyContents_ComponentProperties_ReturnsErrorForInvalidType() + { + string prefabPath = CreateTestPrefab("CompPropInvalidType"); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentProperties"] = new JObject + { + ["NonexistentComponent"] = new JObject { ["foo"] = "bar" } + } + })); + + Assert.IsFalse(result.Value("success")); + Assert.IsTrue(result.Value("error").Contains("not found"), + $"Expected 'not found' error but got: {result.Value("error")}"); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + [Test] + public void ModifyContents_ComponentProperties_CombinesWithOtherModifications() + { + string prefabPath = CreatePrefabWithComponents("CompPropCombined", typeof(Rigidbody)); + + try + { + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["position"] = new JArray(5f, 10f, 15f), + ["name"] = "RenamedWithProps", + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject { ["mass"] = 25f } + } + })); + + Assert.IsTrue(result.Value("success"), $"Expected success but got: {result}"); + + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + Assert.AreEqual("RenamedWithProps", reloaded.name); + Assert.AreEqual(new Vector3(5f, 10f, 15f), reloaded.transform.localPosition); + Assert.AreEqual(25f, reloaded.GetComponent().mass, 0.01f); + } + finally + { + SafeDeleteAsset(prefabPath); + } + } + + #endregion + #region Error Handling [Test] @@ -824,6 +1022,24 @@ private static string CreateNestedTestPrefab(string name) return path; } + private static string CreatePrefabWithComponents(string name, params Type[] componentTypes) + { + EnsureFolder(TempDirectory); + GameObject temp = new GameObject(name); + foreach (var t in componentTypes) + { + temp.AddComponent(t); + } + + string path = Path.Combine(TempDirectory, name + ".prefab").Replace('\\', '/'); + PrefabUtility.SaveAsPrefabAsset(temp, path, out bool success); + UnityEngine.Object.DestroyImmediate(temp); + AssetDatabase.Refresh(); + + if (!success) throw new Exception($"Failed to create test prefab at {path}"); + return path; + } + private static string CreateComplexTestPrefab(string name) { // Creates: Vehicle (root with BoxCollider) diff --git a/TestProjects/UnityMCPTests/Packages/manifest.json b/TestProjects/UnityMCPTests/Packages/manifest.json index 0bd78a670..f7361652a 100644 --- a/TestProjects/UnityMCPTests/Packages/manifest.json +++ b/TestProjects/UnityMCPTests/Packages/manifest.json @@ -1,6 +1,6 @@ { "dependencies": { - "com.coplaydev.unity-mcp": "file:../../../MCPForUnity", + "com.coplaydev.unity-mcp": "file:/Users/davidsarno/unity-mcp/MCPForUnity", "com.unity.ai.navigation": "1.1.4", "com.unity.collab-proxy": "2.5.2", "com.unity.feature.development": "1.0.1", From 0fd392fea12242d7fa778a2123a4a5568c32f33d Mon Sep 17 00:00:00 2001 From: David Sarno Date: Fri, 20 Feb 2026 19:51:13 -0800 Subject: [PATCH 2/3] fix: revert manifest.json path and aggregate componentProperties errors - Revert manifest.json from absolute path back to relative file:../../../MCPForUnity - Change componentProperties error handling to aggregate all errors (missing types, missing components, property failures) instead of short-circuiting on the first component-level error Addresses PR #802 review feedback from Sourcery. Co-Authored-By: Claude Opus 4.6 --- MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs | 6 ++++-- TestProjects/UnityMCPTests/Packages/manifest.json | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs index 2f580f348..05fe6d4e5 100644 --- a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs +++ b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs @@ -769,13 +769,15 @@ private static (bool modified, ErrorResponse error) ApplyModificationsToPrefabOb string typeName = entry.Name; if (!ComponentResolver.TryResolve(typeName, out Type componentType, out string resolveError)) { - return (false, new ErrorResponse($"Component type '{typeName}' not found: {resolveError}")); + errors.Add($"{typeName}: type not found — {resolveError}"); + continue; } Component component = targetGo.GetComponent(componentType); if (component == null) { - return (false, new ErrorResponse($"Component '{typeName}' not found on '{targetGo.name}'.")); + errors.Add($"{typeName}: not found on '{targetGo.name}'"); + continue; } if (entry.Value is not JObject props || !props.HasValues) diff --git a/TestProjects/UnityMCPTests/Packages/manifest.json b/TestProjects/UnityMCPTests/Packages/manifest.json index f7361652a..0bd78a670 100644 --- a/TestProjects/UnityMCPTests/Packages/manifest.json +++ b/TestProjects/UnityMCPTests/Packages/manifest.json @@ -1,6 +1,6 @@ { "dependencies": { - "com.coplaydev.unity-mcp": "file:/Users/davidsarno/unity-mcp/MCPForUnity", + "com.coplaydev.unity-mcp": "file:../../../MCPForUnity", "com.unity.ai.navigation": "1.1.4", "com.unity.collab-proxy": "2.5.2", "com.unity.feature.development": "1.0.1", From bd78328e045a2aed1e46b377ebc0012c5d5da7c8 Mon Sep 17 00:00:00 2001 From: David Sarno Date: Fri, 20 Feb 2026 19:53:30 -0800 Subject: [PATCH 3/3] fix: clarify error message and guard against KeyError in test - Error message now says "no changes saved" instead of misleading "some" - Use .get("kwargs", {}) to avoid KeyError if registry entry lacks kwargs Addresses CodeRabbit review feedback on PR #802. Co-Authored-By: Claude Opus 4.6 --- MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs | 2 +- Server/tests/test_manage_prefabs.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs index 05fe6d4e5..10ebf1677 100644 --- a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs +++ b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs @@ -800,7 +800,7 @@ private static (bool modified, ErrorResponse error) ApplyModificationsToPrefabOb if (errors.Count > 0) { - return (false, new ErrorResponse($"Failed to set some component properties: {string.Join("; ", errors)}")); + return (false, new ErrorResponse($"Failed to set component properties (no changes saved): {string.Join("; ", errors)}")); } } diff --git a/Server/tests/test_manage_prefabs.py b/Server/tests/test_manage_prefabs.py index e2305262a..be441581c 100644 --- a/Server/tests/test_manage_prefabs.py +++ b/Server/tests/test_manage_prefabs.py @@ -28,7 +28,7 @@ def test_tool_description_mentions_component_properties(self): ) assert prefab_tool is not None # Description is stored at top level or in kwargs depending on how the decorator stores it - desc = prefab_tool.get("description") or prefab_tool["kwargs"].get("description", "") + desc = prefab_tool.get("description") or prefab_tool.get("kwargs", {}).get("description", "") assert "component_properties" in desc def test_required_params_include_modify_contents(self):