Skip to content

Conversation

@emre0altan
Copy link
Contributor

@emre0altan emre0altan commented Feb 12, 2024

Suffix options for resource importing in 3d scenes can actually be used with multiple symbols with different case sensitivity. Pointed in this issue: #8950

@emre0altan emre0altan force-pushed the Clarify-suffix-usage-when-importing-resources-with-ResourceImporterScene branch from 12ecce2 to badf7f2 Compare February 12, 2024 23:54
@skyace65
Copy link
Contributor

Do you have a test 3D model you could share for this? I tested out the behavior you described and everything was imported correctly with all 3 suffixes regardless of case. Here's a picture of a test glb file I imported to Godot 4.2.1:
ImportTest

@skyace65 skyace65 added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Feb 14, 2024
@emre0altan
Copy link
Contributor Author

emre0altan commented Feb 14, 2024

Thanks for testing. It's my mistake not testing "$" symbol. Case conversion for it happens differently. Other two symbols are lower cased and then checked, but "$" is used with findn() which I realised is a case-insensitive version of find().

https://github.com/godotengine/godot/blob/b4e2a24c1f62088b3f7ce0197afc90832fc25009/editor/import/3d/resource_importer_scene.cpp#L321-L331

So, I assume nodes are being imported as a child of rigidbody node and I will test them again. Should I just write down all three symbols are case-insensitive after that?

@skyace65
Copy link
Contributor

Yes, if you're seeing that everything is case insensitive for you as well update the PR. When you do please keep your PR to 1 commit, if you don't know how to do that we have a guide here.

@emre0altan emre0altan force-pushed the Clarify-suffix-usage-when-importing-resources-with-ResourceImporterScene branch from badf7f2 to d48821c Compare February 15, 2024 10:17
@emre0altan
Copy link
Contributor Author

Thanks, updated now.

@emre0altan emre0altan force-pushed the Clarify-suffix-usage-when-importing-resources-with-ResourceImporterScene branch from d48821c to a4a1763 Compare February 16, 2024 16:50
@emre0altan emre0altan force-pushed the Clarify-suffix-usage-when-importing-resources-with-ResourceImporterScene branch from a4a1763 to e41af09 Compare February 16, 2024 17:12
@skyace65 skyace65 merged commit 3898c7e into godotengine:master Feb 16, 2024
@skyace65
Copy link
Contributor

Thanks! Congrats on your first merged PR! Also just so you know github has a feature where if you write "closes #issue number" in your PR description it will automatically close that issue when the PR is merged.

@emre0altan
Copy link
Contributor Author

Good to know, thank you!

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

Labels

area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants