chark/scriptable-events

Multi-source event listeners

ChaosCrafter opened this issue · 13 comments

ChaosCrafter commented

I would like BaseScriptableEventMultiListener
If it exists, I can't find it.

It would be great to have a way to declare a listener that was able to listen for multiple events (of the same base type)
The idea here is that you might have multiple game objects raising different types of events that have the same base data type (for example logWarning, logError, logDebug etc. You have objects that want to subscribe to just one type of event, and others that want to subscribe to more than one. We can add multiple listeners, but it would be cleaner and more readable if an object could declare a multi-listener.
As a full example, in the game we are developing there are multiple types of cards. The cards raise an event of card-type-clicked when they are clicked. Many systems just need to know when a particular card is clicked, but several systems want to know if any one of several types is clicked.
So we have CardClickedEvent and CardClickedEventListener.
We have MonsterCardClickedEvent, CharacterCardClickedEvent, TerrainCardClickedEvent, TransportCardClickedEvent etc. etc.
The combat system wants to listen for MonsterCardClickedEvent and CharacterCardClickedEvent. The movement system wants Terrain and Transport Card events. The show focus card wants all cardClicked events.
I know there are ways round it, and the example is (very) simplified, but it'd be great to have a version of BaseScriptableEventListener that was BaseScriptableEventMultiListener with
public List<ScriptableEvent> scriptableEvents;
and the onRaised of a normal ScriptableEventListener.

ChaosCrafter commented

Sample-code: (ssems to work, but I can't bind the MonoGui behaviour locally without pulling the whole project.

using System.Collections.Generic;
using UnityEngine;
using UnityEngine.Events;

namespace ScriptableEvents
{
///


/// Base Scriptable Event Multi Listener which accepts an argument, used as a base by all internal
/// and custom multi-listener components.
///
///
/// Type of data which is passed as an argument to this listener
///
public abstract class BaseScriptableEventMultiListener
: BaseScriptableEventMultiListener, IScriptableEventListener
{
#region Editor
[SerializeField]
[Tooltip("Scriptable Events that trigger the On Raised UnityEvent")]
public List<BaseScriptableEvent> scriptableEvents = new List<BaseScriptableEvent>();

    [Space]
    [SerializeField]
    private UnityEvent<TArg> onRaised;

    #endregion

    #region Unity Lifecycle

    private void OnEnable()
    {
        if (scriptableEvents == null)
        {
            Debug.LogError("ScriptableEvent is not assigned", this);
            enabled = false;
            return;
        }

        foreach (var scriptableEvent in scriptableEvents)
        {
            scriptableEvent.AddListener(this);
        }
    }

    private void OnDisable()
    {
        if (scriptableEvents == null)
        {
            return;
        }
        foreach (var scriptableEvent in scriptableEvents)
        {
            scriptableEvent.RemoveListener(this);
        }
    }

    #endregion

    #region Public Methods

    public void OnRaised(TArg value)
    {
        onRaised.Invoke(value);
    }
    #endregion
}

/// <summary>
/// Base Scriptable Event Multi Listener which is implemented by all  multi-listener components and is used
/// in internal editor scripts.
/// </summary>
/// 
[ScriptableIcon(ScriptableIconType.Listener)]
public abstract class BaseScriptableEventMultiListener : MonoBehaviour
{
}

}

ChaosCrafter commented

Base editor code

using UnityEditor;

namespace ScriptableEvents.Editor
{
///


/// Default editor for Scriptable Event Listeners which don't an explicit editor.
///
[CanEditMultipleObjects]
[CustomEditor(typeof(BaseScriptableEventMultiListener), true)]
internal class BaseScriptableEventMultiListenerEditor
#if ODIN_INSPECTOR
: Sirenix.OdinInspector.Editor.OdinEditor
#else
: UnityEditor.Editor
#endif
{
#region Private Fields

    // Target scriptable event listener fields.
    private BaseScriptableEventMultiListener baseScriptableEventMultiListener;
    private MonoScript monoScript;

    // Serialized properties.

#if ODIN_INSPECTOR
private Sirenix.OdinInspector.Editor.InspectorProperty scriptableEventsProperty;
private Sirenix.OdinInspector.Editor.InspectorProperty onRaisedProperty;
#else
private SerializedProperty scriptableEventsProperty;
private SerializedProperty onRaisedProperty;
#endif

    #endregion

    #region Unity Lifecycle

#if ODIN_INSPECTOR
protected override void OnEnable()
{
#else
protected void OnEnable()
{
#endif
SetupEditor();
}

    public override void OnInspectorGUI()
    {
        DrawMonoScript();

#if ODIN_INSPECTOR
Tree.BeginDraw(true);
#else
serializedObject.Update();
EditorGUI.BeginChangeCheck();
#endif

        DrawScriptableEvent();
        DrawOnRaised();

#if ODIN_INSPECTOR
Tree.EndDraw();
#else
if (EditorGUI.EndChangeCheck())
{
serializedObject.ApplyModifiedProperties();
}
#endif
}

    #endregion

    #region Private Setup Methods

    private void SetupEditor()
    {
        SetupBaseScriptableEventMultiListener();
        SetupMonoScript();
        SetupSerializedProperties();
    }

    private void SetupBaseScriptableEventMultiListener()
    {
        baseScriptableEventMultiListener = target as BaseScriptableEventMultiListener;
    }

    private void SetupMonoScript()
    {
        monoScript = MonoScript.FromMonoBehaviour(baseScriptableEventMultiListener);
    }

    private void SetupSerializedProperties()
    {

#if ODIN_INSPECTOR
scriptableEventsProperty = Tree.GetPropertyAtPath("scriptableEvents");
onRaisedProperty = Tree.GetPropertyAtPath("onRaised");
#else
scriptableEventProperty = serializedObject.FindProperty("scriptableEvent");
onRaisedProperty = serializedObject.FindProperty("onRaised");
#endif
}

    #endregion

    #region Private Draw Methods

    private void DrawMonoScript()
    {
        ScriptableEventGUI.MonoScriptField(monoScript);
    }

    private void DrawScriptableEvent()
    {

#if ODIN_INSPECTOR
scriptableEventsProperty.Draw();
#else
EditorGUILayout.PropertyField(scriptableEventProperty);
#endif
}

    private void DrawOnRaised()
    {

#if ODIN_INSPECTOR
onRaisedProperty.Draw();
#else
EditorGUILayout.PropertyField(onRaisedProperty);
#endif
}

    #endregion
}

}

Edvinas01 commented

Seems like a reasonable feature request. We actually had to implement something like this in one of our games recently, but I was not sure if it made sense to add this to the package :D

I'll think about this on how to structure it properly and will try to squeeze it in. It might involve a bit more changes and tests, also code generation would have to be updated as well. A bit busy at the moment so it might take some time, unless you're willing to make a PR @ChaosCrafter

ChaosCrafter commented
ChaosCrafter commented
Edvinas01 commented

Hey @ChaosCrafter, sorry for not replying sooner, was a bit busy today 😅

I did however spend some time thinking about your suggestions 1 & 2 as I saw your email early this morning. I personally think approach 2 is better as this would reduce the maintenance needed for the package and also keep it a lot more simple (no need to look for specific event types in drop-down menus for multi-event support and also easy migration if suddenly you needed multi-events for a specific thing).

I think we can do this the following way without breaking serialization (we could simply nuke the field, however since this is serialization, it would be extremely painful for the users to migrate to the new major release as they won't get any compilation errors):

  1. Hide the old scriptableEvent field in BaseScriptableEventListener class by adding HideInInspector attribute.
  2. Add a new scriptableEvents list field in BaseScriptableEventListener class.
  3. Mark the old scriptableEvent as obsolete (I believe there is an Obsolete attribute that could be used for this) so we don't use it by accident.
  4. Implement ISerializationCallbackReceiver in BaseScriptableEventListener and override OnBeforeSerialize:
    1. In OnBeforeSerialize, check if scriptableEvent field is non false.
    2. If its not, check if the scriptableEvents does not contain this event already.
    3. If it does not contain it, populate the list with this event.
    4. Set the scriptableEvent to null as now the list contains this event.
    5. One thing to note when using OnBeforeSerialize, is that it should skip any logic when the application is running (do a Application.isPlaying check). There might be more gotchas here though as I've used it sparingly so far.

In regards to making a pull request, I'd suggest to look into Creating a pull request as I'd rather not copy your code from a zip file as that would strip your commit history and also you'll lose your contributor status. The gist of it is:

  • Fork this repository to your GitHub account (click the Fork button at the top right cornet).
  • Make a new branch with your changes on the Forked GitHub repository.
  • Make a pull request from your forked GitHub repository to the master branch of this repository.
  • Once all the PR ceremony is completed and your PR is merged in, I'll update the build number, publish a new release.

Also,

What is meant by code-generation in this context?

See this - I'm personally not a big fan of how this works right now, but it is what it is. If its a showstopper, you can skip this part and I'll look before release.

ChaosCrafter commented
  • 👍1
ChaosCrafter commented
Edvinas01 commented

Resolved in #23

ChaosCrafter commented
  • ❤️2
Edvinas01 commented

Awesome, thanks! Excited to have made my first contribution to an open-source/community project. Now to update my project and get it working in-situ. Thanks for your support with this, and helping me get my PR through. Good luck with the rest of it.
…
On Thu, Aug 4, 2022 at 3:52 PM Edvinas Danevičius @.> wrote: Closed #22 <#22> as completed. — Reply to this email directly, view it on GitHub <#22 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCQRKM6QESCYRH7MKGH65TVXNLB5ANCNFSM55HOMJ4Q . You are receiving this because you were mentioned.Message ID: @.>

I'll try to make a 2.2.0 release this evening and update the UPM branch, should be fine to pull afterwards!

ChaosCrafter commented
ChaosCrafter commented
  • 👍1