Merge pull request #24 from CoderGamester/develop

CoderGamester · web-flow · commit ee0c43ea82da · 2024-11-15T17:58:36.000Z
Release 0.14.0
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,13 +4,16 @@ 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.13.2] - 2024-11-13
+## [0.14.0] - 2024-11-15
 
 **New**:
-- Added a constructor to *GameObjectPool* that allows to setup a costum instantiator
+- Added *PublishSafe* method to *IMessageBrokerService* to allow publishing messages safely in chase of chain subscriptions during publishing of a message
+
+**Changed**:
+- *Subscribe* and *Unsubscribe* throw an *InvalidOperationException* when being executed during a message being published
 
 **Fixed**:
-- Fixed *ObjectPool* & *PoolService* tests that would block builds sometimes
+- CoroutineTests issues running when building released projects
 
 ## [0.13.1] - 2024-11-04
 
diff --git a/Runtime/MessageBrokerService.cs b/Runtime/MessageBrokerService.cs
@@ -25,23 +25,32 @@ public interface IMessageBrokerService
 		/// Publish a message in the message broker.
 		/// If there is no object subscribing the message type, nothing will happen
 		/// </summary>
+		/// <remarks>
+		/// Use <see cref="PublishSafe{T}(T)"/> it there are a chain subscriptions during publishing
+		/// </remarks>
 		void Publish<T>(T message) where T : IMessage;
-		
+
+		/// <summary>
+		/// Publish a message in the message broker.
+		/// If there is no object subscribing the message type, nothing will happen
+		/// </summary>
+		/// <remarks>
+		/// This method can be slow and allocated extra memory if there are a lot of subscribers to the <typeparamref name="T"/>.
+		/// Use <see cref="Publish{T}(T)"/> instead for faster iteration speed IF and ONLY IF there aren't chain subscriptions during publishing
+		/// </remarks>
+		void PublishSafe<T>(T message) where T : IMessage;
+
 		/// <summary>
 		/// Subscribes to the message type.
 		/// Will invoke the <paramref name="action"/> every time the message of the subscribed type is published.
 		/// </summary>
 		void Subscribe<T>(Action<T> action) where T : IMessage;
-		
-		/// <summary>
-		/// Unsubscribe the <paramref name="action"/> from the message broker.
-		/// </summary>
-		void Unsubscribe<T>(Action<T> action) where T : IMessage;
-		
+
 		/// <summary>
-		/// Unsubscribe all actions from the message broker from of the given message type.
+		/// Unsubscribe the action of <typeparamref name="T"/> from the <paramref name="subscriber"/> in the message broker.
+		/// If <paramref name="subscriber"/> is null then will unsubscribe from ALL subscribers currently subscribed to <typeparamref name="T"/>
 		/// </summary>
-		void Unsubscribe<T>() where T : IMessage;
+		void Unsubscribe<T>(object subscriber = null) where T : IMessage;
 		
 		/// <summary>
 		/// Unsubscribe from all messages.
@@ -53,7 +62,9 @@ public interface IMessageBrokerService
 	/// <inheritdoc />
 	public class MessageBrokerService : IMessageBrokerService
 	{
-		private readonly IDictionary<Type, IDictionary<object, IList>> _subscriptions = new Dictionary<Type, IDictionary<object, IList>>();
+		private readonly IDictionary<Type, IDictionary<object, Delegate>> _subscriptions = new Dictionary<Type, IDictionary<object, Delegate>>();
+
+		private (bool, IMessage) _isPublishing;
 
 		/// <inheritdoc />
 		public void Publish<T>(T message) where T : IMessage
@@ -63,18 +74,35 @@ public void Publish<T>(T message) where T : IMessage
 				return;
 			}
 
-			var subscriptionCopy = new IList[subscriptionObjects.Count];
-			
+			_isPublishing = (true, message);
+
+			foreach (var subscription in subscriptionObjects)
+			{
+				var action = (Action<T>)subscription.Value;
+
+				action(message);
+			}
+
+			_isPublishing = (false, message);
+		}
+
+		/// <inheritdoc />
+		public void PublishSafe<T>(T message) where T : IMessage
+		{
+			if (!_subscriptions.TryGetValue(typeof(T), out var subscriptionObjects))
+			{
+				return;
+			}
+
+			var subscriptionCopy = new Delegate[subscriptionObjects.Count];
+
 			subscriptionObjects.Values.CopyTo(subscriptionCopy, 0);
 
 			for (var i = 0; i < subscriptionCopy.Length; i++)
 			{
-				var actions = (List<Action<T>>) subscriptionCopy[i];
+				var action = (Action<T>)subscriptionCopy[i];
 
-				for (var index = 0; index < actions.Count; index++)
-				{
-					actions[index](message);
-				}
+				action(message);
 			}
 		}
 
@@ -88,58 +116,51 @@ public void Subscribe<T>(Action<T> action) where T : IMessage
 			{
 				throw new ArgumentException("Subscribe static functions to a message is not supported!");
 			}
