Skip to content

Added Convenience Method For Entity Mutation#9602

Closed
bushrat011899 wants to merge 1 commit intobevyengine:mainfrom
bushrat011899:EntityCommandsMutableEntityAccess
Closed

Added Convenience Method For Entity Mutation#9602
bushrat011899 wants to merge 1 commit intobevyengine:mainfrom
bushrat011899:EntityCommandsMutableEntityAccess

Conversation

@bushrat011899
Copy link
Copy Markdown
Contributor

Objective

Solution

  • Added for_entity_mut to EntityCommands which allows mutable access to the target Entity. This method is implemented as discussed in #9544
  • Added a unit test and documentation as appropriate.

Notes

  • Ideally this would be an extension to the add method, but FnOnce(Entity, &mut World) already implemented EntityCommand, it is not (currently) possible to also implement it for FnOnce(EntityMut) as well. In the future, it may be possible to overcome this limitation through variadic functions.
  • Name chosen somewhat arbitrarily by me. It seems like a good fit as this method is like an iterator for_each_mut over a single item EntityMut, hence for_entity_mut. The original proposed name was mutate, which I personally felt might have been ambiguous (e.g., are we mutating the command list, the world, etc.)

@joseph-gio
Copy link
Copy Markdown
Member

joseph-gio commented Aug 28, 2023

Ideally this would be an extension to the add method, but FnOnce(Entity, &mut World) already implemented EntityCommand, it is not (currently) possible to also implement it for FnOnce(EntityMut) as well.

We can just remove the impl for FnOnce(Entity, &mut World) imo. The EntityMut variant just makes more sense -- that's probably what I should have done in the first place when I added that trait.

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this much better than the solution in #9544.

I'm not a fan of the repeated entity in the method call, I still like the mutate name best of the options :p

That said, I would actually just follow @JoJoJet's advice above, and break the previous impl in favor of this! It's much more reasonable, and it's good to clean up old bits of the API that didn't turn out useful in practice.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Aug 28, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2023
# Objective

- Fixes #4917
- Replaces #9602

## Solution

- Replaced `EntityCommand` implementation for `FnOnce` to apply to
`FnOnce(EntityMut)` instead of `FnOnce(Entity, &mut World)`

---

## Changelog

- `FnOnce(Entity, &mut World)` no longer implements `EntityCommand`.
This is a breaking change.

## Migration Guide

### 1. New-Type `FnOnce`

Create an `EntityCommand` type which implements the method you
previously wrote:

```rust
pub struct ClassicEntityCommand<F>(pub F);

impl<F> EntityCommand for ClassicEntityCommand<F>
where
    F: FnOnce(Entity, &mut World) + Send + 'static,
{
    fn apply(self, id: Entity, world: &mut World) {
        (self.0)(id, world);
    }
}

commands.add(ClassicEntityCommand(|id: Entity, world: &mut World| {
    /* ... */
}));
```

### 2. Extract `(Entity, &mut World)` from `EntityMut`

The method `into_world_mut` can be used to gain access to the `World`
from an `EntityMut`.

```rust
let old = |id: Entity, world: &mut World| {
    /* ... */
};

let new = |mut entity: EntityMut| {
    let id = entity.id();
    let world = entity.into_world_mut();
    /* ... */
};
```
Shatur pushed a commit to simgine/bevy that referenced this pull request Aug 30, 2023
# Objective

- Fixes bevyengine#4917
- Replaces bevyengine#9602

## Solution

- Replaced `EntityCommand` implementation for `FnOnce` to apply to
`FnOnce(EntityMut)` instead of `FnOnce(Entity, &mut World)`

---

## Changelog

- `FnOnce(Entity, &mut World)` no longer implements `EntityCommand`.
This is a breaking change.

## Migration Guide

### 1. New-Type `FnOnce`

Create an `EntityCommand` type which implements the method you
previously wrote:

```rust
pub struct ClassicEntityCommand<F>(pub F);

impl<F> EntityCommand for ClassicEntityCommand<F>
where
    F: FnOnce(Entity, &mut World) + Send + 'static,
{
    fn apply(self, id: Entity, world: &mut World) {
        (self.0)(id, world);
    }
}

commands.add(ClassicEntityCommand(|id: Entity, world: &mut World| {
    /* ... */
}));
```

### 2. Extract `(Entity, &mut World)` from `EntityMut`

The method `into_world_mut` can be used to gain access to the `World`
from an `EntityMut`.

```rust
let old = |id: Entity, world: &mut World| {
    /* ... */
};

let new = |mut entity: EntityMut| {
    let id = entity.id();
    let world = entity.into_world_mut();
    /* ... */
};
```
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Fixes bevyengine#4917
- Replaces bevyengine#9602

## Solution

- Replaced `EntityCommand` implementation for `FnOnce` to apply to
`FnOnce(EntityMut)` instead of `FnOnce(Entity, &mut World)`

---

## Changelog

- `FnOnce(Entity, &mut World)` no longer implements `EntityCommand`.
This is a breaking change.

## Migration Guide

### 1. New-Type `FnOnce`

Create an `EntityCommand` type which implements the method you
previously wrote:

```rust
pub struct ClassicEntityCommand<F>(pub F);

impl<F> EntityCommand for ClassicEntityCommand<F>
where
    F: FnOnce(Entity, &mut World) + Send + 'static,
{
    fn apply(self, id: Entity, world: &mut World) {
        (self.0)(id, world);
    }
}

commands.add(ClassicEntityCommand(|id: Entity, world: &mut World| {
    /* ... */
}));
```

### 2. Extract `(Entity, &mut World)` from `EntityMut`

The method `into_world_mut` can be used to gain access to the `World`
from an `EntityMut`.

```rust
let old = |id: Entity, world: &mut World| {
    /* ... */
};

let new = |mut entity: EntityMut| {
    let id = entity.id();
    let world = entity.into_world_mut();
    /* ... */
};
```
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-Usability A targeted quality-of-life change that makes Bevy easier to use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EntityCommands get mutable reference to added components

3 participants