feat: add support for unmangled names and nested merging in MergeArmature

Assets/_ModularAvatar/EditModeTests/MergeArmatureTests/MultiLevelMergeTest.cs

@@ -0,0 +1,109 @@
+using System;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using nadena.dev.modular_avatar.core;
+using nadena.dev.modular_avatar.core.editor;
+using NUnit.Framework;
+using UnityEngine;
+using VRC.SDK3.Avatars.Components;
+
+namespace modular_avatar_tests.MergeArmatureTests
+{
+    public class TestComponentA : MonoBehaviour
+    {
+    }
+
+    public class TestComponentB : MonoBehaviour
+    {
+    }
+
+    public class MarkDestroy : MonoBehaviour
+    {
+        private void OnDestroy()
+        {
+            Debug.Log("blah");
+        }
+    }
+
+    public class MultiLevelMergeTest : TestBase
+    {
+        [Test]
+        public void mergeProcessesInTopoOrder()
+        {
+            var root = CreateRoot("root");
+            var armature = CreateChild(root, "Armature");
+            var bone = CreateChild(armature, "Bone");
+
+            var merge1 = CreateChild(root, "merge1");
+            var m1_bone = CreateChild(merge1, "Bone");
+            var m1_leaf = CreateChild(m1_bone, "leaf1");
+            var m1_leaf2 = CreateChild(m1_leaf, "leaf2");
+
+            var merge2 = CreateChild(root, "merge2");
+            var m2_bone = CreateChild(merge2, "Bone");
+            var m2_leaf = CreateChild(m2_bone, "leaf1");
+            var m2_leaf3 = CreateChild(m2_leaf, "leaf3");
+
+            var ma1 = merge1.AddComponent<ModularAvatarMergeArmature>();
+            ma1.mergeTarget.referencePath = RuntimeUtil.AvatarRootPath(armature);
+
+            var ma2 = merge2.AddComponent<ModularAvatarMergeArmature>();
+            ma2.mergeTarget.referencePath = RuntimeUtil.AvatarRootPath(merge1);
+
+            m1_leaf2.AddComponent<TestComponentA>();
+            m2_leaf3.AddComponent<TestComponentB>();
+
+            BuildContext context = new BuildContext(root.GetComponent<VRCAvatarDescriptor>());
+            new MergeArmatureHook().OnPreprocessAvatar(context, root);
+
+            Assert.IsTrue(bone.GetComponentInChildren<TestComponentA>() != null);
+            Assert.IsTrue(bone.GetComponentInChildren<TestComponentB>() != null);
+            Assert.IsTrue(m2_leaf3.GetComponentsInParent<Transform>().Contains(m1_leaf.transform));
+        }
+
+        [Test]
+        public void canDisableNameMangling()
+        {
+            var root = CreateRoot("root");
+            var armature = CreateChild(root, "Armature");
+            var bone = CreateChild(armature, "Bone");
+
+            var merge = CreateChild(root, "merge");
+            var m_bone = CreateChild(merge, "Bone");
+            var m_leaf = CreateChild(m_bone, "leaf");
+
+            //m_bone.AddComponent<MarkDestroy>();
+
+            var ma = merge.AddComponent<ModularAvatarMergeArmature>();
+            ma.mergeTarget.referencePath = RuntimeUtil.AvatarRootPath(armature);
+            ma.mangleNames = false;
+
+            BuildContext context = new BuildContext(root.GetComponent<VRCAvatarDescriptor>());
+            new MergeArmatureHook().OnPreprocessAvatar(context, root);
+
+            Assert.IsTrue(m_bone == null); // destroyed by retargeting pass
+            Assert.IsTrue(m_leaf.transform.name == "leaf");
+        }
+
+        [Test]
+        public void manglesByDefault()
+        {
+            var root = CreateRoot("root");
+            var armature = CreateChild(root, "Armature");
+            var bone = CreateChild(armature, "Bone");
+
+            var merge = CreateChild(root, "merge");
+            var m_bone = CreateChild(merge, "Bone");
+            var m_leaf = CreateChild(m_bone, "leaf");
+
+            var ma = merge.AddComponent<ModularAvatarMergeArmature>();
+            ma.mergeTarget.referencePath = RuntimeUtil.AvatarRootPath(armature);
+
+            BuildContext context = new BuildContext(root.GetComponent<VRCAvatarDescriptor>());
+            new MergeArmatureHook().OnPreprocessAvatar(context, root);
+
+            Assert.IsTrue(m_bone == null); // destroyed by retargeting pass
+            Assert.IsTrue(m_leaf.transform.name != "leaf");
+        }
+    }
+}

