I’m having problems with my Singleton class that I am trying to make so other static classes (such as GameManager) can inherit from them and automatically be singletons. Below is the Singleton class I have made and below that is the beginning of the GameManager script. Basically it seems that its not working at all. The GameManager GameObject does not got into the DontDestroyOnLoad parent folder in the hierarchy once the game is loaded and none of the test debug text is executing . For the life of me I cannot work out why. Do I need to call the Singleton maybe in the GameManager script to enable it?
Also I suspect the FindObjectOfType() is not optimal…
Any help would be greatly appreciated.
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Singleton<T> : MonoBehaviour where T : MonoBehaviour {
private static T instance;
public static T Instance
{
get
{
Debug.Log("test1");
if (instance == null)
{
instance = FindObjectOfType<T>();
Debug.Log("test2");
}
else if (instance != FindObjectOfType<T>())
{
Destroy(FindObjectOfType<T>());
Debug.Log("test3");
}
DontDestroyOnLoad(FindObjectOfType<T>().gameObject.transform.root);
return instance;
}
}
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class GameManager : Singleton<GameManager> {
FindObjectOfType is a really weird way to handle singletons. If you are going to use it, you should call it once and cache the result, there is no gaureentee it will return the same instance each time it’s called.
The typical ‘Unity way’ to handle a singleton is to put a check in Awake to see if the instance already exists, and take action as appropriate.
So your code becomes.
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Singleton<T> : MonoBehaviour where T : MonoBehaviour {
public static T instance {get; private set;}
protected virtual void Awake (){
if (instance != null){
Debug.LogError("A instance already exists");
Detroy(this); //Or GameObject as appropriate
return;
}
instance = this;
}
}
TL;DR; Singleton control belongs in the constructor (ie Awake), not in the accessor.
Thanks for the feedback guys, you are right. I had an inkling while following the course the way they were teaching was incorrect (main reaso I posted here). It did seem weird using the FindObjectofType as I had a feeling that would cause issues with more than one Singleton. Also as you say putting the code in the getter seems odd. I will move the code into the Awake methods as you suggest and see what happens
The major issue I’ve had with the standard Unity Singleton model is that if for example a button’s onclick event calls into the Singleton, the Awake() event never gets called, so you start to get duplicates.
The alternative is to have even more scripts as glue between the button’s event and the Singleton, creating more administrative nightmares.
Neither prospect is ideal or giving the developer options and should not force a certain design pattern on the developer.
Hopefully I’ve missed something obvious and someone helpful person will point out what.
What is the standard unity singleton model? Unity doesn’t have a standard singleton that I know of.
Why has the Awake event never been called? Your gameobject is enabled right?
I don’t know what your design is… so I don’t know what extra glue you’re adding.
You’re not forced to use a specific design pattern. Again, unity doesn’t have a standard singleton model that I know of. You don’t have to use the Singleton pattern at all if you don’t want to.
Not exactly sure what you’re even referring to exactly at this point… if you could let me know, I may try to help out.
I’m not. Unity is creating a new singleton instance behind the scenes when the OnClick event calls it. You’ll only notice this if you dump out instanceID’s from the singleton’s constructor. You wont notice this if you just check ID’s in awake.
I already do that, that’s why I said standard unity singleton. But onclick is not triggering the awake function on the singleton, thus not checking for duplicates.
Just to recap for clarification.
A standard unity singleton gets created by being attached to a GameObject in an empty boot scene. It has the usual public instance handle and in Awake() has anti-duplication methods and DontDestroyOnLoad.
Everything is perfect, calling this will work fine and there will be only one instance of the singleton when called from code anywhere in the project and from across scenes.
Another scene is loaded and the single Singleton persists as you’d expect.
A UI button is clicked, the onclick event points to a method on the Singleton script.
Boom… a 2nd instance had now just been created of the singleton but because Awake is not triggered, it can’t test for duplicates and destroy them.
Again, unless you dump out instance ID’s in the singleton’s class constructor you might never notice there’s a 2nd instance created.
I feel like there might be a bug in your code; I’ve never seen OnClick randomly duplicate objects for no reason. Maybe you could post an example of what you’re doing in OnClick? Why does “the onclick event points to the GameObject that hosts the singleton”? The point of a Singleton is that you access it through the class name, like “MySingleton.instance.DoTheThing()”. You shouldn’t need to reference the actual GameObject inside a button.
Yeah that was a mistake in my last post, I’m accessing the Singleton Script, not the GameObject. (I’ve updated that post)
The code I’m running in OnClick() on the Singleton could be an empty method, it still behaves the same.
The overwhelming majority of Unity singletons are implemented using a variation of the same design pattern. This involves using a public instance variable, checking for duplicates and calling DontDestroyOnLoad in Awake(). Most of which is unique to Unity. This absolutely qualifies as being described as a “standard unity singleton” and everyone else seems to get this.
You seem to have a chip on your shoulder mate and that was obvious from your first response to my question. I don’t care for that attitude or your help.
Oh wait that might explain it. Are you saying you have a constructor in a MonoBehaviour? You’re not supposed to use constructors with MonoBehaviours because Unity will call them constantly at unexpected times, like every time it recompiles or loads a new scene. For MonoBehaviours, you should do anything constructory in Awake().
Show us a code example. Because the issues you are talking about just don’t happen in my singleton implementation. No matter which method I use.
(I also agree that there is no such thing as a ‘standard Unity singleton’. I have about three different implementations that I use depending on the situation.)
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.
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.
