Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] MessageBus system for inter-mod communication #252

Closed
gotmachine opened this issue Aug 17, 2023 · 3 comments
Closed

[FEATURE] MessageBus system for inter-mod communication #252

gotmachine opened this issue Aug 17, 2023 · 3 comments

Comments

@gotmachine
Copy link

Following the discussion on discord the other day, I got bored this afternoon and wrote an alternative implementation for a system allowing mods to send messages to each other.

Compared to the implementation that was proposed on discord, this aim at being as straightforward to use and versatile as possible, while offering the best performance, notably by avoiding GC allocations, type checking and dictionary lookups past the initial MessageBus declaration and reference acquisition. The implementation allows parameterless messages, or messages with up to 4 parameters (a bit of copypasta would likely be nice to extend it to 8). This adds a bit of boilerplate code that could technically be avoided by having a single parameter version than have users define a Tuple, but Tuples have a non-negligible impact on performance compared to using parameters (Tuples are technically structs, and struct copying at scale is far from free).

On a side note, it seemed the discord implementation was aiming at some thread safety capabilities by copying the delegate list. This won't work, as copying the invocation list still is unsafe from a concurrency perspective. Making such a system really thread safe would require something quite a lot more involving, and I'm not sure I see the point since nothing in KSP 2 codebase is thread safe anyway.

public static class MessageBusManager
{
	private static Dictionary<string, MessageBusBase> _messagesBusesByName = new Dictionary<string, MessageBusBase>();

	public static T Add<T>(string name) where T : MessageBusBase, new()
	{
		if (string.IsNullOrEmpty(name))
			throw new ArgumentException("Null or empty MessageBus name");

		if (_messagesBusesByName.ContainsKey(name))
			throw new Exception($"MessageBus '{name}' exists already");

		T messageBus = new T();
		messageBus.Name = name;
		_messagesBusesByName.Add(name, messageBus);

		Debug.Print($"MessageBus '{name}' created");
		return messageBus;
	}

	public static bool Exists(string messageBusName) => _messagesBusesByName.ContainsKey(messageBusName);

	public static bool TryGet<T>(string messageBusName, out T messageBus) where T : MessageBusBase, new()
	{
		if (string.IsNullOrEmpty(messageBusName))
			throw new ArgumentException("Null or empty MessageBus name");

		if (!_messagesBusesByName.TryGetValue(messageBusName, out MessageBusBase messageBusBase))
		{
			messageBus = null;
			return false;
		}
				
		if (messageBusBase is T)
		{
			messageBus = (T)messageBusBase;
			return true;
		}

		throw new Exception($"Message bus '{messageBusBase.Name}' is of type '{messageBusBase.GetType()}' but the requested type was '{typeof(T)}'");
	}

	// Call this (potentially a bit heavy) method on some occasions, for example when a loading screen happens
	internal static void CheckForMemoryLeaks()
	{
		int memoryLeaks = 0;

		foreach (MessageBusBase messageBus in _messagesBusesByName.Values)
		{
			IReadOnlyList<Delegate> handlers = messageBus.Handlers;

			for (int i = handlers.Count; i-- > 0;)
			{
				object target = handlers[i].Target;
				if (ReferenceEquals(target, null)) // bypass UnityEngine.Object null equality overload
				{
					continue;
				}
				else if ((target is UnityEngine.Object && target == null) // check if unity class (ex : MonoBehaviour) is destroyed
					|| (target is NonUnityObject nonUnityObject && nonUnityObject.IsNotAliveAndShouldHaveBeenGarbageCollected))
				{
					Debug.Print($"Memory leak detected : a destroyed instance of the '{target.GetType().Assembly.GetName().Name}:{target.GetType().Name}' class is holding a '{messageBus.Name}' MessageBus handler");
					messageBus.RemoveHandlerAt(i);
					memoryLeaks++;
				}
			}
		}

		if (memoryLeaks > 0)
			Debug.Print($"{memoryLeaks} detected, get your shit together, modders !");
	}
}

public abstract class MessageBusBase
{
	public string Name { get; internal set; }
	internal abstract IReadOnlyList<Delegate> Handlers { get; }
	internal abstract void RemoveHandlerAt(int index);
}

public class MessageBus : MessageBusBase
{
	protected List<Action> _handlers = new List<Action>();
	internal override IReadOnlyList<Delegate> Handlers => _handlers;
	internal override void RemoveHandlerAt(int index) => _handlers.RemoveAt(index);

	public void Subscribe(Action handler)
	{
		if (handler == null)
			throw new ArgumentNullException();

		_handlers.Add(handler);
	}

	public void Unsubscribe(Action handler)
	{
		for (int i = _handlers.Count; i-- > 0;)
			if (_handlers[i] == handler)
				_handlers.RemoveAt(i);
	}

	public void Publish()
	{
		for (int i = _handlers.Count; i-- > 0;)
		{
			try
			{
				_handlers[i].Invoke();
			}
			catch (Exception e)
			{
				Debug.Print($"Error handling message '{Name}' : {e}");
			}
		}
	}
}

public class MessageBus<T1> : MessageBusBase
{
	private List<Action<T1>> _handlers = new List<Action<T1>>();
	internal override IReadOnlyList<Delegate> Handlers => _handlers;
	internal override void RemoveHandlerAt(int index) => _handlers.RemoveAt(index);

	public void Subscribe(Action<T1> handler)
	{
		if (handler == null)
			throw new ArgumentNullException();

		_handlers.Add(handler);
	}

	public void Unsubscribe(Action<T1> handler)
	{
		for (int i = _handlers.Count; i-- > 0;)
			if (_handlers[i] == handler)
				_handlers.RemoveAt(i);
	}

	public void Publish(T1 arg0)
	{
		for (int i = _handlers.Count; i-- > 0;)
		{
			try
			{
				_handlers[i].Invoke(arg0);
			}
			catch (Exception e)
			{
				Debug.Print($"Error handling message '{Name}' : {e}");
			}
		}
	}
}

public class MessageBus<T1, T2> : MessageBusBase
{
	private List<Action<T1, T2>> _handlers = new List<Action<T1, T2>>();
	internal override IReadOnlyList<Delegate> Handlers => _handlers;
	internal override void RemoveHandlerAt(int index) => _handlers.RemoveAt(index);

	public void Subscribe(Action<T1, T2> handler)
	{
		if (handler == null)
			throw new ArgumentNullException();

		_handlers.Add(handler);
	}

	public void Unsubscribe(Action<T1, T2> handler)
	{
		for (int i = _handlers.Count; i-- > 0;)
			if (_handlers[i] == handler)
				_handlers.RemoveAt(i);
	}

	public void Publish(T1 arg0, T2 arg1)
	{
		for (int i = _handlers.Count; i-- > 0;)
		{
			try
			{
				_handlers[i].Invoke(arg0, arg1);
			}
			catch (Exception e)
			{
				Debug.Print($"Error handling message '{Name}' : {e}");
			}
		}
	}
}

public class MessageBus<T1, T2, T3> : MessageBusBase
{
	private List<Action<T1, T2, T3>> _handlers = new List<Action<T1, T2, T3>>();
	internal override IReadOnlyList<Delegate> Handlers => _handlers;
	internal override void RemoveHandlerAt(int index) => _handlers.RemoveAt(index);

	public void Subscribe(Action<T1, T2, T3> handler)
	{
		if (handler == null)
			throw new ArgumentNullException();

		_handlers.Add(handler);
	}

	public void Unsubscribe(Action<T1, T2, T3> handler)
	{
		for (int i = _handlers.Count; i-- > 0;)
			if (_handlers[i] == handler)
				_handlers.RemoveAt(i);
	}

	public void Publish(T1 arg0, T2 arg1, T3 arg2)
	{
		for (int i = _handlers.Count; i-- > 0;)
		{
			try
			{
				_handlers[i].Invoke(arg0, arg1, arg2);
			}
			catch (Exception e)
			{
				Debug.Print($"Error handling message '{Name}' : {e}");
			}
		}
	}
}

public class MessageBus<T1, T2, T3, T4> : MessageBusBase
{
	private List<Action<T1, T2, T3, T4>> _handlers = new List<Action<T1, T2, T3, T4>>();
	internal override IReadOnlyList<Delegate> Handlers => _handlers;
	internal override void RemoveHandlerAt(int index) => _handlers.RemoveAt(index);

	public void Subscribe(Action<T1, T2, T3, T4> handler)
	{
		if (handler == null)
			throw new ArgumentNullException();

		_handlers.Add(handler);
	}

	public void Unsubscribe(Action<T1, T2, T3, T4> handler)
	{
		for (int i = _handlers.Count; i-- > 0;)
			if (_handlers[i] == handler)
				_handlers.RemoveAt(i);
	}

	public void Publish(T1 arg0, T2 arg1, T3 arg2, T4 arg3)
	{
		for (int i = _handlers.Count; i-- > 0;)
		{
			try
			{
				_handlers[i].Invoke(arg0, arg1, arg2, arg3);
			}
			catch (Exception e)
			{
				Debug.Print($"Error handling message '{Name}' : {e}");
			}
		}
	}
}

Example usage :

class ModA
{
	MessageBus OnSimpleEvent;
	MessageBus<string, bool> OnEventWithArgs;

	void OnBeforeModInit() // make sure MessageBus are instantiated before other mods can subscribe to them
	{
		OnSimpleEvent = MessageBusManager.Add<MessageBus>("OnSimpleEvent");
		OnEventWithArgs = MessageBusManager.Add<MessageBus<string, bool>>("OnEventWithArgs");
	}

	void OnSomeEvent()
	{
		OnSimpleEvent.Publish();
		OnEventWithArgs.Publish("event happened !", true);
	}
}

class ModB
{
	MessageBus onSimpleEvent;
	MessageBus<string, bool> onEventWithArgs;

	void OnModInit()
	{
		MessageBusManager.TryGet("OnSimpleEvent", out onSimpleEvent);
		MessageBusManager.TryGet("OnEventWithArgs", out onEventWithArgs);
	}

	void SomeSubsystemInit()
	{
		onSimpleEvent?.Subscribe(OnSimpleEvent);
		onEventWithArgs?.Subscribe(OnEventWithArgs);
	}

	void OnSimpleEvent() { }
	void OnEventWithArgs(string description, bool isCritical) { }
}

This is fully untested, but feel free to do anything with this :)

@jan-bures
Copy link
Contributor

jan-bures commented Aug 17, 2023

Thanks for your insight and feedback, we'll definitely take this into account!

Just to clarify, the example implementation was not really copying the handler list because of multi-threading, it was done simply to allow a handler to unsubscribe itself (remove itself from the list of handlers) while being executed, without causing the Collection was modified; enumeration operation may not execute. error, since the list of handlers was iterated through using a foreach. (Which, as I can see, you prevent by just using a backwards for loop.)

@cheese3660
Copy link
Member

Check the messaging branch

@jan-bures
Copy link
Contributor

Added in #255

@github-project-automation github-project-automation bot moved this from In Production to Released in Space Warp Development Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released
Development

No branches or pull requests

3 participants