Search Unity

Bug:: UIMenuManager:: Throws NullReferenceException on game exit

Discussion in 'Open Projects' started by paciFIST_Studios, Jul 17, 2021.

  1. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    https://github.com/UnityTechnologies/open-project-1/issues/478

    I'm starting to look into this, but I don't really know my way around the codebase. I've watched the "Game architecture with Scriptable Objects" video, and the "Better Data with Scriptable Objects" video.

    Code (csharp):
    1. // Type
    2. VoidEventChannelSO _onGameExitEvent;
    3. // Called in UIMenuManager::HideExitConfirmationPopup(bool)
    4. _onGameExitEvent.OnEventRaised();
    It looks like all VoidEventChannelSO objects are meant to have a member variable of type UnityAction, called OnEventRaised.

    In this case, the OnEventRaised variable is already null, by the time we're trying to exit the game.

    Similar variables of type VoidEventChannelSO, do seem to have UnityActions associated with them. (_startNewGameEvent and _continueGameEvent).

    My assumption is: something is broken in the initialization step for these objects and/or, there isn't a UnityAction defined for _onGameExitEvent.
     
  2. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    Okay! Made some progress. It's more than one bug.

    Firstly, the code was attempting to call a variable as a function--and I think this was actually successful, because the variable is a UnityAction (not sure if this is true).

    The fixed code should read:
    Code (CSharp):
    1. // UIMenuManager::HideExitConfirmationPopup(bool)
    2. if (quitConfirmed)
    3. {
    4.     Application.Quit();
    5.     //_onGameExitEvent.OnEventRaised(); // <-- this is the name of the variable
    6.     _onGameExitEvent.RaiseEvent();
    7. }
    I made this change locally, and the null guard in VoidEventChannelSO works as expected.

    However, now I can't exit the game via the menu, and I think it's because _onGameExitEvent already has a null UnityAction. I still don't know why that would be the case.
     
    Last edited: Jul 17, 2021
  3. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    So, I am stumped. This variable _onGameExitEvent has a null member, and it shouldn't, but similar variables are set up and used in the same way, and those are fine.

    Anyone have an idea of where I should look, or if this happens on anyone else's machine?
     
  4. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    Okay, so I was just listening to the "Game Architecture with Scriptable Objects" devlog again, and it said if an event is invoked without any subscribers, we get a null-reference exception, so it sounds like the most likely culprit at this point, is: nothing is subscribed to _onGameExitEvent yet, and that's why it's null
     
  5. Smurjo

    Smurjo

    Joined:
    Dec 25, 2019
    Posts:
    296
    I have learned here and sucessfuly applied it over and over to initialize the events with an empty delegate, this way nothing (in the sense of no error) happens in case nobody has "subscribed" to the event. Especially at the application start there might be situations when an event is raised but no listener has subscribed yet.

    I would have a look if there is actually anybody subscribing to the event - else nothing happens in the sense of functionality (I could imagine an auto save function or scriptable object that have subscribed to events unsubscribing). Since Scriptable Object somehow continue existing after the application execution has ended, I have also observed problems, when a Scriptable Object doesn't unsubscribe from an event it has subscribed to. Those problems don't arise at the first time running the application, but at the second time.
     
  6. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    @Smurjo, thank you. This is my understanding of your comment:

    1. Programming practice: Initialize variables with default values; event channels should be initialized with empty delegates
      • That is sensible
    2. Check for subscribers, to see if anything is actually subscribed to the _onGameExitEvent
      • That is sensible, I will check.
    3. It is important for all ScriptableObject event subscribers to un-subscribe when they are done.
      1. Not-unsubscribing can cause problems
        1. ScriptableObject objects keep changes made at runtime
        2. MonoBehaviour objects do not keep changes made at runtime
    • This seems good to remember, thank you.
    Thank you for your help.
     
  7. paciFIST_Studios

    paciFIST_Studios

    Joined:
    May 14, 2017
    Posts:
    14
    Summary:

    1. There was a minor bug, there is a 1-line fix for it. The bug was raising NullReferenceExceptions. I have submitted a Pull Request for a fix.

    PR: https://github.com/UnityTechnologies/open-project-1/pull/480



    2. The editor will not exit play mode if you call Application.Quit. This is by design. I will submit a different ticket for a modification, which will cause the editor to stop play mode, if you exit out of the game from the main menu
     
  8. ChemaDmk

    ChemaDmk

    Unity Technologies

    Joined:
    Jan 14, 2020
    Posts:
    65
    Hey @paciFIST_Studios !
    Thank you for catching this error.

    Your fix it is correct :
    It should indeed be _onGameExitEvent.RaiseEvent(); not _onGameExitEvent.OnEventRaised();

    When checking the code, like you, I noticed this event doesn't have any subscribers.
    Digging further, I found out it's not needed anymore: The purpose for this event was to inform the Save System to save the game information.
    The way it's currently done, game information is always saved when the player is in MainMenu. So no need to inform the Save System about anything.

    I pushed a hot fix : Removing the event call, the reference from the UI script and even the event asset from the project. That way, if it's really referenced somewhere else, it will be easier to catch.