Should we rely on Fader to have DontDestroyOnLoad?

Unity Courses Ask
#1

When I was doing the challenge my instinct was to call DontDestroyOnLoad for the fader within Transition(). But then I realized the Fader was already in that scene.

It got me wondering – should the Portal script and namely the Transition method rely on fader already being in the DontDestroyOnLoad scene? My instinct is no, so I added checks for it. Checking an objects scene seems to be the correct way to do this (code below). Curious on feedback here.

FWIW, I also put null checks for the otherPortal parameter passed within UpdatePlayer since its possible for GetOtherPortal() to return null. I think the performance cost for simple input validation (e.g. null checks, bounds checks) is pretty negligible so I tend to do them.

          Fader fader = FindObjectOfType<Fader>();
            if (fader.gameObject.scene.name != "DontDestroyOnLoad")
            {
                DontDestroyOnLoad(fader);
            }
#2

The SavingWrapper will at some point also be reliant on the Fader being persistent, so it can FadeOutImmediate() and then load the game. This becomes especially important in the final section of Shops and Abilities where we create an intro menu system, also switching between scenes. You could add this section to each of the classes that is dependent on Fader, but in the end it’s much easier (and less error prone) to put the Fader in the Persistent Objects Prefab and ensure that it’s always there.

#3

I agree in that I would still keep Fader in Persistent Objects Prefab.

Here’s my problem though. As I’m writing code (and re-reading previously written code) it’s hard to see why things like Transition should work as there’s nothing within the Portal.cs (or Fader.cs or PersistentObjectSpawner.cs for that matter) to say that there will be a Fader in the DontDestroyOnLoad scene.

So short of adding something like the

if (fader.gameObject.scene.name != "DontDestroyOnLoad")
{
   DontDestroyOnLoad(fader);
}

what are good ways to make the code more readable. e.g. maybe having some unifying class that you can reference to get a persistent object? e.g. I literally just thought of this:

