Resolved Singleton Problems

Hey, first post in here. Not sure if this is the right place.

I'm kinda new to Unity, so I'm sorry if this sounds like a beginner question/has been answered already.

I've been working on my level manager and decided to create actions inside it to notify my HUD to update the score, lives and timer text. The level manager itself is a generic singleton, as I want it to persist across scenes.

All my level scenes have a Level Manager game object, as the singleton initialized in scene A would destroy the one in scene B (as far as I know).

Problem is, after I die in scene A and the level restarts, sometimes the manager appears under the DontDestroyOnLoad option in the Hierarchy, and sometimes it doesn't. When it does, I get the following error:



I'm not sure what am I doing wrong (or if I'm getting the wrong understading of how singletons work), but I'm totally lost. Maybe it has to do with the order of scripts executing?

Here is what I've coded so far:

LevelManager.cs

Code (CSharp):

1. public Action<int> OnScoreChange;
2. public Action<int> OnLivesChange;
3. public Action<int> OnLevelTimerChange;
4.  
5. private void Start() => StartLevel();
6.  
7. public void StartLevel()
8. {
9.     if (lives <= 0)
10.     {
11.         GameOver();
12.  
13.         return;
14.     }
15.  
16.     OnScoreChange?.Invoke(currentScore);
17.     OnLivesChange?.Invoke(currentLives);
18.     OnLevelTimerChange?.Invoke(currentLevelTimer);
19.     StartLevelTimer();
20. }

HUD.cs (just OnEnable/OnDisable)

Code (CSharp):

1.  private void OnEnable()
2.        {
3.            LevelManager.Instance.OnScoreChange += UpdateScore;
4.            LevelManager.Instance.OnLivesChange += UpdateLives;
5.            LevelManager.Instance.OnLevelTimerChange += UpdateLevelTimer;
6.        }
7.  
8.        private void OnDisable()
9.        {
10.            LevelManager.Instance.OnScoreChange -= UpdateScore;
11.            LevelManager.Instance.OnLivesChange -= UpdateLives;
12.            LevelManager.Instance.OnLevelTimerChange -= UpdateLevelTimer;
13.        }

Code (CSharp):

1. public abstract class Singleton<T> : MonoBehaviour where T : Component
2. {
3.     private const string SINGLETON_TAG = "(Singleton)";
4.     private static T instance;
5.     public static T Instance
6.     {
7.         get
8.         {
9.             if (instance == null)
10.             {
11.                 instance = FindObjectOfType<T>();
12.  
13.                 if (instance == null)
14.                 {              
15.                      instance = new GameObject(typeof(T).Name + SINGLETON_TAG).AddComponent<T>();
16.                 }
17.             }
18.  
19.             return instance;
20.         }
21.     }
22.  
23.     protected virtual void Awake()
24.     {
25.         if (instance == null)
26.         {
27.             instance = this as T;
28.  
29.             DontDestroyOnLoad(gameObject);
30.         }
31.         else
32.         {
33.             Destroy(gameObject);
34.         }
35.     }
36. }

Any help would be appreciated.

 

You are calling 'Instance' in OnDisable() which is called when the application closes. If the singleton has been cleaned up already then instance == null and you create a new GameObject which is gives you the error as the application is closing and so you should not be spawning new objects.

Early out from your OnDisable if(Application.quitting)

See if that helps. It's just a guess but i think that's the problem.

-Edit-
You also have another possible bug. If you call 'Instance' before Awake() is called DontDestroyOnLoad is never called because you set instance in Instance so the code in Awake() never gets called as instance is no longer null.

-Edit 2-
Personally, seeing as your level manager is a singleton anyway, i'd just make those events static and then register delegates like this:

Code (csharp):

1.  
2. void OnEnable()
3. {
4.   LevelManager.OnScoreChange += UpdateScore;
5.   ...
6. }
7.  
8. void OnDisable()
9. {
10.   LevelManager.OnScoreChange -= UpdateScore;
11.   ...
12. }
13.  

 

Hey, thanks for the explanation. I didn't know OnDisable would run when the Play mode is off.

I was making a few changes in the code and thought about changing to static, but would that be correct (as I use 'Instance' to access everything else)?

Anyway, thanks for the reply.

 

It’s your code. You make the rules. It’s correct if you want it to be. I tend to use static for most things in singletons except stuff that i need to setup in the inspector. It works for me but each to their own.

 

Hmm, I was just worried that making the Actions static wouldn't scale well. Anyway, I'm gonna give it a shot.

Thanks!

 

Well I only suggest it because your class is already a singleton, so it doesn't scale anyway, there'll only be one so a static action is no different and easier to use and has less chance of error because you remove a possible null reference exception.

I do this all the time and have not yet run into issues. You'll be fine.