fix: fix issues with nested armature confusion in Easy Setup Outfit (#469)

This fixes issues with nested armature confusion by changing the name of the
Armature object (only).

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion.meta

@@ -0,0 +1,8 @@
+fileFormatVersion: 2
+guid: e8745411e14ca634e87ffbf19723aab7
+folderAsset: yes
+DefaultImporter:
+  externalObjects: {}
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: 

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/ArmatureConfusionTest.cs

@@ -0,0 +1,60 @@
+using modular_avatar_tests;
+using nadena.dev.modular_avatar.core.editor;
+using NUnit.Framework;
+using UnityEditor;
+using UnityEngine;
+using VRC.SDK3.Avatars.Components;
+using UnityObject = UnityEngine.Object;
+
+public class ArmatureConfusionTest : TestBase
+{
+    [TearDown]
+    public void TearDown()
+    {
+        ESOErrorWindow.Suppress = false;
+    }
+
+    [Test]
+    public void TestArmatureConfusionWorkaround()
+    {
+        ESOErrorWindow.Suppress = true;
+
+        // Arrange for a confused armature
+        var outer = CreatePrefab("shapell.fbx");
+        var inner = CreatePrefab("shapell.fbx");
+
+        var outerAnimator = outer.GetComponent<Animator>();
+        outer.AddComponent<VRCAvatarDescriptor>();
+
+        inner.gameObject.name = "inner";
+        inner.transform.parent = outer.transform;
+
+        // Unity seems to determine which armature is the "true" armature by counting the number of bones that match
+        // the humanoid description, and finding the root which has the most matches. Let's confuse it a bit by removing
+        // some non-humanoid bones from the outer armature.
+        var outerTarget = outer.transform.Find("Armature/Hips/Tail");
+        UnityObject.DestroyImmediate(outerTarget.gameObject);
+
+        // Clear animator cache
+        var avatar = outerAnimator.avatar;
+        outerAnimator.avatar = null;
+        // ReSharper disable once Unity.InefficientPropertyAccess
+        outerAnimator.avatar = avatar;
+
+        // Verify that we're well and confused now
+        Assert.AreSame(
+            outerAnimator.GetBoneTransform(HumanBodyBones.Hips),
+            inner.transform.Find("Armature/Hips")
+        );
+
+        // Now do a setup outfit operation
+        Selection.activeGameObject = inner;
+        EasySetupOutfit.SetupOutfit(new MenuCommand(inner));
+
+        // Verify that we're not confused anymore
+        Assert.AreSame(
+            outerAnimator.GetBoneTransform(HumanBodyBones.Hips),
+            outer.transform.Find("Armature/Hips")
+        );
+    }
+}

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/ArmatureConfusionTest.cs.meta

@@ -0,0 +1,3 @@
+fileFormatVersion: 2
+guid: 01ff1e86623a409ba5248c5fb6a0b53e
+timeCreated: 1696244575

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/README.md

@@ -0,0 +1,2 @@
+shapell.fbx is from Shapell (https://booth.pm/ja/items/1349366) by lowteq
+Released under CC0 1.0 Universal (CC0 1.0) (https://creativecommons.org/publicdomain/zero/1.0/deed.ja)

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/README.md.meta

@@ -0,0 +1,3 @@
+fileFormatVersion: 2
+guid: c5e311d0ce414f56afac9ccff53b16c4
+timeCreated: 1696245763

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/material.mat

@@ -0,0 +1,77 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!21 &2100000
+Material:
+  serializedVersion: 6
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_Name: material
+  m_Shader: {fileID: 46, guid: 0000000000000000f000000000000000, type: 0}
+  m_ShaderKeywords: 
+  m_LightmapFlags: 4
+  m_EnableInstancingVariants: 0
+  m_DoubleSidedGI: 0
+  m_CustomRenderQueue: -1
+  stringTagMap: {}
+  disabledShaderPasses: []
+  m_SavedProperties:
+    serializedVersion: 3
+    m_TexEnvs:
+    - _BumpMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _DetailAlbedoMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _DetailMask:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _DetailNormalMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _EmissionMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _MainTex:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _MetallicGlossMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _OcclusionMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    - _ParallaxMap:
+        m_Texture: {fileID: 0}
+        m_Scale: {x: 1, y: 1}
+        m_Offset: {x: 0, y: 0}
+    m_Floats:
+    - _BumpScale: 1
+    - _Cutoff: 0.5
+    - _DetailNormalMapScale: 1
+    - _DstBlend: 0
+    - _GlossMapScale: 1
+    - _Glossiness: 0.5
+    - _GlossyReflections: 1
+    - _Metallic: 0
+    - _Mode: 0
+    - _OcclusionStrength: 1
+    - _Parallax: 0.02
+    - _SmoothnessTextureChannel: 0
+    - _SpecularHighlights: 1
+    - _SrcBlend: 1
+    - _UVSec: 0
+    - _ZWrite: 1
+    m_Colors:
+    - _Color: {r: 1, g: 1, b: 1, a: 1}
+    - _EmissionColor: {r: 0, g: 0, b: 0, a: 1}

Assets/_ModularAvatar/EditModeTests/ArmatureConfusion/material.mat.meta

@@ -0,0 +1,8 @@
+fileFormatVersion: 2
+guid: 14fe6d8ede57a1a45955b9a0181223ae
+NativeFormatImporter:
+  externalObjects: {}
+  mainObjectFileID: 2100000
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: 

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

@@ -11,10 +11,16 @@ namespace nadena.dev.modular_avatar.core.editor
     {
         private string header;
         private string[] messageGroups;
-        private static readonly GUIStyle buttonStyle, labelStyle;
+        private static GUIStyle buttonStyle, labelStyle;
         private const float SeparatorSize = 6f;
 
+        internal static bool Suppress = false;
+
         static ESOErrorWindow()
+        {
+        }
+
+        internal static void InitStyles()
         {
             buttonStyle = EditorStyles.miniButtonRight;
             labelStyle = EditorStyles.label;
@@ -24,15 +30,15 @@ namespace nadena.dev.modular_avatar.core.editor
             buttonStyle.fixedHeight = EditorGUIUtility.singleLineHeight * 1.5f;
         }
 
-        private void OnEnable()
-        {
-        }
-
         internal static void Show(
             string header,
             string[] messageGroups
         )
         {
+            if (Suppress) return;
+
+            InitStyles();
+
             var window = CreateInstance<ESOErrorWindow>();
             window.titleContent = new GUIContent("Setup Outfit");
             window.header = header;
@@ -99,14 +105,14 @@ namespace nadena.dev.modular_avatar.core.editor
         }
     }
 
-    internal class EasySetupOutfit
+    internal static class EasySetupOutfit
     {
         private const int PRIORITY = 49;
         private static string[] errorMessageGroups;
         private static string errorHeader;
 
         [MenuItem("GameObject/ModularAvatar/Setup Outfit", false, PRIORITY)]
-        static void SetupOutfit(MenuCommand cmd)
+        internal static void SetupOutfit(MenuCommand cmd)
         {
             if (!ValidateSetupOutfit())
             {
@@ -130,6 +136,23 @@ namespace nadena.dev.modular_avatar.core.editor
                 merge.LockMode = ArmatureLockMode.BaseToMerge;
                 merge.InferPrefixSuffix();
                 HeuristicBoneMapper.RenameBonesByHeuristic(merge);
+
+                var avatarRootMatchingArmature = avatarRoot.transform.Find(outfitArmature.gameObject.name);
+                if (merge.prefix == "" && merge.suffix == "" && avatarRootMatchingArmature != null)
+                {
+                    // We have an armature whose names exactly match the root armature - this can cause some serious
+                    // confusion in Unity's humanoid armature matching system. Fortunately, we can avoid this by
+                    // renaming a bone close to the root; this will ensure the number of matching bones is small, and
+                    // Unity's heuristics (apparently) will choose the base avatar's armature as the "true" armature.
+                    outfitArmature.name += ".1";
+
+                    // Also make sure to refresh the avatar's animator humanoid bone cache.
+                    var avatarAnimator = avatarRoot.GetComponent<Animator>();
+                    var humanDescription = avatarAnimator.avatar;
+                    avatarAnimator.avatar = null;
+                    // ReSharper disable once Unity.InefficientPropertyAccess
+                    avatarAnimator.avatar = humanDescription;
+                }
             }
 
             if (outfitRoot != null
@@ -298,7 +321,7 @@ namespace nadena.dev.modular_avatar.core.editor
                 };
                 return false;
             }
-            
+
             avatarHips = avatarAnimator.GetBoneTransform(HumanBodyBones.Hips)?.gameObject;
 
             if (avatarHips == null)
@@ -334,7 +357,7 @@ namespace nadena.dev.modular_avatar.core.editor
                 }
 
                 hipsCandidates.Add(avatarHips.name);
-                
+
                 // If that doesn't work out, we'll check for heuristic bone mapper mappings.
                 foreach (var hbm in HeuristicBoneMapper.BoneToNameMap[HumanBodyBones.Hips])
                 {