From 9e385850d7115f9854f6a666a954a38df4e45b79 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 14 Nov 2024 19:29:53 +0900 Subject: [PATCH 1/3] Add failing test --- .../TestSceneRearrangeableListContainer.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs index 83450a664e..c4397a8c50 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs @@ -1,8 +1,6 @@ // 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.Generic; using System.Linq; @@ -21,9 +19,8 @@ namespace osu.Framework.Tests.Visual.UserInterface { public partial class TestSceneRearrangeableListContainer : ManualInputManagerTestScene { - private TestRearrangeableList list; - - private Container listContainer; + private TestRearrangeableList list = null!; + private Container listContainer = null!; [SetUp] public void Setup() => Schedule(() => @@ -85,7 +82,7 @@ public void TestRemoveItem() addItems(item_count); - List items = null; + List items = null!; AddStep("get item references", () => items = new List(list.ItemMap.Values.ToList())); @@ -278,7 +275,7 @@ public void TestNotScrolledToTopOnRemove() [Test] public void TestRemoveDuringLoadAndReAdd() { - TestDelayedLoadRearrangeableList delayedList = null; + TestDelayedLoadRearrangeableList delayedList = null!; AddStep("create list", () => Child = delayedList = new TestDelayedLoadRearrangeableList()); @@ -327,6 +324,15 @@ public void TestDragSynchronisation() }); } + [Test] + public void TestReplaceEntireList() + { + addItems(1); + + AddStep("replace list", () => list.Items.ReplaceRange(0, list.Items.Count, [100])); + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + } + private void addDragSteps(int from, int to, int[] expectedSequence) { AddStep($"move to {from}", () => From 23443bfd26ef710da0f96dbd75cecdc07dc593c5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 14 Nov 2024 19:57:37 +0900 Subject: [PATCH 2/3] Fix `RearrangeableListContainer<>` crashing on replacement operations --- .../Containers/RearrangeableListContainer.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs index 1363206638..b133534c31 100644 --- a/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs +++ b/osu.Framework/Graphics/Containers/RearrangeableListContainer.cs @@ -183,11 +183,15 @@ private void sortItems() { for (int i = 0; i < Items.Count; i++) { - var drawable = itemMap[Items[i]]; + // A drawable for the item may not exist yet, for example in a replace-range operation where the removal happens first. + if (!itemMap.TryGetValue(Items[i], out var drawable)) + continue; + + // The item may not be loaded yet, because add operations are asynchronous. + if (drawable.Parent != ListContainer) + continue; - // If the async load didn't complete, the item wouldn't exist in the container and an exception would be thrown - if (drawable.Parent == ListContainer) - ListContainer!.SetLayoutPosition(drawable, i); + ListContainer!.SetLayoutPosition(drawable, i); } } From 69d1f7f5b8a88839121353cfb0ee3d705cf30d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 14 Nov 2024 13:43:17 +0100 Subject: [PATCH 3/3] Add another test for partial replace operation For no particular reason other than to see if it does the right thing, and it does. --- .../UserInterface/TestSceneRearrangeableListContainer.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs index c4397a8c50..9c3ede1100 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs @@ -333,6 +333,15 @@ public void TestReplaceEntireList() AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); } + [Test] + public void TestPartialReplace() + { + addItems(5); + + AddStep("replace list", () => list.Items.ReplaceRange(2, 2, [100, 101])); + AddUntilStep("wait for items to load", () => list.ItemMap.Values.All(i => i.IsLoaded)); + } + private void addDragSteps(int from, int to, int[] expectedSequence) { AddStep($"move to {from}", () =>