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

Properly set emitting when particles restart #29974

Merged
merged 1 commit into from
Jun 24, 2019

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Jun 22, 2019

Fixes: #27146

Partially addresses: #26042

For GPU particles, an issue was that when restart() was called, the code waited until the next frame to restart the particles (and set emitting) to true. This resulted in a situation where emitting could be set to false and then the driver would set it back to true. When that issue was fixed, the fix caused restart() to no longer set emitting to true. Now, emitting is set to true at the same time restart() is called, so the previous bug should not arise, and restart() will properly restart the particle system.

The reason #26042 is only partially addressed, is that in that issue, the user wants the particle system to restart when emitting is set to true. However, there are valid use cases where one may want to set emitting on and off without it restarting. So a proper fix should not restart when emitting is set to true (as I suggested in the thread). Instead, in the AnimationPlayer, the user should set emitting to false when they want it to stop, and then call restart() when they want to restart the system. However, the AnimationPlayer won't call methods when running in editor. So the workflow is still not ideal. To partially address that, I added a restart button to the particle editor.

Code graciously donated by Gamblify.

CC @jesperkondrup, @akien-mga

Copy link
Contributor

@JFonS JFonS left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@akien-mga akien-mga added this to the 3.2 milestone Jun 24, 2019
@akien-mga akien-mga merged commit 25022a1 into godotengine:master Jun 24, 2019
@akien-mga
Copy link
Member

Thanks!

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.

Calling Particles2D.restart() no longer turns emitting on automatically if one_shot is true
3 participants