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

Improve observers documentation #14084

Open
1 of 8 tasks
cBournhonesque opened this issue Jul 1, 2024 · 1 comment
Open
1 of 8 tasks

Improve observers documentation #14084

cBournhonesque opened this issue Jul 1, 2024 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation

Comments

@cBournhonesque
Copy link
Contributor

cBournhonesque commented Jul 1, 2024

Observers is a super cool new feature, but I think there might be some gaps currently in the documentation around them, especially w.r.t when to use observers vs events.

A quick list of things to mention:

  • Explain that an observer system needs to have Trigger in the first position of the SystemParam list, but otherwise it's the same as another system (actually it's mentioned here already: https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/observer/runner.rs#L152-L152)

  • Make the distinction between the OnAdd and OnInsert triggers more clear. Also explain that OnInsert only happens if a new version of the component is inserted on the entity; it is not triggered if the component is simply mutated

  • Observer vs events:

    • maybe mention that observers/EventReaders are basically two ways to react to events. One is more push-based and the other pull-based
    • explain that one of the key strengths of observers systems is that they only run when the event is triggered. This can be more performant than having a system run every frame and read events from a EventReader. In the case where the events are emitted infrequently, the system is scheduled every frame but would often do nothing, whereas the observer systems only runs when the event is actually triggered. Conversely, if an event is triggered multiple times every frame, it is more efficient to have a system that reads all buffered events from EventReader than to trigger the observer system individually for each event. (we need benchmarks to compare the two)
  • Maybe better explain the concept of the observer target?
    The trigger is an Event which can target optionally for one or more entities, one or more components. I feel like the docs on TriggerTargets and TriggerEvent are pretty light, whereas the concepts are not that straightforward.
    I think this is the part that needs the most work. For example I see in the code that Vec<ComponentId> can be a target, but then it seems like each observer actually only fires for at most 1 componentId. (see https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/observer/mod.rs#L135-L135)

  • Clarify how to access component data from the trigger. Is it this function? https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/observer/mod.rs#L51

  • There's a lot of questions on discord about observers vs hooks, and observer order

  • Users could more or less not worry at all about applying the command buffers since the apply_command_buffers system was added automatically in the schedule if there is an ordering dependency between systems that use Commands. It is not really true anymore, with observers you have to think about:

    • applying command_buffers after calls to commands.trigger(event) to make sure that the observers are triggered
    • applying command_buffers after the observer triggers so that any commands buffered by the observer are applied.
  • Clarify current observer limitations:

    • can't update an observers component or entity targets after the observer has been spawned
@cBournhonesque cBournhonesque added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events S-Needs-Triage This issue needs to be labelled labels Jul 1, 2024
@janhohenheim janhohenheim removed the S-Needs-Triage This issue needs to be labelled label Jul 1, 2024
@MaxwellZunders
Copy link

Asked about this in the #ecs-dev discord channel out of curiosity: it sounds like it's possible for users to introduce multiple observers that could feasibly cause cycles (observer system 'A' sends an event that triggers observer system 'B'...and wraps back to 'A' again). It sounds like the current behavior is that bevy will loop in this case (I'm not suggesting any changes to that). I do think it might be worth adding docs saying that this can happen and that it's up to users to design observer systems so that it doesn't.

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-Docs An addition or correction to our documentation
Projects
None yet
Development

No branches or pull requests

3 participants