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

Optimal event handling #99

Open
Ralith opened this issue Oct 20, 2020 · 11 comments
Open

Optimal event handling #99

Ralith opened this issue Oct 20, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@Ralith
Copy link
Owner

Ralith commented Oct 20, 2020

This event handling pattern has came up a few times lately:

for (entity, event) in events.drain(..) {
    if let Ok(components) = world.query_one::<A>(entity) {
        handle_a(event, components);
    }
    if let Ok(components) = world.query_one::<B>(entity) {
        handle_b(event, components);
    }
    // ...
}

This works well, but is slightly suboptimal. It should be possible to precompute the subset of these queries (A, B, ...) which are satisfied by entities in each archetype, then iterate through that list instead of checking every possible query each time.

One approach would be to allow queries to be registered to obtain an ID, then expose a list of satisfied query IDs for a given entity, stored in the archetype. The above pattern could then be optimized by walking the satisfied ID list to select the appropriate handlers.

Some challenges:

  • Queries must be registered exactly once. OnceCell and statics? HashSet of query TypeIds?
  • How do we actually branch to the proper handler based on a dynamic ID? A HashMap<QueryID, &dyn FnMut> would have to be reallocated every time to avoid a 'static bound on the closures, which kinda sucks. When TypeId::of becomes a const fn (Tracking Issue for const fn type_id rust-lang/rust#77125), we could use the TypeId of each query as its ID and just match.
  • The interface all this implies is a bit ugly, but could be encapsulated gracefully in a declarative macro.
  • This is a lot of faffing about for a probably very marginal efficiency gain.
@Ralith Ralith added the enhancement New feature or request label Oct 20, 2020
@Ralith Ralith changed the title Prepared single-entity queries Optimal event handling Oct 20, 2020
@AngelOfSol
Copy link
Contributor

AngelOfSol commented Oct 20, 2020

I wonder if an API like the following makes sense:

impl World {
  fn handle_events<Event, F: FnMut(Q::Item), Q: Query, I: Iterator<Item=(Entity, Event)>>(&mut self, iterator: I, f: F);
  fn handle_events2<Event, F1: FnMut(Q1::Item), Q1: Query, F2: FnMut(Q2::Item), Q2: Query,I: Iterator<Item=(Entity, Event)>>(&mut self, iterator: I, f: F1, f2: F2);
 ...
}

You could probably do something with a Trait parameterized on tuples of queries?

trait EventHandler<Q, F, E> {
  fn handle_events(&mut self, iter: impl Iterator<Item=(Entity, E)>, f: F);
}

impl<Q: Query, U: Query, E> EventHandler<(Q, U), (FnMut(Event, Q::Item), FnMut(Event, U::Item)), E> for World {
  fn handle_events(&mut self, iter: impl Iterator<Item=(Entity, Event)>, f: F) { 
    // do stuff here...
  }
}

Really rough spitball here, but I think my point is there? (Not sure what Fn trait to use).

I'm just trying to come at it from the user side and figure out what would be nice to use.

@AngelOfSol
Copy link
Contributor

AngelOfSol commented Oct 21, 2020

Another idea I had that doesn't really necessarily optimize the problem on the backend, but might look ok for the user was something like this:

impl World {
  fn try_on_entity<T, Q: Query, F: FnOnce(Q) -> T>(&mut self, entity: Entity: f: F) -> Option<T> { ... }
}

@Ralith
Copy link
Owner Author

Ralith commented Oct 21, 2020

I wonder if an API like the following makes sense:

I think that could work, but the ergonomics of passing a bunch of queries in type parameters and then matching them up with a giant tuple of functions seem unpleasant enough to not be worth it.

Another idea

How does that differ from the existing World::query_one?

@AngelOfSol
Copy link
Contributor

AngelOfSol commented Oct 21, 2020

I think that could work, but the ergonomics of passing a bunch of queries in type parameters and then matching them up with a giant tuple of functions seem unpleasant enough to not be worth it.

I was hoping typeck could figure out the types for everything to make it so the client didn't have to figure it out themselves, or at least only have to annotate the function type parameters.

How does that differ from the existing World::query_one?

Same as above in regards to typeck. It's main use would be to reduce boilerplate by providing an Option::map like interface on the entity. At least that's what I was trying to get at.

@Ralith
Copy link
Owner Author

Ralith commented Oct 21, 2020

Type inference can't work backwards from an associated type.

@AngelOfSol
Copy link
Contributor

That makes sense. I was hoping there was some way to decrease boilerplate a little bit. Either way I'm interested to see where this goes from the user-side, so I'll be watching.

