-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Unify ExclusiveSystemParams with normal SystemParams #16622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
How are you handling parameters like fn do_something(world: &mut World, archetypes: &Archetypes, query: Query<Entity>) {}would be UB, since the I think that's why #4166 had |
|
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 Another thing this would enable is adding |
9a77c73 to
150266d
Compare
# Objective - Required by #16622 due to differing implementations of `System` by `FunctionSystem` and `ExclusiveFunctionSystem`. - Optimize the memory usage of instances of `apply_deferred` in system schedules. ## Solution By changing `apply_deferred` from being an ordinary system that ends up as an `ExclusiveFunctionSystem`, and instead into a ZST struct that implements `System` manually, we save ~320 bytes per instance of `apply_deferred` in any schedule. ## Testing - All current tests pass. --- ## Migration Guide - If you were previously calling the special `apply_deferred` system via `apply_deferred(world)`, don't.
# Objective - Required by bevyengine#16622 due to differing implementations of `System` by `FunctionSystem` and `ExclusiveFunctionSystem`. - Optimize the memory usage of instances of `apply_deferred` in system schedules. ## Solution By changing `apply_deferred` from being an ordinary system that ends up as an `ExclusiveFunctionSystem`, and instead into a ZST struct that implements `System` manually, we save ~320 bytes per instance of `apply_deferred` in any schedule. ## Testing - All current tests pass. --- ## Migration Guide - If you were previously calling the special `apply_deferred` system via `apply_deferred(world)`, don't.
0fe16c6 to
ee98027
Compare
…lusive parameters
f06d99c to
ed7762e
Compare
Objective
Remove
ExclusiveSystemParamandExclusiveFunctionSystemin favor of justSystemParams, and define a system's exclusivity based on its parameters'SystemParam::world_access_levelreturn value.Solution
Note: This PR relies on #16642 to be merged first.EDIT: Merged.ExclusiveSystemParam,ExclusiveFunctionSystem, andExclusiveSystemParamFunction.&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 itSystemParams:&mut QueryStatenow will automatically update its archetypes viaSystemParam::new_archetype&mut SystemStatewill now have its deferred mutations automatically applied viaSystemParam::applyDeferredWorld'sinit_statewas updated to align with&Worldand&mut WorldTODO
&mut World, but this PR in its current state allows it.DeferredWorld, however. We should consider splitting "exclusivity" into "structural exclusivity" and "deferred exclusivity".&mut QueryStateand&mut SystemStateshouldn't do their things above automatically.bevy_lintto tell users not to use two&mut Worlds in a system or&mut World+Query/Res/etcTesting
Most pre-existing tests are passing currently, will need to add some new ones before merging.
Showcase
Systems with
&mut Worldno longer require it to be the first parameter (or second if using system input). It can now be placed anywhere in the function arguments:Migration Guide
TODO