[Merged by Bors] - Tidy up the code of events #4713
Closed
+122
−136
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DJMcNab
added
A-ECS
Nilirad
reviewed
May 10, 2022
Nilirad
reviewed
May 10, 2022
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
TheRawMeatball
approved these changes
May 10, 2022
|
I would also suggest moving the |
IceSentry
approved these changes
May 10, 2022
tim-blackbird
approved these changes
May 10, 2022
IceSentry
added
the
S-Ready-For-Final-Review
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
May 10, 2022
# Objective - The code in `events.rs` was a bit messy. There was lots of duplication between `EventReader` and `ManualEventReader`, and the state management code is not needed. ## Solution - Clean it up. ## Future work Should we remove the type parameter from `ManualEventReader`? It doesn't have any meaning outside of its source `Events`. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to `&mut World` usage)
bors
bot
changed the title
robtfm
pushed a commit
to robtfm/bevy
that referenced
this pull request
May 17, 2022
# Objective - The code in `events.rs` was a bit messy. There was lots of duplication between `EventReader` and `ManualEventReader`, and the state management code is not needed. ## Solution - Clean it up. ## Future work Should we remove the type parameter from `ManualEventReader`? It doesn't have any meaning outside of its source `Events`. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to `&mut World` usage)
exjam
pushed a commit
to exjam/bevy
that referenced
this pull request
May 22, 2022
# Objective - The code in `events.rs` was a bit messy. There was lots of duplication between `EventReader` and `ManualEventReader`, and the state management code is not needed. ## Solution - Clean it up. ## Future work Should we remove the type parameter from `ManualEventReader`? It doesn't have any meaning outside of its source `Events`. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to `&mut World` usage)
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective - The code in `events.rs` was a bit messy. There was lots of duplication between `EventReader` and `ManualEventReader`, and the state management code is not needed. ## Solution - Clean it up. ## Future work Should we remove the type parameter from `ManualEventReader`? It doesn't have any meaning outside of its source `Events`. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to `&mut World` usage)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
events.rswas a bit messy. There was lots of duplication betweenEventReaderandManualEventReader, and the state management code is not needed.Solution
Future work
Should we remove the type parameter from
ManualEventReader?It doesn't have any meaning outside of its source
Events. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to&mut Worldusage)