Fix GDScript load with relative path

[RedMser]

Supersedes #37324
Fixes #35832

First time touching GDScript internals, hope I didn't mess anything up!

As suggested in a comment in the previous PR, all utility function calls now pass the script reference, which is used for figuring out the absolute path.
Also by using get_path() instead of path, this PR supports built-in scripts as well.

Open Question:
I'm currently passing the analyzer a nullptr script reference. While ResourceLoader::load(parser->script_path, "Script") is possible, I don't understand yet what the "benefits" are, since I don't understand the analyzer's purpose beyond type checking.

[adamscott]

Could you add a unit test to the PR? I would hope to merge this PR for the 4.1 release, but time is running out.
If you need more info, don't hesitate to chat us in #gdscript.

[RedMser]

@adamscott Added some tests. It seems like type inference does not work, since it's emitting warnings. Probably due to me leaving out the analyzer.

I don't really have time to figure this stuff out, so if you or another GDScript dev knows more about this, please fix up my commit or attach a diff that fixes this part, if it's urgent to land it.

---

EDIT: I confused load with preload - the former always returns Resource so I assume this is OK. I also realize that ResourceLoader::load(parser->script_path, "Script") would be possible, but I have no idea if this is correct -or- what is even the reason I'd want to do this 🙃

[adamscott]

The PR seems good to me. OK for 4.1!

[akien-mga]

We usually do const Ref<T> & where suitable, would that work here?

[dalexeev]

Might it be better to resolve this in the compiler and VM (with a separate opcode to convert relative paths, if the argument is unknown at compile time) than to construct a Ref<GDScript> on each utility function call? I do not think that your option is significantly more expensive, just mention an alternative solution.

[adamscott]

Added the PR to the PRs to review by the GDScript team.

[vnen]

I do think this is overkill to solve this particular issue. Adding an argument to every function just because one needs it looks like too much.

We can discuss other solutions to this.

For instance, we could have a second, optional argument to load that works as the base for relative paths and at compile time we can insert this (if not explicitly set). So relative paths would work even if generated dynamically in the user script.

If dynamic is not a necessity, we could detect relative paths at runtime and prepend the script's base directory to it so it becomes an absolute path.

I think an opcode as @dalexeev suggested is also too much, but we could generate the logic with a combination of opcodes (it does increase the overhead of calling load though).

In any case, I do believe we can find a more localized solution rather than changing all utility functions.

[dalexeev]

I think an opcode as @dalexeev suggested is also too much, but we could generate the logic with a combination of opcodes (it does increase the overhead of calling load though).

Note that if the load() argument is a constant expression, then the path is resolved in the compiler and the opcode is not generated. (Which is quite common in my opinion, users may still use load() instead of preload(), even for constant expressions.)

This only affects variable expressions such as load("path/" + file) or load("path/%s" % file). In theory, we could optimize some of these cases too, as long as the prefix is known at compile time.

[RedMser]

I'll leave a more optimized implementation to someone who has more experience with GDScript internals. Thanks for the feedback!

[dalexeev]

@RedMser Thanks for your contribution nonetheless!