Merge pull request #1253 from anatawa12/fix-invalid-operation

fix: InvalidOperationException in AutoMergeSkinnedMesh
anatawa12 · Oct 12, 2024 · ed2e7cd · ed2e7cd
2 parents 1eb84e1 + 9b07d5e
commit ed2e7cd
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 1 deletion.
diff --git a/CHANGELOG-PRERELEASE.md b/CHANGELOG-PRERELEASE.md
@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog].
 ### Removed
 
 ### Fixed
+- InvalidOperationException with AutoMergeSkinnedMesh `#1253`
 
 ### Security
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,7 +70,7 @@ The format is based on [Keep a Changelog].
 - Typo in menu for creating Asset Description `#1213`
 - maxSquish cannot be configured for mergePB`#1231`
 - Avatar Descriptor can be removed by Avatar Optimizer in extreamely rare case `#1242`
-- Material property animation with weight 0 layer might be broken with AutoMergeSkinnedMesh `#1248`
+- Material property animation with weight 0 layer might be broken with AutoMergeSkinnedMesh `#1248` `#1253`
 
 ### Security
 

diff --git a/Editor/Processors/TraceAndOptimize/AutoMergeSkinnedMesh.cs b/Editor/Processors/TraceAndOptimize/AutoMergeSkinnedMesh.cs
@@ -408,6 +408,7 @@ private static (Activeness, EqualsHashSet<(bool initial, EqualsHashSet<Animation
             foreach (var (property, node) in animationComponent.GetAllFloatProperties())
             {
                 if (property == Props.EnabledFor(typeof(SkinnedMeshRenderer))) continue; // m_Enabled is proceed separatedly
+                if (!node.ComponentNodes.Any()) continue; // skip empty nodes, likely means PPtr animation
                 if (node.ComponentNodes.Any(x => x is not AnimatorParsersV2.AnimatorPropModNode<AnimatorParsersV2.FloatValueInfo>))
                     return null;
 

diff --git a/Test~/Basic/AutoMergeSkinnedMeshTest.cs b/Test~/Basic/AutoMergeSkinnedMeshTest.cs
@@ -393,6 +393,33 @@ public void CategorizeMeshesForMerge_Split_NeverAppliedAnimationWithDifferentIni
             Assert.That(categorization.Values, Is.EquivalentTo(Enumerable.Empty<List<MeshInfo2>>()));
         }
 
+        [Test]
+        public void Issue1252NoCrashWithObjectReferenceCurve()
+        {
+            // initialize test avatar
+            var avatar = new TwoCubeAvatar(0);
+
+            TestUtils.SetFxLayer(avatar.avatar, BuildAnimatorController("")
+                .AddLayer("Base", _ => { })
+                .AddLayer("ZeroWeight", 0, sm => sm
+                    .NewClipState("State0", clip => clip
+                        .AddObjectReferenceBinding("Renderer0", typeof(SkinnedMeshRenderer),
+                            "m_Materials.Array.data[0]",
+                            (0, new Material(Shader.Find("Standard"))))))
+                .Build());
+
+            // preprocess
+            var buildContext = PreprocessAvatar(avatar.avatar);
+
+            // do process
+            AutoMergeSkinnedMesh.CategoryMeshesForMerge(buildContext, new List<MeshInfo2>()
+            {
+                buildContext.GetMeshInfoFor(avatar.renderer0), buildContext.GetMeshInfoFor(avatar.renderer1),
+            });
+
+            // No crash
+        }
+
         private static Mesh GetCubeMesh()
         {
             return AssetDatabase.LoadAllAssetsAtPath("Library/unity default resources")

diff --git a/Test~/Utils/AnimatorControllerGenerator.cs b/Test~/Utils/AnimatorControllerGenerator.cs
@@ -4,6 +4,7 @@
 using UnityEngine;
 
 using static Anatawa12.AvatarOptimizer.Test.AnimatorControllerGeneratorStatics;
+using Object = UnityEngine.Object;
 
 namespace Anatawa12.AvatarOptimizer.Test
 {
@@ -93,6 +94,17 @@ public AnimationClipBuilder AddPropertyBinding(string path, Type type, string pr
         public AnimationClipBuilder AddPropertyBinding(string path, Type type, string propertyName, params Keyframe[] keyframes) => 
             AddPropertyBinding(path, type, propertyName, new AnimationCurve(keyframes));
 
+        public AnimationClipBuilder AddObjectReferenceBinding(string path, Type type, string propertyName, params ObjectReferenceKeyframe[] keyframes)
+        {
+            AnimationUtility.SetObjectReferenceCurve(_clip, new EditorCurveBinding { path = path, type = type, propertyName = propertyName }, keyframes);
+            return this;
+        }
+
+        public AnimationClipBuilder AddObjectReferenceBinding(string path, Type type, string propertyName,
+            params (float time, Object value)[] keyframe) =>
+            AddObjectReferenceBinding(path, type, propertyName,
+                Array.ConvertAll(keyframe, k => new ObjectReferenceKeyframe { time = k.time, value = k.value }));
+
         public AnimationClip Build() => _clip;
     }
 }