-
-
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
Event Stages #1041
Comments
This seems like a useful abstraction: handling events in a sequence of systems definitely seems valuable. EventSystemSet seems like a better way to handle this than EventStage. You might not always want these to run in their own unique stage, and if you do, you can always just stick them in a stage you initiated. From an ergonomic perspective, is it possible to write this as a modifier to systems / system sets in the same way as |
That is a very interesting point. Maybe instead this sort of functionality could be expressed like: SystemStage::parallel()
.with_system(event_handler_system.with_event_input::<MyEvent>())
.with_system(event_handler_system.with_event_input::<MyEvent>()); In this case an EventSystemSet/Stage wouldn't be necessary at all. |
I don't think a manual type on with_event_input is even required. It can be inferred from the input type. We could implement an Then you could just do this: app.add_system(handler.event_system());
fn handler(In(event): In<MyEvent>, something: Res<Something>) {
} |
I think this is a particular design that could be used to solve #2192. |
What problem does this solve or what need does it fill?
Systems that consume events currently must do so using
EventReader
to iterate through unprocessed events. It would be more intuitive if systems instead could be run only when an event occurs and passed that event as input. The changes proposed in #1021 make this possible withStages
andRunCriteria
.Describe the solution would you like?
We should add in a struct
EventStage
orEventSystemSet
to represent a collection of systems that should be run when an event is fired. The stage would check if any events have been fired since the last frame as its run criteria, and trigger each system to run one time for each event.Here is a working implementation of this feature forked off #1021.
This code is essentially equivalent to:
Describe the alternative(s) you've considered?
There are a few issues to address with the above implementation and in general with adding in this feature.
Event references across systems As I understand it, system inputs are required to be 'static, which causes obvious problems with attempting to pass a reference of an event from one system to another. I attempted to solve this by storing the events internally as
Arc<T>
and force systems to takeIn<Arc<T>>
, however this did not mesh well withEvents.drain()
so I imagine this is not quite the right solution. I opted to just add a restriction ofClone
to the event types for now to get the EventStage functional, but I'm sure there's a better solution. We could just useArc<Event>
instead ofEvent
in our applications but it's easily prone to error and doesn't seem like a decision that should be left up to the end user.SystemStageExecutor Currently every system added to EventStage is added as
next_event_system.chain(event_handler_system)
, however this creates a new reader for every system added. I imagine it would be slightly more efficient to read the event once from EventStage and pass it in to the executor, however SystemStageExecutor does not currently allow for any input to be passed in to its systems. It shouldn't be too difficult, but my attempts ended up thwarted bydowncast!
Additional context
It is also worth discussing the possibility of composing events together. It would be very useful to be able to create system sets that run when multiple events are fired. However, I think it is debatable whether this should be an aspect of
EventStages
or instead an aspect of the event system itself (by creating event streams composed of other event streams). Event streams could also be composed together in different ways, say by retaining the latest value for each stream and firing them all together.I have here an implementation and working example for AnyEventStage, which runs its systems if any of its 3 events fire.
The text was updated successfully, but these errors were encountered: