Skip to content

Commit 1b4f116

Browse files
committed
Supplement the case of scene instantiation for "Editable Children"
This is a follow-up to #65011. For scenes with **Editable Children** enabled, the main scene will record more information and resource mapping will be valid for multiple nodes.
1 parent 7ed0b61 commit 1b4f116

File tree

2 files changed

+56
-58
lines changed

2 files changed

+56
-58
lines changed

scene/resources/packed_scene.cpp

Lines changed: 52 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,22 @@ static Array _sanitize_node_pinned_properties(Node *p_node) {
7878
return pinned;
7979
}
8080

81-
Ref<Resource> SceneState::get_remap_resource(const Ref<Resource> &p_resource, HashMap<Ref<Resource>, Ref<Resource>> &remap_cache, const Ref<Resource> &p_fallback, Node *p_for_scene) {
81+
Ref<Resource> SceneState::get_remap_resource(const Ref<Resource> &p_resource, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &remap_cache, const Ref<Resource> &p_fallback, Node *p_for_scene) {
8282
ERR_FAIL_COND_V(p_resource.is_null(), Ref<Resource>());
8383

84-
Ref<Resource> remap_resource;
84+
bool reuse_fallback = p_fallback.is_valid() && p_fallback->is_local_to_scene() && p_fallback->get_class() == p_resource->get_class();
8585

86-
// Find the shared copy of the source resource.
87-
HashMap<Ref<Resource>, Ref<Resource>>::Iterator R = remap_cache.find(p_resource);
88-
if (R) {
89-
remap_resource = R->value;
90-
} else if (p_fallback.is_valid() && p_fallback->is_local_to_scene() && p_fallback->get_class() == p_resource->get_class()) {
91-
// Simply copy the data from the source resource to update the fallback resource that was previously set.
86+
if (reuse_fallback) {
87+
// The fallback resource can only be mapped at most once when it is valid.
88+
for (const KeyValue<Ref<Resource>, Ref<Resource>> &E : remap_cache[p_for_scene]) {
89+
if (E.value == p_fallback) {
90+
reuse_fallback = false;
91+
break;
92+
}
93+
}
94+
}
9295

96+
if (reuse_fallback) { // Simply copy the data from the source resource to update the fallback resource that was previously set.
9397
p_fallback->reset_state(); // May want to reset state.
9498

9599
List<PropertyInfo> pi;
@@ -113,18 +117,14 @@ Ref<Resource> SceneState::get_remap_resource(const Ref<Resource> &p_resource, Ha
113117

114118
p_fallback->set(E.name, value);
115119
}
116-
117-
p_fallback->set_scene_unique_id(p_resource->get_scene_unique_id()); // Get the id from the main scene, in case the id changes again when saving the scene.
118-
119-
remap_cache[p_resource] = p_fallback;
120-
remap_resource = p_fallback;
121-
} else { // A copy of the source resource is required to overwrite the previous one.
122-
Ref<Resource> local_dupe = p_resource->duplicate_for_local_scene(p_for_scene, remap_cache);
123-
remap_cache[p_resource] = local_dupe;
124-
remap_resource = local_dupe;
120+
remap_cache[p_for_scene][p_resource] = p_fallback;
121+
return p_fallback;
125122
}
126123

127-
return remap_resource;
124+
// A copy of the source resource is required to overwrite the previous one.
125+
Ref<Resource> local_dupe = p_resource->duplicate_for_local_scene(p_for_scene, remap_cache[p_for_scene]);
126+
remap_cache[p_for_scene][p_resource] = local_dupe;
127+
return local_dupe;
128128
}
129129

130130
static Node *_find_node_by_id(Node *p_owner, Node *p_node, int32_t p_id) {
@@ -185,7 +185,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
185185

186186
bool gen_node_path_cache = p_edit_state != GEN_EDIT_STATE_DISABLED && node_path_cache.is_empty();
187187

188-
HashMap<Ref<Resource>, Ref<Resource>> resources_local_to_scene;
188+
HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> resources_local_to_scenes; // Record the mappings in sub-scenes.
189189

190190
LocalVector<DeferredNodePathProperties> deferred_node_paths;
191191

@@ -359,7 +359,6 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
359359
const NodeData::Property *nprops = &n.properties[0];
360360

361361
Dictionary missing_resource_properties;
362-
HashMap<Ref<Resource>, Ref<Resource>> resources_local_to_sub_scene; // Record the mappings in the sub-scene.
363362

364363
for (int j = 0; j < nprop_count; j++) {
365364
bool valid;
@@ -430,7 +429,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
430429
//handle resources that are local to scene by duplicating them if needed
431430
Ref<Resource> res = value;
432431
if (res.is_valid()) {
433-
value = make_local_resource(value, n, resources_local_to_sub_scene, node, snames[nprops[j].name], resources_local_to_scene, i, ret_nodes, p_edit_state);
432+
value = make_local_resource(value, n, resources_local_to_scenes, node, snames[nprops[j].name], i, ret_nodes, p_edit_state);
434433
}
435434
} else {
436435
// Making sure that instances of inherited scenes don't share the same
@@ -454,7 +453,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
454453
}
455454
}
456455

457-
value = setup_resources_in_array(set_array, n, resources_local_to_sub_scene, node, snames[nprops[j].name], resources_local_to_scene, i, ret_nodes, p_edit_state);
456+
value = setup_resources_in_array(set_array, n, resources_local_to_scenes, node, snames[nprops[j].name], i, ret_nodes, p_edit_state);
458457
}
459458

460459
if (value.get_type() == Variant::DICTIONARY) {
@@ -471,7 +470,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
471470
}
472471
}
473472

474-
value = setup_resources_in_dictionary(set_dict, n, resources_local_to_sub_scene, node, snames[nprops[j].name], resources_local_to_scene, i, ret_nodes, p_edit_state);
473+
value = setup_resources_in_dictionary(set_dict, n, resources_local_to_scenes, node, snames[nprops[j].name], i, ret_nodes, p_edit_state);
475474
}
476475

477476
bool set_valid = true;
@@ -494,12 +493,6 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
494493
if (!missing_resource_properties.is_empty()) {
495494
node->set_meta(META_MISSING_RESOURCES, missing_resource_properties);
496495
}
497-
498-
for (KeyValue<Ref<Resource>, Ref<Resource>> &E : resources_local_to_sub_scene) {
499-
if (E.value->get_local_scene() == node) {
500-
E.value->setup_local_to_scene(); // Setup may be required for the resource to work properly.
501-
}
502-
}
503496
}
504497

505498
//name
@@ -639,9 +632,9 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
639632
}
640633
}
641634

642-
for (KeyValue<Ref<Resource>, Ref<Resource>> &E : resources_local_to_scene) {
643-
if (E.value->get_local_scene() == ret_nodes[0]) {
644-
E.value->setup_local_to_scene();
635+
for (KeyValue<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &E : resources_local_to_scenes) {
636+
for (KeyValue<Ref<Resource>, Ref<Resource>> &R : E.value) {
637+
R.value->setup_local_to_scene(); // Setup may be required for the resource to work properly.
645638
}
646639
}
647640

@@ -702,50 +695,55 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
702695
return ret_nodes[0];
703696
}
704697

705-
Variant SceneState::make_local_resource(Variant &p_value, const SceneState::NodeData &p_node_data, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_sub_scene, Node *p_node, const StringName p_sname, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_scene, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
698+
Variant SceneState::make_local_resource(Variant &p_value, const SceneState::NodeData &p_node_data, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *p_node, const StringName p_sname, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
706699
Ref<Resource> res = p_value;
707700
if (res.is_null() || !res->is_local_to_scene()) {
708701
return p_value;
709702
}
710703

711-
if (p_node_data.instance >= 0) { // For the root node of a sub-scene, treat it as part of the sub-scene.
712-
return get_remap_resource(res, p_resources_local_to_sub_scene, p_node->get(p_sname), p_node);
713-
} else {
714-
HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = p_resources_local_to_scene.find(res);
715-
Node *base = p_i == 0 ? p_node : p_ret_nodes[0];
716-
if (E) {
717-
return E->value;
718-
} else if (p_edit_state == GEN_EDIT_STATE_MAIN) { // For the main scene, use the resource as is
719-
res->configure_for_local_scene(base, p_resources_local_to_scene);
720-
p_resources_local_to_scene[res] = res;
721-
return res;
722-
} else { // For instances, a copy must be made.
723-
Ref<Resource> local_dupe = res->duplicate_for_local_scene(base, p_resources_local_to_scene);
724-
p_resources_local_to_scene[res] = local_dupe;
725-
return local_dupe;
726-
}
704+
Node *base = (p_i == 0 || p_node->is_instance()) ? p_node : (p_node->get_owner() ? p_node->get_owner() : p_ret_nodes[0]);
705+
706+
// Find the shared copy of the source resource.
707+
HashMap<Ref<Resource>, Ref<Resource>>::Iterator R = p_resources_local_to_scenes[base].find(res);
708+
if (R) {
709+
return R->value;
727710
}
711+
712+
if (p_node_data.type == TYPE_INSTANTIATED) { // For the (root) nodes of sub-scenes, treat them as parts of the sub-scenes.
713+
return get_remap_resource(res, p_resources_local_to_scenes, p_node->get(p_sname), base);
714+
}
715+
716+
if (p_edit_state == GEN_EDIT_STATE_MAIN) { // For the main scene, use the resource as is
717+
res->configure_for_local_scene(base, p_resources_local_to_scenes[base]);
718+
p_resources_local_to_scenes[base][res] = res;
719+
return res;
720+
}
721+
722+
// For instances, a copy must be made.
723+
Ref<Resource> local_dupe = res->duplicate_for_local_scene(base, p_resources_local_to_scenes[base]);
724+
p_resources_local_to_scenes[base][res] = local_dupe;
725+
return local_dupe;
728726
}
729727

730-
Array SceneState::setup_resources_in_array(Array &p_array_to_scan, const SceneState::NodeData &p_n, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_sub_scene, Node *p_node, const StringName p_sname, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_scene, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
728+
Array SceneState::setup_resources_in_array(Array &p_array_to_scan, const SceneState::NodeData &p_n, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *p_node, const StringName p_sname, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
731729
for (int i = 0; i < p_array_to_scan.size(); i++) {
732730
if (p_array_to_scan[i].get_type() == Variant::OBJECT) {
733-
p_array_to_scan[i] = make_local_resource(p_array_to_scan[i], p_n, p_resources_local_to_sub_scene, p_node, p_sname, p_resources_local_to_scene, p_i, p_ret_nodes, p_edit_state);
731+
p_array_to_scan[i] = make_local_resource(p_array_to_scan[i], p_n, p_resources_local_to_scenes, p_node, p_sname, p_i, p_ret_nodes, p_edit_state);
734732
}
735733
}
736734
return p_array_to_scan;
737735
}
738736

739-
Dictionary SceneState::setup_resources_in_dictionary(Dictionary &p_dictionary_to_scan, const SceneState::NodeData &p_n, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_sub_scene, Node *p_node, const StringName p_sname, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_scene, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
737+
Dictionary SceneState::setup_resources_in_dictionary(Dictionary &p_dictionary_to_scan, const SceneState::NodeData &p_n, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *p_node, const StringName p_sname, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const {
740738
const Array keys = p_dictionary_to_scan.keys();
741739
const Array values = p_dictionary_to_scan.values();
742740

743741
if (has_local_resource(values) || has_local_resource(keys)) {
744742
Array duplicated_keys = keys.duplicate(true);
745743
Array duplicated_values = values.duplicate(true);
746744

747-
duplicated_keys = setup_resources_in_array(duplicated_keys, p_n, p_resources_local_to_sub_scene, p_node, p_sname, p_resources_local_to_scene, p_i, p_ret_nodes, p_edit_state);
748-
duplicated_values = setup_resources_in_array(duplicated_values, p_n, p_resources_local_to_sub_scene, p_node, p_sname, p_resources_local_to_scene, p_i, p_ret_nodes, p_edit_state);
745+
duplicated_keys = setup_resources_in_array(duplicated_keys, p_n, p_resources_local_to_scenes, p_node, p_sname, p_i, p_ret_nodes, p_edit_state);
746+
duplicated_values = setup_resources_in_array(duplicated_values, p_n, p_resources_local_to_scenes, p_node, p_sname, p_i, p_ret_nodes, p_edit_state);
749747
p_dictionary_to_scan.clear();
750748

751749
for (int i = 0; i < keys.size(); i++) {

scene/resources/packed_scene.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class SceneState : public RefCounted {
139139
};
140140

141141
static void set_disable_placeholders(bool p_disable);
142-
static Ref<Resource> get_remap_resource(const Ref<Resource> &p_resource, HashMap<Ref<Resource>, Ref<Resource>> &remap_cache, const Ref<Resource> &p_fallback, Node *p_for_scene);
142+
static Ref<Resource> get_remap_resource(const Ref<Resource> &p_resource, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &remap_cache, const Ref<Resource> &p_fallback, Node *p_for_scene);
143143

144144
int find_node_by_path(const NodePath &p_node) const;
145145
Variant get_property_value(int p_node, const StringName &p_property, bool &r_found, bool &r_node_deferred) const;
@@ -160,9 +160,9 @@ class SceneState : public RefCounted {
160160
bool can_instantiate() const;
161161
Node *instantiate(GenEditState p_edit_state) const;
162162

163-
Array setup_resources_in_array(Array &array_to_scan, const SceneState::NodeData &n, HashMap<Ref<Resource>, Ref<Resource>> &resources_local_to_sub_scene, Node *node, const StringName sname, HashMap<Ref<Resource>, Ref<Resource>> &resources_local_to_scene, int i, Node **ret_nodes, SceneState::GenEditState p_edit_state) const;
164-
Dictionary setup_resources_in_dictionary(Dictionary &p_dictionary_to_scan, const SceneState::NodeData &p_n, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_sub_scene, Node *p_node, const StringName p_sname, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_scene, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const;
165-
Variant make_local_resource(Variant &value, const SceneState::NodeData &p_node_data, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_sub_scene, Node *p_node, const StringName p_sname, HashMap<Ref<Resource>, Ref<Resource>> &p_resources_local_to_scene, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const;
163+
Array setup_resources_in_array(Array &array_to_scan, const SceneState::NodeData &n, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *node, const StringName sname, int i, Node **ret_nodes, SceneState::GenEditState p_edit_state) const;
164+
Dictionary setup_resources_in_dictionary(Dictionary &p_dictionary_to_scan, const SceneState::NodeData &p_n, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *p_node, const StringName p_sname, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const;
165+
Variant make_local_resource(Variant &value, const SceneState::NodeData &p_node_data, HashMap<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &p_resources_local_to_scenes, Node *p_node, const StringName p_sname, int p_i, Node **p_ret_nodes, SceneState::GenEditState p_edit_state) const;
166166
bool has_local_resource(const Array &p_array) const;
167167

168168
Ref<SceneState> get_base_scene_state() const;

0 commit comments

Comments
 (0)