Merge pull request #46939 from abaire/relaxes_gltf_name_sanitization_3.2

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

core/ustring.cpp

@@ -4111,6 +4111,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 = find_last(".");

core/ustring.h

@@ -340,6 +340,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;

core/variant_call.cpp

@@ -300,6 +300,7 @@ struct _VariantCall {
 	VCALL_LOCALMEM0R(String, json_escape);
 	VCALL_LOCALMEM0R(String, percent_encode);
 	VCALL_LOCALMEM0R(String, percent_decode);
+	VCALL_LOCALMEM0R(String, validate_node_name);
 	VCALL_LOCALMEM0R(String, is_valid_identifier);
 	VCALL_LOCALMEM0R(String, is_valid_integer);
 	VCALL_LOCALMEM0R(String, is_valid_float);
@@ -1646,6 +1647,7 @@ void register_variant_methods() {
 	ADDFUNC0R(STRING, STRING, String, json_escape, varray());
 	ADDFUNC0R(STRING, STRING, String, percent_encode, varray());
 	ADDFUNC0R(STRING, STRING, String, percent_decode, varray());
+	ADDFUNC0R(STRING, STRING, String, validate_node_name, varray());
 	ADDFUNC0R(STRING, BOOL, String, is_valid_identifier, varray());
 	ADDFUNC0R(STRING, BOOL, String, is_valid_integer, varray());
 	ADDFUNC0R(STRING, BOOL, String, is_valid_float, varray());

doc/classes/String.xml

@@ -959,6 +959,13 @@
 				Removes a given string from the end if it ends with it or leaves the string unchanged.
 			</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>

editor/import/editor_scene_importer_gltf.cpp

@@ -40,6 +40,7 @@
 #include "scene/3d/camera.h"
 #include "scene/3d/mesh_instance.h"
 #include "scene/animation/animation_player.h"
+#include "scene/main/node.h"
 #include "scene/resources/surface_tool.h"
 
 uint32_t EditorSceneImporterGLTF::get_import_flags() const {
@@ -155,15 +156,9 @@ static Transform _arr_to_xform(const Array &p_array) {
 	return xform;
 }
 
-String EditorSceneImporterGLTF::_sanitize_scene_name(const String &name) {
-	RegEx regex("([^a-zA-Z0-9_ -]+)");
-	String p_name = regex.sub(name, "", true);
-	return p_name;
-}
-
 String EditorSceneImporterGLTF::_gen_unique_name(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;
@@ -171,7 +166,7 @@ String EditorSceneImporterGLTF::_gen_unique_name(GLTFState &state, const String
 		name = s_name;
 
 		if (index > 1) {
-			name += " " + itos(index);
+			name += itos(index);
 		}
 		if (!state.unique_names.has(name)) {
 			break;
@@ -184,6 +179,40 @@ String EditorSceneImporterGLTF::_gen_unique_name(GLTFState &state, const String
 	return name;
 }
 
+String EditorSceneImporterGLTF::_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 EditorSceneImporterGLTF::_gen_unique_animation_name(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 EditorSceneImporterGLTF::_sanitize_bone_name(const String &name) {
 	String p_name = name.camelcase_to_underscore(true);
 
@@ -2473,7 +2502,7 @@ Error EditorSceneImporterGLTF::_parse_animations(GLTFState &state) {
 			if (name.begins_with("loop") || name.ends_with("loop") || name.begins_with("cycle") || name.ends_with("cycle")) {
 				animation.loop = true;
 			}
-			animation.name = _sanitize_scene_name(name);
+			animation.name = _gen_unique_animation_name(state, name);
 		}
 
 		for (int j = 0; j < channels.size(); j++) {

editor/import/editor_scene_importer_gltf.h

@@ -342,6 +342,7 @@ class EditorSceneImporterGLTF : public EditorSceneImporter {
 		Vector<GLTFLight> lights;
 
 		Set<String> unique_names;
+		Set<String> unique_animation_names;
 
 		Vector<GLTFSkeleton> skeletons;
 		Vector<GLTFAnimation> animations;
@@ -358,9 +359,11 @@ class EditorSceneImporterGLTF : public EditorSceneImporter {
 		}
 	};
 
-	String _sanitize_scene_name(const String &name);
 	String _gen_unique_name(GLTFState &state, const String &p_name);
 
+	String _sanitize_animation_name(const String &p_name);
+	String _gen_unique_animation_name(GLTFState &state, const String &p_name);
+
 	String _sanitize_bone_name(const String &name);
 	String _gen_unique_bone_name(GLTFState &state, const GLTFSkeletonIndex skel_i, const String &p_name);
 

editor/scene_tree_editor.cpp

@@ -785,10 +785,10 @@ 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_minsize();
 
 		if (new_name.empty()) {

scene/main/node.cpp

@@ -927,23 +927,9 @@ 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;

scene/main/node.h

@@ -188,12 +188,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--; }