Ideas on avoiding the singleton pattern through static events

Not sure how this code will eventually develop (so it is possible that it will not work in future) but in general, when events are used, there is often no need to use the singleton pattern.

There are two reasons it was needed in the video:

  • to access the event “queue” so “selected unit visuals” can subscribe
  • to get the currently selected unit by calling a getter on the UnitActionSystem

While the latter one is nice, it couples the SelectedUnitVisual class with UnitActionSystem a bit more (since it now expects it to own/have the information about currently selected character). It could be avoided if the event itself carried the new Unit value (which is easily doable by replacing EventHandler with Action) making the decoupling stronger.

So we are left with the problem of how to allow all those SelectedUnitVisuals to subscribe. This can be solved by making the event handler static.

Altogether it could look like this:

public static event Action<Unit> OnSelectedUnitChanged;

on UnitActionSystem side and:

    void Awake() {
       //...
        UnitActionSystem.OnSelectedUnitChanged += UnitActionSystem_OnSelectedUnitChanged;
    }

    void UnitActionSystem_OnSelectedUnitChanged(Unit newUnit) {
        _meshRenderer.enabled = (newUnit == _associatedUnit);
    }

in SelectedUnitVisual.

One disadvantage I can see is that if multiple UnitActionSystems got created, the events will probably be fired twice each time and I don’t see a nice way of alerting the programmer to that (other than doing the whole Instance check in Awake method, at which point it is basically a full singleton).

But in general, I find this solution nicer, as it forces stronger decoupling by default.
It also requires less boilerplate code than the singleton solution, which I’d say makes it more readable.

1 Like

If you use a static singleton and all events are non-static, then the clean up is handled automatically. If you go to a new scene and the UnitActionSystem is destroyed then the clean up is automatic.
Whereas if you use static events you need to reset them yourself.
And as the UnitActionSystem becomes more complex you need to add more and more static events making clean up more and more complex.

So yes that approach does work but be aware of that difference. I prefer to keep as few things static as possible so that clean up is simpler, objects are automatically destroyed when moving scenes so that simplifies things as opposed to manually cleaning up every single static event.

Yes you can also just pass in the Unit in the events, although it won’t affect coupling since you still need to subscribe to the event so in that case it’s really personal preference.

3 Likes

Ah, I missed that part about cleanup. I am not used to garbage collected languages - when I saw it still working after scene reset I assumed the event queue can handle nulls. But of course, this is the other way around and those objects would not actually be garbage collected at all leading to a non obvious memory leak… (at least non-obvious to my mind, not accustomed to thinking in terms of garbage collection. it is definitely easier to reason about this stuff in non-garbage collected languages :slight_smile: )

Thanks, CodeMonkey. I will add

    void OnDestroy() {
        OnSelectedUnitChanged = null;
    }

to UnitActionSystem for now and see how far can I go with this approach.

1 Like

On second thought, how is the singleton pattern supposed to work here with the cleanup? Say that I want to restart the scene for whatever reason - the current Instance would not be destroyed would it? There is a static reference to it still for the whole time of life of the program.
And if it won’t destroy, same for all the event handlers and thus for all the subscribers. So, if we happen to restart the scene, all that will still sit in memory and if there was any state in UnitActionSystem, it will still be there (even though OnDestroy was called on it).

So, to do a clean restart, the Instance would need to be set to null in OnDestroy(), wouldn’t it? (unless of course we want something to be preserved across the scenes)

The Game Object is destroyed on scene change and after that happens the static field will simply point towards a null reference, no need to manually clean it.

1 Like

Your response didn’t make sense to me, since how can it be actually destroyed when this is a garbage collected language and there is definitely still a reference to the object for the whole time the program is running (the static Instance).

But I did some Google searches on how Destroy actually works in Unity, and it seems that they are able to basically force the garbage collection here by doing this double reference trick (edit: since the underlying object is actually c++, I am not sure “garbage collection” is actually in play here, but the gist is the same): https://forum.unity.com/threads/destroy-gameobject-and-c-how-does-it-work.824298/#post-5456688 (which to me is like: if they wanted to manage their own memory, why can’t we use a normal language, without GC, instead of C#, but ok…).
I couldn’t find any of this in the official documentation unfortunately (happy to learn if you happen to have a link), only this blog entry: Custom == operator, should we keep it? | Unity Blog

So yeah, it seems that the only thing Instance variable prevents from garbage collecting is the C# wrapper object, with an overloaded == operator, thus Awake will properly create another instance in future, and the memory penalty is almost nonexistent (assuming here that the wrapper object has a size on the order of a few raw pointers).

It is a shame that they don’t explain it more often in their documentation - it is quite confusing from a newcomer perspective. But oh well, Unity does not have the greatest documentation anyway… That’s definitely one thing that UnrealEngine seems to do much better.

But back to the topic - thanks for the pointers, I will keep all that in mind for any future singletons of mine :wink:

1 Like

Privacy & Terms