PublicArchive/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy.git synced 2026-05-06 06:06:42 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge SystemParam::validate_param into SystemParam::get_param (#23225)

Browse Source 
# Objective

As raised in
[#23174](https://github.com/bevyengine/bevy/pull/23174#discussion_r2868030355_),
we currently duplicate working when looking up our system parameters:
once during validation, and then again when actually fetching the data.

This is (maybe) slow, and would worsen the performance regression
incurred by resources-as-components (#19731).

This strategy also imposes some non-trivial complexity and
maintainability costs. Because "validate" is a distinct step from "use",
it's possible to skip validation! As far as I could tell, this is the
case in a number of places before this PR: particularly in the
unconventional "please just run my system" path. While in most cases
this will simply result in a crash in a different place, it causes these
paths to not handle

Fixes #23179. Fixes #15505.

## Solution

Fundamentally, what we're doing is rolling the
`SystemParam::validate_param` behavior into `SystemParam::get_param`, by
making the latter return a `Result`.

However, there is a tremendous amount of splash damage required to get
that to actually compile and expose the correct semantics. The most
important of these are:

- `SystemState::get` and friends now returns a `Result`
- this leads to a fair bit of assorted unwrap spam in our tests and
weird internal usages
- these tests can probably be refactored to not use `SystemState`
directly in the future now that we have better tools like
`run_system_once`, but eh, not this PR's job
- this is semantically correct, as these params could fail validation
- `System::validate_param_unsafe` has been removed, and validation now
occurs inside of `System::run_unsafe`
- very much a net positive for both abstract robustness and current
correctness
- this impacts the strategy that various executors use: see the next
section

There are a *lot* of moving parts here: I'm sorry that I couldn't get
this into a smaller, more incremental refactor. When reviewing this PR,
you should begin with the migration guide to help get you oriented on
the details: `validation_merging.md`.

From there, the most important files to review are:

1. `system_param.rs`: trait changes and implementers
2. `function_system.rs`: primary implementer of `System`
3. `multithreaded.rs`: the parallel executor

**NOTE TO REVIEWERS:** Please make comments to generate threads; this PR
review might get fairly hairy.

### Performance discussion

For the parallel `MultithreadedExecutor`, validation was previously done
as a cheap pre-validation step,
while checking run conditions.
Now, tasks will be spawned for systems which would fail or are skipped
during validation.

In most cases, avoiding the extra overhead of looking up the required
data twice should dominate.
However, this change may negatively affect systems which are frequently
skipped (e.g. due to `Single`).

### Paths not taken

In this PR, I've decided not to:

- Add another variant
[RunSystemError](https://docs.rs/bevy/latest/bevy/ecs/system/enum.RunSystemError.html),
distinguishing "validation failed" from "system ran but returned an
error".
- While reusing
[RunSystemError::Failed](https://docs.rs/bevy/latest/bevy/ecs/system/enum.RunSystemError.html#variant.Failed)
for both cases is messy, this PR is already a bit of a nightmare to
review.
- Return a result from `ParamSet::get_mut`.
   - Instead, we just `unwrap`.
- Bubbling up the `Result` is technically more correct, but these were
already panicking before if e.g. a resource is missing, and `ParamSet`
is already an ergonomic abomination.

## Testing

I've added a number of new tests to exercise the system param validation
paths, ensuring that validation is done when systems are run.

However, I would appreciate some help benchmarking the net impact on
realistic-ish scenes. `breakout`, `bevy_city` and `many_animated_foxes`
are probably a decent scattering, but I'd be very open to other
suggestions.

Having done this refactor, I think that it's a net improvement for
robustness and clarity even without the perf benefits however, and that
we should proceed unless this is a clear regression.

---------

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: Kevin Chen <chen.kevin.f@gmail.com>
This commit is contained in:
Alice Cecile
2026-03-11 12:03:10 -07:00
committed by GitHub
parent f9ddae0b28
commit b265dc042a
53 changed files with 794 additions and 856 deletions
Download Patch File Download Diff File Expand all files Collapse all files
benches/benches/bevy_ecs/fragmentation.rs
+2 -2
View File
@@ -25,7 +25,7 @@ fn iter_frag_empty(c: &mut Criterion) {
spawn_empty_frag_archetype::<Table>(&mut world);
let mut q: SystemState<Query<(Entity, &Table)>> =
SystemState::<Query<(Entity, &Table<0>)>>::new(&mut world);
let query = q.get(&world);
let query = q.get(&world).unwrap();
b.iter(move || {
let mut res = 0;
query.iter().for_each(|(e, t)| {
@@ -39,7 +39,7 @@ fn iter_frag_empty(c: &mut Criterion) {
spawn_empty_frag_archetype::<Sparse>(&mut world);
let mut q: SystemState<Query<(Entity, &Sparse)>> =
SystemState::<Query<(Entity, &Sparse<0>)>>::new(&mut world);
let query = q.get(&world);
let query = q.get(&world).unwrap();
b.iter(move || {
let mut res = 0;
query.iter().for_each(|(e, t)| {
benches/benches/bevy_ecs/world/world_get.rs
+4 -4
View File
@@ -269,7 +269,7 @@ pub fn query_get(criterion: &mut Criterion) {
.collect();
entities.shuffle(&mut deterministic_rand());
let mut query = SystemState::<Query<&Table>>::new(&mut world);
let query = query.get(&world);
let query = query.get(&world).unwrap();
bencher.iter(|| {
let mut count = 0;
@@ -288,7 +288,7 @@ pub fn query_get(criterion: &mut Criterion) {
.collect();
entities.shuffle(&mut deterministic_rand());
let mut query = SystemState::<Query<&Sparse>>::new(&mut world);
let query = query.get(&world);
let query = query.get(&world).unwrap();
bencher.iter(|| {
let mut count = 0;
@@ -319,7 +319,7 @@ pub fn query_get_many<const N: usize>(criterion: &mut Criterion) {
entity_groups.shuffle(&mut deterministic_rand());
let mut query = SystemState::<Query<&Table>>::new(&mut world);
let query = query.get(&world);
let query = query.get(&world).unwrap();
bencher.iter(|| {
let mut count = 0;
@@ -342,7 +342,7 @@ pub fn query_get_many<const N: usize>(criterion: &mut Criterion) {
entity_groups.shuffle(&mut deterministic_rand());
let mut query = SystemState::<Query<&Sparse>>::new(&mut world);
let query = query.get(&world);
let query = query.get(&world).unwrap();
bencher.iter(|| {
let mut count = 0;
crates/bevy_ecs/compile_fail/tests/ui/query_lens_lifetime_safety.rs
+1 -1
View File
@@ -13,7 +13,7 @@ fn main() {
let mut system_state = SystemState::<(Query<&mut Foo>, Query<&mut Bar>)>::new(&mut world);
{
let (mut foo_query, mut bar_query) = system_state.get_mut(&mut world);
let (mut foo_query, mut bar_query) = system_state.get_mut(&mut world).unwrap();
dbg!("hi");
{
let mut lens = foo_query.as_query_lens();
crates/bevy_ecs/compile_fail/tests/ui/query_lifetime_safety.rs
+1 -1
View File
@@ -10,7 +10,7 @@ fn main() {
let mut system_state = SystemState::<Query<&mut Foo>>::new(&mut world);
{
let mut query = system_state.get_mut(&mut world);
let mut query = system_state.get_mut(&mut world).unwrap();
dbg!("hi");
{
let data: &Foo = query.get(e).unwrap();
crates/bevy_ecs/compile_fail/tests/ui/query_transmute_safety.rs
+1 -1
View File
@@ -12,7 +12,7 @@ fn main() {
world.spawn(Foo(10));
let mut system_state = SystemState::<Query<(&mut Foo, &Bar)>>::new(&mut world);
let mut query = system_state.get_mut(&mut world);
let mut query = system_state.get_mut(&mut world).unwrap();
{
let mut lens_a = query.transmute_lens::<&mut Foo>();
crates/bevy_ecs/compile_fail/tests/ui/system_state_get_lifetime_safety.rs
+2 -2
View File
@@ -14,10 +14,10 @@ struct State {
impl State {
fn get_component(&mut self, world: &mut World, entity: Entity) {
let q1 = self.state_r.get(&world);
let q1 = self.state_r.get(&world).unwrap();
let a1 = q1.get(entity).unwrap();
let mut q2 = self.state_w.get_mut(world);
let mut q2 = self.state_w.get_mut(world).unwrap();
//~^ E0502
let _ = q2.get_mut(entity).unwrap();
crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_lifetime_safety.rs
+2 -2
View File
@@ -14,10 +14,10 @@ struct State {
impl State {
fn get_components(&mut self, world: &mut World) {
let q1 = self.state_r.get(&world);
let q1 = self.state_r.get(&world).unwrap();
let a1 = q1.iter().next().unwrap();
let mut q2 = self.state_w.get_mut(world);
let mut q2 = self.state_w.get_mut(world).unwrap();
//~^ E0502
let _ = q2.iter_mut().next().unwrap();
crates/bevy_ecs/compile_fail/tests/ui/system_state_iter_mut_overlap_safety.rs
+1 -1
View File
@@ -11,7 +11,7 @@ fn main() {
let mut system_state = SystemState::<Query<&mut A>>::new(&mut world);
{
let mut query = system_state.get_mut(&mut world);
let mut query = system_state.get_mut(&mut world).unwrap();
let mut_vec = query.iter_mut().collect::<Vec<bevy_ecs::prelude::Mut<A>>>();
assert_eq!(
// this should fail to compile due to the later use of mut_vec
crates/bevy_ecs/macros/src/lib.rs
+9 -20
View File
@@ -451,33 +451,22 @@ fn derive_system_param_impl(
<#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::queue(&mut state.state, system_meta, world);
}
#[inline]
unsafe fn validate_param<'w, 's>(
state: &'s mut Self::State,
_system_meta: &#path::system::SystemMeta,
_world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
) -> Result<(), #path::system::SystemParamValidationError> {
let #state_struct_name { state: (#(#tuple_patterns,)*) } = state;
#(
<#field_types as #path::system::SystemParam>::validate_param(#field_locals, _system_meta, _world)
.map_err(|err| #path::system::SystemParamValidationError::new::<Self>(err.skipped, #field_validation_messages, #field_validation_names))?;
)*
Result::Ok(())
}
#[inline]
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &#path::system::SystemMeta,
world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
change_tick: #path::change_detection::Tick,
) -> Self::Item<'w, 's> {
let (#(#tuple_patterns,)*) = <
(#(#tuple_types,)*) as #path::system::SystemParam
>::get_param(&mut state.state, system_meta, world, change_tick);
#struct_name {
) -> Result<Self::Item<'w, 's>, #path::system::SystemParamValidationError> {
let (#(#tuple_patterns,)*) = &mut state.state;
#(
let #field_locals = unsafe {
<#field_types as #path::system::SystemParam>::get_param(#field_locals, system_meta, world, change_tick)
}.map_err(|err| #path::system::SystemParamValidationError::new::<Self>(err.skipped, #field_validation_messages, #field_validation_names))?;
)*
Result::Ok(#struct_name {
#(#field_members: #field_locals,)*
}
})
}
}
crates/bevy_ecs/src/lifecycle.rs
+3 -3
View File
@@ -60,7 +60,7 @@ use crate::{
query::FilteredAccessSet,
relationship::RelationshipHookMode,
storage::SparseSet,
system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam},
system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamValidationError},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
@@ -639,7 +639,7 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentMessages {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.removed_components()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.removed_components())
}
}
crates/bevy_ecs/src/message/message_reader.rs
+3 -23
View File
@@ -162,35 +162,15 @@ unsafe impl<'w, 's, M: Message> SystemParam for PopulatedMessageReader<'w, 's, M
system_meta: &crate::system::SystemMeta,
world: crate::world::unsafe_world_cell::UnsafeWorldCell<'world>,
change_tick: crate::change_detection::Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: requirements are upheld by MessageReader's implementation
unsafe {
PopulatedMessageReader(MessageReader::get_param(
state,
system_meta,
world,
change_tick,
))
}
}
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &crate::system::SystemMeta,
world: crate::world::unsafe_world_cell::UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: requirements are upheld by MessageReader's implementation
unsafe { MessageReader::<M>::validate_param(state, system_meta, world) }?;
// SAFETY: requirements are upheld by MessageReader's implementation
let reader =
unsafe { MessageReader::get_param(state, system_meta, world, world.change_tick()) };
let reader = unsafe { MessageReader::get_param(state, system_meta, world, change_tick)? };
if reader.is_empty() {
Err(SystemParamValidationError::skipped::<Self>(
"message queue is empty",
))
} else {
Ok(())
Ok(PopulatedMessageReader(reader))
}
}
}
crates/bevy_ecs/src/observer/runner.rs
+1 -5
View File
@@ -107,11 +107,7 @@ pub(super) unsafe fn observer_system_runner<E: Event, B: Bundle, S: ObserverSyst
(*system).refresh_hotpatch();
};
if let Err(RunSystemError::Failed(err)) = (*system)
.validate_param_unsafe(world)
.map_err(From::from)
.and_then(|()| (*system).run_unsafe(on, world))
{
if let Err(RunSystemError::Failed(err)) = (*system).run_unsafe(on, world) {
let handler = state
.error_handler
.unwrap_or_else(|| world.default_error_handler());
crates/bevy_ecs/src/query/mod.rs
+1 -1
View File
@@ -774,7 +774,7 @@ mod tests {
// system param
let mut q = SystemState::<Query<&mut Foo>>::new(&mut world);
let q = q.get_mut(&mut world);
let q = q.get_mut(&mut world).unwrap();
let _: Option<&Foo> = q.iter().next();
let _: Option<[&Foo; 2]> = q.iter_combinations::<2>().next();
let _: Option<&Foo> = q.iter_many([e]).next();
crates/bevy_ecs/src/reflect/entity_commands.rs
+8 -8
View File
@@ -462,7 +462,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn_empty().id();
let entity2 = commands.spawn_empty().id();
@@ -508,7 +508,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn_empty().id();
@@ -538,7 +538,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn(ComponentA(0)).id();
@@ -567,7 +567,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn(ComponentA(0)).id();
@@ -596,7 +596,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn_empty().id();
let bundle = Box::new(BundleA {
@@ -626,7 +626,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands.spawn_empty().id();
let bundle = Box::new(BundleA {
@@ -656,7 +656,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands
.spawn(BundleA {
@@ -694,7 +694,7 @@ mod tests {
world.insert_resource(type_registry);
let mut system_state: SystemState<Commands> = SystemState::new(&mut world);
let mut commands = system_state.get_mut(&mut world);
let mut commands = system_state.get_mut(&mut world).unwrap();
let entity = commands
.spawn(BundleA {
crates/bevy_ecs/src/schedule/executor/mod.rs
+328 -10
View File
@@ -22,7 +22,7 @@ use crate::{
ConditionWithAccess, InternedSystemSet, SystemKey, SystemSetKey, SystemTypeSet,
SystemWithAccess,
},
system::{RunSystemError, System, SystemIn, SystemParamValidationError, SystemStateFlags},
system::{RunSystemError, System, SystemIn, SystemStateFlags},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
@@ -197,15 +197,6 @@ impl System for ApplyDeferred {
fn queue_deferred(&mut self, _world: DeferredWorld) {}
unsafe fn validate_param_unsafe(
&mut self,
_world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// This system is always valid to run because it doesn't do anything,
// and only used as a marker for the executor.
Ok(())
}
fn initialize(&mut self, _world: &mut World) -> FilteredAccessSet {
FilteredAccessSet::new()
}
@@ -561,3 +552,330 @@ mod tests {
assert_eq!(counter.0, 0);
}
}
#[cfg(test)]
mod validation_tests {
use crate::{
prelude::{Component, In, IntoSystem, Resource, Schedule},
schedule::ExecutorKind,
system::{
DynParamBuilder, DynSystemParam, ExclusiveSystemParam, Local, ParamBuilder, ParamSet,
Query, Res, ResMut, RunSystemError, RunSystemOnce, Single, SystemMeta,
SystemParamBuilder, SystemParamValidationError,
},
world::World,
};
#[derive(Component)]
struct TestComponent;
const EXECUTORS: [ExecutorKind; 2] =
[ExecutorKind::SingleThreaded, ExecutorKind::MultiThreaded];
#[derive(Resource, Default)]
struct Counter(u8);
// A resource that won't be inserted, causing validation to fail.
#[derive(Resource)]
struct MissingResource;
/// An [`ExclusiveSystemParam`] that always fails validation.
struct AlwaysInvalid;
impl ExclusiveSystemParam for AlwaysInvalid {
type State = ();
type Item<'s> = AlwaysInvalid;
fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
fn get_param<'s>(
_state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Err(SystemParamValidationError::invalid::<Self>(
"always invalid",
))
}
}
#[test]
fn function_system_validation_failure_is_error() {
fn system(_res: Res<MissingResource>) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed, got {result:?}"
);
}
#[test]
fn function_system_validation_skip() {
fn system(_single: Single<&TestComponent>) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped, got {result:?}"
);
}
#[test]
fn function_system_validation_success() {
fn system(_res: Res<Counter>) {}
let mut world = World::new();
world.init_resource::<Counter>();
let result = world.run_system_once(system);
assert!(result.is_ok(), "Expected Ok, got {result:?}");
}
#[test]
fn adapter_system_validation_failure() {
fn system(_res: Res<MissingResource>) -> u32 {
42
}
let mut world = World::new();
let result = world.run_system_once(system.map(|_x| {}));
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from adapter system, got {result:?}"
);
}
#[test]
fn adapter_system_validation_skip() {
fn system(_single: Single<&TestComponent>) -> u32 {
42
}
let mut world = World::new();
let result = world.run_system_once(system.map(|_x| {}));
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from adapter system, got {result:?}"
);
}
#[test]
fn pipe_system_validation_failure_in_first() {
fn first(_res: Res<MissingResource>) -> u32 {
42
}
fn second(_input: In<u32>) {}
let mut world = World::new();
let result = world.run_system_once(first.pipe(second));
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from pipe first system, got {result:?}"
);
}
#[test]
fn pipe_system_validation_failure_in_second() {
fn first() -> u32 {
42
}
fn second(_input: In<u32>, _res: Res<MissingResource>) {}
let mut world = World::new();
let result = world.run_system_once(first.pipe(second));
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from pipe second system, got {result:?}"
);
}
#[test]
fn pipe_system_validation_skip_in_first() {
fn first(_single: Single<&TestComponent>) -> u32 {
42
}
fn second(_input: In<u32>) {}
let mut world = World::new();
let result = world.run_system_once(first.pipe(second));
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from pipe first system, got {result:?}"
);
}
#[test]
fn pipe_system_validation_skip_in_second() {
fn first() -> u32 {
42
}
fn second(_input: In<u32>, _single: Single<&TestComponent>) {}
let mut world = World::new();
let result = world.run_system_once(first.pipe(second));
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from pipe second system, got {result:?}"
);
}
#[test]
fn builder_system_validation_failure() {
fn system(_res: Res<MissingResource>) {}
let mut world = World::new();
let result = world.run_system_once(ParamBuilder.build_system(system));
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from builder system, got {result:?}"
);
}
#[test]
fn builder_system_validation_skip() {
fn system(_single: Single<&TestComponent>) {}
let mut world = World::new();
let result = world.run_system_once(ParamBuilder.build_system(system));
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from builder system, got {result:?}"
);
}
#[test]
fn dyn_system_param_validation_failure() {
let mut world = World::new();
let system = (DynParamBuilder::new::<Res<MissingResource>>(ParamBuilder),)
.build_state(&mut world)
.build_system(|_param: DynSystemParam| {});
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from DynSystemParam system, got {result:?}"
);
}
#[test]
fn dyn_system_param_validation_skip() {
let mut world = World::new();
let system = (DynParamBuilder::new::<Single<&TestComponent>>(ParamBuilder),)
.build_state(&mut world)
.build_system(|_param: DynSystemParam| {});
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from DynSystemParam system, got {result:?}"
);
}
#[test]
fn dyn_system_param_validation_success() {
let mut world = World::new();
world.init_resource::<Counter>();
let system = (DynParamBuilder::new::<Res<Counter>>(ParamBuilder),)
.build_state(&mut world)
.build_system(|_param: DynSystemParam| {});
let result = world.run_system_once(system);
assert!(
result.is_ok(),
"Expected Ok from DynSystemParam system, got {result:?}"
);
}
#[test]
fn exclusive_system_validation_failure() {
fn system(_world: &mut World, _param: AlwaysInvalid) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from exclusive system, got {result:?}"
);
}
#[test]
fn exclusive_system_validation_success() {
fn system(_world: &mut World, mut _local: Local<u32>) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
result.is_ok(),
"Expected Ok from exclusive system, got {result:?}"
);
}
#[test]
fn validation_skips_system_in_schedule() {
// Ensure the executor properly handles validation failures by skipping
// and not running the system body.
fn skippable_system(_single: Single<&TestComponent>, mut counter: ResMut<Counter>) {
counter.0 += 1;
}
for executor in EXECUTORS {
let mut world = World::new();
world.init_resource::<Counter>();
let mut schedule = Schedule::default();
schedule.set_executor_kind(executor);
schedule.add_systems(skippable_system);
// No TestComponent entity exists, so the system should be skipped.
schedule.run(&mut world);
assert_eq!(
world.resource::<Counter>().0,
0,
"System should have been skipped with {executor:?}"
);
}
}
#[test]
fn param_set_validation_skip() {
// A system using ParamSet with a Single sub-param should be skipped
// when the Single has no matching entities, rather than panicking.
fn system(mut _set: ParamSet<(Single<&TestComponent>,)>) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Skipped(_))),
"Expected Skipped from ParamSet with invalid Single, got {result:?}"
);
}
#[test]
fn param_set_validation_failure() {
// A system using ParamSet with a Res sub-param should fail validation
// when the resource does not exist.
fn system(mut _set: ParamSet<(Query<&TestComponent>, Res<MissingResource>)>) {}
let mut world = World::new();
let result = world.run_system_once(system);
assert!(
matches!(result, Err(RunSystemError::Failed(_))),
"Expected Failed from ParamSet with missing resource, got {result:?}"
);
}
#[test]
fn param_set_validation_success() {
// A system using ParamSet with valid sub-params should succeed.
fn system(mut set: ParamSet<(Query<&TestComponent>, Res<Counter>)>) {
let _q = set.p0();
}
let mut world = World::new();
world.init_resource::<Counter>();
let result = world.run_system_once(system);
assert!(
result.is_ok(),
"Expected Ok from ParamSet with valid params, got {result:?}"
);
}
}
crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+1 -36
View File
@@ -644,32 +644,6 @@ impl ExecutorState {
should_run &= system_conditions_met;
if should_run {
// SAFETY:
// - The caller ensures that `world` has permission to read any data
// required by the system.
let valid_params = match unsafe { system.validate_param_unsafe(world) } {
Ok(()) => true,
Err(e) => {
if !e.skipped {
error_handler(
e.into(),
ErrorContext::System {
name: system.name(),
last_run: system.get_last_run(),
},
);
}
false
}
};
if !valid_params {
self.skipped_systems.insert(system_index);
}
should_run &= valid_params;
}
should_run
}
@@ -852,16 +826,7 @@ unsafe fn evaluate_and_fold_conditions(
// SAFETY:
// - The caller ensures that `world` has permission to read any data
// required by the condition.
unsafe { condition.validate_param_unsafe(world) }
.map_err(From::from)
.and_then(|()| {
// SAFETY:
// - The caller ensures that `world` has permission to read any data
// required by the condition.
unsafe {
__rust_begin_short_backtrace::readonly_run_unsafe(&mut **condition, world)
}
})
unsafe { __rust_begin_short_backtrace::readonly_run_unsafe(&mut **condition, world) }
.unwrap_or_else(|err| {
if let RunSystemError::Failed(err) = err {
error_handler(
crates/bevy_ecs/src/schedule/mod.rs
+1 -1
View File
@@ -1304,7 +1304,7 @@ mod tests {
// start a new frame by running ihe begin_frame() system
let mut system_state: SystemState<Option<ResMut<Stepping>>> =
SystemState::new(&mut world);
let res = system_state.get_mut(&mut world);
let res = system_state.get_mut(&mut world).unwrap();
Stepping::begin_frame(res);
// now run the schedule; this will panic if the executor doesn't
crates/bevy_ecs/src/schedule/node.rs
+1 -22
View File
@@ -24,10 +24,7 @@ use crate::{
BoxedCondition, InternedSystemSet, ScheduleGraph,
},
storage::SparseSetIndex,
system::{
ReadOnlySystem, RunSystemError, ScheduleSystem, System, SystemParamValidationError,
SystemStateFlags,
},
system::{ReadOnlySystem, RunSystemError, ScheduleSystem, System, SystemStateFlags},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
@@ -101,15 +98,6 @@ impl System for SystemWithAccess {
self.system.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.system.validate_param_unsafe(world) }
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
self.system.initialize(world)
@@ -201,15 +189,6 @@ impl System for ConditionWithAccess {
self.condition.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Caller ensures the same safety requirements.
unsafe { self.condition.validate_param_unsafe(world) }
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
self.condition.initialize(world)
crates/bevy_ecs/src/system/adapter_system.rs
+1 -10
View File
@@ -1,7 +1,7 @@
use alloc::vec::Vec;
use bevy_utils::prelude::DebugName;
use super::{IntoSystem, ReadOnlySystem, RunSystemError, System, SystemParamValidationError};
use super::{IntoSystem, ReadOnlySystem, RunSystemError, System};
use crate::{
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn},
@@ -162,15 +162,6 @@ where
self.system.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.system.validate_param_unsafe(world) }
}
fn initialize(&mut self, world: &mut crate::prelude::World) -> crate::query::FilteredAccessSet {
self.system.initialize(world)
}
crates/bevy_ecs/src/system/builder.rs
-18
View File
@@ -420,24 +420,6 @@ where
}
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
match &mut self.inner {
// SAFETY: requirements upheld by the caller.
BuilderSystemInner::Initialized { system, .. } => unsafe {
system.validate_param_unsafe(world)
},
BuilderSystemInner::Uninitialized { .. } => panic!(
"BuilderSystem {} was not initialized before calling validate_param_unsafe.",
self.name()
),
BuilderSystemInner::Invalid => unreachable!(),
}
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
let inner = mem::replace(&mut self.inner, BuilderSystemInner::Invalid);
crates/bevy_ecs/src/system/combinator.rs
+2 -31
View File
@@ -8,7 +8,7 @@ use crate::{
prelude::World,
query::FilteredAccessSet,
schedule::InternedSystemSet,
system::{input::SystemInput, SystemIn, SystemParamValidationError},
system::{input::SystemInput, SystemIn},
world::unsafe_world_cell::UnsafeWorldCell,
};
@@ -168,10 +168,7 @@ where
world: &mut PrivateUnsafeWorldCell,
) -> Result<S::Out, RunSystemError> {
// SAFETY: see comment on `Func::combine` call
match (|| unsafe {
system.validate_param_unsafe(world.0)?;
system.run_unsafe(input, world.0)
})() {
match unsafe { system.run_unsafe(input, world.0) } {
// let the world's default error handler handle the error if `Failed(_)`
Err(RunSystemError::Failed(err)) => {
// SAFETY: We registered access to DefaultErrorHandler in `initialize`.
@@ -233,17 +230,6 @@ where
self.b.queue_deferred(world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
_world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// Both systems are validated in `Self::run_unsafe`, so that we get the
// chance to run the second system even if the first one fails to
// validate.
Ok(())
}
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
let mut a_access = self.a.initialize(world);
let b_access = self.b.initialize(world);
@@ -415,9 +401,6 @@ where
// SAFETY: Upheld by caller
unsafe {
let value = self.a.run_unsafe(input, world)?;
// `Self::validate_param_unsafe` already validated the first system,
// but we still need to validate the second system once the first one runs.
self.b.validate_param_unsafe(world)?;
self.b.run_unsafe(value, world)
}
}
@@ -439,18 +422,6 @@ where
self.b.queue_deferred(world);
}
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// We only validate parameters for the first system,
// since it may make changes to the world that affect
// whether the second system has valid parameters.
// The second system will be validated in `Self::run_unsafe`.
// SAFETY: Delegate to the `System` implementation for `a`.
unsafe { self.a.validate_param_unsafe(world) }
}
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
let mut a_access = self.a.initialize(world);
let b_access = self.b.initialize(world);
crates/bevy_ecs/src/system/commands/mod.rs
+4 -20
View File
@@ -176,22 +176,6 @@ const _: () = {
);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &bevy_ecs::system::SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe {
<__StructFieldsAlias as bevy_ecs::system::SystemParam>::validate_param(
&mut state.state,
system_meta,
world,
)
}
}
#[inline]
#[track_caller]
unsafe fn get_param<'w, 's>(
@@ -199,7 +183,7 @@ const _: () = {
system_meta: &bevy_ecs::system::SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: bevy_ecs::change_detection::Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: Upheld by caller
let params = unsafe {
<__StructFieldsAlias as bevy_ecs::system::SystemParam>::get_param(
@@ -207,13 +191,13 @@ const _: () = {
system_meta,
world,
change_tick,
)
)?
};
Commands {
Ok(Commands {
queue: InternalQueue::CommandQueue(params.0),
allocator: params.1,
entities: params.2,
}
})
}
}
// SAFETY: Only reads Entities
crates/bevy_ecs/src/system/exclusive_function_system.rs
+2 -11
View File
@@ -15,7 +15,7 @@ use bevy_utils::prelude::DebugName;
use core::marker::PhantomData;
use variadics_please::all_tuples;
use super::{RunSystemError, SystemParamValidationError, SystemStateFlags};
use super::{RunSystemError, SystemStateFlags};
/// A function system that runs with exclusive [`World`] access.
///
@@ -117,7 +117,7 @@ where
let params = F::Param::get_param(
self.param_state.as_mut().expect(PARAM_MESSAGE),
&self.system_meta,
);
)?;
#[cfg(feature = "hotpatching")]
let out = {
@@ -166,15 +166,6 @@ where
// might have buffers to apply, but this is handled by `PipeSystem`.
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
_world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// All exclusive system params are always available.
Ok(())
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX);
crates/bevy_ecs/src/system/exclusive_system_param.rs
+27 -12
View File
@@ -1,7 +1,7 @@
use crate::{
prelude::{FromWorld, QueryState},
query::{QueryData, QueryFilter},
system::{Local, SystemMeta, SystemParam, SystemState},
system::{Local, SystemMeta, SystemParam, SystemParamValidationError, SystemState},
world::World,
};
use bevy_platform::cell::SyncCell;
@@ -27,7 +27,10 @@ pub trait ExclusiveSystemParam: Sized {
/// Creates a parameter to be passed into an [`ExclusiveSystemParamFunction`].
///
/// [`ExclusiveSystemParamFunction`]: super::ExclusiveSystemParamFunction
fn get_param<'s>(state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s>;
fn get_param<'s>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError>;
}
/// Shorthand way of accessing the associated type [`ExclusiveSystemParam::Item`]
@@ -44,8 +47,11 @@ impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> ExclusiveSystemParam
QueryState::new(world)
}
fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> {
state
fn get_param<'s>(
state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(state)
}
}
@@ -57,8 +63,11 @@ impl<'a, P: SystemParam + 'static> ExclusiveSystemParam for &'a mut SystemState<
SystemState::new(world)
}
fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> {
state
fn get_param<'s>(
state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(state)
}
}
@@ -70,8 +79,11 @@ impl<'_s, T: FromWorld + Send + 'static> ExclusiveSystemParam for Local<'_s, T>
SyncCell::new(T::from_world(world))
}
fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> {
Local(state.get())
fn get_param<'s>(
state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(Local(state.get()))
}
}
@@ -81,8 +93,11 @@ impl<S: ?Sized> ExclusiveSystemParam for PhantomData<S> {
fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
fn get_param<'s>(_state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> {
PhantomData
fn get_param<'s>(
_state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(PhantomData)
}
}
@@ -115,13 +130,13 @@ macro_rules! impl_exclusive_system_param_tuple {
fn get_param<'s>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
) -> Self::Item<'s> {
) -> Result<Self::Item<'s>, SystemParamValidationError> {
let ($($param,)*) = state;
#[allow(
clippy::unused_unit,
reason = "Zero-length tuples won't have any params to get."
)]
($($param::get_param($param, system_meta),)*)
Ok(($($param::get_param($param, system_meta)?,)*))
}
}
};
crates/bevy_ecs/src/system/function_system.rs
+25 -41
View File
@@ -183,7 +183,7 @@ impl SystemMeta {
///
/// // Use system_state.get_mut(&mut world) and unpack your system parameters into variables!
/// // system_state.get(&world) provides read-only versions of your system parameters instead.
/// let (message_writer, maybe_resource, query) = system_state.get_mut(&mut world);
/// let (message_writer, maybe_resource, query) = system_state.get_mut(&mut world).unwrap();
///
/// // If you are using `Commands`, you can choose when you want to apply them to the world.
/// // You need to manually call `.apply(world)` on the `SystemState` to apply them.
@@ -213,7 +213,7 @@ impl SystemMeta {
///
/// // Later, fetch the cached system state, saving on overhead
/// world.resource_scope(|world, mut cached_state: Mut<CachedSystemState>| {
/// let mut message_reader = cached_state.message_state.get_mut(world);
/// let mut message_reader = cached_state.message_state.get_mut(world).unwrap();
///
/// for message in message_reader.read() {
/// println!("Hello World!");
@@ -229,7 +229,7 @@ impl SystemMeta {
/// # struct MyMessage;
/// #
/// fn exclusive_system(world: &mut World, system_state: &mut SystemState<MessageReader<MyMessage>>) {
/// let mut message_reader = system_state.get_mut(world);
/// let mut message_reader = system_state.get_mut(world).unwrap();
///
/// for message in message_reader.read() {
/// println!("Hello World!");
@@ -367,8 +367,13 @@ impl<Param: SystemParam> SystemState<Param> {
}
/// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only.
///
/// Returns an error if system parameter validation fails.
#[inline]
pub fn get<'w, 's>(&'s mut self, world: &'w World) -> SystemParamItem<'w, 's, Param>
pub fn get<'w, 's>(
&'s mut self,
world: &'w World,
) -> Result<SystemParamItem<'w, 's, Param>, SystemParamValidationError>
where
Param: ReadOnlySystemParam,
{
@@ -379,9 +384,14 @@ impl<Param: SystemParam> SystemState<Param> {
}
/// Retrieve the mutable [`SystemParam`] values.
///
/// Returns an error if system parameter validation fails.
#[inline]
#[track_caller]
pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> {
pub fn get_mut<'w, 's>(
&'s mut self,
world: &'w mut World,
) -> Result<SystemParamItem<'w, 's, Param>, SystemParamValidationError> {
self.validate_world(world.id());
// SAFETY: World is uniquely borrowed and matches the World this SystemState was created with.
unsafe { self.get_unchecked(world.as_unsafe_world_cell()) }
@@ -395,21 +405,6 @@ impl<Param: SystemParam> SystemState<Param> {
Param::apply(&mut self.param_state, &self.meta, world);
}
/// Wrapper over [`SystemParam::validate_param`].
///
/// # Safety
///
/// - The passed [`UnsafeWorldCell`] must have read-only access to
/// world data in `component_access_set`.
/// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state).
pub unsafe fn validate_param(
state: &mut Self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegated to existing `SystemParam` implementations.
unsafe { Param::validate_param(&mut state.param_state, &state.meta, world) }
}
/// Returns `true` if `world_id` matches the [`World`] that was used to call [`SystemState::new`].
/// Otherwise, this returns false.
#[inline]
@@ -435,6 +430,8 @@ impl<Param: SystemParam> SystemState<Param> {
/// Retrieve the [`SystemParam`] values.
///
/// Returns an error if system parameter validation fails.
///
/// # Safety
/// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data
/// access is safe in the context of global [`World`] access. The passed-in [`World`] _must_ be the [`World`] the [`SystemState`] was
@@ -444,7 +441,7 @@ impl<Param: SystemParam> SystemState<Param> {
pub unsafe fn get_unchecked<'w, 's>(
&'s mut self,
world: UnsafeWorldCell<'w>,
) -> SystemParamItem<'w, 's, Param> {
) -> Result<SystemParamItem<'w, 's, Param>, SystemParamValidationError> {
let change_tick = world.increment_change_tick();
// SAFETY: The invariants are upheld by the caller.
unsafe { self.fetch(world, change_tick) }
@@ -460,12 +457,12 @@ impl<Param: SystemParam> SystemState<Param> {
&'s mut self,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> SystemParamItem<'w, 's, Param> {
) -> Result<SystemParamItem<'w, 's, Param>, SystemParamValidationError> {
// SAFETY: The invariants are upheld by the caller.
let param =
unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) };
unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) }?;
self.meta.last_run = change_tick;
param
Ok(param)
}
/// Returns a reference to the current system param states.
@@ -523,7 +520,7 @@ struct FunctionSystemState<P: SystemParam> {
/// The cached state of the system's [`SystemParam`]s.
param: P::State,
/// The id of the [`World`] this system was initialized with. If the world
/// passed to [`System::run_unsafe`] or [`System::validate_param_unsafe`] does not match
/// passed to [`System::run_unsafe`] does not match
/// this id, a panic will occur.
world_id: WorldId,
}
@@ -680,8 +677,9 @@ where
// - The above assert ensures the world matches.
// - All world accesses used by `F::Param` have been registered, so the caller
// will ensure that there are no data access conflicts.
let params =
unsafe { F::Param::get_param(&mut state.param, &self.system_meta, world, change_tick) };
let params = unsafe {
F::Param::get_param(&mut state.param, &self.system_meta, world, change_tick)
}?;
#[cfg(feature = "hotpatching")]
let out = {
@@ -723,20 +721,6 @@ where
F::Param::queue(param_state, &self.system_meta, world);
}
#[inline]
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
let state = self.state.as_mut().expect(Self::ERROR_UNINITIALIZED);
assert_eq!(state.world_id, world.id(), "Encountered a mismatched World. A System cannot be used with Worlds other than the one it was initialized with.");
// SAFETY:
// - The above assert ensures the world matches.
// - All world accesses used by `F::Param` have been registered, so the caller
// will ensure that there are no data access conflicts.
unsafe { F::Param::validate_param(&mut state.param, &self.system_meta, world) }
}
#[inline]
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
if let Some(state) = &self.state {
crates/bevy_ecs/src/system/mod.rs
+14 -14
View File
@@ -1318,7 +1318,7 @@ mod tests {
Option<Single<&B>>,
ParamSet<(Query<&C>, Query<&D>)>,
)> = SystemState::new(&mut world);
let (a, query, _) = system_state.get(&world);
let (a, query, _) = system_state.get(&world).unwrap();
assert_eq!(*a, A(42), "returned resource matches initial value");
assert_eq!(
**query.unwrap(),
@@ -1345,7 +1345,7 @@ mod tests {
// The following line shouldn't compile because the parameters used are not ReadOnlySystemParam
// let (a, query) = system_state.get(&world);
let (a, query) = system_state.get_mut(&mut world);
let (a, query) = system_state.get_mut(&mut world).unwrap();
assert_eq!(*a, A(42), "returned resource matches initial value");
assert_eq!(
**query.unwrap(),
@@ -1365,18 +1365,18 @@ mod tests {
let mut system_state: SystemState<Option<Single<&A, Changed<A>>>> =
SystemState::new(&mut world);
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert_eq!(**query.unwrap(), A(1));
}
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert!(query.is_none());
}
world.entity_mut(entity).get_mut::<A>().unwrap().0 = 2;
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert_eq!(**query.unwrap(), A(2));
}
}
@@ -1390,12 +1390,12 @@ mod tests {
let mut system_state: SystemState<Option<Single<(&A, SpawnDetails), Spawned>>> =
SystemState::new(&mut world);
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert_eq!(query.unwrap().1.spawn_tick(), spawn_tick);
}
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert!(query.is_none());
}
}
@@ -1406,7 +1406,7 @@ mod tests {
let mut world = World::default();
let mut system_state = SystemState::<Query<&A>>::new(&mut world);
let mismatched_world = World::default();
system_state.get(&mismatched_world);
system_state.get(&mismatched_world).unwrap();
}
#[test]
@@ -1422,7 +1422,7 @@ mod tests {
let mut system_state = SystemState::<Query<&A>>::new(&mut world);
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert_eq!(
query.iter().collect::<Vec<_>>(),
vec![&A(1)],
@@ -1432,7 +1432,7 @@ mod tests {
world.spawn((A(2), B(2)));
{
let query = system_state.get(&world);
let query = system_state.get(&world).unwrap();
assert_eq!(
query.iter().collect::<Vec<_>>(),
vec![&A(1), &A(2)],
@@ -1462,19 +1462,19 @@ mod tests {
impl State {
fn hold_res<'w>(&mut self, world: &'w World) -> ResourceHolder<'w> {
let a = self.state.get(world);
let a = self.state.get(world).unwrap();
ResourceHolder {
value: a.into_inner(),
}
}
fn hold_component<'w>(&mut self, world: &'w World, entity: Entity) -> Holder<'w> {
let q = self.state_q.get(world);
let q = self.state_q.get(world).unwrap();
let a = q.get_inner(entity).unwrap();
Holder { value: a }
}
fn hold_components<'w>(&mut self, world: &'w World) -> Vec<Holder<'w>> {
let mut components = Vec::new();
let q = self.state_q.get(world);
let q = self.state_q.get(world).unwrap();
for a in q.iter_inner() {
components.push(Holder { value: a });
}
@@ -1494,7 +1494,7 @@ mod tests {
let mut system_state = SystemState::<Query<&mut A>>::new(&mut world);
{
let mut query = system_state.get_mut(&mut world);
let mut query = system_state.get_mut(&mut world).unwrap();
assert_eq!(
query.iter_mut().map(|m| *m).collect::<Vec<A>>(),
vec![A(1), A(2)],
crates/bevy_ecs/src/system/schedule_system.rs
+1 -17
View File
@@ -8,7 +8,7 @@ use crate::{
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FromWorld, World},
};
use super::{IntoSystem, SystemParamValidationError, SystemStateFlags};
use super::{IntoSystem, SystemStateFlags};
/// See [`IntoSystem::with_input`] for details.
pub struct WithInputWrapper<S, T>
@@ -84,14 +84,6 @@ where
self.system.queue_deferred(world);
}
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { self.system.validate_param_unsafe(world) }
}
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
self.system.initialize(world)
}
@@ -183,14 +175,6 @@ where
self.system.queue_deferred(world);
}
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { self.system.validate_param_unsafe(world) }
}
fn initialize(&mut self, world: &mut World) -> FilteredAccessSet {
if self.value.is_none() {
self.value = Some(T::from_world(world));
crates/bevy_ecs/src/system/system.rs
-36
View File
@@ -132,9 +132,6 @@ pub trait System: Send + Sync + 'static {
let world_cell = world.as_unsafe_world_cell();
// SAFETY:
// - We have exclusive access to the entire world.
unsafe { self.validate_param_unsafe(world_cell) }?;
// SAFETY:
// - We have exclusive access to the entire world.
unsafe { self.run_unsafe(input, world_cell) }
}
@@ -147,36 +144,6 @@ pub trait System: Send + Sync + 'static {
/// of this system into the world's command buffer.
fn queue_deferred(&mut self, world: DeferredWorld);
/// Validates that all parameters can be acquired and that system can run without panic.
/// Built-in executors use this to prevent invalid systems from running.
///
/// However calling and respecting [`System::validate_param_unsafe`] or its safe variant
/// is not a strict requirement, both [`System::run`] and [`System::run_unsafe`]
/// should provide their own safety mechanism to prevent undefined behavior.
///
/// This method has to be called directly before [`System::run_unsafe`] with no other (relevant)
/// world mutations in between. Otherwise, while it won't lead to any undefined behavior,
/// the validity of the param may change.
///
/// # Safety
///
/// - The caller must ensure that [`world`](UnsafeWorldCell) has permission to access any world data
/// registered in the access returned from [`System::initialize`]. There must be no conflicting
/// simultaneous accesses while the system is running.
unsafe fn validate_param_unsafe(
&mut self,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError>;
/// Safe version of [`System::validate_param_unsafe`].
/// that runs on exclusive, single-threaded `world` pointer.
fn validate_param(&mut self, world: &World) -> Result<(), SystemParamValidationError> {
let world_cell = world.as_unsafe_world_cell_readonly();
// SAFETY:
// - We have exclusive access to the entire world.
unsafe { self.validate_param_unsafe(world_cell) }
}
/// Initialize the system.
///
/// Returns a [`FilteredAccessSet`] with the access required to run the system.
@@ -237,9 +204,6 @@ pub unsafe trait ReadOnlySystem: System {
let world = world.as_unsafe_world_cell_readonly();
// SAFETY:
// - We have read-only access to the entire world.
unsafe { self.validate_param_unsafe(world) }?;
// SAFETY:
// - We have read-only access to the entire world.
unsafe { self.run_unsafe(input, world) }
}
}
crates/bevy_ecs/src/system/system_name.rs
+11 -5
View File
@@ -2,7 +2,10 @@ use crate::{
change_detection::Tick,
prelude::World,
query::FilteredAccessSet,
system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam},
system::{
ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam,
SystemParamValidationError,
},
world::unsafe_world_cell::UnsafeWorldCell,
};
use bevy_utils::prelude::DebugName;
@@ -65,8 +68,8 @@ unsafe impl SystemParam for SystemName {
system_meta: &SystemMeta,
_world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
SystemName(system_meta.name.clone())
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(SystemName(system_meta.name.clone()))
}
}
@@ -79,8 +82,11 @@ impl ExclusiveSystemParam for SystemName {
fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
fn get_param<'s>(_state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s> {
SystemName(system_meta.name.clone())
fn get_param<'s>(
_state: &'s mut Self::State,
system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(SystemName(system_meta.name.clone()))
}
}
crates/bevy_ecs/src/system/system_param.rs
+137 -359
View File
@@ -15,7 +15,6 @@ use crate::{
QuerySingleError, QueryState, ReadOnlyQueryData,
},
resource::{Resource, IS_RESOURCE},
storage::NonSendData,
system::{Query, Single, SystemMeta},
world::{
unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut,
@@ -257,53 +256,17 @@ pub unsafe trait SystemParam: Sized {
)]
fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) {}
/// Validates that the param can be acquired by the [`get_param`](SystemParam::get_param).
/// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction).
///
/// Built-in executors use this to prevent systems with invalid params from running,
/// and any failures here will be bubbled up to the default error handler defined in [`bevy_ecs::error`],
/// with a value of type [`SystemParamValidationError`].
/// This method also validates that the param can be acquired. If validation fails,
/// an appropriate [`SystemParamValidationError`] should be returned.
/// Systems will convert this to a [`RunSystemError`](super::RunSystemError),
/// and the built-in executors will ignore any "skipped" validation results,
/// but pass any "invalid" results to the default error handler defined in [`bevy_ecs::error`].
///
/// For nested [`SystemParam`]s validation will fail if any
/// delegated validation fails.
///
/// However calling and respecting [`SystemParam::validate_param`]
/// is not a strict requirement, [`SystemParam::get_param`] should
/// provide it's own safety mechanism to prevent undefined behavior.
///
/// The [`world`](UnsafeWorldCell) can only be used to read param's data
/// and world metadata. No data can be written.
///
/// When using system parameters that require `change_tick` you can use
/// [`UnsafeWorldCell::change_tick()`]. Even if this isn't the exact
/// same tick used for [`SystemParam::get_param`], the world access
/// ensures that the queried data will be the same in both calls.
///
/// This method has to be called directly before [`SystemParam::get_param`] with no other (relevant)
/// world mutations inbetween. Otherwise, while it won't lead to any undefined behavior,
/// the validity of the param may change.
///
/// [`System::validate_param`](super::system::System::validate_param),
/// calls this method for each supplied system param.
///
/// # Safety
///
/// - The passed [`UnsafeWorldCell`] must have read-only access to world data
/// registered in [`init_access`](SystemParam::init_access).
/// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state).
#[expect(
unused_variables,
reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion."
)]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
Ok(())
}
/// Creates a parameter to be passed into a [`SystemParamFunction`](super::SystemParamFunction).
///
/// # Safety
///
/// - The passed [`UnsafeWorldCell`] must have access to any world data registered
@@ -317,7 +280,7 @@ pub unsafe trait SystemParam: Sized {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state>;
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError>;
}
/// A [`SystemParam`] that only reads a given [`World`].
@@ -363,12 +326,12 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam for Qu
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: We have registered all of the query's world accesses,
// so the caller ensures that `world` has permission to access any
// world data that the query needs.
// The caller ensures the world matches the one used in init_state.
unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) }
Ok(unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) })
}
}
@@ -399,34 +362,16 @@ unsafe impl<'a, 'b, D: IterQueryData + 'static, F: QueryFilter + 'static> System
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: State ensures that the components it accesses are not accessible somewhere elsewhere.
// The caller ensures the world matches the one used in init_state.
let query =
unsafe { state.query_unchecked_with_ticks(world, system_meta.last_run, change_tick) };
let single = query
.single_inner()
.expect("The query was expected to contain exactly one matching entity.");
Single {
item: single,
_filter: PhantomData,
}
}
#[inline]
unsafe fn validate_param(
state: &mut 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_with_ticks(world, system_meta.last_run, world.change_tick())
};
match query.single_inner() {
Ok(_) => Ok(()),
Ok(single) => Ok(Single {
item: single,
_filter: PhantomData,
}),
Err(QuerySingleError::NoEntities(_)) => Err(
SystemParamValidationError::skipped::<Self>("No matching entities"),
),
@@ -470,30 +415,15 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// 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: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY:
// - We have read-only access to the components accessed by query.
// - The caller ensures the world matches the one used in init_state.
let query = unsafe {
state.query_unchecked_with_ticks(world, system_meta.last_run, world.change_tick())
};
let query = unsafe { Query::get_param(state, system_meta, world, change_tick) }?;
if query.is_empty() {
Err(SystemParamValidationError::skipped::<Self>(
"No matching entities",
))
} else {
Ok(())
Ok(Populated(query))
}
}
}
@@ -682,31 +612,29 @@ macro_rules! impl_param_set {
<($($param,)*) as SystemParam>::queue(state, system_meta, world.reborrow());
}
#[inline]
unsafe fn validate_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe {
<($($param,)*) as SystemParam>::validate_param(state, system_meta, world)
}
}
#[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> {
ParamSet {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// Validate each sub-param eagerly so that the system is correctly
// skipped by the executor when any sub-param is unavailable.
// PERF: the sub-params will be fetched again lazily when accessed through
// the ParamSet, but this is no worse than the previous
// validate_param + get_param pattern.
$(
// SAFETY: Upheld by caller.
drop(unsafe { $param::get_param(&mut state.$index, system_meta, world, change_tick) }?);
)*
Ok(ParamSet {
param_states: state,
system_meta: system_meta.clone(),
world,
change_tick,
}
})
}
}
@@ -724,6 +652,7 @@ macro_rules! impl_param_set {
unsafe {
$param::get_param(&mut self.param_states.$index, &self.system_meta, self.world, self.change_tick)
}
.unwrap_or_else(|err| panic!("ParamSet parameter validation failed: {err}"))
}
)*
}
@@ -769,42 +698,17 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
panic!("error[B0002]: Res<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access using `Without<IsResource>` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::<T>(), system_meta.name);
}
#[inline]
unsafe fn validate_param(
&mut component_id: &mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to the resource
if let Some(entity) = unsafe { world.resource_entities() }.get(component_id)
&& let Ok(entity_ref) = world.get_entity(*entity)
&& entity_ref.contains_id(component_id)
{
Ok(())
} else {
Err(SystemParamValidationError::invalid::<Self>(
"Resource does not exist",
))
}
}
#[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> {
let (ptr, ticks) = world
.get_resource_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
"Resource requested by {} does not exist: {}",
system_meta.name,
DebugName::type_name::<T>()
);
});
Res {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
let (ptr, ticks) = world.get_resource_with_ticks(component_id).ok_or_else(|| {
SystemParamValidationError::invalid::<Self>("Resource does not exist")
})?;
Ok(Res {
value: ptr.deref(),
ticks: ComponentTicksRef {
added: ticks.added.deref(),
@@ -813,7 +717,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
last_run: system_meta.last_run,
this_run: change_tick,
},
}
})
}
}
@@ -851,42 +755,17 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
panic!("error[B0002]: ResMut<{}> in system {} conflicts with a previous system parameter. Consider removing the duplicate access or using `Without<IsResource>` to create disjoint Queries or merging conflicting Queries into a `ParamSet`. See: https://bevy.org/learn/errors/b0002", DebugName::type_name::<T>(), system_meta.name);
}
#[inline]
unsafe fn validate_param(
&mut component_id: &mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to the resource.
if let Some(entity) = unsafe { world.resource_entities() }.get(component_id)
&& let Ok(entity_ref) = world.get_entity(*entity)
&& entity_ref.contains_id(component_id)
{
Ok(())
} else {
Err(SystemParamValidationError::invalid::<Self>(
"Resource does not exist",
))
}
}
#[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> {
let value = world
.get_resource_mut_by_id(component_id)
.unwrap_or_else(|| {
panic!(
"Resource requested by {} does not exist: {}",
system_meta.name,
DebugName::type_name::<T>()
);
});
ResMut {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
let value = world.get_resource_mut_by_id(component_id).ok_or_else(|| {
SystemParamValidationError::invalid::<Self>("Resource does not exist")
})?;
Ok(ResMut {
value: value.value.deref_mut::<T>(),
ticks: ComponentTicksMut {
added: value.ticks.added,
@@ -895,7 +774,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
last_run: system_meta.last_run,
this_run: change_tick,
},
}
})
}
}
@@ -933,9 +812,9 @@ unsafe impl SystemParam for &'_ World {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: Read-only access to the entire world was registered in `init_state`.
unsafe { world.world() }
Ok(unsafe { world.world() })
}
}
@@ -965,9 +844,9 @@ unsafe impl<'w> SystemParam for DeferredWorld<'w> {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
_change_tick: Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { world.into_deferred() }
Ok(unsafe { world.into_deferred() })
}
}
@@ -1156,8 +1035,8 @@ unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> {
_system_meta: &SystemMeta,
_world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
Local(state.get())
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(Local(state.get()))
}
}
@@ -1360,8 +1239,8 @@ unsafe impl<T: SystemBuffer> SystemParam for Deferred<'_, T> {
_system_meta: &SystemMeta,
_world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
Deferred(state.get())
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(Deferred(state.get()))
}
}
@@ -1391,8 +1270,8 @@ unsafe impl SystemParam for ExclusiveMarker {
_system_meta: &SystemMeta,
_world: UnsafeWorldCell<'world>,
_change_tick: Tick,
) -> Self::Item<'world, 'state> {
Self(PhantomData)
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
Ok(Self(PhantomData))
}
}
@@ -1425,8 +1304,8 @@ unsafe impl SystemParam for NonSendMarker {
_system_meta: &SystemMeta,
_world: UnsafeWorldCell<'world>,
_change_tick: Tick,
) -> Self::Item<'world, 'state> {
Self(PhantomData)
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
Ok(Self(PhantomData))
}
}
@@ -1464,46 +1343,20 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> {
component_access_set.add_unfiltered_component_read(component_id);
}
#[inline]
unsafe fn validate_param(
&mut component_id: &mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to non-send metadata.
if unsafe { world.storages() }
.non_sends
.get(component_id)
.is_some_and(NonSendData::is_present)
{
Ok(())
} else {
Err(SystemParamValidationError::invalid::<Self>(
"Non-send resource does not exist",
))
}
}
#[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> {
let (ptr, ticks) = world
.get_non_send_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
"Non-send resource requested by {} does not exist: {}",
system_meta.name,
DebugName::type_name::<T>()
);
});
NonSend {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| {
SystemParamValidationError::invalid::<Self>("Non-send data not found")
})?;
Ok(NonSend {
value: ptr.deref(),
ticks: ComponentTicksRef::from_tick_cells(ticks, system_meta.last_run, change_tick),
}
})
}
}
@@ -1538,46 +1391,20 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
component_access_set.add_unfiltered_component_write(component_id);
}
#[inline]
unsafe fn validate_param(
&mut component_id: &mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to non-send metadata.
if unsafe { world.storages() }
.non_sends
.get(component_id)
.is_some_and(NonSendData::is_present)
{
Ok(())
} else {
Err(SystemParamValidationError::invalid::<Self>(
"Non-send resource does not exist",
))
}
}
#[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> {
let (ptr, ticks) = world
.get_non_send_with_ticks(component_id)
.unwrap_or_else(|| {
panic!(
"Non-send resource requested by {} does not exist: {}",
system_meta.name,
DebugName::type_name::<T>()
);
});
NonSendMut {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
let (ptr, ticks) = world.get_non_send_with_ticks(component_id).ok_or_else(|| {
SystemParamValidationError::invalid::<Self>("Non-send data not found")
})?;
Ok(NonSendMut {
value: ptr.assert_unique().deref_mut(),
ticks: ComponentTicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick),
}
})
}
}
@@ -1605,8 +1432,8 @@ unsafe impl<'a> SystemParam for &'a Archetypes {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.archetypes()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.archetypes())
}
}
@@ -1634,8 +1461,8 @@ unsafe impl<'a> SystemParam for &'a Components {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.components()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.components())
}
}
@@ -1663,8 +1490,8 @@ unsafe impl<'a> SystemParam for &'a Entities {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.entities()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.entities())
}
}
@@ -1692,8 +1519,8 @@ unsafe impl<'a> SystemParam for &'a EntityAllocator {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.entity_allocator()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.entity_allocator())
}
}
@@ -1721,8 +1548,8 @@ unsafe impl<'a> SystemParam for &'a Bundles {
_system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Self::Item<'w, 's> {
world.bundles()
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(world.bundles())
}
}
@@ -1779,11 +1606,11 @@ unsafe impl SystemParam for SystemChangeTick {
system_meta: &SystemMeta,
_world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
SystemChangeTick {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
Ok(SystemChangeTick {
last_run: system_meta.last_run,
this_run: change_tick,
}
})
}
}
@@ -1812,13 +1639,9 @@ unsafe impl<T: SystemParam> SystemParam for Option<T> {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe {
T::validate_param(state, system_meta, world)
.ok()
.map(|()| T::get_param(state, system_meta, world, change_tick))
}
Ok(unsafe { T::get_param(state, system_meta, world, change_tick) }.ok())
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
@@ -1858,12 +1681,9 @@ unsafe impl<T: SystemParam> SystemParam for Result<T, SystemParamValidationError
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe {
T::validate_param(state, system_meta, world)
.map(|()| T::get_param(state, system_meta, world, change_tick))
}
Ok(unsafe { T::get_param(state, system_meta, world, change_tick) })
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
@@ -1950,28 +1770,20 @@ unsafe impl<T: SystemParam> SystemParam for If<T> {
T::init_access(state, system_meta, component_access_set, world);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { T::validate_param(state, system_meta, world) }.map_err(|mut e| {
e.skipped = true;
e
})
}
#[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> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: Upheld by caller.
If(unsafe { T::get_param(state, system_meta, world, change_tick) })
unsafe { T::get_param(state, system_meta, world, change_tick) }
.map(If)
.map_err(|mut e| {
e.skipped = true;
e
})
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
@@ -2008,26 +1820,13 @@ unsafe impl<T: SystemParam> SystemParam for Vec<T> {
}
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
for state in state {
// SAFETY: Upheld by caller
unsafe { T::validate_param(state, system_meta, world)? };
}
Ok(())
}
#[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> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
state
.iter_mut()
// SAFETY:
@@ -2088,13 +1887,23 @@ unsafe impl<T: SystemParam> SystemParam for ParamSet<'_, '_, Vec<T>> {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
ParamSet {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// Validate each sub-param eagerly so that the system is correctly
// skipped by the executor when any sub-param is unavailable.
// PERF: the sub-params will be fetched again lazily when accessed through
// the ParamSet, but this is no worse than the previous
// validate_param + get_param pattern.
for s in state.iter_mut() {
// SAFETY: Upheld by caller.
drop(unsafe { T::get_param(s, system_meta, world, change_tick) }?);
}
Ok(ParamSet {
param_states: state,
system_meta: system_meta.clone(),
world,
change_tick,
}
})
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
@@ -2125,6 +1934,7 @@ impl<T: SystemParam> ParamSet<'_, '_, Vec<T>> {
self.world,
self.change_tick,
)
.unwrap()
}
}
@@ -2136,7 +1946,8 @@ impl<T: SystemParam> ParamSet<'_, '_, Vec<T>> {
// - We initialized the access for each parameter, so the caller ensures we have access to any world data needed by any param.
// We have mutable access to the ParamSet, so no other params in the set are active.
// - The caller of `get_param` ensured that this was the world used to initialize our state, and we used that world to initialize parameter states
unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) },
unsafe { T::get_param(state, &self.system_meta, self.world, self.change_tick) }
.unwrap_or_else(|err| panic!("ParamSet parameter validation failed: {err}")),
);
});
}
@@ -2192,27 +2003,6 @@ macro_rules! impl_system_param_tuple {
$($param::queue($param, system_meta, world.reborrow());)*
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
let ($($param,)*) = state;
#[allow(
unused_unsafe,
reason = "Zero-length tuples won't have any params to validate."
)]
// SAFETY: Upheld by caller
unsafe {
$(
$param::validate_param($param, system_meta, world)?;
)*
}
Ok(())
}
#[inline]
#[track_caller]
unsafe fn get_param<'w, 's>(
@@ -2220,7 +2010,7 @@ macro_rules! impl_system_param_tuple {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
let ($($param,)*) = state;
#[allow(
@@ -2233,7 +2023,7 @@ macro_rules! impl_system_param_tuple {
clippy::unused_unit,
reason = "Zero-length tuples won't have any params to get."
)]
($($param::get_param($param, system_meta, world, change_tick),)*)
Ok(($($param::get_param($param, system_meta, world, change_tick)?,)*))
}
}
}
@@ -2381,25 +2171,15 @@ unsafe impl<P: SystemParam + 'static> SystemParam for StaticSystemParam<'_, '_,
P::queue(state, system_meta, world);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { P::validate_param(state, system_meta, world) }
}
#[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> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: Defer to the safety of P::SystemParam
StaticSystemParam(unsafe { P::get_param(state, system_meta, world, change_tick) })
unsafe { P::get_param(state, system_meta, world, change_tick) }.map(StaticSystemParam)
}
}
@@ -2424,8 +2204,8 @@ unsafe impl<T: ?Sized> SystemParam for PhantomData<T> {
_system_meta: &SystemMeta,
_world: UnsafeWorldCell<'world>,
_change_tick: Tick,
) -> Self::Item<'world, 'state> {
PhantomData
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
Ok(PhantomData)
}
}
@@ -2599,12 +2379,12 @@ where
{
state
.downcast_mut::<ParamState<T::Item<'static, 'static>>>()
.map(|state| {
.and_then(|state| {
// SAFETY:
// - The caller ensures the world has access for the underlying system param,
// and since the downcast succeeded, the underlying system param is T.
// - The caller ensures the `world` matches.
unsafe { T::Item::get_param(&mut state.0, system_meta, world, change_tick) }
unsafe { T::Item::get_param(&mut state.0, system_meta, world, change_tick) }.ok()
})
}
@@ -2636,14 +2416,15 @@ trait DynParamState: Sync + Send + Any {
world: &mut World,
);
/// Refer to [`SystemParam::validate_param`].
/// Validates the inner parameter by calling [`SystemParam::get_param`] and discarding the value.
///
/// # Safety
/// Refer to [`SystemParam::validate_param`].
unsafe fn validate_param(
/// Refer to [`SystemParam::get_param`].
unsafe fn validate(
&mut self,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
change_tick: Tick,
) -> Result<(), SystemParamValidationError>;
}
@@ -2668,13 +2449,14 @@ impl<T: SystemParam + 'static> DynParamState for ParamState<T> {
T::init_access(&self.0, system_meta, component_access_set, world);
}
unsafe fn validate_param(
unsafe fn validate(
&mut self,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
change_tick: Tick,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller
unsafe { T::validate_param(&mut self.0, system_meta, world) }
// SAFETY: Upheld by caller.
unsafe { T::get_param(&mut self.0, system_meta, world, change_tick) }.map(drop)
}
}
@@ -2699,29 +2481,25 @@ unsafe impl SystemParam for DynSystemParam<'_, '_> {
.init_access(system_meta, component_access_set, world);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Upheld by caller.
unsafe { state.0.validate_param(system_meta, world) }
}
#[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> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// Validate the inner parameter eagerly so that systems using DynSystemParam
// are correctly skipped by the executor when the inner param is unavailable.
// SAFETY: Upheld by caller.
unsafe { state.0.validate(system_meta, world, change_tick) }?;
// SAFETY:
// - `state.0` is a boxed `ParamState<T>`.
// - `init_access` calls `DynParamState::init_access`, which calls `init_access` on the inner parameter,
// so the caller ensures the world has the necessary access.
// - The caller ensures that the provided world is the same and has the required access.
unsafe { DynSystemParam::new(state.0.as_mut(), world, system_meta.clone(), change_tick) }
Ok(unsafe {
DynSystemParam::new(state.0.as_mut(), world, system_meta.clone(), change_tick)
})
}
fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {
@@ -2769,10 +2547,10 @@ unsafe impl SystemParam for FilteredResources<'_, '_> {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: The caller ensures that `world` has access to anything registered in `init_access`,
// and we registered all resource access in `state``.
unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) }
Ok(unsafe { FilteredResources::new(world, state, system_meta.last_run, change_tick) })
}
}
@@ -2815,17 +2593,17 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> {
system_meta: &SystemMeta,
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
// SAFETY: The caller ensures that `world` has access to anything registered in `init_access`,
// and we registered all resource access in `state``.
unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) }
Ok(unsafe { FilteredResourcesMut::new(world, state, system_meta.last_run, change_tick) })
}
}
/// An error that occurs when a system parameter is not valid,
/// used by system executors to determine what to do with a system.
///
/// Returned as an error from [`SystemParam::validate_param`],
/// Returned as an error from [`SystemParam::get_param`],
/// and handled using the unified error handling mechanisms defined in [`bevy_ecs::error`].
#[derive(Debug, PartialEq, Eq, Clone, Error)]
pub struct SystemParamValidationError {
crates/bevy_ecs/src/system/system_registry.rs
+1 -1
View File
@@ -1015,7 +1015,7 @@ mod tests {
let result = world.run_system(id);
assert!(matches!(result, Err(RegisteredSystemError::Failed { .. })));
let expected = "Resource does not exist";
let expected = "does not exist";
let actual = result.unwrap_err().to_string();
assert!(
crates/bevy_ecs/src/world/identifier.rs
+11 -5
View File
@@ -2,7 +2,10 @@ use crate::{
change_detection::Tick,
query::FilteredAccessSet,
storage::SparseSetIndex,
system::{ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam},
system::{
ExclusiveSystemParam, ReadOnlySystemParam, SystemMeta, SystemParam,
SystemParamValidationError,
},
world::{FromWorld, World},
};
use bevy_platform::sync::atomic::{AtomicUsize, Ordering};
@@ -70,8 +73,8 @@ unsafe impl SystemParam for WorldId {
_: &SystemMeta,
world: UnsafeWorldCell<'world>,
_: Tick,
) -> Self::Item<'world, 'state> {
world.id()
) -> Result<Self::Item<'world, 'state>, SystemParamValidationError> {
Ok(world.id())
}
}
@@ -83,8 +86,11 @@ impl ExclusiveSystemParam for WorldId {
world.id()
}
fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> {
*state
fn get_param<'s>(
state: &'s mut Self::State,
_system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(*state)
}
}
crates/bevy_gizmos/src/gizmos.rs
+12 -36
View File
@@ -228,44 +228,13 @@ where
GizmosState::<Config, Clear>::queue(&mut state.state, system_meta, world);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Delegated to existing `SystemParam` implementation.
unsafe {
GizmosState::<Config, Clear>::validate_param(&mut state.state, system_meta, world)?;
}
// SAFETY: Delegated to existing `SystemParam` implementation.
let (_, f1) = unsafe {
GizmosState::<Config, Clear>::get_param(
&mut state.state,
system_meta,
world,
world.change_tick(),
)
};
// This if-block is to accommodate an Option<Gizmos> SystemParam.
// The user may decide not to initialize a gizmo group, so its config will not exist.
if f1.get_config::<Config>().is_none() {
Err(SystemParamValidationError::invalid::<Self>(
format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group<T>()`?",
Config::type_path())))
} else {
Ok(())
}
}
#[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> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: Delegated to existing `SystemParam` implementations.
let (mut f0, f1) = unsafe {
GizmosState::<Config, Clear>::get_param(
@@ -273,20 +242,27 @@ where
system_meta,
world,
change_tick,
)
)?
};
// Accessing the GizmoConfigStore in every API call reduces performance significantly.
// Implementing SystemParam manually allows us to cache whether the config is currently enabled.
// Having this available allows for cheap early returns when gizmos are disabled.
let (config, config_ext) = f1.into_inner().config::<Config>();
//
// We use `get_config` instead of `config` to accommodate `Option<Gizmos>`:
// the user may decide not to initialize a gizmo group, so its config will not exist.
let (config, config_ext) = f1.into_inner().get_config::<Config>().ok_or_else(|| {
SystemParamValidationError::invalid::<Self>(
format!("Requested config {} does not exist in `GizmoConfigStore`! Did you forget to add it using `app.init_gizmo_group<T>()`?",
Config::type_path()))
})?;
f0.enabled = config.enabled;
Gizmos {
Ok(Gizmos {
buffer: f0,
config,
config_ext,
}
})
}
}
crates/bevy_input_focus/src/tab_navigation.rs
+2 -2
View File
@@ -478,7 +478,7 @@ mod tests {
let tab_entity_2 = world.spawn((TabIndex(1), ChildOf(tab_group_entity))).id();
let mut system_state: SystemState<TabNavigation> = SystemState::new(world);
let tab_navigation = system_state.get(world);
let tab_navigation = system_state.get(world).unwrap();
assert_eq!(tab_navigation.tabgroup_query.iter().count(), 1);
assert!(tab_navigation.tabindex_query.iter().count() >= 2);
@@ -511,7 +511,7 @@ mod tests {
let tab_entity_4 = world.spawn((TabIndex(1), ChildOf(tab_group_2))).id();
let mut system_state: SystemState<TabNavigation> = SystemState::new(world);
let tab_navigation = system_state.get(world);
let tab_navigation = system_state.get(world).unwrap();
assert_eq!(tab_navigation.tabgroup_query.iter().count(), 2);
assert!(tab_navigation.tabindex_query.iter().count() >= 4);
crates/bevy_pbr/src/material.rs
+1 -1
View File
@@ -929,7 +929,7 @@ pub(crate) fn specialize_material_meshes(
mut specialized_material_pipeline_cache,
mut pending_mesh_material_queues,
dirty_specializations,
} = state.get_mut(world);
} = state.get_mut(world).unwrap();
for (view, visible_entities) in &views {
all_views.insert(view.retained_view_entity);
crates/bevy_pbr/src/meshlet/instance_manager.rs
+1 -1
View File
@@ -220,7 +220,7 @@ pub fn extract_meshlet_mesh_entities(
}
let system_state = system_state.as_mut().unwrap();
let (instances_query, asset_server, mut assets, mut asset_events) =
system_state.get_mut(&mut main_world);
system_state.get_mut(&mut main_world).unwrap();
// Reset per-frame data
instance_manager.reset(render_entities);
crates/bevy_pbr/src/prepass/mod.rs
+1 -1
View File
@@ -909,7 +909,7 @@ pub(crate) fn specialize_prepass_material_meshes(
mut pending_prepass_mesh_material_queues,
dirty_specializations,
this_run: system_change_tick,
} = state.get_mut(world);
} = state.get_mut(world).unwrap();
this_run = system_change_tick.this_run();
crates/bevy_pbr/src/render/light.rs
+1 -1
View File
@@ -1942,7 +1942,7 @@ pub(crate) fn specialize_shadows(
mut specialized_shadow_material_pipeline_cache,
mut pending_shadow_queues,
dirty_specializations,
} = state.get_mut(world);
} = state.get_mut(world).unwrap();
for (entity, view_lights) in &view_lights {
for view_light_entity in view_lights.lights.iter().copied() {
crates/bevy_render/src/erased_render_asset.rs
+1 -1
View File
@@ -248,7 +248,7 @@ pub(crate) fn extract_erased_render_asset<A: ErasedRenderAsset>(
) {
main_world.resource_scope(
|world, mut cached_state: Mut<CachedExtractErasedRenderAssetSystemState<A>>| {
let (mut events, mut assets) = cached_state.state.get_mut(world);
let (mut events, mut assets) = cached_state.state.get_mut(world).unwrap();
let mut needs_extracting = <HashSet<_>>::default();
let mut removed = <HashSet<_>>::default();
crates/bevy_render/src/extract_param.rs
+4 -28
View File
@@ -94,37 +94,13 @@ where
);
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// SAFETY: Read-only access to world data registered in `init_state`.
let result = unsafe { world.get_resource_by_id(state.main_world_state) };
let Some(main_world) = result else {
return Err(SystemParamValidationError::invalid::<Self>(
"`MainWorld` resource does not exist",
));
};
// SAFETY: Type is guaranteed by `SystemState`.
let main_world: &World = unsafe { main_world.deref() };
// SAFETY: We provide the main world on which this system state was initialized on.
unsafe {
SystemState::<P>::validate_param(
&mut state.state,
main_world.as_unsafe_world_cell_readonly(),
)
}
}
#[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> {
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY:
// - The caller ensures that `world` is the same one that `init_state` was called with.
// - The caller ensures that no other `SystemParam`s will conflict with the accesses we have registered.
@@ -134,10 +110,10 @@ where
system_meta,
world,
change_tick,
)
)?
};
let item = state.state.get(main_world.into_inner());
Extract { item }
let item = state.state.get(main_world.into_inner())?;
Ok(Extract { item })
}
}
crates/bevy_render/src/render_asset.rs
+1 -1
View File
@@ -262,7 +262,7 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
) {
main_world.resource_scope(
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
let (mut events, mut assets, maybe_render_assets) = cached_state.state.get_mut(world);
let (mut events, mut assets, maybe_render_assets) = cached_state.state.get_mut(world).unwrap();
let mut needs_extracting = <HashSet<_>>::default();
let mut removed = <HashSet<_>>::default();
crates/bevy_render/src/render_phase/draw.rs
+1 -1
View File
@@ -326,7 +326,7 @@ where
view: Entity,
item: &P,
) -> Result<(), DrawError> {
let param = self.state.get(world);
let param = self.state.get(world).unwrap();
let view = match self.view.get_manual(world, view) {
Ok(view) => view,
Err(err) => match err {
crates/bevy_render/src/renderer/mod.rs
+1 -1
View File
@@ -89,7 +89,7 @@ pub fn render_system(
let _span = info_span!("present_frames").entered();
world.resource_scope(|world, mut windows: Mut<ExtractedWindows>| {
let views = state.get(world);
let views = state.get(world).unwrap();
for window in windows.values_mut() {
let view_needs_present = views.iter().any(|(view_target, camera)| {
matches!(
crates/bevy_render/src/renderer/render_context.rs
+10 -41
View File
@@ -260,11 +260,12 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
}
#[inline]
unsafe fn validate_param(
state: &mut Self::State,
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
_system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
world: UnsafeWorldCell<'w>,
_change_tick: Tick,
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// SAFETY: We have registered resource read access in init_access
let current_view = unsafe { world.get_resource::<CurrentView>() };
@@ -278,47 +279,15 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
// SAFETY: Query state access is properly registered in init_access.
// The caller ensures the world matches the one used in init_state.
let result = unsafe { state.query_state.get_unchecked(world, entity) };
let item = unsafe { state.query_state.get_unchecked(world, entity) }.map_err(|_| {
SystemParamValidationError::skipped::<Self>("Current view entity does not match query")
})?;
if result.is_err() {
return Err(SystemParamValidationError::skipped::<Self>(
"Current view entity does not match query",
));
}
Ok(())
}
#[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: We have registered resource read access and validate_param succeeded
let current_view = unsafe {
world
.get_resource::<CurrentView>()
.expect("CurrentView must exist")
};
let entity = current_view.entity();
// SAFETY: Query state access is properly registered in init_access.
// validate_param verified the entity matches.
let item = unsafe {
state
.query_state
.get_unchecked(world, entity)
.expect("view entity must match query")
};
ViewQuery {
Ok(ViewQuery {
entity,
item,
_filter: PhantomData,
}
})
}
}
crates/bevy_render/src/sync_world.rs
+1 -1
View File
@@ -253,7 +253,7 @@ pub(crate) fn despawn_temporary_render_entities(
state: &mut SystemState<Query<Entity, With<TemporaryRenderEntity>>>,
mut local: Local<Vec<Entity>>,
) {
let query = state.get(world);
let query = state.get(world).unwrap();
local.extend(query.iter());
crates/bevy_render/src/view/window/screenshot.rs
+2 -1
View File
@@ -229,7 +229,8 @@ fn extract_screenshots(
*system_state = Some(SystemState::new(&mut main_world));
}
let system_state = system_state.as_mut().unwrap();
let (mut commands, primary_window, screenshots) = system_state.get_mut(&mut main_world);
let (mut commands, primary_window, screenshots) =
system_state.get_mut(&mut main_world).unwrap();
targets.clear();
seen_targets.clear();
crates/bevy_transform/src/helper.rs
+1 -1
View File
@@ -138,7 +138,7 @@ mod tests {
let transform = *app.world().get::<GlobalTransform>(leaf_entity).unwrap();
let mut state = SystemState::<TransformHelper>::new(app.world_mut());
let helper = state.get(app.world());
let helper = state.get(app.world()).unwrap();
let computed_transform = helper.compute_global_transform(leaf_entity).unwrap();
crates/bevy_ui/src/experimental/ghost_hierarchy.rs
+2 -2
View File
@@ -230,7 +230,7 @@ mod tests {
});
let mut system_state = SystemState::<(UiRootNodes, Query<&A>)>::new(world);
let (ui_root_nodes, a_query) = system_state.get(world);
let (ui_root_nodes, a_query) = system_state.get(world).unwrap();
let result: Vec<_> = a_query.iter_many(ui_root_nodes.iter()).collect();
@@ -263,7 +263,7 @@ mod tests {
world.entity_mut(n9).add_children(&[n10]);
let mut system_state = SystemState::<(UiChildren, Query<&A>)>::new(world);
let (ui_children, a_query) = system_state.get(world);
let (ui_children, a_query) = system_state.get(world).unwrap();
let result: Vec<_> = a_query
.iter_many(ui_children.iter_ui_children(n1))
crates/bevy_winit/src/cursor/mod.rs
+2 -2
View File
@@ -67,13 +67,13 @@ impl WinitAppRunnerState {
Query<(Entity, &mut PendingCursor), Changed<PendingCursor>>,
)> = SystemState::new(self.world_mut());
#[cfg(feature = "custom_cursor")]
let (mut cursor_cache, mut windows) = windows_state.get_mut(self.world_mut());
let (mut cursor_cache, mut windows) = windows_state.get_mut(self.world_mut()).unwrap();
#[cfg(not(feature = "custom_cursor"))]
let mut windows_state: SystemState<(
Query<(Entity, &mut PendingCursor), Changed<PendingCursor>>,
)> = SystemState::new(self.world_mut());
#[cfg(not(feature = "custom_cursor"))]
let (mut windows,) = windows_state.get_mut(self.world_mut());
let (mut windows,) = windows_state.get_mut(self.world_mut()).unwrap();
WINIT_WINDOWS.with_borrow(|winit_windows| {
for (entity, mut pending_cursor) in windows.iter_mut() {
crates/bevy_winit/src/state.rs
+12 -8
View File
@@ -181,7 +181,7 @@ impl ApplicationHandler<WinitUserEvent> for WinitAppRunnerState {
// Create the initial window if needed
let mut create_window = SystemState::<CreateWindowParams>::from_world(self.world_mut());
create_windows(event_loop, create_window.get_mut(self.world_mut()));
create_windows(event_loop, create_window.get_mut(self.world_mut()).unwrap());
create_window.apply(self.world_mut());
}
@@ -195,7 +195,7 @@ impl ApplicationHandler<WinitUserEvent> for WinitAppRunnerState {
WinitUserEvent::WindowAdded => {
let mut create_window =
SystemState::<CreateWindowParams>::from_world(self.world_mut());
create_windows(event_loop, create_window.get_mut(self.world_mut()));
create_windows(event_loop, create_window.get_mut(self.world_mut()).unwrap());
create_window.apply(self.world_mut());
}
}
@@ -224,7 +224,8 @@ impl ApplicationHandler<WinitUserEvent> for WinitAppRunnerState {
mut windows,
) = self
.message_writer_system_state
.get_mut(self.app.world_mut());
.get_mut(self.app.world_mut())
.unwrap();
let Some(window) = winit_windows.get_window_entity(window_id) else {
warn!("Skipped event {event:?} for unknown winit Window Id {window_id:?}");
@@ -459,7 +460,10 @@ impl ApplicationHandler<WinitUserEvent> for WinitAppRunnerState {
fn about_to_wait(&mut self, event_loop: &ActiveEventLoop) {
let mut create_monitor = SystemState::<CreateMonitorParams>::from_world(self.world_mut());
create_monitors(event_loop, create_monitor.get_mut(self.world_mut()));
create_monitors(
event_loop,
create_monitor.get_mut(self.world_mut()).unwrap(),
);
create_monitor.apply(self.world_mut());
// TODO: This is a workaround for https://github.com/bevyengine/bevy/issues/17488
@@ -516,7 +520,7 @@ impl WinitAppRunnerState {
let mut focused_windows_state: SystemState<(Res<WinitSettings>, Query<(Entity, &Window)>)> =
SystemState::new(self.world_mut());
let (config, windows) = focused_windows_state.get(self.world());
let (config, windows) = focused_windows_state.get(self.world()).unwrap();
let focused = windows.iter().any(|(_, window)| window.focused);
let mut update_mode = config.update_mode(focused);
@@ -572,7 +576,7 @@ impl WinitAppRunnerState {
SystemState::<CreateWindowParams>::from_world(self.world_mut());
let (.., mut handlers, accessibility_requested, monitors) =
create_window.get_mut(self.world_mut());
create_window.get_mut(self.world_mut()).unwrap();
let winit_window = winit_windows.create_window(
event_loop,
@@ -605,7 +609,7 @@ impl WinitAppRunnerState {
let begin_frame_time = Instant::now();
if should_update {
let (_, windows) = focused_windows_state.get(self.world());
let (_, windows) = focused_windows_state.get(self.world()).unwrap();
// If no windows exist, this will evaluate to `true`.
let all_invisible = windows.iter().all(|w| !w.1.visible);
@@ -648,7 +652,7 @@ impl WinitAppRunnerState {
}
// Running the app may have changed the WinitSettings resource, so we have to re-extract it.
let (config, windows) = focused_windows_state.get(self.world());
let (config, windows) = focused_windows_state.get(self.world()).unwrap();
let focused = windows.iter().any(|(_, window)| window.focused);
update_mode = config.update_mode(focused);
}
examples/async_tasks/async_compute.rs
+1 -1
View File
@@ -91,7 +91,7 @@ fn spawn_tasks(mut commands: Commands) {
Res<BoxMaterialHandle>,
)>::new(world);
let (box_mesh_handle, box_material_handle) =
system_state.get_mut(world);
system_state.get_mut(world).unwrap();
(box_mesh_handle.clone(), box_material_handle.clone())
};
examples/ecs/fallible_params.rs
+2 -2
View File
@@ -10,14 +10,14 @@
//! Other system parameters, such as [`Query`], will never fail validation: returning a query with no matching entities is valid.
//!
//! The result of failed system parameter validation is determined by the [`SystemParamValidationError`] returned
//! by [`SystemParam::validate_param`] for each system parameter.
//! by [`SystemParam::get_param`] for each system parameter.
//! Each system will pass if all of its parameters are valid, or else return [`SystemParamValidationError`] for the first failing parameter.
//!
//! To learn more about setting the fallback behavior for [`SystemParamValidationError`] failures,
//! please see the `error_handling.rs` example.
//!
//! [`SystemParamValidationError`]: bevy::ecs::system::SystemParamValidationError
//! [`SystemParam::validate_param`]: bevy::ecs::system::SystemParam::validate_param
//! [`SystemParam::get_param`]: bevy::ecs::system::SystemParam::get_param
use bevy::ecs::error::warn;
use bevy::prelude::*;
release-content/migration-guides/validation_merging.md
+129
View File
@@ -0,0 +1,129 @@
---
title: "`SystemParam` validation is now done when fetching the data"
pull_requests: [23225]
---
In an effort to improve performance by reducing redundant data fetches and simplify internals,
system parameter validation is now done as part of fetching the data for those system parameters.
To be more precise:
- `SystemParam::get_param` now returns a `Result<Self::Item<'world, 'state>, SystemParamValidationError>`, instead of simply a `Self::Item<'world, 'state>`
- If validation fails, an appropriate `SystemParamValidationError` should be returned
- If validation passes, the item should be returned wrapped in `Ok`
- `SystemParam::validate_param` has been removed
- All logic that was done in this method should be moved to the `get_param` method of that type
- `SystemState::validate_param` has been removed
- Validation now happens automatically when calling `get`, `get_mut`, or `get_unchecked`
- `SystemState::fetch`, `get_unchecked`, `get` and `get_mut` now return a `Result<..., SystemParamValidationError>`. Callers that previously destructured the result directly will need to add `.unwrap()` or handle the `Result`:
```rust
// Before
let (res, query) = system_state.get(&world);
// After
let (res, query) = system_state.get(&world).unwrap();
```
When executing systems, we no longer check for system validation before running the systems.
As a result of these changes, `System::validate_param` and `System::validate_param_unsafe` have been removed.
Instead, validation has been moved to be part of the trait implementation for `System::run_unsafe`.
All implementations of the `System` trait should validate that their parameters are valid during this method,
bubbling up any errors originating in `SystemParam::get_param`.
## Custom `SystemParam` implementations
If you have a custom `SystemParam` implementation, you need to:
1. Remove the `validate_param` method.
2. Move any validation logic into `get_param`.
3. Change `get_param` to return `Result<Self::Item<'world, 'state>, SystemParamValidationError>`.
```rust
// Before
unsafe impl SystemParam for MyParam<'_> {
// ...
unsafe fn validate_param(
state: &Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell,
) -> Result<(), SystemParamValidationError> {
// validation logic
if !is_valid(state, world) {
return Err(SystemParamValidationError::invalid::<Self>("not valid"));
}
Ok(())
}
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Self::Item<'w, 's> {
// fetch logic
MyParam { /* ... */ }
}
}
// After
unsafe impl SystemParam for MyParam<'_> {
// ...
unsafe fn get_param<'w, 's>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
world: UnsafeWorldCell<'w>,
change_tick: Tick,
) -> Result<Self::Item<'w, 's>, SystemParamValidationError> {
// validation logic merged into get_param
if !is_valid(state, world) {
return Err(SystemParamValidationError::invalid::<Self>("not valid"));
}
// fetch logic
Ok(MyParam { /* ... */ })
}
}
```
## Custom `ExclusiveSystemParam` implementations
Similarly, `ExclusiveSystemParam::get_param` now returns a `Result<Self::Item<'s>, SystemParamValidationError>` instead of `Self::Item<'s>`.
Existing implementations should wrap their return value in `Ok(...)` and return an appropriate `SystemParamValidationError` if validation fails.
```rust
// Before
impl ExclusiveSystemParam for MyExclusiveParam {
// ...
fn get_param<'s>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
) -> Self::Item<'s> {
MyExclusiveParam { /* ... */ }
}
}
// After
impl ExclusiveSystemParam for MyExclusiveParam {
// ...
fn get_param<'s>(
state: &'s mut Self::State,
system_meta: &SystemMeta,
) -> Result<Self::Item<'s>, SystemParamValidationError> {
Ok(MyExclusiveParam { /* ... */ })
}
}
```
## Custom `System` implementations
If you have a custom `System` implementation, remove the `validate_param_unsafe` method. Parameter validation should now occur inside `run_unsafe` by propagating errors from `SystemParam::get_param`.
## `MultithreadedExecutor` performance changes
For the parallel `MultithreadedExecutor`, validation was previously done as a cheap pre-validation step,
while checking run conditions.
Now, tasks will be spawned for systems which would fail or are skipped during validation.
In most cases, avoiding the extra overhead of looking up the required data twice should dominate.
However, this change may negatively affect systems which are frequently skipped (e.g. due to `Single`).
If you find that this is a significant performance overhead for your use case,
the previous behavior can be recovered by adding run conditions.