-
-			if (!_subscriptions.TryGetValue(type, out var subscriptionObjects))
+			if(_isPublishing.Item1)
 			{
-				subscriptionObjects = new Dictionary<object, IList>();
-				_subscriptions.Add(type, subscriptionObjects);
+				throw new InvalidOperationException($"Cannot subscribe to {type.Name} message while publishing " +
+					$"{_isPublishing.Item2.GetType().Name} message. Use {nameof(PublishSafe)} instead!");
 			}
 
-			if (!subscriptionObjects.TryGetValue(subscriber, out IList actions))
+			if (!_subscriptions.TryGetValue(type, out var subscriptionObjects))
 			{
-				actions = new List<Action<T>>();
-				subscriptionObjects.Add(subscriber, actions);
+				subscriptionObjects = new Dictionary<object, Delegate>();
+				_subscriptions.Add(type, subscriptionObjects);
 			}
 
-			actions.Add(action);
+			subscriptionObjects[subscriber] = action;
 		}
 
 		/// <inheritdoc />
-		public void Unsubscribe<T>(Action<T> action) where T : IMessage
+		public void Unsubscribe<T>(object subscriber = null) where T : IMessage
 		{
 			var type = typeof(T);
-			var subscriber = action.Target;
 
 			if (subscriber == null)
 			{
-				throw new ArgumentException("Subscribe static functions to a message is not supported!");
-			}
+				_subscriptions.Remove(type);
 
-			if (!_subscriptions.TryGetValue(type, out var subscriptionObjects) || 
-			    !subscriptionObjects.TryGetValue(subscriber, out var actions))
-			{
 				return;
 			}
 
-			actions.Remove(action);
-
-			if (actions.Count == 0)
+			if (_isPublishing.Item1)
 			{
-				subscriptionObjects.Remove(subscriber);
+				throw new InvalidOperationException($"Cannot unsubscribe to {type.Name} message while publishing " +
+					$"{_isPublishing.Item2.GetType().Name} message. Use {nameof(PublishSafe)} instead!");
 			}
+			if (!_subscriptions.TryGetValue(type, out var subscriptionObjects))
+			{
+				return;
+			}
+
+			subscriptionObjects.Remove(subscriber);
 
 			if (subscriptionObjects.Count == 0)
 			{
 				_subscriptions.Remove(type);
 			}
 		}
 
-		/// <inheritdoc />
-		public void Unsubscribe<T>() where T : IMessage
-		{
-			_subscriptions.Remove(typeof(T));
-		}
-
 		/// <inheritdoc />
 		public void UnsubscribeAll(object subscriber = null)
 		{
@@ -149,12 +170,14 @@ public void UnsubscribeAll(object subscriber = null)
 				return;
 			}
 
+			if (_isPublishing.Item1)
+			{
+				throw new InvalidOperationException($"Cannot unsubscribe from {subscriber} message while publishing " +
+					$"{_isPublishing.Item2.GetType().Name} message. Use {nameof(PublishSafe)} instead!");
+			}
 			foreach (var subscriptionObjects in _subscriptions.Values)
 			{
-				if (subscriptionObjects.ContainsKey(subscriber))
-				{
-					subscriptionObjects.Remove(subscriber);
-				}
+				subscriptionObjects.Remove(subscriber);
 			}
 		}
 	}
diff --git a/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef b/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef
diff --git a/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef.meta b/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef.meta
diff --git a/Tests/Editor/EditMode/InstallerTest.cs b/Tests/Editor/EditMode/InstallerTest.cs
@@ -1,9 +1,10 @@
 using System;
+using GameLovers.Services;
 using NUnit.Framework;
 
 // ReSharper disable once CheckNamespace
 
-namespace GameLovers.Services.Tests
+namespace GameLoversEditor.Services.Tests
 {
 	public class InstallerTest
 	{
diff --git a/Tests/Editor/EditMode/MessageBrokerServiceTest.cs b/Tests/Editor/EditMode/MessageBrokerServiceTest.cs
@@ -38,14 +38,35 @@ public void Subscribe_Publish_Successfully()
 		{
 			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall);
 			_messageBroker.Publish(_messageType1);
+			_messageBroker.PublishSafe(_messageType1);
 
-			_subscriber.Received().MockMessageCall(_messageType1);
+			_subscriber.Received(2).MockMessageCall(_messageType1);
+		}
+
+		[Test]
+		public void Subscribe_MultipleSubscriptionSameType_ReplacePreviousSubscription()
+		{
+			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall);
+			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall2);
+			_messageBroker.Publish(_messageType1);
+			_messageBroker.PublishSafe(_messageType1);
+
+			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
+			_subscriber.Received(2).MockMessageCall2(_messageType1);
+		}
+
+		[Test]
+		public void Publish_ChainSubscribe_Successfully()
+		{
+			// TODO: Test
+			Assert.True(true);
 		}
 
 		[Test]
 		public void Publish_WithoutSubscription_DoesNothing()
 		{
 			_messageBroker.Publish(_messageType1);
+			_messageBroker.PublishSafe(_messageType1);
 
 			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
 		}
@@ -54,22 +75,24 @@ public void Publish_WithoutSubscription_DoesNothing()
 		public void Unsubscribe_Successfully()
 		{
 			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall);
