CODEWITHSUNDEEP
c#unity-game-enginesingletonmonodevelop
Vince McDonnell
Vince McDonnell

Reputation: 41

Why does my singleton have two different instances?

I use the following pattern to make my singleton in Unity

public class BlobManager : MonoBehaviour  
{
    public static BlobManager instance {get; private set;}
    void Awake () 
    {
        if(instance != null && instance != this)
           Destroy(gameObject);

        instance = this;
        DontDestroyOnLoad(gameObject);
    }

    public void SomeFunction()
    {
       if (this != instance)
           Debug.Log("They're Different!")
    }
}

They always end up being different as shown in SomeFunction(). When i set a value locally for the class BlobManager and another for using the static var "instance" like so: 

foo = "bar"; 
BlobManager.instance.foo = "foo";

the value of foo a seen in the debugger on a breakpoint within the class, will always be "bar". But when other classes try to access the same variable it will be "foo".

I'm not sure how to look at memory addresses in Monodevelop, but im sure this and this.instance would have different memory addresses. Is there something wrong with my singleton pattern? I've tried may other patterns as well with the same result.

Upvotes: 4

Views: 1953

Answers (5)

Eidivandi
Eidivandi

Reputation: 1

a singleton pattern need to have a private constructor otherwise it s not a singleton

by default a class has a public default constructor so to avoid this behavior as default we break it by a private default constructor

Upvotes: 0

Colton White
Colton White

Reputation: 976

Here is really well formed Singleton pattern that is easily reusable for monobehaviours

public class Singleton<T> : MonoBehaviour where T : MonoBehaviour
{
    public static T Instance
    {
        get
        {
            if(mInstance == null && !isQuitting)
            {
                mInstance = FindObjectOfType(typeof(T)) as T;
                if(mInstance == null)
                {
                    GameObject go = new GameObject(typeof(T).Name);
                    mInstance = go.AddComponent<T>();
                    Logger.LogWarning("Creating " + go.name + " since one does not exist in the scene currently");
                }
            }
            return mInstance;
        }
    }

    public static bool HasInstance
    {
        get
        {
            //if we dont have one we try to find one
            if(mInstance == null)
            {
                mInstance = FindObjectOfType(typeof(T)) as T;
            }
            return mInstance != null;
        }
    }

    static T mInstance;

    protected static bool isQuitting = false;
    protected virtual void OnApplicationQuit()
    {
        isQuitting = true;
    }

}

The reason that your singleton pattern is failing is because Awake is not guaranteed to be called before you are able to access it via the Singleton interface, despite the documentations claim that Awake is always called first.

The above allows you to reference the singleton at any point and it will always return a valid instance, unless the game is quitting, in which case creating a new instance will throw uncleaned up errors and leave an extra instance stranded in the scene after stopping.

Upvotes: 1

Kay
Kay

Reputation: 13146

The best way in Unity would be to avoid creating BlobManager more than once. You could achieve this for example with a Special boot scene that is responsible for setting up scene independent objects i.e. having application scope. If initialisation is done this scene is never loaded again and that's it.

The problem with your implementation of Awake is that instance is always set to the currently instantiated BlobManager component, regardless is there is one already or not. This is because Destroy does not perform a return and the code afterwards is executed anyway. Even worse it is not executed immediately. That said instance will always be updated with the most recent component even if it will be destroyed at the end of the frame and will be set to null then.

I suggest that you modify the Awake method:

void Awake () 
{
    if(instance == null) {
        instance = this;
        DontDestroyOnLoad(gameObject);
    } else {
        Destroy(gameObject);
    }
}

Note that singletons are useful if they are used sparingly. Having a bunch of them is considered as bad practice and might turn your project into a nightmare one day (s. the Links in Wikipedia for more about this).

To at least have them attached to one main game object you can use an approach dscribed in this answer: Unity singleton manager classes

Upvotes: 0

Reasurria
Reasurria

Reputation: 1838

I would advise against trying to use the classic Singleton pattern with Monobehaviour. Instead perhaps have a container object for these type of things, like an empty gameobject, and retrieve the required behaviours like so:

GameObject.Find("ContainerName").GetComponent<BlobManager>();

This is globally accessible like the Singleton pattern. There will also always just be this one instance that you manually put on your container object. This also allows for the niceties of the editor inspector.

Upvotes: 0

Stas BZ
Stas BZ

Reputation: 1192

In your example the BlobManager instantiation occur many times, but only first instance saved in property Instance.

Deriving from MonoBehaviour means that you can attach your script to many objects coursing many instances. Singleton must have private constructor, no MonoBehaviour derivation and you have to not attach this script to any object. If you need to attach the script to object, you should create another script that derive from MonoBehaviour and controls singleton

Example (not tested):

public class BlobManager
{
    static BlobManager _inst;
    public static BlobManager Instance {
    get
    {
        if (_inst == null)
            _inst = new BlobManager();
        return _inst;
    }
    }

    private BlobManager() 
    {

    }
}

Upvotes: 1

Related Questions