From d2093232bdb428f477d0bc4090950f763708ceed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 17 Dec 2024 11:33:35 +0100 Subject: [PATCH] Overhaul `Resource::duplicate()` - Thanks to a refactor, `duplicate_for_local_scene()` and `duplicate()` are now both users of the same, parametrized, implementation - `duplicate()` now honors with/without subresources modes better: - With subresources: Previously, only resources found as direct property values of the one to copy would be, recursively, duplicated. Now, in addition, arrays and dictionaries are walked so the copy is truly deep. - Without subresources: Previously, the first level of resources found as direct property values would be duplicated unconditionally. Now, no subresource found at any level in any form, will be duplicated, but the original reference kept instead. --- core/io/resource.cpp | 97 ++++++++++++++++---------------------------- core/io/resource.h | 8 +++- 2 files changed, 41 insertions(+), 64 deletions(-) diff --git a/core/io/resource.cpp b/core/io/resource.cpp index c99ed14f7dcb..a8c47aa12058 100644 --- a/core/io/resource.cpp +++ b/core/io/resource.cpp @@ -242,16 +242,16 @@ void Resource::reload_from_file() { copy_from(s); } -Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant, Node *p_for_scene, HashMap, Ref> &p_remap_cache) { +Variant Resource::_duplicate_recursive_impl(const Variant &p_variant, const DuplicateParams &p_params) { switch (p_variant.get_type()) { case Variant::OBJECT: { const Ref &sr = p_variant; if (sr.is_valid() && sr->is_local_to_scene()) { - if (p_remap_cache.has(sr)) { - return p_remap_cache[sr]; + if (p_params.remap_cache->has(sr)) { + return p_params.remap_cache->get(sr); } else { - const Ref &dupe = sr->duplicate_for_local_scene(p_for_scene, p_remap_cache); - p_remap_cache[sr] = dupe; + const Ref &dupe = sr->_duplicate_impl(p_params); + p_params.remap_cache->insert(sr, dupe); return dupe; } } else { @@ -263,7 +263,7 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant, Array dst; dst.resize(src.size()); for (int i = 0; i < src.size(); i++) { - dst[i] = _duplicate_recursive_for_local_scene(src[i], p_for_scene, p_remap_cache); + dst[i] = _duplicate_recursive_impl(src[i], p_params); } return dst; } break; @@ -275,8 +275,8 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant, for (const Variant &k : keys) { const Variant &v = src[k]; dst.set( - _duplicate_recursive_for_local_scene(k, p_for_scene, p_remap_cache), - _duplicate_recursive_for_local_scene(v, p_for_scene, p_remap_cache)); + _duplicate_recursive_impl(k, p_params), + _duplicate_recursive_impl(v, p_params)); } return dst; } break; @@ -286,16 +286,18 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant, } } -Ref Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap, Ref> &p_remap_cache) { +Ref Resource::_duplicate_impl(const DuplicateParams &p_params) { List plist; get_property_list(&plist); Ref r = Object::cast_to(ClassDB::instantiate(get_class())); ERR_FAIL_COND_V(r.is_null(), Ref()); - p_remap_cache[this] = r; + p_params.remap_cache->insert(this, r); - r->local_scene = p_for_scene; + if (p_params.local_scene) { + r->local_scene = p_params.local_scene; + } for (const PropertyInfo &E : plist) { if (!(E.usage & PROPERTY_USAGE_STORAGE)) { @@ -303,13 +305,15 @@ Ref Resource::duplicate_for_local_scene(Node *p_for_scene, HashMapset(E.name, p); @@ -318,6 +322,14 @@ Ref Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap, Ref> &p_remap_cache) { + DuplicateParams params; + params.deep = true; + params.local_scene = p_for_scene; + params.remap_cache = &p_remap_cache; + return _duplicate_impl(params); +} + void Resource::_find_sub_resources(const Variant &p_variant, HashSet> &p_resources_found) { switch (p_variant.get_type()) { case Variant::ARRAY: { @@ -374,52 +386,11 @@ void Resource::configure_for_local_scene(Node *p_for_scene, HashMap Resource::duplicate(bool p_subresources) const { - List plist; - get_property_list(&plist); - - Ref r = static_cast(ClassDB::instantiate(get_class())); - ERR_FAIL_COND_V(r.is_null(), Ref()); - - for (const PropertyInfo &E : plist) { - if (!(E.usage & PROPERTY_USAGE_STORAGE)) { - continue; - } - Variant p = get(E.name); - - switch (p.get_type()) { - case Variant::Type::DICTIONARY: - case Variant::Type::ARRAY: - case Variant::Type::PACKED_BYTE_ARRAY: - case Variant::Type::PACKED_COLOR_ARRAY: - case Variant::Type::PACKED_INT32_ARRAY: - case Variant::Type::PACKED_INT64_ARRAY: - case Variant::Type::PACKED_FLOAT32_ARRAY: - case Variant::Type::PACKED_FLOAT64_ARRAY: - case Variant::Type::PACKED_STRING_ARRAY: - case Variant::Type::PACKED_VECTOR2_ARRAY: - case Variant::Type::PACKED_VECTOR3_ARRAY: - case Variant::Type::PACKED_VECTOR4_ARRAY: { - r->set(E.name, p.duplicate(p_subresources)); - } break; - - case Variant::Type::OBJECT: { - if (!(E.usage & PROPERTY_USAGE_NEVER_DUPLICATE) && (p_subresources || (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE))) { - Ref sr = p; - if (sr.is_valid()) { - r->set(E.name, sr->duplicate(p_subresources)); - } - } else { - r->set(E.name, p); - } - } break; - - default: { - r->set(E.name, p); - } - } - } - - return r; + HashMap, Ref> remap_cache; + DuplicateParams params; + params.deep = p_subresources; + params.remap_cache = &remap_cache; + return _duplicate_impl(params); } void Resource::_set_path(const String &p_path) { diff --git a/core/io/resource.h b/core/io/resource.h index 15d6202568d6..35f5ac000dc3 100644 --- a/core/io/resource.h +++ b/core/io/resource.h @@ -74,7 +74,13 @@ class Resource : public RefCounted { SelfList remapped_list; - Variant _duplicate_recursive_for_local_scene(const Variant &p_variant, Node *p_for_scene, HashMap, Ref> &p_remap_cache); + struct DuplicateParams { + bool deep = false; // With subresources? + Node *local_scene = nullptr; + HashMap, Ref> *remap_cache = nullptr; + }; + Ref _duplicate_impl(const DuplicateParams &p_params); + Variant _duplicate_recursive_impl(const Variant &p_variant, const DuplicateParams &p_params); void _find_sub_resources(const Variant &p_variant, HashSet> &p_resources_found); protected: