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 cone angle control to particle emission ring shape #91973

Merged

Conversation

Arnklit
Copy link
Contributor

@Arnklit Arnklit commented May 15, 2024

This adds a Cone Angle control to the particles ring emission shape for GPU and CPU particles.

image

As far as I know there is no way to ensure completely correct distribution when the shape get's more and more cone shaped. I skew the distribution using pow here, but if someone knows of a better way that would be great.

Note: this will require some additions to #86902

EDIT: Existing proposal this might close: godotengine/godot-proposals#6761
I went for adding an angle rather than a second radius control because that made more sense with the inner radius.

@Arnklit Arnklit requested review from a team as code owners May 15, 2024 08:49
@AThousandShips
Copy link
Member

Please open a proposal to track the support and details of this feature

@Arnklit
Copy link
Contributor Author

Arnklit commented May 15, 2024

Please open a proposal to track the support and details of this feature

Linked the existing proposal godotengine/godot-proposals#6761.
Maybe @QbieShay has time to look this over and see if it works well enough.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it mostly works as expected.

Testing project: test_pr_91973.zip

I suggest adding property hints to the ring properties, as right now, you can drag them towards negative values, which doesn't make sense.

image

The angle in particular should be clamped between 0.0 and 180.0, and the degrees inspector property hint should be used (see Node2D's rotation property for an example). All properties (except cone angle) should have or_greater, so you can input values as large as you might need.

Emission Height, Radius and Inner Radius should be limited to positive values as well, as strange things can happen if you use negative values:

ring_negative_radius.mp4
ring_negative_2.mp4

The angle property feels a bit strange, as the cone already has a starting angle of 0 degrees when the value is about 45 in the inspector:

ring_negative_3.mp4

@Arnklit Arnklit force-pushed the particle-cylinder-cone-emission-shape branch from 21b239f to e5f8428 Compare May 16, 2024 09:14
@Arnklit
Copy link
Contributor Author

Arnklit commented May 16, 2024

Thanks @Calinou . I added the property hints and it makes it nicer and I think resolves any strange behaviour.

image

It did make me wonder whether it might make sense to turn the inner radius and radius into min and max values with the ADD_MIN_MAX_PROPERTY

@Arnklit
Copy link
Contributor Author

Arnklit commented May 16, 2024

Just realised that the shader crashes on radius 0. Gonna have to max the value with 0.001. Give me a minute. I'll do another commit.

EDIT: hmm found some unexpected behaviour I don't like. I'll mark this as a draft until I can do a good solution later today.

@Arnklit Arnklit marked this pull request as draft May 16, 2024 09:49
@Arnklit Arnklit force-pushed the particle-cylinder-cone-emission-shape branch from e5f8428 to e8ac224 Compare May 20, 2024 09:53
@Arnklit
Copy link
Contributor Author

Arnklit commented May 20, 2024

OK, so I think I worked out all unexpected behaviour. I ended up doing it by limiting the angle between 0 and 90 instead of 0 and 180. This avoided all the strange behaviour when inner radius was larger than outer radius and you can still create the same shapes, you might just have to change the orientation of the shape to do so.

@Arnklit Arnklit marked this pull request as ready for review May 20, 2024 09:56
@Arnklit
Copy link
Contributor Author

Arnklit commented Jun 10, 2024

@Calinou if you have time to give this another look I'd appreaciate it :)

@QbieShay QbieShay self-assigned this Jul 9, 2024
@QbieShay QbieShay modified the milestones: 4.x, 4.4 Jul 9, 2024
@Mickeon Mickeon requested a review from Calinou July 9, 2024 15:27
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Documentation itself is fine if that was of any concern.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of 26d1577 with "Accept incoming" chosen during rebase), it works as expected for both CPU and GPU particles. The new behavior (with the effective angle clamped if the end of the cone is reached) makes sense.

Code looks good to me.

@Arnklit Arnklit force-pushed the particle-cylinder-cone-emission-shape branch 2 times, most recently from 2d2fc4e to 3139a5c Compare July 15, 2024 15:52
@Arnklit
Copy link
Contributor Author

Arnklit commented Jul 15, 2024

Rebased and added in the docs as suggested by @Calinou

@akien-mga
Copy link
Member

The docs need to be moved so that they are in alphabetical order: https://github.com/godotengine/godot/actions/runs/9942582709/job/27464431529?pr=91973

@Arnklit
Copy link
Contributor Author

Arnklit commented Aug 21, 2024

Oh sorry, did not realise that. I will do this when I'm back from vacation, or if I find a free evening soon.

@Arnklit Arnklit force-pushed the particle-cylinder-cone-emission-shape branch from cc14a63 to 40b9516 Compare September 2, 2024 12:36
@Arnklit
Copy link
Contributor Author

Arnklit commented Sep 2, 2024

@akien-mga rebased and squashed

@akien-mga akien-mga merged commit 160e3b3 into godotengine:master Sep 2, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

@Arnklit Arnklit deleted the particle-cylinder-cone-emission-shape branch March 11, 2025 13:57
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.

Add cone emitter to particles
6 participants