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

Deprecate particle emitter, and use scatter ratio in new particle mes… #986

Merged
merged 4 commits into from
Aug 20, 2021

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Aug 19, 2021

🎉 New feature

Summary

Use the particle_scatter_ratio in the particle emitter message available in fortress. This also starts the process of tick-tocking the ParticleEmitterSystem.

Test it

Run the test suite.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@nkoenig nkoenig requested a review from chapulina as a code owner August 19, 2021 22:23
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Aug 19, 2021
Nate Koenig added 3 commits August 19, 2021 15:31
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #986 (dd75161) into main (f274863) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #986      +/-   ##
==========================================
- Coverage   64.51%   64.50%   -0.02%     
==========================================
  Files         246      246              
  Lines       19746    19743       -3     
==========================================
- Hits        12740    12735       -5     
- Misses       7006     7008       +2     
Impacted Files Coverage Δ
src/Conversions.cc 83.70% <100.00%> (-0.06%) ⬇️
src/systems/particle_emitter2/ParticleEmitter2.cc 55.40% <0.00%> (-2.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f274863...dd75161. Read the comment docs.

@nkoenig
Copy link
Contributor Author

nkoenig commented Aug 20, 2021

The failing builds look unrelated to this PR.

Copy link
Contributor

@iche033 iche033 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.

@nkoenig nkoenig merged commit 81f3da9 into main Aug 20, 2021
@nkoenig nkoenig deleted the particle_emitter_fortress_update branch August 20, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants