Merge pull request #12 from CoderGamester/develop

Release 0.6.5

Author: CoderGamester

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**:

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)
 		/// <inheritdoc />
 		public virtual void Clear()
 		{
-			var dictionary = new Dictionary<TKey, TValue>(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<TKey, IList<Action<TKey, TValue, TValue, ObservableUpdateType>>>(_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();
 		}
 
 		/// <inheritdoc />
@@ -371,6 +384,7 @@ public void StopObserving(Action<TKey, TValue, TValue, ObservableUpdateType> onU
 					if (actions.Value[i] == onUpdate)
 					{
 						actions.Value.RemoveAt(i);
+						break;
 					}
 				}
 			}
@@ -380,6 +394,7 @@ public void StopObserving(Action<TKey, TValue, TValue, ObservableUpdateType> 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<TKey, TValue, TValue, ObservableUpdateType> action,
+			IList<Action<TKey, TValue, TValue, ObservableUpdateType>> 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;
+		}
 	}
 
 	/// <inheritdoc />

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);
 			}
 		}
 
 		/// <inheritdoc />
 		public virtual void Clear()
 		{
-			var list = new List<T>(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();
 		}
 
 		/// <inheritdoc />
@@ -306,6 +312,24 @@ protected void InvokeUpdate(int index, T previousValue)
 				_updateActions[i](index, previousValue, data, ObservableUpdateType.Updated);
 			}
 		}
+
+		private int AdjustIndex(int index, Action<int, T, T, ObservableUpdateType> 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;
+		}
 	}
 
 	/// <inheritdoc />

Tests/Editor/ObservableDictionaryTest.cs

@@ -3,7 +3,6 @@
 using GameLovers;
 using NSubstitute;
 using NUnit.Framework;
-using UnityEngine;
 
 // ReSharper disable once CheckNamespace
 

Tests/Editor/ObservableListTest.cs

@@ -125,6 +125,23 @@ public void StopObserveCheck()
 			_caller.DidNotReceive().Call(Arg.Any<int>(), Arg.Any<int>(), Arg.Any<int>(), Arg.Any<ObservableUpdateType>());
 		}
 
+		[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<int>(), 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);

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",