Make events no longer components

[SpecificProtagonist]

Objective

Events are components, but are not intended to be used as components. Fix this.

Solution

Make events no longer components. Use TypeId instead of ComponentId for identifying event types in observers.

Is there a reason (such as planned changes) that ComponentIds were used (in which case this PR can be closed / reworked)?

Benchmarks

Alternatives

Add a HasComponentId trait

event_propagation/single_event_type
                        time:   [1.2415 ms 1.2450 ms 1.2487 ms]
                        change: [-1.8997% -1.4333% -0.9529%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
event_propagation/single_event_type_no_listeners
                        time:   [755.14 µs 758.78 µs 762.71 µs]
                        change: [+1.6922% +2.0421% +2.4454%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe
Benchmarking event_propagation/four_event_types: Warming up for 500.00 ms
Warning: Unable to complete 100 samples in 4.0s. You may wish to increase target time to 5.2s, enable flat sampling, or reduce sample count to 60.
event_propagation/four_event_types
                        time:   [1.0522 ms 1.0548 ms 1.0575 ms]
                        change: [-1.4786% -0.5521% +0.1518%] (p = 0.20 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

observe/trigger_simple  time:   [39.725 µs 39.827 µs 39.938 µs]
                        change: [-58.378% -58.186% -58.002%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) high mild
  4 (4.00%) high severe
Benchmarking observe/trigger_targets_simple/10000_entity: Warming up for 500.00 ms
Warning: Unable to complete 100 samples in 4.0s. You may wish to increase target time to 6.3s, enable flat sampling, or reduce sample count to 50.
observe/trigger_targets_simple/10000_entity
                        time:   [1.2621 ms 1.2827 ms 1.3055 ms]
                        change: [+1.4548% +5.3022% +9.1547%] (p = 0.01 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe

Migration Guide

The Event trait no longer requires the Component trait. If you were relying on this behavior, change your trait bounds from Event to Event + Component. If you also want your Event type to implement Component, add a derive.

[alice-i-cecile]

Use TypeId instead of ComponentId for observers.

This will break observers for dynamic components. I don't like Event: Component, but I don't think this is a viable path forward.

[SpecificProtagonist]

This will break observers for dynamic components.

Sorry for the misunderstanding: I meant observers use of TypeId to refer to events, I didn't change how components are targeted!

(I assume the Contentious is because of this, I'll re-add it otherwise)

[alice-i-cecile]

Ah, I see. This may cause us pain if we try to introduce dynamic events in the future, but the clarity gains are worth it to me.

[james-j-obrien]

I don't like this because of the affect it has on dynamic event types. In my opinion all data types in the ECS should be referred to by component IDs (and in future entity IDs) to maximize flexibility and code re-use.

How would someone define an event type in a script and register an observer for it?

That being said if short-term conceptual clarity is the priority this change is fine, it's just a step back in my mind.

[iiYese]

I don't think any part of this was ever confusing to begin with. This just breaks a nice dogfood.

[alice-i-cecile]

I don't think any part of this was ever confusing to begin with.

As a new user, you go and look up the Event type. You see Event: Component. You intuit that "aha! Events must be special types of components!". This is false.

I don't mind using ComponentId here, and generally prefer it to TypeId in order to better support dynamic event types. The trait bound + automatic derive is the big problem here.

I would prefer to rename ComponentId to something more agnostic already (resources are not components, even if in the future we may implement them as such).

[iiYese]

As a new user, you go and look up the Event type. You see Event: Component. You intuit that "aha! Events must be special types of components!". This is false.

No that's correct? They only reason they're "special" is because of Event. Triggering events is just a special way to use a component. It becomes a payload in the ECS reactivity primitives instead of a field on an Entity. Bevy should be avoiding creating type level distinctions like this because it's why it has hooks & observers... but not for resources.

[alice-i-cecile]

This is part of the problem of lumping EventReader events in with Observer events: it makes sense for the latter to be related to components, but not the former.

[chescock]

Would it help to have a wrapper component like

#[derive(Component)]
#[repr(transparent)]
struct EventComponent<T: Event>(T);

?

Then you could easily get a ComponentId for an Event with the correct type layout, so you can keep using ComponentId everywhere. But events wouldn't be components, so you would get a compilation error if you tried to add one to an entity.

[alice-i-cecile]

That would be better. I'm still not For this PR though, I think we should:

a) remove Event: Component
b) keep the internals using ComponentId

To do that, we should merge register_component into register_resource and remove the trait bounds there. I'll make a PR first.