Skip to content

Commit

Permalink
Merge pull request #93346 from RandomShaper/gds_cyc_alt
Browse files Browse the repository at this point in the history
GDScript: Enhance handling of cyclic dependencies
  • Loading branch information
akien-mga committed Jun 26, 2024
2 parents 3d8562d + c139148 commit 0364443
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 58 deletions.
68 changes: 39 additions & 29 deletions core/io/resource_format_binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -749,44 +749,54 @@ Error ResourceLoaderBinary::load() {
String t = get_unicode_string();

Ref<Resource> res;
Resource *r = nullptr;

if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && ResourceCache::has(path)) {
//use the existing one
Ref<Resource> cached = ResourceCache::get_ref(path);
if (cached->get_class() == t) {
cached->reset_state();
res = cached;
}
MissingResource *missing_resource = nullptr;

if (main) {
res = ResourceLoader::get_resource_ref_override(local_path);
r = res.ptr();
}
if (!r) {
if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && ResourceCache::has(path)) {
//use the existing one
Ref<Resource> cached = ResourceCache::get_ref(path);
if (cached->get_class() == t) {
cached->reset_state();
res = cached;
}
}

MissingResource *missing_resource = nullptr;
if (res.is_null()) {
//did not replace

Object *obj = ClassDB::instantiate(t);
if (!obj) {
if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
//create a missing resource
missing_resource = memnew(MissingResource);
missing_resource->set_original_class(t);
missing_resource->set_recording_properties(true);
obj = missing_resource;
} else {
error = ERR_FILE_CORRUPT;
ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource of unrecognized type in file: " + t + ".");
}
}

if (res.is_null()) {
//did not replace

Object *obj = ClassDB::instantiate(t);
if (!obj) {
if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
//create a missing resource
missing_resource = memnew(MissingResource);
missing_resource->set_original_class(t);
missing_resource->set_recording_properties(true);
obj = missing_resource;
} else {
r = Object::cast_to<Resource>(obj);
if (!r) {
String obj_class = obj->get_class();
error = ERR_FILE_CORRUPT;
ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource of unrecognized type in file: " + t + ".");
memdelete(obj); //bye
ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource type in resource field not a resource, type is: " + obj_class + ".");
}
}

Resource *r = Object::cast_to<Resource>(obj);
if (!r) {
String obj_class = obj->get_class();
error = ERR_FILE_CORRUPT;
memdelete(obj); //bye
ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource type in resource field not a resource, type is: " + obj_class + ".");
res = Ref<Resource>(r);
}
}

res = Ref<Resource>(r);
if (r) {
if (!path.is_empty()) {
if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
r->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); // If got here because the resource with same path has different type, replace it.
Expand Down
36 changes: 36 additions & 0 deletions core/io/resource_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ Ref<Resource> ResourceLoader::_load(const String &p_path, const String &p_origin
}

load_paths_stack->resize(load_paths_stack->size() - 1);
res_ref_overrides.erase(load_nesting);
load_nesting--;

if (!res.is_null()) {
Expand Down Expand Up @@ -730,6 +731,40 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
}
}

Ref<Resource> ResourceLoader::ensure_resource_ref_override_for_outer_load(const String &p_path, const String &p_res_type) {
ERR_FAIL_COND_V(load_nesting == 0, Ref<Resource>()); // It makes no sense to use this from nesting level 0.
const String &local_path = _validate_local_path(p_path);
HashMap<String, Ref<Resource>> &overrides = res_ref_overrides[load_nesting - 1];
HashMap<String, Ref<Resource>>::Iterator E = overrides.find(local_path);
if (E) {
return E->value;
} else {
Object *obj = ClassDB::instantiate(p_res_type);
ERR_FAIL_NULL_V(obj, Ref<Resource>());
Ref<Resource> res(obj);
if (!res.is_valid()) {
memdelete(obj);
ERR_FAIL_V(Ref<Resource>());
}
overrides[local_path] = res;
return res;
}
}

Ref<Resource> ResourceLoader::get_resource_ref_override(const String &p_path) {
DEV_ASSERT(p_path == _validate_local_path(p_path));
HashMap<int, HashMap<String, Ref<Resource>>>::Iterator E = res_ref_overrides.find(load_nesting);
if (!E) {
return nullptr;
}
HashMap<String, Ref<Resource>>::Iterator F = E->value.find(p_path);
if (!F) {
return nullptr;
}

return F->value;
}

bool ResourceLoader::exists(const String &p_path, const String &p_type_hint) {
String local_path = _validate_local_path(p_path);

Expand Down Expand Up @@ -1222,6 +1257,7 @@ bool ResourceLoader::timestamp_on_load = false;
thread_local int ResourceLoader::load_nesting = 0;
thread_local WorkerThreadPool::TaskID ResourceLoader::caller_task_id = 0;
thread_local Vector<String> *ResourceLoader::load_paths_stack;
thread_local HashMap<int, HashMap<String, Ref<Resource>>> ResourceLoader::res_ref_overrides;

template <>
thread_local uint32_t SafeBinaryMutex<ResourceLoader::BINARY_MUTEX_TAG>::count = 0;
Expand Down
4 changes: 4 additions & 0 deletions core/io/resource_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ class ResourceLoader {

static thread_local int load_nesting;
static thread_local WorkerThreadPool::TaskID caller_task_id;
static thread_local HashMap<int, HashMap<String, Ref<Resource>>> res_ref_overrides; // Outermost key is nesting level.
static thread_local Vector<String> *load_paths_stack; // A pointer to avoid broken TLS implementations from double-running the destructor.
static SafeBinaryMutex<BINARY_MUTEX_TAG> thread_load_mutex;
static HashMap<String, ThreadLoadTask> thread_load_tasks;
Expand Down Expand Up @@ -272,6 +273,9 @@ class ResourceLoader {
static void set_create_missing_resources_if_class_unavailable(bool p_enable);
_FORCE_INLINE_ static bool is_creating_missing_resources_if_class_unavailable_enabled() { return create_missing_resources_if_class_unavailable; }

static Ref<Resource> ensure_resource_ref_override_for_outer_load(const String &p_path, const String &p_res_type);
static Ref<Resource> get_resource_ref_override(const String &p_path);

static bool is_cleaning_tasks();

static void initialize();
Expand Down
9 changes: 7 additions & 2 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4299,15 +4299,20 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {

// Must load GDScript separately to permit cyclic references
// as ResourceLoader::load() detects and rejects those.
if (ResourceLoader::get_resource_type(p_preload->resolved_path) == "GDScript") {
const String &res_type = ResourceLoader::get_resource_type(p_preload->resolved_path);
if (res_type == "GDScript") {
Error err = OK;
Ref<GDScript> res = GDScriptCache::get_shallow_script(p_preload->resolved_path, err, parser->script_path);
p_preload->resource = res;
if (err != OK) {
push_error(vformat(R"(Could not preload resource script "%s".)", p_preload->resolved_path), p_preload->path);
}
} else {
p_preload->resource = ResourceLoader::load(p_preload->resolved_path);
Error err = OK;
p_preload->resource = ResourceLoader::load(p_preload->resolved_path, res_type, ResourceFormatLoader::CACHE_MODE_REUSE, &err);
if (err == ERR_BUSY) {
p_preload->resource = ResourceLoader::ensure_resource_ref_override_for_outer_load(p_preload->resolved_path, res_type);
}
if (p_preload->resource.is_null()) {
push_error(vformat(R"(Could not preload resource file "%s".)", p_preload->resolved_path), p_preload->path);
}
Expand Down
59 changes: 32 additions & 27 deletions scene/resources/resource_format_text.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,10 @@ Error ResourceLoaderText::_parse_ext_resource(VariantParser::Stream *p_stream, R
}

Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourceParser &parser) {
Ref<PackedScene> packed_scene;
packed_scene.instantiate();
Ref<PackedScene> packed_scene = ResourceLoader::get_resource_ref_override(local_path);
if (packed_scene.is_null()) {
packed_scene.instantiate();
}

while (true) {
if (next_tag.name == "node") {
Expand Down Expand Up @@ -664,39 +666,42 @@ Error ResourceLoaderText::load() {
return error;
}

Ref<Resource> cache = ResourceCache::get_ref(local_path);
if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && cache.is_valid() && cache->get_class() == res_type) {
cache->reset_state();
resource = cache;
}

MissingResource *missing_resource = nullptr;

if (!resource.is_valid()) {
Object *obj = ClassDB::instantiate(res_type);
if (!obj) {
if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
missing_resource = memnew(MissingResource);
missing_resource->set_original_class(res_type);
missing_resource->set_recording_properties(true);
obj = missing_resource;
} else {
error_text += "Can't create sub resource of type: " + res_type;
resource = ResourceLoader::get_resource_ref_override(local_path);
if (resource.is_null()) {
Ref<Resource> cache = ResourceCache::get_ref(local_path);
if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && cache.is_valid() && cache->get_class() == res_type) {
cache->reset_state();
resource = cache;
}

if (!resource.is_valid()) {
Object *obj = ClassDB::instantiate(res_type);
if (!obj) {
if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
missing_resource = memnew(MissingResource);
missing_resource->set_original_class(res_type);
missing_resource->set_recording_properties(true);
obj = missing_resource;
} else {
error_text += "Can't create sub resource of type: " + res_type;
_printerr();
error = ERR_FILE_CORRUPT;
return error;
}
}

Resource *r = Object::cast_to<Resource>(obj);
if (!r) {
error_text += "Can't create sub resource of type, because not a resource: " + res_type;
_printerr();
error = ERR_FILE_CORRUPT;
return error;
}
}

Resource *r = Object::cast_to<Resource>(obj);
if (!r) {
error_text += "Can't create sub resource of type, because not a resource: " + res_type;
_printerr();
error = ERR_FILE_CORRUPT;
return error;
resource = Ref<Resource>(r);
}

resource = Ref<Resource>(r);
}

Dictionary missing_resource_properties;
Expand Down

0 comments on commit 0364443

Please sign in to comment.