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.

Sign up for GitHub

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

Jump to bottom

Only allow MeshInstance3D-based nodes in particles emission shape node selector #84891

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Only allow MeshInstance3D-based nodes in particles emission shape node selector #84891

merged 1 commit into from
Dec 4, 2023

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Nov 14, 2023

This applies to both GPUParticles3D and CPUParticles3D, as CPUParticles3DEditor inherits from GPUParticles3DEditorBase.

Preview

Screenshot_20231114_161246

@Calinou Calinou requested a review from a team as a code owner November 14, 2023 15:14
@Calinou Calinou added bug topic:editor topic:3d topic:particles cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Nov 14, 2023
@Calinou Calinou added this to the 4.3 milestone Nov 14, 2023
AThousandShips
AThousandShips approved these changes Nov 14, 2023
View reviewed changes
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM, makes sense

@Calinou Calinou mentioned this pull request Nov 15, 2023
Merged
@smix8
Copy link
Contributor

smix8 commented Nov 19, 2023

I think the same MeshInstance3D filter should be applied like with the MultiMesh Plugin as this feature requires geometry data from a MeshInstance3D Mesh resource to work.

The selection throws an error if anything other than a MeshInstance3D with a valid Mesh is selected.

@Calinou
Only allow MeshInstance3D-based nodes in particles emission shape nod…
c6a16b1 
…e selector

This applies to both GPUParticles3D and CPUParticles3D, as
CPUParticles3DEditor inherits from GPUParticles3DEditorBase.
@Calinou
Copy link
Member Author

Calinou commented Nov 19, 2023

I think the same MeshInstance3D filter should be applied like with the MultiMesh Plugin as this feature requires geometry data from a MeshInstance3D Mesh resource to work.

The selection throws an error if anything other than a MeshInstance3D with a valid Mesh is selected.

Indeed, I read the code too quickly and thought any node inheriting from Node3D would work (in particular, anything extending GeometryInstance3D).

Fixed.

@Calinou Calinou force-pushed the editor-particles-generate-emission-fix-node-type branch from 7bc4f24 to c6a16b1 Compare November 19, 2023 10:25
@Calinou Calinou changed the title Only allow Node3D-inherited nodes in particles emission shape node selector Only allow MeshInstance3D-based nodes in particles emission shape node selector Nov 19, 2023
@akien-mga akien-mga merged commit 8b3ba7e into godotengine:master Dec 4, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.1.

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips approved these changes

@smix8 smix8 smix8 approved these changes

Assignees
No one assigned
Labels
bug topic:editor topic:particles topic:3d
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

"Create Emission Points From Node" doesn't filter out non Node3D nodes when it should.
5 participants
@Calinou @smix8 @akien-mga @YuriSizov @AThousandShips