Fixed texture animation speed when using random lifetime

[RPicster]

I had to close the first Pull Request for this fix as it was from my master branch...
Sorry for the extra work!

The old Pull Request was this one:
#54994

[akien-mga]

I'm not sure about all the default value changes. The default settings for a 3D particle IMO should not be to have a fixed linear, orbital, angular velocity + accelerations.

It should be to have no start parameters of its own, and just be subjected to gravity. That's how it works in 2D too.

This is the current behavior (GPU on left, CPU on right):
https://user-images.githubusercontent.com/4701338/143868904-9e74b2ed-0831-4831-83b4-7b28ff76e901.mp4

If I understand this PR correctly (didn't compile to test), this would become:
https://user-images.githubusercontent.com/4701338/143869157-f9607ae4-09fb-43b0-8769-30e41328d784.mp4

[Calinou]

I'm not sure about all the default value changes. The default settings for a 3D particle IMO should not be to have a fixed linear, orbital, angular velocity + accelerations.

There's a proposal about this, but no consensus was reached yet: godotengine/godot-proposals#2533

This needs to be addressed in a different PR if we ever reach a consensus.

[RPicster]

I'm not sure about all the default value changes. The default settings for a 3D particle IMO should not be to have a fixed linear, orbital, angular velocity + accelerations.

It should be to have no start parameters of its own, and just be subjected to gravity. That's how it works in 2D too.

This is the current behavior (GPU on left, CPU on right): https://user-images.githubusercontent.com/4701338/143868904-9e74b2ed-0831-4831-83b4-7b28ff76e901.mp4

If I understand this PR correctly (didn't compile to test), this would become: https://user-images.githubusercontent.com/4701338/143869157-f9607ae4-09fb-43b0-8769-30e41328d784.mp4

I actually didn't change any of the default parameters. only the tex_ variables default values (so the default values of the curve's variables when no curve is used).

The default behaviour of a CPUParticles3D system won't be any different.
I did this change to make CPUParticles3D be more consistent to CPUParticles2D where those values are already used.

[akien-mga]

Ah I see, those are factors applied to the actual properties. Makes sense then.

I did this change to make CPUParticles3D be more consistent to CPUParticles2D where those values are already used.

That kind of information is always worth stating in the PR description to help reviewers understand the motivation for a change. The codebase is big and we can't do a deep dive for all PR reviews, so any context you possess after working on this code is good to share :) (to a reasonable extent of course, we also don't have time to read a 5 pages book ;) ).

[akien-mga]

Thanks!

[RPicster]

Ah I see, those are factors applied to the actual properties. Makes sense then.

I did this change to make CPUParticles3D be more consistent to CPUParticles2D where those values are already used.

That kind of information is always worth stating in the PR description to help reviewers understand the motivation for a change. The codebase is big and we can't do a deep dive for all PR reviews, so any context you possess after working on this code is good to share :) (to a reasonable extent of course, we also don't have time to read a 5 pages book ;) ).

Thanks for the feedback, now that you say it... It sound very obvious 😅

[akien-mga]

Would you be able to make a dedicated PR for 3.x (if it's relevant there)? I see that the CPUParticles2D change for default values hasn't been done there yet. Might be worth checking what changed that in master to make sure that the backport is consistent with what we have in master (as long as it's not breaking compatibility).

[RPicster]

Sure, I can do that 👍