Merge pull request #57205 from TechnoPorg/variant-template-cast

Allow method binds to take Object subclasses as arguments

core/io/resource.h

@@ -37,6 +37,8 @@
 #include "core/templates/safe_refcount.h"
 #include "core/templates/self_list.h"
 
+class Node;
+
 #define RES_BASE_EXTENSION(m_ext)                                                                                   \
 public:                                                                                                             \
 	static void register_custom_data_to_otdb() { ClassDB::add_resource_base_extension(m_ext, get_class_static()); } \

core/variant/binder_common.h

@@ -44,24 +44,42 @@
 
 #include <stdio.h>
 
+// Variant cannot define an implicit cast operator for every Object subclass, so the
+// casting is done here, to allow binding methods with parameters more specific than Object *
+
 template <class T>
 struct VariantCaster {
 	static _FORCE_INLINE_ T cast(const Variant &p_variant) {
-		return p_variant;
+		using TStripped = std::remove_pointer_t<T>;
+		if constexpr (std::is_base_of<Object, TStripped>::value) {
+			return Object::cast_to<TStripped>(p_variant);
+		} else {
+			return p_variant;
+		}
 	}
 };
 
 template <class T>
 struct VariantCaster<T &> {
 	static _FORCE_INLINE_ T cast(const Variant &p_variant) {
-		return p_variant;
+		using TStripped = std::remove_pointer_t<T>;
+		if constexpr (std::is_base_of<Object, TStripped>::value) {
+			return Object::cast_to<TStripped>(p_variant);
+		} else {
+			return p_variant;
+		}
 	}
 };
 
 template <class T>
 struct VariantCaster<const T &> {
 	static _FORCE_INLINE_ T cast(const Variant &p_variant) {
-		return p_variant;
+		using TStripped = std::remove_pointer_t<T>;
+		if constexpr (std::is_base_of<Object, TStripped>::value) {
+			return Object::cast_to<TStripped>(p_variant);
+		} else {
+			return p_variant;
+		}
 	}
 };
 
@@ -135,7 +153,13 @@ struct PtrToArg<char32_t> {
 template <typename T>
 struct VariantObjectClassChecker {
 	static _FORCE_INLINE_ bool check(const Variant &p_variant) {
-		return true;
+		using TStripped = std::remove_pointer_t<T>;
+		if constexpr (std::is_base_of<Object, TStripped>::value) {
+			Object *obj = p_variant;
+			return Object::cast_to<TStripped>(p_variant) || !obj;
+		} else {
+			return true;
+		}
 	}
 };
 
@@ -151,24 +175,6 @@ struct VariantObjectClassChecker<const Ref<T> &> {
 	}
 };
 
-template <>
-struct VariantObjectClassChecker<Node *> {
-	static _FORCE_INLINE_ bool check(const Variant &p_variant) {
-		Object *obj = p_variant;
-		Node *node = p_variant;
-		return node || !obj;
-	}
-};
-
-template <>
-struct VariantObjectClassChecker<Control *> {
-	static _FORCE_INLINE_ bool check(const Variant &p_variant) {
-		Object *obj = p_variant;
-		Control *control = p_variant;
-		return control || !obj;
-	}
-};
-
 #ifdef DEBUG_METHODS_ENABLED
 
 template <class T>

core/variant/variant.cpp

@@ -38,8 +38,6 @@
 #include "core/math/math_funcs.h"
 #include "core/string/print_string.h"
 #include "core/variant/variant_parser.h"
