From 46b936333b3aa320f18c3cf0f4a5287c971b5254 Mon Sep 17 00:00:00 2001 From: Adriano Orioli Date: Sat, 6 Apr 2024 16:40:44 +0200 Subject: [PATCH] resolve nodes and node arrays during InstancePlaceholder instantiation --- scene/main/instance_placeholder.cpp | 103 +++++++++++++++++++++++++--- scene/main/instance_placeholder.h | 4 ++ scene/resources/packed_scene.cpp | 5 +- 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/scene/main/instance_placeholder.cpp b/scene/main/instance_placeholder.cpp index fe23ca1800d8..9911c68d1938 100644 --- a/scene/main/instance_placeholder.cpp +++ b/scene/main/instance_placeholder.cpp @@ -88,16 +88,16 @@ Node *InstancePlaceholder::create_instance(bool p_replace, const Refinstantiate(); - if (!scene) { + Node *instance = ps->instantiate(); + if (!instance) { return nullptr; } - scene->set_name(get_name()); - scene->set_multiplayer_authority(get_multiplayer_authority()); + instance->set_name(get_name()); + instance->set_multiplayer_authority(get_multiplayer_authority()); int pos = get_index(); for (const PropSet &E : stored_values) { - scene->set(E.name, E.value); + set_value_on_instance(this, instance, E); } if (p_replace) { @@ -105,10 +105,97 @@ Node *InstancePlaceholder::create_instance(bool p_replace, const Refremove_child(this); } - base->add_child(scene); - base->move_child(scene, pos); + base->add_child(instance); + base->move_child(instance, pos); - return scene; + return instance; +} + +// This method will attempt to set the correct values on the placeholder instance +// for regular types this is trivial and unnecessary. +// For nodes however this becomes a bit tricky because they might now have existed until the instantiation, +// so this method will try to find the correct nodes and resolve them. +void InstancePlaceholder::set_value_on_instance(InstancePlaceholder *placeholder, Node *instance, const InstancePlaceholder::PropSet &E) { + bool is_valid; + + // if we don't have any info, we can't do anything + // so try setting the value directly + Variant current = instance->get(E.name, &is_valid); + if (!is_valid) { + instance->set(E.name, E.value, &is_valid); + return; + } + + Variant::Type current_type = current.get_type(); + Variant::Type placeholder_type = E.value.get_type(); + + // check if the variant types match + if (Variant::evaluate(Variant::OP_EQUAL, current_type, placeholder_type)) { + instance->set(E.name, E.value, &is_valid); + if (is_valid) + return; + // types match but setting failed? it's probably an array and we have a TypedArray issue + } + + switch (current.get_type()) { + case Variant::Type::NIL: + if (placeholder_type != Variant::Type::NODE_PATH) + break; + // if it's nil but we have a nodepath, we guess what works + + instance->set(E.name, E.value, &is_valid); + if (is_valid) + break; + + instance->set(E.name, try_get_node(placeholder, instance, E.value), &is_valid); + break; + case Variant::Type::OBJECT: + if (placeholder_type != Variant::Type::NODE_PATH) + break; + // easiest case, we want a node, but we have a deferred nodepath + instance->set(E.name, try_get_node(placeholder, instance, E.value)); + break; + case Variant::Type::ARRAY: { + // if we have reached here it means our array types don't match + // so we will convert the placeholder array into the correct type + // and resolve nodes if necessary + Array current_array = current; + Array converted_array; + Array placeholder_array = E.value; + converted_array = current_array.duplicate(); + converted_array.resize(placeholder_array.size()); + + if (Variant::evaluate(Variant::OP_EQUAL, current_array.get_typed_builtin(), Variant::Type::NODE_PATH)) { + // we want a typed nodepath array + for (int i = 0; i < placeholder_array.size(); i++) { + converted_array.set(i, placeholder_array[i]); + } + } else { + // we want nodes, convert nodepaths + for (int i = 0; i < placeholder_array.size(); i++) { + converted_array.set(i, try_get_node(placeholder, instance, placeholder_array[i])); + } + } + + instance->set(E.name, converted_array, &is_valid); + if (!is_valid) + WARN_PRINT(vformat("{Property '%s' with type %s could not be set when creating instance of '%s'}", E.name, Variant::get_type_name(current_type), placeholder->get_name())); + break; + } + default: + WARN_PRINT(vformat("{Property '%s' with type '%s' could not be set when creating instance of '%s'}", E.name, Variant::get_type_name(current_type), placeholder->get_name())); + break; + } +} + +Node *InstancePlaceholder::try_get_node(InstancePlaceholder *placeholder, Node *instance, const NodePath &np) { + // first try to resolve internally + // if that fails try resolving externally + Node *node = instance->get_node_or_null(np); + if (node == nullptr) + node = placeholder->get_node_or_null(np); + + return node; } Dictionary InstancePlaceholder::get_stored_values(bool p_with_order) { diff --git a/scene/main/instance_placeholder.h b/scene/main/instance_placeholder.h index 480474d0bd4b..fa50d2f769e3 100644 --- a/scene/main/instance_placeholder.h +++ b/scene/main/instance_placeholder.h @@ -46,6 +46,10 @@ class InstancePlaceholder : public Node { List stored_values; +private: + void set_value_on_instance(InstancePlaceholder *placeholder, Node *instance, const InstancePlaceholder::PropSet &E); + Node *try_get_node(InstancePlaceholder *placeholder, Node *instance, const NodePath &np); + protected: bool _set(const StringName &p_name, const Variant &p_value); bool _get(const StringName &p_name, Variant &r_ret) const; diff --git a/scene/resources/packed_scene.cpp b/scene/resources/packed_scene.cpp index 976210668ff1..fa755c5765c7 100644 --- a/scene/resources/packed_scene.cpp +++ b/scene/resources/packed_scene.cpp @@ -315,8 +315,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const { if (nprops[j].name & FLAG_PATH_PROPERTY_IS_NODE) { if (node->get_scene_instance_load_placeholder()) { - // nodes might not exist yet, so we skip deffering nodepath resolution to placeholder instantiation - // this does not solve arrays problems, and creates issues with external node referneces + // we cannot know if the referenced nodes exist yet, so instead of defering, we write the nodepaths directly uint32_t name_idx = nprops[j].name & (FLAG_PATH_PROPERTY_IS_NODE - 1); ERR_FAIL_UNSIGNED_INDEX_V(name_idx, (uint32_t)sname_count, nullptr); @@ -369,7 +368,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const { 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); bool is_get_valid = false; - Variant get_value = node->get(snames[nprops[j].name], &is_get_valid); // this will always be false for InstancePlaceholders because set has not been called, resulting in un-typed arrays + Variant get_value = node->get(snames[nprops[j].name], &is_get_valid); if (is_get_valid && get_value.get_type() == Variant::ARRAY) { Array get_array = get_value;