-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Make SystemMeta::new public #11155
base: main
Are you sure you want to change the base?
Make SystemMeta::new public #11155
Conversation
Off topic for this PR, but you opened a few PRs now and never use the template. Could you try to keep the PR template and fill the sections? The |
@mockersf these sections add noise to trivial commits IMHO. I tried reformatting this PR, and now it has "SystemMeta::new public" said four times. But if it is important to use this structure, I don't mind. |
0a2081a
to
6e70eda
Compare
thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason it isn't pub is to try and make it not possible to remove accesses in SystemParam::init
. So the question is whether exposing SystemMeta::new
changes the safety requirements of implementing the SystemParam
trait. Currently the safety requirements are:
The implementor must ensure the following is true.
* [SystemParam::init_state](https://docs.rs/bevy/latest/bevy/ecs/system/trait.SystemParam.html#tymethod.init_state) correctly registers all [World](https://docs.rs/bevy/latest/bevy/ecs/world/struct.World.html) accesses used by [SystemParam::get_param](https://docs.rs/bevy/latest/bevy/ecs/system/trait.SystemParam.html#tymethod.get_param) with the provided [system_meta](https://docs.rs/bevy/latest/bevy/ecs/system/struct.SystemMeta.html).
* None of the world accesses may conflict with any prior accesses registered on system_meta.
We might want to add that you're not allowed to remove accesses previously registered.
But I don't think exposing SystemMeta::new
will make it any harder to implement the trait safely.
Objective
Without public
SystemMeta::new
it is hard to constructExclusiveSystemParam
.While technically it is possible (probably accidentally): by calling
SystemState::<PhantomData<T>>::new().meta().clone()
. So this new function being public does not allow anything was not possible before. Just makes it work naturally and more efficient.Context. Trying to implement custom systems. For non-exclusive systems, there's convenient
SystemState
utility.There's no such utility for
ExclusiveFunctionSystem
.Alternatively we can kick#11163SystemState
fromExclusiveSystemParam
signatures: it is not used in any implementations, and cannot do a lot of useful things.Solution
Make
SystemMeta::new
public.Changelog
SystemMeta::new
is made public