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.

Sign up for GitHub

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

Jump to bottom

Added Convenience Method For Entity Mutation #9602

Closed
wants to merge 1 commit into from
Closed

Added Convenience Method For Entity Mutation #9602

wants to merge 1 commit into from

Conversation

bushrat011899
Copy link
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.)

@JoJoJet
Copy link
Member

JoJoJet commented Aug 28, 2023 •
edited
Loading

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.

alice-i-cecile
alice-i-cecile requested changes Aug 28, 2023
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment •
edited
Loading

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
@bushrat011899 bushrat011899 mentioned this pull request Aug 28, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2023
@bushrat011899
Replaced EntityCommand Implementation for FnOnce (#9604)
1839ff7 
# 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 projectharmonia/bevy that referenced this pull request Aug 30, 2023
@bushrat011899 @Shatur
Replaced EntityCommand Implementation for FnOnce (bevyengine#9604)
18fd719 
# 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
@bushrat011899
Replaced EntityCommand Implementation for FnOnce (bevyengine#9604)
c1da5bc 
# 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
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile requested changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

EntityCommands get mutable reference to added components
3 participants
@bushrat011899 @JoJoJet @alice-i-cecile