diff --git a/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs b/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs deleted file mode 100644 index ee47d6f59..000000000 --- a/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs +++ /dev/null @@ -1,42 +0,0 @@ -using System; -using MCPForUnity.Editor.Helpers; -using Newtonsoft.Json.Linq; -using UnityEditor.SceneManagement; - -namespace MCPForUnity.Editor.Resources.Editor -{ - /// - /// Provides information about the current prefab editing context. - /// - [McpForUnityResource("get_prefab_stage")] - public static class PrefabStage - { - public static object HandleCommand(JObject @params) - { - try - { - var stage = PrefabStageUtility.GetCurrentPrefabStage(); - - if (stage == null) - { - return new SuccessResponse("No prefab stage is currently open.", new { isOpen = false }); - } - - var stageInfo = new - { - isOpen = true, - assetPath = stage.assetPath, - prefabRootName = stage.prefabContentsRoot?.name, - mode = stage.mode.ToString(), - isDirty = stage.scene.isDirty - }; - - return new SuccessResponse("Prefab stage info retrieved.", stageInfo); - } - catch (Exception e) - { - return new ErrorResponse($"Error getting prefab stage info: {e.Message}"); - } - } - } -} diff --git a/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs.meta b/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs.meta deleted file mode 100644 index 31bc264cb..000000000 --- a/MCPForUnity/Editor/Resources/Editor/PrefabStage.cs.meta +++ /dev/null @@ -1,11 +0,0 @@ -fileFormatVersion: 2 -guid: 7a30b083e68bd4ae3b3d1ce5a45a9414 -MonoImporter: - externalObjects: {} - serializedVersion: 2 - defaultReferences: [] - executionOrder: 0 - icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: diff --git a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs index 4223a2561..9b4df7be8 100644 --- a/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs +++ b/MCPForUnity/Editor/Tools/Prefabs/ManagePrefabs.cs @@ -12,18 +12,17 @@ namespace MCPForUnity.Editor.Tools.Prefabs { [McpForUnityTool("manage_prefabs", AutoRegister = false)] /// - /// Tool to manage Unity Prefab stages and create prefabs from GameObjects. + /// Tool to manage Unity Prefabs: create, inspect, and modify prefab assets. + /// Uses headless editing (no UI, no dialogs) for reliable automated workflows. /// public static class ManagePrefabs { // Action constants - private const string ACTION_OPEN_STAGE = "open_stage"; - private const string ACTION_CLOSE_STAGE = "close_stage"; - private const string ACTION_SAVE_OPEN_STAGE = "save_open_stage"; private const string ACTION_CREATE_FROM_GAMEOBJECT = "create_from_gameobject"; private const string ACTION_GET_INFO = "get_info"; private const string ACTION_GET_HIERARCHY = "get_hierarchy"; - private const string SupportedActions = ACTION_OPEN_STAGE + ", " + ACTION_CLOSE_STAGE + ", " + ACTION_SAVE_OPEN_STAGE + ", " + ACTION_CREATE_FROM_GAMEOBJECT + ", " + ACTION_GET_INFO + ", " + ACTION_GET_HIERARCHY; + private const string ACTION_MODIFY_CONTENTS = "modify_contents"; + private const string SupportedActions = ACTION_CREATE_FROM_GAMEOBJECT + ", " + ACTION_GET_INFO + ", " + ACTION_GET_HIERARCHY + ", " + ACTION_MODIFY_CONTENTS; public static object HandleCommand(JObject @params) { @@ -42,18 +41,14 @@ public static object HandleCommand(JObject @params) { switch (action) { - case ACTION_OPEN_STAGE: - return OpenStage(@params); - case ACTION_CLOSE_STAGE: - return CloseStage(@params); - case ACTION_SAVE_OPEN_STAGE: - return SaveOpenStage(@params); case ACTION_CREATE_FROM_GAMEOBJECT: return CreatePrefabFromGameObject(@params); case ACTION_GET_INFO: return GetInfo(@params); case ACTION_GET_HIERARCHY: return GetHierarchy(@params); + case ACTION_MODIFY_CONTENTS: + return ModifyContents(@params); default: return new ErrorResponse($"Unknown action: '{action}'. Valid actions are: {SupportedActions}."); } @@ -65,192 +60,6 @@ public static object HandleCommand(JObject @params) } } - /// - /// Opens a prefab in prefab mode for editing. - /// - private static object OpenStage(JObject @params) - { - string prefabPath = @params["prefabPath"]?.ToString(); - if (string.IsNullOrEmpty(prefabPath)) - { - return new ErrorResponse("'prefabPath' parameter is required for open_stage."); - } - - string sanitizedPath = AssetPathUtility.SanitizeAssetPath(prefabPath); - if (string.IsNullOrEmpty(sanitizedPath)) - { - return new ErrorResponse($"Invalid prefab path: '{prefabPath}'."); - } - GameObject prefabAsset = AssetDatabase.LoadAssetAtPath(sanitizedPath); - if (prefabAsset == null) - { - return new ErrorResponse($"No prefab asset found at path '{sanitizedPath}'."); - } - - PrefabStage stage = PrefabStageUtility.OpenPrefab(sanitizedPath); - if (stage == null) - { - return new ErrorResponse($"Failed to open prefab stage for '{sanitizedPath}'."); - } - - return new SuccessResponse($"Opened prefab stage for '{sanitizedPath}'.", SerializeStage(stage)); - } - - /// - /// Closes the currently open prefab stage, optionally saving first. - /// - private static object CloseStage(JObject @params) - { - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - if (stage == null) - { - return new SuccessResponse("No prefab stage was open."); - } - - string assetPath = stage.assetPath; - bool saveBeforeClose = @params["saveBeforeClose"]?.ToObject() ?? false; - - if (saveBeforeClose && stage.scene.isDirty) - { - try - { - SaveAndRefreshStage(stage); - } - catch (Exception e) - { - return new ErrorResponse($"Failed to save prefab before closing: {e.Message}"); - } - } - - StageUtility.GoToMainStage(); - return new SuccessResponse($"Closed prefab stage for '{assetPath}'."); - } - - /// - /// Saves changes to the currently open prefab stage. - /// Supports a 'force' parameter for automated workflows where isDirty may not be set. - /// - private static object SaveOpenStage(JObject @params) - { - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - if (stage == null) - { - return new ErrorResponse("No prefab stage is currently open."); - } - - if (!ValidatePrefabStageForSave(stage)) - { - return new ErrorResponse("Prefab stage validation failed. Cannot save."); - } - - // Check for force parameter (useful for automated workflows) - bool force = @params?["force"]?.ToObject() ?? false; - - // Check if there are actual changes to save - bool wasDirty = stage.scene.isDirty; - if (!wasDirty && !force) - { - return new SuccessResponse($"Prefab stage for '{stage.assetPath}' has no unsaved changes.", SerializeStage(stage)); - } - - try - { - SaveAndRefreshStage(stage, force); - return new SuccessResponse($"Saved prefab stage for '{stage.assetPath}'.", SerializeStage(stage)); - } - catch (Exception e) - { - return new ErrorResponse($"Failed to save prefab: {e.Message}"); - } - } - - #region Prefab Save Operations - - /// - /// Saves the prefab stage and refreshes the asset database. - /// Uses PrefabUtility.SaveAsPrefabAsset for reliable prefab saving without dialogs. - /// - /// The prefab stage to save. - /// If true, marks the prefab dirty before saving to ensure changes are captured. - private static void SaveAndRefreshStage(PrefabStage stage, bool force = false) - { - if (stage == null) - { - throw new ArgumentNullException(nameof(stage), "Prefab stage cannot be null."); - } - - if (stage.prefabContentsRoot == null) - { - throw new InvalidOperationException("Cannot save prefab stage without a prefab root."); - } - - if (string.IsNullOrEmpty(stage.assetPath)) - { - throw new InvalidOperationException("Prefab stage has invalid asset path."); - } - - // When force=true, mark the prefab root dirty to ensure changes are saved - // This is useful for automated workflows where isDirty may not be set correctly - if (force) - { - EditorUtility.SetDirty(stage.prefabContentsRoot); - EditorSceneManager.MarkSceneDirty(stage.scene); - } - - // Mark all children as dirty to ensure their changes are captured - foreach (Transform child in stage.prefabContentsRoot.GetComponentsInChildren(true)) - { - if (child != stage.prefabContentsRoot.transform) - { - EditorUtility.SetDirty(child.gameObject); - } - } - - // Use PrefabUtility.SaveAsPrefabAsset which saves without dialogs - // This is more reliable for automated workflows than EditorSceneManager.SaveScene - bool success; - PrefabUtility.SaveAsPrefabAsset(stage.prefabContentsRoot, stage.assetPath, out success); - - if (!success) - { - throw new InvalidOperationException($"Failed to save prefab asset for '{stage.assetPath}'."); - } - - // Ensure changes are persisted to disk - AssetDatabase.SaveAssets(); - AssetDatabase.Refresh(); - - McpLog.Info($"[ManagePrefabs] Successfully saved prefab '{stage.assetPath}'."); - } - - /// - /// Validates prefab stage before saving. - /// - private static bool ValidatePrefabStageForSave(PrefabStage stage) - { - if (stage == null) - { - McpLog.Warn("[ManagePrefabs] No prefab stage is open."); - return false; - } - - if (stage.prefabContentsRoot == null) - { - McpLog.Error($"[ManagePrefabs] Prefab stage '{stage.assetPath}' has no root object."); - return false; - } - - if (string.IsNullOrEmpty(stage.assetPath)) - { - McpLog.Error("[ManagePrefabs] Prefab stage has invalid asset path."); - return false; - } - - return true; - } - - #endregion - #region Create Prefab from GameObject /// @@ -618,6 +427,307 @@ private static object GetHierarchy(JObject @params) #endregion + #region Headless Prefab Editing + + /// + /// Modifies a prefab's contents directly without opening the prefab stage. + /// This is ideal for automated/agentic workflows as it avoids UI, dirty flags, and dialogs. + /// + private static object ModifyContents(JObject @params) + { + string prefabPath = @params["prefabPath"]?.ToString() ?? @params["path"]?.ToString(); + if (string.IsNullOrEmpty(prefabPath)) + { + return new ErrorResponse("'prefabPath' parameter is required for modify_contents."); + } + + string sanitizedPath = AssetPathUtility.SanitizeAssetPath(prefabPath); + if (string.IsNullOrEmpty(sanitizedPath)) + { + return new ErrorResponse($"Invalid prefab path '{prefabPath}'. Path traversal sequences are not allowed."); + } + + // Load prefab contents in isolated context (no UI) + GameObject prefabContents = PrefabUtility.LoadPrefabContents(sanitizedPath); + if (prefabContents == null) + { + return new ErrorResponse($"Failed to load prefab contents from '{sanitizedPath}'."); + } + + try + { + // Find target object within the prefab (defaults to root) + string targetName = @params["target"]?.ToString(); + GameObject targetGo = FindInPrefabContents(prefabContents, targetName); + + if (targetGo == null) + { + string searchedFor = string.IsNullOrEmpty(targetName) ? "root" : $"'{targetName}'"; + return new ErrorResponse($"Target {searchedFor} not found in prefab '{sanitizedPath}'."); + } + + // Apply modifications + var modifyResult = ApplyModificationsToPrefabObject(targetGo, @params, prefabContents); + if (modifyResult.error != null) + { + return modifyResult.error; + } + + // Skip saving when no modifications were made to avoid unnecessary asset writes + if (!modifyResult.modified) + { + return new SuccessResponse( + $"Prefab '{sanitizedPath}' is already up to date; no changes were applied.", + new + { + prefabPath = sanitizedPath, + targetName = targetGo.name, + modified = false + } + ); + } + + // Save the prefab + bool success; + PrefabUtility.SaveAsPrefabAsset(prefabContents, sanitizedPath, out success); + + if (!success) + { + return new ErrorResponse($"Failed to save prefab asset at '{sanitizedPath}'."); + } + + AssetDatabase.Refresh(); + + McpLog.Info($"[ManagePrefabs] Successfully modified and saved prefab '{sanitizedPath}' (headless)."); + + return new SuccessResponse( + $"Prefab '{sanitizedPath}' modified and saved successfully.", + new + { + prefabPath = sanitizedPath, + targetName = targetGo.name, + modified = modifyResult.modified, + transform = new + { + position = new { x = targetGo.transform.localPosition.x, y = targetGo.transform.localPosition.y, z = targetGo.transform.localPosition.z }, + rotation = new { x = targetGo.transform.localEulerAngles.x, y = targetGo.transform.localEulerAngles.y, z = targetGo.transform.localEulerAngles.z }, + scale = new { x = targetGo.transform.localScale.x, y = targetGo.transform.localScale.y, z = targetGo.transform.localScale.z } + }, + componentTypes = PrefabUtilityHelper.GetComponentTypeNames(targetGo) + } + ); + } + finally + { + // Always unload prefab contents to free memory + PrefabUtility.UnloadPrefabContents(prefabContents); + } + } + + /// + /// Finds a GameObject within loaded prefab contents by name or path. + /// + private static GameObject FindInPrefabContents(GameObject prefabContents, string target) + { + if (string.IsNullOrEmpty(target)) + { + // Return root if no target specified + return prefabContents; + } + + // Try to find by path first (e.g., "Parent/Child/Target") + if (target.Contains("/")) + { + Transform found = prefabContents.transform.Find(target); + if (found != null) + { + return found.gameObject; + } + + // If path starts with root name, try without it + if (target.StartsWith(prefabContents.name + "/")) + { + string relativePath = target.Substring(prefabContents.name.Length + 1); + found = prefabContents.transform.Find(relativePath); + if (found != null) + { + return found.gameObject; + } + } + } + + // Check if target matches root name + if (prefabContents.name == target) + { + return prefabContents; + } + + // Search by name in hierarchy + foreach (Transform t in prefabContents.GetComponentsInChildren(true)) + { + if (t.gameObject.name == target) + { + return t.gameObject; + } + } + + return null; + } + + /// + /// Applies modifications to a GameObject within loaded prefab contents. + /// Returns (modified: bool, error: ErrorResponse or null). + /// + private static (bool modified, ErrorResponse error) ApplyModificationsToPrefabObject(GameObject targetGo, JObject @params, GameObject prefabRoot) + { + bool modified = false; + + // Name change + string newName = @params["name"]?.ToString(); + if (!string.IsNullOrEmpty(newName) && targetGo.name != newName) + { + // If renaming the root, this will affect the prefab asset name on save + targetGo.name = newName; + modified = true; + } + + // Active state + bool? setActive = @params["setActive"]?.ToObject(); + if (setActive.HasValue && targetGo.activeSelf != setActive.Value) + { + targetGo.SetActive(setActive.Value); + modified = true; + } + + // Tag + string tag = @params["tag"]?.ToString(); + if (tag != null && targetGo.tag != tag) + { + string tagToSet = string.IsNullOrEmpty(tag) ? "Untagged" : tag; + try + { + targetGo.tag = tagToSet; + modified = true; + } + catch (Exception ex) + { + return (false, new ErrorResponse($"Failed to set tag to '{tagToSet}': {ex.Message}")); + } + } + + // Layer + string layerName = @params["layer"]?.ToString(); + if (!string.IsNullOrEmpty(layerName)) + { + int layerId = LayerMask.NameToLayer(layerName); + if (layerId == -1) + { + return (false, new ErrorResponse($"Invalid layer specified: '{layerName}'. Use a valid layer name.")); + } + if (targetGo.layer != layerId) + { + targetGo.layer = layerId; + modified = true; + } + } + + // Transform: position, rotation, scale + Vector3? position = VectorParsing.ParseVector3(@params["position"]); + Vector3? rotation = VectorParsing.ParseVector3(@params["rotation"]); + Vector3? scale = VectorParsing.ParseVector3(@params["scale"]); + + if (position.HasValue && targetGo.transform.localPosition != position.Value) + { + targetGo.transform.localPosition = position.Value; + modified = true; + } + if (rotation.HasValue && targetGo.transform.localEulerAngles != rotation.Value) + { + targetGo.transform.localEulerAngles = rotation.Value; + modified = true; + } + if (scale.HasValue && targetGo.transform.localScale != scale.Value) + { + targetGo.transform.localScale = scale.Value; + modified = true; + } + + // Parent change (within prefab hierarchy) + JToken parentToken = @params["parent"]; + if (parentToken != null) + { + string parentTarget = parentToken.ToString(); + Transform newParent = null; + + if (!string.IsNullOrEmpty(parentTarget)) + { + GameObject parentGo = FindInPrefabContents(prefabRoot, parentTarget); + if (parentGo == null) + { + return (false, new ErrorResponse($"Parent '{parentTarget}' not found in prefab.")); + } + if (parentGo.transform.IsChildOf(targetGo.transform)) + { + return (false, new ErrorResponse($"Cannot parent '{targetGo.name}' to '{parentGo.name}' as it would create a hierarchy loop.")); + } + newParent = parentGo.transform; + } + + if (targetGo.transform.parent != newParent) + { + targetGo.transform.SetParent(newParent, true); + modified = true; + } + } + + // Components to add + if (@params["componentsToAdd"] is JArray componentsToAdd) + { + foreach (var compToken in componentsToAdd) + { + string typeName = compToken.Type == JTokenType.String + ? compToken.ToString() + : (compToken as JObject)?["typeName"]?.ToString(); + + if (!string.IsNullOrEmpty(typeName)) + { + if (!ComponentResolver.TryResolve(typeName, out Type componentType, out string error)) + { + return (false, new ErrorResponse($"Component type '{typeName}' not found: {error}")); + } + targetGo.AddComponent(componentType); + modified = true; + } + } + } + + // Components to remove + if (@params["componentsToRemove"] is JArray componentsToRemove) + { + foreach (var compToken in componentsToRemove) + { + string typeName = compToken.ToString(); + if (!string.IsNullOrEmpty(typeName)) + { + if (!ComponentResolver.TryResolve(typeName, out Type componentType, out string error)) + { + return (false, new ErrorResponse($"Component type '{typeName}' not found: {error}")); + } + Component comp = targetGo.GetComponent(componentType); + if (comp != null) + { + UnityEngine.Object.DestroyImmediate(comp); + modified = true; + } + } + } + } + + return (modified, null); + } + + #endregion + #region Hierarchy Builder /// @@ -689,25 +799,5 @@ private static void BuildHierarchyItemsRecursive(Transform transform, Transform } #endregion - - /// - /// Serializes the prefab stage information for response. - /// - private static object SerializeStage(PrefabStage stage) - { - if (stage == null) - { - return new { isOpen = false }; - } - - return new - { - isOpen = true, - assetPath = stage.assetPath, - prefabRootName = stage.prefabContentsRoot != null ? stage.prefabContentsRoot.name : null, - mode = stage.mode.ToString(), - isDirty = stage.scene.isDirty - }; - } } } diff --git a/Server/src/services/tools/manage_prefabs.py b/Server/src/services/tools/manage_prefabs.py index 38d28322c..cca195088 100644 --- a/Server/src/services/tools/manage_prefabs.py +++ b/Server/src/services/tools/manage_prefabs.py @@ -15,17 +15,16 @@ REQUIRED_PARAMS = { "get_info": ["prefab_path"], "get_hierarchy": ["prefab_path"], - "open_stage": ["prefab_path"], "create_from_gameobject": ["target", "prefab_path"], - "save_open_stage": [], - "close_stage": [], + "modify_contents": ["prefab_path"], } @mcp_for_unity_tool( description=( - "Manages Unity Prefab assets and stages. " - "Actions: get_info, get_hierarchy, open_stage, close_stage, save_open_stage, create_from_gameobject. " + "Manages Unity Prefab assets via headless operations (no UI, no prefab stages). " + "Actions: get_info, get_hierarchy, create_from_gameobject, modify_contents. " + "Use modify_contents for headless prefab editing - ideal for automated workflows. " "Use manage_asset action=search filterType=Prefab to list prefabs." ), annotations=ToolAnnotations( @@ -37,27 +36,39 @@ async def manage_prefabs( ctx: Context, action: Annotated[ Literal[ - "open_stage", - "close_stage", - "save_open_stage", "create_from_gameobject", "get_info", "get_hierarchy", + "modify_contents", ], "Prefab operation to perform.", ], prefab_path: Annotated[str, "Prefab asset path (e.g., Assets/Prefabs/MyPrefab.prefab)."] | None = None, - save_before_close: Annotated[bool, "Save before closing if unsaved changes exist."] | None = None, - target: Annotated[str, "Scene GameObject name for create_from_gameobject."] | None = None, + target: Annotated[str, "Target GameObject: scene object for create_from_gameobject, or object within prefab for modify_contents (name or path like 'Parent/Child')."] | None = None, allow_overwrite: Annotated[bool, "Allow replacing existing prefab."] | None = None, search_inactive: Annotated[bool, "Include inactive GameObjects in search."] | None = None, unlink_if_instance: Annotated[bool, "Unlink from existing prefab before creating new one."] | None = None, - force: Annotated[bool, "Force save even if no changes detected. Useful for automated workflows."] | None = None, + # modify_contents parameters + position: Annotated[list[float], "New local position [x, y, z] for modify_contents."] | None = None, + rotation: Annotated[list[float], "New local rotation (euler angles) [x, y, z] for modify_contents."] | None = None, + scale: Annotated[list[float], "New local scale [x, y, z] for modify_contents."] | None = None, + name: Annotated[str, "New name for the target object in modify_contents."] | None = None, + tag: Annotated[str, "New tag for the target object in modify_contents."] | None = None, + layer: Annotated[str, "New layer name for the target object in modify_contents."] | None = None, + set_active: Annotated[bool, "Set active state of target object in modify_contents."] | None = None, + parent: Annotated[str, "New parent object name/path within prefab for modify_contents."] | None = None, + 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, ) -> 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: + target = name + # Validate required parameters required = REQUIRED_PARAMS.get(action, []) for param_name in required: - param_value = locals().get(param_name) + # Use updated local value for target after back-compat mapping + param_value = target if param_name == "target" else locals().get(param_name) # Check for None and empty/whitespace strings if param_value is None or (isinstance(param_value, str) and not param_value.strip()): return { @@ -86,11 +97,6 @@ async def manage_prefabs( if prefab_path: params["prefabPath"] = prefab_path - # Handle boolean parameters with proper coercion - save_before_close_val = coerce_bool(save_before_close) - if save_before_close_val is not None: - params["saveBeforeClose"] = save_before_close_val - if target: params["target"] = target @@ -106,9 +112,28 @@ async def manage_prefabs( if unlink_if_instance_val is not None: params["unlinkIfInstance"] = unlink_if_instance_val - force_val = coerce_bool(force) - if force_val is not None: - params["force"] = force_val + # modify_contents parameters + if position is not None: + params["position"] = position + if rotation is not None: + params["rotation"] = rotation + if scale is not None: + params["scale"] = scale + if name is not None: + params["name"] = name + if tag is not None: + params["tag"] = tag + if layer is not None: + params["layer"] = layer + set_active_val = coerce_bool(set_active) + if set_active_val is not None: + params["setActive"] = set_active_val + if parent is not None: + params["parent"] = parent + if components_to_add is not None: + params["componentsToAdd"] = components_to_add + if components_to_remove is not None: + params["componentsToRemove"] = components_to_remove # Send command to Unity response = await send_with_unity_instance( @@ -137,4 +162,4 @@ async def manage_prefabs( return { "success": False, "message": f"Error managing prefabs: {exc}" - } \ No newline at end of file + } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs index 3323632f3..da6c5063d 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs @@ -14,9 +14,7 @@ namespace MCPForUnityTests.Editor.Tools { /// - /// Comprehensive test suite for Prefab CRUD operations and new features. - /// Tests cover: Create, Read, Update, Delete patterns, force save, unlink-if-instance, - /// overwrite handling, inactive object search, and save dialog prevention. + /// Tests for Prefab CRUD operations: create_from_gameobject, get_info, get_hierarchy, modify_contents. /// public class ManagePrefabsCrudTests { @@ -45,7 +43,7 @@ public void TearDown() #region CREATE Tests [Test] - public void CreateFromGameObject_CreatesNewPrefab_WithValidParameters() + public void CreateFromGameObject_CreatesNewPrefab() { string prefabPath = Path.Combine(TempDirectory, "NewPrefab.prefab").Replace('\\', '/'); GameObject sceneObject = new GameObject("TestObject"); @@ -59,12 +57,9 @@ public void CreateFromGameObject_CreatesNewPrefab_WithValidParameters() ["prefabPath"] = prefabPath })); - Assert.IsTrue(result.Value("success"), "create_from_gameobject should succeed."); - var data = result["data"] as JObject; - Assert.AreEqual(prefabPath, data.Value("prefabPath")); - - GameObject prefabAsset = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.IsNotNull(prefabAsset, "Prefab asset should exist at path."); + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(prefabPath, result["data"].Value("prefabPath")); + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(prefabPath)); } finally { @@ -74,176 +69,71 @@ public void CreateFromGameObject_CreatesNewPrefab_WithValidParameters() } [Test] - public void CreateFromGameObject_UnlinksInstance_WhenUnlinkIfInstanceIsTrue() + public void CreateFromGameObject_HandlesExistingPrefabsAndLinks() { - // Create an initial prefab - string initialPrefabPath = Path.Combine(TempDirectory, "Original.prefab").Replace('\\', '/'); + // Tests: unlinkIfInstance, allowOverwrite, unique path generation + string prefabPath = Path.Combine(TempDirectory, "Existing.prefab").Replace('\\', '/'); GameObject sourceObject = new GameObject("SourceObject"); - GameObject instance = null; try { - // Create initial prefab and connect source object to it - PrefabUtility.SaveAsPrefabAssetAndConnect( - sourceObject, initialPrefabPath, InteractionMode.AutomatedAction); - - // Verify source object is now linked - Assert.IsTrue(PrefabUtility.IsAnyPrefabInstanceRoot(sourceObject), - "Source object should be linked to prefab after SaveAsPrefabAssetAndConnect."); + // Create initial prefab and link source object + PrefabUtility.SaveAsPrefabAssetAndConnect(sourceObject, prefabPath, InteractionMode.AutomatedAction); + Assert.IsTrue(PrefabUtility.IsAnyPrefabInstanceRoot(sourceObject)); - // Create new prefab with unlinkIfInstance - // The command will find sourceObject by name and unlink it - string newPrefabPath = Path.Combine(TempDirectory, "NewFromLinked.prefab").Replace('\\', '/'); - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Without unlink - should fail (already linked) + string newPath = Path.Combine(TempDirectory, "New.prefab").Replace('\\', '/'); + var failResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", ["target"] = sourceObject.name, - ["prefabPath"] = newPrefabPath, - ["unlinkIfInstance"] = true + ["prefabPath"] = newPath })); + Assert.IsFalse(failResult.Value("success")); + Assert.IsTrue(failResult.Value("error").Contains("already linked")); - Assert.IsTrue(result.Value("success"), "create_from_gameobject with unlinkIfInstance should succeed."); - var data = result["data"] as JObject; - Assert.IsTrue(data.Value("wasUnlinked"), "wasUnlinked should be true."); - - // Note: After creating the new prefab, the sourceObject is now linked to the NEW prefab - // (via SaveAsPrefabAssetAndConnect in CreatePrefabAsset), which is the correct behavior. - // What matters is that it was unlinked from the original prefab first. - Assert.IsTrue(PrefabUtility.IsAnyPrefabInstanceRoot(sourceObject), - "Source object should now be linked to the new prefab."); - string currentPrefabPath = PrefabUtility.GetPrefabAssetPathOfNearestInstanceRoot(sourceObject); - Assert.AreNotEqual(initialPrefabPath, currentPrefabPath, - "Source object should NOT be linked to original prefab anymore."); - Assert.AreEqual(newPrefabPath, currentPrefabPath, - "Source object should now be linked to the new prefab."); - } - finally - { - SafeDeleteAsset(initialPrefabPath); - SafeDeleteAsset(Path.Combine(TempDirectory, "NewFromLinked.prefab").Replace('\\', '/')); - if (sourceObject != null) UnityEngine.Object.DestroyImmediate(sourceObject, true); - if (instance != null) UnityEngine.Object.DestroyImmediate(instance, true); - } - } - - [Test] - public void CreateFromGameObject_Fails_WhenTargetIsAlreadyLinked() - { - string prefabPath = Path.Combine(TempDirectory, "Existing.prefab").Replace('\\', '/'); - GameObject sourceObject = new GameObject("SourceObject"); - - try - { - // Create initial prefab and connect the source object to it - GameObject connectedInstance = PrefabUtility.SaveAsPrefabAssetAndConnect( - sourceObject, prefabPath, InteractionMode.AutomatedAction); - - // Verify the source object is now linked to the prefab - Assert.IsTrue(PrefabUtility.IsAnyPrefabInstanceRoot(sourceObject), - "Source object should be linked to prefab after SaveAsPrefabAssetAndConnect."); - - // Try to create again without unlink - sourceObject.name should find the connected instance - string newPath = Path.Combine(TempDirectory, "Duplicate.prefab").Replace('\\', '/'); - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // With unlinkIfInstance - should succeed + var unlinkResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", ["target"] = sourceObject.name, - ["prefabPath"] = newPath + ["prefabPath"] = newPath, + ["unlinkIfInstance"] = true })); + Assert.IsTrue(unlinkResult.Value("success")); + Assert.IsTrue(unlinkResult["data"].Value("wasUnlinked")); - Assert.IsFalse(result.Value("success"), - "create_from_gameobject should fail when target is already linked."); - Assert.IsTrue(result.Value("error").Contains("already linked"), - "Error message should mention 'already linked'."); - } - finally - { - SafeDeleteAsset(prefabPath); - SafeDeleteAsset(Path.Combine(TempDirectory, "Duplicate.prefab").Replace('\\', '/')); - if (sourceObject != null) UnityEngine.Object.DestroyImmediate(sourceObject, true); - } - } - - [Test] - public void CreateFromGameObject_Overwrites_WhenAllowOverwriteIsTrue() - { - string prefabPath = Path.Combine(TempDirectory, "OverwriteTest.prefab").Replace('\\', '/'); - GameObject firstObject = new GameObject("OverwriteTest"); // Use path filename - GameObject secondObject = new GameObject("OverwriteTest"); // Use path filename - - try - { - // Create initial prefab - PrefabUtility.SaveAsPrefabAsset(firstObject, prefabPath, out bool _); - AssetDatabase.Refresh(); - - GameObject firstPrefab = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual("OverwriteTest", firstPrefab.name, "First prefab should have name 'OverwriteTest'."); - - // Overwrite with new object - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // With allowOverwrite - should replace + GameObject anotherObject = new GameObject("AnotherObject"); + var overwriteResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", - ["target"] = secondObject.name, - ["prefabPath"] = prefabPath, + ["target"] = anotherObject.name, + ["prefabPath"] = newPath, ["allowOverwrite"] = true })); + Assert.IsTrue(overwriteResult.Value("success")); + Assert.IsTrue(overwriteResult["data"].Value("wasReplaced")); + UnityEngine.Object.DestroyImmediate(anotherObject, true); - Assert.IsTrue(result.Value("success"), "create_from_gameobject with allowOverwrite should succeed."); - var data = result["data"] as JObject; - Assert.IsTrue(data.Value("wasReplaced"), "wasReplaced should be true."); - - AssetDatabase.Refresh(); - GameObject updatedPrefab = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual("OverwriteTest", updatedPrefab.name, "Prefab should be overwritten (keeps filename as name)."); - } - finally - { - SafeDeleteAsset(prefabPath); - if (firstObject != null) UnityEngine.Object.DestroyImmediate(firstObject, true); - if (secondObject != null) UnityEngine.Object.DestroyImmediate(secondObject, true); - } - } - - [Test] - public void CreateFromGameObject_GeneratesUniquePath_WhenFileExistsAndNoOverwrite() - { - string prefabPath = Path.Combine(TempDirectory, "UniqueTest.prefab").Replace('\\', '/'); - GameObject firstObject = new GameObject("FirstObject"); - GameObject secondObject = new GameObject("SecondObject"); - - try - { - // Create initial prefab - PrefabUtility.SaveAsPrefabAsset(firstObject, prefabPath, out bool _); - AssetDatabase.Refresh(); - - // Create again without overwrite - should generate unique path - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Without overwrite on existing - should generate unique path + GameObject thirdObject = new GameObject("ThirdObject"); + var uniqueResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", - ["target"] = secondObject.name, - ["prefabPath"] = prefabPath + ["target"] = thirdObject.name, + ["prefabPath"] = newPath })); - - Assert.IsTrue(result.Value("success"), "create_from_gameobject should succeed with unique path."); - var data = result["data"] as JObject; - string actualPath = data.Value("prefabPath"); - Assert.AreNotEqual(prefabPath, actualPath, "Path should be different (unique)."); - Assert.IsTrue(actualPath.Contains("UniqueTest 1"), "Unique path should contain suffix."); - - // Verify both prefabs exist - Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(prefabPath), - "Original prefab should still exist."); - Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(actualPath), - "New prefab should exist at unique path."); + Assert.IsTrue(uniqueResult.Value("success")); + Assert.AreNotEqual(newPath, uniqueResult["data"].Value("prefabPath")); + SafeDeleteAsset(uniqueResult["data"].Value("prefabPath")); + UnityEngine.Object.DestroyImmediate(thirdObject, true); } finally { SafeDeleteAsset(prefabPath); - SafeDeleteAsset(Path.Combine(TempDirectory, "UniqueTest 1.prefab").Replace('\\', '/')); - if (firstObject != null) UnityEngine.Object.DestroyImmediate(firstObject, true); - if (secondObject != null) UnityEngine.Object.DestroyImmediate(secondObject, true); + SafeDeleteAsset(Path.Combine(TempDirectory, "New.prefab").Replace('\\', '/')); + if (sourceObject != null) UnityEngine.Object.DestroyImmediate(sourceObject, true); } } @@ -256,28 +146,25 @@ public void CreateFromGameObject_FindsInactiveObject_WhenSearchInactiveIsTrue() try { - // Try without searchInactive - should fail - var resultWithout = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Without searchInactive - should fail to find inactive object + var failResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", ["target"] = inactiveObject.name, ["prefabPath"] = prefabPath })); + Assert.IsFalse(failResult.Value("success")); - Assert.IsFalse(resultWithout.Value("success"), - "Should fail when object is inactive and searchInactive=false."); - - // Try with searchInactive - should succeed - var resultWith = ToJObject(ManagePrefabs.HandleCommand(new JObject + // With searchInactive - should succeed + var successResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "create_from_gameobject", ["target"] = inactiveObject.name, ["prefabPath"] = prefabPath, ["searchInactive"] = true })); - - Assert.IsTrue(resultWith.Value("success"), - "Should succeed when searchInactive=true."); + Assert.IsTrue(successResult.Value("success")); + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(prefabPath)); } finally { @@ -286,43 +173,14 @@ public void CreateFromGameObject_FindsInactiveObject_WhenSearchInactiveIsTrue() } } - [Test] - public void CreateFromGameObject_CreatesDirectory_WhenPathDoesNotExist() - { - string prefabPath = Path.Combine(TempDirectory, "Nested/Deep/Directory/NewPrefab.prefab").Replace('\\', '/'); - GameObject sceneObject = new GameObject("TestObject"); - - try - { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject - { - ["action"] = "create_from_gameobject", - ["target"] = sceneObject.name, - ["prefabPath"] = prefabPath - })); - - Assert.IsTrue(result.Value("success"), "Should create directories as needed."); - - GameObject prefabAsset = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.IsNotNull(prefabAsset, "Prefab should exist at nested path."); - Assert.IsTrue(AssetDatabase.IsValidFolder(Path.Combine(TempDirectory, "Nested").Replace('\\', '/')), - "Nested directory should be created."); - } - finally - { - SafeDeleteAsset(prefabPath); - if (sceneObject != null) UnityEngine.Object.DestroyImmediate(sceneObject, true); - } - } - #endregion - #region READ Tests (GetInfo & GetHierarchy) + #region READ Tests [Test] - public void GetInfo_ReturnsCorrectMetadata_ForValidPrefab() + public void GetInfo_ReturnsMetadata() { - string prefabPath = CreateTestPrefab("InfoTestPrefab"); + string prefabPath = CreateTestPrefab("InfoTest"); try { @@ -332,19 +190,12 @@ public void GetInfo_ReturnsCorrectMetadata_ForValidPrefab() ["prefabPath"] = prefabPath })); - Assert.IsTrue(result.Value("success"), "get_info should succeed."); + Assert.IsTrue(result.Value("success")); var data = result["data"] as JObject; - Assert.AreEqual(prefabPath, data.Value("assetPath")); - Assert.IsNotNull(data.Value("guid"), "GUID should be present."); - Assert.AreEqual("Regular", data.Value("prefabType"), "Should be Regular prefab type."); - Assert.AreEqual("InfoTestPrefab", data.Value("rootObjectName")); - Assert.AreEqual(0, data.Value("childCount"), "Should have no children."); - Assert.IsFalse(data.Value("isVariant"), "Should not be a variant."); - - var components = data["rootComponentTypes"] as JArray; - Assert.IsNotNull(components, "Component types should be present."); - Assert.IsTrue(components.Count > 0, "Should have at least one component."); + Assert.IsNotNull(data.Value("guid")); + Assert.AreEqual("Regular", data.Value("prefabType")); + Assert.AreEqual("InfoTest", data.Value("rootObjectName")); } finally { @@ -353,399 +204,399 @@ public void GetInfo_ReturnsCorrectMetadata_ForValidPrefab() } [Test] - public void GetInfo_ReturnsError_ForInvalidPath() + public void GetHierarchy_ReturnsHierarchyWithNestingInfo() { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Create a prefab with nested prefab instance + string childPrefabPath = CreateTestPrefab("ChildPrefab"); + string containerPath = null; + + try { - ["action"] = "get_info", - ["prefabPath"] = "Assets/Nonexistent/Prefab.prefab" - })); + GameObject container = new GameObject("Container"); + GameObject child1 = new GameObject("Child1"); + child1.transform.parent = container.transform; - Assert.IsFalse(result.Value("success"), "get_info should fail for invalid path."); - Assert.IsTrue(result.Value("error").Contains("No prefab asset found") || - result.Value("error").Contains("not found"), - "Error should mention prefab not found."); - } + // Add nested prefab instance + GameObject nestedInstance = PrefabUtility.InstantiatePrefab( + AssetDatabase.LoadAssetAtPath(childPrefabPath)) as GameObject; + nestedInstance.transform.parent = container.transform; - [Test] - public void GetHierarchy_ReturnsCompleteHierarchy_ForNestedPrefab() - { - string prefabPath = CreateNestedTestPrefab("HierarchyTest"); + containerPath = Path.Combine(TempDirectory, "Container.prefab").Replace('\\', '/'); + PrefabUtility.SaveAsPrefabAsset(container, containerPath, out bool _); + UnityEngine.Object.DestroyImmediate(container); + AssetDatabase.Refresh(); - try - { var result = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "get_hierarchy", - ["prefabPath"] = prefabPath + ["prefabPath"] = containerPath })); - Assert.IsTrue(result.Value("success"), "get_hierarchy should succeed."); + Assert.IsTrue(result.Value("success")); var data = result["data"] as JObject; - - Assert.AreEqual(prefabPath, data.Value("prefabPath")); - int total = data.Value("total"); - Assert.IsTrue(total >= 3, $"Should have at least 3 objects (root + 2 children), got {total}."); - var items = data["items"] as JArray; - Assert.IsNotNull(items, "Items should be present."); - Assert.AreEqual(total, items.Count, "Items count should match total."); + Assert.IsTrue(data.Value("total") >= 3); // Container, Child1, nested prefab - // Find root object + // Verify root and nested prefab info var root = items.Cast().FirstOrDefault(j => j["prefab"]["isRoot"].Value()); - Assert.IsNotNull(root, "Should have a root object with isRoot=true."); - Assert.AreEqual("HierarchyTest", root.Value("name")); + Assert.IsNotNull(root); + Assert.AreEqual("Container", root.Value("name")); + + var nested = items.Cast().FirstOrDefault(j => j["prefab"]["isNestedRoot"].Value()); + Assert.IsNotNull(nested); + Assert.AreEqual(1, nested["prefab"]["nestingDepth"].Value()); } finally { - SafeDeleteAsset(prefabPath); + if (containerPath != null) SafeDeleteAsset(containerPath); + SafeDeleteAsset(childPrefabPath); } } + #endregion + + #region UPDATE Tests (ModifyContents) + [Test] - public void GetHierarchy_IncludesNestingInfo_ForNestedPrefabs() + public void ModifyContents_ModifiesTransformWithoutOpeningStage() { - // Create a parent prefab first - string parentPath = CreateTestPrefab("ParentPrefab"); + string prefabPath = CreateTestPrefab("ModifyTest"); try { - // Create a prefab that contains the parent prefab as nested - string childPath = CreateTestPrefab("ChildPrefab"); - GameObject container = new GameObject("Container"); - GameObject nestedInstance = PrefabUtility.InstantiatePrefab( - AssetDatabase.LoadAssetAtPath(childPath)) as GameObject; - nestedInstance.transform.parent = container.transform; - - string nestedPrefabPath = Path.Combine(TempDirectory, "NestedContainer.prefab").Replace('\\', '/'); - PrefabUtility.SaveAsPrefabAsset(container, nestedPrefabPath, out bool _); - UnityEngine.Object.DestroyImmediate(container); - - AssetDatabase.Refresh(); + StageUtility.GoToMainStage(); + Assert.IsNull(PrefabStageUtility.GetCurrentPrefabStage()); var result = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "get_hierarchy", - ["prefabPath"] = nestedPrefabPath + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["position"] = new JArray(1f, 2f, 3f), + ["rotation"] = new JArray(45f, 0f, 0f), + ["scale"] = new JArray(2f, 2f, 2f) })); - Assert.IsTrue(result.Value("success"), "get_hierarchy should succeed."); - var data = result["data"] as JObject; - var items = data["items"] as JArray; + Assert.IsTrue(result.Value("success")); - // Find the nested prefab - var nested = items.Cast().FirstOrDefault(j => j["prefab"]["isNestedRoot"].Value()); - Assert.IsNotNull(nested, "Should have a nested prefab root."); - Assert.AreEqual(1, nested["prefab"]["nestingDepth"].Value(), - "Nested prefab should have depth 1."); + // Verify no stage was opened (headless editing) + Assert.IsNull(PrefabStageUtility.GetCurrentPrefabStage()); + + // Verify changes persisted + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + Assert.AreEqual(new Vector3(1f, 2f, 3f), reloaded.transform.localPosition); + Assert.AreEqual(new Vector3(2f, 2f, 2f), reloaded.transform.localScale); } finally { - // Delete nested container first (before deleting prefabs it references) - SafeDeleteAsset(Path.Combine(TempDirectory, "NestedContainer.prefab").Replace('\\', '/')); - SafeDeleteAsset(parentPath); - SafeDeleteAsset(Path.Combine(TempDirectory, "ChildPrefab.prefab").Replace('\\', '/')); + SafeDeleteAsset(prefabPath); } } - #endregion - - #region UPDATE Tests (Open, Save, Close) - [Test] - public void SaveOpenStage_WithForce_SavesEvenWhenNotDirty() + public void ModifyContents_TargetsChildrenByNameAndPath() { - string prefabPath = CreateTestPrefab("ForceSaveTest"); - Vector3 originalScale = Vector3.one; + string prefabPath = CreateNestedTestPrefab("TargetTest"); try { - ManagePrefabs.HandleCommand(new JObject - { - ["action"] = "open_stage", - ["prefabPath"] = prefabPath - }); - - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - Assert.IsNotNull(stage, "Stage should be open."); - Assert.IsFalse(stage.scene.isDirty, "Stage should not be dirty initially."); - - // Save without force - should succeed but indicate no changes - var noForceResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Target by name + var nameResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "save_open_stage" + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child1", + ["position"] = new JArray(10f, 10f, 10f) })); + Assert.IsTrue(nameResult.Value("success")); - Assert.IsTrue(noForceResult.Value("success"), - "Save should succeed even when not dirty."); - - // Now save with force - var forceResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Target by path + var pathResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "save_open_stage", - ["force"] = true + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child1/Grandchild", + ["scale"] = new JArray(3f, 3f, 3f) })); + Assert.IsTrue(pathResult.Value("success")); - Assert.IsTrue(forceResult.Value("success"), "Force save should succeed."); - var data = forceResult["data"] as JObject; - Assert.IsTrue(data.Value("isDirty") || data.Value("isOpen"), - "Stage should still be open after force save."); + // Verify changes + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + Assert.AreEqual(new Vector3(10f, 10f, 10f), reloaded.transform.Find("Child1").localPosition); + Assert.AreEqual(new Vector3(3f, 3f, 3f), reloaded.transform.Find("Child1/Grandchild").localScale); } finally { - StageUtility.GoToMainStage(); SafeDeleteAsset(prefabPath); } } [Test] - public void SaveOpenStage_DoesNotShowSaveDialog() + public void ModifyContents_AddsAndRemovesComponents() { - string prefabPath = CreateTestPrefab("NoDialogTest"); + string prefabPath = CreateTestPrefab("ComponentTest"); + // Cube primitive has BoxCollider by default try { - ManagePrefabs.HandleCommand(new JObject + // Add Rigidbody + var addResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "open_stage", - ["prefabPath"] = prefabPath - }); - - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - stage.prefabContentsRoot.transform.localScale = new Vector3(2f, 2f, 2f); - // Mark as dirty to ensure changes are tracked - EditorUtility.SetDirty(stage.prefabContentsRoot); + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentsToAdd"] = new JArray("Rigidbody") + })); + Assert.IsTrue(addResult.Value("success")); - // This save should NOT show a dialog - it should complete synchronously - // If a dialog appeared, this would hang or require user interaction - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Remove BoxCollider + var removeResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "save_open_stage", - ["force"] = true // Use force to ensure save happens + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentsToRemove"] = new JArray("BoxCollider") })); + Assert.IsTrue(removeResult.Value("success")); - // If we got here without hanging, no dialog was shown - Assert.IsTrue(result.Value("success"), - "Save should complete without showing dialog."); - - // Verify the change was saved + // Verify GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual(new Vector3(2f, 2f, 2f), reloaded.transform.localScale, - "Changes should be saved without dialog."); + Assert.IsNotNull(reloaded.GetComponent()); + Assert.IsNull(reloaded.GetComponent()); } finally { - StageUtility.GoToMainStage(); SafeDeleteAsset(prefabPath); } } [Test] - public void CloseStage_WithSaveBeforeClose_SavesDirtyChanges() + public void ModifyContents_SetsPropertiesAndRenames() { - string prefabPath = CreateTestPrefab("CloseSaveTest"); + string prefabPath = CreateNestedTestPrefab("PropertiesTest"); try { - ManagePrefabs.HandleCommand(new JObject - { - ["action"] = "open_stage", - ["prefabPath"] = prefabPath - }); - - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - stage.prefabContentsRoot.transform.position = new Vector3(5f, 5f, 5f); - // Mark as dirty to ensure changes are tracked - EditorUtility.SetDirty(stage.prefabContentsRoot); - - // Close with save var result = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "close_stage", - ["saveBeforeClose"] = true + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child1", + ["name"] = "RenamedChild", + ["tag"] = "MainCamera", + ["layer"] = "UI", + ["setActive"] = false })); - Assert.IsTrue(result.Value("success"), "Close with save should succeed."); - Assert.IsNull(PrefabStageUtility.GetCurrentPrefabStage(), - "Stage should be closed after close_stage."); + Assert.IsTrue(result.Value("success")); - // Verify changes were saved GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual(new Vector3(5f, 5f, 5f), reloaded.transform.position, - "Position change should be saved before close."); + Transform renamed = reloaded.transform.Find("RenamedChild"); + Assert.IsNotNull(renamed); + Assert.IsNull(reloaded.transform.Find("Child1")); // Old name gone + Assert.AreEqual("MainCamera", renamed.gameObject.tag); + Assert.AreEqual(LayerMask.NameToLayer("UI"), renamed.gameObject.layer); + Assert.IsFalse(renamed.gameObject.activeSelf); } finally { - StageUtility.GoToMainStage(); SafeDeleteAsset(prefabPath); } } [Test] - public void OpenEditClose_CompleteWorkflow_Succeeds() + public void ModifyContents_WorksOnComplexMultiComponentPrefab() { - string prefabPath = CreateTestPrefab("WorkflowTest"); + // Create a complex prefab: Vehicle with multiple children, each with multiple components + string prefabPath = CreateComplexTestPrefab("Vehicle"); try { - // OPEN - var openResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Modify root - add Rigidbody + var rootResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "open_stage", - ["prefabPath"] = prefabPath + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["componentsToAdd"] = new JArray("Rigidbody") })); - Assert.IsTrue(openResult.Value("success"), "Open should succeed."); + Assert.IsTrue(rootResult.Value("success")); - // EDIT - PrefabStage stage = PrefabStageUtility.GetCurrentPrefabStage(); - stage.prefabContentsRoot.transform.localRotation = Quaternion.Euler(45f, 45f, 45f); - // Mark as dirty to ensure changes are tracked - EditorUtility.SetDirty(stage.prefabContentsRoot); + // Modify child by name - reposition FrontWheel, add SphereCollider + var wheelResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "FrontWheel", + ["position"] = new JArray(0f, 0.5f, 2f), + ["componentsToAdd"] = new JArray("SphereCollider") + })); + Assert.IsTrue(wheelResult.Value("success")); - // SAVE - var saveResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Modify nested child by path - scale Barrel inside Turret + var barrelResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "save_open_stage", - ["force"] = true // Use force to ensure save happens + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Turret/Barrel", + ["scale"] = new JArray(0.5f, 0.5f, 3f), + ["tag"] = "Player" })); - Assert.IsTrue(saveResult.Value("success"), "Save should succeed."); - // Note: stage.scene.isDirty may still be true in Unity's internal state - // The important thing is that changes were saved (verified below) + Assert.IsTrue(barrelResult.Value("success")); - // CLOSE - var closeResult = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Remove component from child + var removeResult = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "close_stage" + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "BackWheel", + ["componentsToRemove"] = new JArray("BoxCollider") })); - Assert.IsTrue(closeResult.Value("success"), "Close should succeed."); - Assert.IsNull(PrefabStageUtility.GetCurrentPrefabStage(), - "No stage should be open after close."); + Assert.IsTrue(removeResult.Value("success")); - // VERIFY + // Verify all changes persisted GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual(Quaternion.Euler(45f, 45f, 45f), reloaded.transform.localRotation, - "Rotation should be saved and persisted."); + + // Root has Rigidbody + Assert.IsNotNull(reloaded.GetComponent(), "Root should have Rigidbody"); + + // FrontWheel repositioned and has SphereCollider + Transform frontWheel = reloaded.transform.Find("FrontWheel"); + Assert.AreEqual(new Vector3(0f, 0.5f, 2f), frontWheel.localPosition); + Assert.IsNotNull(frontWheel.GetComponent(), "FrontWheel should have SphereCollider"); + + // Turret/Barrel scaled and tagged + Transform barrel = reloaded.transform.Find("Turret/Barrel"); + Assert.AreEqual(new Vector3(0.5f, 0.5f, 3f), barrel.localScale); + Assert.AreEqual("Player", barrel.gameObject.tag); + + // BackWheel BoxCollider removed + Transform backWheel = reloaded.transform.Find("BackWheel"); + Assert.IsNull(backWheel.GetComponent(), "BackWheel BoxCollider should be removed"); } finally { - StageUtility.GoToMainStage(); SafeDeleteAsset(prefabPath); } } - #endregion - - #region Edge Cases & Error Handling - - [Test] - public void HandleCommand_ReturnsError_ForUnknownAction() - { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject - { - ["action"] = "unknown_action" - })); - - Assert.IsFalse(result.Value("success"), "Unknown action should fail."); - Assert.IsTrue(result.Value("error").Contains("Unknown action"), - "Error should mention unknown action."); - } - [Test] - public void HandleCommand_ReturnsError_ForNullParameters() + public void ModifyContents_ReparentsChildWithinPrefab() { - var result = ToJObject(ManagePrefabs.HandleCommand(null)); - - Assert.IsFalse(result.Value("success"), "Null parameters should fail."); - Assert.IsTrue(result.Value("error").Contains("null"), - "Error should mention null parameters."); - } - - [Test] - public void HandleCommand_ReturnsError_WhenActionIsMissing() - { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject())); - - Assert.IsFalse(result.Value("success"), "Missing action should fail."); - Assert.IsTrue(result.Value("error").Contains("Action parameter is required"), - "Error should mention required action parameter."); - } + string prefabPath = CreateNestedTestPrefab("ReparentTest"); - [Test] - public void CreateFromGameObject_ReturnsError_ForEmptyTarget() - { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + try { - ["action"] = "create_from_gameobject", - ["prefabPath"] = "Assets/Test.prefab" - })); + // Reparent Child2 under Child1 + var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child2", + ["parent"] = "Child1" + })); - Assert.IsFalse(result.Value("success"), "Missing target should fail."); - Assert.IsTrue(result.Value("error").Contains("'target' parameter is required"), - "Error should mention required target parameter."); - } + Assert.IsTrue(result.Value("success")); - [Test] - public void CreateFromGameObject_ReturnsError_ForEmptyPrefabPath() - { - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Verify Child2 is now under Child1 + GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); + Assert.IsNull(reloaded.transform.Find("Child2"), "Child2 should no longer be direct child of root"); + Assert.IsNotNull(reloaded.transform.Find("Child1/Child2"), "Child2 should now be under Child1"); + } + finally { - ["action"] = "create_from_gameobject", - ["target"] = "SomeObject" - })); - - Assert.IsFalse(result.Value("success"), "Missing prefabPath should fail."); - Assert.IsTrue(result.Value("error").Contains("'prefabPath' parameter is required"), - "Error should mention required prefabPath parameter."); + SafeDeleteAsset(prefabPath); + } } [Test] - public void CreateFromGameObject_ReturnsError_ForPathTraversal() + public void ModifyContents_PreventsHierarchyLoops() { - GameObject testObject = new GameObject("TestObject"); + string prefabPath = CreateNestedTestPrefab("HierarchyLoopTest"); try { + // Attempt to parent Child1 under its own descendant (Grandchild) var result = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "create_from_gameobject", - ["target"] = "TestObject", - ["prefabPath"] = "../../etc/passwd" + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "Child1", + ["parent"] = "Child1/Grandchild" })); - Assert.IsFalse(result.Value("success"), "Path traversal should be blocked."); - Assert.IsTrue(result.Value("error").Contains("path traversal") || - result.Value("error").Contains("Invalid"), - "Error should mention path traversal or invalid path."); + Assert.IsFalse(result.Value("success")); + Assert.IsTrue(result.Value("error").Contains("hierarchy loop") || + result.Value("error").Contains("would create"), + "Error should mention hierarchy loop prevention"); } finally { - if (testObject != null) UnityEngine.Object.DestroyImmediate(testObject, true); + SafeDeleteAsset(prefabPath); } } + #endregion + + #region Error Handling + [Test] - public void CreateFromGameObject_AutoPrependsAssets_WhenPathIsRelative() + public void HandleCommand_ValidatesParameters() { - GameObject testObject = new GameObject("TestObject"); + // Null params + var nullResult = ToJObject(ManagePrefabs.HandleCommand(null)); + Assert.IsFalse(nullResult.Value("success")); + Assert.IsTrue(nullResult.Value("error").Contains("null")); + + // Missing action + var missingAction = ToJObject(ManagePrefabs.HandleCommand(new JObject())); + Assert.IsFalse(missingAction.Value("success")); + Assert.IsTrue(missingAction.Value("error").Contains("Action parameter is required")); + + // Unknown action + var unknownAction = ToJObject(ManagePrefabs.HandleCommand(new JObject { ["action"] = "invalid" })); + Assert.IsFalse(unknownAction.Value("success")); + Assert.IsTrue(unknownAction.Value("error").Contains("Unknown action")); + + // Path traversal + GameObject testObj = new GameObject("Test"); + var traversal = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "create_from_gameobject", + ["target"] = "Test", + ["prefabPath"] = "../../etc/passwd" + })); + Assert.IsFalse(traversal.Value("success")); + Assert.IsTrue(traversal.Value("error").Contains("path traversal") || + traversal.Value("error").Contains("Invalid")); + UnityEngine.Object.DestroyImmediate(testObj, true); + } + + [Test] + public void ModifyContents_ReturnsErrorsForInvalidInputs() + { + string prefabPath = CreateTestPrefab("ErrorTest"); try { - // SanitizeAssetPath auto-prepends "Assets/" to relative paths - var result = ToJObject(ManagePrefabs.HandleCommand(new JObject + // Invalid target + var invalidTarget = ToJObject(ManagePrefabs.HandleCommand(new JObject { - ["action"] = "create_from_gameobject", - ["target"] = "TestObject", - ["prefabPath"] = "SomeFolder/Prefab.prefab" + ["action"] = "modify_contents", + ["prefabPath"] = prefabPath, + ["target"] = "NonexistentChild" })); + Assert.IsFalse(invalidTarget.Value("success")); + Assert.IsTrue(invalidTarget.Value("error").Contains("not found")); - Assert.IsTrue(result.Value("success"), "Should auto-prepend Assets/ to relative path."); - - // Clean up the created prefab at the corrected path - SafeDeleteAsset("Assets/SomeFolder/Prefab.prefab"); + // Invalid path + LogAssert.Expect(LogType.Error, new Regex(".*modify_contents.*does not exist.*")); + var invalidPath = ToJObject(ManagePrefabs.HandleCommand(new JObject + { + ["action"] = "modify_contents", + ["prefabPath"] = "Assets/Nonexistent.prefab" + })); + Assert.IsFalse(invalidPath.Value("success")); } finally { - if (testObject != null) UnityEngine.Object.DestroyImmediate(testObject, true); + SafeDeleteAsset(prefabPath); } } @@ -764,10 +615,7 @@ private static string CreateTestPrefab(string name) UnityEngine.Object.DestroyImmediate(temp); AssetDatabase.Refresh(); - if (!success) - { - throw new Exception($"Failed to create test prefab at {path}"); - } + if (!success) throw new Exception($"Failed to create test prefab at {path}"); return path; } @@ -775,27 +623,56 @@ private static string CreateNestedTestPrefab(string name) { EnsureFolder(TempDirectory); GameObject root = new GameObject(name); + GameObject child1 = new GameObject("Child1") { transform = { parent = root.transform } }; + GameObject child2 = new GameObject("Child2") { transform = { parent = root.transform } }; + GameObject grandchild = new GameObject("Grandchild") { transform = { parent = child1.transform } }; + + string path = Path.Combine(TempDirectory, name + ".prefab").Replace('\\', '/'); + PrefabUtility.SaveAsPrefabAsset(root, path, out bool success); + UnityEngine.Object.DestroyImmediate(root); + AssetDatabase.Refresh(); + + if (!success) throw new Exception($"Failed to create nested test prefab at {path}"); + return path; + } + + private static string CreateComplexTestPrefab(string name) + { + // Creates: Vehicle (root with BoxCollider) + // - FrontWheel (Cube with MeshRenderer, BoxCollider) + // - BackWheel (Cube with MeshRenderer, BoxCollider) + // - Turret (empty) + // - Barrel (Cylinder with MeshRenderer, CapsuleCollider) + EnsureFolder(TempDirectory); + + GameObject root = new GameObject(name); + root.AddComponent(); - // Add children - GameObject child1 = new GameObject("Child1"); - child1.transform.parent = root.transform; + GameObject frontWheel = GameObject.CreatePrimitive(PrimitiveType.Cube); + frontWheel.name = "FrontWheel"; + frontWheel.transform.parent = root.transform; + frontWheel.transform.localPosition = new Vector3(0, 0.5f, 1f); - GameObject child2 = new GameObject("Child2"); - child2.transform.parent = root.transform; + GameObject backWheel = GameObject.CreatePrimitive(PrimitiveType.Cube); + backWheel.name = "BackWheel"; + backWheel.transform.parent = root.transform; + backWheel.transform.localPosition = new Vector3(0, 0.5f, -1f); - // Add grandchild - GameObject grandchild = new GameObject("Grandchild"); - grandchild.transform.parent = child1.transform; + GameObject turret = new GameObject("Turret"); + turret.transform.parent = root.transform; + turret.transform.localPosition = new Vector3(0, 1f, 0); + + GameObject barrel = GameObject.CreatePrimitive(PrimitiveType.Cylinder); + barrel.name = "Barrel"; + barrel.transform.parent = turret.transform; + barrel.transform.localPosition = new Vector3(0, 0, 1f); string path = Path.Combine(TempDirectory, name + ".prefab").Replace('\\', '/'); PrefabUtility.SaveAsPrefabAsset(root, path, out bool success); UnityEngine.Object.DestroyImmediate(root); AssetDatabase.Refresh(); - if (!success) - { - throw new Exception($"Failed to create nested test prefab at {path}"); - } + if (!success) throw new Exception($"Failed to create complex test prefab at {path}"); return path; }