Merge pull request #45545 from abaire/relaxes_gltf_name_sanitization

Relaxes node name sanitization in gltf documents.
This commit is contained in:
Rémi Verschelde 2021-03-09 14:54:33 +01:00 committed by GitHub
commit 83b1acdc60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 99 additions and 34 deletions

View File

@ -4394,6 +4394,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("\\"))) {

View File

@ -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;

View File

@ -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());

View File

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

View File

@ -776,9 +776,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()) {

View File

@ -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++) {

View File

@ -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,

View File

@ -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);
}

View File

@ -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);

View File

@ -1021,22 +1021,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;

View File

@ -190,12 +190,6 @@ private:
_FORCE_INLINE_ bool _can_process(bool p_paused) const;
#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--; }

View File

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