Singleton class inheritance not working

[Ziplock9000]

@JoshuaMcKenzie

from the moment I saw the title of this thread I knew something was wrong. Singletons and Inheritance should not even belong in the same sentence.

Singletons are designed to solve a very, very specific design problem and as a cost they violate almost every other programming principal in the book. The fact that they are misused in the Unity community to such a degree should be criminal. They support inheritance poorly, they can’t be polymorphic, and they are a pain in both multi-threading and unit testing as well. I also die a little inside when a see a singleton coupling code.

so for that reason I rarely implement Singletons, only for the case it was actually designed for. ScriptableObjects are so much better in terms of access, testing, persistance, polymorphism, threading, coupling, race conditions, and yes… inheritance. and if MonoBehaviour messages are needed making a runner to run the scriptableObject is dead simple.

spread the word, end the tyranny of the singleton

To be fair, I did kinda accidently derail and extend the OP’s original post. That’s my fault. Sorry!

[Kiwasi]

I don’t see anything obviously wrong with the code, although it’s somewhat redundant.

@Ziplock9000

Could share those three different implementations.

Sure.

You’ve got the basic Awake style, which is the same pattern you used.

static Singleton instance;

void Awake () {
    if (instance) {
        Debug.LogError("An instance already exists", instance);
        Destroy (this);
    }
    instance = this;
    DontDestroyOnLoad(GameObject);
}

Then you can run with a lazy pattern

Singleton _instance;

Singleton instance {
    get {
        if (_instance) return _instance;
        GameObject clone = new GameObject ("Singleton");
        DontDestroyOnLoad (clone);
        _instance = clone.AddComponent<Singleton>();
        return _instance;
    }
}

The other patterns are variations of these two. Sometimes I don’t need a GameObject at all, so you can just use the lazy pattern with a vanilla new.

Often I don’t want true singletons, I want a new instance to replace the old one. Great to set up scene specific singletons.

Of course often Singletons are a bad idea altogether. But that’s another thread.

[methos5k]

I tried to read and follow what this post was going on about… Is it resolved or not?

[Ironmax]

fascinating how people complicateing simple things

[Suddoha]

@Ziplock9000

One of those scenes has a button on it that has its OnClick event tied to TestSingleton.ChangeScene();
[/code]

That’s something which should have raised some attention.

According to your explanation, the SingletonScriptHost lives in the initial boot scene and the button in another scene (SceneX).
Is that correct?

So in the editor, the button in SceneX can only access the Scene X gameObjects (but there’s no way for the SingletonScriptHost to be assigned) and besides that, all the assets.

If you had another instance in SceneX, this could be wired in the editor but the reference would be missing once it calls ‘Awake’ due to its destruction and the callback wouldn’t be available anymore, so the button qouls ignore it.

Based on that, I assume you’ve accidently wired up a prefab. That’s another ‘instance’ that exists more or less inactively and that one wouldn’t run Awake or any other methods, thus sitting around and responding as a ‘valid’ subscriber.

@Kiwasi

Then you can run with a lazy pattern

Singleton _instance;

Singleton instance {
    get {
        if (_instance) return _instance;
        GameObject clone = new GameObject ("Singleton");
        DontDestroyOnLoad (clone);
        _instance = clone.AddComponent<Singleton>();
        return _instance;
    }
}

Just a little note for the ones that might find this useful:

That implementation should be used with care.
I’d still combine this one with an Awake() approach. Otherwise you can just drag another instance into your scene or someone uses AddComponent etc…, Sure, the getter will only ever get the one that has been set up lazily, but the other instances could still mess around.

[methos5k]

I was thinking the same thing about the button referencing in the wrong scene. I asked if this was ever resolved, before I went into explaining how to maybe fix that up… Anyhow

[makeshiftwings]

One of those scenes has a button on it that has its OnClick event tied to TestSingleton.ChangeScene();

Ah, this is your problem. There’s one TestSingleton on the game object in the first scene, and there is one (I’m guessing) on a prefab in your project folder that you’re dragging onto the button. So it’s not actually duplicating anything; you’re just directly referencing the one that’s sitting on the prefab rather than the one you put into a scene. Like I was saying earlier, you pretty much never want to grab a copy of the Singleton component or drag it anywhere onto any fields; the whole point of a Singleton is that you only access it through the class name and its static “Instance” field. You’re bypassing that entirely by dragging a copy onto the button and calling its methods directly. The button should have its own script with an OnClick that it calls, like this:

public class ButtonScript : MonoBehaviour {
    public void OnClick()
    {
        TestSingleton.Instance.ChangeScene("Test");
    }
}

Then there’s no need to drag a copy of the TestSingleton object anywhere; you don’t have to care where it is, you just access it through the static Instance variable.

[methos5k]

@makeshiftwings

public class ButtonScript : MonoBehaviour {
    public void OnClick()
    {
        TestSingleton.Instance.ChangeScene("Test");
    }
}

Then there’s no need to drag a copy of the TestSingleton object anywhere; you don’t have to care where it is, you just access it through the static Instance variable.

Cool, that’s exactly the suggestion I was waiting to make if he still hadn’t gotten it

[lordofduct]

@Ziplock9000

I’ve written a new simplified class to isolate the issue that manifests the same problem.

Setup
The singleton script below lives on a GameObject called “SingletonScriptHost” that sits on the first “boot” scene by itself.
After starting, scenes are additively loaded and unloaded.
One of those scenes has a button on it that has its OnClick event tied to TestSingleton.ChangeScene();

