From b3df22d452e4013d2f3615e9381b6d2b22d0e69d Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Tue, 7 Jul 2026 10:45:29 -0700 Subject: [PATCH 1/9] Fix settings persistence dirty state --- .../Data/DataVisualizerSettings.cs | 44 +++++ .../Data/DataVisualizerUserState.cs | 20 +++ .../Data/NamespaceCollapseState.cs | 48 ++++++ Editor/DataVisualizer/DataVisualizer.cs | 49 ++---- Editor/DataVisualizer/NamespaceController.cs | 51 +++--- Tests/Editor/SettingsPersistenceTests.cs | 160 ++++++++++++++++++ Tests/Editor/SettingsPersistenceTests.cs.meta | 3 + ...os.DataVisualizer.Tests.Editor.asmdef.meta | 6 +- 8 files changed, 323 insertions(+), 58 deletions(-) create mode 100644 Tests/Editor/SettingsPersistenceTests.cs create mode 100644 Tests/Editor/SettingsPersistenceTests.cs.meta diff --git a/Editor/DataVisualizer/Data/DataVisualizerSettings.cs b/Editor/DataVisualizer/Data/DataVisualizerSettings.cs index 06e0682..7e43663 100644 --- a/Editor/DataVisualizer/Data/DataVisualizerSettings.cs +++ b/Editor/DataVisualizer/Data/DataVisualizerSettings.cs @@ -96,6 +96,18 @@ public void MarkDirty() #endif } + public bool SetSelectActiveObject(bool value) + { + if (selectActiveObject == value) + { + return false; + } + + selectActiveObject = value; + MarkDirty(); + return true; + } + public void HydrateFrom(DataVisualizerUserState userState) { if (userState == null) @@ -199,6 +211,38 @@ internal bool HasCollapseState(string namespaceKey) return entry != null; } + public bool SetNamespaceCollapsed(string namespaceKey, bool isCollapsed) + { + if (string.IsNullOrWhiteSpace(namespaceKey)) + { + return false; + } + + namespaceCollapseStates ??= new List(); + bool changed = NamespaceCollapseState.SetCollapsed( + namespaceCollapseStates, + namespaceKey, + isCollapsed + ); + if (changed) + { + MarkDirty(); + } + + return changed; + } + + public bool RemoveNamespaceCollapseState(string namespaceKey) + { + bool changed = NamespaceCollapseState.Remove(namespaceCollapseStates, namespaceKey); + if (changed) + { + MarkDirty(); + } + + return changed; + } + internal NamespaceCollapseState GetOrCreateCollapseState(string namespaceKey) { NamespaceCollapseState entry = namespaceCollapseStates.Find(o => diff --git a/Editor/DataVisualizer/Data/DataVisualizerUserState.cs b/Editor/DataVisualizer/Data/DataVisualizerUserState.cs index e3df585..ac0d457 100644 --- a/Editor/DataVisualizer/Data/DataVisualizerUserState.cs +++ b/Editor/DataVisualizer/Data/DataVisualizerUserState.cs @@ -122,6 +122,26 @@ public bool HasCollapseState(string namespaceKey) return entry != null; } + public bool SetNamespaceCollapsed(string namespaceKey, bool isCollapsed) + { + if (string.IsNullOrWhiteSpace(namespaceKey)) + { + return false; + } + + namespaceCollapseStates ??= new List(); + return NamespaceCollapseState.SetCollapsed( + namespaceCollapseStates, + namespaceKey, + isCollapsed + ); + } + + public bool RemoveNamespaceCollapseState(string namespaceKey) + { + return NamespaceCollapseState.Remove(namespaceCollapseStates, namespaceKey); + } + public NamespaceCollapseState GetOrCreateCollapseState(string namespaceKey) { NamespaceCollapseState entry = namespaceCollapseStates.Find(o => diff --git a/Editor/DataVisualizer/Data/NamespaceCollapseState.cs b/Editor/DataVisualizer/Data/NamespaceCollapseState.cs index c433924..b86ffd6 100644 --- a/Editor/DataVisualizer/Data/NamespaceCollapseState.cs +++ b/Editor/DataVisualizer/Data/NamespaceCollapseState.cs @@ -1,6 +1,7 @@ namespace WallstopStudios.DataVisualizer.Editor.Data { using System; + using System.Collections.Generic; [Serializable] public sealed class NamespaceCollapseState @@ -8,6 +9,53 @@ public sealed class NamespaceCollapseState public string namespaceKey = string.Empty; public bool isCollapsed; + public static bool SetCollapsed( + List states, + string namespaceKey, + bool isCollapsed + ) + { + if (states == null || string.IsNullOrWhiteSpace(namespaceKey)) + { + return false; + } + + NamespaceCollapseState entry = states.Find(state => + string.Equals(state?.namespaceKey, namespaceKey, StringComparison.Ordinal) + ); + if (entry == null) + { + states.Add( + new NamespaceCollapseState + { + namespaceKey = namespaceKey, + isCollapsed = isCollapsed, + } + ); + return true; + } + + if (entry.isCollapsed == isCollapsed) + { + return false; + } + + entry.isCollapsed = isCollapsed; + return true; + } + + public static bool Remove(List states, string namespaceKey) + { + if (states == null || string.IsNullOrWhiteSpace(namespaceKey)) + { + return false; + } + + return states.RemoveAll(state => + string.Equals(state?.namespaceKey, namespaceKey, StringComparison.Ordinal) + ) > 0; + } + public NamespaceCollapseState Clone() { return new NamespaceCollapseState diff --git a/Editor/DataVisualizer/DataVisualizer.cs b/Editor/DataVisualizer/DataVisualizer.cs index 4710861..4d7d8bc 100644 --- a/Editor/DataVisualizer/DataVisualizer.cs +++ b/Editor/DataVisualizer/DataVisualizer.cs @@ -767,6 +767,20 @@ Func userStateApplier } } + public static bool ApplySelectActiveObjectPreference( + DataVisualizerSettings settings, + bool selectActiveObject + ) + { + if (settings == null || !settings.SetSelectActiveObject(selectActiveObject)) + { + return false; + } + + AssetDatabase.SaveAssets(); + return true; + } + private void RefreshAllViews() { Type selectedType = _namespaceController.SelectedType; @@ -3423,16 +3437,7 @@ private void BuildSettingsPopoverContent() selectionToggle.AddToClassList("settings-prefs-toggle"); selectionToggle.RegisterValueChangedCallback(evt => { - bool newSelectActiveObject = evt.newValue; - bool previousSelectActiveObject = Settings.selectActiveObject; - if (newSelectActiveObject == previousSelectActiveObject) - { - return; - } - - DataVisualizerSettings localSettings = Settings; - localSettings.selectActiveObject = newSelectActiveObject; - AssetDatabase.SaveAssets(); + ApplySelectActiveObjectPreference(Settings, evt.newValue); }); contentWrapper.Add(selectionToggle); @@ -9187,28 +9192,8 @@ private void SetIsNamespaceCollapsed(string namespaceKey, bool isCollapsed) } PersistSettings( - settings => - { - NamespaceCollapseState entry = settings.GetOrCreateCollapseState(namespaceKey); - if (entry.isCollapsed == isCollapsed) - { - return false; - } - - entry.isCollapsed = isCollapsed; - return true; - }, - userState => - { - NamespaceCollapseState entry = userState.GetOrCreateCollapseState(namespaceKey); - if (entry.isCollapsed == isCollapsed) - { - return false; - } - - entry.isCollapsed = isCollapsed; - return true; - } + settings => settings.SetNamespaceCollapsed(namespaceKey, isCollapsed), + userState => userState.SetNamespaceCollapsed(namespaceKey, isCollapsed) ); } diff --git a/Editor/DataVisualizer/NamespaceController.cs b/Editor/DataVisualizer/NamespaceController.cs index 83c8e25..0b94db3 100644 --- a/Editor/DataVisualizer/NamespaceController.cs +++ b/Editor/DataVisualizer/NamespaceController.cs @@ -692,29 +692,8 @@ bool isCollapsed } dataVisualizer.PersistSettings( - settings => - { - bool dirty = !settings.HasCollapseState(namespaceKey); - NamespaceCollapseState entry = settings.GetOrCreateCollapseState(namespaceKey); - if (entry.isCollapsed == isCollapsed) - { - dirty = true; - } - - entry.isCollapsed = isCollapsed; - return dirty; - }, - userState => - { - bool dirty = !userState.HasCollapseState(namespaceKey); - NamespaceCollapseState entry = userState.GetOrCreateCollapseState(namespaceKey); - if (entry.isCollapsed != isCollapsed) - { - dirty = true; - } - entry.isCollapsed = isCollapsed; - return dirty; - } + settings => settings.SetNamespaceCollapsed(namespaceKey, isCollapsed), + userState => userState.SetNamespaceCollapsed(namespaceKey, isCollapsed) ); } @@ -774,6 +753,11 @@ List typesToRemove if (changed) { + if (currentManagedList.Count == 0) + { + RemoveNamespaceCollapseState(dataVisualizer, namespaceKey); + } + PersistManagedTypesList(dataVisualizer, currentManagedList); DataVisualizer.SignalRefresh(); } @@ -802,6 +786,11 @@ private void HandleRemoveTypeConfirmed(DataVisualizer dataVisualizer, Type typeT { dataVisualizer.SetLastSelectedObjectGuidForType(typeName, null); RemoveTypeOrderEntry(dataVisualizer, namespaceKey, typeName); + if (currentManagedList.Count == 0) + { + RemoveNamespaceCollapseState(dataVisualizer, namespaceKey); + } + PersistManagedTypesList(dataVisualizer, currentManagedList); DataVisualizer.SignalRefresh(); } @@ -869,6 +858,22 @@ string typeName ); } + private static void RemoveNamespaceCollapseState( + DataVisualizer dataVisualizer, + string namespaceKey + ) + { + if (string.IsNullOrWhiteSpace(namespaceKey)) + { + return; + } + + dataVisualizer.PersistSettings( + settings => settings.RemoveNamespaceCollapseState(namespaceKey), + userState => userState.RemoveNamespaceCollapseState(namespaceKey) + ); + } + internal List GetManagedTypeNames(string namespaceKey) { return _managedTypes diff --git a/Tests/Editor/SettingsPersistenceTests.cs b/Tests/Editor/SettingsPersistenceTests.cs new file mode 100644 index 0000000..f456cbf --- /dev/null +++ b/Tests/Editor/SettingsPersistenceTests.cs @@ -0,0 +1,160 @@ +namespace WallstopStudios.DataVisualizer.Tests.Editor +{ + using System; + using NUnit.Framework; + using UnityEditor; + using UnityEngine; + using WallstopStudios.DataVisualizer.Editor; + using WallstopStudios.DataVisualizer.Editor.Data; + + public sealed class SettingsPersistenceTests + { + [Test] + public void Should_MarkSettingsDirty_When_SelectActiveObjectChanges() + { + DataVisualizerSettings settings = + ScriptableObject.CreateInstance(); + try + { + settings.selectActiveObject = false; + EditorUtility.ClearDirty(settings); + + Assert.IsFalse(EditorUtility.IsDirty(settings)); + Assert.IsTrue(settings.SetSelectActiveObject(true)); + Assert.IsTrue(settings.selectActiveObject); + Assert.IsTrue(EditorUtility.IsDirty(settings)); + } + finally + { + UnityEngine.Object.DestroyImmediate(settings); + } + } + + [Test] + public void Should_NotMarkSettingsDirty_When_SelectActiveObjectIsUnchanged() + { + DataVisualizerSettings settings = + ScriptableObject.CreateInstance(); + try + { + settings.selectActiveObject = true; + EditorUtility.ClearDirty(settings); + + Assert.IsFalse(settings.SetSelectActiveObject(true)); + Assert.IsTrue(settings.selectActiveObject); + Assert.IsFalse(EditorUtility.IsDirty(settings)); + } + finally + { + UnityEngine.Object.DestroyImmediate(settings); + } + } + + [Test] + public void Should_MarkSettingsDirty_When_SelectActiveObjectPreferenceChangesThroughWindowHelper() + { + DataVisualizerSettings settings = + ScriptableObject.CreateInstance(); + try + { + settings.selectActiveObject = false; + EditorUtility.ClearDirty(settings); + + Assert.IsTrue(DataVisualizer.ApplySelectActiveObjectPreference(settings, true)); + Assert.IsTrue(settings.selectActiveObject); + Assert.IsTrue(EditorUtility.IsDirty(settings)); + } + finally + { + UnityEngine.Object.DestroyImmediate(settings); + } + } + + [Test] + public void Should_ReportDirtyOnlyForActualCollapseStateChanges_When_UsingSettingsStore() + { + DataVisualizerSettings settings = + ScriptableObject.CreateInstance(); + try + { + const string namespaceKey = "Gameplay"; + + EditorUtility.ClearDirty(settings); + Assert.IsTrue( + settings.SetNamespaceCollapsed(namespaceKey, false), + "first write stores expanded state" + ); + Assert.IsTrue(EditorUtility.IsDirty(settings)); + + EditorUtility.ClearDirty(settings); + Assert.IsFalse( + settings.SetNamespaceCollapsed(namespaceKey, false), + "duplicate expanded write is unchanged" + ); + Assert.IsFalse(EditorUtility.IsDirty(settings)); + + Assert.IsTrue( + settings.SetNamespaceCollapsed(namespaceKey, true), + "changed collapse state is dirty" + ); + Assert.IsTrue(EditorUtility.IsDirty(settings)); + + EditorUtility.ClearDirty(settings); + Assert.IsFalse( + settings.SetNamespaceCollapsed(namespaceKey, true), + "duplicate collapsed write is unchanged" + ); + Assert.IsFalse(EditorUtility.IsDirty(settings)); + + Assert.IsTrue( + settings.RemoveNamespaceCollapseState(namespaceKey), + "removing stored state is dirty" + ); + Assert.IsTrue(EditorUtility.IsDirty(settings)); + + EditorUtility.ClearDirty(settings); + Assert.IsFalse( + settings.RemoveNamespaceCollapseState(namespaceKey), + "removing absent state is unchanged" + ); + Assert.IsFalse(EditorUtility.IsDirty(settings)); + } + finally + { + UnityEngine.Object.DestroyImmediate(settings); + } + } + + [Test] + public void Should_ReportDirtyOnlyForActualCollapseStateChanges_When_UsingUserStateStore() + { + DataVisualizerUserState userState = new(); + + AssertCollapseStateDirtySemantics( + userState.SetNamespaceCollapsed, + userState.RemoveNamespaceCollapseState + ); + } + + private static void AssertCollapseStateDirtySemantics( + Func setCollapsed, + Func removeCollapsed + ) + { + const string namespaceKey = "Gameplay"; + + Assert.IsTrue(setCollapsed(namespaceKey, false), "first write stores expanded state"); + Assert.IsFalse( + setCollapsed(namespaceKey, false), + "duplicate expanded write is unchanged" + ); + Assert.IsTrue(setCollapsed(namespaceKey, true), "changed collapse state is dirty"); + Assert.IsFalse( + setCollapsed(namespaceKey, true), + "duplicate collapsed write is unchanged" + ); + Assert.IsTrue(removeCollapsed(namespaceKey), "removing stored state is dirty"); + Assert.IsFalse(removeCollapsed(namespaceKey), "removing absent state is unchanged"); + } + } +} diff --git a/Tests/Editor/SettingsPersistenceTests.cs.meta b/Tests/Editor/SettingsPersistenceTests.cs.meta new file mode 100644 index 0000000..e3822bb --- /dev/null +++ b/Tests/Editor/SettingsPersistenceTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 3d777a01284649a5846b13a6a0354b8f +timeCreated: 1783445700 diff --git a/Tests/Editor/WallstopStudios.DataVisualizer.Tests.Editor.asmdef.meta b/Tests/Editor/WallstopStudios.DataVisualizer.Tests.Editor.asmdef.meta index 36116f3..034d9a3 100644 --- a/Tests/Editor/WallstopStudios.DataVisualizer.Tests.Editor.asmdef.meta +++ b/Tests/Editor/WallstopStudios.DataVisualizer.Tests.Editor.asmdef.meta @@ -2,6 +2,6 @@ fileFormatVersion: 2 guid: ab0fcdeaf8cd4b56aac58d4f88b1aa5f AssemblyDefinitionImporter: externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: \ No newline at end of file + userData: "" + assetBundleName: "" + assetBundleVariant: "" From d4a3f44b11bebacc4c2cb1cf30bc7d2583fb6398 Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Tue, 7 Jul 2026 11:03:55 -0700 Subject: [PATCH 2/9] Fix type ordering full-name identity --- .../DataVisualizer/Data/NamespaceTypeOrder.cs | 84 +++++++++++++ Editor/DataVisualizer/DataVisualizer.cs | 37 +++--- Tests/Editor/NamespaceOrderingTests.cs | 119 ++++++++++++++++++ Tests/Editor/NamespaceOrderingTests.cs.meta | 11 ++ 4 files changed, 237 insertions(+), 14 deletions(-) create mode 100644 Tests/Editor/NamespaceOrderingTests.cs create mode 100644 Tests/Editor/NamespaceOrderingTests.cs.meta diff --git a/Editor/DataVisualizer/Data/NamespaceTypeOrder.cs b/Editor/DataVisualizer/Data/NamespaceTypeOrder.cs index 9267cd9..b1ba373 100644 --- a/Editor/DataVisualizer/Data/NamespaceTypeOrder.cs +++ b/Editor/DataVisualizer/Data/NamespaceTypeOrder.cs @@ -10,6 +10,72 @@ public sealed class NamespaceTypeOrder public string namespaceKey = string.Empty; public List typeNames = new(); + public static int CompareTypesByFullNameOrder( + Type lhs, + Type rhs, + IReadOnlyList typeFullNameOrder + ) + { + return CompareTypeFullNames( + lhs?.FullName ?? string.Empty, + rhs?.FullName ?? string.Empty, + typeFullNameOrder + ); + } + + public static int CompareTypesByFullName(Type lhs, Type rhs) + { + return string.Compare( + lhs?.FullName ?? string.Empty, + rhs?.FullName ?? string.Empty, + StringComparison.Ordinal + ); + } + + public static int CompareTypesByNameThenFullName(Type lhs, Type rhs) + { + int nameComparison = string.Compare( + lhs?.Name ?? string.Empty, + rhs?.Name ?? string.Empty, + StringComparison.Ordinal + ); + return nameComparison != 0 ? nameComparison : CompareTypesByFullName(lhs, rhs); + } + + public static int CompareTypeFullNames( + string lhsFullName, + string rhsFullName, + IReadOnlyList typeFullNameOrder + ) + { + int indexA = IndexOf(typeFullNameOrder, lhsFullName); + int indexB = IndexOf(typeFullNameOrder, rhsFullName); + + switch (indexA) + { + case >= 0 when indexB >= 0: + return indexA.CompareTo(indexB); + case >= 0: + return -1; + } + + return 0 <= indexB + ? 1 + : string.Compare(lhsFullName, rhsFullName, StringComparison.Ordinal); + } + + public static Type FindTypeByFullName(IEnumerable types, string typeFullName) + { + if (types == null || string.IsNullOrWhiteSpace(typeFullName)) + { + return null; + } + + return types.FirstOrDefault(type => + string.Equals(type?.FullName, typeFullName, StringComparison.Ordinal) + ); + } + public NamespaceTypeOrder Clone() { return new NamespaceTypeOrder @@ -18,5 +84,23 @@ public NamespaceTypeOrder Clone() typeNames = typeNames?.ToList() ?? new List(), }; } + + private static int IndexOf(IReadOnlyList values, string value) + { + if (values == null || string.IsNullOrWhiteSpace(value)) + { + return -1; + } + + for (int i = 0; i < values.Count; ++i) + { + if (string.Equals(values[i], value, StringComparison.Ordinal)) + { + return i; + } + } + + return -1; + } } } diff --git a/Editor/DataVisualizer/DataVisualizer.cs b/Editor/DataVisualizer/DataVisualizer.cs index 4d7d8bc..b317857 100644 --- a/Editor/DataVisualizer/DataVisualizer.cs +++ b/Editor/DataVisualizer/DataVisualizer.cs @@ -789,7 +789,7 @@ private void RefreshAllViews() selectedType != null ? NamespaceController.GetNamespaceKey(selectedType) : string.Empty; - string previousTypeName = selectedType?.Name; + string previousTypeFullName = selectedType?.FullName; string previousObjectGuid = null; if (_selectedObject != null) { @@ -821,10 +821,11 @@ private void RefreshAllViews() ); if (0 < typesInNamespace?.Count) { - if (!string.IsNullOrWhiteSpace(previousTypeName)) + if (!string.IsNullOrWhiteSpace(previousTypeFullName)) { - selectedType = typesInNamespace.Find(t => - string.Equals(t.Name, previousTypeName, StringComparison.Ordinal) + selectedType = NamespaceTypeOrder.FindTypeByFullName( + typesInNamespace, + previousTypeFullName ); } @@ -4167,6 +4168,7 @@ private VisualElement CreateNamespaceColumn() .Where(type => !currentlyManagedTypes.Contains(type)) .Distinct() .ToList(); + scriptableObjectTypes.Sort(NamespaceTypeOrder.CompareTypesByFullName); bool stateChanged = false; foreach (Type typeToAdd in scriptableObjectTypes) @@ -4257,8 +4259,8 @@ private VisualElement CreateNamespaceColumn() .Where(IsLoadableType) .Where(type => !currentlyManagedTypes.Contains(type)) .Distinct() - .OrderBy(type => type.Name) .ToList(); + scriptableObjectTypes.Sort(NamespaceTypeOrder.CompareTypesByFullName); bool stateChanged = false; foreach (Type typeToAdd in scriptableObjectTypes) @@ -4341,7 +4343,7 @@ private void BuildTypeAddList(string filter = null) HashSet managedTypeFullNames = _namespaceController .GetAllManagedTypeNames() - .ToHashSet(StringComparer.OrdinalIgnoreCase); + .ToHashSet(StringComparer.Ordinal); IOrderedEnumerable> groupedTypes = LoadRelevantScriptableObjectTypes() @@ -4353,6 +4355,8 @@ private void BuildTypeAddList(string filter = null) foreach (IGrouping group in groupedTypes) { string namespaceKey = group.Key; + List orderedGroupTypes = group.ToList(); + orderedGroupTypes.Sort(NamespaceTypeOrder.CompareTypesByNameThenFullName); List addableTypes = new(); List typesToShowInGroup = new(); @@ -4362,7 +4366,7 @@ private void BuildTypeAddList(string filter = null) namespaceKey.Contains(term, StringComparison.OrdinalIgnoreCase) ); - foreach (Type type in group.OrderBy(t => t.Name)) + foreach (Type type in orderedGroupTypes) { string typeName = type.Name; bool typeMatchesSearch = @@ -4398,7 +4402,7 @@ private void BuildTypeAddList(string filter = null) header.AddToClassList(StyleConstants.ExpandedClass); } - foreach (Type type in group.OrderBy(t => t.Name)) + foreach (Type type in orderedGroupTypes) { string typeName = type.Name; bool typeMatchesSearch = @@ -7942,16 +7946,16 @@ private void LoadScriptableObjectTypes() managedTypeFullNames = settings .typeOrders?.SelectMany(order => order.typeNames) - .ToHashSet(StringComparer.OrdinalIgnoreCase) - ?? new HashSet(StringComparer.OrdinalIgnoreCase); + .ToHashSet(StringComparer.Ordinal) + ?? new HashSet(StringComparer.Ordinal); } else { managedTypeFullNames = UserState .typeOrders?.SelectMany(order => order.typeNames) - .ToHashSet(StringComparer.OrdinalIgnoreCase) - ?? new HashSet(StringComparer.OrdinalIgnoreCase); + .ToHashSet(StringComparer.Ordinal) + ?? new HashSet(StringComparer.Ordinal); } List allObjectTypes = LoadRelevantScriptableObjectTypes(); @@ -7976,9 +7980,14 @@ private void LoadScriptableObjectTypes() foreach ((string key, List types) in orderedTypes) { - List customTypeNameOrder = GetTypeOrderForNamespace(key); + List customTypeFullNameOrder = GetTypeOrderForNamespace(key); types.Sort( - (lhs, rhs) => CompareUsingCustomOrder(lhs.Name, rhs.Name, customTypeNameOrder) + (lhs, rhs) => + NamespaceTypeOrder.CompareTypesByFullNameOrder( + lhs, + rhs, + customTypeFullNameOrder + ) ); } diff --git a/Tests/Editor/NamespaceOrderingTests.cs b/Tests/Editor/NamespaceOrderingTests.cs new file mode 100644 index 0000000..ac74245 --- /dev/null +++ b/Tests/Editor/NamespaceOrderingTests.cs @@ -0,0 +1,119 @@ +namespace WallstopStudios.DataVisualizer.Tests.Editor +{ + using System; + using System.Collections.Generic; + using System.Linq; + using NUnit.Framework; + using UnityEngine; + using WallstopStudios.DataVisualizer.Editor.Data; + using CollisionA = WallstopStudios.DataVisualizer.Tests.Editor.TypeIdentityCollision.First.Data; + using CollisionB = WallstopStudios.DataVisualizer.Tests.Editor.TypeIdentityCollision.Second.Data; + + public sealed class NamespaceOrderingTests + { + [Test] + public void Should_OrderTypesByFullName_When_ShortTypeNamesCollide() + { + Type firstType = typeof(CollisionA.OrderCollisionData); + Type secondType = typeof(CollisionB.OrderCollisionData); + List types = new() { firstType, secondType }; + List persistedFullNameOrder = new() { secondType.FullName, firstType.FullName }; + + types.Sort( + (lhs, rhs) => + NamespaceTypeOrder.CompareTypesByFullNameOrder(lhs, rhs, persistedFullNameOrder) + ); + + CollectionAssert.AreEqual( + persistedFullNameOrder, + types.Select(type => type.FullName).ToList() + ); + Assert.AreEqual(firstType.Name, secondType.Name); + } + + [Test] + public void Should_FallBackToFullNameOrdering_When_NoCustomOrderExists() + { + Type firstType = typeof(CollisionA.OrderCollisionData); + Type secondType = typeof(CollisionB.OrderCollisionData); + List types = new() { secondType, firstType }; + + types.Sort( + (lhs, rhs) => + NamespaceTypeOrder.CompareTypesByFullNameOrder(lhs, rhs, Array.Empty()) + ); + + CollectionAssert.AreEqual( + new[] { firstType.FullName, secondType.FullName }.OrderBy( + typeFullName => typeFullName, + StringComparer.Ordinal + ), + types.Select(type => type.FullName).ToList() + ); + } + + [Test] + public void Should_OrderTypesByFullName_When_SeedingPersistedOrder() + { + Type firstType = typeof(CollisionA.OrderCollisionData); + Type secondType = typeof(CollisionB.OrderCollisionData); + List types = new() { secondType, firstType }; + + types.Sort(NamespaceTypeOrder.CompareTypesByFullName); + + CollectionAssert.AreEqual( + new[] { firstType.FullName, secondType.FullName }.OrderBy( + typeFullName => typeFullName, + StringComparer.Ordinal + ), + types.Select(type => type.FullName).ToList() + ); + } + + [Test] + public void Should_UseFullNameTieBreaker_When_DisplayNamesCollide() + { + Type firstType = typeof(CollisionA.OrderCollisionData); + Type secondType = typeof(CollisionB.OrderCollisionData); + List types = new() { secondType, firstType }; + + types.Sort(NamespaceTypeOrder.CompareTypesByNameThenFullName); + + CollectionAssert.AreEqual( + new[] { firstType.FullName, secondType.FullName }.OrderBy( + typeFullName => typeFullName, + StringComparer.Ordinal + ), + types.Select(type => type.FullName).ToList() + ); + Assert.AreEqual(firstType.Name, secondType.Name); + } + + [Test] + public void Should_FindTypeByFullName_When_ShortTypeNamesCollide() + { + Type firstType = typeof(CollisionA.OrderCollisionData); + Type secondType = typeof(CollisionB.OrderCollisionData); + List types = new() { firstType, secondType }; + + Type resolvedType = NamespaceTypeOrder.FindTypeByFullName(types, secondType.FullName); + + Assert.AreSame(secondType, resolvedType); + Assert.AreEqual(firstType.Name, secondType.Name); + } + } +} + +namespace WallstopStudios.DataVisualizer.Tests.Editor.TypeIdentityCollision.First.Data +{ + using UnityEngine; + + public sealed class OrderCollisionData : ScriptableObject { } +} + +namespace WallstopStudios.DataVisualizer.Tests.Editor.TypeIdentityCollision.Second.Data +{ + using UnityEngine; + + public sealed class OrderCollisionData : ScriptableObject { } +} diff --git a/Tests/Editor/NamespaceOrderingTests.cs.meta b/Tests/Editor/NamespaceOrderingTests.cs.meta new file mode 100644 index 0000000..e97d355 --- /dev/null +++ b/Tests/Editor/NamespaceOrderingTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6046e5e9b9084fce809605fd0b94290d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: "" + assetBundleName: "" + assetBundleVariant: "" From 08ba4e60ed38c7712df2a3dee84448948210a9a5 Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Tue, 7 Jul 2026 11:30:06 -0700 Subject: [PATCH 3/9] Fix selection persistence identity --- .../Data/DataVisualizerSettings.cs | 50 ++- .../Data/DataVisualizerUserState.cs | 48 ++- Editor/DataVisualizer/DataVisualizer.cs | 346 +++++++++++------- .../Unity/DataVisualizerAssetPostProcessor.cs | 22 +- Tests/Editor/SelectionPersistenceTests.cs | 254 +++++++++++++ .../Editor/SelectionPersistenceTests.cs.meta | 11 + 6 files changed, 577 insertions(+), 154 deletions(-) create mode 100644 Tests/Editor/SelectionPersistenceTests.cs create mode 100644 Tests/Editor/SelectionPersistenceTests.cs.meta diff --git a/Editor/DataVisualizer/Data/DataVisualizerSettings.cs b/Editor/DataVisualizer/Data/DataVisualizerSettings.cs index 7e43663..bacc5be 100644 --- a/Editor/DataVisualizer/Data/DataVisualizerSettings.cs +++ b/Editor/DataVisualizer/Data/DataVisualizerSettings.cs @@ -156,24 +156,54 @@ internal List GetOrCreateObjectOrderList(string typeFullName) return entry.ObjectGuids; } - internal void SetLastObjectForType(string typeName, string guid) + internal bool SetLastObjectForType(string typeName, string guid) { if (string.IsNullOrWhiteSpace(typeName)) { - return; + return false; } + lastObjectSelections ??= new List(); + int existingIndex = lastObjectSelections.FindIndex(e => + string.Equals(e.typeFullName, typeName, StringComparison.Ordinal) + ); if (string.IsNullOrWhiteSpace(guid)) { - return; + if (existingIndex < 0) + { + return false; + } + + lastObjectSelections.RemoveAt(existingIndex); + MarkDirty(); + return true; } - lastObjectSelections.RemoveAll(e => - string.Equals(e.typeFullName, typeName, StringComparison.Ordinal) - ); - lastObjectSelections.Add( - new LastObjectSelectionEntry { typeFullName = typeName, objectGuid = guid } - ); + if ( + existingIndex >= 0 + && string.Equals( + lastObjectSelections[existingIndex].objectGuid, + guid, + StringComparison.Ordinal + ) + ) + { + return false; + } + + if (existingIndex >= 0) + { + lastObjectSelections[existingIndex].objectGuid = guid; + } + else + { + lastObjectSelections.Add( + new LastObjectSelectionEntry { typeFullName = typeName, objectGuid = guid } + ); + } + + MarkDirty(); + return true; } internal string GetLastObjectForType(string typeName) @@ -184,7 +214,7 @@ internal string GetLastObjectForType(string typeName) } return lastObjectSelections - .Find(e => string.Equals(e.typeFullName, typeName, StringComparison.Ordinal)) + ?.Find(e => string.Equals(e.typeFullName, typeName, StringComparison.Ordinal)) ?.objectGuid; } diff --git a/Editor/DataVisualizer/Data/DataVisualizerUserState.cs b/Editor/DataVisualizer/Data/DataVisualizerUserState.cs index ac0d457..572e252 100644 --- a/Editor/DataVisualizer/Data/DataVisualizerUserState.cs +++ b/Editor/DataVisualizer/Data/DataVisualizerUserState.cs @@ -67,24 +67,52 @@ public List GetOrCreateObjectOrderList(string typeFullName) return entry.ObjectGuids; } - public void SetLastObjectForType(string typeName, string guid) + public bool SetLastObjectForType(string typeName, string guid) { if (string.IsNullOrWhiteSpace(typeName)) { - return; + return false; } + lastObjectSelections ??= new List(); + int existingIndex = lastObjectSelections.FindIndex(e => + string.Equals(e.typeFullName, typeName, StringComparison.Ordinal) + ); if (string.IsNullOrWhiteSpace(guid)) { - return; + if (existingIndex < 0) + { + return false; + } + + lastObjectSelections.RemoveAt(existingIndex); + return true; } - lastObjectSelections.RemoveAll(e => - string.Equals(e.typeFullName, typeName, StringComparison.Ordinal) - ); - lastObjectSelections.Add( - new LastObjectSelectionEntry { typeFullName = typeName, objectGuid = guid } - ); + if ( + existingIndex >= 0 + && string.Equals( + lastObjectSelections[existingIndex].objectGuid, + guid, + StringComparison.Ordinal + ) + ) + { + return false; + } + + if (existingIndex >= 0) + { + lastObjectSelections[existingIndex].objectGuid = guid; + } + else + { + lastObjectSelections.Add( + new LastObjectSelectionEntry { typeFullName = typeName, objectGuid = guid } + ); + } + + return true; } public string GetLastObjectForType(string typeName) @@ -95,7 +123,7 @@ public string GetLastObjectForType(string typeName) } return lastObjectSelections - .Find(e => string.Equals(e.typeFullName, typeName, StringComparison.Ordinal)) + ?.Find(e => string.Equals(e.typeFullName, typeName, StringComparison.Ordinal)) ?.objectGuid; } diff --git a/Editor/DataVisualizer/DataVisualizer.cs b/Editor/DataVisualizer/DataVisualizer.cs index b317857..e61879a 100644 --- a/Editor/DataVisualizer/DataVisualizer.cs +++ b/Editor/DataVisualizer/DataVisualizer.cs @@ -42,9 +42,7 @@ public sealed class DataVisualizer : EditorWindow private const string SettingsDefaultPath = "Assets/Editor/DataVisualizerSettings.asset"; private const string UserStateFileName = "DataVisualizerUserState.json"; - private const string NamespaceItemClass = "namespace-item"; private const string NamespaceGroupHeaderClass = "namespace-group-header"; - private const string NamespaceIndicatorClass = "namespace-indicator"; private const string ObjectItemClass = "object-item"; private const string ObjectItemContentClass = "object-item-content"; private const string ObjectItemActionsClass = "object-item-actions"; @@ -802,36 +800,12 @@ private void RefreshAllViews() LoadScriptableObjectTypes(); - int namespaceIndex = -1; - if (!string.IsNullOrWhiteSpace(previousNamespaceKey)) - { - namespaceIndex = _namespaceOrder.GetValueOrDefault(previousNamespaceKey, -1); - } - - if (namespaceIndex < 0 && 0 < _scriptableObjectTypes.Count) - { - namespaceIndex = 0; - } - - if (0 <= namespaceIndex) - { - List typesInNamespace = _scriptableObjectTypes.GetValueOrDefault( - previousNamespaceKey, - null - ); - if (0 < typesInNamespace?.Count) - { - if (!string.IsNullOrWhiteSpace(previousTypeFullName)) - { - selectedType = NamespaceTypeOrder.FindTypeByFullName( - typesInNamespace, - previousTypeFullName - ); - } - - selectedType ??= typesInNamespace[0]; - } - } + selectedType = ResolveSelectedTypeByFullName( + _scriptableObjectTypes, + _namespaceOrder, + previousNamespaceKey, + previousTypeFullName + ); // Load once. For an unchanged type the SelectType call near the end no-ops its own load, // so reload here (a refresh must re-scan). For a changed type, let that single SelectType @@ -907,6 +881,14 @@ private void RefreshAllViews() } private VisualElement FindAncestorNamespaceGroup(VisualElement startingElement) + { + return FindAncestorNamespaceGroup(startingElement, _namespaceListContainer); + } + + public static VisualElement FindAncestorNamespaceGroup( + VisualElement startingElement, + VisualElement namespaceListContainer + ) { if (startingElement == null) { @@ -914,9 +896,9 @@ private VisualElement FindAncestorNamespaceGroup(VisualElement startingElement) } VisualElement currentElement = startingElement; - while (currentElement != null && currentElement != _namespaceListContainer) + while (currentElement != null && currentElement != namespaceListContainer) { - if (currentElement.ClassListContains("object-item")) + if (currentElement.ClassListContains(StyleConstants.NamespaceItemClass)) { return currentElement; } @@ -927,6 +909,107 @@ private VisualElement FindAncestorNamespaceGroup(VisualElement startingElement) return null; } + public static string FindFirstNamespaceKeyByOrder( + IReadOnlyDictionary namespaceOrder + ) + { + if (namespaceOrder == null || namespaceOrder.Count == 0) + { + return null; + } + + string firstNamespaceKey = null; + int firstNamespaceIndex = int.MaxValue; + foreach (KeyValuePair entry in namespaceOrder) + { + if ( + entry.Value < firstNamespaceIndex + || ( + entry.Value == firstNamespaceIndex + && ( + firstNamespaceKey == null + || string.CompareOrdinal(entry.Key, firstNamespaceKey) < 0 + ) + ) + ) + { + firstNamespaceKey = entry.Key; + firstNamespaceIndex = entry.Value; + } + } + + return firstNamespaceKey; + } + + public static Type ResolveSelectedTypeByFullName( + IReadOnlyDictionary> typesByNamespace, + IReadOnlyDictionary namespaceOrder, + string savedNamespaceKey, + string savedTypeFullName + ) + { + if (typesByNamespace == null || typesByNamespace.Count == 0) + { + return null; + } + + if (!string.IsNullOrWhiteSpace(savedTypeFullName)) + { + Type savedType = NamespaceTypeOrder.FindTypeByFullName( + typesByNamespace.Values.SelectMany(types => types ?? Enumerable.Empty()), + savedTypeFullName + ); + if (savedType != null) + { + return savedType; + } + } + + if ( + !string.IsNullOrWhiteSpace(savedNamespaceKey) + && typesByNamespace.TryGetValue(savedNamespaceKey, out List savedTypes) + && savedTypes is { Count: > 0 } + ) + { + return savedTypes[0]; + } + + string firstOrderedNamespaceKey = FindFirstNamespaceKeyByOrder(namespaceOrder); + if ( + !string.IsNullOrWhiteSpace(firstOrderedNamespaceKey) + && typesByNamespace.TryGetValue( + firstOrderedNamespaceKey, + out List firstOrderedTypes + ) + && firstOrderedTypes is { Count: > 0 } + ) + { + return firstOrderedTypes[0]; + } + + foreach ( + string namespaceKey in (namespaceOrder ?? new Dictionary()) + .OrderBy(entry => entry.Value) + .ThenBy(entry => entry.Key, StringComparer.Ordinal) + .Select(entry => entry.Key) + ) + { + if ( + typesByNamespace.TryGetValue(namespaceKey, out List orderedTypes) + && orderedTypes is { Count: > 0 } + ) + { + return orderedTypes[0]; + } + } + + return typesByNamespace + .OrderBy(entry => entry.Key, StringComparer.Ordinal) + .Select(entry => entry.Value) + .FirstOrDefault(types => types is { Count: > 0 }) + ?.FirstOrDefault(); + } + private void ExpandNamespaceGroupIfNeeded(VisualElement namespaceGroupItem, bool saveState) { if (namespaceGroupItem == null) @@ -1060,58 +1143,18 @@ private void RestorePreviousSelection() return; } - string savedNamespaceKey = GetLastSelectedNamespaceKey(); - List typesInNamespace; - int namespaceIndex = -1; - - if (!string.IsNullOrWhiteSpace(savedNamespaceKey)) - { - namespaceIndex = _namespaceOrder.GetValueOrDefault(savedNamespaceKey, -1); - } - - if (0 <= namespaceIndex) - { - typesInNamespace = _scriptableObjectTypes.GetValueOrDefault( - savedNamespaceKey, - null - ); - } - else if (0 < _namespaceOrder.Count) - { - int bestIndex = _scriptableObjectTypes.Count; - string bestNamespace = null; - foreach (KeyValuePair entry in _namespaceOrder) - { - if (entry.Value < bestIndex) - { - bestNamespace = entry.Key; - } - } - - typesInNamespace = _scriptableObjectTypes.GetValueOrDefault(bestNamespace, null); - } - else - { - typesInNamespace = null; - } + Type selectedType = ResolveSelectedTypeByFullName( + _scriptableObjectTypes, + _namespaceOrder, + GetLastSelectedNamespaceKey(), + GetLastSelectedTypeName() + ); - if (typesInNamespace is not { Count: > 0 }) + if (selectedType == null) { return; } - string savedTypeName = GetLastSelectedTypeName(); - Type selectedType = null; - - if (!string.IsNullOrWhiteSpace(savedTypeName)) - { - selectedType = typesInNamespace.Find(t => - string.Equals(t.FullName, savedTypeName, StringComparison.Ordinal) - ); - } - - selectedType ??= typesInNamespace[0]; - // Build namespace view first so type selection is visible BuildNamespaceView(); @@ -1125,34 +1168,10 @@ private void RestorePreviousSelection() VisualElement typeElementToSelect = FindTypeElement(selectedType); if (typeElementToSelect != null) { - VisualElement ancestorGroup = null; - VisualElement currentElement = typeElementToSelect; - while (currentElement != null && currentElement != _namespaceListContainer) - { - if (currentElement.ClassListContains(NamespaceItemClass)) - { - ancestorGroup = currentElement; - break; - } - - currentElement = currentElement.parent; - } - + VisualElement ancestorGroup = FindAncestorNamespaceGroup(typeElementToSelect); if (ancestorGroup != null) { - Label indicator = ancestorGroup.Q