From 63af7b0dc160e268c7135cc049d0e9c2abe02bf4 Mon Sep 17 00:00:00 2001 From: CoderGamester Date: Fri, 15 Nov 2024 17:40:02 +0000 Subject: [PATCH 1/2] **New**: - 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**: - CoroutineTests issues running when building released projects --- CHANGELOG.md | 9 +- Runtime/MessageBrokerService.cs | 110 ++++++++++-------- .../GameLovers.Services.Tests.asmdef | 0 .../GameLovers.Services.Tests.asmdef.meta | 0 Tests/Editor/EditMode/InstallerTest.cs | 3 +- .../EditMode/MessageBrokerServiceTest.cs | 42 +++++-- .../GameLovers.Services.Tests.Playmode.asmdef | 9 +- package.json | 2 +- 8 files changed, 113 insertions(+), 62 deletions(-) rename Tests/Editor/{ => EditMode}/GameLovers.Services.Tests.asmdef (100%) rename Tests/Editor/{ => EditMode}/GameLovers.Services.Tests.asmdef.meta (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25ef062..c1d8e15 100644 --- 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 index 68ce101..3006f5e 100644 --- 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 /// + /// + /// Use it there are a chain subscriptions during publishing + /// void Publish(T message) where T : IMessage; - + + /// + /// Publish a message in the message broker. + /// If there is no object subscribing the message type, nothing will happen + /// + /// + /// This method can be slow and allocated extra memory if there are a lot of subscribers to the . + /// Use instead for faster iteration speed IF and ONLY IF there aren't chain subscriptions during publishing + /// + void PublishSafe(T message) where T : IMessage; + /// /// Subscribes to the message type. /// Will invoke the every time the message of the subscribed type is published. /// void Subscribe(Action action) where T : IMessage; - - /// - /// Unsubscribe the from the message broker. - /// - void Unsubscribe(Action action) where T : IMessage; - + /// - /// Unsubscribe all actions from the message broker from of the given message type. + /// Unsubscribe the action of from the in the message broker. + /// If is null then will unsubscribe from ALL subscribers currently subscribed to /// - void Unsubscribe() where T : IMessage; + void Unsubscribe(object subscriber = null) where T : IMessage; /// /// Unsubscribe from all messages. @@ -53,7 +62,9 @@ public interface IMessageBrokerService /// public class MessageBrokerService : IMessageBrokerService { - private readonly IDictionary> _subscriptions = new Dictionary>(); + private readonly IDictionary> _subscriptions = new Dictionary>(); + + private (bool, IMessage) _isPublishing; /// public void Publish(T message) where T : IMessage @@ -63,18 +74,35 @@ public void Publish(T message) where T : IMessage return; } - var subscriptionCopy = new IList[subscriptionObjects.Count]; - + _isPublishing = (true, message); + + foreach (var subscription in subscriptionObjects) + { + var action = (Action)subscription.Value; + + action(message); + } + + _isPublishing = (false, message); + } + + /// + public void PublishSafe(T message) where T : IMessage + { + if (!_subscriptions.TryGetValue(typeof(T), out var subscriptionObjects)) + { + return; + } + + var subscriptionCopy = new object[subscriptionObjects.Count]; + subscriptionObjects.Values.CopyTo(subscriptionCopy, 0); for (var i = 0; i < subscriptionCopy.Length; i++) { - var actions = (List>) subscriptionCopy[i]; + var action = (Action)subscriptionCopy[i]; - for (var index = 0; index < actions.Count; index++) - { - actions[index](message); - } + action(message); } } @@ -88,58 +116,51 @@ public void Subscribe(Action 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(); - _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>(); - subscriptionObjects.Add(subscriber, actions); + subscriptionObjects = new Dictionary(); + _subscriptions.Add(type, subscriptionObjects); } - actions.Add(action); + subscriptionObjects[subscriber] = action; } /// - public void Unsubscribe(Action action) where T : IMessage + public void Unsubscribe(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); } } - /// - public void Unsubscribe() where T : IMessage - { - _subscriptions.Remove(typeof(T)); - } - /// public void UnsubscribeAll(object subscriber = null) { @@ -151,10 +172,7 @@ public void UnsubscribeAll(object subscriber = null) foreach (var subscriptionObjects in _subscriptions.Values) { - if (subscriptionObjects.ContainsKey(subscriber)) - { - subscriptionObjects.Remove(subscriber); - } + subscriptionObjects.Remove(subscriber); } } } diff --git a/Tests/Editor/GameLovers.Services.Tests.asmdef b/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef similarity index 100% rename from Tests/Editor/GameLovers.Services.Tests.asmdef rename to Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef diff --git a/Tests/Editor/GameLovers.Services.Tests.asmdef.meta b/Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef.meta similarity index 100% rename from Tests/Editor/GameLovers.Services.Tests.asmdef.meta rename to Tests/Editor/EditMode/GameLovers.Services.Tests.asmdef.meta diff --git a/Tests/Editor/EditMode/InstallerTest.cs b/Tests/Editor/EditMode/InstallerTest.cs index 3444753..5ae37aa 100644 --- 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 index 498cf6d..8c6234d 100644 --- a/Tests/Editor/EditMode/MessageBrokerServiceTest.cs +++ b/Tests/Editor/EditMode/MessageBrokerServiceTest.cs @@ -38,14 +38,35 @@ public void Subscribe_Publish_Successfully() { _messageBroker.Subscribe(_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(_subscriber.MockMessageCall); + _messageBroker.Subscribe(_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(_subscriber.MockMessageCall); - _messageBroker.Unsubscribe(_subscriber.MockMessageCall); + _messageBroker.Unsubscribe(_subscriber); _messageBroker.Publish(_messageType1); + _messageBroker.PublishSafe(_messageType1); _subscriber.DidNotReceive().MockMessageCall(_messageType1); } [Test] - public void UnsubscribeWithAction_KeepsSubscriptionSameType_Successfully() + public void UnsubscribeWithAction_MultipleSubscriptionSameType_RemoveAllScriptionsOfSameType() { _messageBroker.Subscribe(_subscriber.MockMessageCall); _messageBroker.Subscribe(_subscriber.MockMessageCall2); - _messageBroker.Unsubscribe(_subscriber.MockMessageCall); + _messageBroker.Unsubscribe(_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(_subscriber.MockMessageAlternativeCall); _messageBroker.Unsubscribe(); _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(_subscriber.MockMessageAlternativeCall); _messageBroker.Subscribe(_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(_subscriber.MockMessageCall)); + Assert.DoesNotThrow(() => _messageBroker.Unsubscribe(_subscriber)); Assert.DoesNotThrow(() => _messageBroker.Unsubscribe()); Assert.DoesNotThrow(() => _messageBroker.UnsubscribeAll()); } diff --git a/Tests/Editor/PlayMode/GameLovers.Services.Tests.Playmode.asmdef b/Tests/Editor/PlayMode/GameLovers.Services.Tests.Playmode.asmdef index f49b82a..1df839c 100644 --- 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 } \ No newline at end of file diff --git a/package.json b/package.json index b7f07ab..3576be2 100644 --- 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", From 4592e11f5dec52029833fd845e6bddd654bf4208 Mon Sep 17 00:00:00 2001 From: CoderGamester Date: Fri, 15 Nov 2024 17:57:10 +0000 Subject: [PATCH 2/2] PR fixes --- Runtime/MessageBrokerService.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Runtime/MessageBrokerService.cs b/Runtime/MessageBrokerService.cs index 3006f5e..b62ae28 100644 --- a/Runtime/MessageBrokerService.cs +++ b/Runtime/MessageBrokerService.cs @@ -62,7 +62,7 @@ public interface IMessageBrokerService /// public class MessageBrokerService : IMessageBrokerService { - private readonly IDictionary> _subscriptions = new Dictionary>(); + private readonly IDictionary> _subscriptions = new Dictionary>(); private (bool, IMessage) _isPublishing; @@ -94,7 +94,7 @@ public void PublishSafe(T message) where T : IMessage return; } - var subscriptionCopy = new object[subscriptionObjects.Count]; + var subscriptionCopy = new Delegate[subscriptionObjects.Count]; subscriptionObjects.Values.CopyTo(subscriptionCopy, 0); @@ -124,7 +124,7 @@ public void Subscribe(Action action) where T : IMessage if (!_subscriptions.TryGetValue(type, out var subscriptionObjects)) { - subscriptionObjects = new Dictionary(); + subscriptionObjects = new Dictionary(); _subscriptions.Add(type, subscriptionObjects); } @@ -170,6 +170,11 @@ 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) { subscriptionObjects.Remove(subscriber);