Fix import hints that are followed by dot.number

[capnm]

Fixes #78881.

[dalexeev]

Could you please also update the docs: String.validate_node_name(), StringName.validate_node_name()?

Returns a copy of the string with all characters that are not allowed in Node.name removed (. : @ / " %).

# 4.0.3: 123
# master: 1_2_3
print("1.2@3".validate_node_name())

This is not directly related to the PR title, but it is also a consequence of #75760.

[akien-mga]

So I did some testing, it seems like it's a decent workaround for the current regression, though this warrants further work to make things more robust.

The key change here is that this logic used to run before validating the node names, so we could properly handle the .001 from Blender. Now that it runs after the rename, we can't differentiate between .001 and _001 coming from Blender. So e.g. in 4.0.3, a node named Cube-convcolonly_001 would not be handled as having the -convcolonly hint, while with this change it will.

Also worth noting that the current code is super hacky and greedy, and will replace random sequences of ., _ and numbers at the end of any hint. So mycube123-col0128_9128.1789.198.22 would become mycube123. This really needs to be cleaned up in the future.

To test this I made this scene derived from the MRP of #78881:

Test project with blend and exported glb and dae:
ImportHints.zip

4.0.3

Here's how it imports in 4.0.3 using different formats (.blend file directly, .glb exported from Blender 3.6, .dae exported from Blender 3.6):

blend

Behaves as expected for the 4.0.3 logic, the .001 and .002 hints are properly handled. The one I added with _002 which is invalid syntax for an import hint is left untouched, so it's imported as a normal cube and keeps its name.

The Cube-convcolonly which conflicts with the existing Cube gets auto-renamed to solve the naming conflict.

glb

Similar behavior as blend, minus the light and camera but that's properly from not setting the right export settings, and unrelated to this issue.

dae

Also similar result. The scene tree order seems backwards, but that might be a bug in the Collada pipeline which we don't actively maintain.

4.1 RC 3

Reproducing the bug first to be able to validate the change.

blend

As expected, hints aren't properly handled, aside from the one on Cube-convcolonly.

Worth noting that the node name disambiguation logic changed too, so it's no longer named @Cube@... but it's actually using the node type instead with @StaticBody3D@.... This may or may not be another intentional change from #75760 (judging by the code I would say it's intentional and part of the optimization).

glb

Consistent with blend.

dae

Consistent with above results too. Note how the name conflict handling differs between the glTF and collada pipelines (duplicate Cube-convcolonly_002 becomes respectively Cube-convcolonly_0022 for glTF and Cube-convcolonly_003 for collada).

This PR

Confirming that it fixes the issue on the "expected" case of having .001 suffix, and creates a minor bug with now supporting _001 as a valid suffix to trim too. This is a slight breakage, but I think acceptable to fix the regression in the short term.

blend

As expected, fixes the bug, creates a minor behavior change for Cube-convcolonly_002 which is now handled as a -convcolonly hint. Technically a bug, could be considered a feature ;)

glb

Same result.

dae

Also working as expected.

---

TL;DR: It's not perfect, but I think it's good enough to YOLO merge now for 4.1 and fix the regression. I'd appreciate that the @godotengine/import teams looks further into it afterwards anyway, and makes sure that this system is more robust.

[akien-mga]

@capnm Could you rebase your branch? The current version is 800+ commits behind master, would be good to reduce the diff.

[capnm]

@capnm Could you rebase your branch? The current version is 800+ commits behind master, would be good to reduce the diff.

I forgot to rebase the github repo.

TL;DR: It's not perfect, but I think it's good enough to YOLO merge now for 4.1 and fix the regression. I'd appreciate that the @godotengine/import teams looks further into it afterwards anyway, and makes sure that this system is more robust.

It is a good idea from old times, but changing names is very limited and as you can see very bad to handle. Now the Blender glTF exporter is able to export custom object properties, it is IMO much better to use that.

"extras":{
	"prop_capnm":24.329999923706055,
	"capnm_str":"Das ist mein string mit \u00df\u00df\u00df!"
},

For example, this doesn't look fast...

[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'colonly' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'convcolonly' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'rigid' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'col' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'convcol' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'navmesh' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'occ' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'occonly' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'vehicle' what ''
[L] GD (./editor/import/resource_importer_scene.cpp:333) _teststr:
return FALSE: p_what '' p_str 'wheel' what ''

[Calinou]

Tested locally on https://github.com/godotengine/tps-demo (rebased on top of master cdd2313), it works.

[YuriSizov]

Thanks! You are officially closing the development cycle for 4.1 🎉

Edit: Ah, crap, I thought you have rebased already. Mea culpa.

[capnm]

Several seconds too late :)