Skip to content

Unload scene specific resources on demand #7381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ViktorVovk
Copy link
Contributor

This is an implementation of functionality for unloading all resources from the scene we are leaving. For example, we have several scenes (Preloader, postPreloader) that contain a significant number of resources such as images, audio, spine animations, atlases, JSON files, and so on. After these scenes have been loaded, once we transition to the so-called main scene, we want to remove (unload) all resources from the scenes that we do not expect to return to during the game’s runtime.

Plan for implementing the unloading of scene-specific resources on the GDevelop side:

  1. An additional boolean parameter needs to be added to the "change scene" action. Enabling this parameter will indicate that all resources specific to the scene being exited should be unloaded. It is assumed that we will not return to this scene during the game’s runtime.

  2. During the execution of the "change scene" action, the _destroy method is eventually called on the scene object (in runtimescene.ts). At that point, the resource cleanup method disposeScene—implemented in the ResourceLoader class—is invoked.

  3. The disposeScene method accepts the scene name as a parameter and creates a Map where the keys are resource types (ResourceKind) and the values are arrays of ResourceData.

  4. In the ResourceLoader class, there is a field _resourceManagersMap where the keys are resource types (ResourceKind) and the values are resource managers. This makes it easy to correlate this map with the map of resources grouped by type since they share the same keys.

  5. In each resource manager class, a method disposeByResourcesList has been implemented, which takes an array of resources that need to be cleared. This method performs the cleanup of the resources in the provided list.

@ViktorVovk ViktorVovk marked this pull request as ready for review February 7, 2025 16:50
@ViktorVovk ViktorVovk requested a review from 4ian as a code owner February 7, 2025 16:50
@ViktorVovk ViktorVovk marked this pull request as draft February 11, 2025 20:25
@ViktorVovk ViktorVovk marked this pull request as ready for review February 13, 2025 10:01
const currentSceneName = currentScene.getName();
const newSceneName = currentScene.getRequestedScene();

if (this._sceneWasUnloadedResources.has(newSceneName)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a loading screen that is shown if a scene is not yet ready. We could probably rely on this instead of adding a special case? I.e: calling replace should call push that will display a loading screen if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored

@4ian
Copy link
Owner

4ian commented Mar 20, 2025

Hi @ViktorVovk!
Main feedback is that we think the parameter to choose if a scene resources should be unloaded when the scene is left should be a property of the scene, instead of being a parameter of the action.
I.e: something that would be stored on the scene (in Layout.h, and serialized into the JSON, so you could read it ).

(we could imagine an action to change this at runtime, but in most cases it could be something that never changes).

The unloading algorithm could unload resources that are neither from a living scene (including the next one) nor a scene with the "unload" property disabled.

Let's discuss what this would imply and how this would impact the algorithm that unloads resources :)

@ViktorVovk
Copy link
Contributor Author

Hi @4ian.
Thank you for taking the time to review this PR.
I agree with your comment regarding where the need to unload resources from the scene we’re leaving should be specified.
I’ve done some refactoring and moved this parameter into the scene’s properties.
By default, this parameter is set to false.

I would really appreciate it if you could take another look at this PR.
Best regards.

@ViktorVovk ViktorVovk requested a review from 4ian March 25, 2025 13:38
Copy link
Collaborator

@D8H D8H left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's going in the right direction, thanks! I've added a few more comments.

@@ -381,6 +393,7 @@ class GD_CORE_API Layout {
behaviorsSharedData; ///< Initial shared datas of behaviors
bool stopSoundsOnStartup = true; ///< True to make the scene stop all sounds at
///< startup.
bool unloadSceneAssets = false; ///< True to unload scene assets after exit scene
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool UnloadSceneAssets() sounds as a method that actually unloads the assets.
What do you think about shouldUnloadAssetsWhenUnloaded?

Suggested change
bool unloadSceneAssets = false; ///< True to unload scene assets after exit scene
bool shouldUnloadAssetsWhenUnloaded = false; ///< True to unload scene assets after exit scene

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. Thanks for you review. I totally agree with you on that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed!

Comment on lines +844 to +847
async loadSceneAssetsBySceneName(
sceneName: string,
progressCallback?: (progress: float) => void
): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is similar to gdjs.RuntimeGame.loadSceneAssets.
Did you face any issue using loadSceneAssets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I tried to understand and use the existing API, specifically RuntimeGame.loadSceneAssets. However, when I used it and then returned to a previously unloaded scene, the resources for that scene were not loaded again. Although the methods RuntimeGame.loadSceneAssets and RuntimeGame.loadSceneAssetsBySceneName seem quite similar, I assumed they perform different tasks, as they invoke different methods in the ResourceLoader.

RuntimeGame.loadSceneAssets calls this._resourcesLoader.loadAndProcessSceneResources, which in turn calls ResourceLoader.loadSceneResources. From my understanding, this method does not actually load the resources but rather is intended for scenarios when switching to a scene whose resources are not yet loaded, but already queued for loading.

In contrast, RuntimeGame.loadSceneAssetsBySceneName invokes this._resourcesLoader.loadSceneResourcesBySceneName, which explicitly loads resources of a previously unloaded scene.

As an external developer, and not fully immersed in the entire context of the project, I tried to alter or modify the existing codebase as minimally as possible, just to avoid breaking any existing functionality that I might not be aware of. I would greatly appreciate your help in understanding why I couldn’t successfully use RuntimeGame.loadSceneAssets. Perhaps there is something I overlooked.

@ViktorVovk ViktorVovk requested a review from D8H April 4, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants