forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Generic SystemParam impls for Option and Result (#18766)

Browse Source 
# Objective

Provide a generic `impl SystemParam for Option<P>` that uses system
parameter validation. This immediately gives useful impls for params
like `EventReader` and `GizmosState` that are defined in terms of `Res`.
It also allows third-party system parameters to be usable with `Option`,
which was previously impossible due to orphan rules.

Note that this is a behavior change for `Option<Single>`. It currently
fails validation if there are multiple matching entities, but with this
change it will pass validation and produce `None`.

Also provide an impl for `Result<P, SystemParamValidationError>`. This
allows systems to inspect the error if necessary, either for bubbling it
up or for checking the `skipped` flag.

Fixes #12634
Fixes #14949
Related to #18516

## Solution

Add generic `SystemParam` impls for `Option` and `Result`, and remove
the impls for specific types.

Update documentation and `fallible_params` example with the new
semantics for `Option<Single>`.
This commit is contained in:
Chris Russell 2025-05-07 14:20:08 -04:00 committed by GitHub
parent 60ea43d01d
commit 9e2bd8ac18
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 148 additions and 202 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
crates/bevy_ecs/src/system/builder.rs
View File

@ -7,7 +7,8 @@ use crate::{
query::{QueryData, QueryFilter, QueryState},
resource::Resource,
system::{
DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam, When,
DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam,
SystemParamValidationError, When,
},
world::{
FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut,
@ -710,6 +711,36 @@ unsafe impl<'w, 's, T: FnOnce(&mut FilteredResourcesMutBuilder)>
}
}
/// A [`SystemParamBuilder`] for an [`Option`].
#[derive(Clone)]
pub struct OptionBuilder<T>(T);
// SAFETY: `OptionBuilder<B>` builds a state that is valid for `P`, and any state valid for `P` is valid for `Option<P>`
unsafe impl<P: SystemParam, B: SystemParamBuilder<P>> SystemParamBuilder<Option<P>>
for OptionBuilder<B>
{
fn build(self, world: &mut World, meta: &mut SystemMeta) -> <Option<P> as SystemParam>::State {
self.0.build(world, meta)
}
}
/// A [`SystemParamBuilder`] for a [`Result`] of [`SystemParamValidationError`].
#[derive(Clone)]
pub struct ResultBuilder<T>(T);
// SAFETY: `ResultBuilder<B>` builds a state that is valid for `P`, and any state valid for `P` is valid for `Result<P, SystemParamValidationError>`
unsafe impl<P: SystemParam, B: SystemParamBuilder<P>>
SystemParamBuilder<Result<P, SystemParamValidationError>> for ResultBuilder<B>
{
fn build(
self,
world: &mut World,
meta: &mut SystemMeta,
) -> <Result<P, SystemParamValidationError> as SystemParam>::State {
self.0.build(world, meta)
}
}
/// A [`SystemParamBuilder`] for a [`When`].
#[derive(Clone)]
pub struct WhenBuilder<T>(T);

282
crates/bevy_ecs/src/system/system_param.rs
View File

@ -477,87 +477,12 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo
}
}
// 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<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
for Option<Single<'a, D, F>>
{
type State = QueryState<D, F>;
type Item<'w, 's> = Option<Single<'w, D, F>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
Single::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 { Single::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> {
state.validate_world(world.id());
// SAFETY: State ensures that the components it accesses are not accessible elsewhere.
// The caller ensures the world matches the one used in init_state.
let query = unsafe {
state.query_unchecked_manual_with_ticks(world, system_meta.last_run, change_tick)
};
match query.single_inner() {
Ok(single) => Some(Single {
item: single,
_filter: PhantomData,
}),
Err(QuerySingleError::NoEntities(_)) => None,
Err(QuerySingleError::MultipleEntities(e)) => panic!("{}", e),
}
}
#[inline]
unsafe fn validate_param(
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: State ensures that the components it accesses are not mutably accessible elsewhere
// and the query is read only.
// The caller ensures the world matches the one used in init_state.
let query = unsafe {
state.query_unchecked_manual_with_ticks(
world,
system_meta.last_run,
world.change_tick(),
)
};
match query.single_inner() {
Ok(_) | Err(QuerySingleError::NoEntities(_)) => Ok(()),
Err(QuerySingleError::MultipleEntities(_)) => Err(
SystemParamValidationError::skipped::<Self>("Multiple matching entities"),
),
}
}
}
// SAFETY: QueryState is constrained to read-only fetches, so it only reads World.
unsafe impl<'a, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> ReadOnlySystemParam
for Single<'a, D, F>
{
}
// SAFETY: QueryState is constrained to read-only fetches, so it only reads World.
unsafe impl<'a, D: ReadOnlyQueryData + 'static, F: QueryFilter + 'static> ReadOnlySystemParam
for Option<Single<'a, D, F>>
{
}
// 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
@ -931,40 +856,6 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
}
}
// SAFETY: Only reads a single World resource
unsafe impl<'a, T: Resource> ReadOnlySystemParam for Option<Res<'a, T>> {}
// SAFETY: this impl defers to `Res`, which initializes and validates the correct world access.
unsafe impl<'a, T: Resource> SystemParam for Option<Res<'a, T>> {
type State = ComponentId;
type Item<'w, 's> = Option<Res<'w, T>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
Res::<T>::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'w, 's>(
&mut component_id: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
world
.get_resource_with_ticks(component_id)
.map(|(ptr, ticks, caller)| Res {
value: ptr.deref(),
ticks: Ticks {
added: ticks.added.deref(),
changed: ticks.changed.deref(),
last_run: system_meta.last_run,
this_run: change_tick,
},
changed_by: caller.map(|caller| caller.deref()),
})
}
}
// SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res
// conflicts with any prior access, a panic will occur.
unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
@ -1045,37 +936,6 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
}
}
// SAFETY: this impl defers to `ResMut`, which initializes and validates the correct world access.
unsafe impl<'a, T: Resource> SystemParam for Option<ResMut<'a, T>> {
type State = ComponentId;
type Item<'w, 's> = Option<ResMut<'w, T>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
ResMut::<T>::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'w, 's>(
&mut component_id: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
world
.get_resource_mut_by_id(component_id)
.map(|value| ResMut {
value: value.value.deref_mut::<T>(),
ticks: TicksMut {
added: value.ticks.added,
changed: value.ticks.changed,
last_run: system_meta.last_run,
this_run: change_tick,
},
changed_by: value.changed_by,
})
}
}
/// SAFETY: only reads world
unsafe impl<'w> ReadOnlySystemParam for &'w World {}
@ -1644,37 +1504,6 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> {
}
}
// SAFETY: Only reads a single World non-send resource
unsafe impl<T: 'static> ReadOnlySystemParam for Option<NonSend<'_, T>> {}
// SAFETY: this impl defers to `NonSend`, which initializes and validates the correct world access.
unsafe impl<T: 'static> SystemParam for Option<NonSend<'_, T>> {
type State = ComponentId;
type Item<'w, 's> = Option<NonSend<'w, T>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
NonSend::<T>::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'w, 's>(
&mut component_id: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
world
.get_non_send_with_ticks(component_id)
.map(|(ptr, ticks, caller)| NonSend {
value: ptr.deref(),
ticks: ticks.read(),
last_run: system_meta.last_run,
this_run: change_tick,
changed_by: caller.map(|caller| caller.deref()),
})
}
}
// SAFETY: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this
// NonSendMut conflicts with any prior access, a panic will occur.
unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
@ -1753,32 +1582,6 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
}
}
// SAFETY: this impl defers to `NonSendMut`, which initializes and validates the correct world access.
unsafe impl<'a, T: 'static> SystemParam for Option<NonSendMut<'a, T>> {
type State = ComponentId;
type Item<'w, 's> = Option<NonSendMut<'w, T>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
NonSendMut::<T>::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'w, 's>(
&mut component_id: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
world
.get_non_send_with_ticks(component_id)
.map(|(ptr, ticks, caller)| NonSendMut {
value: ptr.assert_unique().deref_mut(),
ticks: TicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick),
changed_by: caller.map(|caller| caller.deref_mut()),
})
}
}
// SAFETY: Only reads World archetypes
unsafe impl<'a> ReadOnlySystemParam for &'a Archetypes {}
@ -1916,6 +1719,91 @@ unsafe impl SystemParam for SystemChangeTick {
}
}
// SAFETY: Delegates to `T`, which ensures the safety requirements are met
unsafe impl<T: SystemParam> SystemParam for Option<T> {
type State = T::State;
type Item<'world, 'state> = Option<T::Item<'world, 'state>>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
T::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'world, 'state>(
state: &'state mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
T::validate_param(state, system_meta, world)
.ok()
.map(|()| T::get_param(state, system_meta, world, change_tick))
}
unsafe fn new_archetype(
state: &mut Self::State,
archetype: &Archetype,
system_meta: &mut SystemMeta,
) {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { T::new_archetype(state, archetype, system_meta) };
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
T::apply(state, system_meta, world);
}
fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) {
T::queue(state, system_meta, world);
}
}
// SAFETY: Delegates to `T`, which ensures the safety requirements are met
unsafe impl<T: ReadOnlySystemParam> ReadOnlySystemParam for Option<T> {}
// SAFETY: Delegates to `T`, which ensures the safety requirements are met
unsafe impl<T: SystemParam> SystemParam for Result<T, SystemParamValidationError> {
type State = T::State;
type Item<'world, 'state> = Result<T::Item<'world, 'state>, SystemParamValidationError>;
fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State {
T::init_state(world, system_meta)
}
#[inline]
unsafe fn get_param<'world, 'state>(
state: &'state mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
T::validate_param(state, system_meta, world)
.map(|()| T::get_param(state, system_meta, world, change_tick))
}
unsafe fn new_archetype(
state: &mut Self::State,
archetype: &Archetype,
system_meta: &mut SystemMeta,
) {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { T::new_archetype(state, archetype, system_meta) };
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
T::apply(state, system_meta, world);
}
fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) {
T::queue(state, system_meta, world);
}
}
// SAFETY: Delegates to `T`, which ensures the safety requirements are met
unsafe impl<T: ReadOnlySystemParam> ReadOnlySystemParam for Result<T, SystemParamValidationError> {}
/// A [`SystemParam`] that wraps another parameter and causes its system to skip instead of failing when the parameter is invalid.
///
/// # Example

7
examples/ecs/fallible_params.rs
View File

@ -131,13 +131,12 @@ fn move_targets(mut enemies: Populated<(&mut Transform, &mut Enemy)>, time: Res<
}
/// System that moves the player, causing them to track a single enemy.
/// The player will search for enemies if there are none.
/// If there is one, player will track it.
/// If there are too many enemies, the player will cease all action (the system will not run).
/// If there is exactly one, player will track it.
/// Otherwise, the player will search for enemies.
fn track_targets(
// `Single` ensures the system runs ONLY when exactly one matching entity exists.
mut player: Single<(&mut Transform, &Player)>,
// `Option<Single>` ensures that the system runs ONLY when zero or one matching entity exists.
// `Option<Single>` never prevents the system from running, but will be `None` if there is not exactly one matching entity.
enemy: Option<Single<&Transform, (With<Enemy>, Without<Player>)>>,
time: Res<Time>,
) {

28
release-content/migration-guides/generic-option-parameter.md Normal file
View File

@ -0,0 +1,28 @@
---
title: Generic `Option` Parameter
pull_requests: [18766]
---
`Option<Single<D, F>>` will now resolve to `None` if there are multiple entities matching the query.
Previously, it would only resolve to `None` if there were no entities, and would skip the system if there were multiple.
We have introduced a blanket `impl SystemParam for Option` that resolves to `None` if the parameter is invalid.
This allows third-party system parameters to work with `Option`, and makes the behavior more consistent.
If you want a system to run when there are no matching entities but skip when there are multiple,
you will need to use `Query<D, F>` and call `single()` yourself.
```rust
// 0.16
fn my_system(single: Option<Single<&Player>>) {
}
// 0.17
fn my_system(query: Query<&Player>) {
let result = query.single();
if matches!(r, Err(QuerySingleError(MultipleEntities(_)))) {
return;
}
let single: Option<&Player> = r.ok();
}
```