resolve nodes and node arrays during InstancePlaceholder instantiation

scene/main/instance_placeholder.cpp

@@ -88,27 +88,114 @@ Node *InstancePlaceholder::create_instance(bool p_replace, const Ref<PackedScene
 	if (!ps.is_valid()) {
 		return nullptr;
 	}
-	Node *scene = ps->instantiate();
-	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) {
 		queue_free();
 		base->remove_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) {

scene/main/instance_placeholder.h

@@ -46,6 +46,10 @@ class InstancePlaceholder : public Node {
 
 	List<PropSet> 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;

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;