diff --git a/MCPForUnity/Editor/Tools/Sprite2D.meta b/MCPForUnity/Editor/Tools/Sprite2D.meta new file mode 100644 index 000000000..574f9695a --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: e7ba99f77eb524525964129bb1fcd94c +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs b/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs new file mode 100644 index 000000000..c9a55d5fb --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs @@ -0,0 +1,55 @@ +using Newtonsoft.Json.Linq; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + [McpForUnityTool("manage_sprite", AutoRegister = false, Group = "animation")] + public static class ManageSprite + { + private static readonly string[] ValidActions = + { + "get_info", "slice_sheet", "setup_clips", + "setup_controller", "full_setup" + }; + + public static object HandleCommand(JObject @params) + { + var diagnostics = new SpriteDiagnosticBuilder(); + + string action = @params["action"]?.ToString()?.ToLowerInvariant(); + if (string.IsNullOrEmpty(action)) + return diagnostics.Fail("BAD_PARAM", + "'action' is required. Valid: " + string.Join(", ", ValidActions)); + + try + { + switch (action) + { + case "get_info": + return SpriteImportSetup.GetInfo(@params, diagnostics); + + case "slice_sheet": + return SpriteImportSetup.SliceSheet(@params, diagnostics); + + case "setup_clips": + return SpriteClipBuilder.SetupClips(@params, diagnostics); + + case "setup_controller": + return SpriteControllerBuilder.Build(@params, diagnostics); + + case "full_setup": + return SpriteFullSetup.Run(@params, diagnostics); + + default: + return diagnostics.Fail("BAD_PARAM", + $"Unknown action '{action}'. Valid: " + string.Join(", ", ValidActions)); + } + } + catch (System.Exception e) + { + McpLog.Error($"[ManageSprite] Action '{action}' failed: {e}"); + return diagnostics.Fail("INTERNAL", $"Internal error processing action '{action}': {e.Message}"); + } + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs.meta new file mode 100644 index 000000000..9445f93fa --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/ManageSprite.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 636047e62387a46a39a2cdfd31172f8a \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs new file mode 100644 index 000000000..db406c7be --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs @@ -0,0 +1,256 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal class SpriteClipInfo + { + public string name; + public string path; + public int frame_count; + public float fps; + public bool loop; + public float duration; + } + + internal static class SpriteClipBuilder + { + /// + /// Builds AnimationClips out of sliced sprites and saves them as .anim assets. + /// params: + /// path - sprite texture asset path + /// clips - [{name, start_frame, end_frame, fps (opt, def=12), loop (opt)}] + /// output_dir - where the clips are written (default: the sprite's own folder) + /// overwrite - bool (default false); an existing clip is kept unless this is true + /// + public static object SetupClips(JObject @params, SpriteDiagnosticBuilder diagnostics) + { + if (!SpriteParams.TryReadAssetPath(@params, "path", out string path, out string pathError)) + return diagnostics.Fail("BAD_PARAM", pathError); + + var clipsToken = @params["clips"] as JArray; + if (clipsToken == null || clipsToken.Count == 0) + return diagnostics.Fail("BAD_PARAM", "'clips' array is required."); + + bool overwrite = ParamCoercion.CoerceBool(@params["overwrite"], false); + + var clips = CreateClips(path, clipsToken, @params["output_dir"]?.ToString(), overwrite, diagnostics); + if (diagnostics.HasErrors) + return diagnostics.Fail(); + + return new + { + success = true, + sprite_path = path, + clip_count = clips.Count, + clips, + diagnostics = diagnostics.Build(), + }; + } + + /// `path` is already sanitized; `outputDir` null means the sprite's own folder. + internal static List CreateClips(string path, JArray clipsToken, string outputDir, + bool overwrite, SpriteDiagnosticBuilder diagnostics) + { + var created = new List(); + + var allSprites = AssetDatabase.LoadAllAssetsAtPath(path) + .OfType() + .OrderBy(s => NaturalSortKey(s.name)) + .ToArray(); + + if (allSprites.Length == 0) + { + diagnostics.AddError("NOT_FOUND", $"No sprites found at '{path}'. Run slice_sheet first."); + return created; + } + + outputDir = outputDir ?? Path.GetDirectoryName(path)?.Replace('\\', '/') ?? "Assets"; + + if (!SpriteParams.TryReadAssetPath(new JObject { ["output_dir"] = outputDir }, "output_dir", out outputDir, out string dirError)) + { + diagnostics.AddError("BAD_PARAM", dirError); + return created; + } + if (!AssetDatabase.IsValidFolder(outputDir)) + CreateFolders(outputDir); + + foreach (JToken clipToken in clipsToken) + { + // Measured: a non-object clips entry threw InvalidCastException on a typed cast. + if (!(clipToken is JObject clipDef)) + { + diagnostics.AddWarning("CLIP_NOT_AN_OBJECT", "A clips entry is not an object - skipped.", "Each clip must be an object with a 'name'."); + continue; + } + + string clipName = clipDef["name"]?.ToString(); + if (string.IsNullOrEmpty(clipName)) + { diagnostics.AddWarning("CLIP_NO_NAME", "Clip name is missing — skipped.", "Add a 'name' field to each clip definition."); continue; } + + // Measured: "nested/walk" either threw from CreateAsset or, where the folder + // existed, wrote the clip outside output_dir. + if (clipName.Contains("/") || clipName.Contains("\\")) + { + diagnostics.AddWarning("CLIP_BAD_NAME", $"Clip '{clipName}': the name cannot contain a path separator - skipped.", "Remove '..' and path separators from the clip name."); + continue; + } + + // Sequential, not chained with ||: a short-circuited call leaves its out + // parameter unassigned and the second value is used below. + int endFrame = allSprites.Length - 1; + bool rangeOk = SpriteParams.TryReadWholeNumber(clipDef, "start_frame", 0, out int startFrame, out string frameError); + if (rangeOk) rangeOk = SpriteParams.TryReadWholeNumber(clipDef, "end_frame", allSprites.Length - 1, out endFrame, out frameError); + if (!rangeOk) + { + diagnostics.AddWarning("CLIP_BAD_RANGE", $"Clip '{clipName}': {frameError} - skipped.", "start_frame and end_frame must be whole numbers within a sprite index."); + continue; + } + if (endFrame > allSprites.Length - 1) + { + // Skip/Take clamps silently: an end_frame past the last sprite produced a + // shorter clip and reported success. + diagnostics.AddWarning("CLIP_BAD_RANGE", $"Clip '{clipName}': end_frame {endFrame} is past the last sprite index {allSprites.Length - 1} - skipped.", $"This sheet has {allSprites.Length} sprites, so end_frame must be at most {allSprites.Length - 1}."); + continue; + } + if (startFrame < 0 || endFrame < startFrame) + { + // Skip yields everything for a negative count: start_frame=-2 with + // end_frame=3 wrote frames 0..5 as a success. + diagnostics.AddWarning("CLIP_BAD_RANGE", $"Clip '{clipName}': frame range [{startFrame},{endFrame}] is invalid - skipped.", "start_frame must be 0 or more, and end_frame must not be below start_frame."); + continue; + } + // `fps <= 0f` is false for NaN, so a NaN rate wrote a clip of NaN keyframe + // times and reported success. + if (!SpriteParams.TryReadFiniteFloat(clipDef, "fps", 12f, out float fps, out string fpsError)) + { + diagnostics.AddWarning("CLIP_BAD_FPS", $"Clip '{clipName}': {fpsError} - skipped.", "Leave fps out to use the default of 12."); + continue; + } + if (fps <= 0f) + { + // Times are i / fps, so a non-positive rate puts every key at infinity. + diagnostics.AddWarning("CLIP_BAD_FPS", $"Clip '{clipName}': fps must be greater than 0, got {fps} - skipped.", "Leave fps out to use the default of 12."); + continue; + } + + var entry = SpriteNamingDetector.Detect(clipName); + if (!SpriteParams.TryReadBool(clipDef, "loop", entry.Loop, out bool loop, out string loopError)) + { + diagnostics.AddWarning("CLIP_BAD_LOOP", $"Clip '{clipName}': {loopError} - skipped.", "Leave loop out to let the clip name decide."); + continue; + } + + var frameSprites = allSprites.Skip(startFrame).Take(endFrame - startFrame + 1).ToArray(); + if (frameSprites.Length <= 2) + diagnostics.AddWarning("LOW_FRAME_COUNT", $"Clip '{clipName}' has only {frameSprites.Length} frame(s) — animation may not be visible."); + + // Refusals come before the allocation: a `new AnimationClip` that never becomes + // an asset leaks. + string clipPath = AssetPathUtility.SanitizeAssetPath($"{outputDir}/{clipName}.anim"); + if (clipPath == null || !AssetPathUtility.IsValidAssetPath(clipPath)) + { + diagnostics.AddWarning("CLIP_BAD_NAME", $"Clip '{clipName}': the name cannot be used as a file name - skipped.", "Remove '..', path separators and characters like : * ? \" < > | from the clip name."); + continue; + } + + var existing = AssetDatabase.LoadAssetAtPath(clipPath); + if (existing != null && !overwrite) + { + // Measured: an unrelated clip at this path was replaced by a request carrying + // no overwrite field. Same policy as the controller builder: destruction + // needs authorisation. + diagnostics.AddWarning("CLIP_EXISTS", $"Clip '{clipName}': an animation clip already exists at '{clipPath}' - skipped.", "Set overwrite=true to replace it.", "Choose a different clip name or output_dir."); + continue; + } + + var clip = new AnimationClip { frameRate = fps }; + + var binding = new EditorCurveBinding + { + type = typeof(SpriteRenderer), + path = "", + propertyName = "m_Sprite", + }; + + var keyframes = new ObjectReferenceKeyframe[frameSprites.Length]; + for (int i = 0; i < frameSprites.Length; i++) + { + keyframes[i] = new ObjectReferenceKeyframe + { + time = i / fps, + value = frameSprites[i], + }; + } + + AnimationUtility.SetObjectReferenceCurve(clip, binding, keyframes); + + var settings = AnimationUtility.GetAnimationClipSettings(clip); + settings.loopTime = loop; + AnimationUtility.SetAnimationClipSettings(clip, settings); + + // CreateAsset replaces an existing asset itself; deleting first left nothing at + // the path when the replacement failed to be written. Reference equality, not a + // null check: a failed replacement leaves the old asset loadable at the path. + AssetDatabase.CreateAsset(clip, clipPath); + if (AssetDatabase.LoadAssetAtPath(clipPath) != clip) + { + Object.DestroyImmediate(clip); + diagnostics.AddWarning("CLIP_WRITE_FAILED", $"Clip '{clipName}': Unity did not write '{clipPath}' - skipped.", "Check the Unity console for the AssetDatabase error."); + continue; + } + + created.Add(new SpriteClipInfo + { + name = clipName, + path = clipPath, + frame_count = frameSprites.Length, + fps = fps, + loop = loop, + duration = frameSprites.Length / fps, + }); + } + + AssetDatabase.SaveAssets(); + return created; + } + + // Plain string sort puts hero_10 before hero_2, which reorders the animation. + private static string NaturalSortKey(string name) + { + var sb = new System.Text.StringBuilder(); + int i = 0; + while (i < name.Length) + { + if (char.IsDigit(name[i])) + { + int start = i; + while (i < name.Length && char.IsDigit(name[i])) i++; + // Left-pad the run of digits so a lexicographic sort compares them numerically. + sb.Append(name.Substring(start, i - start).PadLeft(10, '0')); + } + else + { + sb.Append(name[i++]); + } + } + return sb.ToString(); + } + + /// Creates an asset folder and any missing parents above it. + internal static void CreateFolders(string path) + { + string parent = Path.GetDirectoryName(path)?.Replace('\\', '/') ?? "Assets"; + if (!AssetDatabase.IsValidFolder(parent)) + CreateFolders(parent); + string folderName = Path.GetFileName(path); + if (!string.IsNullOrEmpty(folderName)) + AssetDatabase.CreateFolder(parent, folderName); + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs.meta new file mode 100644 index 000000000..35d61a4a3 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteClipBuilder.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 8e9b1196056fa41d5ba138f1dba9d544 \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs new file mode 100644 index 000000000..73235a26a --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs @@ -0,0 +1,251 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEditor.Animations; +using UnityEngine; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal static class SpriteControllerBuilder + { + /// + /// params: + /// clips - [{name, path}] where path is an .anim asset path + /// controller_path - output .controller path (required) + /// overwrite - bool (default false) + /// + public static object Build(JObject @params, SpriteDiagnosticBuilder diagnostics) + { + var clipsToken = @params["clips"] as JArray; + if (clipsToken == null || clipsToken.Count == 0) + return diagnostics.Fail("BAD_PARAM", "'clips' array is required."); + + string controllerPath = @params["controller_path"]?.ToString(); + if (string.IsNullOrEmpty(controllerPath)) + return diagnostics.Fail("BAD_PARAM", "'controller_path' is required."); + + bool overwrite = ParamCoercion.CoerceBool(@params["overwrite"], false); + + var clips = new List<(string name, string path)>(); + foreach (JToken clipToken in clipsToken) + { + // Measured: a non-object clips entry threw InvalidCastException on a typed cast. + if (!(clipToken is JObject cd)) + { + diagnostics.AddWarning("CLIP_NOT_AN_OBJECT", "A clips entry is not an object - skipped.", "Each clip must be an object with a 'name'."); + continue; + } + string name = cd["name"]?.ToString(); + if (string.IsNullOrEmpty(name)) + { + diagnostics.AddWarning("CLIP_NO_NAME", "A clips entry has no name - skipped.", "Each clip must be an object with a 'name'."); + continue; + } + clips.Add((name, cd["path"]?.ToString() ?? "")); + } + + var built = BuildController(clips, controllerPath, overwrite, diagnostics); + if (diagnostics.HasErrors) + return diagnostics.Fail(); + + return new + { + success = true, + controller_path = built.path, + state_count = built.stateCount, + diagnostics = diagnostics.Build(), + }; + } + + /// Returns default when refused; the diagnostics say why. + internal static (string path, int stateCount) BuildController( + IEnumerable<(string name, string path)> clips, string controllerPath, bool overwrite, + SpriteDiagnosticBuilder diagnostics) + { + controllerPath = string.IsNullOrWhiteSpace(controllerPath) ? null : AssetPathUtility.SanitizeAssetPath(controllerPath.Trim()); + if (controllerPath == null) + { + diagnostics.AddError("BAD_PARAM", "'controller_path' must stay under Assets/ and cannot contain '..'."); + return default; + } + if (AssetDatabase.IsValidFolder(controllerPath)) + { + diagnostics.AddError("BAD_PARAM", $"'controller_path' names the folder '{controllerPath}'; give the controller a file name inside it."); + return default; + } + if (!controllerPath.EndsWith(".controller")) + controllerPath += ".controller"; + // Checked after the suffix: a bare 'Assets' passes SanitizeAssetPath and then + // becomes 'Assets.controller', a file at the project root; 'Assets/' becomes + // 'Assets/.controller', a file with no name. + if (!AssetPathUtility.IsValidAssetPath(controllerPath) || Path.GetFileName(controllerPath) == ".controller") + { + diagnostics.AddError("BAD_PARAM", "'controller_path' must name a file under Assets/ without characters like : * ? \" < > |."); + return default; + } + + var entries = new List<(SpriteAnimEntry entry, AnimationClip clip)>(); + foreach (var (clipName, clipPath) in clips) + { + string safeClipPath = AssetPathUtility.SanitizeAssetPath(clipPath); + if (safeClipPath == null) + { diagnostics.AddWarning("CLIP_BAD_PATH", $"Clip '{clipName}': path '{clipPath}' must stay under Assets/ and cannot contain '..' - skipped."); continue; } + var clip = AssetDatabase.LoadAssetAtPath(safeClipPath); + if (clip == null) + { diagnostics.AddWarning("CLIP_NOT_FOUND", $"Clip '{clipName}' not found at '{clipPath}' — skipped."); continue; } + entries.Add((SpriteNamingDetector.Detect(clipName), clip)); + } + + if (entries.Count == 0) + { + diagnostics.AddError("NO_CLIPS", "No valid clips loaded."); + return default; + } + + // Not deleted here: CreateAnimatorControllerAtPath replaces the asset itself, and + // deleting first left a failed rebuild with no controller at all. + if (!overwrite && AssetDatabase.LoadAssetAtPath(controllerPath) != null) + { + diagnostics.AddError("CONTROLLER_EXISTS", $"Controller already exists at '{controllerPath}'.", "Set overwrite=true to replace it."); + return default; + } + + string dir = Path.GetDirectoryName(controllerPath)?.Replace('\\', '/'); + if (!string.IsNullOrEmpty(dir) && !AssetDatabase.IsValidFolder(dir)) + SpriteClipBuilder.CreateFolders(dir); + + var controller = AnimatorController.CreateAnimatorControllerAtPath(controllerPath); + // Reference equality, not a null check: a replacement that failed leaves the old + // asset loadable at the same path. + if (controller == null || AssetDatabase.LoadAssetAtPath(controllerPath) != controller) + { + diagnostics.AddError("CONTROLLER_WRITE_FAILED", $"Unity did not write '{controllerPath}'.", "Check the Unity console for the AssetDatabase error."); + return default; + } + var rootSM = controller.layers[0].stateMachine; + + // ── Parameters ────────────────────────────────────────────────── + + var locomotionPairs = entries.Where(e => e.entry.Category == SpriteAnimCategory.Locomotion).ToList(); + if (locomotionPairs.Count > 0) + controller.AddParameter("Speed", AnimatorControllerParameterType.Float); + + var triggerNames = entries + .Where(e => !string.IsNullOrEmpty(e.entry.TriggerName) && + (e.entry.Category == SpriteAnimCategory.Combat || + e.entry.Category == SpriteAnimCategory.Jump || + e.entry.Category == SpriteAnimCategory.Object)) + .Select(e => e.entry.TriggerName) + .Distinct(); + foreach (var t in triggerNames) + controller.AddParameter(t, AnimatorControllerParameterType.Trigger); + + // ── Idle state ──────────────────────────────────────────────────── + + var idlePair = entries.FirstOrDefault(e => e.entry.Category == SpriteAnimCategory.Idle); + AnimatorState idleState = null; + if (idlePair.clip != null) + { + idleState = rootSM.AddState("Idle"); + idleState.motion = idlePair.clip; + rootSM.defaultState = idleState; + } + + // ── Locomotion ──────────────────────────────────────────────────── + + if (locomotionPairs.Count > 0) + { + if (locomotionPairs.Count == 1) + { + var locoState = rootSM.AddState(locomotionPairs[0].entry.ClipName); + locoState.motion = locomotionPairs[0].clip; + if (rootSM.defaultState == null) rootSM.defaultState = locoState; + if (idleState != null) + { + var t1 = idleState.AddTransition(locoState); + t1.AddCondition(AnimatorConditionMode.Greater, 0.1f, "Speed"); + t1.hasExitTime = false; + var t2 = locoState.AddTransition(idleState); + t2.AddCondition(AnimatorConditionMode.Less, 0.1f, "Speed"); + t2.hasExitTime = false; + } + } + else + { + var blendState = rootSM.AddState("Locomotion"); + var blendTree = new BlendTree { name = "LocomotionTree", blendType = BlendTreeType.Simple1D, blendParameter = "Speed" }; + // Off, or Unity silently redistributes the thresholds and the BlendValues + // below never reach the asset - measured live: walk/run wrote 1/2, read back 0/1. + blendTree.useAutomaticThresholds = false; + AssetDatabase.AddObjectToAsset(blendTree, controllerPath); + + foreach (var pair in locomotionPairs.OrderBy(p => p.entry.BlendValue)) + blendTree.AddChild(pair.clip, pair.entry.BlendValue); + + blendState.motion = blendTree; + if (rootSM.defaultState == null) rootSM.defaultState = blendState; + + if (idleState != null) + { + var t1 = idleState.AddTransition(blendState); + t1.AddCondition(AnimatorConditionMode.Greater, 0.1f, "Speed"); + t1.hasExitTime = false; + var t2 = blendState.AddTransition(idleState); + t2.AddCondition(AnimatorConditionMode.Less, 0.1f, "Speed"); + t2.hasExitTime = false; + } + } + } + + // ── Trigger states (combat, jump, object) ───────────────────────── + + var triggerPairs = entries.Where(e => + e.entry.Category == SpriteAnimCategory.Combat || + e.entry.Category == SpriteAnimCategory.Jump || + e.entry.Category == SpriteAnimCategory.Object).ToList(); + + foreach (var pair in triggerPairs) + { + var state = rootSM.AddState(pair.entry.ClipName); + state.motion = pair.clip; + + string trigger = pair.entry.TriggerName ?? pair.entry.ClipName; + + foreach (var existingState in rootSM.states.Select(s => s.state)) + { + if (existingState == state) continue; + var tr = existingState.AddTransition(state); + tr.AddCondition(AnimatorConditionMode.If, 0, trigger); + tr.hasExitTime = false; + } + + // A one-shot state has to hand control back, so it exits to idle on its own. + if (idleState != null && !pair.entry.Loop) + { + var exitTr = state.AddTransition(idleState); + exitTr.hasExitTime = true; + exitTr.exitTime = 1f; + exitTr.hasFixedDuration = false; + } + } + + // ── Generic / single animation ─────────────────────────────────────── + + foreach (var pair in entries.Where(e => e.entry.Category == SpriteAnimCategory.Generic)) + { + var state = rootSM.AddState(pair.entry.ClipName); + state.motion = pair.clip; + if (rootSM.defaultState == null) + rootSM.defaultState = state; + } + + EditorUtility.SetDirty(controller); + AssetDatabase.SaveAssets(); + + return (controllerPath, rootSM.states.Length); + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs.meta new file mode 100644 index 000000000..56de6ad80 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteControllerBuilder.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 41822fd4062094cf38edf541771a53d2 \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs new file mode 100644 index 000000000..1605a5c87 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs @@ -0,0 +1,42 @@ +using System.Collections.Generic; +using System.Linq; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal class SpriteDiagnostic + { + public string code; + public string severity; + public string message; + public string[] fix_options; + } + + internal class SpriteDiagnosticBuilder + { + private readonly List _list = new List(); + + public bool HasErrors => _list.Any(d => d.severity == "error"); + + public string FirstError => _list.FirstOrDefault(d => d.severity == "error")?.message; + + public void AddError(string code, string message, params string[] fixes) => + Add(code, "error", message, fixes); + + public void AddWarning(string code, string message, params string[] fixes) => + Add(code, "warning", message, fixes); + + public object Fail(string code, string message, params string[] fixes) + { + AddError(code, message, fixes); + return Fail(); + } + + public object Fail() => + new { success = false, message = FirstError, diagnostics = Build() }; + + public List Build() => new List(_list); + + private void Add(string code, string severity, string message, params string[] fixes) => + _list.Add(new SpriteDiagnostic { code = code, severity = severity, message = message, fix_options = fixes }); + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs.meta new file mode 100644 index 000000000..d2720d644 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteDiagnostics.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 54884f7a243d94239b8ad49db451e83d \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs new file mode 100644 index 000000000..bb5de4109 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs @@ -0,0 +1,177 @@ +using System.IO; +using System.Linq; +using Newtonsoft.Json.Linq; +using UnityEditor; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal static class SpriteFullSetup + { + /// + /// params: + /// path - sprite texture path (required) + /// cols - grid columns (required) + /// rows - grid rows (default 1) + /// frame_width - alternative to cols: explicit frame size + /// frame_height - alternative to rows: explicit frame size + /// clips - [{name, start_frame, end_frame, fps, loop}]; + /// omitted means every frame becomes one clip named animation_name + /// animation_name - used when clips is omitted (default: the file name) + /// controller_path - default: the sprite's own folder + /// overwrite - bool (default false) + /// add_to_scene - add an Animator to a target GameObject + /// scene_target - GameObject name + /// + public static object Run(JObject @params, SpriteDiagnosticBuilder diagnostics) + { + if (!SpriteParams.TryReadAssetPath(@params, "path", out string path, out string pathError)) + return diagnostics.Fail("BAD_PARAM", pathError); + + // ── Step 1: Slice ────────────────────────────────────────────────── + + SpriteImportSetup.SliceSheet(@params, diagnostics); + if (diagnostics.HasErrors) + return Stop("slice_sheet", diagnostics); + + // ── Step 2: Clips ────────────────────────────────────────────────── + + string outputDir = @params["output_dir"]?.ToString() + ?? Path.GetDirectoryName(path)?.Replace('\\', '/') ?? "Assets"; + + var clipsToken = @params["clips"] as JArray; + if (clipsToken == null || clipsToken.Count == 0) + { + string animName = @params["animation_name"]?.ToString() + ?? Path.GetFileNameWithoutExtension(path); + int totalFrames = GetSliceCount(path); + clipsToken = new JArray(new JObject + { + ["name"] = animName, + ["start_frame"] = 0, + ["end_frame"] = totalFrames - 1, + ["fps"] = 12, + }); + } + + bool overwrite = ParamCoercion.CoerceBool(@params["overwrite"], false); + + var clips = SpriteClipBuilder.CreateClips(path, clipsToken, outputDir, overwrite, diagnostics); + if (diagnostics.HasErrors) + return Stop("setup_clips", diagnostics); + + // ── Step 3: Controller ───────────────────────────────────────────── + + string controllerPath = @params["controller_path"]?.ToString() + ?? $"{outputDir}/{Path.GetFileNameWithoutExtension(path)}_Controller.controller"; + + var controller = SpriteControllerBuilder.BuildController( + clips.Select(c => (c.name, c.path)), controllerPath, overwrite, diagnostics); + if (diagnostics.HasErrors) + return Stop("setup_controller", diagnostics); + + // ── Step 4: Add to scene ─────────────────────────────────────────── + + bool addToScene = ParamCoercion.CoerceBool(@params["add_to_scene"], false); + string sceneTarget = @params["scene_target"]?.ToString(); + + // An attachment asked for but not made is not a success, so both misses are errors. + if (addToScene && string.IsNullOrEmpty(sceneTarget)) + { + diagnostics.AddError("SCENE_TARGET_MISSING", + "'add_to_scene' is true but 'scene_target' is empty.", + "Pass 'scene_target' with the GameObject name.", "Set add_to_scene=false."); + } + else if (addToScene) + { + // The shared lookup, not GameObject.Find: Find skips inactive objects and + // picks one of several with the same name without saying so. + var matches = GameObjectLookup.SearchGameObjects("by_name", sceneTarget, includeInactive: true); + var go = matches.Count == 1 ? GameObjectLookup.FindById(matches[0]) : null; + if (matches.Count > 1) + { + diagnostics.AddError("SCENE_TARGET_AMBIGUOUS", + $"{matches.Count} GameObjects are named '{sceneTarget}'; nothing was attached.", + "Rename the target or pick a unique name."); + } + else if (go != null) + { + var asset = AssetDatabase.LoadAssetAtPath(controller.path); + if (asset != null) + { + // `??` compares references and never sees Unity's overloaded ==, so + // AddComponent was skipped and the next line threw. Measured: this path + // never once worked. + var animator = go.GetComponent(); + if (animator == null) + { + UnityEditor.Undo.RecordObject(go, "Add Animator Component"); + animator = UnityEditor.Undo.AddComponent(go); + } + // The clips bind to SpriteRenderer.m_Sprite: without one the Animator + // plays into nothing and the call still reports success. + if (go.GetComponent() == null) + { + UnityEditor.Undo.AddComponent(go); + diagnostics.AddWarning("SCENE_SPRITE_RENDERER_ADDED", + $"'{sceneTarget}' had no SpriteRenderer, so one was added for the clips to drive."); + } + // Recorded and dirtied like the sibling controller_assign path. + UnityEditor.Undo.RecordObject(animator, "Assign AnimatorController"); + animator.runtimeAnimatorController = asset; + EditorUtility.SetDirty(go); + } + else + { + diagnostics.AddError("SCENE_CONTROLLER_NOT_LOADED", + $"The controller at '{controller.path}' could not be loaded, so '{sceneTarget}' was left unchanged.", + "Check the controller_path in the response."); + } + } + else + { + diagnostics.AddError("SCENE_TARGET_NOT_FOUND", + $"GameObject '{sceneTarget}' not found in scene.", + "Check GameObject name or open the correct scene first."); + } + } + + // Shaped like the other three steps' refusals rather than like a success with the + // flag flipped: a step-4 failure used to be the one refusal in the tool carrying + // neither 'step' nor 'message', leaving the reason only inside the diagnostics + // array. The asset fields stay on it, because by this point steps 1-3 have written + // and the caller needs to know what is already on disk. + if (diagnostics.HasErrors) + return new + { + success = false, + step = "add_to_scene", + message = diagnostics.FirstError, + sprite_path = path, + controller_path = controller.path, + state_count = controller.stateCount, + clip_count = clips.Count, + diagnostics = diagnostics.Build(), + }; + + return new + { + success = true, + sprite_path = path, + controller_path = controller.path, + state_count = controller.stateCount, + clip_count = clips.Count, + diagnostics = diagnostics.Build(), + }; + } + + private static object Stop(string step, SpriteDiagnosticBuilder diagnostics) => + new { success = false, step, message = diagnostics.FirstError, diagnostics = diagnostics.Build() }; + + private static int GetSliceCount(string path) + { + int count = AssetDatabase.LoadAllAssetsAtPath(path).OfType().Count(); + return count > 0 ? count : 1; + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs.meta new file mode 100644 index 000000000..d72bd8bf3 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteFullSetup.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 2a02bdfbdc3b049aa81ab404b6e48590 \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs new file mode 100644 index 000000000..2ad77d602 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs @@ -0,0 +1,340 @@ +using System; +using System.IO; +using System.Linq; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; +using MCPForUnity.Editor.Helpers; +// TextureImporter.spritesheet is obsolete as of Unity 6, but the replacement +// (ISpriteEditorDataProvider) needs the 2D Sprite package for the same result. +#pragma warning disable CS0618 + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal static class SpriteImportSetup + { + // ── GetInfo ────────────────────────────────────────────────────────── + + public static object GetInfo(JObject @params, SpriteDiagnosticBuilder diagnostics) + { + if (!SpriteParams.TryReadAssetPath(@params, "path", out string path, out string pathError)) + return diagnostics.Fail("BAD_PARAM", pathError); + var importer = AssetImporter.GetAtPath(path) as TextureImporter; + if (importer == null) + return diagnostics.Fail("NOT_FOUND", $"No TextureImporter found at '{path}'. Is it a texture/sprite?"); + + var texture = AssetDatabase.LoadAssetAtPath(path); + int w = texture != null ? texture.width : 0; + int h = texture != null ? texture.height : 0; + + // Paged because this reads what is already on the asset: the 4096 ceiling + // slice_sheet applies when WRITING never bounded a sheet sliced by hand. + // Changing either number means changing the page_size description in + // Server/src/services/tools/manage_sprite.py - that copy is the published promise. + const int DefaultSlicePageSize = 512; + const int MaxSlicePageSize = 4096; + + if (!SpriteParams.TryReadWholeNumber(@params, "page_size", DefaultSlicePageSize, out int pageSize, out string paramError)) + return diagnostics.Fail("BAD_PARAM", paramError); + if (pageSize < 1 || pageSize > MaxSlicePageSize) + return diagnostics.Fail("BAD_PARAM", $"'page_size' must be between 1 and {MaxSlicePageSize}; got {pageSize}."); + + int totalSlices = importer.spritesheet.Length; + if (!SpriteParams.TryReadWholeNumber(@params, "cursor", 0, out int cursor, out paramError)) + return diagnostics.Fail("BAD_PARAM", paramError); + // Skip yields everything for a negative count rather than throwing, so a negative + // cursor would return page one as a success. Landing exactly on totalSlices is + // legal: it is the end, and cursor 0 on an unsliced sheet is that same case. + if (cursor < 0 || cursor > totalSlices) + return diagnostics.Fail("BAD_PARAM", $"'cursor' must be between 0 and {totalSlices}; got {cursor}."); + + var existingSlices = importer.spritesheet.Skip(cursor).Take(pageSize).Select(s => new + { + name = s.name, + x = (int)s.rect.x, + y = (int)s.rect.y, + width = (int)s.rect.width, + height = (int)s.rect.height, + }).ToArray(); + + int nextIndex = cursor + existingSlices.Length; + int? nextCursor = nextIndex < totalSlices ? nextIndex : (int?)null; + + // Base64 payload so a vision-capable caller can read the grid off the image. + // Bounded by size rather than paged: an image split across cursors is not an image + // any client can reassemble. 4 MB is a budget, not a protocol boundary. The bound + // is on the ENCODED length - base64 emits 4 chars per 3 bytes, and bounding the + // source instead let a measured 3.67 MB sheet through as a 4.89 MB payload. + const int MaxInlinePayloadBytes = 4 * 1024 * 1024; + string imageBase64 = null; + string imageOmittedReason = null; + if (cursor > 0) + { + // First page only: repeating it would multiply what paging exists to cap. + imageOmittedReason = + "The image is returned only on the first page. Request this path with " + + "cursor 0 (or omit cursor) if the image itself is needed."; + } + else + { + try + { + // Not dataPath.Replace("/Assets", ""): Replace removes EVERY occurrence, so + // a project under /work/AssetsLab lost the wrong segment and missed the file. + string projectRoot = Directory.GetParent(Application.dataPath)?.FullName; + string fullPath = projectRoot != null ? Path.Combine(projectRoot, path) : null; + if (fullPath == null) + { + imageOmittedReason = "The project root could not be resolved from Application.dataPath."; + } + else if (!File.Exists(fullPath)) + { + // Asset path over the bridge, absolute path to the log only: the caller + // gains nothing from it and it discloses the machine's directory layout. + McpLog.Warn($"[Sprite2D] get_info found no file on disk at '{fullPath}'."); + imageOmittedReason = $"No file on disk for '{path}'."; + } + else + { + string ext = Path.GetExtension(path).ToLowerInvariant(); + string mime = (ext == ".jpg" || ext == ".jpeg") ? "image/jpeg" : "image/png"; + string prefix = $"data:{mime};base64,"; + long size = new FileInfo(fullPath).Length; + long encoded = 4L * ((size + 2) / 3) + prefix.Length; + if (encoded > MaxInlinePayloadBytes) + { + imageOmittedReason = + $"The {size}-byte source encodes to {encoded} base64 bytes, above the " + + $"{MaxInlinePayloadBytes}-byte inline limit. Read the file directly if the " + + "image itself is needed."; + } + else + { + imageBase64 = prefix + Convert.ToBase64String(File.ReadAllBytes(fullPath)); + } + } + } + catch (Exception ex) + { + // A swallowed failure and a deliberate omission are different answers. + // Type over the bridge, not message: messages carry the path that threw. + McpLog.Warn($"[Sprite2D] get_info could not read '{path}': {ex}"); + imageOmittedReason = + $"The image could not be read ({ex.GetType().Name}); the Unity console has the detail."; + } + } + + return new + { + success = true, + path, + width = w, + height = h, + sprite_mode = importer.spriteImportMode.ToString(), + pixels_per_unit = importer.spritePixelsPerUnit, + filter_mode = importer.filterMode.ToString(), + slice_count = totalSlices, + slices = existingSlices, + next_cursor = nextCursor, + image_base64 = imageBase64, + image_omitted_reason = imageOmittedReason, + }; + } + + /// Undoes the conversion above when the request is refused after it. + private static void RestoreTextureType(TextureImporter importer, TextureImporterType previous) + { + if (importer.textureType == previous) return; + importer.textureType = previous; + EditorUtility.SetDirty(importer); + importer.SaveAndReimport(); + } + + // ── SliceSheet ─────────────────────────────────────────────────────── + + public static object SliceSheet(JObject @params, SpriteDiagnosticBuilder diagnostics) + { + if (!SpriteParams.TryReadAssetPath(@params, "path", out string path, out string pathError)) + return diagnostics.Fail("BAD_PARAM", pathError); + var importer = AssetImporter.GetAtPath(path) as TextureImporter; + if (importer == null) + return diagnostics.Fail("NOT_FOUND", $"No TextureImporter found at '{path}'."); + + // Checked before the conversion below: a refused request used to leave the texture + // already turned into a Sprite. Sequential rather than chained with ||, because a + // short-circuited call leaves its out parameter unassigned. + int rows = 0, frameW = 0, frameH = 0; + bool gridOk = SpriteParams.TryReadWholeNumber(@params, "cols", 0, out int cols, out string gridError); + if (gridOk) gridOk = SpriteParams.TryReadWholeNumber(@params, "rows", 0, out rows, out gridError); + if (gridOk) gridOk = SpriteParams.TryReadWholeNumber(@params, "frame_width", 0, out frameW, out gridError); + if (gridOk) gridOk = SpriteParams.TryReadWholeNumber(@params, "frame_height", 0, out frameH, out gridError); + if (!gridOk) + return diagnostics.Fail("BAD_PARAM", gridError); + + if (cols < 0 || rows < 0 || frameW < 0 || frameH < 0) + return diagnostics.Fail("BAD_PARAM", "'cols', 'rows', 'frame_width' and 'frame_height' cannot be negative."); + + if (cols <= 0 && frameW <= 0) + return diagnostics.Fail("BAD_PARAM", "Either 'cols' (1 or more) or 'frame_width' is required."); + + // Like cols, rows=0 means "derive it from frame_height"; an explicit 0 with nothing + // to derive from would reach the texH / rows division below and throw. An absent + // rows means one row unless frame_height is there to derive it from. + bool rowsGiven = @params["rows"] != null && @params["rows"].Type != JTokenType.Null; + if (rowsGiven && rows == 0 && frameH <= 0) + return diagnostics.Fail("BAD_PARAM", "'rows' must be 1 or more; pass 'frame_height' instead if the row count is unknown."); + if (!rowsGiven && frameH <= 0) + rows = 1; + + // Measure only once imported as a sprite sheet: a Default-type import rescales a + // non-power-of-two sheet (96px to 128px) and the trailing frames then land outside + // the real texture, where Unity drops them silently - measured on 6000.4.4f1, a + // 96x16 sheet asked for 6 columns gave 4 sprites of 21px. Later refusals restore + // the previous type: a refused request must not leave a converted texture behind. + var previousType = importer.textureType; + try + { + // npotScale as well as the type: Unity refuses sprite generation outright on a + // non-power-of-two texture that carries NPOT scaling ("Sprites can not be + // generated from textures with NPOT scaling"), and the refusal is a console + // message, not an exception - measured on 2021.3.45f2, a sheet already typed + // Sprite skipped this block entirely, wrote its metadata, and reported six + // frames with nothing on the asset. Sprite-mode textures cannot use NPOT + // scaling at all, so clearing it takes nothing away. + if (importer.textureType != TextureImporterType.Sprite + || importer.npotScale != TextureImporterNPOTScale.None) + { + importer.textureType = TextureImporterType.Sprite; + importer.npotScale = TextureImporterNPOTScale.None; + EditorUtility.SetDirty(importer); + importer.SaveAndReimport(); + } + return SliceConverted(@params, diagnostics, path, importer, previousType, cols, rows, frameW, frameH); + } + catch + { + // A restore that throws must not replace the exception that caused it. + try { RestoreTextureType(importer, previousType); } + catch (Exception restoreError) { McpLog.Error($"[ManageSprite] Could not restore the importer type of '{path}': {restoreError.Message}"); } + throw; + } + } + + private static object SliceConverted(JObject @params, SpriteDiagnosticBuilder diagnostics, string path, + TextureImporter importer, TextureImporterType previousType, + int cols, int rows, int frameW, int frameH) + { + var texture = AssetDatabase.LoadAssetAtPath(path); + if (texture == null) + { + RestoreTextureType(importer, previousType); + return diagnostics.Fail("NOT_FOUND", $"Could not load texture at '{path}'."); + } + + int texW = texture.width; + int texH = texture.height; + + if (frameW <= 0) frameW = texW / cols; + if (frameH <= 0) frameH = texH / rows; + if (cols <= 0) cols = texW / frameW; + if (rows <= 0) rows = texH / frameH; + + // Three ways to fail, only the first obvious. An oversized frame yields a non-zero + // grid whose rects land outside the texture (measured: frame_height=4096 on a 16px + // sheet, dropped silently, success). Integer division can drive a derived frame size + // to zero (measured: 64 zero-width sprites, success). The product is long because + // two large caller values wrap in 32-bit arithmetic and slip under the comparison. + if (frameW <= 0 || frameH <= 0 + || (long)cols * frameW > texW || (long)rows * frameH > texH) + { + RestoreTextureType(importer, previousType); + return diagnostics.Fail("SLICE_OUT_OF_BOUNDS", + $"A {cols}x{rows} grid of {frameW}x{frameH} frames does not fit inside the {texW}x{texH} texture, so some frames would fall outside it.", + "Reduce frame_width/frame_height, or cols/rows", "Confirm the texture dimensions with get_info"); + } + + // Fitting is not covering: the guard above only refuses a grid that is too BIG. + // Measured on 6000.4.4f1 - a 100x16 sheet at 6 columns covered 96 of 100 pixels, + // success, no diagnostic. Warns rather than refuses because a remainder is often + // deliberate (a trailing margin, a separator, an intentional sub-region); silence + // was the defect, not the behaviour. + int uncoveredW = texW - cols * frameW; + int uncoveredH = texH - rows * frameH; + if (uncoveredW > 0 || uncoveredH > 0) + diagnostics.AddWarning("SLICE_GRID_REMAINDER", + $"The grid covers {cols * frameW}x{rows * frameH} of a {texW}x{texH} texture, leaving {uncoveredW}px on the right and {uncoveredH}px at the bottom unused.", + "Deliberate if the sheet has a margin or a separator", "Otherwise check cols/rows against the texture size with get_info"); + + // Every frame is allocated and reimported in one call, so this is a precaution + // rather than a reproduction; far above any real sheet, it catches a cols/rows typo. + const int MaxFrames = 4096; + long totalFrames = (long)cols * rows; + if (totalFrames > MaxFrames) + { + RestoreTextureType(importer, previousType); + return diagnostics.Fail("SLICE_TOO_MANY_FRAMES", + $"The grid works out to {totalFrames} frames, above the {MaxFrames}-frame limit.", + "Increase frame_width/frame_height", "Slice the sheet in smaller pieces"); + } + + if (totalFrames == 0) + { + RestoreTextureType(importer, previousType); + return diagnostics.Fail("SLICE_EMPTY", + $"A {cols}x{rows} grid works out to 0 frames - cols/rows or the frame size is wrong.", + "Check the cols and rows values", "Confirm the texture dimensions with get_info"); + } + + string baseName = @params["base_name"]?.ToString() + ?? Path.GetFileNameWithoutExtension(path); + + var metas = new SpriteMetaData[(int)totalFrames]; + for (int r = 0; r < rows; r++) + { + for (int c = 0; c < cols; c++) + { + int i = r * cols + c; + metas[i] = new SpriteMetaData + { + name = $"{baseName}_{i}", + rect = new Rect(c * frameW, texH - (r + 1) * frameH, frameW, frameH), + pivot = new Vector2(0.5f, 0.5f), + alignment = 0, + }; + } + } + + importer.spriteImportMode = SpriteImportMode.Multiple; + importer.spritesheet = metas; + importer.filterMode = FilterMode.Point; // pixel-perfect default + // Assigning spritesheet on an already-Multiple importer does not mark it dirty, so + // SaveAndReimport would restore the old grid - measured, a second slice did nothing. + EditorUtility.SetDirty(importer); + importer.SaveAndReimport(); + + // Unity can accept every SpriteMetaData entry and still emit no sprite for it, and + // it says so in the console rather than throwing. NPOT scaling was one such path + // and is closed above; an import that fails for any other reason would report the + // same success over an empty asset. Counting what is actually on the asset is the + // only answer that does not depend on knowing the causes in advance. + int generated = AssetDatabase.LoadAllAssetsAtPath(path).OfType().Count(); + if (generated != totalFrames) + return diagnostics.Fail("SLICE_NOT_GENERATED", + $"Unity accepted a {cols}x{rows} grid but generated {generated} of {totalFrames} sprites for '{path}'.", + "Check the Unity console for the import error", + "Confirm the texture's import settings allow sprite generation"); + + return new + { + success = true, + path, + cols, + rows, + frame_width = frameW, + frame_height = frameH, + total_frames = totalFrames, + diagnostics = diagnostics.Build(), + }; + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs.meta new file mode 100644 index 000000000..88b6df14b --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteImportSetup.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 9f82d8c42bec5436fb4bdef16db29f93 \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs new file mode 100644 index 000000000..736d61c14 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs @@ -0,0 +1,118 @@ +using System.Collections.Generic; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + internal enum SpriteAnimCategory + { + Idle, + Locomotion, // walk or run: a candidate for a 1D blend tree. + Jump, + Combat, // attack, slash, combo and the like: a trigger state. + Object, // open, close, activate: a single state. + Generic, + } + + internal class SpriteAnimEntry + { + public string ClipName; + public SpriteAnimCategory Category; + public bool Loop; + public string TriggerName; + public float BlendValue; // Position on the 1D blend tree: walk=1, run=2. + } + + internal static class SpriteNamingDetector + { + public static SpriteAnimEntry Detect(string clipName) + { + var entry = new SpriteAnimEntry { ClipName = clipName }; + // The raw name, not a lowercased one: Words splits camelCase on char.IsUpper, so + // 'heroAttack' pre-lowered collapsed to 'heroattack' and lost its Attack trigger. + Categorize(clipName, entry); + entry.Loop = AutoDetectLoop(entry.Category); + return entry; + } + + // ── Private ────────────────────────────────────────────────────────── + + private static void Categorize(string name, SpriteAnimEntry entry) + { + var words = Words(name); + + if (Has(words, "idle", "stand")) + { entry.Category = SpriteAnimCategory.Idle; return; } + + if (words.Contains("walk")) + { entry.Category = SpriteAnimCategory.Locomotion; entry.BlendValue = 1f; return; } + + if (Has(words, "run", "sprint")) + { entry.Category = SpriteAnimCategory.Locomotion; entry.BlendValue = 2f; return; } + + string hit = Match(words, "jump", "fall", "land"); + if (hit != null) + { entry.Category = SpriteAnimCategory.Jump; entry.TriggerName = Capitalize(hit); return; } + + hit = Match(words, "attack", "slash", "punch", "combo", "cast", "shoot"); + if (hit != null) + { entry.Category = SpriteAnimCategory.Combat; entry.TriggerName = Capitalize(hit); return; } + + hit = Match(words, "open", "close", "activate", "die", "death", "hurt", "hit"); + if (hit != null) + { entry.Category = SpriteAnimCategory.Object; entry.TriggerName = Capitalize(hit); return; } + + entry.Category = SpriteAnimCategory.Generic; + entry.TriggerName = Capitalize(name.ToLowerInvariant()); + } + + /// + /// The words in a clip name, split on separators, camelCase humps and letter/digit + /// boundaries. Raw substring matching instead files 'white_flash' under 'hit' and + /// 'drunk_walk' under 'run', shaping the controller around the wrong category. + /// + private static HashSet Words(string name) + { + var words = new HashSet(); + var word = new System.Text.StringBuilder(); + + for (int i = 0; i < name.Length; i++) + { + char c = name[i]; + bool breaks = !char.IsLetterOrDigit(c) + || (i > 0 && char.IsUpper(c) && char.IsLower(name[i - 1])) + // End of an acronym: 'heroXMLAttack' has no lower-to-upper boundary at the + // 'A', so the tail read as 'xmlattack' and lost its keyword. + || (i > 0 && i + 1 < name.Length + && char.IsUpper(c) && char.IsUpper(name[i - 1]) && char.IsLower(name[i + 1])) + || (i > 0 && char.IsDigit(c) && char.IsLetter(name[i - 1])) + || (i > 0 && char.IsLetter(c) && char.IsDigit(name[i - 1])); + + if (breaks && word.Length > 0) + { + words.Add(word.ToString().ToLowerInvariant()); + word.Clear(); + } + if (char.IsLetterOrDigit(c)) word.Append(c); + } + if (word.Length > 0) words.Add(word.ToString().ToLowerInvariant()); + + return words; + } + + private static bool Has(HashSet words, params string[] keys) => + Match(words, keys) != null; + + /// The first key the name contains, so the trigger is named after the action. + private static string Match(HashSet words, params string[] keys) + { + foreach (string k in keys) + if (words.Contains(k)) return k; + return null; + } + + private static bool AutoDetectLoop(SpriteAnimCategory cat) => + cat == SpriteAnimCategory.Idle || cat == SpriteAnimCategory.Locomotion; + + private static string Capitalize(string s) => + string.IsNullOrEmpty(s) ? s : char.ToUpperInvariant(s[0]) + s.Substring(1); + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs.meta new file mode 100644 index 000000000..53d29cdac --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteNamingDetector.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 59e42167a60fa4590b40c0c6edaca5f9 \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs b/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs new file mode 100644 index 000000000..e4f3ed5f5 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs @@ -0,0 +1,156 @@ +using System; +using Newtonsoft.Json.Linq; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnity.Editor.Tools.Sprite2D +{ + /// + /// Reads the sprite tool's numeric parameters without throwing and without rounding. + /// ToObject<T> does both; measured through the live tool 2026-08-21: an out-of-int + /// grid value or start_frame threw OverflowException (a transport failure, not a named + /// refusal), start_frame 2.7 silently became 3, and fps NaN wrote a clip with NaN times. + /// Shared rather than private because the same parameters are read in three places. + /// + internal static class SpriteParams + { + internal static bool TryReadAssetPath(JObject @params, string key, out string path, out string error) + { + error = null; + path = @params[key]?.ToString(); + if (string.IsNullOrEmpty(path)) + { + error = $"'{key}' is required."; + return false; + } + + path = AssetPathUtility.SanitizeAssetPath(path); + if (path == null) + { + error = $"'{key}' must stay under Assets/ and cannot contain '..'."; + return false; + } + if (path != "Assets" && !AssetPathUtility.IsValidAssetPath(path)) + { + error = $"'{key}' contains a character that is not allowed in an asset path."; + return false; + } + return true; + } + + /// + /// Reads an optional whole number. Returns false with a caller-facing reason when + /// the value is present but is not a whole number an int can hold. + /// + internal static bool TryReadWholeNumber(JObject @params, string key, int fallback, + out int value, out string error) + { + value = fallback; + if (!ParamCoercion.ValidateIntegerField(@params, key, out error)) + { + error = $"'{key}' {error}."; + return false; + } + + JToken token = @params[key]; + if (token == null || token.Type == JTokenType.Null) + return true; + + long raw; + try + { + raw = token.Value(); + } + catch (Exception) + { + // Too large for long parses as a BigInteger, still typed Integer, and throws. + error = $"'{key}' must fit in a 32-bit integer."; + return false; + } + + if (raw < int.MinValue || raw > int.MaxValue) + { + // Worded unlike the range guards elsewhere on purpose: with shared wording a + // test still passed while the cast wrapped and a LATER guard did the refusing + // (measured on the page_size and cols tests, which both did). + error = $"'{key}' must fit in a 32-bit integer; got {raw}."; + return false; + } + + value = (int)raw; + return true; + } + + /// + /// Reads an optional flag. Needed for `loop`, which hides inside the untyped `clips` + /// array where nothing above C# validates it - measured 2026-08-21: ToObject<bool?> + /// threw on `loop: "maybe"` and silently accepted `loop: 2`. + /// + internal static bool TryReadBool(JObject @params, string key, bool fallback, + out bool value, out string error) + { + value = fallback; + error = null; + + JToken token = @params[key]; + if (token == null || token.Type == JTokenType.Null) + return true; + + bool? parsed = ParamCoercion.CoerceBoolNullable(token); + if (parsed == null) + { + error = $"'{key}' must be true or false; got {token.Type.ToString().ToLowerInvariant()}."; + return false; + } + + value = parsed.Value; + return true; + } + + /// + /// Reads an optional rate. NaN and the infinities pass every comparison-based guard + /// (`fps <= 0f` is false for NaN), so a NaN rate reached the keyframe arithmetic + /// and wrote a clip whose frame times were all NaN. + /// + internal static bool TryReadFiniteFloat(JObject @params, string key, float fallback, + out float value, out string error) + { + value = fallback; + if (!ParamCoercion.ValidateNumericField(@params, key, out error)) + { + error = $"'{key}' {error}."; + return false; + } + + JToken token = @params[key]; + if (token == null || token.Type == JTokenType.Null) + return true; + + double raw; + try + { + raw = token.Value(); + } + catch (Exception) + { + error = $"'{key}' is out of range for a number."; + return false; + } + + if (double.IsNaN(raw) || double.IsInfinity(raw)) + { + error = $"'{key}' must be a finite number."; + return false; + } + + // Read as double first: the cast would silently make an infinity of this. + if (raw > float.MaxValue || raw < -float.MaxValue) + { + error = $"'{key}' is out of range for a 32-bit float."; + return false; + } + + value = (float)raw; + return true; + } + } +} diff --git a/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs.meta b/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs.meta new file mode 100644 index 000000000..a6302d408 --- /dev/null +++ b/MCPForUnity/Editor/Tools/Sprite2D/SpriteParams.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: e8816110e93149fcb381bf3b9a01ba40 diff --git a/Server/src/services/tools/manage_sprite.py b/Server/src/services/tools/manage_sprite.py new file mode 100644 index 000000000..45b26d806 --- /dev/null +++ b/Server/src/services/tools/manage_sprite.py @@ -0,0 +1,153 @@ +""" +2D sprite animation tool. +Automates: sprite sheet slicing, AnimationClip creation from sliced frames, +and AnimatorController generation. +""" +from typing import Annotated, Any, Literal, get_args + +from fastmcp import Context +from mcp.types import ToolAnnotations + +from services.registry import mcp_for_unity_tool +from services.tools import get_unity_instance_from_context +from transport.unity_transport import send_with_unity_instance +from transport.legacy.unity_connection import async_send_command_with_retry + +SpriteAction = Literal["get_info", "slice_sheet", "setup_clips", "setup_controller", "full_setup"] + +VALID_ACTIONS: list[str] = list(get_args(SpriteAction)) + + +@mcp_for_unity_tool( + group="animation", + description=( + "2D sprite animation tool. " + "get_info: read sprite import settings + return image for vision analysis; " + "the slice list is paged (page_size / cursor). " + "slice_sheet: apply grid slicing to a sprite sheet. " + "setup_clips: create AnimationClips from sliced sprites. " + "setup_controller: build AnimatorController with smart complexity (1D blend tree for locomotion, " + "trigger states for combat, simple state for single animations). " + "full_setup: one command — slice → clips → controller." + ), + annotations=ToolAnnotations( + title="Manage Sprite", + destructiveHint=True, + ), +) +async def manage_sprite( + ctx: Context, + action: Annotated[SpriteAction, "Action to perform."], + path: Annotated[ + str | None, + "Sprite texture asset path (e.g. 'Assets/Sprites/hero_walk.png'). Required for get_info, slice_sheet, setup_clips, full_setup.", + ] = None, + cols: Annotated[ + int | None, + "Number of columns in the sprite sheet grid. Used by slice_sheet and full_setup.", + ] = None, + rows: Annotated[ + int | None, + "Number of rows in the sprite sheet grid. Default: 1.", + ] = None, + frame_width: Annotated[ + int | None, + "Frame width in pixels. Alternative to cols.", + ] = None, + frame_height: Annotated[ + int | None, + "Frame height in pixels. Alternative to rows.", + ] = None, + base_name: Annotated[ + str | None, + "Base name for sliced sprite frames (default: texture filename).", + ] = None, + clips: Annotated[ + list[dict[str, Any]] | None, + "Clip definitions: [{name, start_frame, end_frame, fps (default 12), loop (auto-detect if omitted)}]. " + "For setup_controller: [{name, path}] where path is the .anim asset path.", + ] = None, + animation_name: Annotated[ + str | None, + "Animation name for full_setup when clips are not specified (all frames = one clip).", + ] = None, + output_dir: Annotated[ + str | None, + "Output directory for .anim and .controller assets (default: same folder as sprite).", + ] = None, + controller_path: Annotated[ + str | None, + "Path for the .controller asset (e.g. 'Assets/Animators/Hero.controller').", + ] = None, + overwrite: Annotated[ + bool, + "Replace an existing .anim or .controller at the target path. Off by default: " + "without it an existing asset is kept and reported back, not silently replaced.", + ] = False, + add_to_scene: Annotated[bool, "Attach Animator + controller to a scene GameObject."] = False, + scene_target: Annotated[ + str | None, + "Existing GameObject name to attach Animator to.", + ] = None, + # The numbers below are documentation, not enforcement: SpriteParams and + # SpriteImportSetup.GetInfo are what actually refuse an out-of-range page_size, and + # this text is what the generated reference publishes to callers. Two copies, so + # changing the C# bounds means changing this line in the same commit. + page_size: Annotated[ + int | None, + "get_info: how many entries of the 'slices' list to return (1-4096, default 512). " + "A sheet sliced by hand can hold more slices than one response should carry.", + ] = None, + cursor: Annotated[ + int | None, + "get_info: index to start the 'slices' page at. Pass back the 'next_cursor' from " + "the previous response; absent next_cursor means the list is finished. The image " + "is returned only on the first page.", + ] = None, +) -> dict[str, Any]: + """2D sprite animation tool.""" + + action_lower = action.lower() if action else "" + + if action_lower not in VALID_ACTIONS: + return { + "success": False, + "message": f"Unknown action '{action}'. Valid: {', '.join(VALID_ACTIONS)}", + } + + # Python-side validation + if action_lower in ("get_info", "slice_sheet", "setup_clips", "full_setup") and not path: + return {"success": False, "message": f"'path' is required for action '{action}'."} + + if action_lower in ("slice_sheet", "full_setup") and cols is None and frame_width is None: + return {"success": False, "message": f"'cols' or 'frame_width' is required for '{action}'. " + "Use get_info first to retrieve image_base64, analyze the grid visually, then call full_setup with cols/rows."} + + if action_lower == "setup_controller" and not controller_path: + return {"success": False, "message": "'controller_path' is required for setup_controller (e.g. 'Assets/Animators/Hero.controller')."} + + unity_instance = await get_unity_instance_from_context(ctx) + + # `or None` on the two flags, so a False is dropped rather than sent: the C# side + # reads a missing key as the default, and forwarding every argument buries the real + # ones in nulls on the wire. + optional = { + "path": path, "cols": cols, "rows": rows, + "frame_width": frame_width, "frame_height": frame_height, + "base_name": base_name, "clips": clips, + "animation_name": animation_name, "output_dir": output_dir, + "controller_path": controller_path, "page_size": page_size, + "cursor": cursor, "scene_target": scene_target, + "overwrite": overwrite or None, "add_to_scene": add_to_scene or None, + } + + params: dict[str, Any] = {"action": action_lower} + params.update({k: v for k, v in optional.items() if v is not None}) + + result = await send_with_unity_instance( + async_send_command_with_retry, + unity_instance, + "manage_sprite", + params, + ) + return result if isinstance(result, dict) else {"success": False, "message": str(result)} diff --git a/Server/tests/test_manage_sprite.py b/Server/tests/test_manage_sprite.py new file mode 100644 index 000000000..68b4a88fb --- /dev/null +++ b/Server/tests/test_manage_sprite.py @@ -0,0 +1,160 @@ +"""Tests for the manage_sprite tool. + +These cover the Python side only: the action list and the argument checks that run +before anything is sent to Unity. The behaviour of the slicing, clip and controller +builders is covered by the EditMode tests in TestProjects, because it only means +anything against a real AssetDatabase. +""" +import asyncio +import inspect +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from services.tools.manage_sprite import VALID_ACTIONS, manage_sprite + + +@pytest.fixture +def mock_unity(monkeypatch): + captured = {} + + async def fake_send(send_fn, unity_instance, tool_name, params): + captured["params"] = params + captured["calls"] = captured.get("calls", 0) + 1 + return {"success": True} + + monkeypatch.setattr( + "services.tools.manage_sprite.get_unity_instance_from_context", + AsyncMock(return_value=None), + ) + monkeypatch.setattr("services.tools.manage_sprite.send_with_unity_instance", fake_send) + return captured + + +def call(**kwargs): + return asyncio.run(manage_sprite(SimpleNamespace(), **kwargs)) + + +def test_actions_are_the_documented_five(): + assert set(VALID_ACTIONS) == { + "get_info", "slice_sheet", "setup_clips", + "setup_controller", "full_setup", + } + + +class TestManageSpriteValidation: + """Every case here must fail before a Unity round-trip is attempted.""" + + def test_unknown_action_returns_error(self): + result = call(action="nonexistent") + assert result["success"] is False + # The message has to name the alternatives, or the caller has nowhere to go. + assert "get_info" in result["message"] + + @pytest.mark.parametrize("action", ["get_info", "slice_sheet", "setup_clips", "full_setup"]) + def test_path_is_required(self, action): + result = call(action=action, path=None) + assert result["success"] is False + assert "path" in result["message"] + + @pytest.mark.parametrize("action", ["slice_sheet", "full_setup"]) + def test_cols_or_frame_width_is_required(self, action): + result = call(action=action, path="Assets/hero.png") + assert result["success"] is False + # Asserting on success alone would pass for the wrong reason: with the check + # removed the call reaches an absent Unity and fails there instead. + assert "cols" in result["message"] + + def test_slice_sheet_accepts_frame_width_instead_of_cols(self, mock_unity): + # frame_width is the documented alternative to cols; rejecting it would make + # the error message above a lie. + result = call(action="slice_sheet", path="Assets/hero.png", frame_width=32) + assert result["success"] is True + assert mock_unity["calls"] == 1 + + def test_explicit_zero_cols_is_forwarded_not_reported_missing(self, mock_unity): + # `not cols` read an explicit 0 as an omitted parameter and told the caller to + # supply what they had supplied; the C# side is the one that names a bad value. + call(action="slice_sheet", path="Assets/hero.png", cols=0) + assert mock_unity["calls"] == 1 + assert mock_unity["params"]["cols"] == 0 + + def test_setup_controller_requires_controller_path(self): + result = call(action="setup_controller", clips=[{"name": "walk", "path": "a.anim"}]) + assert result["success"] is False + assert "controller_path" in result["message"] + + +class TestParameterForwarding: + def test_a_bridge_reply_that_is_not_a_dict_becomes_a_failure(self, monkeypatch): + async def fake_send(send_fn, unity_instance, tool_name, params): + return "connection dropped" + + monkeypatch.setattr( + "services.tools.manage_sprite.get_unity_instance_from_context", + AsyncMock(return_value=None), + ) + monkeypatch.setattr("services.tools.manage_sprite.send_with_unity_instance", fake_send) + + result = call(action="get_info", path="Assets/hero.png") + assert result == {"success": False, "message": "connection dropped"} + + def test_only_supplied_parameters_are_forwarded(self, mock_unity): + """Unset optional arguments must not reach Unity as nulls. + + C# reads a forwarded null and a missing key the same way, so this is about + keeping the wire readable rather than about a broken call. + """ + call(action="slice_sheet", path="Assets/hero.png", cols=4) + assert mock_unity["params"] == {"action": "slice_sheet", "path": "Assets/hero.png", "cols": 4} + + def test_paging_arguments_reach_unity_only_when_asked_for(self, mock_unity): + """page_size and cursor are get_info's, and absent means "use the default".""" + call(action="get_info", path="Assets/atlas.png") + plain = mock_unity["params"] + + call(action="get_info", path="Assets/atlas.png", page_size=100, cursor=200) + paged = mock_unity["params"] + + assert plain == {"action": "get_info", "path": "Assets/atlas.png"} + assert paged["page_size"] == 100 + assert paged["cursor"] == 200 + + def test_every_optional_argument_has_a_forwarding_branch(self, mock_unity): + """A parameter accepted at the surface but dropped before the bridge is silent. + + Limitation: this drives every parameter through one action, so it assumes the + forwarder stays action-agnostic. Scoping any entry to its own action means + scoping this test with it. + """ + fn = getattr(manage_sprite, "fn", manage_sprite) + # A value each parameter's annotation accepts. Booleans must be True: the + # forwarder deliberately omits a False flag, so False would look like a + # dropped branch and this guard would cry wolf. + sample = { + "path": "Assets/a.png", "cols": 1, "rows": 1, "frame_width": 1, + "frame_height": 1, "base_name": "b", "clips": [{"name": "walk"}], + "animation_name": "walk", "output_dir": "Assets/out", + "controller_path": "Assets/a.controller", "overwrite": True, + "add_to_scene": True, "scene_target": "Hero", "page_size": 1, "cursor": 1, + } + optional = [ + name for name, prm in inspect.signature(fn).parameters.items() + if name not in ("ctx", "action") and prm.default is not inspect.Parameter.empty + ] + missing_sample = [n for n in optional if n not in sample] + assert not missing_sample, ( + f"this test has no sample value for {missing_sample}; add one rather than " + "narrowing the guard" + ) + + call(action="full_setup", **{k: sample[k] for k in optional}) + + forwarded = mock_unity["params"] + dropped = [n for n in optional if n not in forwarded] + assert not dropped, f"accepted at the surface but never sent to Unity: {dropped}" + # The value too, not only the key. An audit reproduced a branch that kept the key + # and replaced the caller's value; membership alone stayed green for it. + changed = {n: (sample[n], forwarded[n]) for n in optional if forwarded[n] != sample[n]} + assert not changed, f"forwarded under a different value than the caller sent: {changed}" diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs new file mode 100644 index 000000000..d9c41dc3a --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs @@ -0,0 +1,1534 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using UnityEditor; +using UnityEditor.Animations; +using UnityEngine; +using MCPForUnity.Editor.Tools.Sprite2D; +using static MCPForUnityTests.Editor.TestUtilities; + +namespace MCPForUnityTests.Editor.Tools +{ + public class ManageSpriteTests + { + private const string TempRoot = "Assets/Temp/ManageSpriteTests"; + + // Each cell is 16x16, so a 4x2 sheet is 64x32. Small enough to import fast, + // big enough that a wrong row/column order is visible in the rects. + private const int Cell = 16; + + [SetUp] + public void SetUp() => EnsureFolder(TempRoot); + + [TearDown] + public void TearDown() + { + if (AssetDatabase.IsValidFolder(TempRoot)) + AssetDatabase.DeleteAsset(TempRoot); + CleanupEmptyParentFolders(TempRoot); + } + + // ===================================================================== + // Helpers + // ===================================================================== + + /// + /// Writes a real PNG into the project and imports it, so the tools run against + /// an actual TextureImporter rather than a stand-in. + /// + private static string CreateSheet(string name, int cols, int rows) + { + var tex = new Texture2D(cols * Cell, rows * Cell, TextureFormat.RGBA32, false); + var pixels = new Color32[tex.width * tex.height]; + for (int i = 0; i < pixels.Length; i++) + pixels[i] = new Color32(255, 0, 0, 255); + tex.SetPixels32(pixels); + tex.Apply(); + + string assetPath = $"{TempRoot}/{name}.png"; + string sysPath = Path.Combine( + Directory.GetParent(Application.dataPath).FullName, assetPath); + File.WriteAllBytes(sysPath, tex.EncodeToPNG()); + Object.DestroyImmediate(tex); + + AssetDatabase.ImportAsset(assetPath, ImportAssetOptions.ForceSynchronousImport); + + // Only the asset's existence: the texture is still Default-type here, and that + // import rescales a non-power-of-two sheet. Slice() asserts the frame count. + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(assetPath), + $"fixture: {assetPath} did not import"); + return assetPath; + } + + + /// + /// A square PNG of incompressible noise, used to exceed the inline-image ceiling. + /// Written through the same import path as CreateSheet. + /// + private static string CreateNoiseSheet(string name, int side, bool assertOverCeiling = true) + { + var tex = new Texture2D(side, side, TextureFormat.RGBA32, false); + var pixels = new Color32[side * side]; + uint state = 0x13579BDFu; // fixed seed: the file size must not vary between runs + for (int i = 0; i < pixels.Length; i++) + { + state = state * 1664525u + 1013904223u; + pixels[i] = new Color32((byte)(state >> 24), (byte)(state >> 16), (byte)(state >> 8), 255); + } + tex.SetPixels32(pixels); + tex.Apply(); + + string assetPath = $"{TempRoot}/{name}.png"; + string sysPath = Path.Combine( + Directory.GetParent(Application.dataPath).FullName, assetPath); + File.WriteAllBytes(sysPath, tex.EncodeToPNG()); + Object.DestroyImmediate(tex); + + // Asserts which side of SpriteImportSetup's 4 MB ceiling this landed on rather + // than trusting the compressor; changing that ceiling breaks these lines loudly. + if (assertOverCeiling) + Assert.Greater(new FileInfo(sysPath).Length, 4 * 1024 * 1024, + "fixture: the noise sheet must exceed the inline-image ceiling"); + AssetDatabase.ImportAsset(assetPath, ImportAssetOptions.ForceSynchronousImport); + return assetPath; + } + + /// A flat sheet of an exact pixel size, for grids that do not divide evenly. + private static string CreateSheetOfSize(string name, int width, int height) + { + var tex = new Texture2D(width, height, TextureFormat.RGBA32, false); + var pixels = new Color32[width * height]; + for (int i = 0; i < pixels.Length; i++) + pixels[i] = new Color32(255, 0, 0, 255); + tex.SetPixels32(pixels); + tex.Apply(); + + string assetPath = $"{TempRoot}/{name}.png"; + string sysPath = Path.Combine( + Directory.GetParent(Application.dataPath).FullName, assetPath); + File.WriteAllBytes(sysPath, tex.EncodeToPNG()); + // A Texture2D built in an EditMode test is not collected on its own. + Object.DestroyImmediate(tex); + + AssetDatabase.ImportAsset(assetPath, ImportAssetOptions.ForceSynchronousImport); + return assetPath; + } + + private static JObject Run(JObject p) => ToJObject(ManageSprite.HandleCommand(p)); + + private static string ErrorText(JObject result) => result.Value("message") ?? ""; + + private static JObject Slice(string path, int cols, int rows) + { + var result = Run(new JObject + { + ["action"] = "slice_sheet", + ["path"] = path, + ["cols"] = cols, + ["rows"] = rows, + }); + // The refusal tests call this helper too and check the failure themselves. + if (result.Value("success")) + Assert.AreEqual(cols * rows, SpritesOf(path).Length, + "fixture: slice_sheet produced fewer frames than the grid asked for"); + return result; + } + + /// The sliced frames, in the natural order their names imply. + private static Sprite[] SpritesOf(string path) => + AssetDatabase.LoadAllAssetsAtPath(path) + .OfType() + .OrderBy(s => int.Parse(s.name.Split('_').Last())) + .ToArray(); + + // ===================================================================== + // Dispatch + // ===================================================================== + + [Test] + public void HandleCommand_MissingAction_ReturnsError() + { + var result = Run(new JObject()); + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("'action' is required")); + } + + [Test] + public void HandleCommand_UnknownAction_NamesTheValidOnes() + { + var result = Run(new JObject { ["action"] = "not_an_action" }); + Assert.IsFalse(result.Value("success")); + // Listing the alternatives is the difference between a dead end and a retry. + Assert.That(ErrorText(result), Does.Contain("slice_sheet")); + Assert.That(ErrorText(result), Does.Contain("full_setup")); + } + + // ===================================================================== + // get_info + // ===================================================================== + + [Test] + public void GetInfo_MissingPath_ReturnsError() + { + var result = Run(new JObject { ["action"] = "get_info" }); + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("'path' is required"), + "success alone would also be false on the importer-not-found branch"); + } + + [Test] + public void GetInfo_PathIsNotATexture_ReturnsError() + { + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = $"{TempRoot}/nothing_here.png", + }); + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("TextureImporter")); + } + + [Test] + public void GetInfo_ReportsTheTextureDimensions() + { + string path = CreateSheet("info", 4, 2); + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(4 * Cell, result.Value("width")); + Assert.AreEqual(2 * Cell, result.Value("height")); + } + + [Test] + public void GetInfo_OnAnUnslicedSheet_ReportsNoSlices() + { + string path = CreateSheet("unsliced", 4, 2); + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + Assert.IsNotNull(result["slice_count"], + "an absent field also reads as 0, so the field itself has to be there"); + Assert.AreEqual(0, result.Value("slice_count")); + // The count alone would pass against a response that reported slices it never counted. + Assert.AreEqual(0, ((JArray)result["slices"]).Count); + } + + [Test] + public void GetInfo_AfterSlicing_ReportsEverySlice() + { + string path = CreateSheet("sliced", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + Assert.AreEqual(8, result.Value("slice_count")); + // slice_count is independent of the projected list - measured 2026-08-21, + // emptying `slices` entirely left this test green. + var names = ((JArray)result["slices"]).Select(t => t.Value("name")).ToArray(); + Assert.That(names, Is.EquivalentTo(SpritesOf(path).Select(s => s.name)), + "every slice is reported, not just counted"); + } + + [Test] + public void GetInfo_ModestSheet_ComesBackInOnePageWithNoCursor() + { + string path = CreateSheet("onepage", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + // Under the default page of 512, so this asserts the default, not a guarantee. + Assert.AreEqual(8, ((JArray)result["slices"]).Count); + // Value rather than indexing: an omitted property would throw instead of + // asserting. Absent and null both mean "finished" - do not pin the shape. + Assert.IsNull(result.Value("next_cursor"), + "a finished list has no next cursor"); + } + + [Test] + public void GetInfo_MoreSlicesThanThePage_ReturnsOnePageAndPointsAtTheRest() + { + string path = CreateSheet("paged", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = 3, + }); + + Assert.AreEqual(3, ((JArray)result["slices"]).Count, "the page is bounded"); + Assert.AreEqual(8, result.Value("slice_count"), + "slice_count stays the total, not the size of the page"); + Assert.AreEqual(3, result.Value("next_cursor")); + } + + [Test] + public void GetInfo_WalkingTheCursor_VisitsEverySliceOnceAndThenStops() + { + string path = CreateSheet("walk", 4, 2); + Slice(path, 4, 2); + + var seen = new List(); + int? cursor = 0; + // Bounded so a cursor that never advances fails rather than hanging the run. + for (int page = 0; page < 10 && cursor != null; page++) + { + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = 3, + ["cursor"] = cursor.Value, + }); + seen.AddRange(((JArray)result["slices"]).Select(t => t.Value("name"))); + cursor = result.Value("next_cursor"); + } + + Assert.IsNull(cursor, "the walk has to terminate on its own"); + Assert.AreEqual(8, seen.Count, "no slice returned twice and none skipped"); + CollectionAssert.AllItemsAreUnique(seen); + } + + [Test] + public void GetInfo_CursorAtTheEnd_ReturnsAnEmptyPageRatherThanAnError() + { + string path = CreateSheet("tail", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["cursor"] = 8, + }); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(0, ((JArray)result["slices"]).Count); + } + + // Skip(-3) yields the whole list, so without the guard a negative cursor answers + // with every slice and reports success - a right-looking answer, hence the refusal. + [TestCase(-3)] + [TestCase(9)] + public void GetInfo_CursorOutsideTheList_IsRefused(int cursor) + { + string path = CreateSheet("cursor", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path, ["cursor"] = cursor }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("cursor")); + } + + [Test] + public void GetInfo_MissingFileOnDisk_DoesNotPutTheAbsolutePathInTheResponse() + { + string path = CreateSheet("nofile", 4, 2); + // Deleted WITHOUT Refresh, so the importer still resolves and the File.Exists + // branch answers - the only branch that used to leak the absolute path. + string full = Path.Combine( + Directory.GetParent(Application.dataPath).FullName, path); + File.Delete(full); + + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + string reason = result.Value("image_omitted_reason"); + + Assert.IsNotNull(reason, "fixture: the image was supposed to be dropped here"); + Assert.That(reason, Does.Not.Contain(Application.dataPath), + "the response must not disclose where the project lives on disk"); + Assert.That(reason, Does.Contain(path), + "it still has to say which asset it could not read"); + } + + [Test] + public void SliceSheet_GridThatDoesNotCoverTheTexture_SucceedsButSaysSo() + { + // 100 / 6 = 16, so the grid covers 96px and drops four. Measured before the + // warning existed: success, six sprites, empty diagnostics. + string path = CreateSheetOfSize("remainder", 100, 16); + + var result = Run(new JObject { ["action"] = "slice_sheet", ["path"] = path, + ["cols"] = 6, ["rows"] = 1 }); + + // Still a success: a trailing margin is ordinary and refusing would break it. + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(6, SpritesOf(path).Length); + Assert.That(result["diagnostics"].ToString(), Does.Contain("SLICE_GRID_REMAINDER")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("100"), + "the warning has to name the texture size, or it cannot be acted on"); + } + + [Test] + public void SliceSheet_GridThatCoversTheTextureExactly_WarnsAboutNothing() + { + // Without this, a warning firing on every slice looks like one firing correctly. + string path = CreateSheetOfSize("exact", 96, 16); + + var result = Run(new JObject { ["action"] = "slice_sheet", ["path"] = path, + ["cols"] = 6, ["rows"] = 1 }); + + Assert.IsTrue(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Not.Contain("SLICE_GRID_REMAINDER")); + } + + [TestCase("cols")] + [TestCase("rows")] + [TestCase("frame_width")] + [TestCase("frame_height")] + public void SliceSheet_GridValueTooLargeForAnInt_IsRefusedNotThrown(string key) + { + string path = CreateSheet($"gridovf{key}", 4, 2); + var request = new JObject { ["action"] = "slice_sheet", ["path"] = path, + ["cols"] = 4, ["rows"] = 2 }; + request[key] = 2147483648L; + + // Measured 2026-08-21, before the guard: all four raised an uncaught + // OverflowException, so reaching the assertions at all is half of this test. + var result = Run(request); + + Assert.IsFalse(result.Value("success")); + // "32-bit", not just the key name: a wrapped value lands on a different message + // that also contains the key - measured, the weaker assertion passed the mutation. + Assert.That(ErrorText(result), Does.Contain(key).And.Contain("32-bit")); + } + + [Test] + public void SliceSheet_FractionalGridValue_IsRefusedRatherThanRounded() + { + string path = CreateSheet("gridfrac", 4, 2); + var result = Run(new JObject { ["action"] = "slice_sheet", ["path"] = path, + ["cols"] = 2.7, ["rows"] = 2 }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("cols")); + } + + [TestCase("page_size")] + [TestCase("cursor")] + public void GetInfo_PagingValueTooLargeForAnInt_IsRefusedNotThrown(string key) + { + string path = CreateSheet($"overflow{key}", 4, 2); + Slice(path, 4, 2); + + var request = new JObject { ["action"] = "get_info", ["path"] = path }; + request[key] = 2147483648L; + + // Measured before the guard: ToObject raised an uncaught OverflowException, + // so reaching the assertions at all is half of this test. + var result = Run(request); + + Assert.IsFalse(result.Value("success")); + // A wrapped value is still refused, but by the range guard downstream; only this + // phrase tells the two apart. + Assert.That(ErrorText(result), Does.Contain(key).And.Contain("32-bit")); + } + + [Test] + public void GetInfo_FractionalPageSize_IsRefusedRatherThanRounded() + { + string path = CreateSheet("fractional", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = 2.7, + }); + + // Measured before the guard: returned three slices and reported success. + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("page_size")); + } + + [TestCase(0)] + [TestCase(-1)] + [TestCase(4097)] + public void GetInfo_PageSizeOutsideItsRange_IsRefused(int pageSize) + { + string path = CreateSheet($"pagesize{pageSize}", 4, 2); + Slice(path, 4, 2); + + var result = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = pageSize, + }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("page_size")); + } + + [Test] + public void GetInfo_PagesAfterTheFirst_DropTheImageAndSayWhy() + { + string path = CreateSheet("imageonce", 4, 2); + Slice(path, 4, 2); + + var first = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = 3, + }); + var second = Run(new JObject + { + ["action"] = "get_info", + ["path"] = path, + ["page_size"] = 3, + ["cursor"] = 3, + }); + + Assert.IsNotNull(first.Value("image_base64"), + "fixture: the first page is supposed to carry the image"); + Assert.IsNull(second.Value("image_base64")); + Assert.That(second.Value("image_omitted_reason"), Does.Contain("first page")); + } + + // ===================================================================== + // slice_sheet + // ===================================================================== + + [TestCase("slice_sheet")] + [TestCase("full_setup")] + public void WithoutColsOrFrameWidth_ReturnsError(string action) + { + string path = CreateSheet("nogrid", 4, 2); + var result = Run(new JObject { ["action"] = action, ["path"] = path }); + + Assert.IsFalse(result.Value("success")); + // Not just success=false: Newtonsoft reads an absent "success" as false too. + Assert.That(ErrorText(result), Does.Contain("frame_width")); + } + + [Test] + public void SliceSheet_ProducesOneSpritePerGridCell() + { + string path = CreateSheet("grid", 4, 2); + var result = Slice(path, 4, 2); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(8, result.Value("total_frames")); + // The reported count is a claim; the sub-assets on disk are the fact. + Assert.AreEqual(8, SpritesOf(path).Length); + } + + [Test] + public void SliceSheet_FrameZeroIsTheTopLeftCell() + { + // Sheets read top-to-bottom, but Unity's texture origin is bottom-left; getting + // this backwards silently plays the animation in the wrong order. + string path = CreateSheet("order", 4, 2); + Slice(path, 4, 2); + + var first = SpritesOf(path).First(); + Assert.AreEqual(0, (int)first.rect.x, "frame 0 should sit at the left edge"); + Assert.AreEqual(Cell, (int)first.rect.y, "frame 0 should sit on the top row"); + } + + [Test] + public void SliceSheet_LastFrameIsTheBottomRightCell() + { + string path = CreateSheet("order2", 4, 2); + Slice(path, 4, 2); + + var last = SpritesOf(path).Last(); + Assert.AreEqual(3 * Cell, (int)last.rect.x); + Assert.AreEqual(0, (int)last.rect.y); + } + + [Test] + public void SliceSheet_EveryFrameHasTheCellSize() + { + string path = CreateSheet("size", 4, 2); + Slice(path, 4, 2); + + foreach (var s in SpritesOf(path)) + { + Assert.AreEqual(Cell, (int)s.rect.width, $"{s.name} width"); + Assert.AreEqual(Cell, (int)s.rect.height, $"{s.name} height"); + } + } + + [Test] + public void SliceSheet_NonPowerOfTwoSheet_KeepsEveryFrame() + { + // 96px is not a power of two: a Default-type import rescales it to 128, giving + // 21px cells whose last two frames fall outside the real texture - success anyway. + string path = CreateSheet("npot", 6, 1); + var result = Slice(path, 6, 1); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(Cell, result.Value("frame_width"), + "the grid must be measured against the sheet's real width"); + Assert.AreEqual(6, SpritesOf(path).Length, "no frame may be dropped"); + } + + [Test] + public void SliceSheet_TextureAlreadyConvertedToSprite_KeepsEveryFrame() + { + // Pins the branch where the texture is already a Sprite, which used to skip the + // conversion block and so never normalised npotScale. On 2021.3.45f2 that made + // Unity refuse sprite generation and slice_sheet report six frames over an empty + // asset; reverting the npotScale line turns this case red there. + string path = CreateSheet("npot_preset", 6, 1); + var importer = (TextureImporter)AssetImporter.GetAtPath(path); + importer.textureType = TextureImporterType.Sprite; + importer.npotScale = TextureImporterNPOTScale.ToNearest; + EditorUtility.SetDirty(importer); + importer.SaveAndReimport(); + + var result = Slice(path, 6, 1); + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(6, SpritesOf(path).Length, "no frame may be dropped"); + } + + [Test] + public void SliceSheet_FrameWidthAloneDerivesTheColumnCount() + { + string path = CreateSheet("derive", 4, 1); + var result = Run(new JObject + { + ["action"] = "slice_sheet", + ["path"] = path, + ["frame_width"] = Cell, + ["frame_height"] = Cell, + }); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(4, result.Value("cols")); + Assert.AreEqual(4, SpritesOf(path).Length); + } + + [Test] + public void SliceSheet_BaseNameOverridesTheFileName() + { + string path = CreateSheet("filename", 2, 1); + Run(new JObject + { + ["action"] = "slice_sheet", + ["path"] = path, + ["cols"] = 2, + ["base_name"] = "hero", + }); + + Assert.That(SpritesOf(path).Select(s => s.name), Is.EquivalentTo(new[] { "hero_0", "hero_1" })); + } + + [Test] + public void SliceSheet_FrameHeightAloneDerivesTheRowCount() + { + // rows defaulted to 1 even when frame_height was there to derive it from, so the + // documented alternative produced a single row and a remainder warning. + string path = CreateSheet("derive_rows", 4, 4); + var result = Run(new JObject { ["action"] = "slice_sheet", ["path"] = path, + ["cols"] = 4, ["frame_height"] = Cell }); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual(4, result.Value("rows")); + Assert.AreEqual(16, SpritesOf(path).Length); + } + + private static IEnumerable RefusedGrids() + { + TestCaseData Case(int sheetCols, int sheetRows, JObject grid, string code) => + new TestCaseData(sheetCols, sheetRows, grid, code) + .SetName($"{code} from {string.Join(",", grid.Properties().Select(p => p.Name + "=" + p.Value))}"); + + // A frame wider than the sheet derives 0 columns. + yield return Case(2, 1, new JObject { ["frame_width"] = 4096 }, "SLICE_EMPTY"); + // `?? 1` only covers a missing key; an explicit 0 reaches the `texH / rows` division. + yield return Case(4, 1, new JObject { ["cols"] = 4, ["rows"] = 0 }, "BAD_PARAM"); + // Only reachable after the texture is measured, so moving the argument checks + // earlier could not close this form of the class; the height axis reaches it too. + yield return Case(2, 1, new JObject { ["cols"] = 2, ["frame_width"] = 4096 }, "SLICE_OUT_OF_BOUNDS"); + yield return Case(2, 1, new JObject { ["cols"] = 2, ["rows"] = 1, ["frame_height"] = 4096 }, "SLICE_OUT_OF_BOUNDS"); + // 64 columns across 32 pixels derives a 0-wide frame, whose product passes any + // "does it fit" test - 64 degenerate rects, reported as success. + yield return Case(2, 1, new JObject { ["cols"] = 64 }, "SLICE_OUT_OF_BOUNDS"); + yield return Case(2, 1, new JObject { ["cols"] = 2, ["rows"] = 32 }, "SLICE_OUT_OF_BOUNDS"); + // 65536 * 65536 wraps to 0 in 32-bit arithmetic and slipped under the comparison. + // The code, not just the failure: measured 2026-08-21, with the (long) cast + // removed this stayed GREEN because the frame ceiling refused it instead. + yield return Case(2, 1, new JObject { ["cols"] = 65536, ["frame_width"] = 65536 }, "SLICE_OUT_OF_BOUNDS"); + // 16,384 entries that fit inside the texture: only the frame ceiling stops this. + yield return Case(8, 8, new JObject { ["cols"] = 128, ["rows"] = 128 }, "SLICE_TOO_MANY_FRAMES"); + // A negative alternative used to be silently replaced by the value derived from cols. + yield return Case(2, 1, new JObject { ["cols"] = 2, ["frame_width"] = -1 }, "BAD_PARAM"); + } + + [TestCaseSource(nameof(RefusedGrids))] + public void SliceSheet_GridThatCannotBeCut_IsRefusedAndLeavesTheTextureAlone( + int sheetCols, int sheetRows, JObject grid, string code) + { + string path = CreateSheet("badgrid", sheetCols, sheetRows); + var before = ((TextureImporter)AssetImporter.GetAtPath(path)).textureType; + + var request = new JObject { ["action"] = "slice_sheet", ["path"] = path }; + foreach (var p in grid.Properties()) + request[p.Name] = p.Value; + var result = Run(request); + + Assert.IsFalse(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Contain(code)); + Assert.AreEqual(0, SpritesOf(path).Length, "a refused grid must not write any frame"); + // Every refusal after the Sprite conversion owes a RestoreTextureType call, an + // obligation the code cannot enforce; this assertion makes a forgotten one fail. + Assert.AreEqual(before, ((TextureImporter)AssetImporter.GetAtPath(path)).textureType, + "a refused request must not leave the texture converted behind it"); + } + + [Test] + public void SliceSheet_ReslicingWithADifferentGrid_ReplacesTheOldFrames() + { + string path = CreateSheet("reslice", 4, 2); + Slice(path, 4, 2); + Assert.AreEqual(8, SpritesOf(path).Length); + + Slice(path, 2, 1); + var after = SpritesOf(path).Select(s => s.name).ToArray(); +#pragma warning disable CS0618 // same API the tool writes through + int configured = ((TextureImporter)AssetImporter.GetAtPath(path)).spritesheet.Length; +#pragma warning restore CS0618 + Assert.AreEqual(2, after.Length, + $"stale frames must not survive a reslice; importer holds {configured}, " + + "project holds: " + string.Join(", ", after)); + } + + // ===================================================================== + // setup_clips + // ===================================================================== + + private static JObject SetupClips(string path, JArray clips) => Run(new JObject + { + ["action"] = "setup_clips", + ["path"] = path, + ["clips"] = clips, + ["output_dir"] = TempRoot, + }); + + private static JArray OneClip(string name, int start, int end, float? fps = null, bool? loop = null) + { + var clip = new JObject { ["name"] = name, ["start_frame"] = start, ["end_frame"] = end }; + if (fps.HasValue) clip["fps"] = fps.Value; + if (loop.HasValue) clip["loop"] = loop.Value; + return new JArray { clip }; + } + + [Test] + public void SetupClips_OnAnUnslicedSheet_TellsYouToSliceFirst() + { + string path = CreateSheet("noslice", 4, 1); + var result = SetupClips(path, OneClip("walk", 0, 3)); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("slice_sheet")); + } + + [Test] + public void SetupClips_WritesAClipAssetWithOneKeyPerFrame() + { + string path = CreateSheet("clips", 4, 1); + Slice(path, 4, 1); + + var result = SetupClips(path, OneClip("walk", 0, 3)); + Assert.IsTrue(result.Value("success")); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + Assert.IsNotNull(clip, "the .anim asset should exist on disk"); + + var binding = AnimationUtility.GetObjectReferenceCurveBindings(clip).Single(); + Assert.AreEqual(typeof(SpriteRenderer), binding.type); + Assert.AreEqual("m_Sprite", binding.propertyName, + "anything else animates the wrong property and shows nothing"); + Assert.AreEqual(4, AnimationUtility.GetObjectReferenceCurve(clip, binding).Length); + } + + [Test] + public void SetupClips_FpsDrivesTheFrameRateAndTheKeyTimes() + { + string path = CreateSheet("fps", 4, 1); + Slice(path, 4, 1); + SetupClips(path, OneClip("walk", 0, 3, fps: 8f)); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + Assert.AreEqual(8f, clip.frameRate); + + var binding = AnimationUtility.GetObjectReferenceCurveBindings(clip).Single(); + var keys = AnimationUtility.GetObjectReferenceCurve(clip, binding); + Assert.AreEqual(0f, keys[0].time, 0.0001f); + Assert.AreEqual(1f / 8f, keys[1].time, 0.0001f); + } + + [Test] + public void SetupClips_KeyframesFollowTheSlicedOrder() + { + string path = CreateSheet("seq", 4, 1); + Slice(path, 4, 1); + SetupClips(path, OneClip("walk", 0, 3)); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + var binding = AnimationUtility.GetObjectReferenceCurveBindings(clip).Single(); + var keys = AnimationUtility.GetObjectReferenceCurve(clip, binding); + + var expected = SpritesOf(path).Select(s => s.name).ToArray(); + var actual = keys.Select(k => k.value.name).ToArray(); + Assert.AreEqual(expected, actual, "frames must play in sheet order"); + } + + [Test] + public void SetupClips_TenthFrameSortsAfterTheSecond() + { + // A plain string sort puts hero_10 between hero_1 and hero_2. + string path = CreateSheet("natural", 11, 1); + Slice(path, 11, 1); + SetupClips(path, OneClip("walk", 0, 10)); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + var binding = AnimationUtility.GetObjectReferenceCurveBindings(clip).Single(); + var keys = AnimationUtility.GetObjectReferenceCurve(clip, binding); + + Assert.AreEqual("natural_2", keys[2].value.name); + Assert.AreEqual("natural_10", keys[10].value.name); + } + + [Test] + public void SetupClips_LoopIsInferredFromTheClipName() + { + string path = CreateSheet("loopname", 4, 1); + Slice(path, 4, 1); + SetupClips(path, new JArray + { + new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 1 }, + new JObject { ["name"] = "attack", ["start_frame"] = 2, ["end_frame"] = 3 }, + }); + + var walk = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + var attack = AssetDatabase.LoadAssetAtPath($"{TempRoot}/attack.anim"); + + Assert.IsTrue(AnimationUtility.GetAnimationClipSettings(walk).loopTime, + "locomotion should loop"); + Assert.IsFalse(AnimationUtility.GetAnimationClipSettings(attack).loopTime, + "a one-shot attack should not loop"); + } + + [Test] + public void SetupClips_ExplicitLoopBeatsTheNameGuess() + { + string path = CreateSheet("loopflag", 4, 1); + Slice(path, 4, 1); + SetupClips(path, OneClip("walk", 0, 3, loop: false)); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + Assert.IsFalse(AnimationUtility.GetAnimationClipSettings(clip).loopTime); + } + + private static IEnumerable RefusedClips() + { + TestCaseData Case(string label, JToken clip, string code) => + new TestCaseData(clip, code).SetName($"{code}: {label}"); + + // Measured 2026-08-21: `loop: "maybe"` raised an uncaught FormatException, and + // `loop: 2` was accepted silently. loop is the one flag with no type above C#. + yield return Case("loop is not a bool", new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 3, ["loop"] = "maybe" }, "CLIP_BAD_LOOP"); + // Before the guard these threw OverflowException out of the tool. + yield return Case("start_frame overflows int", new JObject { ["name"] = "walk", ["start_frame"] = 2147483648L, ["end_frame"] = 3 }, "CLIP_BAD_RANGE"); + yield return Case("end_frame overflows int", new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 2147483648L }, "CLIP_BAD_RANGE"); + // Measured before the guard: rounded to 3, wrote a clip, reported success. + yield return Case("fractional start_frame", new JObject { ["name"] = "walk", ["start_frame"] = 2.7, ["end_frame"] = 5 }, "CLIP_BAD_RANGE"); + // `fps <= 0f` is false for NaN, so the clip was written with NaN keyframe times. + yield return Case("NaN fps", new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 5, ["fps"] = double.NaN }, "CLIP_BAD_FPS"); + yield return Case("zero fps", new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 3, ["fps"] = 0f }, "CLIP_BAD_FPS"); + // Skip/Take clamps silently: an eight-frame clip for a hundred-frame request. + yield return Case("end_frame past the sheet", new JObject { ["name"] = "walk", ["start_frame"] = 0, ["end_frame"] = 99 }, "CLIP_BAD_RANGE"); + yield return Case("range beyond the sheet", new JObject { ["name"] = "walk", ["start_frame"] = 90, ["end_frame"] = 99 }, "CLIP_BAD_RANGE"); + // Skip ignores a negative count, so [-2,3] used to select frames 0..5. + yield return Case("negative start_frame", new JObject { ["name"] = "walk", ["start_frame"] = -2, ["end_frame"] = 3 }, "CLIP_BAD_RANGE"); + yield return Case("no name", new JObject { ["start_frame"] = 0, ["end_frame"] = 3 }, "CLIP_NO_NAME"); + // The name is joined into a file path, so separators would escape output_dir. + yield return Case("name escapes output_dir", new JObject { ["name"] = "../../evil", ["start_frame"] = 0, ["end_frame"] = 3 }, "CLIP_BAD_NAME"); + yield return Case("name with a separator", new JObject { ["name"] = "nested/walk", ["start_frame"] = 0, ["end_frame"] = 3 }, "CLIP_BAD_NAME"); + // Forwarded unchanged by the Python surface; the typed cast threw on them. + yield return Case("entry is a string", new JValue("not_an_object"), "CLIP_NOT_AN_OBJECT"); + yield return Case("entry is a number", new JValue(7), "CLIP_NOT_AN_OBJECT"); + // Legal in an asset path as far as SanitizeAssetPath is concerned, illegal in a file name. + yield return Case("name with a character no file name allows", new JObject { ["name"] = "bad:name", ["start_frame"] = 0, ["end_frame"] = 3 }, "CLIP_BAD_NAME"); + } + + [TestCaseSource(nameof(RefusedClips))] + public void SetupClips_ClipThatCannotBeBuilt_IsSkippedWithACode(JToken clip, string code) + { + string path = CreateSheet("badclip", 4, 2); + Slice(path, 4, 2); + + var result = SetupClips(path, new JArray { clip }); + + Assert.AreEqual(0, result.Value("clip_count")); + Assert.That(result["diagnostics"].ToString(), Does.Contain(code)); + Assert.AreEqual(0, AssetDatabase.FindAssets("t:AnimationClip", new[] { TempRoot }).Length, + "a skipped clip must not leave an asset behind"); + } + + [Test] + public void SetupClips_OutputDirEscapingAssets_IsRefused() + { + string path = CreateSheet("escape", 4, 1); + Slice(path, 4, 1); + + var result = Run(new JObject + { + ["action"] = "setup_clips", + ["path"] = path, + ["clips"] = OneClip("walk", 0, 3), + ["output_dir"] = $"{TempRoot}/../../../outside", + }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("output_dir")); + } + + [Test] + public void SetupClips_ExistingClipWithoutOverwrite_IsLeftAlone() + { + // Clips used to delete whatever sat at the composed path, so an unrelated clip + // sharing a name was destroyed by a request that never asked for a replacement. + string path = CreateSheet("existing", 4, 1); + Slice(path, 4, 1); + + var sentinel = new AnimationClip { frameRate = 99f }; + AssetDatabase.CreateAsset(sentinel, $"{TempRoot}/walk.anim"); + AssetDatabase.SaveAssets(); + + var result = SetupClips(path, OneClip("walk", 0, 3)); + + var after = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + Assert.IsNotNull(after, "the existing clip must survive"); + Assert.AreEqual(99f, after.frameRate, "the existing clip must not be replaced"); + Assert.AreEqual(0, result.Value("clip_count")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("CLIP_EXISTS")); + } + + [Test] + public void SetupClips_ExistingClipWithOverwrite_IsReplaced() + { + string path = CreateSheet("existing2", 4, 1); + Slice(path, 4, 1); + + var sentinel = new AnimationClip { frameRate = 99f }; + AssetDatabase.CreateAsset(sentinel, $"{TempRoot}/walk.anim"); + AssetDatabase.SaveAssets(); + + var result = Run(new JObject + { + ["action"] = "setup_clips", + ["path"] = path, + ["clips"] = OneClip("walk", 0, 3), + ["output_dir"] = TempRoot, + ["overwrite"] = true, + }); + + Assert.AreEqual(1, result.Value("clip_count")); + var after = AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"); + Assert.AreEqual(12f, after.frameRate, "an authorised overwrite must actually replace it"); + } + + [Test] + public void SetupClips_NameThatMerelyContainsAKeyword_IsNotTreatedAsLocomotion() + { + // 'grunt' contains the letters of 'run'; substring matching makes it loop. + string path = CreateSheet("substr", 4, 1); + Slice(path, 4, 1); + SetupClips(path, OneClip("grunt", 0, 3)); + + var clip = AssetDatabase.LoadAssetAtPath($"{TempRoot}/grunt.anim"); + Assert.IsFalse(AnimationUtility.GetAnimationClipSettings(clip).loopTime); + } + + // ===================================================================== + // setup_controller + // ===================================================================== + + private static JObject SetupController(JArray clips, bool overwrite = false) => Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = clips, + ["controller_path"] = $"{TempRoot}/Hero.controller", + ["overwrite"] = overwrite, + }); + + /// Slices a sheet and builds the named clips, returning [{name, path}] for the controller. + private static JArray BuildClips(string sheet, params string[] names) + { + string path = CreateSheet(sheet, names.Length * 2, 1); + Slice(path, names.Length * 2, 1); + + var defs = new JArray(); + for (int i = 0; i < names.Length; i++) + defs.Add(new JObject { ["name"] = names[i], ["start_frame"] = i * 2, ["end_frame"] = i * 2 + 1 }); + var clipResult = SetupClips(path, defs); + + var refs = new JArray(); + foreach (string n in names) + { + string clipPath = $"{TempRoot}/{n}.anim"; + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(clipPath), + $"fixture: clip '{n}' was not written to {clipPath}; setup_clips said " + + clipResult.ToString(Newtonsoft.Json.Formatting.None)); + refs.Add(new JObject { ["name"] = n, ["path"] = clipPath }); + } + return refs; + } + + [Test] + public void SetupController_WithoutClips_ReturnsError() + { + var result = Run(new JObject + { + ["action"] = "setup_controller", + ["controller_path"] = $"{TempRoot}/Hero.controller", + }); + Assert.IsFalse(result.Value("success")); + // Not just success=false: Newtonsoft reads an absent "success" as false too. + Assert.That(ErrorText(result), Does.Contain("clips")); + } + + [Test] + public void SetupController_WithoutControllerPath_ReturnsError() + { + var result = Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = new JArray { new JObject { ["name"] = "walk", ["path"] = "x.anim" } }, + }); + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("controller_path")); + } + + // 'Assets' passes SanitizeAssetPath, and the suffix then made it 'Assets.controller' at + // the project root; 'Assets/' became 'Assets/.controller', a file with no name. + [TestCase("Assets", "Assets.controller")] + [TestCase("Assets/", "Assets/.controller")] + [TestCase(" ", "Assets/ .controller")] + [TestCase(TempRoot, TempRoot + ".controller")] // an existing folder, no trailing slash + public void SetupController_ControllerPathThatIsOnlyAFolder_IsRefusedBeforeWriting(string controllerPath, string strayRelative) + { + var clips = BuildClips("folderctrl", "idle", "walk"); + var result = Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = clips, + ["controller_path"] = controllerPath, + }); + + string stray = Path.Combine(Directory.GetParent(Application.dataPath).FullName, strayRelative); + bool leaked = File.Exists(stray); + if (leaked) { File.Delete(stray); File.Delete(stray + ".meta"); } + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("controller_path")); + Assert.IsFalse(leaked, $"nothing may be written at {strayRelative}"); + } + + [Test] + public void SetupController_ClipEntryWithoutAName_IsSkippedWithAWarning() + { + var clips = BuildClips("nonamectrl", "idle", "walk"); + clips.Add(new JObject { ["path"] = $"{TempRoot}/walk.anim" }); + + var result = SetupController(clips); + + Assert.IsTrue(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("CLIP_NO_NAME")); + } + + [Test] + public void SetupController_ClipsThatDoNotExist_ReturnsError() + { + var result = SetupController(new JArray + { + new JObject { ["name"] = "walk", ["path"] = $"{TempRoot}/missing.anim" }, + }); + Assert.IsFalse(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("CLIP_NOT_FOUND")); + } + + [Test] + public void SetupController_EveryEntrySkipped_StillReportsWhy() + { + // The all-skipped path returns a generic error, so the caller learned the clips + // did not load without learning why. + var result = Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = new JArray { 7 }, + ["controller_path"] = $"{TempRoot}/Skipped.controller", + }); + + Assert.IsFalse(result.Value("success")); + Assert.That(result.ToString(), Does.Contain("CLIP_NOT_AN_OBJECT"), + "the response must carry the reason the builder recorded"); + } + + [Test] + public void SetupController_IdleAndWalk_WritesAControllerWithBothStates() + { + var result = SetupController(BuildClips("ctrl", "idle", "walk")); + Assert.IsTrue(result.Value("success")); + + var controller = AssetDatabase.LoadAssetAtPath($"{TempRoot}/Hero.controller"); + Assert.IsNotNull(controller); + + var states = controller.layers[0].stateMachine.states.Select(s => s.state.name).ToArray(); + Assert.That(states, Contains.Item("Idle")); + Assert.That(states, Contains.Item("walk")); + Assert.AreEqual("Idle", controller.layers[0].stateMachine.defaultState.name, + "idle is the state a character rests in, so it should be the entry point"); + } + + [Test] + public void SetupController_WalkAndRun_BuildsASpeedDrivenBlendTree() + { + var result = SetupController(BuildClips("blend", "idle", "walk", "run")); + Assert.IsTrue(result.Value("success")); + + var controller = AssetDatabase.LoadAssetAtPath($"{TempRoot}/Hero.controller"); + Assert.That(controller.parameters.Select(p => p.name), Contains.Item("Speed")); + + var loco = controller.layers[0].stateMachine.states + .Select(s => s.state) + .SingleOrDefault(s => s.name == "Locomotion"); + Assert.IsNotNull(loco, + "two locomotion clips should collapse into one blend tree state; states were: " + + string.Join(", ", controller.layers[0].stateMachine.states.Select(s => s.state.name))); + + var tree = loco.motion as BlendTree; + Assert.IsNotNull(tree); + Assert.AreEqual("Speed", tree.blendParameter); + // walk sits below run on the axis, otherwise the character sprints while strolling. + Assert.AreEqual(new[] { "walk", "run" }, + tree.children.Select(c => c.motion.name).ToArray()); + // The values, not just the order: with automatic thresholds left on, Unity + // overwrote 1/2 with 0/1 and this line is what notices. + Assert.AreEqual(new[] { 1f, 2f }, + tree.children.Select(c => c.threshold).ToArray()); + } + + [TestCase("attack", "Attack")] + // Naming the trigger after the first segment would give 'Hero', not 'Attack'. + [TestCase("hero_attack", "Attack", "Hero")] + // The letters of 'hit' sit inside 'white', which under substring matching arms + // a trigger the clip never asked for. + [TestCase("white_flash", null, "Hit", "White")] + // Detect used to lowercase before tokenizing, and the tokenizer splits camelCase + // on char.IsUpper - so 'heroAttack' became one word and matched no keyword. + [TestCase("heroAttack", "Attack")] + // 'heroXMLAttack' has no lower-to-upper boundary at the acronym's end, so it + // tokenized as 'xmlattack' and lost the trigger its snake_case twin gets. + [TestCase("heroXMLAttack", "Attack")] + // 'heroATTACK' held already - the break comes from the lowercase 'o' - so it is a + // parity tripwire; 'XMLSlash' has no lowercase at the boundary and depends on the fix. + [TestCase("heroATTACK", "Attack")] + [TestCase("XMLSlash", "Slash")] + public void SetupController_TriggerIsNamedAfterTheActionWord(string clipName, string trigger, params string[] absent) + { + var result = SetupController(BuildClips("trig", "idle", clipName)); + Assert.IsTrue(result.Value("success")); + + var controller = AssetDatabase.LoadAssetAtPath($"{TempRoot}/Hero.controller"); + var names = controller.parameters.Select(p => p.name).ToArray(); + if (trigger != null) + { + var found = controller.parameters.SingleOrDefault(p => p.name == trigger); + Assert.IsNotNull(found, $"expected trigger '{trigger}'; parameters present: " + string.Join(", ", names)); + Assert.AreEqual(AnimatorControllerParameterType.Trigger, found.type); + } + foreach (string name in absent) + Assert.That(names, Has.No.Member(name)); + } + + [Test] + public void SetupController_ControllerPathEscapingAssets_FailsWithAMessage() + { + var clips = BuildClips("escapectrl", "idle", "walk"); + var result = Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = clips, + ["controller_path"] = $"{TempRoot}/../../../Hero.controller", + }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("controller_path")); + } + + [Test] + public void SetupController_ExistingControllerWithoutOverwrite_RefusesInsteadOfReplacing() + { + var clips = BuildClips("exists", "idle", "walk"); + Assert.IsTrue(SetupController(clips).Value("success")); + + var second = SetupController(clips); + Assert.IsFalse(second.Value("success")); + Assert.That(second["diagnostics"].ToString(), Does.Contain("CONTROLLER_EXISTS")); + } + + [Test] + public void SetupController_ExistingControllerWithOverwrite_Replaces() + { + var clips = BuildClips("overwrite", "idle", "walk"); + Assert.IsTrue(SetupController(clips).Value("success")); + + // Marked so reuse is distinguishable from replacement: two successes fit both. + string ctrlPath = $"{TempRoot}/Hero.controller"; + var first = AssetDatabase.LoadAssetAtPath(ctrlPath); + first.AddParameter("SentinelFromFirstBuild", AnimatorControllerParameterType.Bool); + AssetDatabase.SaveAssets(); + + Assert.IsTrue(SetupController(clips, overwrite: true).Value("success")); + + var second = AssetDatabase.LoadAssetAtPath(ctrlPath); + Assert.That(second.parameters.Select(p => p.name), Has.No.Member("SentinelFromFirstBuild"), + "an authorised overwrite must build a new controller, not reuse the old one"); + } + + [Test] + public void SetupController_OverwriteThatCannotBuildAReplacement_KeepsTheOldController() + { + var clips = BuildClips("s2", "idle", "walk"); + Assert.IsTrue(SetupController(clips).Value("success")); + string ctrl = $"{TempRoot}/Hero.controller"; + var before = AssetDatabase.LoadAssetAtPath(ctrl); + Assert.IsNotNull(before); + + // Every replacement clip is unloadable, so the rebuild cannot succeed. + var doomed = new JArray { + new JObject { ["name"] = "idle", ["path"] = $"{TempRoot}/does_not_exist.anim" }, + }; + Run(new JObject + { + ["action"] = "setup_controller", + ["clips"] = doomed, + ["controller_path"] = ctrl, + ["overwrite"] = true, + }); + + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath(ctrl), + "a failed rebuild must not leave the caller without the controller they had"); + } + + // ===================================================================== + // full_setup + // ===================================================================== + + [Test] + public void FullSetup_ControllerRefusal_StopsBeforeTouchingTheScene() + { + string path = CreateSheet("s5", 4, 1); + var go = new GameObject("SpriteTest_S5"); + try + { + string ctrl = $"{TempRoot}/S5.controller"; + Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = ctrl }); + + // Second run: the controller exists and overwrite is not set, so the + // controller step fails - and a failed step must not fall through. + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = ctrl, + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S5" }); + + Assert.IsFalse(result.Value("success")); + Assert.AreEqual("setup_controller", result.Value("step"), + "the response must name the step that failed"); + Assert.IsNull(go.GetComponent(), + "a refused controller step must not go on to modify the scene"); + } + finally { Object.DestroyImmediate(go); } + } + + [Test] + public void FullSetup_RequestedSceneTargetMissing_IsNotReportedAsSuccess() + { + string path = CreateSheet("s6", 4, 1); + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S6.controller", + ["add_to_scene"] = true, ["scene_target"] = "NoSuchObject" }); + + Assert.IsFalse(result.Value("success"), + "an attachment that was asked for and did not happen is not a success"); + Assert.That(result["diagnostics"].ToString(), Does.Contain("SCENE_TARGET_NOT_FOUND")); + // Asserted on the message rather than only the diagnostics array, because reading + // the array was what let this refusal keep a shape no other refusal in the tool has. + Assert.AreEqual("add_to_scene", result.Value("step")); + Assert.That(ErrorText(result), Does.Contain("NoSuchObject")); + } + + [Test] + public void FullSetup_ControllerPathWithoutExtension_StillReachesTheSceneObject() + { + string path = CreateSheet("s7", 4, 1); + var go = new GameObject("SpriteTest_S7"); + try + { + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, + ["controller_path"] = $"{TempRoot}/S7", // no .controller suffix + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S7" }); + + // Count the components rather than null-checking GetComponent: a missing one + // compares equal to null without being a null reference. + Assert.AreEqual(1, go.GetComponents().Length, + "the object should have received an Animator; result was " + result.ToString(Newtonsoft.Json.Formatting.None)); + Assert.IsTrue(go.GetComponents()[0].runtimeAnimatorController != null, + "the suffix the builder added must not lose the controller on the way to the scene"); + } + finally { Object.DestroyImmediate(go); } + } + + [Test] + public void FullSetup_SceneTargetWithoutASpriteRenderer_GetsOneAndSaysSo() + { + string path = CreateSheet("s8", 4, 1); + var go = new GameObject("SpriteTest_S8"); + try + { + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S8.controller", + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S8" }); + + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(1, go.GetComponents().Length, + "the clips animate a SpriteRenderer, so the attachment has to leave one behind"); + Assert.That(result["diagnostics"].ToString(), Does.Contain("SCENE_SPRITE_RENDERER_ADDED"), + "a component added to the caller's object must be reported"); + } + finally { Object.DestroyImmediate(go); } + } + + [Test] + public void FullSetup_SceneTargetThatAlreadyHasASpriteRenderer_KeepsTheOneItHas() + { + string path = CreateSheet("s9", 4, 1); + var go = new GameObject("SpriteTest_S9"); + var existing = go.AddComponent(); + try + { + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S9.controller", + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S9" }); + + // The scene step has to have run for the renderer assertions to mean anything. + Assert.IsTrue(result.Value("success")); + Assert.AreEqual(1, go.GetComponents().Length); + Assert.IsTrue(go.GetComponent().runtimeAnimatorController != null); + Assert.AreEqual(1, go.GetComponents().Length); + Assert.AreSame(existing, go.GetComponent()); + Assert.That(result["diagnostics"].ToString(), Does.Not.Contain("SCENE_SPRITE_RENDERER_ADDED")); + } + finally { Object.DestroyImmediate(go); } + } + + [Test] + public void FullSetup_InactiveSceneTarget_StillReceivesTheComponents() + { + string path = CreateSheet("s10", 4, 1); + var go = new GameObject("SpriteTest_S10"); + go.SetActive(false); + try + { + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S10.controller", + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S10" }); + + Assert.IsTrue(result.Value("success"), + "an inactive object is still the object the caller named; result was " + result.ToString(Newtonsoft.Json.Formatting.None)); + Assert.AreEqual(1, go.GetComponents().Length); + } + finally { Object.DestroyImmediate(go); } + } + + [Test] + public void FullSetup_DuplicateSceneTargetNames_AreRefusedBeforeTouchingEither() + { + string path = CreateSheet("s11", 4, 1); + var first = new GameObject("SpriteTest_S11"); + var second = new GameObject("SpriteTest_S11"); + try + { + var result = Run(new JObject { ["action"] = "full_setup", ["path"] = path, ["cols"] = 4, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S11.controller", + ["add_to_scene"] = true, ["scene_target"] = "SpriteTest_S11" }); + + Assert.IsFalse(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("SCENE_TARGET_AMBIGUOUS")); + Assert.AreEqual(0, first.GetComponents().Length + second.GetComponents().Length, + "with two candidates the tool must not guess which one the caller meant"); + } + finally { Object.DestroyImmediate(first); Object.DestroyImmediate(second); } + } + + [Test] + public void FullSetup_RefusedClip_IsNotCountedAsCreated() + { + string path = CreateSheet("s4", 6, 1); + var result = Run(new JObject + { + ["action"] = "full_setup", ["path"] = path, ["cols"] = 6, + ["output_dir"] = TempRoot, ["controller_path"] = $"{TempRoot}/S4.controller", + ["clips"] = new JArray { + new JObject { ["name"] = "idle", ["start_frame"] = 0, ["end_frame"] = 1 }, + new JObject { ["name"] = "attack", ["start_frame"] = 2, ["end_frame"] = 3, ["fps"] = 0 }, + new JObject { ["name"] = "walk", ["start_frame"] = 4, ["end_frame"] = 5 }, + }, + }); + + int onDisk = AssetDatabase.FindAssets("t:AnimationClip", new[] { TempRoot }).Length; + Assert.AreEqual(2, onDisk, "idle and walk are valid, attack is refused"); + Assert.AreEqual(onDisk, result.Value("clip_count"), + "clip_count must count the clips that exist, not the ones that were asked for"); + Assert.That(result["diagnostics"].ToString(), Does.Contain("CLIP_BAD_FPS")); + } + + [Test] + public void FullSetup_SlicesBuildsClipsAndWritesAController() + { + string path = CreateSheet("full", 4, 1); + var result = Run(new JObject + { + ["action"] = "full_setup", + ["path"] = path, + ["cols"] = 4, + ["animation_name"] = "walk", + ["output_dir"] = TempRoot, + ["controller_path"] = $"{TempRoot}/Full.controller", + }); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual(4, SpritesOf(path).Length, "the sheet should end up sliced"); + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath($"{TempRoot}/walk.anim"), + "the clip should end up on disk"); + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath($"{TempRoot}/Full.controller"), + "the controller should end up on disk"); + } + + [Test] + public void FullSetup_DefaultsTheClipNameToTheFileName() + { + string path = CreateSheet("hero_idle", 4, 1); + var result = Run(new JObject + { + ["action"] = "full_setup", + ["path"] = path, + ["cols"] = 4, + ["output_dir"] = TempRoot, + ["controller_path"] = $"{TempRoot}/Named.controller", + }); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.IsNotNull(AssetDatabase.LoadAssetAtPath($"{TempRoot}/hero_idle.anim")); + } + + // ===================================================================== + // Refused paths + // + // SanitizeAssetPath answers a traversal path with null, which every AssetDatabase + // entry point accepts, so each action used to describe the result of a lookup that + // never happened. These pin the refusal itself. + // ===================================================================== + + [TestCase("get_info")] + [TestCase("slice_sheet")] + [TestCase("setup_clips")] + [TestCase("full_setup")] + public void PathEscapingAssets_IsRefusedInsteadOfLookedUp(string action) + { + var result = Run(new JObject + { + ["action"] = action, + ["path"] = $"{TempRoot}/../../../outside.png", + ["cols"] = 4, + ["clips"] = OneClip("walk", 0, 3), + ["output_dir"] = TempRoot, + }); + + Assert.IsFalse(result.Value("success")); + Assert.That(ErrorText(result), Does.Contain("..")); + } + + [Test] + public void SetupController_ClipPathEscapingAssets_SkipsThatClipAndSaysWhy() + { + var clips = BuildClips("badclippath", "idle", "walk"); + clips.Add(new JObject + { + ["name"] = "attack", + ["path"] = $"{TempRoot}/../../../outside.anim", + }); + + var result = SetupController(clips); + + // The refused entry must be reported as refused, not as merely missing. + Assert.IsTrue(result.Value("success")); + Assert.That(result["diagnostics"].ToString(), Does.Contain("CLIP_BAD_PATH")); + } + + // ===================================================================== + // Inline image bound + // ===================================================================== + + [Test] + public void GetInfo_SmallSheet_CarriesTheImageInline() + { + string path = CreateSheet("inline", 4, 2); + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + Assert.That(result.Value("image_base64"), Does.StartWith("data:image/png;base64,")); + Assert.IsNull(result.Value("image_omitted_reason")); + } + + [Test] + public void GetInfo_OversizeSheet_DropsTheImageAndSaysWhy() + { + // Noise, not a flat colour, which would compress below the ceiling; and a + // power-of-two side, since a Default-type import rescales anything else + // (measured: a 1200px sheet read back as 1024). + string path = CreateNoiseSheet("oversize", 2048); + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + Assert.IsTrue(result.Value("success"), "the call still answers"); + Assert.IsNull(result.Value("image_base64")); + Assert.That(result.Value("image_omitted_reason"), Does.Contain("limit")); + // Everything a caller needs to work out a grid is still here. + Assert.AreEqual(2048, result.Value("width")); + Assert.AreEqual(2048, result.Value("height")); + } + + [Test] + public void GetInfo_ImageJustUnderTheSourceLimit_StillDoesNotBlowThePayloadLimit() + { + // The ceiling is checked against the file, but base64 emits 4 bytes for every 3, + // so a source under the limit still produces a payload above it. + string path = CreateNoiseSheet("midsize", 1024, assertOverCeiling: false); + long sourceBytes = new FileInfo(Path.Combine( + Directory.GetParent(Application.dataPath).FullName, path)).Length; + Assert.Less(sourceBytes, 4 * 1024 * 1024, + "fixture: this sheet must pass the source-size check to test what happens after it"); + + var result = Run(new JObject { ["action"] = "get_info", ["path"] = path }); + + string b64 = result.Value("image_base64"); + if (b64 != null) + Assert.LessOrEqual(System.Text.Encoding.UTF8.GetByteCount(b64), 4 * 1024 * 1024, + $"inline payload is {System.Text.Encoding.UTF8.GetByteCount(b64)} bytes " + + $"from a {sourceBytes}-byte source; the bound must cover what is sent, not what was read"); + else + Assert.IsNotEmpty(result.Value("image_omitted_reason") ?? "", + "an omitted image must say why"); + } + + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs.meta new file mode 100644 index 000000000..c5b02b23c --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSpriteTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 14ddca8684ebb412985c717045378811 \ No newline at end of file diff --git a/website/docs/reference/tools/animation/index.md b/website/docs/reference/tools/animation/index.md index f2a57be64..79bdbc270 100644 --- a/website/docs/reference/tools/animation/index.md +++ b/website/docs/reference/tools/animation/index.md @@ -9,3 +9,4 @@ description: "MCP for Unity tools in the animation group." Animator control & AnimationClip creation - **[`manage_animation`](./manage_animation.md)** — Manage Unity animation: Animator control and AnimationClip creation. +- **[`manage_sprite`](./manage_sprite.md)** — 2D sprite animation tool. get_info: read sprite import settings + return image for vision analysis; the slice list is paged (page_size / cursor). slice_sheet: apply grid slicing to a sprite sheet. setup_clips: create AnimationClips from… diff --git a/website/docs/reference/tools/animation/manage_sprite.md b/website/docs/reference/tools/animation/manage_sprite.md new file mode 100644 index 000000000..bcef2e1c2 --- /dev/null +++ b/website/docs/reference/tools/animation/manage_sprite.md @@ -0,0 +1,111 @@ +--- +title: manage_sprite +sidebar_label: manage_sprite +description: "2D sprite animation tool. get_info: read sprite import settings + return image for vision analysis; the slice list is paged (page_size / cursor). slice_sheet: apply grid slicing to a sprite sheet. setup_clips: create AnimationClips from…" +--- + +# `manage_sprite` + +> **Auto-generated** from the Python tool registry. Do not hand-edit outside `` blocks — the generator (`tools/generate_docs_reference.py`) will overwrite them. + +**Group:** `animation`  ·  **Module:** `services.tools.manage_sprite` + +## Description + +2D sprite animation tool. get_info: read sprite import settings + return image for vision analysis; the slice list is paged (page_size / cursor). slice_sheet: apply grid slicing to a sprite sheet. setup_clips: create AnimationClips from sliced sprites. setup_controller: build AnimatorController with smart complexity (1D blend tree for locomotion, trigger states for combat, simple state for single animations). full_setup: one command — slice → clips → controller. + +## Parameters + +| Name | Type | Required | Description | +|------|------|----------|-------------| +| `action` | `Literal['get_info', 'slice_sheet', 'setup_clips', 'setup_controller', 'full_setup']` | yes | Action to perform. | +| `path` | `str \| None` | — | Sprite texture asset path (e.g. 'Assets/Sprites/hero_walk.png'). Required for get_info, slice_sheet, setup_clips, full_setup. | +| `cols` | `int \| None` | — | Number of columns in the sprite sheet grid. Used by slice_sheet and full_setup. | +| `rows` | `int \| None` | — | Number of rows in the sprite sheet grid. Default: 1. | +| `frame_width` | `int \| None` | — | Frame width in pixels. Alternative to cols. | +| `frame_height` | `int \| None` | — | Frame height in pixels. Alternative to rows. | +| `base_name` | `str \| None` | — | Base name for sliced sprite frames (default: texture filename). | +| `clips` | `list[dict[str, Any]] \| None` | — | Clip definitions: [{name, start_frame, end_frame, fps (default 12), loop (auto-detect if omitted)}]. For setup_controller: [{name, path}] where path is the .anim asset path. | +| `animation_name` | `str \| None` | — | Animation name for full_setup when clips are not specified (all frames = one clip). | +| `output_dir` | `str \| None` | — | Output directory for .anim and .controller assets (default: same folder as sprite). | +| `controller_path` | `str \| None` | — | Path for the .controller asset (e.g. 'Assets/Animators/Hero.controller'). | +| `overwrite` | `bool` | — | Replace an existing .anim or .controller at the target path. Off by default: without it an existing asset is kept and reported back, not silently replaced. | +| `add_to_scene` | `bool` | — | Attach Animator + controller to a scene GameObject. | +| `scene_target` | `str \| None` | — | Existing GameObject name to attach Animator to. | +| `page_size` | `int \| None` | — | get_info: how many entries of the 'slices' list to return (1-4096, default 512). A sheet sliced by hand can hold more slices than one response should carry. | +| `cursor` | `int \| None` | — | get_info: index to start the 'slices' page at. Pass back the 'next_cursor' from the previous response; absent next_cursor means the list is finished. The image is returned only on the first page. | + +## Returns + +A `dict` containing the Unity response. The exact shape depends on the action. + +## Examples + + +### Read the sheet before slicing it + +The grid is the one thing the tool cannot infer. `get_info` returns the texture's +dimensions and the sheet itself as `image_base64`, so a vision-capable caller can count the +frames before committing to a grid. + +```json +{ "action": "get_info", "path": "Assets/Sprites/hero_walk.png" } +``` + +The `slices` list is paged. A sheet can hold more entries than one response should carry — +`slice_sheet` alone allows up to 4096 — so `slice_count` reports the total and +`next_cursor` appears only while entries remain. Follow it whenever it is present rather +than assuming a sheet arrives whole. +Walk it by passing the previous `next_cursor` back; the image comes with the first page +only, since it is the same picture on every one. + +```json +{ "action": "get_info", "path": "Assets/Sprites/atlas.png", "cursor": 512 } +``` + +### One command from sheet to controller + +```json +{ + "action": "full_setup", + "path": "Assets/Sprites/hero.png", + "cols": 6, + "rows": 4, + "clips": [ + { "name": "idle", "start_frame": 0, "end_frame": 5 }, + { "name": "walk", "start_frame": 6, "end_frame": 11 }, + { "name": "run", "start_frame": 12, "end_frame": 17 }, + { "name": "attack", "start_frame": 18, "end_frame": 23, "fps": 18 } + ], + "controller_path": "Assets/Animators/Hero.controller", + "add_to_scene": true, + "scene_target": "Hero" +} +``` + +Clip names decide the controller's shape: `idle` becomes the default state, `walk` and +`run` collapse into a `Speed`-driven 1D blend tree, and `attack` gets an `Attack` trigger. +Looping follows from the same names — locomotion and idle loop, a one-shot does not — and +an explicit `"loop"` on a clip overrides that. + +### Slicing on its own + +```json +{ "action": "slice_sheet", "path": "Assets/Sprites/hero.png", "frame_width": 32, "frame_height": 32 } +``` + +`frame_width`/`frame_height` are the alternative to `cols`/`rows`; supply either pair. A +grid that does not fit inside the texture is refused rather than silently dropping the +frames that fall outside it. + +### Replacing what is already there + +Existing `.anim` and `.controller` assets are kept unless `overwrite` is set, so a repeated +`full_setup` reports what it found instead of overwriting work: + +```json +{ "action": "setup_clips", "path": "Assets/Sprites/hero.png", + "clips": [{ "name": "walk", "start_frame": 0, "end_frame": 5 }], "overwrite": true } +``` + + diff --git a/website/docs/reference/tools/index.md b/website/docs/reference/tools/index.md index a7466a134..9f37f38b2 100644 --- a/website/docs/reference/tools/index.md +++ b/website/docs/reference/tools/index.md @@ -12,9 +12,10 @@ description: Auto-generated catalog of every MCP for Unity tool, grouped by doma Every tool MCP for Unity exposes, generated directly from the Python `@mcp_for_unity_tool` registry under `Server/src/services/tools/`. -## `animation`   (1 tool) +## `animation`   (2 tools) Animator control & AnimationClip creation - **[`manage_animation`](./animation/manage_animation.md)** — Manage Unity animation: Animator control and AnimationClip creation. +- **[`manage_sprite`](./animation/manage_sprite.md)** — 2D sprite animation tool. get_info: read sprite import settings + return image for vision analysis; the slice list is paged (page_size / cursor). slice_sheet: apply grid slicing to a sprite sheet. setup_clips: create AnimationClips from… ## `asset_gen`   (5 tools) AI asset generation – 3D model gen/import, 2D image gen & audio gen (bring-your-own-key)