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 EnemyDestroyedEvent and EnemyDestroyedSystem #68

Merged
merged 2 commits into from
Sep 23, 2020

Conversation

cdsupina
Copy link
Collaborator

For #67.

I ended up creating a system to handle enemy deaths because spawning entities like the consumable drop and the blast explosion don't necessarily require their own system (I didn't want to create a new explosion system).

I see this event being read in other systems, for example, keeping track of how many enemies the player has destroyed.

I also moved all of the events currently in the game into their own module, rather than keeping them in the already large space_shooter.rs file.

Copy link
Collaborator

@tigleym tigleym left a comment

Choose a reason for hiding this comment

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

Thanks! I left a suggestion about using .get over join to get components for a specific entity. I also had one nit about the organization of systems/mod.rs.

Overall, I think having EnemyDestroyedSystem works well for handling enemy deaths and helps isolate a lot of the logic that was present in systems/enemy.rs.

src/systems/enemy_destroyed.rs Outdated Show resolved Hide resolved
src/systems/mod.rs Show resolved Hide resolved
@cdsupina
Copy link
Collaborator Author

@tigleym I exchanged join for get in enemy_destroyed.rs. See my comment about systems/mod.rs. I'm going to create an issue addressing a config file for rustfmt. If you think it is good feel free to merge!

@cdsupina cdsupina mentioned this pull request Sep 23, 2020
Copy link
Collaborator

@tigleym tigleym left a comment

Choose a reason for hiding this comment

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

LGTM!

src/systems/mod.rs Show resolved Hide resolved
@tigleym tigleym merged commit bfe1bd8 into cdsupina-dev Sep 23, 2020
@tigleym tigleym deleted the enemy_destroyed_event branch September 23, 2020 02:54
tigleym added a commit that referenced this pull request Nov 17, 2020
add EnemyDestroyedEvent and EnemyDestroyedSystem
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.

None yet

2 participants