Improve overriding the root type or root name in the scene importer #79774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fire
approved these changes
Jul 22, 2023
aaronfranke
force-pushed
the
scene-import-root-type-name
branch
from
5bc9338 to
b2f263f
Compare
kdiduk
approved these changes
Jul 27, 2023
aaronfranke
force-pushed
the
scene-import-root-type-name
branch
from
b2f263f to
aa187d8
Compare
|
Thanks! |
This was referenced Nov 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The code to override the root type and root name of an imported 3D scene in the current master is a bit weird/hacky.
For the type, the code in master checks if the type is set to anything other than Node3D, if so then it will replace the root node with that type, if not it will keep the node the importer provided. This means that there is no way to explicitly ask for the type to be Node3D. With this PR, the default value is an empty string, and it can be set to Node3D explicitly. Existing *.import files will continue to work, since they would cause Node3D to be replaced with Node3D.
For the name, the code in master checks if the name is different from "Scene Root", if so then it will replace the name with that name, or if it is equal to "Scene Root" then it will use the file name. Similarly to the above, this means that there is no way to explicitly ask for the name to be "Scene Root". Also, this means that there is no way for the importer to suggest a name for the scene (GLTF files may contain this information, but it's currently not used by Godot for the node name). With this PR, the default value is an empty string. Existing *.import files will continue to work. There still is no way to explicitly set the name to "Scene Root" because I added a special case to keep existing *.import files working, but we can remove this special case in a future compatibility breakage.
This PR also adds documentation for the
nodes/scene import properties.