@Ralith
Copy link
Owner Author

Ralith commented Apr 30, 2021

#154 is a closely related problem, where instead of cacheing a list of queries satisfied by an archetype, we want to cache a list of archetypes that satisfy a query.

@adamreichold
Copy link
Collaborator

I wonder if this use case isn't actually solved well by using the View types? I think the initial example could be rewritten as

let mut query_a = world.query::<A>();
let mut view_a = query_a.view();

let mut query_b = world.query::<B>();
let mut view_b = query_b.view();

for (entity, event) in events.drain(..) {
    if let Some(components) = view_a.get_mut(entity) {
        handle_a(event, components);
    }
    if let Some(components) = view_b.get_mut(entity) {
        handle_b(event, components);
    }
    // ...
}

which should be efficient?

@Ralith
Copy link
Owner Author

Ralith commented Dec 29, 2021

The idea here was to find a way to replace all the if let Some branching with indirection that could determine, based on the entity's archetype, exactly which of a collection of handlers should run without any dynamic filtering. I think the approach you've sketched would be a good enough substitute, since it all the nontrivial work out of the loop, but it's not generally applicable because it adds the constraint that A and B must be able to overlap without violating aliasing rules.

@Ralith
Copy link
Owner Author

Ralith commented May 8, 2024

evenio is an interesting case study:

  • Event handlers and their components of interest are registered in advance (Targeted Events), which allows archetype/event handler relationships to be persisted, allowing event dispatch to involve the minimum amount of indirection. In hecs terms, each event handlers owns a PreparedQuery which is invoked for the benefit of event handlers. Targeted events could be said to pass through a PreparedView. Presumably event dispatch is transposed compared to what's been discussed in this issue so far, traversing all entities that match a given event handler in one pass.
  • Spawn, insert, remove, and despawn are events, and their handlers run before the world is modified, providing an excellent place to hook any external index synchronization, so long as you somehow pipe that index in on the side.

Straw API sketch:

impl EventQueue {
    fn new() -> Self;
    fn push<E: Event>(&mut self, entity: EntityId, event: E);
    fn query<E: Event, Q: Query>(&mut self, world: &mut World) -> impl Iterator<Item=(EntityId, &mut E, Q::Item<'_>)>;
    fn clear<E: Event>(&mut self);
}

(or maybe EventQueue<E: Event>?)

Spawn/etc could be passed through such an EventQueue explicitly by an application, rather than executed directly on the world.

@tornewuff
Copy link

I've been experimenting with both hecs and evenio recently for a project prototype; hecs probably suits my use case much better, but I'm making fairly heavy use of events targeted at specific entities, so I have an interest in this thread :)

The straw API proposed above looks like it wouldn't preserve event ordering, which wouldn't work for me. I think you may have misinterpreted evenio's API/docs here:

Presumably event dispatch is transposed compared to what's been discussed in this issue so far, traversing all entities that match a given event handler in one pass.

evenio's targeted events are always targeted at a single entity ID, so this never comes up - handlers for targeted events are just given the single result for their query for that entity (and not called at all if the query does not match that entity). So, it's functionally the same thing as the initial example in this thread of calling query_one for each handler.

Global event handlers can have ECS query parameters, but the queries don't affect whether they are called or not: the handler still runs even if one or more of the queries matches no entities, and it's up to the body of the handler to choose how it uses (or doesn't use) the queries.

So, yeah, in evenio's model there is no "send a foo event to all the entities that match this query"; there is only "send a foo event to this entity id" and "send a global foo event". Events are always handled in order regardless of type (though the order is "depth-first": all the new events sent during the handling of an event are inserted at the front of the queue).

It was fairly straightforward to implement the same event queue/ordering behavior as evenio using standalone code without any dependencies on the hecs internals, but my current implementation just has each event handler early-out if the target entity doesn't have the required components:

fn take_damage(event: &DamageEvent, world: &mut World, queue: &EventQueue) {
    let Ok(health) = world.query_one_mut::<&mut Health>(event.entity) else { return; };
    // do whatever
}

i.e. there's no distinction between targeted/global events and events are dispatched to handlers solely by the event type. This fits reasonably well with the "systems are just functions" approach in hecs and is fairly ergonomic (the shape of the query is mentioned only once), but if events might have many handlers it would be more efficient to be able to pick the "right" ones based on archetypes as discussed here.

I'm just not sure how that could be done without either making it more verbose, or doing the kind of function-signature-interpretation "magic" that evenio and other Rust ECSes often rely on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants