godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Detect cases where `load()` is used in a suboptimal manner and print a warning #3159

Open Calinou opened 3 years ago

Calinou commented 3 years ago

Describe the project you are working on

The Godot editor :slightly_smiling_face:

Describe the problem or limitation you are having in your project

Many people run into avoidable stuttering at run-time because load() has been used instead of preload(). See https://www.reddit.com/r/godot/comments/p6x9rh/performance_test_how_many_enemies_can_we_put_on/ for a recent example of this.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Detect cases where load() is used in a suboptimal manner and print a warning that recommends to use preload() instead.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This can be done by printing a warning message if the same string is passed to load() more than N times during a single gameplay session. If a different string is passed, we assume that different resources are being loaded or that a "dynamic" string is used instead.

By default, N would be a number like 10, but it can be adjusted in the project settings or set to 0 to disable the warning entirely.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, as load()'s behavior cannot be overriden by a script. You could have your own static function in a singleton that handles this, but that's not very convenient and new users will not use this naturally.

Is there a reason why this should be core and not an add-on in the asset library?

See above.

KoBeWi commented 3 years ago

AFAIK, load() will load a resource only if it isn't loaded, e.g. when you load a Texture and assign it to some node, as long as that Node exists (and contains the texture), subsequent loads() will use the already loaded resource instead of loading it again.

In case of scenes, the PackedScene resource is loaded again, but any external resource in that scene won't be reloaded if an instance exists.

What I mean is that using multiple loads() on the same path doesn't mean it's suboptimal, so it's not that easy to detect (unless we can live with false positives).

Xrayez commented 3 years ago

AFAIK, load() will load a resource only if it isn't loaded, e.g. when you load a Texture and assign it to some node, as long as that Node exists (and contains the texture), subsequent loads() will use the already loaded resource instead of loading it again.

Indeed, I think it's just a documentation issue. Some people don't like warnings polluting the output either, and may prefer to use load() everywhere when prototyping anyways, so it's not always the case that load() would be used suboptimally, when it's just a deliberate decision not to do premature optimizations.