Skip to content
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

Lifetimes and #[derive(WorldQuery)] #9377

Open
Zeenobit opened this issue Aug 6, 2023 · 1 comment
Open

Lifetimes and #[derive(WorldQuery)] #9377

Zeenobit opened this issue Aug 6, 2023 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation

Comments

@Zeenobit
Copy link
Contributor

Zeenobit commented Aug 6, 2023

How can Bevy's documentation be improved?

https://docs.rs/bevy/latest/bevy/ecs/query/trait.WorldQuery.html

When using #[derive(WorldQuery)], based on the documentation, we should be using 'static lifetime for all references:

#[derive(WorldQuery)]
struct MyQuery {
    entity: Entity,
    // It is required that all reference lifetimes are explicitly annotated, just like in any
    // struct. Each lifetime should be 'static.
    component_a: &'static ComponentA,
    component_b: &'static ComponentB,
}

However, it seems like you can also use a lifetime parameter:

#[derive(WorldQuery)]
struct MyQuery<'a> {
    entity: Entity,
    component_a: &'a ComponentA,
    component_b: &'a ComponentB,
}

In fact, I've been using this lifetime parameter in my code (without knowing it should be 'static) to allow construction of WorldQuery types from &World. For example:

#[derive(WorldQuery)]
struct PlayerRef<'a> {
    pub entity: Entity,
    pub name: &'a Name,
    // ...
}
impl<'a> PlayerRef<'a> {
    pub fn from_entity_ref(entity: EntityRef<'a>) -> Option<PlayerRef<'a>> {
        // ...
    }
}

I find this pattern useful because it allows these query types be used in systems, as well as any code that has exclusive &World access (such as debug, tests, editor, etc.):

let player = PlayerRef::from_entity(world.entity(entity)).unwrap();
assert_eq!(player.name, ...)

Is this usage an anti-pattern?

  • If so, I think there would be value in additional documentation about why the references have to be 'static despite having the option for them to not be so. Better yet, I'd say the derive macro should check for lifetimes and disallow them if we have to use 'static.
  • Otherwise, if we don't have to use 'static, and this is an acceptable usage, then I propose it should also be documented as such. The statement about references having to be static should be replaced with documentation on the lifetime parameter and how to use it correctly.
@Zeenobit Zeenobit added C-Docs An addition or correction to our documentation S-Needs-Triage This issue needs to be labelled labels Aug 6, 2023
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Aug 7, 2023
@JoJoJet
Copy link
Member

JoJoJet commented Aug 12, 2023

I have a somewhat related RFC: bevyengine/rfcs#68.

This would make derived WorldQuery types use a lifetime parameter, which I think is far cleaner than using 'static lifetimes for everything, and results in fewer generated types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation
Projects
None yet
Development

No branches or pull requests

3 participants