Relaxes Node naming constraints in glTF documents to match the Editor.

12 files changed, 99 insertions, 34 deletions

diff --git a/core/string/ustring.cpp b/core/string/ustring.cpp
index 59fda65d43..5118adf3da 100644
--- a/core/string/ustring.cpp
+++ b/core/string/ustring.cpp
@@ -4364,6 +4364,18 @@ String String::property_name_encode() const {
 	return *this;
 }
 
+// Changes made to the set of invalid characters must also be reflected in the String documentation.
+const String String::invalid_node_name_characters = ". : @ / \"";
+
+String String::validate_node_name() const {
+	Vector<String> chars = String::invalid_node_name_characters.split(" ");
+	String name = this->replace(chars[0], "");
+	for (int i = 1; i < chars.size(); i++) {
+		name = name.replace(chars[i], "");
+	}
+	return name;
+}
+
 String String::get_basename() const {
 	int pos = rfind(".");
 	if (pos < 0 || pos < MAX(rfind("/"), rfind("\\"))) {
diff --git a/core/string/ustring.h b/core/string/ustring.h
index 821941036f..1e362d7683 100644
--- a/core/string/ustring.h
+++ b/core/string/ustring.h
@@ -419,6 +419,10 @@ public:
 
 	String property_name_encode() const;
 
+	// node functions
+	static const String invalid_node_name_characters;
+	String validate_node_name() const;
+
 	bool is_valid_identifier() const;
 	bool is_valid_integer() const;
 	bool is_valid_float() const;
diff --git a/core/variant/variant_call.cpp b/core/variant/variant_call.cpp
index 8f2cba138b..4df7a26b6d 100644
--- a/core/variant/variant_call.cpp
+++ b/core/variant/variant_call.cpp
@@ -956,6 +956,8 @@ static void _register_variant_builtin_methods() {
 	bind_method(String, c_unescape, sarray(), varray());
 	bind_method(String, json_escape, sarray(), varray());
 
+	bind_method(String, validate_node_name, sarray(), varray());
+
 	bind_method(String, is_valid_identifier, sarray(), varray());
 	bind_method(String, is_valid_integer, sarray(), varray());
 	bind_method(String, is_valid_float, sarray(), varray());
diff --git a/doc/classes/String.xml b/doc/classes/String.xml
index c03f6357ab..ac389b0fbf 100644
--- a/doc/classes/String.xml
+++ b/doc/classes/String.xml
@@ -887,6 +887,13 @@
 				[/codeblocks]
 			</description>
 		</method>
+		<method name="validate_node_name">
+			<return type="String">
+			</return>
+			<description>
+				Removes any characters from the string that are prohibited in [Node] names ([code].[/code] [code]:[/code] [code]@[/code] [code]/[/code] [code]"[/code]).
+			</description>
+		</method>
 		<method name="xml_escape">
 			<return type="String">
 			</return>
diff --git a/editor/scene_tree_editor.cpp b/editor/scene_tree_editor.cpp
index ce44a4bca1..02f88bee0e 100644
--- a/editor/scene_tree_editor.cpp
+++ b/editor/scene_tree_editor.cpp
@@ -767,9 +767,11 @@ void SceneTreeEditor::_renamed() {
 		return;
 	}
 
-	String new_name = which->get_text(0);
-	if (!Node::_validate_node_name(new_name)) {
-		error->set_text(TTR("Invalid node name, the following characters are not allowed:") + "\n" + Node::invalid_character);
+	String raw_new_name = which->get_text(0);
+	String new_name = raw_new_name.validate_node_name();
+
+	if (new_name != raw_new_name) {
+		error->set_text(TTR("Invalid node name, the following characters are not allowed:") + "\n" + String::invalid_node_name_characters);
 		error->popup_centered();
 
 		if (new_name.is_empty()) {
diff --git a/modules/gltf/gltf_document.cpp b/modules/gltf/gltf_document.cpp
index 4868347a74..0a4d4055b4 100644
--- a/modules/gltf/gltf_document.cpp
+++ b/modules/gltf/gltf_document.cpp
@@ -72,6 +72,7 @@
 #include "scene/3d/node_3d.h"
 #include "scene/3d/skeleton_3d.h"
 #include "scene/animation/animation_player.h"
+#include "scene/main/node.h"
 #include "scene/resources/surface_tool.h"
 #include <limits>
 
@@ -449,14 +450,8 @@ Error GLTFDocument::_serialize_nodes(Ref<GLTFState> state) {
 	return OK;
 }
 
-String GLTFDocument::_sanitize_scene_name(const String &name) {
-	RegEx regex("([^a-zA-Z0-9_ -]+)");
-	String p_name = regex.sub(name, "", true);
-	return p_name;
-}
-
 String GLTFDocument::_gen_unique_name(Ref<GLTFState> state, const String &p_name) {
-	const String s_name = _sanitize_scene_name(p_name);
+	const String s_name = p_name.validate_node_name();
 
 	String name;
 	int index = 1;
@@ -464,7 +459,7 @@ String GLTFDocument::_gen_unique_name(Ref<GLTFState> state, const String &p_name
 		name = s_name;
 
 		if (index > 1) {
-			name += " " + itos(index);
+			name += itos(index);
 		}
 		if (!state->unique_names.has(name)) {
 			break;
@@ -477,6 +472,39 @@ String GLTFDocument::_gen_unique_name(Ref<GLTFState> state, const String &p_name
 	return name;
 }
 
+String GLTFDocument::_sanitize_animation_name(const String &p_name) {
+	// Animations disallow the normal node invalid characters as well as  "," and "["
+	// (See animation/animation_player.cpp::add_animation)
+
+	// TODO: Consider adding invalid_characters or a validate_animation_name to animation_player to mirror Node.
+	String name = p_name.validate_node_name();
+	name = name.replace(",", "");
+	name = name.replace("[", "");
+	return name;
+}
+
+String GLTFDocument::_gen_unique_animation_name(Ref<GLTFState> state, const String &p_name) {
+	const String s_name = _sanitize_animation_name(p_name);
+
+	String name;
+	int index = 1;
+	while (true) {
+		name = s_name;
+
+		if (index > 1) {
+			name += itos(index);
+		}
+		if (!state->unique_animation_names.has(name)) {
+			break;
+		}
+		index++;
+	}
+
+	state->unique_animation_names.insert(name);
+
+	return name;
+}
+
 String GLTFDocument::_sanitize_bone_name(const String &name) {
 	String p_name = name.camelcase_to_underscore(true);
 
@@ -4729,7 +4757,7 @@ Error GLTFDocument::_parse_animations(Ref<GLTFState> state) {
 			if (name.begins_with("loop") || name.ends_with("loop") || name.begins_with("cycle") || name.ends_with("cycle")) {
 				animation->set_loop(true);
 			}
-			animation->set_name(_sanitize_scene_name(name));
+			animation->set_name(_gen_unique_animation_name(state, name));
 		}
 
 		for (int j = 0; j < channels.size(); j++) {
diff --git a/modules/gltf/gltf_document.h b/modules/gltf/gltf_document.h
index ddf307e6a7..bda1ce87d6 100644
--- a/modules/gltf/gltf_document.h
+++ b/modules/gltf/gltf_document.h
@@ -162,8 +162,9 @@ private:
 	Error _parse_nodes(Ref<GLTFState> state);
 	String _get_type_name(const GLTFType p_component);
 	String _get_accessor_type_name(const GLTFDocument::GLTFType p_type);
-	String _sanitize_scene_name(const String &name);
 	String _gen_unique_name(Ref<GLTFState> state, const String &p_name);
+	String _sanitize_animation_name(const String &name);
+	String _gen_unique_animation_name(Ref<GLTFState> state, const String &p_name);
 	String _sanitize_bone_name(const String &name);
 	String _gen_unique_bone_name(Ref<GLTFState> state,
 			const GLTFSkeletonIndex skel_i,
diff --git a/modules/gltf/gltf_state.cpp b/modules/gltf/gltf_state.cpp
index eedc743330..c2336bc913 100644
--- a/modules/gltf/gltf_state.cpp
+++ b/modules/gltf/gltf_state.cpp
@@ -71,6 +71,8 @@ void GLTFState::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_lights", "lights"), &GLTFState::set_lights);
 	ClassDB::bind_method(D_METHOD("get_unique_names"), &GLTFState::get_unique_names);
 	ClassDB::bind_method(D_METHOD("set_unique_names", "unique_names"), &GLTFState::set_unique_names);
+	ClassDB::bind_method(D_METHOD("get_unique_animation_names"), &GLTFState::get_unique_animation_names);
+	ClassDB::bind_method(D_METHOD("set_unique_animation_names", "unique_animation_names"), &GLTFState::set_unique_animation_names);
 	ClassDB::bind_method(D_METHOD("get_skeletons"), &GLTFState::get_skeletons);
 	ClassDB::bind_method(D_METHOD("set_skeletons", "skeletons"), &GLTFState::set_skeletons);
 	ClassDB::bind_method(D_METHOD("get_skeleton_to_node"), &GLTFState::get_skeleton_to_node);
@@ -98,6 +100,7 @@ void GLTFState::_bind_methods() {
 	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "cameras", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_cameras", "get_cameras"); // Vector<Ref<GLTFCamera>>
 	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "lights", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_lights", "get_lights"); // Vector<Ref<GLTFLight>>
 	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "unique_names", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_unique_names", "get_unique_names"); // Set<String>
+	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "unique_animation_names", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_unique_animation_names", "get_unique_animation_names"); // Set<String>
 	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "skeletons", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_skeletons", "get_skeletons"); // Vector<Ref<GLTFSkeleton>>
 	ADD_PROPERTY(PropertyInfo(Variant::DICTIONARY, "skeleton_to_node", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_skeleton_to_node", "get_skeleton_to_node"); // Map<GLTFSkeletonIndex,
 	ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "animations", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_animations", "get_animations"); // Vector<Ref<GLTFAnimation>>
@@ -255,6 +258,14 @@ void GLTFState::set_unique_names(Array p_unique_names) {
 	GLTFDocument::set_from_array(unique_names, p_unique_names);
 }
 
+Array GLTFState::get_unique_animation_names() {
+	return GLTFDocument::to_array(unique_animation_names);
+}
+
+void GLTFState::set_unique_animation_names(Array p_unique_animation_names) {
+	GLTFDocument::set_from_array(unique_animation_names, p_unique_animation_names);
+}
+
 Array GLTFState::get_skeletons() {
 	return GLTFDocument::to_array(skeletons);
 }
diff --git a/modules/gltf/gltf_state.h b/modules/gltf/gltf_state.h
index 4ce5aa9491..9030962b03 100644
--- a/modules/gltf/gltf_state.h
+++ b/modules/gltf/gltf_state.h
@@ -80,6 +80,7 @@ class GLTFState : public Resource {
 	Vector<Ref<GLTFCamera>> cameras;
 	Vector<Ref<GLTFLight>> lights;
 	Set<String> unique_names;
+	Set<String> unique_animation_names;
 
 	Vector<Ref<GLTFSkeleton>> skeletons;
 	Map<GLTFSkeletonIndex, GLTFNodeIndex> skeleton_to_node;
@@ -147,6 +148,9 @@ public:
 	Array get_unique_names();
 	void set_unique_names(Array p_unique_names);
 
+	Array get_unique_animation_names();
+	void set_unique_animation_names(Array p_unique_names);
+
 	Array get_skeletons();
 	void set_skeletons(Array p_skeletons);
 
diff --git a/scene/main/node.cpp b/scene/main/node.cpp
index 9ac3b4a691..1f5b54d89e 100644
--- a/scene/main/node.cpp
+++ b/scene/main/node.cpp
@@ -981,22 +981,8 @@ void Node::_set_name_nocheck(const StringName &p_name) {
 	data.name = p_name;
 }
 
-String Node::invalid_character = ". : @ / \"";
-
-bool Node::_validate_node_name(String &p_name) {
-	String name = p_name;
-	Vector<String> chars = Node::invalid_character.split(" ");
-	for (int i = 0; i < chars.size(); i++) {
-		name = name.replace(chars[i], "");
-	}
-	bool is_valid = name == p_name;
-	p_name = name;
-	return is_valid;
-}
-
 void Node::set_name(const String &p_name) {
-	String name = p_name;
-	_validate_node_name(name);
+	String name = p_name.validate_node_name();
 
 	ERR_FAIL_COND(name == "");
 	data.name = name;
diff --git a/scene/main/node.h b/scene/main/node.h
index 5253ed2e45..2bd7ca72fe 100644
--- a/scene/main/node.h
+++ b/scene/main/node.h
@@ -185,12 +185,6 @@ private:
 
 	void _set_tree(SceneTree *p_tree);
 
-#ifdef TOOLS_ENABLED
-	friend class SceneTreeEditor;
-#endif
-	static String invalid_character;
-	static bool _validate_node_name(String &p_name);
-
 protected:
 	void _block() { data.blocked++; }
 	void _unblock() { data.blocked--; }
diff --git a/tests/test_string.h b/tests/test_string.h
index cc3152203e..6d630bde1c 100644
--- a/tests/test_string.h
+++ b/tests/test_string.h
@@ -1272,6 +1272,20 @@ TEST_CASE("[String] humanize_size") {
 	CHECK(String::humanize_size(100523550) == "95.86 MiB");
 	CHECK(String::humanize_size(5345555000) == "4.97 GiB");
 }
+
+TEST_CASE("[String] validate_node_name") {
+	String numeric_only = "12345";
+	CHECK(numeric_only.validate_node_name() == "12345");
+
+	String name_with_spaces = "Name with spaces";
+	CHECK(name_with_spaces.validate_node_name() == "Name with spaces");
+
+	String name_with_kana = "Name with kana ゴドツ";
+	CHECK(name_with_kana.validate_node_name() == "Name with kana ゴドツ");
+
+	String name_with_invalid_chars = "Name with invalid characters :.@removed!";
+	CHECK(name_with_invalid_chars.validate_node_name() == "Name with invalid characters removed!");
+}
 } // namespace TestString
 
 #endif // TEST_STRING_H