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

Retain meta data set on importer nodes #87584

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

tamask
Copy link
Contributor

@tamask tamask commented Jan 25, 2024

During the import process, ImporterMeshInstance3D nodes are replaced with MeshInstance3D nodes. This change copies over any meta data that might have been set during the import process, for example, in a GLTFDocumentExtension.

This is a resolution for godotengine/godot-proposals#6586.

@tamask
Copy link
Contributor Author

tamask commented Jan 25, 2024

I've updated this pull request to be more thorough with copying over meta data on various imported nodes, not just ImporterMeshInstance3D.

@tamask tamask changed the title Retain meta data set on ImporterMeshInstance3D Retain meta data set on imported nodes Jan 25, 2024
@tamask tamask changed the title Retain meta data set on imported nodes Retain meta data set on importer nodes Jan 25, 2024
@tamask
Copy link
Contributor Author

tamask commented Jun 17, 2024

I've updated the pull request to also copy over metadata from ImporterMesh to ArrayMesh. In addition to ImporterMeshInstance3D, ImporterMesh is the other intermediary class used where metadata set in a GLTFDocumentExtension would be lost.

Here's an example use case in a GLTFDocumentExtension that copies over the extras from the gltf json so that it's available in the packed scene in godot:

func _apply_extras_to_meshes(state: GLTFState) -> void:
    for index: int in range(state.meshes.size()):
        var extras: Dictionary = state.json.meshes[index].get("extras", {})
        var mesh: GLTFMesh = state.meshes[index]
        _apply_extras_to_meta(state, mesh.mesh, extras)

This way, custom properties set in blender on the mesh data can be conveyed over to the godot side via gltf.

@lyuma
Copy link
Contributor

lyuma commented Jun 18, 2024

Looks good and easily digestible so I think it should be safe in 4.3 - Would it be possible to implement the same change in modules/gltf/extensions/gltf_document_extension_convert_importer_mesh.cpp ?
Basically, the GLTFDocumentExtension is the corresponding process for runtime GLTFDocument import, since the editor import pipeline code is not enabled at runtime.

https://github.com/godotengine/godot/blob/master/modules/gltf/extensions/gltf_document_extension_convert_importer_mesh.cpp
It's only in one place, so duplicating the contents of _copy_meta is acceptable in this case.

@tamask
Copy link
Contributor Author

tamask commented Jun 18, 2024

Looks good and easily digestible so I think it should be safe in 4.3 - Would it be possible to implement the same change in modules/gltf/extensions/gltf_document_extension_convert_importer_mesh.cpp ? Basically, the GLTFDocumentExtension is the corresponding process for runtime GLTFDocument import, since the editor import pipeline code is not enabled at runtime.

https://github.com/godotengine/godot/blob/master/modules/gltf/extensions/gltf_document_extension_convert_importer_mesh.cpp It's only in one place, so duplicating the contents of _copy_meta is acceptable in this case.

Absolutely, I've updated the pull request source branch to reflect this.

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fix a source of confusion when working with import plugins. Great work!

@lyuma lyuma modified the milestones: 4.x, 4.3 Jun 18, 2024
During the import process, many importer nodes are replaced with their
engine node counterparts. For example, ImporterMeshInstance3D is
replaced with a MeshInstance3D node. Any meta data set on these
importer nodes, i.e. through a GLTFDocumentExtension, are lost during
the conversion. This change copies over any meta data set on these
importer nodes to their engine counterparts.
@akien-mga akien-mga merged commit 1f7ee56 into godotengine:master Jun 19, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@tamask
Copy link
Contributor Author

tamask commented Jun 19, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

Wonderful, and thank you!

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

Successfully merging this pull request may close these issues.

4 participants