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

Missing EntityWorldMut::into_mut(self) #12417

Closed
jkb0o opened this issue Mar 11, 2024 · 4 comments · Fixed by #12419
Closed

Missing EntityWorldMut::into_mut(self) #12417

jkb0o opened this issue Mar 11, 2024 · 4 comments · Fixed by #12419
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@jkb0o
Copy link
Contributor

jkb0o commented Mar 11, 2024

What problem does this solve or what need does it fill?

For my plugin I'd like to have closures that takes Entity and &' mut World and returns Mut<'w, T>:

fn translation<'w>(id: Entity, world: &'w mut World) -> Mut<'w, Vec3> {
    world
        .entity_mut(id)
        .get_mut::<Transform>()
        .unwrap()
        .map_unchanged(|t| &mut t.translation)
}

This code won't compile:

cannot return value referencing temporary value
returns a value referencing data owned by the current function

It happens because EntityWorldMut<'w>::get_mut(&mut self) returns Mut with lifetime of &mut self instead of 'w.

What solution would you like?

It would be nice to have some method on EntityWorldMut like get_mut but that consumes self and returns Mut<'w, T>:

// bevy_ecs/world/entity_ref.rs
impl<'w> EntityWorldMut<'w> {
  pub fn into_mut<T: Component>(self) -> Option<Mut<'w, T>> {
    unsafe { self.into_unsafe_entity_cell().get_mut() }
  }
}

It is possible to implement the translation function for now:

fn translation<'w>(id: Entity, world: &'w mut World) -> Mut<'w, Vec3> {
  world
    .entity_mut(id)
    .into_mut::<Transform>()
    .unwrap()
    .map_unchanged(|t| &mut t.translation)
}

What alternative(s) have you considered?

I didn't find a way to bypass proper lifetime within my transform function without making changes to engine.

@jkb0o jkb0o added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Mar 11, 2024
@james7132 james7132 added A-ECS Entities, components, systems, and events S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Trivial Nice and easy! A great choice to get started with Bevy and removed S-Needs-Triage This issue needs to be labelled labels Mar 11, 2024
@james7132
Copy link
Member

james7132 commented Mar 11, 2024

Might be good to add an equivalent functions for EntityMut and EntityRef:

  • EntityRef::into_borrow
  • EntityRef::into_ref
  • EntityRef::into_borrow
  • EntityMut::into_ref
  • EntityMut::into_mut
  • EntityWorldMut::into_borrow
  • EntityWorldMut::into_ref
  • EntityWorldMut::into_mut

The docs will likely need to clarify what the difference between these and get/get_mut is.

@jkb0o
Copy link
Contributor Author

jkb0o commented Mar 11, 2024

@james7132 I'm not sure we need EntityRef::into_ methods. I have no problems with read-only lifetimes. This one compiles well:

fn translation_ref<'w>(id: Entity, world: &'w mut World) -> Ref<'w, Vec3> {
    world
        .entity(id)
        .get_ref::<Transform>()
        .unwrap()
        .map(|r| &r.translation)
}

The problem only with mutable access.

@jkb0o
Copy link
Contributor Author

jkb0o commented Mar 11, 2024

And I'm not sure what into_borrow means. If it is an alternative to reborrow then it doesn't look practical for me: reborrow is about to convert &'a mut EntityMut<'w> into EntityMut<'a>, there is no place for consuming self here.

@james7132
Copy link
Member

Ah right, Entity*::get returns &'w T. Makes sense as it's Copy. The mutable variants are the only ones that need it.

into_borrow would return &'w T instead of Ref<'w, T>.

github-merge-queue bot pushed a commit that referenced this issue Mar 17, 2024
…#12419)

# Objective

Provide component access to `&'w T`, `Ref<'w, T>`, `Mut<'w, T>`,
`Ptr<'w>` and `MutUntyped<'w>` from `EntityMut<'w>`/`EntityWorldMut<'w>`
with the world `'w` lifetime instead of `'_`.

Fixes #12417

## Solution

Add `into_` prefixed methods for `EntityMut<'w>`/`EntityWorldMut<'w>`
that consume `self` and returns component access with the world `'w`
lifetime unlike the `get_` prefixed methods that takes `&'a self` and
returns component access with `'a` lifetime.


Methods implemented:
- EntityMut::into_borrow
- EntityMut::into_ref
- EntityMut::into_mut
- EntityMut::into_borrow_by_id
- EntityMut::into_mut_by_id
- EntityWorldMut::into_borrow
- EntityWorldMut::into_ref
- EntityWorldMut::into_mut
- EntityWorldMut::into_borrow_by_id
- EntityWorldMut::into_mut_by_id
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-Feature A new feature, making something new possible D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants