(
- "`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::::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, 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 })
}
}
diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs
index 8d90e5b5200b4..43ef2bb5807b5 100644
--- a/crates/bevy_render/src/render_asset.rs
+++ b/crates/bevy_render/src/render_asset.rs
@@ -262,7 +262,7 @@ pub(crate) fn extract_render_asset(
) {
main_world.resource_scope(
|world, mut cached_state: Mut>| {
- 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 = >::default();
let mut removed = >::default();
diff --git a/crates/bevy_render/src/render_phase/draw.rs b/crates/bevy_render/src/render_phase/draw.rs
index 319c907843f68..13d4f67da52eb 100644
--- a/crates/bevy_render/src/render_phase/draw.rs
+++ b/crates/bevy_render/src/render_phase/draw.rs
@@ -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 {
diff --git a/crates/bevy_render/src/renderer/mod.rs b/crates/bevy_render/src/renderer/mod.rs
index d19d29f0eb08a..38bb7f1ccc674 100644
--- a/crates/bevy_render/src/renderer/mod.rs
+++ b/crates/bevy_render/src/renderer/mod.rs
@@ -89,7 +89,7 @@ pub fn render_system(
let _span = info_span!("present_frames").entered();
world.resource_scope(|world, mut windows: Mut| {
- 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!(
diff --git a/crates/bevy_render/src/renderer/render_context.rs b/crates/bevy_render/src/renderer/render_context.rs
index 5a21ada6f1fb2..0fa4ac1eb67f0 100644
--- a/crates/bevy_render/src/renderer/render_context.rs
+++ b/crates/bevy_render/src/renderer/render_context.rs
@@ -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, SystemParamValidationError> {
// SAFETY: We have registered resource read access in init_access
let current_view = unsafe { world.get_resource::() };
@@ -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::("Current view entity does not match query")
+ })?;
- if result.is_err() {
- return Err(SystemParamValidationError::skipped::(
- "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::()
- .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,
- }
+ })
}
}
diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs
index c98afc2c57bf7..cf92577e53326 100644
--- a/crates/bevy_render/src/sync_world.rs
+++ b/crates/bevy_render/src/sync_world.rs
@@ -253,7 +253,7 @@ pub(crate) fn despawn_temporary_render_entities(
state: &mut SystemState>>,
mut local: Local>,
) {
- let query = state.get(world);
+ let query = state.get(world).unwrap();
local.extend(query.iter());
diff --git a/crates/bevy_render/src/view/window/screenshot.rs b/crates/bevy_render/src/view/window/screenshot.rs
index ae2a5f9efaaea..025aac44ac58f 100644
--- a/crates/bevy_render/src/view/window/screenshot.rs
+++ b/crates/bevy_render/src/view/window/screenshot.rs
@@ -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();
diff --git a/crates/bevy_transform/src/helper.rs b/crates/bevy_transform/src/helper.rs
index 1ab33b4b421bc..f0dcfc58b85bd 100644
--- a/crates/bevy_transform/src/helper.rs
+++ b/crates/bevy_transform/src/helper.rs
@@ -138,7 +138,7 @@ mod tests {
let transform = *app.world().get::(leaf_entity).unwrap();
let mut state = SystemState::::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();
diff --git a/crates/bevy_ui/src/experimental/ghost_hierarchy.rs b/crates/bevy_ui/src/experimental/ghost_hierarchy.rs
index 79d081c6bfd4d..2c942cde2ab46 100644
--- a/crates/bevy_ui/src/experimental/ghost_hierarchy.rs
+++ b/crates/bevy_ui/src/experimental/ghost_hierarchy.rs
@@ -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))
diff --git a/crates/bevy_winit/src/cursor/mod.rs b/crates/bevy_winit/src/cursor/mod.rs
index 7dcbc6d99375b..6d5a5471967a6 100644
--- a/crates/bevy_winit/src/cursor/mod.rs
+++ b/crates/bevy_winit/src/cursor/mod.rs
@@ -67,13 +67,13 @@ impl WinitAppRunnerState {
Query<(Entity, &mut PendingCursor), Changed>,
)> = 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>,
)> = 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() {
diff --git a/crates/bevy_winit/src/state.rs b/crates/bevy_winit/src/state.rs
index 1abaaaf6764e4..4af560e16045a 100644
--- a/crates/bevy_winit/src/state.rs
+++ b/crates/bevy_winit/src/state.rs
@@ -181,7 +181,7 @@ impl ApplicationHandler for WinitAppRunnerState {
// Create the initial window if needed
let mut create_window = SystemState::::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 for WinitAppRunnerState {
WinitUserEvent::WindowAdded => {
let mut create_window =
SystemState::::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 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 for WinitAppRunnerState {
fn about_to_wait(&mut self, event_loop: &ActiveEventLoop) {
let mut create_monitor = SystemState::::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, 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::::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);
}
diff --git a/examples/async_tasks/async_compute.rs b/examples/async_tasks/async_compute.rs
index 1018a13c37548..7fab1c7e136bf 100644
--- a/examples/async_tasks/async_compute.rs
+++ b/examples/async_tasks/async_compute.rs
@@ -91,7 +91,7 @@ fn spawn_tasks(mut commands: Commands) {
Res,
)>::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())
};
diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs
index d70ae13bfe191..65008403b51b9 100644
--- a/examples/ecs/fallible_params.rs
+++ b/examples/ecs/fallible_params.rs
@@ -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::*;
diff --git a/release-content/migration-guides/validation_merging.md b/release-content/migration-guides/validation_merging.md
new file mode 100644
index 0000000000000..ad4cfa8099002
--- /dev/null
+++ b/release-content/migration-guides/validation_merging.md
@@ -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, 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, 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::("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, SystemParamValidationError> {
+ // validation logic merged into get_param
+ if !is_valid(state, world) {
+ return Err(SystemParamValidationError::invalid::("not valid"));
+ }
+ // fetch logic
+ Ok(MyParam { /* ... */ })
+ }
+}
+```
+
+## Custom `ExclusiveSystemParam` implementations
+
+Similarly, `ExclusiveSystemParam::get_param` now returns a `Result, 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, 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.