Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replaced EntityCommand Implementation for FnOnce #9604

Conversation

bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Aug 28, 2023

Objective

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:

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.

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

let new = |mut entity: EntityMut| {
    let id = entity.id();
    let world = entity.into_world_mut();
    /* ... */
};

@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
@alice-i-cecile
Copy link
Member

Round three! I definitely like this as a final solution best of all :)

@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Aug 28, 2023
@bushrat011899
Copy link
Contributor Author

Round three! I definitely like this as a final solution best of all :)

Fingers crossed! 😂

Copy link
Member

@joseph-gio joseph-gio left a comment

Choose a reason for hiding this comment

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

The unsafe method world_mut can be used to gain access to the World from an EntityMut if a more reasonable refactor is not possible:

We should encourage people to use into_world_mut or world_scope instead. Otherwise, LGTM.

@bushrat011899
Copy link
Contributor Author

We should encourage people to use into_world_mut or world_scope instead. Otherwise, LGTM.

Didn't know about that method! I've updated the Migration Guide for this PR to reflect the better alternative.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 28, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 28, 2023
Merged via the queue into bevyengine:main with commit 1839ff7 Aug 28, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 29, 2023
# Objective

Fix #4278
Fix #5504
Fix #9422

Provide safe ways to borrow an entire entity, while allowing disjoint
mutable access. `EntityRef` and `EntityMut` are not suitable for this,
since they provide access to the entire world -- they are just helper
types for working with `&World`/`&mut World`.

This has potential uses for reflection and serialization

## Solution

Remove `EntityRef::world`, which allows it to soundly be used within
queries.

`EntityMut` no longer supports structural world mutations, which allows
multiple instances of it to exist for different entities at once.
Structural world mutations are performed using the new type
`EntityWorldMut`.

```rust
fn disjoint_system(
     q2: Query<&mut A>,
     q1: Query<EntityMut, Without<A>>,
) { ... }

let [entity1, entity2] = world.many_entities_mut([id1, id2]);
*entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap();

for entity in world.iter_entities_mut() {
    ...
}
```

---

## Changelog

- Removed `EntityRef::world`, to fix a soundness issue with queries.
+ Removed the ability to structurally mutate the world using
`EntityMut`, which allows it to be used in queries.
+ Added `EntityWorldMut`, which is used to perform structural mutations
that are no longer allowed using `EntityMut`.

## Migration Guide

**Note for maintainers: ensure that the guide for #9604 is updated
accordingly.**

Removed the method `EntityRef::world`, to fix a soundness issue with
queries. If you need access to `&World` while using an `EntityRef`,
consider passing the world as a separate parameter.

`EntityMut` can no longer perform 'structural' world mutations, such as
adding or removing components, or despawning the entity. Additionally,
`EntityMut::world`, `EntityMut::world_mut` , and
`EntityMut::world_scope` have been removed.
Instead, use the newly-added type `EntityWorldMut`, which is a helper
type for working with `&mut World`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Shatur pushed a commit to projectharmonia/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();
    /* ... */
};
```
@cart cart mentioned this pull request Oct 13, 2023
43 tasks
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2024
…ght be despawned (#11107)

# Objective

In #9604 we removed the ability to define an `EntityCommand` as
`fn(Entity, &mut World)`. However I have since realized that `fn(Entity,
&mut World)` is an incredibly expressive and powerful way to define a
command for an entity that may or may not exist (`fn(EntityWorldMut)`
only works on entities that are alive).

## Solution

Support `EntityCommand`s in the style of `fn(Entity, &mut World)`, as
well as `fn(EntityWorldMut)`. Use a marker generic on the
`EntityCommand` trait to allow multiple impls.

The second commit in this PR replaces all of the internal command
definitions with ones using `fn` definitions. This is mostly just to
show off how expressive this style of command is -- we can revert this
commit if we'd rather avoid breaking changes.

---

## Changelog

Re-added support for expressively defining an `EntityCommand` as a
function that takes `Entity, &mut World`.

## Migration Guide

All `Command` types in `bevy_ecs`, such as `Spawn`, `SpawnBatch`,
`Insert`, etc., have been made private. Use the equivalent methods on
`Commands` or `EntityCommands` instead.
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();
    /* ... */
};
```
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Fix bevyengine#4278
Fix bevyengine#5504
Fix bevyengine#9422

Provide safe ways to borrow an entire entity, while allowing disjoint
mutable access. `EntityRef` and `EntityMut` are not suitable for this,
since they provide access to the entire world -- they are just helper
types for working with `&World`/`&mut World`.

This has potential uses for reflection and serialization

## Solution

Remove `EntityRef::world`, which allows it to soundly be used within
queries.

`EntityMut` no longer supports structural world mutations, which allows
multiple instances of it to exist for different entities at once.
Structural world mutations are performed using the new type
`EntityWorldMut`.

```rust
fn disjoint_system(
     q2: Query<&mut A>,
     q1: Query<EntityMut, Without<A>>,
) { ... }

let [entity1, entity2] = world.many_entities_mut([id1, id2]);
*entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap();

for entity in world.iter_entities_mut() {
    ...
}
```

---

## Changelog

- Removed `EntityRef::world`, to fix a soundness issue with queries.
+ Removed the ability to structurally mutate the world using
`EntityMut`, which allows it to be used in queries.
+ Added `EntityWorldMut`, which is used to perform structural mutations
that are no longer allowed using `EntityMut`.

## Migration Guide

**Note for maintainers: ensure that the guide for bevyengine#9604 is updated
accordingly.**

Removed the method `EntityRef::world`, to fix a soundness issue with
queries. If you need access to `&World` while using an `EntityRef`,
consider passing the world as a separate parameter.

`EntityMut` can no longer perform 'structural' world mutations, such as
adding or removing components, or despawning the entity. Additionally,
`EntityMut::world`, `EntityMut::world_mut` , and
`EntityMut::world_scope` have been removed.
Instead, use the newly-added type `EntityWorldMut`, which is a helper
type for working with `&mut World`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EntityCommands get mutable reference to added components
3 participants