Skip to content

Commit

Permalink
Merge pull request #6425 from smoogipoo/rearrangeable-dedupe-replace
Browse files Browse the repository at this point in the history 
Make `RearrangeableListContainer<>` only replace differences
  • Loading branch information
@bdach
bdach authored Nov 18, 2024
2 parents 019b49f + ecf9dd0 commit dc98996
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 17 deletions.
103 changes: 103 additions & 0 deletions osu.Framework.Tests/Visual/UserInterface/TestSceneRearrangeableListContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}", () =>
Expand Down Expand Up @@ -425,5 +501,32 @@ private void load()
}
}
}

private partial class TestLoadCountingList : BasicRearrangeableListContainer<int>
{
/// <summary>
/// Dictionary of item -> # of times a drawable was loaded for it.
/// </summary>
public readonly Dictionary<int, int> LoadCount = new Dictionary<int, int>();

public new IReadOnlyDictionary<int, RearrangeableListItem<int>> ItemMap => base.ItemMap;

protected override BasicRearrangeableListItem<int> CreateBasicItem(int item)
=> new TestRearrangeableListItem(item, () => LoadCount[item] = LoadCount.GetValueOrDefault(item) + 1);

private partial class TestRearrangeableListItem : BasicRearrangeableListItem<int>
{
private readonly Action onLoad;

public TestRearrangeableListItem(int item, Action onLoad)
: base(item, false)
{
this.onLoad = onLoad;
}

[BackgroundDependencyLoader]
private void load() => onLoad();
}
}
}
}
36 changes: 19 additions & 17 deletions osu.Framework/Graphics/Containers/RearrangeableListContainer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. 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;
Expand All @@ -22,6 +19,7 @@ namespace osu.Framework.Graphics.Containers
/// </remarks>
/// <typeparam name="TModel">The type of rearrangeable item.</typeparam>
public abstract partial class RearrangeableListContainer<TModel> : CompositeDrawable
where TModel : notnull
{
private const float exp_base = 1.05f;

Expand Down Expand Up @@ -51,7 +49,7 @@ public abstract partial class RearrangeableListContainer<TModel> : CompositeDraw
protected IReadOnlyDictionary<TModel, RearrangeableListItem<TModel>> ItemMap => itemMap;

private readonly Dictionary<TModel, RearrangeableListItem<TModel>> itemMap = new Dictionary<TModel, RearrangeableListItem<TModel>>();
private RearrangeableListItem<TModel> currentlyDraggedItem;
private RearrangeableListItem<TModel>? currentlyDraggedItem;
private Vector2 screenSpaceDragPosition;

/// <summary>
Expand Down Expand Up @@ -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<TModel>() ?? []);
break;

case NotifyCollectionChangedAction.Remove:
removeItems(e.OldItems);
removeItems(e.OldItems?.Cast<TModel>() ?? []);

// Explicitly reset scroll position here so that ScrollContainer doesn't retain our
// scroll position if we quickly add new items after calling a Clear().
Expand All @@ -107,8 +105,11 @@ private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e
break;

case NotifyCollectionChangedAction.Replace:
removeItems(e.OldItems);
addItems(e.NewItems);
IEnumerable<TModel> tOldItems = e.OldItems?.Cast<TModel>() ?? [];
IEnumerable<TModel> tNewItems = e.NewItems?.Cast<TModel>() ?? [];

removeItems(tOldItems.Except(tNewItems));
addItems(tNewItems.Except(tOldItems));
break;

case NotifyCollectionChangedAction.Move:
Expand All @@ -118,9 +119,9 @@ private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e
}
}

private void removeItems(IList items)
private void removeItems(IEnumerable<TModel> items)
{
foreach (var item in items.Cast<TModel>())
foreach (var item in items)
{
if (currentlyDraggedItem != null && EqualityComparer<TModel>.Default.Equals(currentlyDraggedItem.Model, item))
currentlyDraggedItem = null;
Expand All @@ -137,11 +138,11 @@ private void removeItems(IList items)
OnItemsChanged();
}

private void addItems(IList items)
private void addItems(IEnumerable<TModel> items)
{
var drawablesToAdd = new List<Drawable>();

foreach (var item in items.Cast<TModel>())
foreach (var item in items)
{
if (itemMap.ContainsKey(item))
{
Expand Down Expand Up @@ -191,7 +192,7 @@ private void sortItems()
if (drawable.Parent != ListContainer)
continue;

ListContainer!.SetLayoutPosition(drawable, i);
ListContainer.SetLayoutPosition(drawable, i);
}
}

Expand All @@ -216,9 +217,7 @@ protected override void Update()
protected override void UpdateAfterChildren()
{
base.UpdateAfterChildren();

if (currentlyDraggedItem != null)
updateArrangement();
updateArrangement();
}

private void updateScrollPosition()
Expand All @@ -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);

Expand Down

0 comments on commit dc98996

Please sign in to comment.