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.

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

Jump to bottom

Add any_component_removed condition #8326

Merged
merged 2 commits into from
Apr 18, 2023
Merged

Add any_component_removed condition #8326

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3996c79
Add `any_component_removed` condition
Shatur Apr 7, 2023
eeeabf6
Update crates/bevy_ecs/src/schedule/condition.rs
Shatur Apr 18, 2023
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
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/schedule/condition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ pub mod common_conditions {
change_detection::DetectChanges,
event::{Event, EventReader},
prelude::{Component, Query, With},
removal_detection::RemovedComponents,
schedule::{State, States},
system::{IntoSystem, Res, Resource, System},
};
Expand Down Expand Up @@ -893,6 +894,17 @@ pub mod common_conditions {
move |query: Query<(), With<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 {
// `RemovedComponents` based on events and therefore events need to be consumed,
// so that there are no false positives on subsequent calls of the run condition.
// Simply checking `is_empty` would not be enough.
// PERF: note that `count` is efficient (not actually looping/iterating),
// due to Bevy having a specialized implementation for events.
move |mut removals: RemovedComponents<T>| !removals.iter().count() != 0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
move |mut removals: RemovedComponents<T>| !removals.iter().count() != 0
move |mut removals: RemovedComponents<T>| removals.iter().next().is_none()

This otherwise potentially does a full traversal.

Copy link
Member

Choose a reason for hiding this comment

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

This contradicts the perf note above.

I would still prefer to remove the perf note and just use is_empty though.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I completely forgot we overrode the default implementations for those. Agreed on the use of is_empty instead.

Copy link
Contributor Author

@Shatur Shatur Apr 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

is_empty can't be used. The reason is mentioned in the comment: it won't advance the reader and the condition will be triggered twice. We use the same strategy for events:

bevy/crates/bevy_ecs/src/schedule/condition.rs

Line 856 in 3ead10a

move |mut reader: EventReader<T>| reader.iter().count() > 0

}

/// Generates a [`Condition`](super::Condition) that inverses the result of passed one.
///
/// # Example
Expand Down