From a68d60df354ef96b1cca4b882f2a5cc9371d1a64 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Mon, 23 Oct 2023 15:48:30 +0900 Subject: [PATCH 1/9] chore!: remove fallback animator parser --- Editor/AutomaticConfiguration.cs | 3 -- .../TraceAndOptimize/AnimatorParser.cs | 35 +++---------------- .../AnimatorParserDebugWindow.cs | 4 +-- .../TraceAndOptimizeProcessor.cs | 2 -- Runtime/TraceAndOptimize.cs | 7 ---- 5 files changed, 6 insertions(+), 45 deletions(-) diff --git a/Editor/AutomaticConfiguration.cs b/Editor/AutomaticConfiguration.cs index 967027fa7..c95e69941 100644 --- a/Editor/AutomaticConfiguration.cs +++ b/Editor/AutomaticConfiguration.cs @@ -11,7 +11,6 @@ internal class TraceAndOptimizeEditor : AvatarGlobalComponentEditorBase private SerializedProperty _removeUnusedObjects; private SerializedProperty _preserveEndBone; private SerializedProperty _mmdWorldCompatibility; - private SerializedProperty _advancedAnimatorParser; private SerializedProperty _advancedSettings; private GUIContent _advancedSettingsLabel = new GUIContent(); @@ -21,7 +20,6 @@ private void OnEnable() _removeUnusedObjects = serializedObject.FindProperty(nameof(TraceAndOptimize.removeUnusedObjects)); _preserveEndBone = serializedObject.FindProperty(nameof(TraceAndOptimize.preserveEndBone)); _mmdWorldCompatibility = serializedObject.FindProperty(nameof(TraceAndOptimize.mmdWorldCompatibility)); - _advancedAnimatorParser = serializedObject.FindProperty(nameof(TraceAndOptimize.advancedAnimatorParser)); _advancedSettings = serializedObject.FindProperty(nameof(TraceAndOptimize.advancedSettings)); } @@ -47,7 +45,6 @@ protected override void OnInspectorGUIInner() { EditorGUI.indentLevel++; EditorGUILayout.HelpBox(CL4EE.Tr("TraceAndOptimize:warn:advancedSettings"), MessageType.Warning); - EditorGUILayout.PropertyField(_advancedAnimatorParser); var iterator = _advancedSettings.Copy(); var enterChildren = true; while (iterator.NextVisible(enterChildren)) diff --git a/Editor/Processors/TraceAndOptimize/AnimatorParser.cs b/Editor/Processors/TraceAndOptimize/AnimatorParser.cs index b69ace89f..b0bdd9b47 100644 --- a/Editor/Processors/TraceAndOptimize/AnimatorParser.cs +++ b/Editor/Processors/TraceAndOptimize/AnimatorParser.cs @@ -18,25 +18,16 @@ namespace Anatawa12.AvatarOptimizer.Processors.TraceAndOptimizes class AnimatorParser { private bool mmdWorldCompatibility; - private bool advancedAnimatorParser; private AnimationParser _animationParser = new AnimationParser(); - public AnimatorParser(bool mmdWorldCompatibility, bool advancedAnimatorParser) + public AnimatorParser(bool mmdWorldCompatibility) { this.mmdWorldCompatibility = mmdWorldCompatibility; - this.advancedAnimatorParser = advancedAnimatorParser; - } - - public AnimatorParser(TraceAndOptimize config) - { - mmdWorldCompatibility = config.mmdWorldCompatibility; - advancedAnimatorParser = config.advancedAnimatorParser; } public AnimatorParser(TraceAndOptimizeState config) { mmdWorldCompatibility = config.MmdWorldCompatibility; - advancedAnimatorParser = !config.UseLegacyAnimatorParser; } public ImmutableModificationsContainer GatherAnimationModifications(BuildContext context) @@ -461,30 +452,12 @@ public IModificationsContainer ParseAnimatorController(GameObject root, RuntimeA { return ReportingObject(controller, () => { - if (advancedAnimatorParser) - { - var (animatorController, mapping) = GetControllerAndOverrides(controller); - return AdvancedParseAnimatorController(root, animatorController, mapping, - externallyWeightChanged); - } - else - { - return FallbackParseAnimatorController(root, controller); - } + var (animatorController, mapping) = GetControllerAndOverrides(controller); + return AdvancedParseAnimatorController(root, animatorController, mapping, + externallyWeightChanged); }); } - /// - /// Fallback AnimatorController Parser but always assumed as partially applied. - /// This process assumes everything is applied as non-additive state motion. - /// This parsing MAY not correct with direct blendtree or additive layer - /// but it's extremely rare case so ignoring such case. - /// - private IModificationsContainer FallbackParseAnimatorController(GameObject root, RuntimeAnimatorController controller) - { - return controller.animationClips.Select(clip => _animationParser.GetParsedAnimation(root, clip)).MergeContainersSideBySide(); - } - internal IModificationsContainer AdvancedParseAnimatorController(GameObject root, AnimatorController controller, IReadOnlyDictionary mapping, [CanBeNull] AnimatorLayerWeightMap externallyWeightChanged) diff --git a/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs b/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs index 978ee9a04..bdd096023 100644 --- a/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs +++ b/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs @@ -133,7 +133,7 @@ void OnParserSourceGUI() if (GUILayout.Button("Parse") && avatar) { parsedRootObject = avatar; - Container = new AnimatorParser(true, true).GatherAnimationModifications( + Container = new AnimatorParser(true).GatherAnimationModifications( new BuildContext(avatar, null)); } } @@ -148,7 +148,7 @@ void OnParserSourceGUI() if (GUILayout.Button("Parse") && animatorController && rootGameObject) { parsedRootObject = rootGameObject; - Container = new AnimatorParser(true, true) + Container = new AnimatorParser(true) .ParseAnimatorController(rootGameObject, animatorController) .ToImmutable(); } diff --git a/Editor/Processors/TraceAndOptimize/TraceAndOptimizeProcessor.cs b/Editor/Processors/TraceAndOptimize/TraceAndOptimizeProcessor.cs index c246f856a..7acc465e9 100644 --- a/Editor/Processors/TraceAndOptimize/TraceAndOptimizeProcessor.cs +++ b/Editor/Processors/TraceAndOptimize/TraceAndOptimizeProcessor.cs @@ -12,7 +12,6 @@ class TraceAndOptimizeState public bool MmdWorldCompatibility; public bool PreserveEndBone; - public bool UseLegacyAnimatorParser; public HashSet Exclusions = new HashSet(); public bool GCDebug; public bool NoConfigureMergeBone; @@ -36,7 +35,6 @@ public void Initialize(TraceAndOptimize config) PreserveEndBone = config.preserveEndBone; - UseLegacyAnimatorParser = !config.advancedAnimatorParser; Exclusions = new HashSet(config.advancedSettings.exclusions); GCDebug = config.advancedSettings.gcDebug; NoConfigureMergeBone = config.advancedSettings.noConfigureMergeBone; diff --git a/Runtime/TraceAndOptimize.cs b/Runtime/TraceAndOptimize.cs index 0a737f51c..ddd7bd90d 100644 --- a/Runtime/TraceAndOptimize.cs +++ b/Runtime/TraceAndOptimize.cs @@ -34,13 +34,6 @@ internal class TraceAndOptimize : AvatarGlobalComponent [ToggleLeft] public bool mmdWorldCompatibility = true; - // for compatibility, this is not inside AdvancedSettings but this is part of Advanced Settings - [NotKeyable] - [InspectorName("Use Advanced Animator Parser")] - [Tooltip("Advanced Animator Parser will parse your AnimatorController, including layer structure.")] - [ToggleLeft] - public bool advancedAnimatorParser = true; - [NotKeyable] public AdvancedSettings advancedSettings; From ccb7abce7cda17ca9c414abd034fe68ab475c37b Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Tue, 24 Oct 2023 11:56:17 +0900 Subject: [PATCH 2/9] chore: replace propertyId with AnimationProperty --- Editor/ObjectMapping/ObjectMappingBuilder.cs | 118 ++++++++++--------- 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs index abd828d60..d7540849c 100644 --- a/Editor/ObjectMapping/ObjectMappingBuilder.cs +++ b/Editor/ObjectMapping/ObjectMappingBuilder.cs @@ -71,19 +71,39 @@ public ObjectMapping BuildObjectMapping() _componentInfos.ToDictionary(p => p.Key, p => p.Value.Build())); } + class AnimationProperty + { + [CanBeNull] public readonly BuildingComponentInfo Component; + [CanBeNull] public readonly string Name; + [CanBeNull] public AnimationProperty MergedTo; + // TODO: add AnimationProperty[] CopiedTo and process later + + public AnimationProperty([NotNull] BuildingComponentInfo component, [NotNull] string name) + { + Component = component ?? throw new ArgumentNullException(nameof(component)); + Name = name ?? throw new ArgumentNullException(nameof(name)); + } + + private AnimationProperty() + { + } + + public static readonly AnimationProperty RemovedMarker = new AnimationProperty(); + } + class BuildingComponentInfo { private readonly int _instanceId; private readonly Type _type; - private readonly List MergeSources = new List(); // id in this -> id in merged private BuildingComponentInfo _mergedInto; - // renaming property tracker - private int _nextPropertyId = 1; - private readonly Dictionary _beforePropertyIds = new Dictionary(); - private readonly Dictionary _afterPropertyIds = new Dictionary(); + private readonly Dictionary _beforePropertyIds = + new Dictionary(); + + private readonly Dictionary _afterPropertyIds = + new Dictionary(); public BuildingComponentInfo(Component component) { @@ -91,68 +111,53 @@ public BuildingComponentInfo(Component component) _type = component.GetType(); } + [NotNull] + private AnimationProperty GetProperty(string name, bool remove = false) + { + if (_afterPropertyIds.TryGetValue(name, out var prop)) + { + if (remove) _afterPropertyIds.Remove(name); + return prop; + } + else + { + var newProp = new AnimationProperty(this, name); + if (!remove) _afterPropertyIds.Add(name, newProp); + if (!_beforePropertyIds.ContainsKey(name)) + _beforePropertyIds.Add(name, newProp); + return newProp; + } + } + public void MergedTo([NotNull] BuildingComponentInfo mergeTo) { if (_type == typeof(Transform)) throw new Exception("Merging Transform is not supported!"); - if (mergeTo == null) throw new ArgumentNullException(nameof(mergeTo)); if (_mergedInto != null) throw new InvalidOperationException("Already merged"); - mergeTo.MergeSources.Add(this); - _mergedInto = mergeTo; + _mergedInto = mergeTo ?? throw new ArgumentNullException(nameof(mergeTo)); + foreach (var property in _afterPropertyIds.Values) + property.MergedTo = mergeTo.GetProperty(property.Name); + _afterPropertyIds.Clear(); } public void MoveProperties(params (string old, string @new)[] props) { if (_type == typeof(Transform)) throw new Exception("Move properties of Transform is not supported!"); - foreach (var mergeSource in MergeSources) mergeSource.MoveProperties(props); + if (_mergedInto != null) throw new Exception("Already Merged"); - var propertyIds = new int[props.Length]; + var propertyIds = new AnimationProperty[props.Length]; for (var i = 0; i < props.Length; i++) - { - var (oldProp, newProp) = props[i]; - if (_afterPropertyIds.TryGetValue(oldProp, out var propId)) - { - propertyIds[i] = propId; - } - else - { - if (!_beforePropertyIds.ContainsKey(oldProp)) - { - propertyIds[i] = _nextPropertyId++; - } - } - } + propertyIds[i] = GetProperty(props[i].old, remove: true); for (var i = 0; i < propertyIds.Length; i++) - { - var propId = propertyIds[i]; - var (oldProp, _) = props[i]; - if (propId == 0) continue; - _afterPropertyIds.Remove(oldProp); - } - - for (var i = 0; i < propertyIds.Length; i++) - { - var propId = propertyIds[i]; - var (oldProp, newProp) = props[i]; - if (propId == 0) continue; - _afterPropertyIds[newProp] = propId; - if (!_beforePropertyIds.ContainsKey(oldProp)) - _beforePropertyIds.Add(oldProp, propId); - } + propertyIds[i].MergedTo = GetProperty(props[i].@new); } - public void RemoveProperty(string oldProp) + public void RemoveProperty(string property) { if (_type == typeof(Transform)) throw new Exception("Removing properties of Transform is not supported!"); - foreach (var mergeSource in MergeSources) mergeSource.RemoveProperty(oldProp); - // if (_afterPropertyIds.ContainsKey(oldProp)) - // _afterPropertyIds.Remove(oldProp); - // else - // if (!_beforePropertyIds.ContainsKey(oldProp)) - // _beforePropertyIds.Add(oldProp, _nextPropertyId++); - if (!_afterPropertyIds.Remove(oldProp)) - if (!_beforePropertyIds.ContainsKey(oldProp)) - _beforePropertyIds.Add(oldProp, _nextPropertyId++); + if (_mergedInto != null) throw new Exception("Already Merged"); + + GetProperty(property, remove: true).MergedTo = AnimationProperty.RemovedMarker; } public ComponentInfo Build() @@ -161,11 +166,18 @@ public ComponentInfo Build() while (mergedInfo._mergedInto != null) mergedInfo = mergedInfo._mergedInto; - var idToAfterName = _afterPropertyIds.ToDictionary(p => p.Value, p => p.Key); - var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, - p => idToAfterName.TryGetValue(p.Value, out var name) ? name : null); + var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, p => GetName(p.Value)); return new ComponentInfo(_instanceId, mergedInfo._instanceId, _type, propertyMapping); + + string GetName(AnimationProperty property) + { + while (property.MergedTo != null) + property = property.MergedTo; + if (property.Name == null) return null; + Debug.Assert(property.Component == mergedInfo); + return property.Name; + } } } } From 278484f53ea11be554ddaa4e216c8678489b2b3d Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Tue, 24 Oct 2023 16:50:50 +0900 Subject: [PATCH 3/9] feat: copying property to multiple properties --- Editor/ObjectMapping/AnimationObjectMapper.cs | 44 +++++++---- Editor/ObjectMapping/ObjectMapping.cs | 63 ++++++++++++++- Editor/ObjectMapping/ObjectMappingBuilder.cs | 77 ++++++++++++++----- Editor/ObjectMapping/ObjectMappingContext.cs | 46 +++++++---- 4 files changed, 180 insertions(+), 50 deletions(-) diff --git a/Editor/ObjectMapping/AnimationObjectMapper.cs b/Editor/ObjectMapping/AnimationObjectMapper.cs index 06c5b9c35..6d690f37e 100644 --- a/Editor/ObjectMapping/AnimationObjectMapper.cs +++ b/Editor/ObjectMapping/AnimationObjectMapper.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Anatawa12.AvatarOptimizer.Processors.TraceAndOptimizes; using JetBrains.Annotations; using UnityEditor; using UnityEngine; @@ -118,29 +119,40 @@ public string MapPath(string srcPath, Type type) } } - public EditorCurveBinding MapBinding(EditorCurveBinding binding) + [CanBeNull] + public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) { var gameObjectInfo = GetGameObjectInfo(binding.path); - if (gameObjectInfo == null) return binding; + if (gameObjectInfo == null) + return null; var (instanceId, componentInfo) = gameObjectInfo.GetComponentByType(binding.type); if (componentInfo != null) { - var component = EditorUtility.InstanceIDToObject(componentInfo.MergedInto) as Component; - // there's mapping about component. - // this means the component is merged or some prop has mapping - if (!component) return default; // this means removed. + if (!componentInfo.PropertyMapping.TryGetValue(binding.propertyName, out var newProp)) + return null; - var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); - if (newPath == null) return default; // this means moved to out of the animator scope + var curveBindings = new EditorCurveBinding[newProp.AllCopiedTo.Length]; + for (var i = 0; i < newProp.AllCopiedTo.Length; i++) + { + var descriptor = newProp.AllCopiedTo[i]; + var component = EditorUtility.InstanceIDToObject(descriptor.InstanceId) as Component; + // this means removed. + if (component == null) continue; - binding.path = newPath; + var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); - if (componentInfo.PropertyMapping.TryGetValue(binding.propertyName, out var newProp)) - { - if (newProp == null) return default; - binding.propertyName = newProp; + // this means moved to out of the animator scope + // TODO: add warning + if (newPath == null) return default; + + binding.path = newPath; + binding.type = descriptor.Type; + binding.propertyName = descriptor.Name; + curveBindings[i] = binding; // copy } + + return curveBindings; } else { @@ -152,11 +164,11 @@ public EditorCurveBinding MapBinding(EditorCurveBinding binding) if (!component) return default; // this means removed } - if (gameObjectInfo.NewPath == null) return default; + if (gameObjectInfo.NewPath == null) return Array.Empty(); + if (binding.path == gameObjectInfo.NewPath) return null; binding.path = gameObjectInfo.NewPath; + return new[] { binding }; } - - return binding; } } } diff --git a/Editor/ObjectMapping/ObjectMapping.cs b/Editor/ObjectMapping/ObjectMapping.cs index a2b8c0fe5..6f32f37c7 100644 --- a/Editor/ObjectMapping/ObjectMapping.cs +++ b/Editor/ObjectMapping/ObjectMapping.cs @@ -103,9 +103,10 @@ class ComponentInfo public readonly int InstanceId; public readonly int MergedInto; public readonly Type Type; - public readonly IReadOnlyDictionary PropertyMapping; + public readonly IReadOnlyDictionary PropertyMapping; - public ComponentInfo(int instanceId, int mergedInto, Type type, IReadOnlyDictionary propertyMapping) + public ComponentInfo(int instanceId, int mergedInto, Type type, + IReadOnlyDictionary propertyMapping) { InstanceId = instanceId; MergedInto = mergedInto; @@ -114,6 +115,64 @@ public ComponentInfo(int instanceId, int mergedInto, Type type, IReadOnlyDiction } } + readonly struct PropertyDescriptor : IEquatable + { + public static readonly PropertyDescriptor Removed = default; + public readonly int InstanceId; + [NotNull] public readonly Type Type; + [NotNull] public readonly string Name; + + public PropertyDescriptor(int instanceId, Type type, string name) + { + InstanceId = instanceId; + Type = type; + Name = name; + } + + public override int GetHashCode() + { + unchecked + { + var hashCode = InstanceId; + hashCode = (hashCode * 397) ^ Type.GetHashCode(); + hashCode = (hashCode * 397) ^ Name.GetHashCode(); + return hashCode; + } + } + + public bool Equals(PropertyDescriptor other) => + InstanceId == other.InstanceId && Type == other.Type && Name == other.Name; + public override bool Equals(object obj) => obj is PropertyDescriptor other && Equals(other); + public static bool operator ==(PropertyDescriptor left, PropertyDescriptor right) => left.Equals(right); + public static bool operator !=(PropertyDescriptor left, PropertyDescriptor right) => !left.Equals(right); + } + + readonly struct MappedPropertyInfo + { + public static readonly MappedPropertyInfo Removed = default; + public readonly PropertyDescriptor MappedProperty; + private readonly PropertyDescriptor[] _copiedTo; + + public PropertyDescriptor[] AllCopiedTo => _copiedTo ?? Array.Empty(); + + public MappedPropertyInfo(PropertyDescriptor property, PropertyDescriptor[] copiedTo) + { + MappedProperty = property; + _copiedTo = copiedTo; + } + + public MappedPropertyInfo(int mappedInstanceId, Type mappedType, string mappedName) : this( + new PropertyDescriptor(mappedInstanceId, mappedType, mappedName)) + { + } + + public MappedPropertyInfo(PropertyDescriptor property) + { + MappedProperty = property; + _copiedTo = new[] { property }; + } + } + static class VProp { private const string ExtraProps = "AvatarOptimizerExtraProps"; diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs index d7540849c..82e39aa67 100644 --- a/Editor/ObjectMapping/ObjectMappingBuilder.cs +++ b/Editor/ObjectMapping/ObjectMappingBuilder.cs @@ -76,7 +76,8 @@ class AnimationProperty [CanBeNull] public readonly BuildingComponentInfo Component; [CanBeNull] public readonly string Name; [CanBeNull] public AnimationProperty MergedTo; - // TODO: add AnimationProperty[] CopiedTo and process later + private MappedPropertyInfo? _mappedPropertyInfo; + public AnimationProperty[] CopiedTo = Array.Empty(); public AnimationProperty([NotNull] BuildingComponentInfo component, [NotNull] string name) { @@ -89,12 +90,56 @@ private AnimationProperty() } public static readonly AnimationProperty RemovedMarker = new AnimationProperty(); + + public MappedPropertyInfo GetMappedInfo() + { + if (_mappedPropertyInfo is MappedPropertyInfo property) return property; + property = ComputeMappedInfo(); + _mappedPropertyInfo = property; + return property; + } + + private MappedPropertyInfo ComputeMappedInfo() + { + if (this == RemovedMarker) return MappedPropertyInfo.Removed; + + System.Diagnostics.Debug.Assert(Component != null, nameof(Component) + " != null"); + + if (MergedTo != null) + { + var merged = MergedTo.GetMappedInfo(); + + if (CopiedTo.Length == 0) + return merged; + + var copied = new List(); + copied.AddRange(merged.AllCopiedTo); + foreach (var copiedTo in CopiedTo) + copied.AddRange(copiedTo.GetMappedInfo().AllCopiedTo); + + return new MappedPropertyInfo(merged.MappedProperty, copied.ToArray()); + } + else + { + // this is edge + if (CopiedTo.Length == 0) + return new MappedPropertyInfo(Component.InstanceId, Component.Type, Name); + + var descriptor = new PropertyDescriptor(Component.InstanceId, Component.Type, Name); + + var copied = new List { descriptor }; + foreach (var copiedTo in CopiedTo) + copied.AddRange(copiedTo.GetMappedInfo().AllCopiedTo); + + return new MappedPropertyInfo(descriptor, copied.ToArray()); + } + } } class BuildingComponentInfo { - private readonly int _instanceId; - private readonly Type _type; + internal readonly int InstanceId; + internal readonly Type Type; // id in this -> id in merged private BuildingComponentInfo _mergedInto; @@ -107,10 +152,12 @@ class BuildingComponentInfo public BuildingComponentInfo(Component component) { - _instanceId = component.GetInstanceID(); - _type = component.GetType(); + InstanceId = component.GetInstanceID(); + Type = component.GetType(); } + internal bool IsMerged => _mergedInto != null; + [NotNull] private AnimationProperty GetProperty(string name, bool remove = false) { @@ -131,7 +178,7 @@ private AnimationProperty GetProperty(string name, bool remove = false) public void MergedTo([NotNull] BuildingComponentInfo mergeTo) { - if (_type == typeof(Transform)) throw new Exception("Merging Transform is not supported!"); + if (Type == typeof(Transform)) throw new Exception("Merging Transform is not supported!"); if (_mergedInto != null) throw new InvalidOperationException("Already merged"); _mergedInto = mergeTo ?? throw new ArgumentNullException(nameof(mergeTo)); foreach (var property in _afterPropertyIds.Values) @@ -141,7 +188,7 @@ public void MergedTo([NotNull] BuildingComponentInfo mergeTo) public void MoveProperties(params (string old, string @new)[] props) { - if (_type == typeof(Transform)) throw new Exception("Move properties of Transform is not supported!"); + if (Type == typeof(Transform)) throw new Exception("Move properties of Transform is not supported!"); if (_mergedInto != null) throw new Exception("Already Merged"); var propertyIds = new AnimationProperty[props.Length]; @@ -154,7 +201,7 @@ public void MoveProperties(params (string old, string @new)[] props) public void RemoveProperty(string property) { - if (_type == typeof(Transform)) throw new Exception("Removing properties of Transform is not supported!"); + if (Type == typeof(Transform)) throw new Exception("Removing properties of Transform is not supported!"); if (_mergedInto != null) throw new Exception("Already Merged"); GetProperty(property, remove: true).MergedTo = AnimationProperty.RemovedMarker; @@ -166,18 +213,10 @@ public ComponentInfo Build() while (mergedInfo._mergedInto != null) mergedInfo = mergedInfo._mergedInto; - var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, p => GetName(p.Value)); - - return new ComponentInfo(_instanceId, mergedInfo._instanceId, _type, propertyMapping); + var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, + p => p.Value.GetMappedInfo()); - string GetName(AnimationProperty property) - { - while (property.MergedTo != null) - property = property.MergedTo; - if (property.Name == null) return null; - Debug.Assert(property.Component == mergedInfo); - return property.Name; - } + return new ComponentInfo(InstanceId, mergedInfo.InstanceId, Type, propertyMapping); } } } diff --git a/Editor/ObjectMapping/ObjectMappingContext.cs b/Editor/ObjectMapping/ObjectMappingContext.cs index e81843d4c..2486764aa 100644 --- a/Editor/ObjectMapping/ObjectMappingContext.cs +++ b/Editor/ObjectMapping/ObjectMappingContext.cs @@ -86,14 +86,14 @@ private static void VRCAvatarDescriptor(SerializedObject serialized, var indexProp = eyelidsBlendshapes.GetArrayElementAtIndex(i); if (info.PropertyMapping.TryGetValue( VProp.BlendShapeIndex(indexProp.intValue), - out var mappedPropName)) + out var mappedProp)) { - if (mappedPropName == null) + if (mappedProp.MappedProperty.Name == null) { BuildReport.LogFatal("ApplyObjectMapping:VRCAvatarDescriptor:eyelids BlendShape Removed"); return; } - indexProp.intValue = VProp.ParseBlendShapeIndex(mappedPropName); + indexProp.intValue = VProp.ParseBlendShapeIndex(mappedProp.MappedProperty.Name); } } } @@ -126,20 +126,40 @@ private Object CustomClone(Object o) foreach (var binding in AnimationUtility.GetCurveBindings(clip)) { - var newBinding = _mapping.MapBinding(binding); - _mapped |= newBinding != binding; - if (newBinding.type == null) continue; - newClip.SetCurve(newBinding.path, newBinding.type, newBinding.propertyName, - AnimationUtility.GetEditorCurve(clip, binding)); + var newBindings = _mapping.MapBinding(binding); + if (newBindings == null) + { + newClip.SetCurve(binding.path, binding.type, binding.propertyName, + AnimationUtility.GetEditorCurve(clip, binding)); + } + else + { + _mapped = true; + foreach (var newBinding in newBindings) + { + newClip.SetCurve(newBinding.path, newBinding.type, newBinding.propertyName, + AnimationUtility.GetEditorCurve(clip, binding)); + } + } } foreach (var binding in AnimationUtility.GetObjectReferenceCurveBindings(clip)) { - var newBinding = _mapping.MapBinding(binding); - _mapped |= newBinding != binding; - if (newBinding.type == null) continue; - AnimationUtility.SetObjectReferenceCurve(newClip, newBinding, - AnimationUtility.GetObjectReferenceCurve(clip, binding)); + var newBindings = _mapping.MapBinding(binding); + if (newBindings == null) + { + AnimationUtility.SetObjectReferenceCurve(newClip, binding, + AnimationUtility.GetObjectReferenceCurve(clip, binding)); + } + else + { + _mapped = true; + foreach (var newBinding in newBindings) + { + AnimationUtility.SetObjectReferenceCurve(newClip, newBinding, + AnimationUtility.GetObjectReferenceCurve(clip, binding)); + } + } } newClip.wrapMode = clip.wrapMode; From 97b67fd1c25a05a590c69d9e5582af523c1e9ad2 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 10:13:17 +0900 Subject: [PATCH 4/9] feat: moving & copying property across components --- Editor/ObjectMapping/ObjectMappingBuilder.cs | 26 +++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs index 82e39aa67..427404b80 100644 --- a/Editor/ObjectMapping/ObjectMappingBuilder.cs +++ b/Editor/ObjectMapping/ObjectMappingBuilder.cs @@ -54,6 +54,12 @@ public void RecordMoveProperties(Component from, params (string old, string @new public void RecordMoveProperty(Component from, string oldProp, string newProp) => GetComponentInfo(from).MoveProperties((oldProp, newProp)); + public void RecordMoveProperty(Component fromComponent, string oldProp, Component toComponent, string newProp) => + GetComponentInfo(fromComponent).MoveProperty(GetComponentInfo(toComponent), oldProp, newProp); + + public void RecordCopyProperty(Component fromComponent, string oldProp, Component toComponent, string newProp) => + GetComponentInfo(fromComponent).CopyProperty(GetComponentInfo(toComponent), oldProp, newProp); + public void RecordRemoveProperty(Component from, string oldProp) => GetComponentInfo(from).RemoveProperty(oldProp); @@ -77,7 +83,7 @@ class AnimationProperty [CanBeNull] public readonly string Name; [CanBeNull] public AnimationProperty MergedTo; private MappedPropertyInfo? _mappedPropertyInfo; - public AnimationProperty[] CopiedTo = Array.Empty(); + [CanBeNull] public List CopiedTo; public AnimationProperty([NotNull] BuildingComponentInfo component, [NotNull] string name) { @@ -109,7 +115,7 @@ private MappedPropertyInfo ComputeMappedInfo() { var merged = MergedTo.GetMappedInfo(); - if (CopiedTo.Length == 0) + if (CopiedTo == null || CopiedTo.Count == 0) return merged; var copied = new List(); @@ -122,7 +128,7 @@ private MappedPropertyInfo ComputeMappedInfo() else { // this is edge - if (CopiedTo.Length == 0) + if (CopiedTo == null || CopiedTo.Count == 0) return new MappedPropertyInfo(Component.InstanceId, Component.Type, Name); var descriptor = new PropertyDescriptor(Component.InstanceId, Component.Type, Name); @@ -199,6 +205,20 @@ public void MoveProperties(params (string old, string @new)[] props) propertyIds[i].MergedTo = GetProperty(props[i].@new); } + public void MoveProperty(BuildingComponentInfo toComponent, string oldProp, string newProp) + { + if (Type == typeof(Transform)) throw new Exception("Move properties of Transform is not supported!"); + GetProperty(oldProp, remove: true).MergedTo = toComponent.GetProperty(newProp); + } + + public void CopyProperty(BuildingComponentInfo toComponent, string oldProp, string newProp) + { + var prop = GetProperty(oldProp, remove: true); + if (prop.CopiedTo == null) + prop.CopiedTo = new List(); + prop.CopiedTo.Add(toComponent.GetProperty(newProp)); + } + public void RemoveProperty(string property) { if (Type == typeof(Transform)) throw new Exception("Removing properties of Transform is not supported!"); From c421a76b7d43cce757addd389937f2fb1e8d2e2f Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 12:01:07 +0900 Subject: [PATCH 5/9] test: fix tests --- Test~/AnimatorParser/AnimatorTest.cs | 10 ++-- Test~/ObjectMappingTest.cs | 85 ++++++++++++---------------- 2 files changed, 41 insertions(+), 54 deletions(-) diff --git a/Test~/AnimatorParser/AnimatorTest.cs b/Test~/AnimatorParser/AnimatorTest.cs index 8fa8c885a..587a7d5bd 100644 --- a/Test~/AnimatorParser/AnimatorTest.cs +++ b/Test~/AnimatorParser/AnimatorTest.cs @@ -141,7 +141,7 @@ public void TestLayer21_Animate16ToConst100Weight0() => [Test] public void TestParseWhole() { - var parser = new AnimatorParser(true, true); + var parser = new AnimatorParser(true); // execute var parsed = parser.AdvancedParseAnimatorController(_prefab, _controller, @@ -197,7 +197,7 @@ public void TestParseWhole() [Test] public void TestParseWholeWithExternalWeightChanges() { - var parser = new AnimatorParser(true, true); + var parser = new AnimatorParser(true); var externallyWeightChanged = new AnimatorLayerWeightMap { @@ -252,7 +252,7 @@ public void TestParseWholeWithExternalWeightChanges() [Test] public void TestOneLayerOverrides() { - var parser = new AnimatorParser(true, true); + var parser = new AnimatorParser(true); var controller = TestUtils.GetAssetAt("AnimatorParser/OneLayerOverrideController.overrideController"); var animate0To100 = TestUtils.GetAssetAt("AnimatorParser/Animate0To100.anim"); var animate1To100 = TestUtils.GetAssetAt("AnimatorParser/Animate1To100.anim"); @@ -268,7 +268,7 @@ public void TestOneLayerOverrides() [Test] public void TestTwoLayerOverrides() { - var parser = new AnimatorParser(true, true); + var parser = new AnimatorParser(true); var controller = TestUtils.GetAssetAt("AnimatorParser/TwoLayerOverrideController.overrideController"); var animate0To100 = TestUtils.GetAssetAt("AnimatorParser/Animate0To100.anim"); var animate1To100 = TestUtils.GetAssetAt("AnimatorParser/Animate1To100.anim"); @@ -289,7 +289,7 @@ private void LayerTest(int layerIndex, string layerName, string propertyName, AnimationProperty property, AnimatorLayerBlendingMode blendingMode = AnimatorLayerBlendingMode.Override) { - var parser = new AnimatorParser(true, true); + var parser = new AnimatorParser(true); // preconditions Assert.That(_controller.layers[layerIndex].name, Is.EqualTo(layerName)); diff --git a/Test~/ObjectMappingTest.cs b/Test~/ObjectMappingTest.cs index f268e037d..37bdac72d 100644 --- a/Test~/ObjectMappingTest.cs +++ b/Test~/ObjectMappingTest.cs @@ -26,15 +26,13 @@ public void MoveObjectTest() Assert.That( rootMapper.MapBinding(B("child1/child11", typeof(GameObject), "m_Enabled")), Is.EqualTo(B("child2/child11", typeof(GameObject), "m_Enabled"))); - + Assert.That( rootMapper.MapBinding(B("child1/child11/child111", typeof(GameObject), "m_Enabled")), Is.EqualTo(B("child2/child11/child111", typeof(GameObject), "m_Enabled"))); var child1Mapper = built.CreateAnimationMapper(child1); - Assert.That( - child1Mapper.MapBinding(B("child11", typeof(GameObject), "m_Enabled")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(child1Mapper, B("child11", typeof(GameObject), "m_Enabled")); } [Test] @@ -51,22 +49,14 @@ public void RecordRemoveGameObject() var built = builder.BuildObjectMapping(); var rootMapper = built.CreateAnimationMapper(root); - Assert.That( - rootMapper.MapBinding(B("child1/child11", typeof(GameObject), "m_Enabled")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(rootMapper, B("child1/child11", typeof(GameObject), "m_Enabled")); - Assert.That( - rootMapper.MapBinding(B("child1", typeof(GameObject), "m_Enabled")), - Is.EqualTo(B("child1", typeof(GameObject), "m_Enabled"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child1", typeof(GameObject), "m_Enabled")); - Assert.That( - rootMapper.MapBinding(B("child1/child11/child111", typeof(GameObject), "m_Enabled")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(rootMapper, B("child1/child11/child111", typeof(GameObject), "m_Enabled")); var child1Mapper = built.CreateAnimationMapper(child1); - Assert.That( - child1Mapper.MapBinding(B("child11", typeof(GameObject), "m_Enabled")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(child1Mapper, B("child11", typeof(GameObject), "m_Enabled")); } [Test] @@ -87,9 +77,7 @@ public void RecordMoveComponentTest() var rootMapper = built.CreateAnimationMapper(root); // should not affect to GameObject - Assert.That( - rootMapper.MapBinding(B("child1", typeof(GameObject), "m_Enabled")), - Is.EqualTo(B("child1", typeof(GameObject), "m_Enabled"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child1", typeof(GameObject), "m_Enabled")); // but should affect to component Assert.That( @@ -117,14 +105,10 @@ public void RecordRemoveComponentTest() var rootMapper = built.CreateAnimationMapper(root); // should not affect to GameObject itself - Assert.That( - rootMapper.MapBinding(B("child1", typeof(GameObject), "m_Enabled")), - Is.EqualTo(B("child1", typeof(GameObject), "m_Enabled"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child1", typeof(GameObject), "m_Enabled")); // but should affect to component - Assert.That( - rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.test")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(rootMapper, B("child1", typeof(SkinnedMeshRenderer), "blendShapes.test")); // check for component replication Assert.That(built.MapComponentInstance(child1ComponentId, out var component), Is.True); @@ -146,9 +130,7 @@ public void RecordMovePropertyTest() var rootMapper = built.CreateAnimationMapper(root); // should not affect to other component - Assert.That( - rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test")), - Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test")); // but should affect to component Assert.That( @@ -220,14 +202,10 @@ public void RecordRemovePropertyTest() var rootMapper = built.CreateAnimationMapper(root); // should not affect to other component - Assert.That( - rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test")), - Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child2", typeof(SkinnedMeshRenderer), "blendShapes.test")); // but should affect to component - Assert.That( - rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.test")), - Is.EqualTo(Default)); + ExAsset.MapBindingRemoved(rootMapper, B("child1", typeof(SkinnedMeshRenderer), "blendShapes.test")); } [Test] @@ -262,15 +240,11 @@ public void RecordMovePropertyThenComponentThenPropertyTest() rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.moved")), Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.movedChanged"))); - Assert.That( - rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child1")), - Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child1"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child1")); Assert.That( rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child2")), Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child2Changed"))); - Assert.That( - rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child2Other")), - Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child2Other"))); + ExAsset.MapBindingUnchanged(rootMapper, B("child2", typeof(SkinnedMeshRenderer), "blendShapes.child2Other")); Assert.That( rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.moved")), Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "blendShapes.movedChanged"))); @@ -323,12 +297,8 @@ public void RecordRemovePropertyThenMergeComponent() var rootMapper = built.CreateAnimationMapper(root); - Assert.That( - rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "m_Enabled")), - Is.EqualTo(Default)); - Assert.That( - rootMapper.MapBinding(B("child2", typeof(SkinnedMeshRenderer), "m_Enabled")), - Is.EqualTo(B("child2", typeof(SkinnedMeshRenderer), "m_Enabled"))); + ExAsset.MapBindingRemoved(rootMapper, B("child1", typeof(SkinnedMeshRenderer), "m_Enabled")); + ExAsset.MapBindingUnchanged(rootMapper, B("child2", typeof(SkinnedMeshRenderer), "m_Enabled")); // check for component replication Assert.That(built.MapComponentInstance(child1ComponentId, out var component), Is.True); @@ -337,16 +307,33 @@ public void RecordRemovePropertyThenMergeComponent() private static (string, Type, string) B(string path, Type type, string prop) => (path, type, prop); - private static (string, Type, string) Default = default; } - static class ObjectMappingTestUtils + static class ExAsset { + public static void MapBindingRemoved(AnimationObjectMapper mapping, (string, Type, string) binding) + { + var result = mapping.MapBinding(EditorCurveBinding.PPtrCurve(binding.Item1, binding.Item2, binding.Item3)); + + Assert.That(result, Is.Not.Null); + Assert.That(result.Length, Is.EqualTo(0)); + } + + public static void MapBindingUnchanged(AnimationObjectMapper mapping, (string, Type, string) binding) + { + var result = mapping.MapBinding(EditorCurveBinding.PPtrCurve(binding.Item1, binding.Item2, binding.Item3)); + + Assert.That(result, Is.Null); + } + public static (string, Type, string) MapBinding(this AnimationObjectMapper mapping, (string, Type, string) binding) { var result = mapping.MapBinding(EditorCurveBinding.PPtrCurve(binding.Item1, binding.Item2, binding.Item3)); - return (result.path, result.type, result.propertyName); + Assert.That(result, Is.Not.Null); + Assert.That(result.Length, Is.EqualTo(1)); + + return (result[0].path, result[0].type, result[0].propertyName); } } } From 645edf47cdb41c76bf6a9ac915e7df9290d473dd Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 12:01:14 +0900 Subject: [PATCH 6/9] fix: bugs --- Editor/ObjectMapping/AnimationObjectMapper.cs | 54 ++++++++++++------- Editor/ObjectMapping/ObjectMappingBuilder.cs | 38 ++++++++++--- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/Editor/ObjectMapping/AnimationObjectMapper.cs b/Editor/ObjectMapping/AnimationObjectMapper.cs index 6d690f37e..6d2fbfefc 100644 --- a/Editor/ObjectMapping/AnimationObjectMapper.cs +++ b/Editor/ObjectMapping/AnimationObjectMapper.cs @@ -129,30 +129,44 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) if (componentInfo != null) { - if (!componentInfo.PropertyMapping.TryGetValue(binding.propertyName, out var newProp)) - return null; - - var curveBindings = new EditorCurveBinding[newProp.AllCopiedTo.Length]; - for (var i = 0; i < newProp.AllCopiedTo.Length; i++) + if (componentInfo.PropertyMapping.TryGetValue(binding.propertyName, out var newProp)) + { + // there are mapping for component + var curveBindings = new EditorCurveBinding[newProp.AllCopiedTo.Length]; + for (var i = 0; i < newProp.AllCopiedTo.Length; i++) + { + var descriptor = newProp.AllCopiedTo[i]; + var component = EditorUtility.InstanceIDToObject(descriptor.InstanceId) as Component; + // this means removed. + if (component == null) continue; + + var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); + + // this means moved to out of the animator scope + // TODO: add warning + if (newPath == null) return Array.Empty(); + + binding.path = newPath; + binding.type = descriptor.Type; + binding.propertyName = descriptor.Name; + curveBindings[i] = binding; // copy + } + + return curveBindings; + } + else { - var descriptor = newProp.AllCopiedTo[i]; - var component = EditorUtility.InstanceIDToObject(descriptor.InstanceId) as Component; - // this means removed. - if (component == null) continue; + var component = EditorUtility.InstanceIDToObject(componentInfo.MergedInto) as Component; + // there's mapping about component. + // this means the component is merged or some prop has mapping + if (!component) return Array.Empty(); // this means removed. var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); - - // this means moved to out of the animator scope - // TODO: add warning - if (newPath == null) return default; - + if (newPath == null) return Array.Empty(); // this means moved to out of the animator scope + if (binding.path == newPath) return null; binding.path = newPath; - binding.type = descriptor.Type; - binding.propertyName = descriptor.Name; - curveBindings[i] = binding; // copy + return new []{ binding }; } - - return curveBindings; } else { @@ -161,7 +175,7 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) if (binding.type != typeof(GameObject)) { var component = EditorUtility.InstanceIDToObject(instanceId) as Component; - if (!component) return default; // this means removed + if (!component) return Array.Empty(); // this means removed } if (gameObjectInfo.NewPath == null) return Array.Empty(); diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs index 427404b80..4f22bea31 100644 --- a/Editor/ObjectMapping/ObjectMappingBuilder.cs +++ b/Editor/ObjectMapping/ObjectMappingBuilder.cs @@ -19,6 +19,7 @@ internal class ObjectMappingBuilder private readonly IReadOnlyDictionary _beforeGameObjectInfos; // key: instanceId + private readonly Dictionary _originalComponentInfos = new Dictionary(); private readonly Dictionary _componentInfos = new Dictionary(); public ObjectMappingBuilder([NotNull] GameObject rootObject) @@ -45,8 +46,23 @@ public ObjectMappingBuilder([NotNull] GameObject rootObject) #endif } - public void RecordMergeComponent(T from, T mergeTo) where T: Component => - GetComponentInfo(from).MergedTo(GetComponentInfo(mergeTo)); + public void RecordMergeComponent(T from, T mergeTo) where T: Component + { + if (!_componentInfos.TryGetValue(mergeTo.GetInstanceID(), out var mergeToInfo)) + { + var newMergeToInfo = new BuildingComponentInfo(mergeTo); + _originalComponentInfos.Add(mergeTo.GetInstanceID(), newMergeToInfo); + _componentInfos.Add(mergeTo.GetInstanceID(), newMergeToInfo); + GetComponentInfo(from).MergedTo(newMergeToInfo); + } + else + { + var newMergeToInfo = new BuildingComponentInfo(mergeTo); + _componentInfos[mergeTo.GetInstanceID()]= newMergeToInfo; + mergeToInfo.MergedTo(newMergeToInfo); + GetComponentInfo(from).MergedTo(newMergeToInfo); + } + } public void RecordMoveProperties(Component from, params (string old, string @new)[] props) => GetComponentInfo(from).MoveProperties(props); @@ -66,7 +82,11 @@ public void RecordRemoveProperty(Component from, string oldProp) => private BuildingComponentInfo GetComponentInfo(Component component) { if (!_componentInfos.TryGetValue(component.GetInstanceID(), out var info)) - _componentInfos.Add(component.GetInstanceID(), info = new BuildingComponentInfo(component)); + { + info = new BuildingComponentInfo(component); + _originalComponentInfos.Add(component.GetInstanceID(), info); + _componentInfos.Add(component.GetInstanceID(), info); + } return info; } @@ -74,7 +94,7 @@ public ObjectMapping BuildObjectMapping() { return new ObjectMapping( _beforeGameObjectInfos, - _componentInfos.ToDictionary(p => p.Key, p => p.Value.Build())); + _originalComponentInfos.ToDictionary(p => p.Key, p => p.Value.Build())); } class AnimationProperty @@ -229,12 +249,16 @@ public void RemoveProperty(string property) public ComponentInfo Build() { + var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, + p => p.Value.GetMappedInfo()); var mergedInfo = this; while (mergedInfo._mergedInto != null) + { mergedInfo = mergedInfo._mergedInto; - - var propertyMapping = _beforePropertyIds.ToDictionary(p => p.Key, - p => p.Value.GetMappedInfo()); + foreach (var (key, value) in mergedInfo._beforePropertyIds) + if (!propertyMapping.ContainsKey(key)) + propertyMapping.Add(key, value.GetMappedInfo()); + } return new ComponentInfo(InstanceId, mergedInfo.InstanceId, Type, propertyMapping); } From 9c712e090dee772cc18f22f03fefa5cef61e7897 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 12:11:16 +0900 Subject: [PATCH 7/9] chore: make ComponentOrGameObject general utility --- .../AnimatorParserDebugWindow.cs | 2 +- .../ModificationsContainers.cs | 35 -------------- Editor/Utils/ComponentOrGameObject.cs | 47 +++++++++++++++++++ Editor/Utils/ComponentOrGameObject.cs.meta | 3 ++ 4 files changed, 51 insertions(+), 36 deletions(-) create mode 100644 Editor/Utils/ComponentOrGameObject.cs create mode 100644 Editor/Utils/ComponentOrGameObject.cs.meta diff --git a/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs b/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs index bdd096023..e70956092 100644 --- a/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs +++ b/Editor/Processors/TraceAndOptimize/AnimatorParserDebugWindow.cs @@ -75,7 +75,7 @@ private string CreateText() foreach (var (obj, properties) in Container.ModifiedProperties) { - var gameObject = obj.SelfOrAttachedGameObject.transform; + var gameObject = obj.gameObject.transform; resultText.Append(Utils.RelativePath(root, gameObject)).Append(": ") .Append(((Object)obj).GetType().FullName).Append('\n'); diff --git a/Editor/Processors/TraceAndOptimize/ModificationsContainers.cs b/Editor/Processors/TraceAndOptimize/ModificationsContainers.cs index 8007c4b12..2f1459cd3 100644 --- a/Editor/Processors/TraceAndOptimize/ModificationsContainers.cs +++ b/Editor/Processors/TraceAndOptimize/ModificationsContainers.cs @@ -262,41 +262,6 @@ public void MakeAllVariable() } } - public readonly struct ComponentOrGameObject : IEquatable - { - private readonly Object _object; - - public ComponentOrGameObject(Object o) => _object = o; - - public static implicit operator ComponentOrGameObject(GameObject gameObject) => - new ComponentOrGameObject(gameObject); - - public static implicit operator ComponentOrGameObject(Component component) => - new ComponentOrGameObject(component); - - public static implicit operator Object(ComponentOrGameObject componentOrGameObject) => - componentOrGameObject._object; - - public GameObject SelfOrAttachedGameObject => _object as GameObject ?? ((Component)_object).gameObject; - public Object Value => _object; - - public bool AsGameObject(out GameObject gameObject) - { - gameObject = _object as GameObject; - return gameObject; - } - - public bool AsComponent(out T gameObject) where T : Component - { - gameObject = _object as T; - return gameObject; - } - - public bool Equals(ComponentOrGameObject other) => Equals(_object, other._object); - public override bool Equals(object obj) => obj is ComponentOrGameObject other && Equals(other); - public override int GetHashCode() => _object != null ? _object.GetHashCode() : 0; - } - readonly struct AnimationProperty : IEquatable { public readonly PropertyState State; diff --git a/Editor/Utils/ComponentOrGameObject.cs b/Editor/Utils/ComponentOrGameObject.cs new file mode 100644 index 000000000..2d70a1b65 --- /dev/null +++ b/Editor/Utils/ComponentOrGameObject.cs @@ -0,0 +1,47 @@ +using System; +using UnityEngine; +using Object = UnityEngine.Object; + +namespace Anatawa12.AvatarOptimizer +{ + public readonly struct ComponentOrGameObject : IEquatable + { + private readonly Object _object; + + public ComponentOrGameObject(Object o) + { + if (!(o is null || o is Component || o is GameObject)) + throw new ArgumentException(); + _object = o; + } + + private ComponentOrGameObject(Object o, int marker) => _object = o; + + public static implicit operator ComponentOrGameObject(GameObject gameObject) => + new ComponentOrGameObject(gameObject, 0); + + public static implicit operator ComponentOrGameObject(Component component) => + new ComponentOrGameObject(component, 0); + + public static implicit operator Object(ComponentOrGameObject componentOrGameObject) => + componentOrGameObject._object; + + // ReSharper disable InconsistentNaming + // GameObject-like properties + public GameObject gameObject => _object as GameObject ?? ((Component)_object).gameObject; + public Transform transform => gameObject.transform; + + // ReSharper restore InconsistentNaming + public Object Value => _object; + + public bool TryAs(out T gameObject) where T : Object + { + gameObject = _object as T; + return gameObject; + } + + public bool Equals(ComponentOrGameObject other) => Equals(_object, other._object); + public override bool Equals(object obj) => obj is ComponentOrGameObject other && Equals(other); + public override int GetHashCode() => _object != null ? _object.GetHashCode() : 0; + } +} \ No newline at end of file diff --git a/Editor/Utils/ComponentOrGameObject.cs.meta b/Editor/Utils/ComponentOrGameObject.cs.meta new file mode 100644 index 000000000..be3ee54ea --- /dev/null +++ b/Editor/Utils/ComponentOrGameObject.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: e243c57aebee409794351ba8a385b53f +timeCreated: 1698203270 \ No newline at end of file From 7c24a00f4130d5882591ef9434adf71c2ff40156 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 13:20:57 +0900 Subject: [PATCH 8/9] feat: support GameObject properties in ObjectMapping --- Editor/ObjectMapping/AnimationObjectMapper.cs | 26 +++++++------------ Editor/ObjectMapping/ObjectMapping.cs | 2 ++ Editor/ObjectMapping/ObjectMappingBuilder.cs | 16 ++++++------ Editor/Utils/ComponentOrGameObject.cs | 5 +++- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/Editor/ObjectMapping/AnimationObjectMapper.cs b/Editor/ObjectMapping/AnimationObjectMapper.cs index 6d2fbfefc..5b3a814c5 100644 --- a/Editor/ObjectMapping/AnimationObjectMapper.cs +++ b/Editor/ObjectMapping/AnimationObjectMapper.cs @@ -94,7 +94,7 @@ public string MapPath(string srcPath, Type type) if (componentInfo != null) { - var component = EditorUtility.InstanceIDToObject(componentInfo.MergedInto) as Component; + var component = new ComponentOrGameObject(EditorUtility.InstanceIDToObject(componentInfo.MergedInto)); // there's mapping about component. // this means the component is merged or some prop has mapping if (!component) return null; // this means removed. @@ -107,12 +107,8 @@ public string MapPath(string srcPath, Type type) else { // The component is not merged & no prop mapping so process GameObject mapping - - if (type != typeof(GameObject)) - { - var component = EditorUtility.InstanceIDToObject(instanceId) as Component; - if (!component) return null; // this means removed - } + var component = EditorUtility.InstanceIDToObject(instanceId); + if (!component) return null; // this means removed if (gameObjectInfo.NewPath == null) return null; return gameObjectInfo.NewPath; @@ -129,9 +125,12 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) if (componentInfo != null) { + // there's mapping about component. + // this means the component is merged or some prop has mapping + if (componentInfo.PropertyMapping.TryGetValue(binding.propertyName, out var newProp)) { - // there are mapping for component + // there are mapping for property var curveBindings = new EditorCurveBinding[newProp.AllCopiedTo.Length]; for (var i = 0; i < newProp.AllCopiedTo.Length; i++) { @@ -156,9 +155,7 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) } else { - var component = EditorUtility.InstanceIDToObject(componentInfo.MergedInto) as Component; - // there's mapping about component. - // this means the component is merged or some prop has mapping + var component = new ComponentOrGameObject(EditorUtility.InstanceIDToObject(componentInfo.MergedInto)); if (!component) return Array.Empty(); // this means removed. var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); @@ -172,11 +169,8 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) { // The component is not merged & no prop mapping so process GameObject mapping - if (binding.type != typeof(GameObject)) - { - var component = EditorUtility.InstanceIDToObject(instanceId) as Component; - if (!component) return Array.Empty(); // this means removed - } + var component = EditorUtility.InstanceIDToObject(instanceId); + if (!component) return Array.Empty(); // this means removed if (gameObjectInfo.NewPath == null) return Array.Empty(); if (binding.path == gameObjectInfo.NewPath) return null; diff --git a/Editor/ObjectMapping/ObjectMapping.cs b/Editor/ObjectMapping/ObjectMapping.cs index 6f32f37c7..b22119211 100644 --- a/Editor/ObjectMapping/ObjectMapping.cs +++ b/Editor/ObjectMapping/ObjectMapping.cs @@ -94,6 +94,8 @@ public BeforeGameObjectTree(GameObject gameObject) } } + componentByType[typeof(GameObject)] = InstanceId; + ComponentInstanceIdByType = componentByType; } } diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs index 4f22bea31..dfe1a14e2 100644 --- a/Editor/ObjectMapping/ObjectMappingBuilder.cs +++ b/Editor/ObjectMapping/ObjectMappingBuilder.cs @@ -64,22 +64,22 @@ public void RecordMergeComponent(T from, T mergeTo) where T: Component } } - public void RecordMoveProperties(Component from, params (string old, string @new)[] props) => + public void RecordMoveProperties(ComponentOrGameObject from, params (string old, string @new)[] props) => GetComponentInfo(from).MoveProperties(props); - public void RecordMoveProperty(Component from, string oldProp, string newProp) => + public void RecordMoveProperty(ComponentOrGameObject from, string oldProp, string newProp) => GetComponentInfo(from).MoveProperties((oldProp, newProp)); - public void RecordMoveProperty(Component fromComponent, string oldProp, Component toComponent, string newProp) => + public void RecordMoveProperty(ComponentOrGameObject fromComponent, string oldProp, ComponentOrGameObject toComponent, string newProp) => GetComponentInfo(fromComponent).MoveProperty(GetComponentInfo(toComponent), oldProp, newProp); - public void RecordCopyProperty(Component fromComponent, string oldProp, Component toComponent, string newProp) => + public void RecordCopyProperty(ComponentOrGameObject fromComponent, string oldProp, ComponentOrGameObject toComponent, string newProp) => GetComponentInfo(fromComponent).CopyProperty(GetComponentInfo(toComponent), oldProp, newProp); - public void RecordRemoveProperty(Component from, string oldProp) => + public void RecordRemoveProperty(ComponentOrGameObject from, string oldProp) => GetComponentInfo(from).RemoveProperty(oldProp); - private BuildingComponentInfo GetComponentInfo(Component component) + private BuildingComponentInfo GetComponentInfo(ComponentOrGameObject component) { if (!_componentInfos.TryGetValue(component.GetInstanceID(), out var info)) { @@ -176,10 +176,10 @@ class BuildingComponentInfo private readonly Dictionary _afterPropertyIds = new Dictionary(); - public BuildingComponentInfo(Component component) + public BuildingComponentInfo(ComponentOrGameObject component) { InstanceId = component.GetInstanceID(); - Type = component.GetType(); + Type = component.Value.GetType(); } internal bool IsMerged => _mergedInto != null; diff --git a/Editor/Utils/ComponentOrGameObject.cs b/Editor/Utils/ComponentOrGameObject.cs index 2d70a1b65..427ff15de 100644 --- a/Editor/Utils/ComponentOrGameObject.cs +++ b/Editor/Utils/ComponentOrGameObject.cs @@ -26,11 +26,14 @@ public static implicit operator ComponentOrGameObject(Component component) => public static implicit operator Object(ComponentOrGameObject componentOrGameObject) => componentOrGameObject._object; + public static implicit operator bool(ComponentOrGameObject componentOrGameObject) => + (Object)componentOrGameObject; + // ReSharper disable InconsistentNaming // GameObject-like properties public GameObject gameObject => _object as GameObject ?? ((Component)_object).gameObject; public Transform transform => gameObject.transform; - + public int GetInstanceID() => ((Object)this).GetInstanceID(); // ReSharper restore InconsistentNaming public Object Value => _object; From 9dc35bb89fe0390f1d2c07c1ac36bb1e0d3a73d3 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Wed, 25 Oct 2023 13:59:37 +0900 Subject: [PATCH 9/9] test: add test for copying property and moving GameObject property --- Editor/ObjectMapping/AnimationObjectMapper.cs | 13 ++- Test~/ObjectMappingTest.cs | 80 +++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/Editor/ObjectMapping/AnimationObjectMapper.cs b/Editor/ObjectMapping/AnimationObjectMapper.cs index 5b3a814c5..0a1002652 100644 --- a/Editor/ObjectMapping/AnimationObjectMapper.cs +++ b/Editor/ObjectMapping/AnimationObjectMapper.cs @@ -132,12 +132,17 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) { // there are mapping for property var curveBindings = new EditorCurveBinding[newProp.AllCopiedTo.Length]; + var copiedToIndex = 0; for (var i = 0; i < newProp.AllCopiedTo.Length; i++) { - var descriptor = newProp.AllCopiedTo[i]; - var component = EditorUtility.InstanceIDToObject(descriptor.InstanceId) as Component; + var descriptor = newProp.AllCopiedTo[copiedToIndex++]; + var component = new ComponentOrGameObject(EditorUtility.InstanceIDToObject(descriptor.InstanceId)); // this means removed. - if (component == null) continue; + if (!component) + { + copiedToIndex -= 1; + continue; + } var newPath = Utils.RelativePath(_rootGameObject.transform, component.transform); @@ -151,6 +156,8 @@ public EditorCurveBinding[] MapBinding(EditorCurveBinding binding) curveBindings[i] = binding; // copy } + if (copiedToIndex != curveBindings.Length) + return curveBindings.AsSpan().Slice(0, copiedToIndex).ToArray(); return curveBindings; } else diff --git a/Test~/ObjectMappingTest.cs b/Test~/ObjectMappingTest.cs index 37bdac72d..a23c9027c 100644 --- a/Test~/ObjectMappingTest.cs +++ b/Test~/ObjectMappingTest.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using NUnit.Framework; using UnityEditor; using UnityEngine; @@ -305,8 +306,87 @@ public void RecordRemovePropertyThenMergeComponent() Assert.That(component, Is.SameAs(child2Component)); } + [Test] + public void RecordMoveProperty() + { + var root = new GameObject(); + var child1 = Utils.NewGameObject("child1", root.transform); + var child11 = Utils.NewGameObject("child11", child1.transform); + var child11Component = child11.AddComponent(); + var child2 = Utils.NewGameObject("child2", root.transform); + + var builder = new ObjectMappingBuilder(root); + builder.RecordMoveProperty(child11Component, "blendShapes.child11", "blendShapes.child11Changed"); + child11.transform.parent = child2.transform; + builder.RecordMoveProperty(child11Component, "blendShapes.moved", "blendShapes.movedChanged"); + + var built = builder.BuildObjectMapping(); + + var rootMapper = built.CreateAnimationMapper(root); + + Assert.That( + rootMapper.MapBinding(B("child1/child11", typeof(SkinnedMeshRenderer), "blendShapes.child11")), + Is.EqualTo(B("child2/child11", typeof(SkinnedMeshRenderer), "blendShapes.child11Changed"))); + Assert.That( + rootMapper.MapBinding(B("child1/child11", typeof(SkinnedMeshRenderer), "blendShapes.moved")), + Is.EqualTo(B("child2/child11", typeof(SkinnedMeshRenderer), "blendShapes.movedChanged"))); + + Assert.That(built.MapComponentInstance(child11Component.GetInstanceID(), out var component), Is.False); + } + + [Test] + public void MovePropertyOfGameObject() + { + var root = new GameObject(); + var child1 = Utils.NewGameObject("child1", root.transform); + var child11 = Utils.NewGameObject("child11", child1.transform); + + var builder = new ObjectMappingBuilder(root); + builder.RecordMoveProperty(child11, "m_IsActive", child1, "m_IsActive"); + + var built = builder.BuildObjectMapping(); + + var rootMapper = built.CreateAnimationMapper(root); + + Assert.That( + rootMapper.MapBinding(B("child1/child11", typeof(GameObject), "m_IsActive")), + Is.EqualTo(B("child1", typeof(GameObject), "m_IsActive"))); + } + + + [Test] + public void CopyProperty() + { + var root = new GameObject(); + var child1 = Utils.NewGameObject("child1", root.transform); + var child11 = Utils.NewGameObject("child11", child1.transform); + var child12 = Utils.NewGameObject("child12", child1.transform); + + var builder = new ObjectMappingBuilder(root); + builder.RecordCopyProperty(child11, "m_IsActive", + child12, "m_IsActive"); + + var built = builder.BuildObjectMapping(); + + var rootMapper = built.CreateAnimationMapper(root); + + var mapped = rootMapper.MapBinding(Curve("child1/child11", typeof(GameObject), "m_IsActive")); + Assert.That(mapped, Is.Not.Null); + Assert.That(mapped.Length, Is.EqualTo(2)); + Assert.That(mapped.Select(ToTuple), Is.EquivalentTo(new [] + { + B("child1/child11", typeof(GameObject), "m_IsActive"), + B("child1/child12", typeof(GameObject), "m_IsActive"), + })); + } private static (string, Type, string) B(string path, Type type, string prop) => (path, type, prop); + + private static (string, Type, string) ToTuple(EditorCurveBinding binding) => + (binding.path, binding.type, binding.propertyName); + + private static EditorCurveBinding Curve(string path, Type type, string prop) + => EditorCurveBinding.PPtrCurve(path, type, prop); } static class ExAsset