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 velocity inheritance for particle effects #3691

Merged
merged 5 commits into from
Oct 10, 2021

Conversation

Baezon
Copy link
Member

@Baezon Baezon commented Oct 3, 2021

Uses the previously unused GetVelocity() function to return, not only the particle or object's velocity, but also a plain vector set by the initial caller of the particle source set up, such as the velocity of a weapon when it detonates. Particle Effects can then inherit a specified amount of this velocity when spawned (default 0).

The velocity of the source is on the SourceOrigin which a little awkward perhaps, but I'd be open to a better place for it.

@Baezon Baezon added graphics A feature or issue related to graphics (2d and 3d) feature A totally new sort of functionality labels Oct 3, 2021
Copy link
Member

@wookieejedi wookieejedi left a comment

Choose a reason for hiding this comment

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

Ah this an extremely useful PR and will substantially help with improving the look of particle-based effects. SourceOrigin make sense and everything else looks good, but I'll also tag @asarium.

@@ -179,6 +185,10 @@ class GenericShapeEffect : public ParticleEffect {
saw_deprecated_effect_location = true;
}

if (optional_string("+Velocity Inherit:")) {
Copy link
Member

Choose a reason for hiding this comment

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

To me, this name seems a bit weird. It seems to indicate that it is a simple boolean value but instead is a factor on the inherited velocity.
Maybe something like +Velocity Factor: would be more appropriate? Also, you could make use of a random range here to allow for some variability if desired by the modder.

Copy link
Member Author

@Baezon Baezon Oct 4, 2021

Choose a reason for hiding this comment

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

Parent velocity factor: maybe? I do want to make sure it is known that it is inherited in some way, rather than operating on the regular $Velocity

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you changed it to +Parent Velocity Factor:, and that does make it a bit more immediately clear IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A totally new sort of functionality graphics A feature or issue related to graphics (2d and 3d)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants