Godot: Make preload invalid paths null instead of a compilation error

Created on 14 Nov 2016  Â·  17Comments  Â·  Source: godotengine/godot

Operating system or device - Godot version:
Windows 10
Godot 2.1 stable

Issue description:
The current behavior of preload with invalid paths is give an error in editor (Can't preload resource at path) and crash at runtime.
xh5vlyr 1
A better approach could be instead of give an error in editor and crash at runtime, preload could return null and let the user check ingame if the preloaded var/const is null.

This could be useful for addons, for example I have an addon which has a timer created by script for delayed triggers and I wan't the trigger addon to be independant from the project, but for this project the timer could be paused with a player ability, so I need a way to attach that script for the timer in the trigger. The easiest and optimal way to achieve this could be preload const and check if the script path is null. So with this I could use the addon without major modifications in another project or I could put another timer script at the path for custom behaviour in a new project.

NOTE: I'm aware of load and check/load the path in ready:

const _timer_script_path = "res://resources/scripts/timer.gd"

func _ready():
    var timer = Timer.new()
    var script = load(_timer_script_path)
    if(script != null):
        timer .set_script(script)

But that method should be less optimal because is 1 load for EACH ready vs a SINGLE preload in a CONST at start of the game

Steps to reproduce:
Create a script and try to preload an invalid resource with a const and run the game:
const _script = preload("res://resources/scripts/timer.gd")

archived discussion

Most helpful comment

Crash (parse error actually) is fine, this is meant to always work. It
would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

On Mon, Nov 14, 2016 at 5:49 PM, Marc [email protected] wrote:

I agree a crash is not great. Your proposal should be fine if it doesn't
implies a runtime cost (besides initial load).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260458165,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z28rXhNi8w45Eh-9cs7y4uSKDqr2Lks5q-MlZgaJpZM4Kxtpp
.

All 17 comments

I agree a crash is not great. Your proposal should be fine if it doesn't implies a runtime cost (besides initial load).

Crash (parse error actually) is fine, this is meant to always work. It
would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

On Mon, Nov 14, 2016 at 5:49 PM, Marc [email protected] wrote:

I agree a crash is not great. Your proposal should be fine if it doesn't
implies a runtime cost (besides initial load).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260458165,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z28rXhNi8w45Eh-9cs7y4uSKDqr2Lks5q-MlZgaJpZM4Kxtpp
.

If you are not sure whether it should work or not, use load() instead.

On Mon, Nov 14, 2016 at 6:29 PM, Juan Linietsky [email protected] wrote:

Crash (parse error actually) is fine, this is meant to always work. It
would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

On Mon, Nov 14, 2016 at 5:49 PM, Marc [email protected] wrote:

I agree a crash is not great. Your proposal should be fine if it doesn't
implies a runtime cost (besides initial load).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260458165,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z28rXhNi8w45Eh-9cs7y4uSKDqr2Lks5q-MlZgaJpZM4Kxtpp
.

Crash (parse error actually) is fine, this is meant to always work.

The app should still crash if the user doesn't handle the null preloaded var/const with the approach in my post.

It would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

With load the user could mess up the name and nobody complained about that, this is the same behavior suggested for preload in this issue. The advantage of nullable preloads is custom behaviour for preload paths not found instead of "I can't compile with an invalid path".

If you are not sure whether it should work or not, use load() instead.

Well as I said in first post I'm aware of load, but that method should be less optimal because is 1 load for EACH ready vs a SINGLE preload in a CONST at start of the game.

you can run load in an initializer fine:

var someobj = load(pathtores)

On Mon, Nov 14, 2016 at 7:36 PM, Hearto [email protected] wrote:

Crash (parse error actually) is fine, this is meant to always work.

The app should still crash if the user doesn't handle the null preloaded
var/const with the approach in my post.

It would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

With load the user could mess up the name and nobody complained about
that, this is the same behavior suggested for preload in this issue. The
advantage of nullable preloads is custom behaviour for preload paths not
found instead of I can't compile with an invalid path.

If you are not sure whether it should work or not, use load() instead.

Well as I said in first post I'm aware of load, but that method should be
less optimal because is 1 load for EACH ready vs a SINGLE preload in a
CONST at start of the game.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260485937,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z20rIpdpIMFF5KFK3isQocq_Q0iuZks5q-OJcgaJpZM4Kxtpp
.

6203 mentions some of the things I ran into with load/preload, might be worth bearing in mind?

Preload is supposed to be a static dependency, if it doesn't exist, it's
the same as a .tscn mentioning an external resource that doesn't exist.. It
should abort as if the game data is incorrect

On 14 November 2016 at 18:30, Juan Linietsky [email protected]
wrote:

If you are not sure whether it should work or not, use load() instead.

On Mon, Nov 14, 2016 at 6:29 PM, Juan Linietsky [email protected] wrote:

Crash (parse error actually) is fine, this is meant to always work. It
would be worse if you mess up the name, it doesn t work and you dont
realize you made a mistake.

On Mon, Nov 14, 2016 at 5:49 PM, Marc [email protected] wrote:

I agree a crash is not great. Your proposal should be fine if it doesn't
implies a runtime cost (besides initial load).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
issuecomment-260458165>,
or mute the thread
9cs7y4uSKDqr2Lks5q-MlZgaJpZM4Kxtpp>
.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260468804,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGVmPQfSEFpuuexYxIjcPtFkdtCTlESjks5q-NMHgaJpZM4Kxtpp
.

you can run load in an initializer fine:

var someobj = load(pathtores)

What about CONST values?
At least you can't load in a CONST obj

what does it change for you whether it s a const value or not?
the only difference it makes to gdscript is that it can do compile time
optimization.

On Mon, Nov 14, 2016 at 7:54 PM, Hearto [email protected] wrote:

you can run load in an initializer fine:

var someobj = load(pathtores)

What about CONST values?
At least you can't load in a CONST obj

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7118#issuecomment-260490538,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z27RAUX1T4KNMXqNqBN9LNlbP-KeMks5q-Oa-gaJpZM4Kxtpp
.

AFAIK resources are cached, so a second load() will grab the same resource from memory, not load from the filesystem again.

_If_ it does, the resource doesn't have the same ID.

preload expects a const String. If that const String points to an invalid path, that's a bug from whoever wrote the string, and it's normal that the program gives an error.

You can't use preload to load resources dynamically, that's load's job, so it's definitely what you want to use in your use case. It makes no sense to have preload return a null object if passed a bogus path. It should only return a null object if it was called on a valid path but an invalid resource (i.e. it couldn't load the file because it's not a resource Godot knows how to handle), not if the programmer made a typo.

If it does, the resource doesn't have the same ID.

How is that a problem? As long as each ID points to the same allocated memory, that's AFAIK the way it's intended to be. Resources are reference-counted - the allocated memory will live until all references are freed.

Agree with Akien. Also I prefer a compile error than a successful compilation but a hidden bug.

what does it change for you whether it s a const value or not?
the only difference it makes to gdscript is that it can do compile time
optimization.

Well in theory with CONST the value should be executed 1 time for each script and with var the value should be executed for each instance of the script.
Now that sound less optimal but taking in account this reply from @vnen :

AFAIK resources are cached, so a second load() will grab the same resource from memory, not load from the filesystem again.

With that info var xxx = load() should be the same performance wise as const xxx = preload(). Of course if the cache algorithm isn't limited (something like using a dictionary with a limit of 100 last values)
So this issue could be closed if the performance is the same in both cases.

Well in theory with CONST the value should be executed 1 time for each script and with var the value should be executed for each instance of the script.

That's true, but if it really, really matters, you can make a global singleton holding those variables and doing whatever you want with the resources just once.

@akien-mga Close this?

Was this page helpful?
0 / 5 - 0 ratings