Skip to content

Commit

Permalink
Allow World::entity family of functions to take multiple entities a…
Browse files Browse the repository at this point in the history
…nd get multiple references back (#15614)

# Objective

Following the pattern established in #15593, we can reduce the API
surface of `World` by providing a single function to grab both a
singular entity reference, or multiple entity references.

## Solution

The following functions can now also take multiple entity IDs and will
return multiple entity references back:
- `World::entity`
- `World::get_entity`
- `World::entity_mut`
- `World::get_entity_mut`
- `DeferredWorld::entity_mut`
- `DeferredWorld::get_entity_mut`

If you pass in X, you receive Y:
- give a single `Entity`, receive a single `EntityRef`/`EntityWorldMut`
(matches current behavior)
- give a `[Entity; N]`/`&[Entity; N]` (array), receive an equally-sized
`[EntityRef; N]`/`[EntityMut; N]`
- give a `&[Entity]` (slice), receive a
`Vec<EntityRef>`/`Vec<EntityMut>`
- give a `&EntityHashSet`, receive a
`EntityHashMap<EntityRef>`/`EntityHashMap<EntityMut>`

Note that `EntityWorldMut` is only returned in the single-entity case,
because having multiple at the same time would lead to UB. Also,
`DeferredWorld` receives an `EntityMut` in the single-entity case
because it does not allow structural access.

## Testing

- Added doc-tests on `World::entity`, `World::entity_mut`, and
`DeferredWorld::entity_mut`
- Added tests for aliased mutability and entity existence

---

## Showcase

<details>
  <summary>Click to view showcase</summary>

The APIs for fetching `EntityRef`s and `EntityMut`s from the `World`
have been unified.

```rust
// This code will be referred to by subsequent code blocks.
let world = World::new();
let e1 = world.spawn_empty().id();
let e2 = world.spawn_empty().id();
let e3 = world.spawn_empty().id();
```

Querying for a single entity remains mostly the same:

```rust
// 0.14
let eref: EntityRef = world.entity(e1);
let emut: EntityWorldMut = world.entity_mut(e1);
let eref: Option<EntityRef> = world.get_entity(e1);
let emut: Option<EntityWorldMut> = world.get_entity_mut(e1);

// 0.15
let eref: EntityRef = world.entity(e1);
let emut: EntityWorldMut = world.entity_mut(e1);
let eref: Result<EntityRef, Entity> = world.get_entity(e1);
let emut: Result<EntityWorldMut, Entity> = world.get_entity_mut(e1);
```

Querying for multiple entities with an array has changed:

```rust
// 0.14
let erefs: [EntityRef; 2] = world.many_entities([e1, e2]);
let emuts: [EntityMut; 2] = world.many_entities_mut([e1, e2]);
let erefs: Result<[EntityRef; 2], Entity> = world.get_many_entities([e1, e2]);
let emuts: Result<[EntityMut; 2], QueryEntityError> = world.get_many_entities_mut([e1, e2]);

// 0.15
let erefs: [EntityRef; 2] = world.entity([e1, e2]);
let emuts: [EntityMut; 2] = world.entity_mut([e1, e2]);
let erefs: Result<[EntityRef; 2], Entity> = world.get_entity([e1, e2]);
let emuts: Result<[EntityMut; 2], EntityFetchError> = world.get_entity_mut([e1, e2]);
```

Querying for multiple entities with a slice has changed:

```rust
let ids = vec![e1, e2, e3]);

// 0.14
let erefs: Result<Vec<EntityRef>, Entity> = world.get_many_entities_dynamic(&ids[..]);
let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_dynamic_mut(&ids[..]);

// 0.15
let erefs: Result<Vec<EntityRef>, Entity> = world.get_entity(&ids[..]);
let emuts: Result<Vec<EntityMut>, EntityFetchError> = world.get_entity_mut(&ids[..]);
let erefs: Vec<EntityRef> = world.entity(&ids[..]); // Newly possible!
let emuts: Vec<EntityMut> = world.entity_mut(&ids[..]); // Newly possible!
```

Querying for multiple entities with an `EntityHashSet` has changed:

```rust
let set = EntityHashSet::from_iter([e1, e2, e3]);

// 0.14
let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_from_set_mut(&set);

// 0.15
let emuts: Result<EntityHashMap<EntityMut>, EntityFetchError> = world.get_entity_mut(&set);
let erefs: Result<EntityHashMap<EntityRef>, EntityFetchError> = world.get_entity(&set); // Newly possible!
let emuts: EntityHashMap<EntityMut> = world.entity_mut(&set); // Newly possible!
let erefs: EntityHashMap<EntityRef> = world.entity(&set); // Newly possible!
```

</details>

## Migration Guide

- `World::get_entity` now returns `Result<_, Entity>` instead of
`Option<_>`.
- Use `world.get_entity(..).ok()` to return to the previous behavior.
- `World::get_entity_mut` and `DeferredWorld::get_entity_mut` now return
`Result<_, EntityFetchError>` instead of `Option<_>`.
- Use `world.get_entity_mut(..).ok()` to return to the previous
behavior.
- Type inference for `World::entity`, `World::entity_mut`,
`World::get_entity`, `World::get_entity_mut`,
`DeferredWorld::entity_mut`, and `DeferredWorld::get_entity_mut` has
changed, and might now require the input argument's type to be
explicitly written when inside closures.
- The following functions have been deprecated, and should be replaced
as such:
    - `World::many_entities` -> `World::entity::<[Entity; N]>`
    - `World::many_entities_mut` -> `World::entity_mut::<[Entity; N]>`
    - `World::get_many_entities` -> `World::get_entity::<[Entity; N]>`
- `World::get_many_entities_dynamic` -> `World::get_entity::<&[Entity]>`
- `World::get_many_entities_mut` -> `World::get_entity_mut::<[Entity;
N]>`
- The equivalent return type has changed from `Result<_,
QueryEntityError>` to `Result<_, EntityFetchError>`
- `World::get_many_entities_dynamic_mut` ->
`World::get_entity_mut::<&[Entity]>1
- The equivalent return type has changed from `Result<_,
QueryEntityError>` to `Result<_, EntityFetchError>`
- `World::get_many_entities_from_set_mut` ->
`World::get_entity_mut::<&EntityHashSet>`
- The equivalent return type has changed from `Result<Vec<EntityMut>,
QueryEntityError>` to `Result<EntityHashMap<EntityMut>,
EntityFetchError>`. If necessary, you can still convert the
`EntityHashMap` into a `Vec`.
  • Loading branch information
ItsDoot authored Oct 7, 2024
1 parent 31409eb commit 584d148
Show file tree
Hide file tree
Showing 18 changed files with 967 additions and 284 deletions.
8 changes: 4 additions & 4 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1637,13 +1637,13 @@ mod tests {
"new entity is created immediately after world_a's max entity"
);
assert!(world_b.get::<A>(e1).is_none());
assert!(world_b.get_entity(e1).is_none());
assert!(world_b.get_entity(e1).is_err());

assert!(world_b.get::<A>(e2).is_none());
assert!(world_b.get_entity(e2).is_none());
assert!(world_b.get_entity(e2).is_err());

assert!(world_b.get::<A>(e3).is_none());
assert!(world_b.get_entity(e3).is_none());
assert!(world_b.get_entity(e3).is_err());

world_b.get_or_spawn(e1).unwrap().insert(B(1));
assert_eq!(
Expand Down Expand Up @@ -1694,7 +1694,7 @@ mod tests {

let high_non_existent_but_reserved_entity = Entity::from_raw(5);
assert!(
world_b.get_entity(high_non_existent_but_reserved_entity).is_none(),
world_b.get_entity(high_non_existent_but_reserved_entity).is_err(),
"entities between high-newly allocated entity and continuous block of existing entities don't exist"
);

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/observer/entity_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl Component for ObservedBy {
};
for e in observed_by {
let (total_entities, despawned_watched_entities) = {
let Some(mut entity_mut) = world.get_entity_mut(e) else {
let Ok(mut entity_mut) = world.get_entity_mut(e) else {
continue;
};
let Some(mut state) = entity_mut.get_mut::<ObserverState>() else {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/reflect/entity_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ fn insert_reflect(
.get_represented_type_info()
.expect("component should represent a type.");
let type_path = type_info.type_path();
let Some(mut entity) = world.get_entity_mut(entity) else {
let Ok(mut entity) = world.get_entity_mut(entity) else {
panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003");
};
let Some(type_registration) = type_registry.get(type_info.type_id()) else {
Expand Down Expand Up @@ -284,7 +284,7 @@ fn remove_reflect(
type_registry: &TypeRegistry,
component_type_path: Cow<'static, str>,
) {
let Some(mut entity) = world.get_entity_mut(entity) else {
let Ok(mut entity) = world.get_entity_mut(entity) else {
return;
};
let Some(type_registration) = type_registry.get_with_type_path(&component_type_path) else {
Expand Down
26 changes: 13 additions & 13 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1757,7 +1757,7 @@ fn try_despawn() -> impl EntityCommand {
fn insert<T: Bundle>(bundle: T, mode: InsertMode) -> impl EntityCommand {
let caller = Location::caller();
move |entity: Entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.insert_with_caller(
bundle,
mode,
Expand All @@ -1776,7 +1776,7 @@ fn insert_from_world<T: Component + FromWorld>(mode: InsertMode) -> impl EntityC
let caller = Location::caller();
move |entity: Entity, world: &mut World| {
let value = T::from_world(world);
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.insert_with_caller(
value,
mode,
Expand All @@ -1795,8 +1795,8 @@ fn insert_from_world<T: Component + FromWorld>(mode: InsertMode) -> impl EntityC
fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand {
#[cfg(feature = "track_change_detection")]
let caller = Location::caller();
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
move |entity: Entity, world: &mut World| {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.insert_with_caller(
bundle,
mode,
Expand All @@ -1818,8 +1818,8 @@ unsafe fn insert_by_id<T: Send + 'static>(
value: T,
on_none_entity: impl FnOnce(Entity) + Send + 'static,
) -> impl EntityCommand {
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
move |entity: Entity, world: &mut World| {
if let Ok(mut entity) = world.get_entity_mut(entity) {
// SAFETY:
// - `component_id` safety is ensured by the caller
// - `ptr` is valid within the `make` block;
Expand All @@ -1837,7 +1837,7 @@ unsafe fn insert_by_id<T: Send + 'static>(
/// For a [`Bundle`] type `T`, this will remove any components in the bundle.
/// Any components in the bundle that aren't found on the entity will be ignored.
fn remove<T: Bundle>(entity: Entity, world: &mut World) {
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.remove::<T>();
}
}
Expand All @@ -1848,23 +1848,23 @@ fn remove<T: Bundle>(entity: Entity, world: &mut World) {
/// Panics if the provided [`ComponentId`] does not exist in the [`World`].
fn remove_by_id(component_id: ComponentId) -> impl EntityCommand {
move |entity: Entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.remove_by_id(component_id);
}
}
}

/// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle.
fn remove_with_requires<T: Bundle>(entity: Entity, world: &mut World) {
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.remove_with_requires::<T>();
}
}

/// An [`EntityCommand`] that removes all components associated with a provided entity.
fn clear() -> impl EntityCommand {
move |entity: Entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.clear();
}
}
Expand All @@ -1875,7 +1875,7 @@ fn clear() -> impl EntityCommand {
/// For a [`Bundle`] type `T`, this will remove all components except those in the bundle.
/// Any components in the bundle that aren't found on the entity will be ignored.
fn retain<T: Bundle>(entity: Entity, world: &mut World) {
if let Some(mut entity_mut) = world.get_entity_mut(entity) {
if let Ok(mut entity_mut) = world.get_entity_mut(entity) {
entity_mut.retain::<T>();
}
}
Expand Down Expand Up @@ -1919,8 +1919,8 @@ fn log_components(entity: Entity, world: &mut World) {
fn observe<E: Event, B: Bundle, M>(
observer: impl IntoObserverSystem<E, B, M>,
) -> impl EntityCommand {
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
move |entity: Entity, world: &mut World| {
if let Ok(mut entity) = world.get_entity_mut(entity) {
entity.observe_entity(observer);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ where
/// let entity = world.run_system_once(|mut commands: Commands| {
/// commands.spawn_empty().id()
/// }).unwrap();
/// # assert!(world.get_entity(entity).is_some());
/// # assert!(world.get_entity(entity).is_ok());
/// ```
///
/// ## Immediate Queries
Expand Down
12 changes: 6 additions & 6 deletions crates/bevy_ecs/src/system/system_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl World {
O: 'static,
{
match self.get_entity_mut(id.entity) {
Some(mut entity) => {
Ok(mut entity) => {
let registered_system = entity
.take::<RegisteredSystem<I, O>>()
.ok_or(RegisteredSystemError::SelfRemove(id))?;
Expand All @@ -191,7 +191,7 @@ impl World {
system: registered_system.system,
})
}
None => Err(RegisteredSystemError::SystemIdNotRegistered(id)),
Err(_) => Err(RegisteredSystemError::SystemIdNotRegistered(id)),
}
}

Expand Down Expand Up @@ -327,7 +327,7 @@ impl World {
// lookup
let mut entity = self
.get_entity_mut(id.entity)
.ok_or(RegisteredSystemError::SystemIdNotRegistered(id))?;
.map_err(|_| RegisteredSystemError::SystemIdNotRegistered(id))?;

// take ownership of system trait object
let RegisteredSystem {
Expand All @@ -350,7 +350,7 @@ impl World {
};

// return ownership of system trait object (if entity still exists)
if let Some(mut entity) = self.get_entity_mut(id.entity) {
if let Ok(mut entity) = self.get_entity_mut(id.entity) {
entity.insert::<RegisteredSystem<I, O>>(RegisteredSystem {
initialized,
system,
Expand Down Expand Up @@ -398,7 +398,7 @@ impl World {
}

self.resource_scope(|world, mut id: Mut<CachedSystemId<S::System>>| {
if let Some(mut entity) = world.get_entity_mut(id.0.entity()) {
if let Ok(mut entity) = world.get_entity_mut(id.0.entity()) {
if !entity.contains::<RegisteredSystem<I, O>>() {
entity.insert(system_bundle(Box::new(IntoSystem::into_system(system))));
}
Expand Down Expand Up @@ -538,7 +538,7 @@ where
O: Send + 'static,
{
fn apply(self, world: &mut World) {
if let Some(mut entity) = world.get_entity_mut(self.entity) {
if let Ok(mut entity) = world.get_entity_mut(self.entity) {
entity.insert(system_bundle(self.system));
}
}
Expand Down
Loading

0 comments on commit 584d148

Please sign in to comment.