Unity - Toolbox pattern is not safe

Question

When I create a toolbox such is in this tutorial. How do I defend against breaking the pattern with multiple instances like this:

gameObject.AddComponent<Toolbox>();
gameObject.AddComponent<Toolbox>();

I when calling Toolbox.Instance I get following message:

[Singleton] Something went really wrong - there should never be more than 1 singleton! Reopening the scene might fix it.

Is there a way to defend against it ?

Answer 1

Add [DisallowMultipleComponent]before your class to make sure you can't add more than one instance of it.

example:

[DisallowMultipleComponent]
public class Toolbox : MonoBehaviour

Also, I recommend reading DMGregory's comment before implementing this, it might be better to just create a static class instead.

Answer 2

You can use my singleton class. It is a singleton MonoBehaviour. Use it by extending it:

public class MyClass : MonoBehaviour<MyClass>

The most current version is available on GitHub, but here is the current version of it:

using System.Text.RegularExpressions;
using UnityEngine;

/// <summary>
/// Extending this class creates a MonoBehaviour which may only have on instance and will not be destroyed between scenes.  When extending, the type of the inheriting class must be passed.
/// </summary>
public abstract class ManagerBehaviour<TManager> : MonoBehaviour where TManager : ManagerBehaviour<TManager>
{
    private const string ManagerName = "Manager";

    private static TManager instance;

    /// <summary>
    /// Gets the singleton instance of the Manager
    /// </summary>
    public static TManager Instance
    {
        get
        {
            if (!instance)
            {
                instance = FindObjectOfType<TManager>();
                if (!instance)
                {
                    var masterManager = GameObjectUtility.GetOrAddGameObject(ManagerName);
                    var managerName = Regex.Replace(typeof(TManager).ToString(), @"((?<=\p{Ll})\p{Lu})|((?!\A)\p{Lu}(?>\p{Ll}))", " $0");
                    var manager = GameObjectUtility.GetOrAddGameObject(managerName);
                    manager.transform.SetParent(masterManager.transform);
                    instance = manager.AddComponent<TManager>();
                }
                var root = instance.transform;
                while (root.parent)
                {
                    root = root.parent;
                }
                DontDestroyOnLoad(root.gameObject);
            }
            return instance;
        }
    }

    protected virtual void Awake()
    {
        DestroyDuplicateManagers();
    }

    private void DestroyDuplicateManagers()
    {
        var managers = FindObjectsOfType<TManager>();
        foreach (var manager in managers)
        {
            if (!manager)
            {
                continue;
            }
            if (Instance != manager)
            {
                bool sharesGameObjectWithManager = Instance.gameObject == manager.gameObject;
                bool hasExtraComponents = manager.GetComponents<MonoBehaviour>().Length > 1;
                bool hasChildren = transform.childCount > 0;
                bool destroyGameObject = !sharesGameObjectWithManager && !hasExtraComponents && !hasChildren;
                if (destroyGameObject)
                {
                    Destroy(manager.gameObject);
                }
                else
                {
                    Destroy(manager);
                }
            }
        }
    }
}

I aim to make it a fully functional singleton MonoBehaviour. It is still a work in progress, but it is getting close. (I need to look into DisallowMultipleComponent posted in other answer...I didn't know that was a thing.)

Answer 3

The method gameObject.AddComponent<component-class>() creates a new instance of the component-class. But a singleton only ever has one instance, and that instance is supposed to be created by the singleton internally. That means a class which is supposed to be a Singleton should not be used as a component.

However, a component can use a Singleton internally to share data with other components. That component would then use the static property SingletonClass.Instance to get the global instance of the singleton. When you follow that pattern, reconsider if it really makes sense for your Toolbox class to inherit from MonoBehavior.

Answer 4

All the source I read here still seems a bit unsafe to me, you are not using the lock, even with a lock in a paranoid area of my mind I feel that it is sill possible (though improbable) to create more than one singleton instance (bad).

People have suggested Static Classes, why does no-one just create their manager script in the scene beforehand? Then there is only one! Are some people really that bothered about automatic manager script creation when it's a bad design pattern?

Make a static class that searches for the script and saves a reference, the worst that can happen now is multiple searches might occur for the manager script, but they will just overwrite each other (but not duplicate the manager script).

I wanted to make this comment as I think this toolbox pattern leads people astray, it's only required if you need your units to set up automatically, it's much safer to manually ensure you just have one script!

Answer 5

MonoBehaviours aren't designed to be singletons. They are designed to be instantiated by the Unity engine whenever it feels like that.

And most uses of the "Toolbox Pattern" don't actually need to be MonoBehaviours because there is no good reason for them to be added to any particular game object or for them to implement the Unity standard methods like Update or Start.

• If they need initialization, do it in a C# constructor.
• Static methods inherited from MonoBehaviour, like Instantiate, can be called by explicitly writing the class in front of it (UnityEngine.Object.Instantiate(prefab)).
• One thing you lose is the ability to create coroutines, but you can still implement a method which returns an IEnumerator so it can be used to start a coroutine from a MonoBheaviour.

So just don't use that Toolbox pattern. Just create a plain old C# class instead.