Fix fbx runtime import not generating meshes properly

[zodiepupper]

reviving #96059 to fix fbx runtime mesh importing. previously importing would only work properly for meshes while in the editor.

fixes #96029
fixes #96043

should also accommodate document extensions appropriately

lemme know if there is anything further i should do! ^-^

[fire]

Need to cite the proper people as Co Authored by. Will need to send you the doc for godot pr and code style

[fire]

Here's the guide to https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html and I would use pre-commit run -a to fix the style changes.

[zodiepupper]

Here's the guide to https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html and I would use pre-commit run -a to fix the style changes.

what is this pre-commit thing? i've never heard of it before, i normally do PR cleanup myself.

edit: discussed with ifire over discord. installed pre-commit from my system repos.

Need to cite the proper people as Co Authored by. Will need to send you the doc for godot pr and code style

how should i do that, the docs you links doesn't seem to describe it, unless i'm just not seeing it which is totally possible.

edit: discussed with ifire over discord for this too, adding them now. was genuinely not aware of this lol

[fire]

Optional: @lyuma reviewer

[lyuma]

Overall I approve of re-enabling extension support for FBXDocument and exposing register/unregister extensions.

The code should be changed to use the existing GLTFDocumentExtensionConvertImporterMesh class, given that the API is already based on GLTFDocumentExtension. Then we can delete FBXDocumentExtensionConvertImporterMesh and reduce duplicated code.

The only difference I can see between FBXDocumentExtensionConvertImporterMesh and the original GLTFDocumentExtensionConvertImporterMesh code is that it is not copying the visible property, perhaps because the visible property may have been added very recently.

[zodiepupper]

Overall I approve of re-enabling extension support for FBXDocument and exposing register/unregister extensions.

The code should be changed to use the existing GLTFDocumentExtensionConvertImporterMesh class, given that the API is already based on GLTFDocumentExtension. Then we can delete FBXDocumentExtensionConvertImporterMesh and reduce duplicated code.

The only difference I can see between FBXDocumentExtensionConvertImporterMesh and the original GLTFDocumentExtensionConvertImporterMesh code is that it is not copying the visible property, perhaps because the visible property may have been added very recently.

i made the change and tested it, it works exactly as well as before but actually maybe slightly faster.

you stated that the code is not copying the visible property. did you mean that the GLTFDocumentExtensionConvertImporterMesh doesn't use visible or that the new FBXDocumentExtensionConvertImporterMesh doesn't? when i looked through the code for GLTFDocumentExtensionConvertImporterMesh it seemed to be grabbing the visible value, from a quick glance

[fire]

Can you squash into one commit so it's finalized?

[zodiepupper]

should i squash the commits? i added more commits to keep the file history incase the change suggested by lyuma was more involved. so it's probably not necessary for it to be more than 1 commit

[zodiepupper]

yeah, i'll squash commits. ur message didn't load before i left my comment

[lyuma]

Looks good!

Visible was only missing in the fbx version. It should be fine now.

[Repiteo]

Thanks! Congratulations on your first merged contribution! 🎉

[fire]

@Calinou As per rocketchat discussion, I agree that it is cherry pickable into stable godot engine. It'll need testing for your use in the demo projects but the fbx importer has been static for a while now.

[zodiepupper]

hasn't this already been merged and closed, @fire ?

[fire]

@Calinou wants to cherry pick into 4.4 godot engine