persistentObjectContainer.GetFader().FadeOut(fadeOutTime);`

where persistentObjectContainer can be found with some GameObject.Find* method.

#4

At that point, just using Singletons is probably a better fix.

I’m going to let you in on a poorly kept secret: “Better than a Singleton” is…

just another form of Singleton.

#5

lol… yeah after I wrote that, I realized that GameObject.Find* basically acts like singleton.

I can see why using Singletons is appealing.

#6

Please understand… personally, I hate using Singletons. It’s a bias born into me from my earliest experiences with OOP with Pascal, C++, Java, and carried right on into C#. All of these tricks are centered around one idea, getting around the nature of objects, and there are other ways to accomplish the same results in many cases.

Unfortunately, the way Unity is constructed, with it’s scene based game logic, it’s difficult to get around Singletons, and where I would never use a Singleton when writing a database app or a word processor, they’re close to a necessity in Unity.

For some things a fully static class will suffice… for example, did you know that as presented in the course, you could convert the entire SavingSystem to a static class by removing the MonoBehavior, and setting every method in the class to static. You could then call SavingSystem.LoadLastScene() or SavingSystem.Save(), etc with no loss of functionality. And since all of these calls don’t rely on existing state in the first place, it wouldn’t even be breaking the rules.

That won’t work with the Fader or SavingWrapper, because they need to be attached to a MonoBehaviour to properly run a coroutine, of course.

#7

I’m also not a fan of Singletons. Rather than having lots of Singletons floating around, for my own game, I’m thinking of going with my own service locator pattern. I’m really not a fan of GameObject.Find either because you’re SOL if it returns nothing (and it’s slow). Every time I use it I feel it reveals some gap in my design or approach.

Just getting started with the saving lessons - curious to see how Sam approaches it.

+1 I like the static class for anything truly stateless.

I haven’t gotten there yet, but did you mean SavingWrapper?

#8

Of course I did. Isn’t that what it says? I’m sure I didn’t edit the post after seeing this.

I’ve tried a few things… for example, I’m also the TA of the Turn Based Strategy course which cough uses a cough cough LOT of Singletons. Just to see if it was possible, I stripped the game of it’s Singletons entirely. Admittedly, I did use static EventHandlers to perform a sort of Marco/Polo locator system. It treats getting the class asynchronously… here’s how it works:

LocatableMonoBehaviour.cs 
using System;
using System.Collections.Generic;
using UnityEngine;

public abstract class LocatableMonoBehaviour<T> : MonoBehaviour where T : LocatableMonoBehaviour<T>
{
    /// <summary>
    /// Used by the system to store requests to locate the "Singleton"
    /// </summary>
    private static Queue<System.Action<LocatableMonoBehaviour<T>>> queue = new Queue<Action<LocatableMonoBehaviour<T>>>();
    /// <summary>
    /// Ping() uses this event to tell the instance that it should call the members of the queue.
    /// </summary>
    private static event System.Action pingEvent;
    /// <summary>
    /// Enqueues the callback to the queue, and invokes the pingEvent.  If the behaviour has already called Awake(), then
    /// this should give an immediate callback, otherwise the supplied callback is called when the Awake() method has completed.
    /// </summary>
    /// <param name="callback">A delegate to receive a reference to the instance.</param>
    public static void Ping(System.Action<LocatableMonoBehaviour<T>> callback)
    {
        queue.Enqueue(callback);
        pingEvent?.Invoke();
    }
    /// <summary>
    /// In most virtual Awake() patterns, one calls base:Awake() first before any other methods.  I recommend
    /// base.Awake() be called at the end of your overridden Awake() method to ensure that all needed initializion
    /// has occurred.  This will call the queue of objects needing a reference to the instance, and clear it, finally
    /// signing up for the pingEvent so future calls are handled immediately.
    /// </summary>
    protected virtual void Awake()
    {
        PingMe();
        pingEvent += PingMe;
    }
    /// <summary>
    /// Informs any delegate in the queue that this is the instance.
    /// </summary>
    private void PingMe()
    {
        while (queue.Count > 0)
        {
            queue.Dequeue()?.Invoke(this);
        }
    }

    /// <summary>
    /// Cleans things up, then the T in the next scene will start the cycle all over again.
    /// </summary>
    protected void OnDestroy()
    {
        pingEvent -= PingMe;
    }
}

You create your class inheriting from this LocatableMonoBehaviour

public class MyMonoBehaviour : LocatableMonoBehaviour<MyMonoBehaviour>

Yep, that’s a weird class signature, but just trust me, it works.
Now when you need to get a reference to the class, you call the static Ping() method with a method with a signature to take that type, for example in another class

MyMonoBehaviour myMonoBehaviour;
void Awake()
{
    MyMonoBehaviour.Ping(SetMyMonoBehaviour);
}
void SetMyMonoBehaviour(MyMonoBehaviour instance)
{
    myMonoBehaviour = instance;
    //Do things with the instance here
}
1 Like
#9

Wow this is awesome. Brian you really should do a class. Mine would have looked nothing like this but this is what I wish mine would look like.

I took the TBS course and I have some prior familiarity with generics so I think I can understand what this code is doing and the purpose behind it.

So you’re getting the LocatableMonoBehavior async because it may not have been initialized yet when the requester wants it? Are you using Coroutines or something similar in your Start methods to delay the completion of Start until the “SetMyMonoBehaviour” has been called? So effectively you’re shielding yourself script execution order issues?

Again I wish I could put something like this together from scratch. You should definitely do a class teaching some of these advanced techniques.

#10

Start in these cases is virtually empty. The response method serves as Start. If Update() needs information, then a blocking boolean is put in Update() and cleared in the response method.

1 Like
#11

The response method being “SetMyMonoBehaviour”?

As a practice I guess that means that anyone that extends the LMB should try at all costs to avoid having any code running in Start(). I might even make a variant of the LMB (maybe LocatableMonoBehaviorSafe) where it has an empty Start() method to prevent such issues. Or maybe have an IsInitialized() method as part of the abstract class definition so that anyone who requests a copy can know if its initialized or not. Or maybe a NotifyWhenInitialized() callback that a requestor can use within the SetMyMonoBehaviour class? But that might risk creating deadlocks. I’m coming back to the GameManager concept where a single object can coordinate initialization across multiple other objects.

One thing that messed me up when I tried to expand upon the Turn Based Strategy course was that so many of the Singletons had dependencies on eachother within Start().

#12

This topic was automatically closed 24 hours after the last reply. New replies are no longer allowed.

Privacy & Terms