bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Make `#[system_param(ignore)]` and `#[world_query(ignore)]` unnecessary

Open joseph-gio opened this issue 2 years ago • 1 comments

Objective

When using PhantomData fields with the #[derive(SystemParam)] or #[derive(WorldQuery)] macros, the user is required to add the #[system_param(ignore)] attribute so that the macro knows to treat that field specially. This is undesirable, since it makes the macro more fragile and less consistent.

Solution

Implement SystemParam and WorldQuery for PhantomData. Remove the ignore attributes.

This borrows the approach used in #8012 to prevent generated types from conflicting with user-defined types.


Changelog

  • Implemented SystemParam and WorldQuery for PhantomData<T>.
  • Removed the attributes #[system_param(ignore)] and #[world_query(ignore)].

Migration Guide

The attribute #[system_param(ignore)] has been removed. If you were using this with PhantomData fields, simply remove the attribute. If you were using this for another type that implements Default, consider wrapping that type in Local<>.

#[derive(SystemParam)]
struct MyParam<'w, 's, Marker> {
    ...
    // Before:
    #[system_param(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,

    // Before:
    #[system_param(ignore)]
    value: MyDefaultType, // This will be initialized using `Default` each time `MyParam` is created.

    // After:
    value: Local<MyDefaultType>, // This will be initialized using `Default` the first time `MyParam` is created.
}

The attribute #[world_query(ignore)] has been removed, as it is now unnecessary.

#[derive(WorldQuery)]
struct MyQuery<Marker> {
    ...
    // Before:
    #[world_query(ignore)
    _marker: PhantomData<Marker>,

    // After:
    _marker: PhantomData<Marker>,
}

joseph-gio avatar Mar 10 '23 21:03 joseph-gio

This also prevents you from using any other weirder types in here. But I've never seen a case for that, and can't imagine one, so I think that's net good.

alice-i-cecile avatar Mar 10 '23 21:03 alice-i-cecile

Not totally sure that using PhantomData is a 1:1 replacement for world_query(ignore) and system_param(ignore) for all T: Default, though removing that functionality seems like a good idea to me to encourage users to use Local<T> instead.

Agreed, but I'm hopeful that they'll let us know if they were using it for anything else and we can find a less cludgy solution to their needs.

alice-i-cecile avatar Mar 17 '23 23:03 alice-i-cecile

I reverted the ensure_no_collision changes. I'll keep that in #8012 for now, so we don't have to sync multiple versions of the same code in different PRs.

joseph-gio avatar Mar 18 '23 00:03 joseph-gio

I figured out how to do this without removing the ignore attributes, so this PR is now non-breaking. I will remove the attributes in a follow-up PR.

joseph-gio avatar Mar 28 '23 19:03 joseph-gio