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

Per entity Events #2116

Closed
wants to merge 39 commits into from
Closed

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented May 5, 2021

Closes #2070. Introduces the ability to ergonomically store events as components on entities, creating an event channel like abstraction.

This PR is intended to be merged after #2145, which adds EventConsumer and contains assorted Event-related code cleanup.

Will be much improved by #2073; the example should be updated by whichever PR is merged second.

Adds:

  1. Automatic event cleanup of componentized events when added via add_event.
  2. An example of how you might use this pattern that covers both the sugary and explicit forms.
  3. EventWriter query parameter.
  4. EventReader query parameter, which stores per-entity data about which events have been read.
  5. EventConsumer query parameter. See Add EventConsumer #2145.

To do:

  • Add query parameters for EventWriter.
  • Add query parameter for EventReader. Should probably wait for ParamState
  • Add query parameters for EventConsumer.

For a follow-up PR, we should add a more complex example as a game. The current example is contrived and over-engineered, but at least demonstrates the functionality in an interactive way. In particular, I think it would be nice to demonstrate complex flows of events between entities by combining this feature with Relations. An interactive neural net demo would be a cute way to do so IMO.

@alice-i-cecile alice-i-cecile marked this pull request as draft May 5, 2021 23:10
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible labels May 5, 2021
Example | Main | Description
--- | --- | ---
`hello_world` | [`hello_world.rs`](./hello_world.rs) | Runs a minimal example that outputs "hello world"
| Example | Main | Description |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this formatting stuff should probably be its own PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. It was just a nuisance to do as a separate PR due to format-on-save settings in my IDE. I can split it though if you think it's worth it :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those tables should not be formatted that way actually, otherwise as soon as someone adds a new example with a longer column, you have to change all the lines which break the diff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sad. Okay, I'll fix it.

@mockersf
Copy link
Member

mockersf commented May 6, 2021

Small change for nice feature 👍 (not small anymore)

I feel the example is overly complicated to show it though

@alice-i-cecile
Copy link
Member Author

I feel the example is overly complicated to show it though

Yeah :/ I wasn't sure how to balance "not minimal" vs. "why would you ever want this" and ended up somewhere in the middle.

It might make sense to strip this down to the bare minimum somehow and then demonstrate its value in a game example.

/// and advances the current frame's buffer so it is ready to be cleared.
pub fn update_system(
mut resource_events: ResMut<Self>,
mut component_events: Query<&mut Self>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe entity_events is a more suitable name here, since, as far as I understand, "component" is quite a general term, that can include the concept of "resource" and "event" (which is a resource). Maybe this is not a concern for the public interface, but internally Events are Resources, and Resources are Components.

@mockersf
Copy link
Member

I would be curious to know the perf and memory impact of using entity events

maybe the difference between

struct MyEvent(EntityId);
fn system(mut events: EventReader<MyEvent>, query: Query<Entity>) {
    for event in events.iter() {
        let entity = query.get(event.0).unwrap();
        do_something_with(entity);
    }
}

and

(note sure of the syntax needed here)

struct MyEvent;
fn system(query: Query<(Entity, &Events<MyEvent>)>, extra_parameter_needed_to_read_events: Something) {
    for (entity, events) in query.iter() {
        for event in events.iter(extra_parameter_needed_to_read_events) {
            do_something_with(entity);
        }
    }
}

@alice-i-cecile
Copy link
Member Author

I would be curious to know the perf and memory impact of using entity events

Agreed, this would be a nice little benchmark to include. To be fair, I think we'll also want to vary how often we're searching through the list, since that's one of the most compelling uses of this pattern.

#2073 will also help our perf a ton.

@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Sep 22, 2021
@alice-i-cecile
Copy link
Member Author

Closing this out; I'll make a new PR once we have an accepted RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Entity Events
5 participants