Unify ExclusiveSystemParams with normal SystemParams
Objective
Remove ExclusiveSystemParam and ExclusiveFunctionSystem in favor of just SystemParams, and define a system's exclusivity based on its parameters' SystemParam::is_exclusive return value.
Solution
- Removed
ExclusiveSystemParam,ExclusiveFunctionSystem, andExclusiveSystemParamFunction. - Changed
apply_deferredinto a customSystemimplementation. As a byproduct, this reduces the memory usage of eachapply_deferredinstance significantly (from minimum 320 bytes to just 4 bytes).- This change is necessary because of the differing implementations of
SystemforFunctionSystemandExclusiveFunctionSystem.
- This change is necessary because of the differing implementations of
&mut World,&mut QueryState<D, F>, and&mut SystemState<P>now implementSystemParam.&mut Worldregisters exclusive access, so it cannot be used alongside any parameter that accesses the world&mut QueryStateand&mut SystemStatedon't hold access to the world, so can be used alongside it
- As
SystemParams:&mut QueryStatenow will automatically update its archetypes viaSystemParam::new_archetype&mut SystemStatewill now have its deferred mutations automatically applied viaSystemParam::apply
DeferredWorld'sinit_statewas updated to align with&Worldand&mut World
TODO
- Observers cannot be allowed to use
&mut World, but this PR in its current state allows it.- We should allow them to use
DeferredWorld, however. We should consider splitting "exclusivity" into "structural exclusivity" and "deferred exclusivity".
- We should allow them to use
- Decide if
&mut QueryStateand&mut SystemStateshouldn't do their things above automatically. - Consider adding new lints to
bevy_lintto tell users not to use two&mut Worlds in a system or&mut World+Query/Res/etc
Testing
Most pre-existing tests are passing currently, will need to add some new ones before merging.
Showcase
Systems with &mut World no longer require it to be the first parameter (or second if using system input). It can now be placed anywhere in the function arguments:
// Before:
fn do_stuff(world: &mut World, local: Local<usize>, query: &mut QueryState<&Foo>) {}
// or
fn do_stuff_with(In(v): In<i32>, world: &mut World, local: Local<usize>, query: &mut QueryState<&Foo>) {}
// Now also possible:
fn do_stuff(local: Local<usize>, world: &mut World, query: &mut QueryState<&Foo>) {}
// or
fn do_stuff(local: Local<usize>, query: &mut QueryState<&Foo>, world: &mut World) {}
// or
fn do_stuff_with(In(v): In<i32>, local: Local<usize>, query: &mut QueryState<&Foo>, world: &mut World) {}
// or
fn do_stuff_with(In(v): In<i32>, local: Local<usize>, world: &mut World, query: &mut QueryState<&Foo>) {}
Migration Guide
TODO
How are you handling parameters like &Archetypes and Query<Entity> that have no Access but still rely on the structure of the world not changing? That is, a system like
fn do_something(world: &mut World, archetypes: &Archetypes, query: Query<Entity>) {}
would be UB, since the &mut World could change archetypes or add entities. But those params currently register no access and don't look different from Local, so I think your change might allow them.
I think that's why #4166 had enum WorldAccessLevel.
Added the controversial tag since there was a old pr with this approach, but we merged the current approach instead.
You should add tests for the current SystemParams that conflict with &mut World, since that's now a safety concern. Maybe 2 tests each, one with &mut World first and one with &mut World second.
A fun thing this would enable is putting &mut World inside a ParamSet so you can switch between full access and typed parameters. That's not any more powerful than putting the other params in a SystemState, but it might be more ergonomic in some cases.
Another thing this would enable is adding SystemParamBuilder<SystemState<P>> and SystemParamBuilder<QueryState<D, F>> implementations in the future, which would make it easier to use dynamic queries in exclusive systems.