Skip to content

Commit

Permalink
Merge pull request #6466 from smoogipoo/fix-bindablelist-cast
Browse files Browse the repository at this point in the history
Fix `BindableList` methods crashing with non-`IList` collections
  • Loading branch information
bdach authored Dec 19, 2024
2 parents 17ad069 + 3ac37de commit beb2fa2
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
22 changes: 22 additions & 0 deletions osu.Framework.Tests/Bindables/BindableListTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,17 @@ public void TestAddRangeBranchingBinds()
Assert.That(b4.Count, Is.EqualTo(7));
}

[Test]
public void TestAddHashSetWithCallback()
{
var b1 = new BindableList<int>();

b1.CollectionChanged += (_, _) => { };
b1.AddRange(new HashSet<int>([1, 2, 3, 4]));

Assert.That(b1.Count, Is.EqualTo(4));
}

#endregion

#region .Move(item)
Expand Down Expand Up @@ -1193,6 +1204,17 @@ public void TestReplaceRangeNotifiesBoundLists()
Assert.That(triggeredArgs.OldStartingIndex, Is.EqualTo(0));
}

[Test]
public void TestReplaceRangeHashSetWithCallback()
{
var b1 = new BindableList<int>([1]);

b1.CollectionChanged += (_, _) => { };
b1.ReplaceRange(0, 1, new HashSet<int>([1, 2, 3, 4]));

Assert.That(b1.Count, Is.EqualTo(4));
}

#endregion

#region .Clear()
Expand Down
16 changes: 8 additions & 8 deletions osu.Framework/Bindables/BindableList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ private int removeAll(Predicate<T> match, HashSet<BindableList<T>> appliedInstan
/// <param name="count">The count of items to be removed.</param>
/// <param name="newItems">The items to replace the removed items with.</param>
public void ReplaceRange(int index, int count, IEnumerable<T> newItems)
=> replaceRange(index, count, newItems as ICollection<T> ?? newItems.ToArray(), new HashSet<BindableList<T>>());
=> replaceRange(index, count, newItems as IList ?? newItems.ToArray(), new HashSet<BindableList<T>>());

private void replaceRange(int index, int count, ICollection<T> newItems, HashSet<BindableList<T>> appliedInstances)
private void replaceRange(int index, int count, IList newItems, HashSet<BindableList<T>> appliedInstances)
{
if (checkAlreadyApplied(appliedInstances)) return;

Expand All @@ -335,7 +335,7 @@ private void replaceRange(int index, int count, ICollection<T> newItems, HashSet
List<T> removedItems = CollectionChanged == null ? null : collection.GetRange(index, count);

collection.RemoveRange(index, count);
collection.InsertRange(index, newItems);
collection.InsertRange(index, (IEnumerable<T>)newItems);

if (bindings != null)
{
Expand All @@ -347,7 +347,7 @@ private void replaceRange(int index, int count, ICollection<T> newItems, HashSet
}
}

CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, (IList)newItems, removedItems!, index));
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, newItems, removedItems!, index));
}

/// <summary>
Expand Down Expand Up @@ -544,23 +544,23 @@ public virtual void UnbindFrom(IUnbindable them)
/// <param name="items">The collection whose items should be added to this collection.</param>
/// <exception cref="InvalidOperationException">Thrown if this collection is <see cref="Disabled"/></exception>
public void AddRange(IEnumerable<T> items)
=> addRange(items as ICollection<T> ?? items.ToArray(), new HashSet<BindableList<T>>());
=> addRange(items as IList ?? items.ToArray(), new HashSet<BindableList<T>>());

private void addRange(ICollection<T> items, HashSet<BindableList<T>> appliedInstances)
private void addRange(IList items, HashSet<BindableList<T>> appliedInstances)
{
if (checkAlreadyApplied(appliedInstances)) return;

ensureMutationAllowed();

collection.AddRange(items);
collection.AddRange((IEnumerable<T>)items);

if (bindings != null)
{
foreach (var b in bindings)
b.addRange(items, appliedInstances);
}

CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, (IList)items, collection.Count - items.Count));
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, items, collection.Count - items.Count));
}

/// <summary>
Expand Down

0 comments on commit beb2fa2

Please sign in to comment.