✨ Add support to call `Dispose` on scenes when removing them from the `SceneManager`

[mathew-odwyer]

Complete The Items Below

• [X] I have updated the title without removing the ✨ emoji.
• [X] I searched whether or not a feature request already exists.

Feature Request Purpose

SceneBase implements IDisposable - this is great!

However, when removing a scene, I originally assumed that since the UnloadContent method was going to be invoked that by default, the scene would be disposed. Further consideration made me realize that this might be unwanted behaviour and generally the rule of thumb should be that the end-user is responsible for disposing of resources manually by calling Dispose.

However, my hopes are to simply just create a transient scene, and once it's removed from the SceneManager it's resources can be disposed (if that's what I intended to do).

Solution

I have three solutions:

1. Add a boolean to ISceneManager named AutoSceneDisposal following the trend in Window - if the flag is set to true, we simply call Dispose on the scene being removed prior to removal. The flag should be set to false by default so as not to introduce any breaking changes.

SceneBase? sceneToRemove = this.scenes[sceneBeingRemovedIndex].scene;

// Unload content as usual.
scene?.UnloadContent();

if (AutoSceneDisposal)
{
    scene?.Dispose();
}

2. Add a optional parameter to RemoveScene(Guid id) promptly named isDisposing and set it to false by default so as not to introduce any breaking changes (this one likely gives the user the most control)

public void RemoveScene(Guid id, bool isDisposing = false)
{
    SceneBase? sceneToRemove = this.scenes[sceneBeingRemovedIndex].scene;

    // Unload content as usual.
    scene?.UnloadContent();

    if (isDisposing)
    {
        scene?.Dispose();
    }
}

3. Make RemoveScene(Guid id) return a SceneBase referring to the scene that was removed for the user to call Dispose(). I don't like this approach personally but we could do both 2 and 3 to give the users more control of scene removal as a whole.

public SceneBase RemoveScene(Guid id, bool isDisposing = false)
{
    SceneBase? sceneToRemove = this.scenes[sceneBeingRemovedIndex].scene;

    // Unload content as usual.
    scene?.UnloadContent();

    if (isDisposing)
    {
        scene?.Dispose();
    }

    return scene;
}

Anything Else

I really do believe this feature is necessary ultimately the SceneManager should manage scenes - it also gives the user flexibility and enables them to write less boilerplate code.

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[CalvinWilkinson]

@softwareantics Thanks for bringing this to light!!

I was digging into the code and realized something. Currently, the scene base has a virtual UnloadContent() which is used to dispose of the content. (At least the act of unloading is partially a disposing operation except for one subtle detail.)

I like your first proposal the most and also do like the idea of doing auto disposal. The idea of Velaptor is to cover 90% of everything out of the box and make it super easy for people to use it. When they want more control though, Velaptor should give users the enable and disable CERTAIN features and also the ability to write custom implementations.

But we also have virtual Dispose(). Inside of the base Dispose(), it is indeed marking the scene as disposed of, appropriately following the disposable pattern. But I noticed some things out of place.

1. We need to call Unload() from the Dispose() method. This will provide the ability to unload the user's content in the scene upon disposal due to the user's implementation of the virtual UnloadContent().
2. Internally in the SceneBase, go to other operations and check if the object is disposed and if it is, throw the ObjectDisposedException
3. Maintain the virtual nature of both the UnloadContent() and Dispose() methods to give the user control
4. Currently, shutting the game down automatically will unload each scene. But, it should also dispose of it as well which it is currently not doing.
5. I noticed that in the SceneManager, there is no ability to NOT unload and load the next or previous scene when moving to different scenes. The intent a long time ago was to give the user the ability to move to the next scene without unloading the content of the current scene. The reason for this would be about giving the user control as well as performance. If this feature existed, then the user could move to a scene without having the RELOAD the content all over again, making the scene transition really fast at the cost of memory. This was the intent from the beginning but just simply has not been done yet.

So the intent here is this.

1. To unload content, the user can invoke UnloadContent(), and the content will indeed be unloaded via the user's implementation. Then, the user could reuse the scene and the content would be reloaded using the UnloadContent() implementation.
2. If the user chooses to unload the content and never use the scene instance again, this is where disposal comes in. Since the disposing of the scene will automatically invoke the UnloadContent() method, this will also make sure that content has been unloaded and cleaned up.

@softwareantics I am interested in what you think. I am most likely going to implement some of these things or at least variations of them. Since it is about scene management, I will be putting some priority on them as well.

[stale[bot]]

This issue has been automatically marked as stale due to the lack of activity for 60 days. The issue will be closed after 7 days if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This stale issue has been closed due to a lack of activity.