-#include "scene/gui/control.h"
-#include "scene/main/node.h"
 
 String Variant::get_type_name(Variant::Type p_type) {
 	switch (p_type) {
@@ -2004,22 +2002,6 @@ Object *Variant::get_validated_object() const {
 	}
 }
 
-Variant::operator Node *() const {
-	if (type == OBJECT) {
-		return Object::cast_to<Node>(_get_obj().obj);
-	} else {
-		return nullptr;
-	}
-}
-
-Variant::operator Control *() const {
-	if (type == OBJECT) {
-		return Object::cast_to<Control>(_get_obj().obj);
-	} else {
-		return nullptr;
-	}
-}
-
 Variant::operator Dictionary() const {
 	if (type == DICTIONARY) {
 		return *reinterpret_cast<const Dictionary *>(_data._mem);
@@ -3414,7 +3396,7 @@ String Variant::get_call_error_text(Object *p_base, const StringName &p_method,
 	}
 
 	String class_name = p_base->get_class();
-	Ref<Script> script = p_base->get_script();
+	Ref<Resource> script = p_base->get_script();
 	if (script.is_valid() && script->get_path().is_resource_file()) {
 		class_name += "(" + script->get_path().get_file() + ")";
 	}

core/variant/variant.h

@@ -53,8 +53,6 @@
 #include "core/variant/dictionary.h"
 
 class Object;
-class Node; // helper
-class Control; // helper
 
 struct PropertyInfo;
 struct MethodInfo;
@@ -339,8 +337,6 @@ class Variant {
 	operator ::RID() const;
 
 	operator Object *() const;
-	operator Node *() const;
-	operator Control *() const;
 
 	operator Callable() const;
 	operator Signal() const;

doc/classes/Tree.xml

@@ -50,7 +50,7 @@
 		</method>
 		<method name="create_item">
 			<return type="TreeItem" />
-			<argument index="0" name="parent" type="Object" default="null" />
+			<argument index="0" name="parent" type="TreeItem" default="null" />
 			<argument index="1" name="idx" type="int" default="-1" />
 			<description>
 				Creates an item in the tree and adds it as a child of [code]parent[/code], which can be either a valid [TreeItem] or [code]null[/code].
@@ -170,7 +170,7 @@
 		</method>
 		<method name="get_item_area_rect" qualifiers="const">
 			<return type="Rect2" />
-			<argument index="0" name="item" type="Object" />
+			<argument index="0" name="item" type="TreeItem" />
 			<argument index="1" name="column" type="int" default="-1" />
 			<description>
 				Returns the rectangle area for the specified [TreeItem]. If [code]column[/code] is specified, only get the position and size of that column, otherwise get the rectangle containing all columns.
@@ -185,7 +185,7 @@
 		</method>
 		<method name="get_next_selected">
 			<return type="TreeItem" />
-			<argument index="0" name="from" type="Object" />
+			<argument index="0" name="from" type="TreeItem" />
 			<description>
 				Returns the next selected [TreeItem] after the given one, or [code]null[/code] if the end is reached.
 				If [code]from[/code] is [code]null[/code], this returns the first selected item.
@@ -239,7 +239,7 @@
 		</method>
 		<method name="scroll_to_item">
 			<return type="void" />
-			<argument index="0" name="item" type="Object" />
+			<argument index="0" name="item" type="TreeItem" />
 			<description>
 				Causes the [Tree] to jump to the specified [TreeItem].
 			</description>

editor/editor_properties.cpp

@@ -2753,7 +2753,7 @@ void EditorPropertyNodePath::_node_selected(const NodePath &p_path) {
 	}
 
 	if (!base_node && get_edited_object()->has_method("get_root_path")) {
-		base_node = get_edited_object()->call("get_root_path");
+		base_node = Object::cast_to<Node>(get_edited_object()->call("get_root_path"));
 	}
 
 	if (!base_node && Object::cast_to<RefCounted>(get_edited_object())) {

editor/plugins/canvas_item_editor_plugin.cpp

@@ -3949,7 +3949,7 @@ void CanvasItemEditor::_selection_changed() {
 
 void CanvasItemEditor::edit(CanvasItem *p_canvas_item) {
 	Array selection = editor_selection->get_selected_nodes();
-	if (selection.size() != 1 || (Node *)selection[0] != p_canvas_item) {
+	if (selection.size() != 1 || Object::cast_to<Node>(selection[0]) != p_canvas_item) {
 		drag_type = DRAG_NONE;
 
 		// Clear the selection

editor/plugins/node_3d_editor_plugin.cpp

@@ -6623,7 +6623,7 @@ void Node3DEditor::snap_selected_nodes_to_floor() {
 		// For snapping to be performed, there must be solid geometry under at least one of the selected nodes.
 		// We need to check this before snapping to register the undo/redo action only if needed.
 		for (int i = 0; i < keys.size(); i++) {
-			Node *node = keys[i];
+			Node *node = Object::cast_to<Node>(keys[i]);
 			Node3D *sp = Object::cast_to<Node3D>(node);
 			Dictionary d = snap_data[node];
 			Vector3 from = d["from"];
@@ -6645,7 +6645,7 @@ void Node3DEditor::snap_selected_nodes_to_floor() {
 
 			// Perform snapping if at least one node can be snapped
 			for (int i = 0; i < keys.size(); i++) {
-				Node *node = keys[i];
+				Node *node = Object::cast_to<Node>(keys[i]);
 				Node3D *sp = Object::cast_to<Node3D>(node);
 				Dictionary d = snap_data[node];
 				Vector3 from = d["from"];

editor/plugins/script_editor_plugin.cpp

@@ -2859,7 +2859,7 @@ bool ScriptEditor::can_drop_data_fw(const Point2 &p_point, const Variant &p_data
 	}
 
 	if (String(d["type"]) == "script_list_element") {
-		Node *node = d["script_list_element"];
+		Node *node = Object::cast_to<Node>(d["script_list_element"]);
 
 		ScriptEditorBase *se = Object::cast_to<ScriptEditorBase>(node);
 		if (se) {
@@ -2932,7 +2932,7 @@ void ScriptEditor::drop_data_fw(const Point2 &p_point, const Variant &p_data, Co
 	}
 
 	if (String(d["type"]) == "script_list_element") {
-		Node *node = d["script_list_element"];
+		Node *node = Object::cast_to<Node>(d["script_list_element"]);
 
 		ScriptEditorBase *se = Object::cast_to<ScriptEditorBase>(node);
 		EditorHelp *eh = Object::cast_to<EditorHelp>(node);

editor/rename_dialog.cpp

@@ -363,7 +363,7 @@ void RenameDialog::_post_popup() {
 	Array selected_node_list = editor_selection->get_selected_nodes();
 	ERR_FAIL_COND(selected_node_list.size() == 0);
 
-	preview_node = selected_node_list[0];
+	preview_node = Object::cast_to<Node>(selected_node_list[0]);
 
 	_update_preview();
 	_update_substitute();

modules/gdscript/language_server/gdscript_workspace.cpp

@@ -585,7 +585,7 @@ void GDScriptWorkspace::completion(const lsp::CompletionParams &p_params, List<S
 			stack.push_back(owner_scene_node);
 
 			while (!stack.is_empty()) {
-				current = stack.pop_back();
+				current = Object::cast_to<Node>(stack.pop_back());
 				Ref<GDScript> script = current->get_script();
 				if (script.is_valid() && script->get_path() == path) {
 					break;

scene/debugger/scene_debugger.h

@@ -37,6 +37,7 @@
 #include "core/variant/array.h"
 
 class Script;
+class Node;
 
 class SceneDebugger {
 public:

scene/gui/tree.cpp

@@ -4813,7 +4813,7 @@ bool Tree::get_allow_reselect() const {
 
 void Tree::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("clear"), &Tree::clear);
-	ClassDB::bind_method(D_METHOD("create_item", "parent", "idx"), &Tree::_create_item, DEFVAL(Variant()), DEFVAL(-1));
+	ClassDB::bind_method(D_METHOD("create_item", "parent", "idx"), &Tree::create_item, DEFVAL(Variant()), DEFVAL(-1));
 
 	ClassDB::bind_method(D_METHOD("get_root"), &Tree::get_root);
 	ClassDB::bind_method(D_METHOD("set_column_custom_minimum_width", "column", "min_width"), &Tree::set_column_custom_minimum_width);
@@ -4828,7 +4828,7 @@ void Tree::_bind_methods() {
 
 	ClassDB::bind_method(D_METHOD("set_hide_root", "enable"), &Tree::set_hide_root);
 	ClassDB::bind_method(D_METHOD("is_root_hidden"), &Tree::is_root_hidden);
-	ClassDB::bind_method(D_METHOD("get_next_selected", "from"), &Tree::_get_next_selected);
+	ClassDB::bind_method(D_METHOD("get_next_selected", "from"), &Tree::get_next_selected);
 	ClassDB::bind_method(D_METHOD("get_selected"), &Tree::get_selected);
 	ClassDB::bind_method(D_METHOD("get_selected_column"), &Tree::get_selected_column);
 	ClassDB::bind_method(D_METHOD("get_pressed_button"), &Tree::get_pressed_button);
@@ -4842,7 +4842,7 @@ void Tree::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_edited_column"), &Tree::get_edited_column);
 	ClassDB::bind_method(D_METHOD("edit_selected"), &Tree::edit_selected);
 	ClassDB::bind_method(D_METHOD("get_custom_popup_rect"), &Tree::get_custom_popup_rect);
-	ClassDB::bind_method(D_METHOD("get_item_area_rect", "item", "column"), &Tree::_get_item_rect, DEFVAL(-1));
+	ClassDB::bind_method(D_METHOD("get_item_area_rect", "item", "column"), &Tree::get_item_rect, DEFVAL(-1));
 	ClassDB::bind_method(D_METHOD("get_item_at_position", "position"), &Tree::get_item_at_position);
 	ClassDB::bind_method(D_METHOD("get_column_at_position", "position"), &Tree::get_column_at_position);
 	ClassDB::bind_method(D_METHOD("get_drop_section_at_position", "position"), &Tree::get_drop_section_at_position);
@@ -4866,7 +4866,7 @@ void Tree::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_column_title_language", "column"), &Tree::get_column_title_language);
 
 	ClassDB::bind_method(D_METHOD("get_scroll"), &Tree::get_scroll);
-	ClassDB::bind_method(D_METHOD("scroll_to_item", "item"), &Tree::_scroll_to_item);
+	ClassDB::bind_method(D_METHOD("scroll_to_item", "item"), &Tree::scroll_to_item);
 
 	ClassDB::bind_method(D_METHOD("set_h_scroll_enabled", "h_scroll"), &Tree::set_h_scroll_enabled);
 	ClassDB::bind_method(D_METHOD("is_h_scroll_enabled"), &Tree::is_h_scroll_enabled);

scene/gui/tree.h

@@ -617,23 +617,6 @@ class Tree : public Control {
 protected:
 	static void _bind_methods();
 
-	//bind helpers
-	TreeItem *_create_item(Object *p_parent, int p_idx = -1) {
-		return create_item(Object::cast_to<TreeItem>(p_parent), p_idx);
-	}
-
-	TreeItem *_get_next_selected(Object *p_item) {
-		return get_next_selected(Object::cast_to<TreeItem>(p_item));
-	}
-
-	Rect2 _get_item_rect(Object *p_item, int p_column) const {
-		return get_item_rect(Object::cast_to<TreeItem>(p_item), p_column);
-	}
-
-	void _scroll_to_item(Object *p_item) {
-		scroll_to_item(Object::cast_to<TreeItem>(p_item));
-	}
-
 public:
 	virtual void gui_input(const Ref<InputEvent> &p_event) override;
 

tests/core/object/test_method_bind.h

@@ -51,9 +51,15 @@ class MethodBindTester : public Object {
 		TEST_METHODRC,
 		TEST_METHODRC_ARGS,
 		TEST_METHOD_DEFARGS,
+		TEST_METHOD_OBJECT_CAST,
 		TEST_MAX
 	};
 
+	class ObjectSubclass : public Object {
+	public:
+		int value = 1;
+	};
+
 	int test_num = 0;
 
 	bool test_valid[TEST_MAX];
@@ -98,6 +104,10 @@ class MethodBindTester : public Object {
 		test_valid[TEST_METHOD_DEFARGS] = p_arg1 == 1 && p_arg2 == 2 && p_arg3 == 3 && p_arg4 == 4 && p_arg5 == 5; //temporary
 	}
 
+	void test_method_object_cast(ObjectSubclass *p_object) {
+		test_valid[TEST_METHOD_OBJECT_CAST] = p_object->value == 1;
+	}
+
 	static void _bind_methods() {
 		ClassDB::bind_method(D_METHOD("test_method"), &MethodBindTester::test_method);
 		ClassDB::bind_method(D_METHOD("test_method_args"), &MethodBindTester::test_method_args);
@@ -108,6 +118,7 @@ class MethodBindTester : public Object {
 		ClassDB::bind_method(D_METHOD("test_methodrc"), &MethodBindTester::test_methodrc);
 		ClassDB::bind_method(D_METHOD("test_methodrc_args"), &MethodBindTester::test_methodrc_args);
 		ClassDB::bind_method(D_METHOD("test_method_default_args"), &MethodBindTester::test_method_default_args, DEFVAL(9) /* wrong on purpose */, DEFVAL(4), DEFVAL(5));
+		ClassDB::bind_method(D_METHOD("test_method_object_cast", "object"), &MethodBindTester::test_method_object_cast);
 	}
 
 	virtual void run_tests() {
@@ -134,6 +145,10 @@ class MethodBindTester : public Object {
 		test_valid[TEST_METHODRC_ARGS] = int(call("test_methodrc_args", test_num)) == test_num && test_valid[TEST_METHODRC_ARGS];
 
 		call("test_method_default_args", 1, 2, 3, 4);
+
+		ObjectSubclass *obj = memnew(ObjectSubclass);
+		call("test_method_object_cast", obj);
+		memdelete(obj);
 	}
 };
 
@@ -152,6 +167,7 @@ TEST_CASE("[MethodBind] check all method binds") {
 	CHECK(mbt->test_valid[MethodBindTester::TEST_METHODRC]);
 	CHECK(mbt->test_valid[MethodBindTester::TEST_METHODRC_ARGS]);
 	CHECK(mbt->test_valid[MethodBindTester::TEST_METHOD_DEFARGS]);
+	CHECK(mbt->test_valid[MethodBindTester::TEST_METHOD_OBJECT_CAST]);
 
 	memdelete(mbt);
 }