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

Minimal Bubbling Observers #13991

Merged
merged 21 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/observer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
}

/// Returns the value of the flag that controls event propagation. See [`propagate`] for more information.
///
/// [`propigate`]: Trigger::propagate

Check warning on line 81 in crates/bevy_ecs/src/observer/mod.rs

View workflow job for this annotation

GitHub Actions / typos

"propigate" should be "propagate".
NthTensor marked this conversation as resolved.
Show resolved Hide resolved
pub fn get_propagate(&self) -> bool {
*self.propagate
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ use crate::{
/// events use the [`TraverseNone`] placeholder component, which cannot actually be created or added to
/// an entity and so never causes traversal.
///
/// Infinite loops are possible, and are not checked for. While looping can be desireable in some contexts
/// Infinite loops are possible, and are not checked for. While looping can be desirable in some contexts
/// (for example, an observer that triggers itself multiple times before stopping), following an infinite
/// traversal loop without an eventual exit will can your application to hang. Each implementer of `Traversal`
/// for documenting possible looping behavior, and consumers of those implementations are responsible for
/// avoiding infinite loops in their code.
///
/// [specify the direction]: crate::event::Event::Traverse
/// [specify the direction]: crate::event::Event::Traversal
/// [event propagation]: crate::observer::Trigger::propagate
/// [observers]: crate::observer::Observer
pub trait Traversal: Component {
Copy link
Contributor

@iiYese iiYese Jul 8, 2024

Choose a reason for hiding this comment

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

So it will be highly likely that traversals will be along "relations at home" components. With hooks people can now ensure that these graphs never become invalid. However to do so underlying Component types have to be private otherwise you can break invariants by doing this:

let foo = world.entity_mut(a).take::<Foo>().unwrap();
*world.entity_mut(b).get_mut::<Foo>().unwrap() = foo;

I already do this in aery and hierarchy will have to do this when it starts using hooks so rather than being Component this should be QueryData and next should take the item instead of self. You can just remove TraversalNone and make the default query ().

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh actually this change might require #13375 first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I tried to design it this way originally and couldn't get the ref safety right, then designed a "minimal" version to use a single component. This would be best addressed as early follow-up, but I totally want this.

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_hierarchy/src/components/parent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ impl Deref for Parent {
}
}

/// This provides generalized hiarchy traversal for use in [event propagation].
/// This provides generalized hierarchy traversal for use in [event propagation].
///
/// `Parent::traverse` will never form loops in properly-constructed hierarchies.
///
/// [event propagation]: crate::observer::Trigger::propagate
/// [event propagation]: bevy_ecs::observer::Trigger::propagate
impl Traversal for Parent {
fn traverse(&self) -> Option<Entity> {
Some(self.0)
Expand Down
Loading