Merge pull request #76560 from aaronfranke/node-set-string-name

Change Node `set_name` to use StringName, slightly improves performance

Author: Repiteo

misc/extension_api_validation/4.4-stable.expected

@@ -90,3 +90,10 @@ Validate extension JSON: API was removed: classes/RenderingServer/methods/instan
 Validate extension JSON: API was removed: classes/RenderingServer/methods/instance_reset_physics_interpolation
 
 Functionality moved out of server.
+
+
+GH-76560
+--------
+Validate extension JSON: Error: Field 'classes/Node/methods/set_name/arguments/0': type changed value in new API, from "String" to "StringName".
+
+Change Node `set_name` to use StringName to improve performance. Compatibility method registered.

modules/mono/editor/bindings_generator.cpp

@@ -2732,11 +2732,7 @@ Error BindingsGenerator::_generate_cs_property(const BindingsGenerator::TypeInte
 	if (getter && setter) {
 		const ArgumentInterface &setter_first_arg = setter->arguments.back()->get();
 		if (getter->return_type.cname != setter_first_arg.type.cname) {
-			// Special case for Node::set_name
-			bool whitelisted = getter->return_type.cname == name_cache.type_StringName &&
-					setter_first_arg.type.cname == name_cache.type_String;
-
-			ERR_FAIL_COND_V_MSG(!whitelisted, ERR_BUG,
+			ERR_FAIL_V_MSG(ERR_BUG,
 					"Return type from getter doesn't match first argument of setter for property: '" +
 							p_itype.name + "." + String(p_iprop.cname) + "'.");
 		}

scene/main/node.compat.inc

@@ -0,0 +1,41 @@
+/**************************************************************************/
+/*  node.compat.inc                                                       */
+/**************************************************************************/
+/*                         This file is part of:                          */
+/*                             GODOT ENGINE                               */
+/*                        https://godotengine.org                         */
+/**************************************************************************/
+/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
+/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
+/*                                                                        */
+/* Permission is hereby granted, free of charge, to any person obtaining  */
+/* a copy of this software and associated documentation files (the        */
+/* "Software"), to deal in the Software without restriction, including    */
+/* without limitation the rights to use, copy, modify, merge, publish,    */
+/* distribute, sublicense, and/or sell copies of the Software, and to     */
+/* permit persons to whom the Software is furnished to do so, subject to  */
+/* the following conditions:                                              */
+/*                                                                        */
+/* The above copyright notice and this permission notice shall be         */
+/* included in all copies or substantial portions of the Software.        */
+/*                                                                        */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
+/**************************************************************************/
+
+#ifndef DISABLE_DEPRECATED
+
+void Node::_set_name_bind_compat_76560(const String &p_name) {
+	set_name(p_name);
+}
+
+void Node::_bind_compatibility_methods() {
+	ClassDB::bind_compatibility_method(D_METHOD("set_name", "name"), &Node::_set_name_bind_compat_76560);
+}
+
+#endif

scene/main/node.cpp

@@ -29,6 +29,7 @@
 /**************************************************************************/
 
 #include "node.h"
+#include "node.compat.inc"
 
 #include "core/config/project_settings.h"
 #include "core/io/resource_loader.h"
@@ -1552,17 +1553,23 @@ void Node::_set_name_nocheck(const StringName &p_name) {
 	data.name = p_name;
 }
 
-void Node::set_name(const String &p_name) {
+void Node::set_name(const StringName &p_name) {
 	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Changing the name to nodes inside the SceneTree is only allowed from the main thread. Use `set_name.call_deferred(new_name)`.");
-	String name = p_name.validate_node_name();
-
-	ERR_FAIL_COND(name.is_empty());
+	const StringName old_name = data.name;
+	{
+		const String input_name_str = String(p_name);
+		ERR_FAIL_COND(input_name_str.is_empty());
+		const String validated_node_name_string = input_name_str.validate_node_name();
+		if (input_name_str == validated_node_name_string) {
+			data.name = p_name;
+		} else {
+			data.name = StringName(validated_node_name_string);
+		}
+	}
 
 	if (data.unique_name_in_owner && data.owner) {
 		_release_unique_name_in_owner();
 	}
-	String old_name = data.name;
-	data.name = name;
 
 	if (data.parent) {
 		data.parent->_validate_child_name(this, true);

scene/main/node.h

@@ -410,6 +410,11 @@ class Node : public Object {
 	GDVIRTUAL0RC(RID, _get_focused_accessibility_element)
 	GDVIRTUAL1RC(String, _get_accessibility_container_name, const Node *)
 
+#ifndef DISABLE_DEPRECATED
+	void _set_name_bind_compat_76560(const String &p_name);
+	static void _bind_compatibility_methods();
+#endif
+
 public:
 	enum {
 		// You can make your own, but don't use the same numbers as other notifications in other nodes.
@@ -473,7 +478,7 @@ class Node : public Object {
 
 	StringName get_name() const;
 	String get_description() const;
-	void set_name(const String &p_name);
+	void set_name(const StringName &p_name);
 
 	InternalMode get_internal_mode() const;
 

tests/core/object/test_class_db.h

@@ -359,11 +359,7 @@ void validate_property(const Context &p_context, const ExposedClass &p_class, co
 	if (getter && setter) {
 		const ArgumentData &setter_first_arg = setter->arguments.back()->get();
 		if (getter->return_type.name != setter_first_arg.type.name) {
-			// Special case for Node::set_name
-			bool whitelisted = getter->return_type.name == p_context.names_cache.string_name_type &&
-					setter_first_arg.type.name == p_context.names_cache.string_type;
-
-			TEST_FAIL_COND(!whitelisted,
+			TEST_FAIL(
 					"Return type from getter doesn't match first argument of setter, for property: '", p_class.name, ".", String(p_prop.name), "'.");
 		}
 	}