Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import Hints are no Longer Processed if the Object Name ends with a Number #78881

Closed
ATryder opened this issue Jun 30, 2023 · 4 comments · Fixed by #79050
Closed

Import Hints are no Longer Processed if the Object Name ends with a Number #78881

ATryder opened this issue Jun 30, 2023 · 4 comments · Fixed by #79050

Comments

@ATryder
Copy link

ATryder commented Jun 30, 2023

Godot version

4.1 rc2

System information

Manjaro GL-Compatibility

Issue description

When using import hints, such as -col or -colonly, on object names Godot used to accept the hint even if it was followed by Blender's auto duplicate naming scheme such as .001 or .002. This is no longer the case, items name to the likes of "object-col.001" will not import with collision.

I noticed that, prior to 4.1, Godot would remove periods from imported object names. Object.001 would import as Object001, but now it would import as Object_001. Perhaps this change broke the import hint behavior.

Steps to reproduce

Export a scene with object names using import hints, such as -col or -convcol, where some of the objects have '.001' or similar appended to the end of their names. Import the scene into Godot and create an inherited scene.

Minimal reproduction project

ImportHints.zip

@Calinou
Copy link
Member

Calinou commented Jul 4, 2023

I can confirm this on 4.1.dev1-4.1.rc1 and 4.1.rc 4642448 using https://github.com/godotengine/tps-demo. This does not occur on 4.0.3.stable.

I bisected the regression to 223ce4f. cc @reduz

@akien-mga
Copy link
Member

I bisected the regression to 223ce4f. cc @reduz

As pointed out in the OP, this commit made it so that . in node names gets replaced by _ instead of simply removed.

So the logic here needs to be changed to take this into account:

static bool _teststr(const String &p_what, const String &p_str) {
String what = p_what;
//remove trailing spaces and numbers, some apps like blender add ".number" to duplicates so also compensate for this
while (what.length() && (is_digit(what[what.length() - 1]) || what[what.length() - 1] <= 32 || what[what.length() - 1] == '.')) {
what = what.substr(0, what.length() - 1);
}
if (what.findn("$" + p_str) != -1) { //blender and other stuff
return true;
}
if (what.to_lower().ends_with("-" + p_str)) { //collada only supports "_" and "-" besides letters
return true;
}
if (what.to_lower().ends_with("_" + p_str)) { //collada only supports "_" and "-" besides letters
return true;
}
return false;
}
static String _fixstr(const String &p_what, const String &p_str) {
String what = p_what;
//remove trailing spaces and numbers, some apps like blender add ".number" to duplicates so also compensate for this
while (what.length() && (is_digit(what[what.length() - 1]) || what[what.length() - 1] <= 32 || what[what.length() - 1] == '.')) {
what = what.substr(0, what.length() - 1);
}
String end = p_what.substr(what.length(), p_what.length() - what.length());
if (what.findn("$" + p_str) != -1) { //blender and other stuff
return what.replace("$" + p_str, "") + end;
}
if (what.to_lower().ends_with("-" + p_str)) { //collada only supports "_" and "-" besides letters
return what.substr(0, what.length() - (p_str.length() + 1)) + end;
}
if (what.to_lower().ends_with("_" + p_str)) { //collada only supports "_" and "-" besides letters
return what.substr(0, what.length() - (p_str.length() + 1)) + end;
}
return what;
}

@godotengine/import

@fire
Copy link
Member

fire commented Jul 4, 2023

I'm not able to rush fix this for a RC build. How to proceed?

@akien-mga
Copy link
Member

I think this will have to be a known issue in 4.1 and we'll fix it in 4.1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants