From 10b4cb75f2fb79aef12914d9507803204e398bf3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Cambr=C3=A9?= <david.cambre@gmail.com>
Date: Fri, 25 Jun 2021 20:03:13 +0200
Subject: [PATCH] Improve and streamline VisualScriptFuncNodes Call Set Get

This PR improves and streamlines the workflow for VisualScriptFunctionNodes Call Set Get
Uniform design.
Drag in set-get from tree is now working.
Removes redundant method_select popup.
---
 .../visual_script/visual_script_editor.cpp    | 110 ++++++++-----
 modules/visual_script/visual_script_editor.h  |   6 +-
 .../visual_script_func_nodes.cpp              | 146 ++++++++++++------
 .../visual_script/visual_script_func_nodes.h  |   5 +
 4 files changed, 176 insertions(+), 91 deletions(-)

diff --git a/modules/visual_script/visual_script_editor.cpp b/modules/visual_script/visual_script_editor.cpp
index 69920a81b6d6..8858fe9ff171 100644
--- a/modules/visual_script/visual_script_editor.cpp
+++ b/modules/visual_script/visual_script_editor.cpp
@@ -2234,41 +2234,35 @@ void VisualScriptEditor::drop_data_fw(const Point2 &p_point, const Variant &p_da
 		undo_redo->create_action(TTR("Add Node(s) From Tree"));
 		int base_id = script->get_available_id();
 
-		if (nodes.size() > 1) {
-			use_node = true;
-		}
-
-		for (int i = 0; i < nodes.size(); i++) {
-			NodePath np = nodes[i];
-			Node *node = get_node(np);
-			if (!node) {
-				continue;
-			}
+		if (use_node || nodes.size() > 1) {
+			for (int i = 0; i < nodes.size(); i++) {
+				NodePath np = nodes[i];
+				Node *node = get_node(np);
+				if (!node) {
+					continue;
+				}
 
-			Ref<VisualScriptNode> n;
+				Ref<VisualScriptNode> n;
 
-			if (use_node) {
 				Ref<VisualScriptSceneNode> scene_node;
 				scene_node.instantiate();
 				scene_node->set_node_path(sn->get_path_to(node));
 				n = scene_node;
-			} else {
-				// ! Doesn't work properly.
-				Ref<VisualScriptFunctionCall> call;
-				call.instantiate();
-				call->set_call_mode(VisualScriptFunctionCall::CALL_MODE_NODE_PATH);
-				call->set_base_path(sn->get_path_to(node));
-				call->set_base_type(node->get_class());
-				n = call;
-				method_select->select_from_instance(node, "", true, node->get_class());
-				selecting_method_id = base_id;
-			}
 
-			undo_redo->add_do_method(script.ptr(), "add_node", base_id, n, pos);
-			undo_redo->add_undo_method(script.ptr(), "remove_node", base_id);
+				undo_redo->add_do_method(script.ptr(), "add_node", base_id, n, pos);
+				undo_redo->add_undo_method(script.ptr(), "remove_node", base_id);
+
+				base_id++;
+				pos += Vector2(25, 25);
+			}
 
-			base_id++;
-			pos += Vector2(25, 25);
+		} else {
+			NodePath np = nodes[0];
+			Node *node = get_node(np);
+			drop_position = pos;
+			drop_node = node;
+			drop_path = sn->get_path_to(node);
+			new_connect_node_select->select_from_instance(node, "", false, node->get_class());
 		}
 		undo_redo->add_do_method(this, "_update_graph");
 		undo_redo->add_undo_method(this, "_update_graph");
@@ -2387,14 +2381,6 @@ void VisualScriptEditor::drop_data_fw(const Point2 &p_point, const Variant &p_da
 	}
 }
 
-void VisualScriptEditor::_selected_method(const String &p_method, const String &p_type, const bool p_connecting) {
-	Ref<VisualScriptFunctionCall> vsfc = script->get_node(selecting_method_id);
-	if (!vsfc.is_valid()) {
-		return;
-	}
-	vsfc->set_function(p_method);
-}
-
 void VisualScriptEditor::_draw_color_over_button(Object *obj, Color p_color) {
 	Button *button = Object::cast_to<Button>(obj);
 	if (!button) {
@@ -3122,6 +3108,11 @@ void VisualScriptEditor::_selected_connect_node(const String &p_text, const Stri
 
 	Set<int> vn;
 
+	if (drop_position != Vector2()) {
+		pos = drop_position;
+	}
+	drop_position = Vector2();
+
 	bool port_node_exists = true;
 
 	// if (func == StringName()) {
@@ -3175,18 +3166,62 @@ void VisualScriptEditor::_selected_connect_node(const String &p_text, const Stri
 	if (p_category == String("method")) {
 		Ref<VisualScriptFunctionCall> n;
 		n.instantiate();
+		if (!drop_path.is_empty()) {
+			if (drop_path == ".") {
+				n->set_call_mode(VisualScriptFunctionCall::CALL_MODE_SELF);
+			} else {
+				n->set_call_mode(VisualScriptFunctionCall::CALL_MODE_NODE_PATH);
+				n->set_base_path(drop_path);
+			}
+		}
+		if (drop_node) {
+			n->set_base_type(drop_node->get_class());
+			if (drop_node->get_script_instance()) {
+				n->set_base_script(drop_node->get_script_instance()->get_script()->get_path());
+			}
+		}
 		vnode = n;
 	} else if (p_category == String("set")) {
 		Ref<VisualScriptPropertySet> n;
 		n.instantiate();
+		if (!drop_path.is_empty()) {
+			if (drop_path == ".") {
+				n->set_call_mode(VisualScriptPropertySet::CALL_MODE_SELF);
+			} else {
+				n->set_call_mode(VisualScriptPropertySet::CALL_MODE_NODE_PATH);
+				n->set_base_path(drop_path);
+			}
+		}
+		if (drop_node) {
+			n->set_base_type(drop_node->get_class());
+			if (drop_node->get_script_instance()) {
+				n->set_base_script(drop_node->get_script_instance()->get_script()->get_path());
+			}
+		}
 		vnode = n;
 		script_prop_set = n;
 	} else if (p_category == String("get")) {
 		Ref<VisualScriptPropertyGet> n;
 		n.instantiate();
 		n->set_property(p_text);
+		if (!drop_path.is_empty()) {
+			if (drop_path == ".") {
+				n->set_call_mode(VisualScriptPropertyGet::CALL_MODE_SELF);
+			} else {
+				n->set_call_mode(VisualScriptPropertyGet::CALL_MODE_NODE_PATH);
+				n->set_base_path(drop_path);
+			}
+		}
+		if (drop_node) {
+			n->set_base_type(drop_node->get_class());
+			if (drop_node->get_script_instance()) {
+				n->set_base_script(drop_node->get_script_instance()->get_script()->get_path());
+			}
+		}
 		vnode = n;
 	}
+	drop_path = String();
+	drop_node = nullptr;
 
 	if (p_category == String("action")) {
 		if (p_text == "VisualScriptCondition") {
@@ -4415,11 +4450,6 @@ VisualScriptEditor::VisualScriptEditor() {
 	add_child(default_value_edit);
 	default_value_edit->connect("variant_changed", callable_mp(this, &VisualScriptEditor::_default_value_changed));
 
-	method_select = memnew(VisualScriptPropertySelector);
-	add_child(method_select);
-	method_select->connect("selected", callable_mp(this, &VisualScriptEditor::_selected_method));
-	error_line = -1;
-
 	new_connect_node_select = memnew(VisualScriptPropertySelector);
 	add_child(new_connect_node_select);
 	new_connect_node_select->connect("selected", callable_mp(this, &VisualScriptEditor::_selected_connect_node));
diff --git a/modules/visual_script/visual_script_editor.h b/modules/visual_script/visual_script_editor.h
index 1f0f087be7e2..3b7ed3dba62b 100644
--- a/modules/visual_script/visual_script_editor.h
+++ b/modules/visual_script/visual_script_editor.h
@@ -179,6 +179,9 @@ class VisualScriptEditor : public ScriptEditorBase {
 
 	void connect_data(Ref<VisualScriptNode> vnode_old, Ref<VisualScriptNode> vnode, int new_id);
 
+	NodePath drop_path;
+	Node *drop_node = nullptr;
+	Vector2 drop_position;
 	void _selected_connect_node(const String &p_text, const String &p_category, const bool p_connecting = true);
 	void connect_seq(Ref<VisualScriptNode> vnode_old, Ref<VisualScriptNode> vnode_new, int new_id);
 
@@ -270,9 +273,6 @@ class VisualScriptEditor : public ScriptEditorBase {
 	void _graph_ofs_changed(const Vector2 &p_ofs);
 	void _comment_node_resized(const Vector2 &p_new_size, int p_node);
 
-	int selecting_method_id;
-	void _selected_method(const String &p_method, const String &p_type, const bool p_connecting);
-
 	void _draw_color_over_button(Object *obj, Color p_color);
 	void _button_resource_previewed(const String &p_path, const Ref<Texture2D> &p_preview, const Ref<Texture2D> &p_small_preview, Variant p_ud);
 
diff --git a/modules/visual_script/visual_script_func_nodes.cpp b/modules/visual_script/visual_script_func_nodes.cpp
index 43e1d38a16d3..98383800bad5 100644
--- a/modules/visual_script/visual_script_func_nodes.cpp
+++ b/modules/visual_script/visual_script_func_nodes.cpp
@@ -254,25 +254,32 @@ PropertyInfo VisualScriptFunctionCall::get_output_value_port_info(int p_idx) con
 }
 
 String VisualScriptFunctionCall::get_caption() const {
-	if (call_mode == CALL_MODE_SELF) {
-		return "  " + String(function) + "()";
-	}
-	if (call_mode == CALL_MODE_SINGLETON) {
-		return String(singleton) + ":" + String(function) + "()";
-	} else if (call_mode == CALL_MODE_BASIC_TYPE) {
-		return Variant::get_type_name(basic_type) + "." + String(function) + "()";
-	} else if (call_mode == CALL_MODE_NODE_PATH) {
-		return " [" + String(base_path.simplified()) + "]." + String(function) + "()";
-	} else {
-		return "  " + base_type + "." + String(function) + "()";
-	}
+	return "  " + String(function) + "()";
 }
 
 String VisualScriptFunctionCall::get_text() const {
+	String text;
+
+	if (call_mode == CALL_MODE_BASIC_TYPE) {
+		text = String("On ") + Variant::get_type_name(basic_type);
+	} else if (call_mode == CALL_MODE_INSTANCE) {
+		text = String("On ") + base_type;
+	} else if (call_mode == CALL_MODE_NODE_PATH) {
+		text = "[" + String(base_path.simplified()) + "]";
+	} else if (call_mode == CALL_MODE_SELF) {
+		text = "On Self";
+	} else if (call_mode == CALL_MODE_SINGLETON) {
+		text = String(singleton) + ":" + String(function) + "()";
+	}
+
 	if (rpc_call_mode) {
-		return "RPC";
+		text += " RPC";
+		if (rpc_call_mode == RPC_UNRELIABLE || rpc_call_mode == RPC_UNRELIABLE_TO_ID) {
+			text += " UNREL";
+		}
 	}
-	return "";
+
+	return text;
 }
 
 void VisualScriptFunctionCall::set_basic_type(Variant::Type p_type) {
@@ -901,11 +908,11 @@ static Ref<VisualScriptNode> create_function_call_node(const String &p_name) {
 //////////////////////////////////////////
 
 int VisualScriptPropertySet::get_output_sequence_port_count() const {
-	return call_mode != CALL_MODE_BASIC_TYPE ? 1 : 0;
+	return 1;
 }
 
 bool VisualScriptPropertySet::has_input_sequence_port() const {
-	return call_mode != CALL_MODE_BASIC_TYPE;
+	return 1;
 }
 
 Node *VisualScriptPropertySet::_get_base_node() const {
@@ -990,8 +997,7 @@ PropertyInfo VisualScriptPropertySet::get_input_value_port_info(int p_idx) const
 		if (p_idx == 0) {
 			PropertyInfo pi;
 			pi.type = (call_mode == CALL_MODE_INSTANCE ? Variant::OBJECT : basic_type);
-			pi.name = (call_mode == CALL_MODE_INSTANCE ? String("instance") : Variant::get_type_name(basic_type).to_lower());
-			_adjust_input_index(pi);
+			pi.name = "instance";
 			return pi;
 		}
 	}
@@ -1000,21 +1006,24 @@ PropertyInfo VisualScriptPropertySet::get_input_value_port_info(int p_idx) const
 	ClassDB::get_property_list(_get_base_type(), &props, false);
 	for (List<PropertyInfo>::Element *E = props.front(); E; E = E->next()) {
 		if (E->get().name == property) {
-			PropertyInfo pinfo = PropertyInfo(E->get().type, "value", PROPERTY_HINT_TYPE_STRING, E->get().hint_string);
+			String detail_prop_name = property;
+			if (index != StringName()) {
+				detail_prop_name += "." + String(index);
+			}
+			PropertyInfo pinfo = PropertyInfo(E->get().type, detail_prop_name, PROPERTY_HINT_TYPE_STRING, E->get().hint_string);
 			_adjust_input_index(pinfo);
 			return pinfo;
 		}
 	}
 
 	PropertyInfo pinfo = type_cache;
-	pinfo.name = "value";
 	_adjust_input_index(pinfo);
 	return pinfo;
 }
 
 PropertyInfo VisualScriptPropertySet::get_output_value_port_info(int p_idx) const {
 	if (call_mode == CALL_MODE_BASIC_TYPE) {
-		return PropertyInfo(basic_type, "out");
+		return PropertyInfo(basic_type, "pass");
 	} else if (call_mode == CALL_MODE_INSTANCE) {
 		return PropertyInfo(Variant::OBJECT, "pass", PROPERTY_HINT_TYPE_STRING, get_base_type());
 	} else {
@@ -1036,17 +1045,18 @@ String VisualScriptPropertySet::get_caption() const {
 }
 
 String VisualScriptPropertySet::get_text() const {
+	if (!has_input_sequence_port()) {
+		return "";
+	}
 	if (call_mode == CALL_MODE_BASIC_TYPE) {
 		return String("On ") + Variant::get_type_name(basic_type);
+	} else if (call_mode == CALL_MODE_INSTANCE) {
+		return String("On ") + base_type;
+	} else if (call_mode == CALL_MODE_NODE_PATH) {
+		return " [" + String(base_path.simplified()) + "]";
+	} else {
+		return "On Self";
 	}
-
-	static const char *cname[3] = {
-		"Self",
-		"Scene Node",
-		"Instance"
-	};
-
-	return String("On ") + cname[call_mode];
 }
 
 void VisualScriptPropertySet::_update_base_type() {
@@ -1707,7 +1717,9 @@ int VisualScriptPropertyGet::get_input_value_port_count() const {
 }
 
 int VisualScriptPropertyGet::get_output_value_port_count() const {
-	return 1;
+	int pc = (call_mode == CALL_MODE_BASIC_TYPE || call_mode == CALL_MODE_INSTANCE) ? 2 : 1;
+
+	return pc;
 }
 
 String VisualScriptPropertyGet::get_output_sequence_port_text(int p_port) const {
@@ -1727,33 +1739,46 @@ PropertyInfo VisualScriptPropertyGet::get_input_value_port_info(int p_idx) const
 }
 
 PropertyInfo VisualScriptPropertyGet::get_output_value_port_info(int p_idx) const {
-	List<PropertyInfo> props;
-	ClassDB::get_property_list(_get_base_type(), &props, false);
-	for (List<PropertyInfo>::Element *E = props.front(); E; E = E->next()) {
-		if (E->get().name == property) {
-			return PropertyInfo(E->get().type, "value." + String(index));
+	if (call_mode == CALL_MODE_BASIC_TYPE && p_idx == 0) {
+		return PropertyInfo(basic_type, "pass");
+	} else if (call_mode == CALL_MODE_INSTANCE && p_idx == 0) {
+		return PropertyInfo(Variant::OBJECT, "pass", PROPERTY_HINT_TYPE_STRING, get_base_type());
+	} else {
+		List<PropertyInfo> props;
+		ClassDB::get_property_list(_get_base_type(), &props, false);
+		for (List<PropertyInfo>::Element *E = props.front(); E; E = E->next()) {
+			if (E->get().name == property) {
+				PropertyInfo pinfo = PropertyInfo(E->get().type, String(property) + "." + String(index), E->get().hint, E->get().hint_string);
+				_adjust_input_index(pinfo);
+				return pinfo;
+			}
 		}
 	}
 
-	return PropertyInfo(type_cache, "value");
+	PropertyInfo pinfo = PropertyInfo(type_cache, "value");
+	_adjust_input_index(pinfo);
+	return pinfo;
 }
 
 String VisualScriptPropertyGet::get_caption() const {
-	return String("Get ") + property;
+	String prop = String("Get ") + property;
+	if (index != StringName()) {
+		prop += "." + String(index);
+	}
+
+	return prop;
 }
 
 String VisualScriptPropertyGet::get_text() const {
 	if (call_mode == CALL_MODE_BASIC_TYPE) {
 		return String("On ") + Variant::get_type_name(basic_type);
+	} else if (call_mode == CALL_MODE_INSTANCE) {
+		return String("On ") + base_type;
+	} else if (call_mode == CALL_MODE_NODE_PATH) {
+		return " [" + String(base_path.simplified()) + "]";
+	} else {
+		return "On Self";
 	}
-
-	static const char *cname[3] = {
-		"Self",
-		"Scene Node",
-		"Instance"
-	};
-
-	return String("On ") + cname[call_mode];
 }
 
 void VisualScriptPropertyGet::set_base_type(const StringName &p_type) {
@@ -1933,6 +1958,19 @@ Variant::Type VisualScriptPropertyGet::_get_type_cache() const {
 	return type_cache;
 }
 
+void VisualScriptPropertyGet::_adjust_input_index(PropertyInfo &pinfo) const {
+	if (index != StringName()) {
+		Variant v;
+		Callable::CallError ce;
+		Variant::construct(pinfo.type, v, nullptr, 0, ce);
+		Variant i = v.get(index);
+		pinfo.type = i.get_type();
+		pinfo.name = String(property) + "." + index;
+	} else {
+		pinfo.name = String(property);
+	}
+}
+
 void VisualScriptPropertyGet::set_index(const StringName &p_type) {
 	if (index == p_type) {
 		return;
@@ -2159,15 +2197,17 @@ class VisualScriptNodeInstancePropertyGet : public VisualScriptNodeInstance {
 				bool valid;
 				Variant v = *p_inputs[0];
 
-				*p_outputs[0] = v.get(property, &valid);
+				*p_outputs[1] = v.get(property, &valid);
 				if (index != StringName()) {
-					*p_outputs[0] = p_outputs[0]->get_named(index, valid);
+					*p_outputs[1] = p_outputs[1]->get_named(index, valid);
 				}
 
 				if (!valid) {
 					r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD;
 					r_error_str = RTR("Invalid index property name.");
 				}
+
+				*p_outputs[0] = v;
 			};
 		}
 
@@ -2228,7 +2268,7 @@ int VisualScriptEmitSignal::get_input_value_port_count() const {
 }
 
 int VisualScriptEmitSignal::get_output_value_port_count() const {
-	return 0;
+	return 1;
 }
 
 String VisualScriptEmitSignal::get_output_sequence_port_text(int p_port) const {
@@ -2271,6 +2311,16 @@ StringName VisualScriptEmitSignal::get_signal() const {
 	return name;
 }
 
+void VisualScriptEmitSignal::_adjust_input_index(PropertyInfo &pinfo) const {
+	if (index != StringName()) {
+		Variant v;
+		Callable::CallError ce;
+		Variant::construct(pinfo.type, v, nullptr, 0, ce);
+		Variant i = v.get(index);
+		pinfo.type = i.get_type();
+	}
+}
+
 void VisualScriptEmitSignal::_validate_property(PropertyInfo &property) const {
 	if (property.name == "signal") {
 		property.hint = PROPERTY_HINT_ENUM;
diff --git a/modules/visual_script/visual_script_func_nodes.h b/modules/visual_script/visual_script_func_nodes.h
index eb17be1fbeef..37a707d108dc 100644
--- a/modules/visual_script/visual_script_func_nodes.h
+++ b/modules/visual_script/visual_script_func_nodes.h
@@ -272,6 +272,8 @@ class VisualScriptPropertyGet : public VisualScriptNode {
 	void _set_type_cache(Variant::Type p_type);
 	Variant::Type _get_type_cache() const;
 
+	void _adjust_input_index(PropertyInfo &pinfo) const;
+
 protected:
 	virtual void _validate_property(PropertyInfo &property) const override;
 
@@ -326,6 +328,9 @@ class VisualScriptEmitSignal : public VisualScriptNode {
 
 private:
 	StringName name;
+	StringName index;
+
+	void _adjust_input_index(PropertyInfo &pinfo) const;
 
 protected:
 	virtual void _validate_property(PropertyInfo &property) const override;