-
-
Notifications
You must be signed in to change notification settings - Fork 120
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 feature - export particle objects as multimeshinstance to godot #354
Conversation
This is The Test Blender File, should enable the Particle Enum-Button at the Top while exporting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in charge of this project, but I gave it a quick review since I'm interested in this feature!
I recently had two use cases: scattering lillypads across water and stalactites in a cave. Both were easier to do using particles in blender.
You've got a number of linter warnings, check these out: https://travis-ci.org/github/godotengine/godot-blender-exporter/jobs/714011504#L311
Great work though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much! This is a great functionality to be added. I have played with it and it works great! I am trying to understand how the particle settings being exported, I am still missing some pieces, so I left some comments. Hope you can help me a little bit and get it merged soon. Thanks again!
def export_multimesh(self, escn_file, export_settings, particle_name): | ||
"""Saves a mesh into the escn file""" | ||
converter = MultiMeshConverter(self.particle_system) | ||
key = MultiMeshResourceKey( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, you can directly use the bpy ParticleSystem as the hash key for MultiMeshResource. Just like what we did with shader. Every bpy project is hashable and has a unique hash value.
The reason that we build a custom hash key for the MeshResource is that bpy mesh may need applying modifiers when being exported, which means a reference to the same bpy mesh might be exported to different MeshResources. Therefore, modifier attributes are included in MeshResource's hash key.
I think this is not the case with MultiMesh, correct me if I am wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above,Because if "Render-Render As" are switched to "Collection",there maybe several objects instanced in one particle system.so when we choose one of particle system,maybe different object should be instance,I admit just use hash from Bpy is good for now. I thought too much maybe give an option that can select which object in "collection" to export in future haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point, if in the future we have custom logic for each object referring an instanced particle system to export to a MutltiMeshInstance, then a bpy ParticleSystem hash might not be enough.
I am okay with current implementation and I think it works well. However, I still feel that the MultiMeshResourceKey
is a little bit overcomplicated. In my opinion, make the key refer to bpy ParticleSystem object instead of the particle system name is better. The later is guaranteed unique. The gd_rsc_type
seems unnecessary, only MultiMesh
is involved here.
Anyway I will merge it whether you change it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks and not change for now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, could you also fix up the pycodestyle warning as well. Really appreciate it. : )
pycodestyle io_scene_godot
io_scene_godot/structures.py:425:25: E231 missing whitespace after ','
io_scene_godot/converters/multimesh.py:17:23: E225 missing whitespace around operator
io_scene_godot/converters/multimesh.py:26:80: E501 line too long (82 > 79 characters)
io_scene_godot/converters/multimesh.py:31:80: E501 line too long (80 > 79 characters)
io_scene_godot/converters/multimesh.py:37:1: W293 blank line contains whitespace
io_scene_godot/converters/multimesh.py:38:13: E225 missing whitespace around operator
io_scene_godot/converters/multimesh.py:39:31: E225 missing whitespace around operator
io_scene_godot/converters/multimesh.py:175:55: E231 missing whitespace after ','
io_scene_godot/converters/multimesh.py:175:65: E231 missing whitespace after ','
Makefile:12: recipe for target 'pep8' failed
You can check the Travis CI status, see https://travis-ci.org/github/godotengine/godot-blender-exporter/jobs/741590402
def export_multimesh(self, escn_file, export_settings, particle_name): | ||
"""Saves a mesh into the escn file""" | ||
converter = MultiMeshConverter(self.particle_system) | ||
key = MultiMeshResourceKey( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point, if in the future we have custom logic for each object referring an instanced particle system to export to a MutltiMeshInstance, then a bpy ParticleSystem hash might not be enough.
I am okay with current implementation and I think it works well. However, I still feel that the MultiMeshResourceKey
is a little bit overcomplicated. In my opinion, make the key refer to bpy ParticleSystem object instead of the particle system name is better. The later is guaranteed unique. The gd_rsc_type
seems unnecessary, only MultiMesh
is involved here.
Anyway I will merge it whether you change it or not.
Thank you very much!It looks better now.
|
@Jason0214 |
My bad. Looks like disable pylint's |
Im ok, you can merge it ! |
It has some limitations for now, if a particle-system use a texture to scale particle size,the size in Godot multimesh will be incorrect