-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Commands::send_event #14933
Commands::send_event #14933
Conversation
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.
Since this is in bevy_ecs
itself, we shouldn't use an extension trait.
I'm in favor of adding this though, provided we have a bit of docs laying out the drawbacks: I've seen this helper show up repeatedly in various projects I've reviewed.
The type-erased nature is genuinely useful for more complex patterns, and this is consistent with observers.
FYI, #13818 adds another little helper for sending and receiving events in the same system. |
This pattern is indeed extremely common. Here's a few cursed variations from my game: impl Command for MyEvent {
fn apply(self, world: &mut World) {
world.send_event(self);
}
} commands.add(|world: &mut World| world.send_event(event)); One possible footgun here could be that |
sorry, i don't see it?
i can rename - i generally call it |
I think |
If this is merged, would it be the default way to send events or would there be 2 ways of doing the same thing? If so, which one would be preferred? |
The |
thoughts on automatically registering the event type as well? So that people who use this pattern won't have to call |
We definitely shouldn't do that: |
we could add it only if |
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.
Definitely wanted to use this pattern in the past for code running in unit tests! Thanks for the PR :)
No thoughts on the automatic registration of the passed event.
Got some minor suggestions before approval.
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Merging once CI is green: suggestions probably broke formatting. |
Head branch was pushed to by a user without write access
Objective
sending events tends to be low-frequency so ergonomics can be prioritized over efficiency.
add
Commands::send_event
to send any type of event without needing a writer in hand.i don't know how we feel about these kind of ergonomic things, i add this to all my projects and find it useful. adding
mut this_particular_event_writer: EventWriter<ThisParticularEvent>
every time i want to send something is unnecessarily cumbersome.it also simplifies the "send and receive in the same system" pattern significantly.
basic example before:
basic example after:
send/receive in the same system before:
after: