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

Refactor particle_spawner system #81

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

mnmaita
Copy link
Contributor

@mnmaita mnmaita commented Jul 15, 2024

  • Removed some redundant code from a system that was setting both the color of the sprite and the index of the texture atlas, which is no longer necessary after SpriteSheetBundle was deprecated. TextureAtlas and Sprite components are now independent so we can update Sprite on one system and the TextureAtlas index on the other.
  • Refactored particle_spawner code to reduce duplication. It should be now clearer that the difference between ParticleSpace lies in how the entity hierarchy is built. I feel like this could be simplified a bit more, but I didn't bother fighting against multiple mut borrow issues of Commands right now.
  • Fixed a small typo in a comment.

@mnmaita
Copy link
Contributor Author

mnmaita commented Jul 15, 2024

@abnormalbrain @theanti9 @Bytekeeper you might want to take a look at this before releasing the next version of the crate. I'm mainly concerned about the changes I introduced in my first commit, which will remove some redundant work in systems now that Sprite and TextureAtlas are decoupled in bevy. Feel free to cherry pick just that for releasing, and then review or close this refactor if it feels irrelevant. Thanks!

@mnmaita
Copy link
Contributor Author

mnmaita commented Jul 15, 2024

Also, IMHO a good next step after this would be entirely removing ParticleTexture and having an independent way of adding the Texture Atlas to match Bevy current API.

@theanti9
Copy link
Collaborator

@mnmaita

Always in favor of PR's that remove more code than they add. This looks great.

@theanti9 theanti9 merged commit 8aee8fd into abnormalbrain:main Jul 16, 2024
3 checks passed
@mnmaita mnmaita deleted the mnmaita/texture-atlas-refactor-1 branch July 16, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants