Skip to content

Have Single and Populated fail instead of skipping #19489

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
22 changes: 13 additions & 9 deletions crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ mod tests {
use crate::{
prelude::{Component, In, IntoSystem, Resource, Schedule},
schedule::ExecutorKind,
system::{Populated, Res, ResMut, Single},
system::{Populated, Res, ResMut, Single, When},
world::World,
};

Expand All @@ -327,12 +327,12 @@ mod tests {
#[derive(Resource, Default)]
struct Counter(u8);

fn set_single_state(mut _single: Single<&TestComponent>, mut state: ResMut<TestState>) {
fn set_single_state(mut _single: When<Single<&TestComponent>>, mut state: ResMut<TestState>) {
state.single_ran = true;
}

fn set_populated_state(
mut _populated: Populated<&TestComponent>,
mut _populated: When<Populated<&TestComponent>>,
mut state: ResMut<TestState>,
) {
state.populated_ran = true;
Expand Down Expand Up @@ -404,7 +404,7 @@ mod tests {
#[test]
fn piped_systems_first_system_skipped() {
// This system should be skipped when run due to no matching entity
fn pipe_out(_single: Single<&TestComponent>) -> u8 {
fn pipe_out(_single: When<Single<&TestComponent>>) -> u8 {
42
}

Expand All @@ -431,7 +431,7 @@ mod tests {
}

// This system should be skipped when run due to no matching entity
fn pipe_in(_input: In<u8>, _single: Single<&TestComponent>) {}
fn pipe_in(_input: In<u8>, _single: When<Single<&TestComponent>>) {}

let mut world = World::new();
world.init_resource::<Counter>();
Expand Down Expand Up @@ -482,7 +482,7 @@ mod tests {
#[test]
fn piped_system_skip_and_panic() {
// This system should be skipped when run due to no matching entity
fn pipe_out(_single: Single<&TestComponent>) -> u8 {
fn pipe_out(_single: When<Single<&TestComponent>>) -> u8 {
42
}

Expand All @@ -506,7 +506,7 @@ mod tests {
}

// This system should be skipped when run due to no matching entity
fn pipe_in(_input: In<u8>, _single: Single<&TestComponent>) {}
fn pipe_in(_input: In<u8>, _single: When<Single<&TestComponent>>) {}

let mut world = World::new();
let mut schedule = Schedule::default();
Expand Down Expand Up @@ -538,13 +538,17 @@ mod tests {
fn piped_system_skip_and_skip() {
// This system should be skipped when run due to no matching entity

fn pipe_out(_single: Single<&TestComponent>, mut counter: ResMut<Counter>) -> u8 {
fn pipe_out(_single: When<Single<&TestComponent>>, mut counter: ResMut<Counter>) -> u8 {
counter.0 += 1;
42
}

// This system should be skipped when run due to no matching entity
fn pipe_in(_input: In<u8>, _single: Single<&TestComponent>, mut counter: ResMut<Counter>) {
fn pipe_in(
_input: In<u8>,
_single: When<Single<&TestComponent>>,
mut counter: ResMut<Counter>,
) {
counter.0 += 1;
}

Expand Down
10 changes: 7 additions & 3 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2566,9 +2566,11 @@ impl<'w, 'q, Q: QueryData, F: QueryFilter> From<&'q mut Query<'w, '_, Q, F>>
/// [System parameter] that provides access to single entity's components, much like [`Query::single`]/[`Query::single_mut`].
///
/// This [`SystemParam`](crate::system::SystemParam) fails validation if zero or more than one matching entity exists.
/// This will cause the system to be skipped, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
/// This will cause the system to fail, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
///
/// Use [`Option<Single<D, F>>`] instead if zero or one matching entities can exist.
/// Use [`Option<Single<D, F>>`] instead to run the system but get `None` when there are zero or multiple matching entities.
///
/// Use [`When<Single<D, F>>`](crate::system::When) instead to skip the system when there are zero or multiple matching entities.
///
/// See [`Query`] for more details.
///
Expand Down Expand Up @@ -2617,7 +2619,9 @@ impl<'w, D: QueryData, F: QueryFilter> Single<'w, D, F> {
/// [System parameter] that works very much like [`Query`] except it always contains at least one matching entity.
///
/// This [`SystemParam`](crate::system::SystemParam) fails validation if no matching entities exist.
/// This will cause the system to be skipped, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
/// This will cause the system to fail, according to the rules laid out in [`SystemParamValidationError`](crate::system::SystemParamValidationError).
///
/// Use [`When<Populated<D, F>>`](crate::system::When) instead to skip the system when there are zero matching entities.
///
/// Much like [`Query::is_empty`] the worst case runtime will be `O(n)` where `n` is the number of *potential* matches.
/// This can be notably expensive for queries that rely on non-archetypal filters such as [`Added`](crate::query::Added),
Expand Down
14 changes: 5 additions & 9 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,10 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo
match query.single_inner() {
Ok(_) => Ok(()),
Err(QuerySingleError::NoEntities(_)) => Err(
SystemParamValidationError::skipped::<Self>("No matching entities"),
SystemParamValidationError::invalid::<Self>("No matching entities"),
),
Err(QuerySingleError::MultipleEntities(_)) => Err(
SystemParamValidationError::skipped::<Self>("Multiple matching entities"),
SystemParamValidationError::invalid::<Self>("Multiple matching entities"),
),
}
}
Expand Down Expand Up @@ -502,7 +502,7 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
state.query_unchecked_with_ticks(world, system_meta.last_run, world.change_tick())
};
if query.is_empty() {
Err(SystemParamValidationError::skipped::<Self>(
Err(SystemParamValidationError::invalid::<Self>(
"No matching entities",
))
} else {
Expand Down Expand Up @@ -1827,6 +1827,8 @@ unsafe impl<T: ReadOnlySystemParam> ReadOnlySystemParam for Result<T, SystemPara

/// A [`SystemParam`] that wraps another parameter and causes its system to skip instead of failing when the parameter is invalid.
///
/// This is especially useful with [`Single`] and [`Populated`].
///
/// # Example
///
/// ```
Expand Down Expand Up @@ -2794,12 +2796,6 @@ pub struct SystemParamValidationError {
}

impl SystemParamValidationError {
/// Constructs a `SystemParamValidationError` that skips the system.
/// The parameter name is initialized to the type name of `T`, so a `SystemParam` should usually pass `Self`.
pub fn skipped<T>(message: impl Into<Cow<'static, str>>) -> Self {
Self::new::<T>(true, message, Cow::Borrowed(""))
}

/// Constructs a `SystemParamValidationError` for an invalid parameter that should be treated as an error.
/// The parameter name is initialized to the type name of `T`, so a `SystemParam` should usually pass `Self`.
pub fn invalid<T>(message: impl Into<Cow<'static, str>>) -> Self {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_input_focus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,10 @@ pub type InputFocusSet = InputFocusSystems;
/// If no entity is focused, sets the focus to the primary window, if any.
pub fn set_initial_focus(
mut input_focus: ResMut<InputFocus>,
window: Single<Entity, With<PrimaryWindow>>,
window: When<Single<Entity, With<PrimaryWindow>>>,
) {
if input_focus.0.is_none() {
input_focus.0 = Some(*window);
input_focus.0 = Some(**window);
}
}

Expand Down
6 changes: 3 additions & 3 deletions examples/ecs/fallible_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ fn user_input(

// System that moves the enemies in a circle.
// Only runs if there are enemies, due to the `Populated` parameter.
fn move_targets(mut enemies: Populated<(&mut Transform, &mut Enemy)>, time: Res<Time>) {
for (mut transform, mut target) in &mut *enemies {
fn move_targets(mut enemies: When<Populated<(&mut Transform, &mut Enemy)>>, time: Res<Time>) {
for (mut transform, mut target) in &mut **enemies {
target.rotation += target.rotation_speed * time.delta_secs();
transform.rotation = Quat::from_rotation_z(target.rotation);
let offset = transform.right() * target.radius;
Expand Down Expand Up @@ -160,4 +160,4 @@ fn track_targets(

/// This system always fails param validation, because we never
/// create an entity with both [`Player`] and [`Enemy`] components.
fn do_nothing_fail_validation(_: Single<(), (With<Player>, With<Enemy>)>) {}
fn do_nothing_fail_validation(_: When<Single<(), (With<Player>, With<Enemy>)>>) {}
14 changes: 14 additions & 0 deletions release-content/migration-guides/single_and_populated_fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
title: "`Single` and `Populated` now fail instead of skipping"
pull_requests: [19489, 18765]
---

`Single<D, F>` and `Populated<D, F>` now cause systems to fail instead of skip.

The introduction of `When` makes it possible to skip systems for any invalid parameter, such as `When<Res<R>>`.
The change to the behavior of `Single` and `Populated` keeps them consistent with other parameters,
and makes it possible to use them as assertions instead of only as run conditions.

Replace `Single<D, F>` with `When<Single<D, F>>`
and `Populated<D, F>` with `When<Populated<D, F>>`
to restore the old behavior.