Skip to content

Make HierarchyPropagatePlugin scheduling configurable #20433

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 5, 2025
Merged

Make HierarchyPropagatePlugin scheduling configurable #20433

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5ecbb6
Make `HierarchyPropagatePlugin` scheduling configurable
tim-blackbird Aug 5, 2025
c2f37ea
Remove default impl
cart Aug 5, 2025
38b58fa
Add update
cart Aug 5, 2025
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
30 changes: 25 additions & 5 deletions crates/bevy_app/src/propagate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ use bevy_ecs::{
component::Component,
entity::Entity,
hierarchy::ChildOf,
intern::Interned,
lifecycle::RemovedComponents,
query::{Changed, Or, QueryFilter, With, Without},
relationship::{Relationship, RelationshipTarget},
schedule::{IntoScheduleConfigs, SystemSet},
schedule::{IntoScheduleConfigs, ScheduleLabel, SystemSet},
system::{Commands, Local, Query},
};
#[cfg(feature = "bevy_reflect")]
Expand All @@ -33,15 +34,31 @@ use bevy_reflect::Reflect;
/// Individual entities can be skipped or terminate the propagation with the [`PropagateOver`]
/// and [`PropagateStop`] components.
///
/// Propagation occurs during [`Update`] in the [`PropagateSet<C>`] system set.
/// By default the propagation occurs during [`Update`] schedule in the [`PropagateSet<C>`] system set.
Copy link
Member

Choose a reason for hiding this comment

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

IIRC we wanted to change this to PostUpdate. Can we do that in this PR too?

Copy link
Member

Choose a reason for hiding this comment

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

#20356 is the issue for that.

Copy link
Member

Choose a reason for hiding this comment

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

I think the default value is kind of hard to pick. With Update, it means you don't need to think about PostUpdate ordering (easier on the propagated component dev). With PostUpdate, you always run after all game logic (easier on the propagated component consumers), but the author needs to be very careful about PostUpdate ordering.

I'm cool with leaving this in Update for now, but I think Propagate<TextFont> should be done in PostUpdate.

Copy link
Contributor

@ickshonpe ickshonpe Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Does it need a default? It might be less confusing to always require users to specify a ScheduleLabel like we we do with add_system etc.

Copy link
Member

Choose a reason for hiding this comment

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

I like this. Defining propagation properly is advanced-level territory. Thrusting the choice on the developer makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to suggest removing the default as well, but I was too tired by the time I posted this PR.
Glad it worked out! :)

/// The schedule can be configured via [`HierarchyPropagatePlugin::new`].
/// You should be sure to schedule your logic relative to this set: making changes
/// that modify component values before this logic, and reading the propagated
/// values after it.
pub struct HierarchyPropagatePlugin<
C: Component + Clone + PartialEq,
F: QueryFilter = (),
R: Relationship = ChildOf,
>(PhantomData<fn() -> (C, F, R)>);
> {
schedule: Interned<dyn ScheduleLabel>,
_marker: PhantomData<fn() -> (C, F, R)>,
}

impl<C: Component + Clone + PartialEq, F: QueryFilter, R: Relationship>
HierarchyPropagatePlugin<C, F, R>
{
/// Construct the plugin. The propagation systems will be placed in the specified schedule.
pub fn new(schedule: impl ScheduleLabel) -> Self {
Self {
schedule: schedule.intern(),
_marker: PhantomData,
}
}
}

/// Causes the inner component to be added to this entity and all direct and transient relationship
/// targets. A target with a [`Propagate<C>`] component of its own will override propagation from
Expand Down Expand Up @@ -84,7 +101,10 @@ impl<C: Component + Clone + PartialEq, F: QueryFilter, R: Relationship> Default
for HierarchyPropagatePlugin<C, F, R>
{
fn default() -> Self {
Self(Default::default())
Self {
schedule: Update.intern(),
_marker: PhantomData,
}
}
}

Expand Down Expand Up @@ -129,7 +149,7 @@ impl<C: Component + Clone + PartialEq, F: QueryFilter + 'static, R: Relationship
{
fn build(&self, app: &mut App) {
app.add_systems(
Update,
self.schedule,
(
update_source::<C, F>,
update_stopped::<C, F>,
Expand Down
Loading