Skip to content

Commit

Permalink
Overhaul Resource::duplicate()
Browse files Browse the repository at this point in the history 
- Thanks to a refactor, `Resource::duplicate_for_local_scene()` and `Resource::duplicate()` are now both users of the same, parametrized, implementation
- `Resource::duplicate()` now honors with/without subresources modes in a more consistent and predictable fashion. Among other changes, packed arrays, arrays and dictionaries are copied if, and only if, subresource duplication is requested. Given the lack of separate control for both, that gives a more predictable outcome. Separate control would be not possible anyway because duplicating deep resources may need to duplicate the arrays or dictionaries where it's involved regardless user will. The behavior afte this change is as follows:
  - 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.
    - Previously, subresources would be duplicated as many times as being referenced throughout the main resource.
    - Now, each subresource is only duplicated once and from that point, a referenced to that single copy is used. That's the enhanced behavior that `duplicate_for_local_scene()` already featured.
    - The behavior with respect to packed arrays, arrays and dictionaries stays: recursive duplication.
  - Without subresources:
    - Previously, the first level of resources found as direct property values would be duplicated unconditionally. Packed arrays, arrays and dictionaries were non-recursively duplicated.
    - Now, no subresource found at any level in any form, will be duplicated, but the original reference kept instead. Packed arrays, arrays and dictionaries are referenced, not duplicated at all.
  • Loading branch information
@RandomShaper
RandomShaper committed Dec 18, 2024
1 parent e6f813d commit 6426637
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 69 deletions.
107 changes: 43 additions & 64 deletions core/io/resource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,17 @@ 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) const {
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 duplicating for local-to-scene, only duplicate resources marked as scene-local.
if (sr.is_valid() && (!p_params.local_scene || sr->is_local_to_scene())) {
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 +264,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 +276,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 @@ -298,16 +299,18 @@ 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) const {
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(Ref<Resource>(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)) {
Expand All @@ -316,13 +319,22 @@ Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref

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);
}
} else {
if (p.get_type() == Variant::OBJECT && (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE)) {
const Ref<Resource> &sr = p;
if (sr.is_valid()) {
p = sr->duplicate(true);
}
}
}

r->set(E.name, p);
Expand All @@ -331,6 +343,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) const {
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 @@ -387,52 +407,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);
}

void Resource::_set_path(const String &p_path) {
Expand Down
10 changes: 8 additions & 2 deletions 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) const;
Variant _duplicate_recursive_impl(const Variant &p_variant, const DuplicateParams &p_params) const;
void _find_sub_resources(const Variant &p_variant, HashSet<Ref<Resource>> &p_resources_found);

protected:
Expand Down Expand Up @@ -120,7 +126,7 @@ class Resource : public RefCounted {
String get_scene_unique_id() const;

virtual Ref<Resource> duplicate(bool p_subresources = false) const;
Ref<Resource> duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);
Ref<Resource> duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) const;
void configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);

void set_local_to_scene(bool p_enable);
Expand Down
7 changes: 4 additions & 3 deletions doc/classes/Resource.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@
<param index="0" name="subresources" type="bool" default="false" />
<description>
Duplicates this resource, returning a new resource with its [code]export[/code]ed or [constant PROPERTY_USAGE_STORAGE] properties copied from the original.
If [param subresources] is [code]false[/code], a shallow copy is returned; nested resources within subresources are not duplicated and are shared with the original resource (with one exception; see below). If [param subresources] is [code]true[/code], a deep copy is returned; nested subresources will be duplicated and are not shared (with two exceptions; see below).
If [param subresources] is [code]false[/code], a shallow copy is returned; nested resources within subresources are not duplicated and are shared with the original resource (with one exception; see below).
If [param subresources] is [code]true[/code], a deep copy is returned; nested subresources will be duplicated and are not shared (with two exceptions; see below). Nested arrays and dictionaries will also be recursively duplicated.
[param subresources] is usually respected, with the following exceptions:
- Subresource properties with the [constant PROPERTY_USAGE_ALWAYS_DUPLICATE] flag are always duplicated.
- Subresource properties with the [constant PROPERTY_USAGE_ALWAYS_DUPLICATE] flag are always duplicated (recursively).
- Subresource properties with the [constant PROPERTY_USAGE_NEVER_DUPLICATE] flag are never duplicated.
- Subresources inside [Array] and [Dictionary] properties are never duplicated.
[b]Note:[/b] For custom resources, this method will fail if [method Object._init] has been defined with required parameters.
[b]Note:[/b] When duplicaating with [param subresources] set to [code]true[/code], each resource found, including the one on which this method is called, will be only duplicated once and referenced as many times as needed in the duplicate. For instance, if you are duplicating resource A that happens to have resource B referenced twice, you'll get a new resource A' referencing a new resource B' twice.
</description>
</method>
<method name="emit_changed">
Expand Down

0 comments on commit 6426637

Please sign in to comment.