Merge pull request #64 from anatawa12/reduce-modification

Reduce unnecessary modification in PrefabSafeSet/List
anatawa12 · Mar 23, 2023 · 8a8be5c · 8a8be5c
2 parents f7c567a + ee55891
commit 8a8be5c
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 84 deletions.
diff --git a/CHANGELOG-PRERELEASE.md b/CHANGELOG-PRERELEASE.md
@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog].
 ### Added
 
 ### Changed
+- reduce unnecessary modification in PrefabSafeSet/List `#64`
+  - Previously PrefabSafeSet/List will always generates array size change modification.
+  - Now, array size change will be generated when added/removed elements from the collection.
 
 ### Deprecated
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog].
 ### Added
 
 ### Changed
+- reduce unnecessary modification in PrefabSafeSet/List `#64`
+  - Previously PrefabSafeSet/List will always generates array size change modification.
+  - Now, array size change will be generated when added/removed elements from the collection.
 
 ### Deprecated
 

diff --git a/Internal/PrefabSafeList/Editor/EditorUtil.PrefabModification.cs b/Internal/PrefabSafeList/Editor/EditorUtil.PrefabModification.cs
@@ -1,6 +1,8 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
+using JetBrains.Annotations;
 using UnityEditor;
 using UnityEngine.Assertions;
 
@@ -11,17 +13,17 @@ public abstract partial class EditorUtil
         private struct ArraySizeCheck
         {
             public int Size { get; private set; }
-            private readonly SerializedProperty _prop;
+            [CanBeNull] private readonly SerializedProperty _prop;
 
-            public ArraySizeCheck(SerializedProperty prop)
+            public ArraySizeCheck([CanBeNull] SerializedProperty prop)
             {
                 _prop = prop;
                 Size = 0;
             }
 
-            public bool Changed => Size != _prop.intValue;
+            public bool Changed => Size != (_prop?.intValue ?? 0);
 
-            public void Update() => Size = _prop.intValue;
+            public void Update() => Size = (_prop?.intValue ?? 0);
         }
 
         private sealed class PrefabModification : EditorUtil
@@ -30,12 +32,13 @@ private sealed class PrefabModification : EditorUtil
             private readonly SerializedProperty _rootProperty;
 
             private readonly SerializedProperty _firstLayerProp;
-            private readonly SerializedProperty[] _layerElementsProps;
+            [ItemCanBeNull] private readonly SerializedProperty[] _layerElementsProps;
 
             private readonly int _nestCount;
             private readonly SerializedProperty _prefabLayers;
 
             // upstream change check
+            private ArraySizeCheck _prefabLayersCheck;
             private ArraySizeCheck _firstLayerCheck;
             private readonly ArraySizeCheck[] _layerChecks;
 
@@ -55,8 +58,8 @@ public PrefabModification(SerializedProperty property, int nestCount)
 
                 // apply modifications until previous one
                 _prefabLayers = property.FindPropertyRelative(nameof(Names.prefabLayers));
-                ResetLayers();
-                DoInitialize();
+                _prefabLayersCheck = new ArraySizeCheck(_prefabLayers.FindPropertyRelative("Array.size"));
+                InitCurrentLayer();
             }
 
             private void ClearNonLayerModifications(SerializedProperty property, int nestCount)
@@ -87,29 +90,41 @@ public override int ElementsCount
             /// </summary>
             public void Initialize()
             {
-                if (_nestCount != _prefabLayers.arraySize)
-                    ResetLayers();
+                if (_prefabLayersCheck.Changed)
+                    InitCurrentLayer();
                 if (_firstLayerCheck.Changed || _layerChecks.Any(x => x.Changed))
                 {
                     DoInitialize();
                 }
             }
 
-            private void ResetLayers()
+            private void InitCurrentLayer(bool force = false)
             {
-                // process current layer
-                if (_prefabLayers.arraySize < _nestCount) _prefabLayers.arraySize = _nestCount;
+                if (_prefabLayers.arraySize < _nestCount && force)
+                    _prefabLayers.arraySize = _nestCount;
 
-                for (var i = 0; i < _prefabLayers.arraySize; i++)
+                if (_prefabLayersCheck.Size < _prefabLayers.arraySize)
                 {
-                    var elements = _prefabLayers.GetArrayElementAtIndex(i)
-                        .FindPropertyRelative(nameof(Names.Layer.elements));
+                    for (var i = _prefabLayersCheck.Size; i < _prefabLayers.arraySize; i++)
+                    {
+                        var elements = _prefabLayers.GetArrayElementAtIndex(i)
+                            .FindPropertyRelative(nameof(Names.Layer.elements));
 
-                    _layerElementsProps[i] = elements;
-                    _layerChecks[i] = new ArraySizeCheck(elements.FindPropertyRelative("Array.size"));
+                        _layerElementsProps[i] = elements;
+                        _layerChecks[i] = new ArraySizeCheck(elements.FindPropertyRelative("Array.size"));
+                    }
                 }
+                else if (_prefabLayersCheck.Size > _prefabLayers.arraySize)
+                {
+                    for (var i = _prefabLayers.arraySize; i < _prefabLayersCheck.Size; i++)
+                    {
+                        _layerElementsProps[i] = null;
+                        _layerChecks[i] = new ArraySizeCheck(null);
+                    }
+                }
+                _prefabLayersCheck.Update();
 
-                _elements.Clear();
+                DoInitialize();
             }
 
             /// <summary>
@@ -126,6 +141,7 @@ public void DoInitialize()
                 for (var i = 0; i < _layerElementsProps.Length; i++)
                 {
                     var layerElementsProp = _layerElementsProps[i];
+                    if (layerElementsProp == null) continue;
                     DoInitializeLayer(offset, _layerChecks[i].Size, layerElementsProp,
                         i == _layerElementsProps.Length - 1);
                     offset += layerElementsProp.arraySize;
@@ -161,9 +177,10 @@ void DoInitializeLayer(int offset, int prevSize, SerializedProperty layerElement
 
             public override IElement AddElement()
             {
-                Initialize();
+                InitCurrentLayer(true);
                 var layerIndex = _layerElementsProps.Length - 1;
                 var lastElementsProp = _layerElementsProps[layerIndex];
+                Debug.Assert(lastElementsProp != null, nameof(lastElementsProp) + " != null");
                 var addedElement = AddArrayElement(lastElementsProp);
                 var element = new ElementImpl(this, addedElement, lastElementsProp.arraySize - 1, true);
                 _elements.Add(element);

diff --git a/Internal/PrefabSafeList/Editor/EditorUtil.cs b/Internal/PrefabSafeList/Editor/EditorUtil.cs
@@ -30,7 +30,7 @@ private EditorUtil()
 
         public abstract IElement AddElement();
 
-        private static SerializedProperty AddArrayElement(SerializedProperty array)
+        private static SerializedProperty AddArrayElement([NotNull] SerializedProperty array)
         {
             array.arraySize += 1;
             return array.GetArrayElementAtIndex(array.arraySize - 1);

diff --git a/Internal/PrefabSafeList/Editor/OnBeforeSerializeImpl.cs b/Internal/PrefabSafeList/Editor/OnBeforeSerializeImpl.cs
@@ -19,12 +19,15 @@ public static void Impl(PrefabSafeList<T, TLayer, TContainer> self)
             // match prefabLayers count.
             var nestCount = PrefabSafeListUtil.PrefabNestCount(self.OuterObject);
 
-            if (self.prefabLayers.Length == nestCount)
-                RemoveCheckCheck(self, nestCount);
-            else if (self.prefabLayers.Length < nestCount)
-                self.prefabLayers = PrefabSafeListRuntimeUtil.ResizeArray(self.prefabLayers, nestCount);
+            if (self.prefabLayers.Length < nestCount)
+            {
+                // https://github.com/anatawa12/AvatarOptimizer/issues/52
+                // to avoid unnecessary modifications, resize is not performed later.
+            }
             else if (self.prefabLayers.Length > nestCount)
                 ApplyModificationsToLatestLayer(self, nestCount);
+
+            RemoveCheckCheck(self, nestCount);
         }
 
         private static void RemoveCheckCheck(PrefabSafeList<T, TLayer, TContainer> self, int nestCount)
@@ -35,11 +38,15 @@ private static void RemoveCheckCheck(PrefabSafeList<T, TLayer, TContainer> self,
             }
             else
             {
-                var currentLayer = self.prefabLayers[nestCount - 1] ?? (self.prefabLayers[nestCount - 1] = new TLayer());
-
-                var list = new List<T>();
-                currentLayer.ApplyTo(list);
-                currentLayer.elements = list.Select(x => new TContainer { value = x }).ToArray();
+                if (nestCount < self.prefabLayers.Length)
+                {
+                    var currentLayer = self.prefabLayers[nestCount - 1] ??
+                                       (self.prefabLayers[nestCount - 1] = new TLayer());
+
+                    var list = new List<T>();
+                    currentLayer.ApplyTo(list);
+                    currentLayer.elements = list.Select(x => new TContainer { value = x }).ToArray();
+                }
             }
         }