I have been working on implementing Ads into LuxelOperation’s current project for the past week or so. Having finally wrapped up all of the loose ends I turned it over to one of my routine play testers. He immediately found what was not only a game breaking bug, but a show stopping bug and I didn’t have an easy answer (after looking at the log output) of why.
Our project has a UI overlay that closes two doors over the scene when we change scenes. This covers up the scene change and allows us to give an obvious loading screen if it’s going to be more than an instantaneous change. We decided to trigger the overlay to “close” the doors, and then display an ad, and after the user returns from watching the ad the doors would open back up. This gave some notification that an Ad was about to happen and really helps on older devices where the change in Android Activity’s is a sluggish and jarring process (something which is almost painless on modern devices fortunately).
The Problem:
What was happening in play tests was that the first time you began playing the game in a session, everything would be peachy. The ads would show up fine, the doors would open and close and everything went exactly as planned. However, if you returned to the main menu and then went back into the Gameplay scene as soon as the first ad finished playing the doors would never open and the game would be unplayable.
A short time looking through the logs later showed that the script that moved the UI components was suddenly throwing NullReferenceExceptions because it couldn’t find the doors… Which had worked fine up until we returned from the ad, and worked fine the first time the scene was loaded. This was very confusing because the doors are not persistent through scenes, so by reloading the Gameplay scene a second time it should have been exactly like the first time. (The doors do not use Unity’s DontDestroyOnLoad).
Many hours of printf-style debugging later, I finally came to the conclusion that the MonoBehaviour that was calling the shots was null. Except, the other MonoBehaviours it referenced weren’t null, but things they referenced were null! Confused and frustrated, I called it quits for the night only to have something tickle the back of my brain… C# Events.
As near as I can tell, the final score is this:
- GameObject “A” registered a delegate to an Event that takes place on GameObject “B” (which was marked as DontDestroyOnLoad (and thus was persistent)).
- The Scene was loaded, the Delegate was registered, and everything was fine.
- The Scene was unloaded, and everything was destroyed. Except the GameObject still had a reference to it the form of that Delegate (as far as the C# Garbage Collector is concerned), so the GameObject persisted in C# memory but not in the Unity Hierarchy.
- The Scene was reloaded a short time later and a new copy of it was made and a second event was registered.
- The callback was called when an ad finished and this event was broadcast to both copies of the GameObject, the real one (from the second scene load), and the ‘ghost’ one (which only persisted in C# memory).
- The ‘ghost’ GameObject tried to reference other GameObjects from the first time the scene was loaded which no longer existed and threw a NullReferenceException.
The Fix:
void OnDisable()
{
LuxelAds.OnAdClosed -= OnAdFinishedCallback;
}
Yup, just one line.
Best Practices for C# Events in Unity:
When using Events on Unity GameObjects, they should register their callbacks in the OnEnable() function, and unregister their callbacks in the OnDisable() function. The reason we do this in OnEnable and OnDisable (instead of Awake() and Destroy()) is so that code does not get executed on an Inactive GameObject (which probably leads to more debugging nightmares).