Assets/_ModularAvatar/EditModeTests/MergeArmatureTests/MultiLevelMergeTest.cs.meta

@@ -0,0 +1,3 @@
+fileFormatVersion: 2
+guid: 705b64bd35654fcd819c614fca5caff4
+timeCreated: 1690614538

Packages/nadena.dev.modular-avatar/Editor/Inspector/MergeArmatureEditor.cs

@@ -8,7 +8,7 @@ namespace nadena.dev.modular_avatar.core.editor
     [CustomEditor(typeof(ModularAvatarMergeArmature))]
     internal class MergeArmatureEditor : MAEditorBase
     {
-        private SerializedProperty prop_mergeTarget, prop_prefix, prop_suffix, prop_locked;
+        private SerializedProperty prop_mergeTarget, prop_prefix, prop_suffix, prop_locked, prop_mangleNames;
 
         private void OnEnable()
         {
@@ -16,6 +16,7 @@ namespace nadena.dev.modular_avatar.core.editor
             prop_prefix = serializedObject.FindProperty(nameof(ModularAvatarMergeArmature.prefix));
             prop_suffix = serializedObject.FindProperty(nameof(ModularAvatarMergeArmature.suffix));
             prop_locked = serializedObject.FindProperty(nameof(ModularAvatarMergeArmature.locked));
+            prop_mangleNames = serializedObject.FindProperty(nameof(ModularAvatarMergeArmature.mangleNames));
         }
 
         private void ShowParametersUI()
@@ -25,6 +26,7 @@ namespace nadena.dev.modular_avatar.core.editor
             EditorGUILayout.PropertyField(prop_mergeTarget, G("merge_armature.merge_target"));
             EditorGUILayout.PropertyField(prop_prefix, G("merge_armature.prefix"));
             EditorGUILayout.PropertyField(prop_suffix, G("merge_armature.suffix"));
+            EditorGUILayout.PropertyField(prop_mangleNames, G("merge_armature.mangle_names"));
             EditorGUILayout.PropertyField(prop_locked, G("merge_armature.locked"));
 
             serializedObject.ApplyModifiedProperties();

Packages/nadena.dev.modular-avatar/Editor/Localization/en.json

@@ -39,6 +39,8 @@
   "merge_armature.locked.tooltip": "Lock the position of this armature's bones to the target armature (and vice versa). Useful for creating animations.",
   "merge_armature.adjust_names": "Adjust bone names to match target",
   "merge_armature.adjust_names.tooltip": "Changes bone names to match the target avatar. Useful for porting outfits from one avatar to another.",
+  "merge_armature.mangle_names": "Avoid name collisions",
+  "merge_armature.mangle_names.tooltip": "Avoid name collisions with other assets by mangling newly added bone names.",
   "path_mode.Relative": "Relative to this object",
   "path_mode.Absolute": "Absolute (based on avatar root)",
   "merge_animator.animator": "Animator to merge",

Packages/nadena.dev.modular-avatar/Editor/Localization/ja.json

@@ -38,6 +38,8 @@
   "merge_armature.locked.tooltip": "このオブジェクトのボーンを統合先のボーンに常に相互的に位置を合わせる。アニメーション制作向け",
   "merge_armature.adjust_names": "ボーン名を統合先に合わせる",
   "merge_armature.adjust_names.tooltip": "統合先のボーン名に合わせて、衣装のボーン名を合わせて変更します。統合先アバターに非対応の衣装導入向け機能です。",
+  "merge_armature.mangle_names": "名前かぶりを回避",
+  "merge_armature.mangle_names.tooltip": "ほかのアセットとの名前かぶりを裂けるため、新規ボーンの名前を自動で変更する",
   "path_mode.Relative": "相対的（このオブジェクトからのパスを使用）",
   "path_mode.Absolute": "絶対的（アバタールートからのパスを使用）",
   "merge_animator.animator": "統合されるアニメーター",

Packages/nadena.dev.modular-avatar/Editor/MergeArmatureHook.cs

@@ -46,19 +46,10 @@ namespace nadena.dev.modular_avatar.core.editor
         {
             this.context = context;
 
-            var mergeArmatures = avatarGameObject.transform.GetComponentsInChildren<ModularAvatarMergeArmature>(true);
+            var mergeArmatures =
+                avatarGameObject.transform.GetComponentsInChildren<ModularAvatarMergeArmature>(true);
 
-            foreach (var mergeArmature in mergeArmatures)
-            {
-                BuildReport.ReportingObject(mergeArmature, () =>
-                {
-                    mergedObjects.Clear();
-                    thisPassAdded.Clear();
-                    MergeArmature(mergeArmature);
-                    PruneDuplicatePhysBones();
-                    UnityEngine.Object.DestroyImmediate(mergeArmature);
-                });
-            }
+            TopoProcessMergeArmatures(mergeArmatures);
 
             foreach (var c in avatarGameObject.transform.GetComponentsInChildren<VRCPhysBone>(true))
             {
@@ -86,6 +77,90 @@ namespace nadena.dev.modular_avatar.core.editor
             new RetargetMeshes().OnPreprocessAvatar(avatarGameObject, context);
         }
 
+        private void TopoProcessMergeArmatures(ModularAvatarMergeArmature[] mergeArmatures)
+        {
+            Dictionary<ModularAvatarMergeArmature, List<ModularAvatarMergeArmature>> runsBefore
+                = new Dictionary<ModularAvatarMergeArmature, List<ModularAvatarMergeArmature>>();
+
+            foreach (var config in mergeArmatures)
+            {
+                // TODO - assert that we're not nesting merge armatures?
+
+                var target = config.mergeTargetObject;
+                if (target == null)
+                {
+                    // TODO - report error
+                    continue;
+                }
+
+                var parentConfig = target.GetComponentInParent<ModularAvatarMergeArmature>();
+                if (parentConfig != null)
+                {
+                    if (!runsBefore.ContainsKey(parentConfig))
+                    {
+                        runsBefore[parentConfig] = new List<ModularAvatarMergeArmature>();
+                    }
+
+                    runsBefore[parentConfig].Add(config);
+                }
+            }
+
+            HashSet<ModularAvatarMergeArmature> visited = new HashSet<ModularAvatarMergeArmature>();
+            Stack<ModularAvatarMergeArmature> visitStack = new Stack<ModularAvatarMergeArmature>();
+            foreach (var next in mergeArmatures)
+            {
+                TopoLoop(next);
+            }
+
+            void TopoLoop(ModularAvatarMergeArmature config)
+            {
+                if (visited.Contains(config)) return;
+                if (visitStack.Contains(config))
+                {
+                    BuildReport.LogFatal("merge_armature.circular_dependency", new string[0], config);
+                    return;
+                }
+
+                visitStack.Push(config);
+                var target = config.mergeTargetObject;
+
+                if (target != null)
+                {
+                    if (runsBefore.TryGetValue(config, out var predecessors))
+                    {
+                        foreach (var priorConfig in predecessors)
+                        {
+                            TopoLoop(priorConfig);
+                        }
+                    }
+
+                    MergeArmatureWithReporting(config);
+                }
+
+                visitStack.Pop();
+                visited.Add(config);
+            }
+        }
+
+        private void MergeArmatureWithReporting(ModularAvatarMergeArmature config)
+        {
+            var target = config.mergeTargetObject;
+
+            while (BoneDatabase.IsRetargetable(target.transform))
+            {
+                target = target.transform.parent.gameObject;
+            }
+
+            BuildReport.ReportingObject(config, () =>
+            {
+                mergedObjects.Clear();
+                thisPassAdded.Clear();
+                MergeArmature(config, target);
+                PruneDuplicatePhysBones();
+                UnityEngine.Object.DestroyImmediate(config);
+            });
+        }
+
         private void RetainBoneReferences(Component c)
         {
             if (c == null) return;
@@ -168,14 +243,14 @@ namespace nadena.dev.modular_avatar.core.editor
         private Dictionary<string, List<GameObject>>
             activationPathMappings = new Dictionary<string, List<GameObject>>();
 
-        private void MergeArmature(ModularAvatarMergeArmature mergeArmature)
+        private void MergeArmature(ModularAvatarMergeArmature mergeArmature, GameObject mergeTargetObject)
         {
-            // TODO: error reporting framework?
-            if (mergeArmature.mergeTargetObject == null) return;
+            // TODO: error reporting?
+            if (mergeTargetObject == null) return;
 
             GatherActiveStatePaths(mergeArmature.transform);
 
-            RecursiveMerge(mergeArmature, mergeArmature.gameObject, mergeArmature.mergeTargetObject.gameObject, true);
+            RecursiveMerge(mergeArmature, mergeArmature.gameObject, mergeTargetObject, true);
 
             FixupAnimations();
         }
@@ -329,7 +404,11 @@ namespace nadena.dev.modular_avatar.core.editor
             }
 
             src.transform.SetParent(mergedSrcBone.transform, true);
-            src.name = src.name + "$" + Guid.NewGuid();
+            if (config.mangleNames)
+            {
+                src.name = src.name + "$" + Guid.NewGuid();
+            }
+
             src.GetOrAddComponent<ModularAvatarPBBlocker>();
             mergedSrcBone = src;
 

Packages/nadena.dev.modular-avatar/Editor/MeshRetargeter.cs

@@ -34,37 +34,42 @@ namespace nadena.dev.modular_avatar.core.editor
 {
     internal static class BoneDatabase
     {
-        private static Dictionary<Transform, bool> IsRetargetable = new Dictionary<Transform, bool>();
+        private static Dictionary<Transform, bool> m_IsRetargetable = new Dictionary<Transform, bool>();
 
         internal static void ResetBones()
         {
-            IsRetargetable.Clear();
+            m_IsRetargetable.Clear();
+        }
+
+        internal static bool IsRetargetable(Transform t)
+        {
+            return m_IsRetargetable.TryGetValue(t, out var result) && result;
         }
 
         internal static void AddMergedBone(Transform bone)
         {
-            IsRetargetable[bone] = true;
+            m_IsRetargetable[bone] = true;
         }
 
         internal static void RetainMergedBone(Transform bone)
         {
             if (bone == null) return;
-            if (IsRetargetable.ContainsKey(bone)) IsRetargetable[bone] = false;
+            if (m_IsRetargetable.ContainsKey(bone)) m_IsRetargetable[bone] = false;
         }
 
         internal static Transform GetRetargetedBone(Transform bone)
         {
-            if (bone == null || !IsRetargetable.ContainsKey(bone)) return null;
+            if (bone == null || !m_IsRetargetable.ContainsKey(bone)) return null;
 
-            while (bone != null && IsRetargetable.ContainsKey(bone) && IsRetargetable[bone]) bone = bone.parent;
+            while (bone != null && m_IsRetargetable.ContainsKey(bone) && m_IsRetargetable[bone]) bone = bone.parent;
 
-            if (IsRetargetable.ContainsKey(bone)) return null;
+            if (m_IsRetargetable.ContainsKey(bone)) return null;
             return bone;
         }
 
         internal static IEnumerable<KeyValuePair<Transform, Transform>> GetRetargetedBones()
         {
-            return IsRetargetable.Where((kvp) => kvp.Value)
+            return m_IsRetargetable.Where((kvp) => kvp.Value)
                 .Select(kvp => new KeyValuePair<Transform, Transform>(kvp.Key, GetRetargetedBone(kvp.Key)))
                 .Where(kvp => kvp.Value != null);
         }

Packages/nadena.dev.modular-avatar/Editor/Util.cs

@@ -30,6 +30,7 @@ using JetBrains.Annotations;
 using UnityEditor;
 using UnityEditor.Animations;
 using UnityEngine;
+using VRC.SDK3.Avatars.Components;
 using VRC.SDKBase.Editor.BuildPipeline;
 using Object = UnityEngine.Object;
 
@@ -238,6 +239,18 @@ namespace nadena.dev.modular_avatar.core.editor
             return ValidateExpressionMenuIconResult.Success;
         }
 
+        internal static IEnumerable<T> FindComponentInParents<T>(this Component t) where T : Component
+        {
+            Transform ptr = t.transform.parent;
+            while (ptr != null)
+            {
+                var component = ptr.GetComponent<T>();
+                if (component != null) yield return component;
+                if (ptr.GetComponent<VRCAvatarDescriptor>() != null) break;
+                ptr = ptr.parent;
+            }
+        }
+
         internal static IEnumerable<AnimatorState> States(AnimatorController ac)
         {
             HashSet<AnimatorStateMachine> visitedStateMachines = new HashSet<AnimatorStateMachine>();

Packages/nadena.dev.modular-avatar/Runtime/ModularAvatarMergeArmature.cs

@@ -40,12 +40,13 @@ namespace nadena.dev.modular_avatar.core
         private const float POS_EPSILON = 0.001f * 0.001f;
         private const float ROT_EPSILON = 0.001f * 0.001f;
 
-        public AvatarObjectReference mergeTarget;
+        public AvatarObjectReference mergeTarget = new AvatarObjectReference();
         public GameObject mergeTargetObject => mergeTarget.Get(this);
 
-        public string prefix;
-        public string suffix;
-        public bool locked;
+        public string prefix = "";
+        public string suffix = "";
+        public bool locked = false;
+        public bool mangleNames = true;
 
         private class BoneBinding
         {

docs/docs/reference/merge-armature.md

@@ -32,6 +32,9 @@ Merge Armature goes to a lot of trouble to ensure that components configured on
 Merge Armature will leave portions of the original hierarchy behind - specifically, if they contain any components other than Transforms, they will be retained, and otherwise will generally be deleted.
 Where necessary, PhysBone objects will have their targets updated, and ParentConstraints may be created as necessary to make things Just Work (tm).
 
+As of Modular Avatar 1.7.0, it is now possible to perform nested merges - that is, merge an armature A into B, then 
+merge B into C. Modular Avatar will automatically determine the correct order to apply these merges.
+
 ## Locked mode
 
 If the locked option is enabled, the position and rotation of the merged bones will be locked to their parents in the editor. This is a two-way relationship; if you move the merged bone, the avatar's bone will move, and vice versa.
@@ -48,3 +51,12 @@ This allows the merge armature component to automatically restore its Merge Targ
 Since Merge Animator will attempt to match bones by name, just attaching it won't always work to make an outfit designed for one avatar work with another avatar.
 You can click the "Adjust bone names to match target" button to attempt to rename bones in the outfit to match the base avatar it's currently attached to.
 This will be done automatically if you added the Merge Armature component using the "Setup Outfit" menu item.
+
+## Avoid name collisions
+
+Although merge armature merges bones that have names matching ones on the merge target, by default any _newly added_
+bones that are unique to this new merged asset will have their names changed. This helps avoid conflicting with other
+assets that also use merge armature, and which happen to have chosen the same bone name.
+
+In some special circumstances, it can be helpful to disable this behavior. In those cases, you can uncheck the "avoid 
+name collisions" box.

docs/i18n/ja/docusaurus-plugin-content-docs/current/reference/merge-armature.md

@@ -35,6 +35,9 @@ Merge Armatureは衣装アセット専用のもので、Skinned Mesh Rendererの
 Transform以外のコンポーネントが入っているボーンがある場合、そのボーンが残ります。そのほかのボーンは原則として統合後に削除されます。
 必要に応じてPhysBoneのターゲットが調整されたり、ParentConstraintが生成されることで、なんとなく動くようになります。
 
+Modular Avatar 1.7.0以降、連鎖的に統合することができます。つまり、AをBに統合して、BをCに統合することができます。
+統合参照に応じてModular Avatarが自動的に統合順序を計算します。
+
 ## 位置を固定
 
 位置を固定を設定すると、統合先のボーンがエディタ上で元のボーンと同じ位置になります。これは総合的な関係で、どちらかのボーンが
@@ -51,3 +54,10 @@ Transform以外のコンポーネントが入っているボーンがある場
 Merge Animatorがボーンを名前で照合するので、つけるだけでは非対応衣装がうまく動かない場合があります。
 対策として、「ボーン名を統合先に合わせる」ボタンを押すことで、衣装側のボーン名を自動的にアバターのボーン名に合わせようとします。
 なお、「Setup outfit」でMerge Armatureをつける場合はこの処理が自動的に走ります。
+
+## 名前かぶりを回避
+
+統合先と一致する名前のボーンがある場合は統合しますが、統合元のボーンのうち統合先にないものは名前を変更した上で配置します。
+これは、統合先と偶然同じ名前のボーンがある他のアセットと衝突することを避けるためです。
+
+特殊な場合でこの処理を無効にしたい場合は「名前かぶりを回避」のチェックを外してください。