Skip to content

Conversation

@Ducklett
Copy link

@Ducklett Ducklett commented Apr 10, 2025

Revival of #79527

This PR implements the amount_ratio field for CPUParticles2D and CPUParticles3D. This already landed for GPUParticles2D and GPUParticles3D back in Godot v4.2 but the original PR for CPU particles never made it in.

Changes from the original PR

  • changing amount_ratio will no longer change the emitting flag
  • updated docs for amount and amount_ratio to match those for GPUParticles2D and GPUParticles3D

Fixes godotengine/godot-proposals#5939

@Ducklett Ducklett requested review from a team as code owners April 10, 2025 18:07
@Calinou Calinou added this to the 4.x milestone Apr 10, 2025
@arkology
Copy link
Contributor

arkology commented Apr 15, 2025

This PR suppresses #70145

@Mickeon
Copy link
Member

Mickeon commented Apr 15, 2025

This PR suppresses #70145

Supersedes* 😅

@KoBeWi
Copy link
Member

KoBeWi commented Apr 15, 2025

I think the OP has meant to mention it instead of #79527

@Ducklett
Copy link
Author

I think the OP has meant to mention it instead of #79527

Yeah, my bad 😅

@KoBeWi
Copy link
Member

KoBeWi commented Apr 15, 2025

That last change seems to simplify the code a lot.
But it doesn't work 🙃
When you decrease ratio, the particles freeze instead of disappearing.

@Ducklett
Copy link
Author

When you decrease ratio, the particles freeze instead of disappearing.

I see, it's keeping inactive particles in their final state and the effects I was testing with had a scale/fade out so they seemed to be gone. I'll look into it.

@Ducklett
Copy link
Author

When you decrease ratio, the particles freeze instead of disappearing.

Solved this in the latest commit.

if (amount_ratio_accumulator >= 1.0) {
active_by_ratio = true;
amount_ratio_accumulator -= 1.0;
} else if (!p.active) {
Copy link
Member

@KoBeWi KoBeWi Apr 15, 2025

Choose a reason for hiding this comment

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

The same condition is few lines below.

@KoBeWi KoBeWi requested a review from QbieShay April 15, 2025 21:35
@QbieShay
Copy link
Contributor

This needs testing to check how it behaves compared to GPUParticles. Code here:

code += " if (rand_from_seed(alt_seed) > AMOUNT_RATIO) {\n";

GPUParticles set particles inactive at birth depending on a random value

code += " if (rand_from_seed(alt_seed) > AMOUNT_RATIO) {\n";

This gives it an organic feel while still maintaining that zero has no emission and 1 is full emission. Your PR kills one particle every X, which is not the same behaviour.

@Calinou
Copy link
Member

Calinou commented Apr 17, 2025

This gives it an organic feel while still maintaining that zero has no emission and 1 is full emission. Your PR kills one particle every X, which is not the same behaviour.

See discussion in #104276. The "kill one particle every X" behavior seems more desirable overall, especially at low particle counts.

That said, we should aim for consistency between CPUParticles and GPUParticles, so this PR should be changed to use the random approach for now.

@akien-mga akien-mga marked this pull request as draft June 5, 2025 09:41
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.

Dynamically adjust the amount of particles emitted without clearing all the particles

8 participants