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

Add option to export Geometry Nodes Instances in Blender importer #87735

Merged

Conversation

ywmaa
Copy link
Contributor

@ywmaa ywmaa commented Jan 30, 2024

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

I approve of Export Geometry Nodes Instances in blend importer as as a Godot Engine proposal. I haven't done the technical review yet.

@ywmaa
Copy link
Contributor Author

ywmaa commented May 26, 2024

Hey, I just gave this PR a test, it works great if this is solved:
https://projects.blender.org/blender/blender/issues/122237

which is already solved, I think it will land with a new blender 4.1 minor release and/or with 4.2 release.

I think we can safely merge this PR if the issue on blender side is included with their distrubted blender version.

edit:
the blender issue above is fixed here:
https://projects.blender.org/blender/blender-addons/commit/6b8b29648c3bb05f20d99f6eca775bffd05d490a

got the link from the issue discussion.

@ywmaa
Copy link
Contributor Author

ywmaa commented May 26, 2024

I kindly suggest merging this PR as it adds only 9 lines of code, that hopefully won't break anything, and the feature will be there once blender includes the fix on their end.

also the geometry nodes feature will only work if blender's major is 4 and minor is 1 or more, meaning it shouldn't do anything to users who still use blender 3.x as the importer.

@ywmaa
Copy link
Contributor Author

ywmaa commented May 28, 2024

Ok, I will apply this patch, but also I found out that with blender 4.1.1 the importing fails if export_lights is true, only one export_lights or export_geometry_nodes_instances must be true and the other is false, see the discussion here about this issue:
https://projects.blender.org/blender/blender/issues/122237

Now should I:
1- bump the blender version for this feature to 4.2, though I am still reporting the issue to blender team, so not 100% sure it will land in 4.2 (but then we won't be able to use export_geometry_nodes_instances in 4.1)
2- leave it at 4.1, but add a workaround for 4.1 only: disabling export_lights if export_geometry_nodes_instances is enabled.

what is the best solution ?

@fire
Copy link
Member

fire commented May 28, 2024

I think 2 is better for wider compatibility. Blender 4.2 doesn't work fully yet due to the vertex color error.

@ywmaa
Copy link
Contributor Author

ywmaa commented May 30, 2024

Great news, Blender 4.2 (Development) now has the issue solved:
https://projects.blender.org/blender/blender/issues/122379#issuecomment-1200660

so this PR will work with 4.2 without the 4.1 workaround.

This PR as it is now should work with 4.1 (with the workaround), and 4.2 and up (without any workaround).

@warcaninos
Copy link

Could this be merged for 4.3 or is it planned for 4.4 ?

@AThousandShips
Copy link
Member

For 4.4 at the earliest, we're not accepting new features for 4.3 at this time

@ywmaa ywmaa force-pushed the blend_file_geometrynodes_instances branch from 7efcebd to efe25d1 Compare July 19, 2024 11:56
@ywmaa
Copy link
Contributor Author

ywmaa commented Jul 19, 2024

I just rebased and tested it with the new released Blender 4.2 LTS, and it works!

this PR should be ready when the door opens for Godot 4.4.

@ywmaa
Copy link
Contributor Author

ywmaa commented Aug 18, 2024

is it possible for this to get merged since 4.4 is now WIP ?

@akien-mga akien-mga force-pushed the blend_file_geometrynodes_instances branch from efe25d1 to c460f1d Compare August 19, 2024 10:23
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Aug 19, 2024
@akien-mga akien-mga merged commit 3a4e0f1 into godotengine:master Aug 19, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@ywmaa ywmaa deleted the blend_file_geometrynodes_instances branch August 23, 2024 00:42
@akien-mga akien-mga changed the title Add Option for Export Geometry Nodes Instances in blend importer Add option to export Geometry Nodes Instances in Blender importer Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Add support for importing Geometry Nodes instances from .blend files
7 participants