diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a8ad5a..9c85b70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this package will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [0.6.5] - 2024-11-20 + +**Fix**: +- Fixed the issues of *ObservableDictionary* when subscribing/unsubscribing to actions while removing/adding elements +- Fixed the issues of *ObservableList* when subscribing/unsubscribing to actions while removing/adding elements + ## [0.6.4] - 2024-11-13 **Fix**: diff --git a/Runtime/ObservableDictionary.cs b/Runtime/ObservableDictionary.cs index 5f44ce0..e3ce5fb 100644 --- a/Runtime/ObservableDictionary.cs +++ b/Runtime/ObservableDictionary.cs @@ -2,7 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; -using UnityEngine.UIElements; +using System.Linq; // ReSharper disable once CheckNamespace @@ -275,16 +275,26 @@ public virtual bool Remove(TKey key) if (ObservableUpdateFlag != ObservableUpdateFlag.UpdateOnly && _keyUpdateActions.TryGetValue(key, out var actions)) { - for (var i = 0; i < actions.Count; i++) + for (var i = actions.Count - 1; i > -1; i--) { - actions[i](key, value, default, ObservableUpdateType.Removed); + var action = actions[i]; + + action(key, value, default, ObservableUpdateType.Removed); + + // Shift the index if an action was unsubscribed + i = AdjustIndex(i, action, actions); } } if (ObservableUpdateFlag != ObservableUpdateFlag.KeyUpdateOnly) { - for (var i = 0; i < _updateActions.Count; i++) + for (var i = _updateActions.Count - 1; i > -1; i--) { - _updateActions[i](key, value, default, ObservableUpdateType.Removed); + var action = _updateActions[i]; + + action(key, value, default, ObservableUpdateType.Removed); + + // Shift the index if an action was unsubscribed + i = AdjustIndex(i, action, _updateActions); } } @@ -294,31 +304,34 @@ public virtual bool Remove(TKey key) /// public virtual void Clear() { - var dictionary = new Dictionary(Dictionary); - - Dictionary.Clear(); - if (ObservableUpdateFlag != ObservableUpdateFlag.UpdateOnly) { - foreach (var data in _keyUpdateActions) + // Create a copy in case that one of the callbacks modifies the list (Ex: removing a subscriber) + var copy = new Dictionary>>(_keyUpdateActions); + + foreach (var data in copy) { - for (var i = 0; i < data.Value.Count; i++) + var listCopy = data.Value.ToList(); + for (var i = 0; i < listCopy.Count; i++) { - data.Value[i](data.Key, dictionary[data.Key], default, ObservableUpdateType.Removed); + listCopy[i](data.Key, Dictionary[data.Key], default, ObservableUpdateType.Removed); } } } if (ObservableUpdateFlag != ObservableUpdateFlag.KeyUpdateOnly) { - foreach (var data in dictionary) + foreach (var data in Dictionary) { - for (var i = 0; i < _updateActions.Count; i++) + var listCopy = _updateActions.ToList(); + for (var i = 0; i < listCopy.Count; i++) { - _updateActions[i](data.Key, data.Value, default, ObservableUpdateType.Removed); + listCopy[i](data.Key, data.Value, default, ObservableUpdateType.Removed); } } } + + Dictionary.Clear(); } /// @@ -371,6 +384,7 @@ public void StopObserving(Action onU if (actions.Value[i] == onUpdate) { actions.Value.RemoveAt(i); + break; } } } @@ -380,6 +394,7 @@ public void StopObserving(Action onU if (_updateActions[i] == onUpdate) { _updateActions.RemoveAt(i); + break; } } } @@ -434,6 +449,25 @@ protected void InvokeUpdate(TKey key, TValue previousValue) } } } + + private int AdjustIndex(int index, Action action, + IList> list) + { + if (index < list.Count && list[index] == action) + { + return index; + } + + for (var i = index - 1; i > -1; i--) + { + if (list[i] == action) + { + return i; + } + } + + return index + 1; + } } /// diff --git a/Runtime/ObservableList.cs b/Runtime/ObservableList.cs index 354278a..954fc7b 100644 --- a/Runtime/ObservableList.cs +++ b/Runtime/ObservableList.cs @@ -232,26 +232,32 @@ public virtual void RemoveAt(int index) List.RemoveAt(index); - for (var i = 0; i < _updateActions.Count; i++) + for (var i = _updateActions.Count - 1; i > -1; i--) { - _updateActions[i](index, data, default, ObservableUpdateType.Removed); + var action = _updateActions[i]; + + action(index, data, default, ObservableUpdateType.Removed); + + // Shift the index if an action was unsubscribed + i = AdjustIndex(i, action); } } /// public virtual void Clear() { - var list = new List(List); - - List.Clear(); + // Create a copy in case that one of the callbacks modifies the list (Ex: removing a subscriber) + var copy = _updateActions.ToList(); - for (var i = 0; i < _updateActions.Count; i++) + for (var i = copy.Count - 1; i > -1; i--) { - for (var j = 0; j < list.Count; j++) + for (var j = 0; j < List.Count; j++) { - _updateActions[i](j, list[j], default, ObservableUpdateType.Removed); + copy[i](j, List[j], default, ObservableUpdateType.Removed); } } + + List.Clear(); } /// @@ -306,6 +312,24 @@ protected void InvokeUpdate(int index, T previousValue) _updateActions[i](index, previousValue, data, ObservableUpdateType.Updated); } } + + private int AdjustIndex(int index, Action action) + { + if (index < _updateActions.Count && _updateActions[index] == action) + { + return index; + } + + for (var i = index - 1; i > -1; i--) + { + if (_updateActions[i] == action) + { + return i; + } + } + + return index + 1; + } } /// diff --git a/Tests/Editor/ObservableDictionaryTest.cs b/Tests/Editor/ObservableDictionaryTest.cs index 02f51b1..b978a1a 100644 --- a/Tests/Editor/ObservableDictionaryTest.cs +++ b/Tests/Editor/ObservableDictionaryTest.cs @@ -3,7 +3,6 @@ using GameLovers; using NSubstitute; using NUnit.Framework; -using UnityEngine; // ReSharper disable once CheckNamespace diff --git a/Tests/Editor/ObservableListTest.cs b/Tests/Editor/ObservableListTest.cs index be9b939..b5b9df7 100644 --- a/Tests/Editor/ObservableListTest.cs +++ b/Tests/Editor/ObservableListTest.cs @@ -125,6 +125,23 @@ public void StopObserveCheck() _caller.DidNotReceive().Call(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } + [Test] + public void StopObserve_WhenCalledOnce_RemovesOnlyOneObserverInstance() + { + _list.Observe(_caller.Call); + _list.Observe(_caller.Call); + _list.StopObserving(_caller.Call); + _list.Add(_previousValue); + + _list[_index] = _previousValue; + + _list.RemoveAt(_index); + + _caller.Received(1).Call(Arg.Any(), Arg.Is(0), Arg.Is(_previousValue), ObservableUpdateType.Added); + _caller.Received(1).Call(_index, _previousValue, _previousValue, ObservableUpdateType.Updated); + _caller.Received(1).Call(_index, _previousValue, 0, ObservableUpdateType.Removed); + } + [Test] public void StopObservingAllCheck() { @@ -137,7 +154,7 @@ public void StopObservingAllCheck() } [Test] - public void StopObservingAll_MultipleCalls_Check() + public void StopObservingAll_MultipleCalls_StopsAll() { _list.Observe(_caller.Call); _list.Observe(_caller.Call); diff --git a/package.json b/package.json index ba47160..0740b40 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "com.gamelovers.dataextensions", "displayName": "Unity Data Type Extensions", "author": "Miguel Tomas", - "version": "0.6.4", + "version": "0.6.5", "unity": "2022.3", "license": "MIT", "description": "This package extends various sets of data types to be used in any type of data containers or persistent serializable data",