From 61cc1c8624cdf2ef56b807c70f76dd96cc0ebcb7 Mon Sep 17 00:00:00 2001 From: abaire Date: Thu, 28 Jan 2021 12:48:12 -0800 Subject: [PATCH] Relaxes Node naming constraints in glTF documents to match the Editor. --- core/string/ustring.cpp | 12 +++++++++ core/string/ustring.h | 4 +++ core/variant/variant_call.cpp | 2 ++ doc/classes/String.xml | 7 ++++++ editor/scene_tree_editor.cpp | 8 +++--- modules/gltf/gltf_document.cpp | 46 +++++++++++++++++++++++++++------- modules/gltf/gltf_document.h | 3 ++- modules/gltf/gltf_state.cpp | 11 ++++++++ modules/gltf/gltf_state.h | 4 +++ scene/main/node.cpp | 16 +----------- scene/main/node.h | 6 ----- tests/test_string.h | 14 +++++++++++ 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 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] + + + + + Removes any characters from the string that are prohibited in [Node] names ([code].[/code] [code]:[/code] [code]@[/code] [code]/[/code] [code]"[/code]). + + 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 @@ -449,14 +450,8 @@ Error GLTFDocument::_serialize_nodes(Ref 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 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 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 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 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 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 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 state, const String &p_name); + String _sanitize_animation_name(const String &name); + String _gen_unique_animation_name(Ref state, const String &p_name); String _sanitize_bone_name(const String &name); String _gen_unique_bone_name(Ref 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> ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "lights", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_lights", "get_lights"); // Vector> 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 + 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 ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "skeletons", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_EDITOR), "set_skeletons", "get_skeletons"); // Vector> 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> @@ -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> cameras; Vector> lights; Set unique_names; + Set unique_animation_names; Vector> skeletons; Map 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 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