[GDScript] Loading non-existing gdscript returns valid instance.

[zaevi]

Godot version

4.1.dev

System information

Win11

Issue description

I load a non-existing GDScript, but it still returns a valid instance instead of null.

@tool
extends EditorScript

func _run():
	var s = ResourceLoader.load("non_exists.gd", "", ResourceLoader.CACHE_MODE_IGNORE)
	print(s)
	assert(s == null)

  Attempt to open script 'res://non_exists.gd' resulted in error 'File not found'.
<GDScript#-9223369015982753437>
  res://new_script.gd:7 - Assertion failed.

Steps to reproduce

N/A

Minimal reproduction project

N/A

[zaevi]

godot/modules/gdscript/gdscript.cpp

Lines 2592 to 2610 in fbb1a92

	Ref<Resource> ResourceFormatLoaderGDScript::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) {
	if (r_error) {
	*r_error = ERR_FILE_CANT_OPEN;
	}
	Error err;
	Ref<GDScript> scr = GDScriptCache::get_full_script(p_path, err, "", p_cache_mode == CACHE_MODE_IGNORE);
	if (scr.is_null()) {
	// Don't fail loading because of parsing error.
	scr.instantiate();
	}
	if (r_error) {
	*r_error = OK;
	}
	return scr;
	}

Seems if not exist it will instantiate. cc @vnen

[Rindbee]

Currently, there doesn't seem to be a distinction between errors in parsing and errors in load_source_code(). When load_source_code() fails, GDScriptCache::get_shallow_script() should return Ref<GDScript>() without caching.

godot/modules/gdscript/gdscript_cache.cpp

Line 256 in fd4a06c

script->load_source_code(p_path);

[KoBeWi]

This leads to a bug where opening a scene with missing script dependencies will open them as empty scripts.

[anvilfolk]

I was looking at the linked PR and I'm not 100% it's the best approach. I think the real issue is that in C++, ResourceLoader::load() has a return parameter for the error errors, but you can't do that in GDScript ResourceLoader.load().

I suspect the right behavior is still to return the most-parsed/analyzed script possible, but still have access to an error of some sort... though I have no idea how we'd go about implementing that, unless we had some weird ResourceLoader.error(), which would attempt the load (possibly cache so it's fast to grab the resource after), but then return the error instead of the resource 🙃