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

Add common component conditions #7711

Closed
wants to merge 14 commits into from
22 changes: 21 additions & 1 deletion crates/bevy_ecs/src/schedule/condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub mod common_conditions {
use crate::{
change_detection::DetectChanges,
event::{Event, EventReader},
prelude::{Component, Query, With},
prelude::{Added, Changed, Component, Query, With},
removal_detection::RemovedComponents,
schedule::{State, States},
system::{IntoSystem, Res, Resource, System},
Expand Down Expand Up @@ -894,6 +894,26 @@ pub mod common_conditions {
move |query: Query<(), With<T>>| !query.is_empty()
}

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the given component type was added to any entities.
///
/// Run conditions are evaluated on the main thread, blocking any other systems from running.
/// This run condition is relatively expensive, as it iterates over every entity with this component.
/// As a result, you likely only want to use this run condition when the number of entities with the component `T` is small.
pub fn any_component_added<T: Component>() -> impl FnMut(Query<(), Added<T>>) -> bool {
move |query: Query<(), Added<T>>| !query.is_empty()
Copy link
Member

@james7132 james7132 Feb 16, 2023

Choose a reason for hiding this comment

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

These potentially do a full world scan in the multithreaded executor, which not only runs single threaded but blocks any further system's run conditions from being evaluated and tasks launched. The only early return is if no archetypes match the query, everything else does a full scan, and will scale linearly with the number of those components in the world. This is relatively easy performance footgun, same with the other condition below.

Even with it documented, I still don't think it's a good idea to readily provide this to users without some form of archetype-level change detection optimization.

Copy link
Contributor Author

@Shatur Shatur Feb 16, 2023

Choose a reason for hiding this comment

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

Yep, this is what I mentioned in the description. I thought it's fine if documented. But your concern is exactly why it wasn't included in #7579.

}

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if the value of the given component type has been changed on any entities.
///
/// Run conditions are evaluated on the main thread, blocking any other systems from running.
/// This run condition is relatively expensive, as it iterates over every entity with this component.
/// As a result, you likely only want to use this run condition when the number of entities with the component `T` is small.
pub fn any_component_changed<T: Component>() -> impl FnMut(Query<(), Changed<T>>) -> bool {
move |query: Query<(), Changed<T>>| !query.is_empty()
}

/// Generates a [`Condition`](super::Condition)-satisfying closure that returns `true`
/// if there are any entity with a component of the given type removed.
pub fn any_component_removed<T: Component>() -> impl FnMut(RemovedComponents<T>) -> bool {
Expand Down