-
Notifications
You must be signed in to change notification settings - Fork 11
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
Velocity Modifiers #44
Conversation
Add a new drag.rs example
Update example/velocity_modifiers.rs Add PrecalculatedParticleVariables
# Conflicts: # examples/basic.rs # src/components.rs # src/systems.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments but overall this looks awesome, thank you!
src/systems.rs
Outdated
velocity.0 += *v * delta_time; | ||
} | ||
|
||
ScalarAcceleration(v) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful to also have a plain Acceleration
variant that is a Vec3
. I think you mentioned it elsewhere, was the main blocker from doing that that we don't have a lerp for Vec3
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a Vec3 variant that was poorly named Constant.
I will rename this Vector, and ScalarAcceleration -> Scalar.
There was no blocker, but indeed I mentioned that it would be cool to have key values for Vec3 and f32. Not only lerp, which is cool, but don't offer as much control as key values (like gradient for ColorOverTime).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the modification, see the post below for its content !
src/systems.rs
Outdated
Noise(n) => { | ||
let offset = n.sample( | ||
Vec2::new(transform.translation.x, transform.translation.y), | ||
time.elapsed_seconds_wrapped(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make the same distinction here as we do with delta_time above, using elapsed_seconds_wrapped()
if we're respecting time scaling and raw_elapsed_seconds_wrapped()
if we're not, just for consistencies sake.
…ic struct CurvePoint. In VelocityModifier, rename ScalarAcceleration to Scalar, and Constant to Vector. Add VectorOverTime and replace Vector(Vec3) by Vector(VectorOverTime) in VelocityModifier. Fix example/shape_emitter.rs lifetime so its nicer and we can see the gradient.
As we mentioned previously, a VectorOverTime was needed. So I changed the Gradient struct to a "Curve" struct that is generic and is used as Color, Vec3 and f32. This change allow us to animate Vec3 and f32 with keys, when it was only possible for Color. Plus we can now use a VectorOverTime as a VelocityModifier, its more freedom for the user ! The name "Curve" was inspired by Unreal's "Cascade" particle system editor, that features the same value/key pair animation method, and "Gradient" wasn't relevant for f32 and Vec3 values. I kept the ValueOverTime/ColorOverTime/VectorOverTime enums implementations separated because they probably won't handle the same variants: like Sin(SinWave) for ValueOverTime for example. Such a variant wouldn't make any sense with Vectors or Colors. Also we might want to rotate vectors, or change HSV for colors, etc... I ended up thinking that these should not be in the same enum generic type despite all having some common variants. I updated the examples and "fixed" shape_emitter.rs where it was using a Curve but its lifetime + max_distance didn't allow the particles to finish their lifetime, and by extension their Curve. I lowered the lifetime, and now we can see them nicely fade. Let me know what you think ! |
Fantastic! |
Let me know what you think of this !