diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs index 9c3ede1100..9b907e1755 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs @@ -342,6 +342,82 @@ public void TestPartialReplace() AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); } + [Test] + public void TestReplaceWithNoItems() + { + addItems(5); + + AddStep("replace list", () => list.Items.ReplaceRange(0, list.Items.Count, [])); + AddUntilStep("wait for clear", () => !list.ItemMap.Values.Any()); + } + + [Test] + public void TestReplaceEmptyListWithNoItems() + { + AddStep("replace list", () => list.Items.ReplaceRange(0, 0, [])); + } + + [Test] + public void TestReplaceEmptyListWithItems() + { + AddStep("replace list", () => list.Items.ReplaceRange(0, 0, [100, 101])); + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + } + + [Test] + public void TestReplaceOnlyAppliesDifferences() + { + // ReSharper disable once LocalVariableHidesMember (intentional, using the ƒield is wrong for this test) + TestLoadCountingList list = null!; + + AddStep("create list", () => Child = list = new TestLoadCountingList + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Size = new Vector2(500, 300) + }); + + AddStep("replace {[]} with [1]", () => list.Items.ReplaceRange(0, list.Items.Count, [1])); + assertLoadCount(1, 1); + + AddStep("replace {[1]} with [1]", () => list.Items.ReplaceRange(0, 1, [1])); + assertLoadCount(1, 1); + + AddStep("add {1, []} with [2, 3, 4]", () => list.Items.ReplaceRange(list.Items.Count, 0, [2, 3, 4])); + assertLoadCount(2, 1); + assertLoadCount(3, 1); + assertLoadCount(4, 1); + + AddStep("replace {1, [2, 3], 4} with [3, 5]", () => list.Items.ReplaceRange(1, 2, [3, 5])); + assertLoadCount(3, 1); + assertLoadCount(5, 1); + + AddStep("replace {1, [3, 5], 4} with [2, 3]", () => list.Items.ReplaceRange(1, 2, [2, 3])); + assertLoadCount(2, 2); + assertLoadCount(3, 1); + + AddStep("replace {[1, 2, 3, 4]} with [1]", () => list.Items.ReplaceRange(0, list.Items.Count, [1])); + assertLoadCount(1, 1); + + AddStep("replace {[1]} with []", () => list.Items.ReplaceRange(0, list.Items.Count, [])); + assertLoadCount(1, 1); + + AddStep("replace {[]} with [0, 1, 2, 3, 4, 5, 6]", () => list.Items.ReplaceRange(0, list.Items.Count, [0, 1, 2, 3, 4, 5, 6])); + assertLoadCount(0, 1); + assertLoadCount(1, 2); + assertLoadCount(2, 3); + assertLoadCount(3, 2); + assertLoadCount(4, 2); + assertLoadCount(5, 2); + assertLoadCount(6, 1); + + void assertLoadCount(int item, int expectedTimesLoaded) + { + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + AddAssert($"item {item} loaded {expectedTimesLoaded} times", () => list.LoadCount[item], () => Is.EqualTo(expectedTimesLoaded)); + } + } + private void addDragSteps(int from, int to, int[] expectedSequence) { AddStep($"move to {from}", () => @@ -425,5 +501,32 @@ private void load() } } } + + private partial class TestLoadCountingList : BasicRearrangeableListContainer + { + /// + /// Dictionary of item -> # of times a drawable was loaded for it. + /// + public readonly Dictionary LoadCount = new Dictionary(); + + public new IReadOnlyDictionary> ItemMap => base.ItemMap; + + protected override BasicRearrangeableListItem CreateBasicItem(int item) + => new TestRearrangeableListItem(item, () => LoadCount[item] = LoadCount.GetValueOrDefault(item) + 1); + + private partial class TestRearrangeableListItem : BasicRearrangeableListItem + { + private readonly Action onLoad; + + public TestRearrangeableListItem(int item, Action onLoad) + : base(item, false) + { + this.onLoad = onLoad; + } + + [BackgroundDependencyLoader] + private void load() => onLoad(); + } + } } } diff --git a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs index b133534c31..ebef10929e 100644 --- a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs +++ b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs @@ -1,10 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; -using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; @@ -22,6 +19,7 @@ namespace osu.Framework.Graphics.Containers /// /// The type of rearrangeable item. public abstract partial class RearrangeableListContainer : CompositeDrawable + where TModel : notnull { private const float exp_base = 1.05f; @@ -51,7 +49,7 @@ public abstract partial class RearrangeableListContainer : CompositeDraw protected IReadOnlyDictionary> ItemMap => itemMap; private readonly Dictionary> itemMap = new Dictionary>(); - private RearrangeableListItem currentlyDraggedItem; + private RearrangeableListItem? currentlyDraggedItem; private Vector2 screenSpaceDragPosition; /// @@ -82,16 +80,16 @@ protected virtual void OnItemsChanged() { } - private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e) + private void collectionChanged(object? sender, NotifyCollectionChangedEventArgs e) { switch (e.Action) { case NotifyCollectionChangedAction.Add: - addItems(e.NewItems); + addItems(e.NewItems?.Cast() ?? []); break; case NotifyCollectionChangedAction.Remove: - removeItems(e.OldItems); + removeItems(e.OldItems?.Cast() ?? []); // Explicitly reset scroll position here so that ScrollContainer doesn't retain our // scroll position if we quickly add new items after calling a Clear(). @@ -107,8 +105,11 @@ private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e break; case NotifyCollectionChangedAction.Replace: - removeItems(e.OldItems); - addItems(e.NewItems); + IEnumerable tOldItems = e.OldItems?.Cast() ?? []; + IEnumerable tNewItems = e.NewItems?.Cast() ?? []; + + removeItems(tOldItems.Except(tNewItems)); + addItems(tNewItems.Except(tOldItems)); break; case NotifyCollectionChangedAction.Move: @@ -118,9 +119,9 @@ private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e } } - private void removeItems(IList items) + private void removeItems(IEnumerable items) { - foreach (var item in items.Cast()) + foreach (var item in items) { if (currentlyDraggedItem != null && EqualityComparer.Default.Equals(currentlyDraggedItem.Model, item)) currentlyDraggedItem = null; @@ -137,11 +138,11 @@ private void removeItems(IList items) OnItemsChanged(); } - private void addItems(IList items) + private void addItems(IEnumerable items) { var drawablesToAdd = new List(); - foreach (var item in items.Cast()) + foreach (var item in items) { if (itemMap.ContainsKey(item)) { @@ -191,7 +192,7 @@ private void sortItems() if (drawable.Parent != ListContainer) continue; - ListContainer!.SetLayoutPosition(drawable, i); + ListContainer.SetLayoutPosition(drawable, i); } } @@ -216,9 +217,7 @@ protected override void Update() protected override void UpdateAfterChildren() { base.UpdateAfterChildren(); - - if (currentlyDraggedItem != null) - updateArrangement(); + updateArrangement(); } private void updateScrollPosition() @@ -243,6 +242,9 @@ private void updateScrollPosition() private void updateArrangement() { + if (currentlyDraggedItem == null) + return; + var localPos = ListContainer.ToLocalSpace(screenSpaceDragPosition); int srcIndex = Items.IndexOf(currentlyDraggedItem.Model);