-			_messageBroker.Unsubscribe<MessageType1>(_subscriber.MockMessageCall);
+			_messageBroker.Unsubscribe<MessageType1>(_subscriber);
 			_messageBroker.Publish(_messageType1);
+			_messageBroker.PublishSafe(_messageType1);
 
 			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
 		}
 
 		[Test]
-		public void UnsubscribeWithAction_KeepsSubscriptionSameType_Successfully()
+		public void UnsubscribeWithAction_MultipleSubscriptionSameType_RemoveAllScriptionsOfSameType()
 		{
 			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall);
 			_messageBroker.Subscribe<MessageType1>(_subscriber.MockMessageCall2);
-			_messageBroker.Unsubscribe<MessageType1>(_subscriber.MockMessageCall);
+			_messageBroker.Unsubscribe<MessageType1>(_subscriber);
 			_messageBroker.Publish(_messageType1);
+			_messageBroker.PublishSafe(_messageType1);
 
 			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
-			_subscriber.Received().MockMessageCall2(_messageType1);
+			_subscriber.DidNotReceive().MockMessageCall2(_messageType1);
 		}
 
 		[Test]
@@ -79,9 +102,10 @@ public void UnsubscribeWithoutAction_KeepsSubscriptionDifferentType_Successfully
 			_messageBroker.Subscribe<MessageType2>(_subscriber.MockMessageAlternativeCall);
 			_messageBroker.Unsubscribe<MessageType1>();
 			_messageBroker.Publish(_messageType2);
+			_messageBroker.PublishSafe(_messageType2);
 
 			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
-			_subscriber.Received().MockMessageAlternativeCall(_messageType2);
+			_subscriber.Received(2).MockMessageAlternativeCall(_messageType2);
 		}
 
 		[Test]
@@ -92,8 +116,10 @@ public void UnsubscribeAll_Successfully()
 			_messageBroker.Subscribe<MessageType2>(_subscriber.MockMessageAlternativeCall);
 			_messageBroker.Subscribe<MessageType2>(_subscriber.MockMessageAlternativeCall2);
 			_messageBroker.UnsubscribeAll();
+			_messageBroker.Publish(_messageType1);
 			_messageBroker.Publish(_messageType2);
-			_messageBroker.Publish(_messageType2);
+			_messageBroker.PublishSafe(_messageType1);
+			_messageBroker.PublishSafe(_messageType2);
 
 			_subscriber.DidNotReceive().MockMessageCall(_messageType1);
 			_subscriber.DidNotReceive().MockMessageCall2(_messageType1);
@@ -104,7 +130,7 @@ public void UnsubscribeAll_Successfully()
 		[Test]
 		public void Unsubscribe_WithoutSubscription_DoesNothing()
 		{
-			Assert.DoesNotThrow(() => _messageBroker.Unsubscribe<MessageType1>(_subscriber.MockMessageCall));
+			Assert.DoesNotThrow(() => _messageBroker.Unsubscribe<MessageType1>(_subscriber));
 			Assert.DoesNotThrow(() => _messageBroker.Unsubscribe<MessageType1>());
 			Assert.DoesNotThrow(() => _messageBroker.UnsubscribeAll());
 		}
diff --git a/Tests/Editor/PlayMode/GameLovers.Services.Tests.Playmode.asmdef b/Tests/Editor/PlayMode/GameLovers.Services.Tests.Playmode.asmdef
@@ -2,8 +2,9 @@
     "name": "GameLovers.Services.Tests.Playmode",
     "rootNamespace": "",
     "references": [
-        "GUID:27619889b8ba8c24980f49ee34dbb44a",
-        "GUID:15a85301a6cee40849303ad50f7a0322"
+        "UnityEngine.TestRunner",
+        "UnityEditor.TestRunner",
+        "GameLovers.Services"
     ],
     "includePlatforms": [],
     "excludePlatforms": [],
@@ -13,7 +14,9 @@
         "nunit.framework.dll"
     ],
     "autoReferenced": true,
-    "defineConstraints": [],
+    "defineConstraints": [
+        "UNITY_INCLUDE_TESTS"
+    ],
     "versionDefines": [],
     "noEngineReferences": false
 }
diff --git a/package.json b/package.json
@@ -2,7 +2,7 @@
   "name": "com.gamelovers.services",
   "displayName": "Services",
   "author": "Miguel Tomas",
-  "version": "0.13.2",
+  "version": "0.14.0",
   "unity": "2022.3",
   "license": "MIT",
   "description": "The purpose of this package is to provide a set of services to ease the development of a basic game architecture",

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,10 @@`
`1`	`1`	`using System;`
	`2`	`+using GameLovers.Services;`
`2`	`3`	`using NUnit.Framework;`
`3`	`4`
`4`	`5`	`// ReSharper disable once CheckNamespace`
`5`	`6`
`6`		`-namespace GameLovers.Services.Tests`
	`7`	`+namespace GameLoversEditor.Services.Tests`
`7`	`8`	`{`
`8`	`9`	`public class InstallerTest`
`9`	`10`	`{`