-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - Remove unnecessary system labels #4340
Conversation
.add_system(print_components_read_only) | ||
.add_system(print_components_iter_mut.after(print_components_read_only)) | ||
.add_system(print_components_iter.after(print_components_iter_mut)) | ||
.add_system(print_components_tuple.after(print_components_iter)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so much better...
MyStage::AfterRound, | ||
game_over_system.after(MyLabels::ScoreCheck), | ||
game_over_system.after(score_check_system), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree on this change: public system labels deserve discussion elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fully approve of these changes; this is much nicer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely
bors r+ |
# Objective - Since #4224, using labels which only refer to one system doesn't make sense. ## Solution - Remove some of those. ## Future work - We should remove the ability to use strings as system labels entirely. I haven't in this PR because there are tests which use this, and that's a lot of code to change. - The only use cases for labels are either intra-crate, which use #4224, or inter-crate, which should either use #4224 or explicit types. Neither of those should use strings.
# Objective - Since bevyengine#4224, using labels which only refer to one system doesn't make sense. ## Solution - Remove some of those. ## Future work - We should remove the ability to use strings as system labels entirely. I haven't in this PR because there are tests which use this, and that's a lot of code to change. - The only use cases for labels are either intra-crate, which use bevyengine#4224, or inter-crate, which should either use bevyengine#4224 or explicit types. Neither of those should use strings.
# Objective - Since bevyengine#4224, using labels which only refer to one system doesn't make sense. ## Solution - Remove some of those. ## Future work - We should remove the ability to use strings as system labels entirely. I haven't in this PR because there are tests which use this, and that's a lot of code to change. - The only use cases for labels are either intra-crate, which use bevyengine#4224, or inter-crate, which should either use bevyengine#4224 or explicit types. Neither of those should use strings.
Objective
Solution
Future work