Skip to content

Commit

Permalink
Overhaul Resource::duplicate()
Browse files Browse the repository at this point in the history 
- 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.
  • Loading branch information
@RandomShaper
RandomShaper committed Dec 17, 2024
1 parent e083009 commit d209323
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 64 deletions.
97 changes: 34 additions & 63 deletions core/io/resource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Resource>, Ref<Resource>> &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<Resource> &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<Resource> &dupe = sr->duplicate_for_local_scene(p_for_scene, p_remap_cache);
p_remap_cache[sr] = dupe;
const Ref<Resource> &dupe = sr->_duplicate_impl(p_params);
p_params.remap_cache->insert(sr, dupe);
return dupe;
}
} else {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -286,30 +286,34 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant,
}
}

Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) {
Ref<Resource> Resource::_duplicate_impl(const DuplicateParams &p_params) {
List<PropertyInfo> plist;
get_property_list(&plist);

Ref<Resource> r = Object::cast_to<Resource>(ClassDB::instantiate(get_class()));
ERR_FAIL_COND_V(r.is_null(), Ref<Resource>());

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)) {
continue;
}
Variant p = get(E.name);

bool should_recurse = true;
if (p.get_type() == Variant::OBJECT && !(E.usage & PROPERTY_USAGE_NEVER_DUPLICATE)) {
should_recurse = false;
}
if (p_params.deep) {
bool should_recurse = true;
if (p.get_type() == Variant::OBJECT && !(E.usage & PROPERTY_USAGE_NEVER_DUPLICATE)) {
should_recurse = false;
}

if (should_recurse) {
p = _duplicate_recursive_for_local_scene(p, p_for_scene, p_remap_cache);
if (should_recurse) {
p = _duplicate_recursive_impl(p, p_params);
}
}

r->set(E.name, p);
Expand All @@ -318,6 +322,14 @@ Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref
return r;
}

Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &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<Ref<Resource>> &p_resources_found) {
switch (p_variant.get_type()) {
case Variant::ARRAY: {
Expand Down Expand Up @@ -374,52 +386,11 @@ void Resource::configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource
}

Ref<Resource> Resource::duplicate(bool p_subresources) const {
List<PropertyInfo> plist;
get_property_list(&plist);

Ref<Resource> r = static_cast<Resource *>(ClassDB::instantiate(get_class()));
ERR_FAIL_COND_V(r.is_null(), Ref<Resource>());

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<Resource> 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<Resource>, Ref<Resource>> remap_cache;
DuplicateParams params;
params.deep = p_subresources;
params.remap_cache = &remap_cache;
return _duplicate_impl(params);

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🍎 macOS / Editor (target=editor, tests=yes) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🍎 macOS / Template (target=template_release, tests=yes) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🍏 iOS / Template (target=template_release) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🤖 Android / Editor (target=editor) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor w/ Mono (target=editor) 

passing 'const Resource' as 'this' argument discards qualifiers

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🤖 Android / Template arm32 (target=template_release, arch=arm32) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with doubles and GCC sanitizers (target=editor, tests=yes, dev_build=yes, scu_build=yes, precision=double, use_asan=yes, use_ubsan=yes, linker=gold) 

passing 'const Resource' as 'this' argument discards qualifiers

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/ threads (target=template_release, threads=yes) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🤖 Android / Template arm64 (target=template_release, arch=arm64) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🌐 Web / Template w/o threads (target=template_release, threads=no) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with clang sanitizers (target=editor, tests=yes, dev_build=yes, use_asan=yes, use_ubsan=yes, use_llvm=yes, linker=lld) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with ThreadSanitizer (target=editor, tests=yes, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Template w/ Mono (target=template_release, tests=yes) 

passing 'const Resource' as 'this' argument discards qualifiers

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🏁 Windows / Editor (target=editor, tests=yes) 

'Ref<Resource> Resource::_duplicate_impl(const Resource::DuplicateParams &)': cannot convert 'this' pointer from 'const Resource' to 'Resource &'

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Minimal template (target=template_release, tests=yes, everything disabled) 

passing 'const Resource' as 'this' argument discards qualifiers

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🏁 Windows / Editor w/ clang-cl (target=editor, tests=yes, use_llvm=yes) 

'this' argument to member function '_duplicate_impl' has type 'const Resource', but function is not marked const

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🏁 Windows / Template (target=template_release, tests=yes) 

'Ref<Resource> Resource::_duplicate_impl(const Resource::DuplicateParams &)': cannot convert 'this' pointer from 'const Resource' to 'Resource &'

Check failure on line 393 in core/io/resource.cpp

View workflow job for this annotation

GitHub Actions / 🏁 Windows / Template w/ GCC (target=template_release, tests=yes, use_mingw=yes) 

passing 'const Resource' as 'this' argument discards qualifiers
}

void Resource::_set_path(const String &p_path) {
Expand Down
8 changes: 7 additions & 1 deletion core/io/resource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ class Resource : public RefCounted {

SelfList<Resource> remapped_list;

Variant _duplicate_recursive_for_local_scene(const Variant &p_variant, Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);
struct DuplicateParams {
bool deep = false; // With subresources?
Node *local_scene = nullptr;
HashMap<Ref<Resource>, Ref<Resource>> *remap_cache = nullptr;
};
Ref<Resource> _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<Ref<Resource>> &p_resources_found);

protected:
Expand Down

0 comments on commit d209323

Please sign in to comment.