Skip to content

Commit

Permalink
Emphasize no structural changes in SystemParam::get_param (#17996)
Browse files Browse the repository at this point in the history
# Objective

Many systems like `Schedule` rely on the fact that every structural ECS
changes are deferred until an exclusive system flushes the `World`
itself. This gives us the benefits of being able to run systems in
parallel without worrying about dangling references caused by memory
(re)allocations, which will in turn lead to **Undefined Behavior**.
However, this isn't explicitly documented in `SystemParam`; currently it
only vaguely hints that in `init_state`, based on the fact that
structural ECS changes require mutable access to the _whole_ `World`.

## Solution

Document this behavior explicitly in `SystemParam`'s type-level
documentations.
  • Loading branch information
GlennFolker authored Mar 4, 2025
1 parent 019a6fd commit c819beb
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,10 @@ pub unsafe trait SystemParam: Sized {
///
/// # Safety
///
/// - The passed [`UnsafeWorldCell`] must have access to any world data
/// registered in [`init_state`](SystemParam::init_state).
/// - The passed [`UnsafeWorldCell`] must have access to any world data registered
/// in [`init_state`](SystemParam::init_state).
/// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state).
/// - all `world`'s archetypes have been processed by [`new_archetype`](SystemParam::new_archetype).
/// - All `world`'s archetypes have been processed by [`new_archetype`](SystemParam::new_archetype).
unsafe fn get_param<'world, 'state>(
state: &'state mut Self::State,
system_meta: &SystemMeta,
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_ecs/src/world/unsafe_world_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ impl<'w> UnsafeWorldCell<'w> {
/// Gets a mutable reference to the [`World`] this [`UnsafeWorldCell`] belongs to.
/// This is an incredibly error-prone operation and is only valid in a small number of circumstances.
///
/// Calling this method implies mutable access to the *whole* world (see first point on safety section
/// below), which includes all entities, components, and resources. Notably, calling this on
/// [`WorldQuery::init_fetch`](crate::query::WorldQuery::init_fetch) and
/// [`SystemParam::get_param`](crate::system::SystemParam::get_param) are most likely *unsound* unless
/// you can prove that the underlying [`World`] is exclusive, which in normal circumstances is not.
///
/// # Safety
/// - `self` must have been obtained from a call to [`World::as_unsafe_world_cell`]
/// (*not* `as_unsafe_world_cell_readonly` or any other method of construction that
Expand Down

0 comments on commit c819beb

Please sign in to comment.