Skip to content

Commit c3e80ba

Browse files
committedNov 20, 2024·
PR fix for cases where unsubscriptions could happen in different places on the order list
1 parent 206ab27 commit c3e80ba

File tree

4 files changed

+62
-12
lines changed

4 files changed

+62
-12
lines changed
 

‎Runtime/ObservableDictionary.cs

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using System.Collections.Generic;
44
using System.Collections.ObjectModel;
55
using System.Linq;
6-
using UnityEngine.UIElements;
76

87
// ReSharper disable once CheckNamespace
98

@@ -276,18 +275,26 @@ public virtual bool Remove(TKey key)
276275

277276
if (ObservableUpdateFlag != ObservableUpdateFlag.UpdateOnly && _keyUpdateActions.TryGetValue(key, out var actions))
278277
{
279-
var listCopy = actions.ToList();
280-
for (var i = 0; i < listCopy.Count; i++)
278+
for (var i = actions.Count - 1; i > -1; i--)
281279
{
282-
listCopy[i](key, value, default, ObservableUpdateType.Removed);
280+
var action = actions[i];
281+
282+
action(key, value, default, ObservableUpdateType.Removed);
283+
284+
// Shift the index if an action was unsubscribed
285+
i = AdjustIndex(i, action, actions);
283286
}
284287
}
285288
if (ObservableUpdateFlag != ObservableUpdateFlag.KeyUpdateOnly)
286289
{
287-
var listCopy = _updateActions.ToList();
288-
for (var i = 0; i < listCopy.Count; i++)
290+
for (var i = _updateActions.Count - 1; i > -1; i--)
289291
{
290-
listCopy[i](key, value, default, ObservableUpdateType.Removed);
292+
var action = _updateActions[i];
293+
294+
action(key, value, default, ObservableUpdateType.Removed);
295+
296+
// Shift the index if an action was unsubscribed
297+
i = AdjustIndex(i, action, _updateActions);
291298
}
292299
}
293300

@@ -442,6 +449,25 @@ protected void InvokeUpdate(TKey key, TValue previousValue)
442449
}
443450
}
444451
}
452+
453+
private int AdjustIndex(int index, Action<TKey, TValue, TValue, ObservableUpdateType> action,
454+
IList<Action<TKey, TValue, TValue, ObservableUpdateType>> list)
455+
{
456+
if (index < list.Count && list[index] == action)
457+
{
458+
return index;
459+
}
460+
461+
for (var i = index - 1; i > -1; i--)
462+
{
463+
if (list[index] == action)
464+
{
465+
return i;
466+
}
467+
}
468+
469+
return index + 1;
470+
}
445471
}
446472

447473
/// <inheritdoc />

‎Runtime/ObservableList.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,14 @@ public virtual void RemoveAt(int index)
232232

233233
List.RemoveAt(index);
234234

235-
for (var i = 0; i < _updateActions.Count; i++)
235+
for (var i = _updateActions.Count - 1; i > -1; i--)
236236
{
237-
_updateActions[i](index, data, default, ObservableUpdateType.Removed);
237+
var action = _updateActions[i];
238+
239+
action(index, data, default, ObservableUpdateType.Removed);
240+
241+
// Shift the index if an action was unsubscribed
242+
i = AdjustIndex(i, action);
238243
}
239244
}
240245

@@ -307,6 +312,24 @@ protected void InvokeUpdate(int index, T previousValue)
307312
_updateActions[i](index, previousValue, data, ObservableUpdateType.Updated);
308313
}
309314
}
315+
316+
private int AdjustIndex(int index, Action<int, T, T, ObservableUpdateType> action)
317+
{
318+
if (index < _updateActions.Count && _updateActions[index] == action)
319+
{
320+
return index;
321+
}
322+
323+
for (var i = index - 1; i > -1; i--)
324+
{
325+
if (_updateActions[index] == action)
326+
{
327+
return i;
328+
}
329+
}
330+
331+
return index + 1;
332+
}
310333
}
311334

312335
/// <inheritdoc />

‎Tests/Editor/ObservableDictionaryTest.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using GameLovers;
44
using NSubstitute;
55
using NUnit.Framework;
6-
using UnityEngine;
76

87
// ReSharper disable once CheckNamespace
98

‎Tests/Editor/ObservableListTest.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public void StopObserveCheck()
126126
}
127127

128128
[Test]
129-
public void StopObserve_MultipleCalls_StopsOnlyOne()
129+
public void StopObserve_WhenCalledOnce_RemovesOnlyOneObserverInstance()
130130
{
131131
_list.Observe(_caller.Call);
132132
_list.Observe(_caller.Call);
@@ -137,7 +137,9 @@ public void StopObserve_MultipleCalls_StopsOnlyOne()
137137

138138
_list.RemoveAt(_index);
139139

140-
_caller.Received().Call(Arg.Any<int>(), Arg.Any<int>(), Arg.Any<int>(), Arg.Any<ObservableUpdateType>());
140+
_caller.Received(1).Call(Arg.Any<int>(), Arg.Is(0), Arg.Is(_previousValue), ObservableUpdateType.Added);
141+
_caller.Received(1).Call(_index, _previousValue, _previousValue, ObservableUpdateType.Updated);
142+
_caller.Received(1).Call(_index, _previousValue, 0, ObservableUpdateType.Removed);
141143
}
142144

143145
[Test]

0 commit comments

Comments
 (0)
Please sign in to comment.