What happens
The debug output shows that the first instance of the singleton was created right at the very start of the game execution (as expected) and it obtained the instance ID of -10546 as shown when its TestSingleton.Awake() method.
Later on the 3rd scene when the Button is clicked, the Id is now -4826, which means a new instance of the Singleton exists. Not surprising since singleton duplication checks are in awake() method that the button never triggers.

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


public class TestSingleton : MonoBehaviour
{

    public static TestSingleton Instance = null;

    private void Awake()
    {
        Debug.Log("TestSingleton.Awake() GetInstanceID=" + this.GetInstanceID().ToString());

        if (Instance == null)
        {
            Instance = this;  //This is the first Singleton instance. Retain a handle to it.
        }
        else
        {
            if (Instance != this)
            {
                Destroy(this); //This is a duplicate Singleton. Destroy this instance.
            }
            else
            {
                //Existing Singleton instance found. All is good. No change.
            }
        }

        DontDestroyOnLoad(gameObject);
    }
 
    public void ChangeScene(string sceneName)
    {
        Debug.Log("TestSingleton.ChangeScene() GetInstanceID=" + this.GetInstanceID().ToString());
    }

}

As I mentioned in the first line of my OP; If the issue is due to relying on singleton duplication detection in Awake() that simply won’t work with Button.OnClick(), then I’ll have to change my implementation.

Sure there’s other implementations of Singletons, I never said or implied there wasn’t. If you look around the internet, 90% of the implementations of Unity singletons use exactly the same overall design and thus fulfill the definition of the word standard.

Found your issue.

You have UnityEvent instances that are referencing TestSingleton in subsequently loaded scenes.

You say that come scene 3, the id changes. Yeah… because the button in scene 3 is referencing the singleton you put in scene 3.

SURE, you destroyed said Singleton. But it was already created. And here’s the thing… the ‘Destroy’ method only destroys the unity engine side of things (the C++ object). The mono/C# object still exists in memory, and it’ll get cleaned up by garbage collection when it becomes eligible.

BUT, because your various UnityEvents (OnClick) is referencing that object, the mono/C# object isn’t getting cleaned up.

Anyways, even if it did… it would have no object to reference. Because UnityEvent is referencing the object directly, not the Singleton.Instance property.

This is why I have a ‘SingletonProxy’ that I use for such things… you tell it the type of the singleton, and it reaches out for it. So that you don’t have this overlapping issue.
https://github.com/lordofduct/spacepuppy-unity-framework/blob/master/SpacepuppyBase/SingletonProxy.cs

…

See, this is why I asked how YOU are implementing it, and to give an example of the problem actually happening. Just because you’re using a likeness of a common implementation, we don’t know exactly how you implemented or what bugs may exist in your specific implementation.

Oh wait… sorry, forget everything I said. It’s all just a chip on my shoulder.

Or was that chip on yours?

[Suddoha]

@lordofduct

Found your issue.

You have UnityEvent instances that are referencing TestSingleton in subsequently loaded scenes.

You say that come scene 3, the id changes. Yeah… because the button in scene 3 is referencing the singleton you put in scene 3.

SURE, you destroyed said Singleton. But it was already created. And here’s the thing… the ‘Destroy’ method only destroys the unity engine side of things (the C++ object). The mono/C# object still exists in memory, and it’ll get cleaned up by garbage collection when it becomes eligible.

BUT, because your various UnityEvents (OnClick) is referencing that object, the mono/C# object isn’t getting cleaned up.

That’s the first thing that comes to mind, but the event system takes care of destroyed objects even though they do technically still exist in memory in that case.

[Kiwasi]

@Suddoha

That’s the first thing that comes to mind, but the event system takes care of destroyed objects even though they do technically still exist in memory in that case.

I haven’t checked the implementation of the event system to be sure.

In general terms if you keep a reference to a MonoBehaviour it’s possible to call code on it after it’s been destroyed.

[makeshiftwings]

Ah yeah if there is a Singleton in every scene and you’re directly referencing one, it’s the same problem as directly referencing a prefab. Still, same solution: don’t directly reference Singletons anywhere, use Singleton.Instance.

[Suddoha]

@Kiwasi

In general terms if you keep a reference to a MonoBehaviour it’s possible to call code on it after it’s been destroyed.

Yup, I’m aware of that. However, the event system apparently takes these objects into account and prevents any calls to these objects even though a non-Unity-API touching method would run just fine.

[Ziplock9000]

@Suddoha

Based on that, I assume you’ve accidently wired up a prefab. That’s another ‘instance’ that exists more or less inactively and that one wouldn’t run Awake or any other methods, thus sitting around and responding as a ‘valid’ subscriber…

I owe you a pint, that’s exactly what’s gone wrong.

[Ziplock9000]

@makeshiftwings

public class ButtonScript : MonoBehaviour {
    public void OnClick()
    {
        TestSingleton.Instance.ChangeScene("Test");
    }
}

Indeed thanks. I had identified this as an option in my first reply but it seemed to get ignored by subsequent posts and things started to go around in circles. I got there in the end though.

@Ziplock9000

The alternative is to have even more scripts as glue between the button’s event and the Singleton, creating more administrative nightmares."

[Ziplock9000]

So the problem is solved, I just have to use the second method I mentioned at the start and use script to glue the UI events to the singleton. I really wanted to avoid this sort of design, but hey ho.

Thanks again @Suddoha and everyone else that helped.
Sometimes the most obvious mistakes stare you right in the face!.

[DryreL]

Thanks!