Skip to content

Commit

Permalink
Populated (query) system param (#15488)
Browse files Browse the repository at this point in the history
# Objective

Add a `Populated` system parameter that acts like `Query`, but prevents
system from running if there are no matching entities.

Fixes: #15302

## Solution

Implement the system param which newtypes the `Query`.
The only change is new validation, which fails if query is empty.

The new system param is used in `fallible_params` example.

## Testing

Ran `fallible_params` example.

---------

Co-authored-by: Alice Cecile <[email protected]>
  • Loading branch information
MiniaczQ and alice-i-cecile authored Sep 30, 2024
1 parent 397f20e commit fc93e13
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 8 deletions.
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ pub mod prelude {
},
system::{
Commands, Deferred, EntityCommand, EntityCommands, In, InMut, InRef, IntoSystem, Local,
NonSend, NonSendMut, ParallelCommands, ParamSet, Query, ReadOnlySystem, Res, ResMut,
Resource, Single, System, SystemIn, SystemInput, SystemParamBuilder,
NonSend, NonSendMut, ParallelCommands, ParamSet, Populated, Query, ReadOnlySystem, Res,
ResMut, Resource, Single, System, SystemIn, SystemInput, SystemParamBuilder,
SystemParamFunction,
},
world::{
Expand Down
42 changes: 42 additions & 0 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ use core::{
///
/// [`World`]: crate::world::World
///
/// # Similar parameters
///
/// [`Query`] has few sibling [`SystemParam`](crate::system::system_param::SystemParam)s, which perform additional validation:
/// - [`Single`] - Exactly one matching query item.
/// - [`Option<Single>`] - Zero or one matching query item.
/// - [`Populated`] - At least one matching query item.
///
/// Those parameters will prevent systems from running if their requirements aren't met.
///
/// # System parameter declaration
///
/// A query should always be declared as a system parameter.
Expand Down Expand Up @@ -1667,3 +1676,36 @@ impl<'w, D: QueryData, F: QueryFilter> Single<'w, D, F> {
self.item
}
}

/// [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 systems that use this parameter to be skipped.
///
/// 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) or [`Changed`](crate::query::Changed)
/// which must individually check each query result for a match.
///
/// See [`Query`] for more details.
pub struct Populated<'w, 's, D: QueryData, F: QueryFilter = ()>(pub(crate) Query<'w, 's, D, F>);

impl<'w, 's, D: QueryData, F: QueryFilter> Deref for Populated<'w, 's, D, F> {
type Target = Query<'w, 's, D, F>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<D: QueryData, F: QueryFilter> DerefMut for Populated<'_, '_, D, F> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl<'w, 's, D: QueryData, F: QueryFilter> Populated<'w, 's, D, F> {
/// Returns the inner item with ownership.
pub fn into_inner(self) -> Query<'w, 's, D, F> {
self.0
}
}
57 changes: 57 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ use core::{
ops::{Deref, DerefMut},
};

use super::Populated;

/// A parameter that can be used in a [`System`](super::System).
///
/// # Derive
Expand Down Expand Up @@ -497,6 +499,61 @@ unsafe impl<'a, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> ReadOn
{
}

// SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If
// this Query conflicts with any prior access, a panic will occur.
unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
for Populated<'_, '_, D, F>
{
type State = QueryState<D, F>;
type Item<'w, 's> = Populated<'w, 's, D, F>;

fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
Query::init_state(world, system_meta)
}

unsafe fn new_archetype(
state: &mut Self::State,
archetype: &Archetype,
system_meta: &mut SystemMeta,
) {
// SAFETY: Delegate to existing `SystemParam` implementations.
unsafe { Query::new_archetype(state, archetype, system_meta) };
}

#[inline]
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
// SAFETY: Delegate to existing `SystemParam` implementations.
let query = unsafe { Query::get_param(state, system_meta, world, change_tick) };
Populated(query)
}

#[inline]
unsafe fn validate_param(
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> bool {
state.validate_world(world.id());
// SAFETY:
// - We have read-only access to the components accessed by query.
// - The world has been validated.
!unsafe {
state.is_empty_unsafe_world_cell(world, system_meta.last_run, world.change_tick())
}
}
}

// SAFETY: QueryState is constrained to read-only fetches, so it only reads World.
unsafe impl<'w, 's, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> ReadOnlySystemParam
for Populated<'w, 's, D, F>
{
}

/// A collection of potentially conflicting [`SystemParam`]s allowed by disjoint access.
///
/// Allows systems to safely access and interact with up to 8 mutually exclusive [`SystemParam`]s, such as
Expand Down
13 changes: 7 additions & 6 deletions examples/ecs/fallible_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
//! from running if their acquiry conditions aren't met.
//!
//! Fallible parameters include:
//! - [`Res<R>`], [`ResMut<R>`] - If resource doesn't exist.
//! - [`Single<D, F>`] - If there is no or more than one entities matching.
//! - [`Option<Single<D, F>>`] - If there are more than one entities matching.
//! - [`Res<R>`], [`ResMut<R>`] - Resource has to exist.
//! - [`Single<D, F>`] - There must be exactly one matching entity.
//! - [`Option<Single<D, F>>`] - There must be zero or one matching entity.
//! - [`Populated<D, F>`] - There must be at least one matching entity.

use bevy::prelude::*;
use rand::Rng;
Expand Down Expand Up @@ -105,9 +106,9 @@ fn user_input(
}

// System that moves the enemies in a circle.
// TODO: Use [`NonEmptyQuery`] when it exists.
fn move_targets(mut enemies: Query<(&mut Transform, &mut Enemy)>, time: Res<Time>) {
for (mut transform, mut target) in &mut enemies {
// Only runs if there are enemies.
fn move_targets(mut enemies: Populated<(&mut Transform, &mut Enemy)>, time: Res<Time>) {
for (mut transform, mut target) in &mut *enemies {
target.rotation += target.rotation_speed * time.delta_seconds();
transform.rotation = Quat::from_rotation_z(target.rotation);
let offset = transform.right() * target.radius;
Expand Down

0 comments on commit fc93e13

Please sign in to comment.