Skip to content

Commit

Permalink
Fix crash with user-defined ResourceFormatLoader.load
Browse files Browse the repository at this point in the history
There's still some fishy recursive relationship between `load_interactive` and
`load` which needs to be investigated here, but this patch solves the crash
when returning an error code in user-defined `load`.

Fixes godotengine#48463.
  • Loading branch information
akien-mga committed May 5, 2021
1 parent 6194824 commit bf9f288
Showing 1 changed file with 21 additions and 16 deletions.
37 changes: 21 additions & 16 deletions core/io/resource_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,6 @@ class ResourceInteractiveLoaderDefault : public ResourceInteractiveLoader {
ResourceInteractiveLoaderDefault() {}
};

Ref<ResourceInteractiveLoader> ResourceFormatLoader::load_interactive(const String &p_path, const String &p_original_path, Error *r_error) {
//either this
Ref<Resource> res = load(p_path, p_original_path, r_error);
if (res.is_null()) {
return Ref<ResourceInteractiveLoader>();
}

Ref<ResourceInteractiveLoaderDefault> ril = Ref<ResourceInteractiveLoaderDefault>(memnew(ResourceInteractiveLoaderDefault));
ril->resource = res;
return ril;
}

bool ResourceFormatLoader::exists(const String &p_path) const {
return FileAccess::exists(p_path); //by default just check file
}
Expand All @@ -160,24 +148,41 @@ void ResourceFormatLoader::get_recognized_extensions(List<String> *p_extensions)
}
}

// Warning: Derived classes must override either `load` or `load_interactive`. The base code
// here can trigger an infinite recursion otherwise, since `load` calls `load_interactive`
// vice versa.

Ref<ResourceInteractiveLoader> ResourceFormatLoader::load_interactive(const String &p_path, const String &p_original_path, Error *r_error) {
// Warning: See previous note about the risk of infinite recursion.
Ref<Resource> res = load(p_path, p_original_path, r_error);
if (res.is_null()) {
return Ref<ResourceInteractiveLoader>();
}

Ref<ResourceInteractiveLoaderDefault> ril = Ref<ResourceInteractiveLoaderDefault>(memnew(ResourceInteractiveLoaderDefault));
ril->resource = res;
return ril;
}

RES ResourceFormatLoader::load(const String &p_path, const String &p_original_path, Error *r_error) {
// Check user-defined loader if there's any. Hard fail if it returns an error.
if (get_script_instance() && get_script_instance()->has_method("load")) {
Variant res = get_script_instance()->call("load", p_path, p_original_path);

if (res.get_type() == Variant::INT) {
if (res.get_type() == Variant::INT) { // Error code, abort.
if (r_error) {
*r_error = (Error)res.operator int64_t();
}

} else {
return RES();
} else { // Success, pass on result.
if (r_error) {
*r_error = OK;
}
return res;
}
}

//or this must be implemented
// Warning: See previous note about the risk of infinite recursion.
Ref<ResourceInteractiveLoader> ril = load_interactive(p_path, p_original_path, r_error);
if (!ril.is_valid()) {
return RES();
Expand Down

0 comments on commit bf9f288

Please sign in to comment.