Restore separate methods for `World::get_many_entities` #18234

chescock · 2025-03-10T17:33:53Z

Objective

Reduce the complexity of the World::get_entity API by removing the F: WorldEntityFetch type parameter and restoring separate World::get_many_entities methods.

It would be good for consistency to have Query::get and World::get_entity work the same way, but per discussion on #18036 (comment), WorldEntityFetch was too much complexity for Query::get. Instead, let's remove WorldEntityFetch and make World::get_entity work like Query::get.

This undoes #15614 (although I did not attempt to implement it as a revert). A few things have changed since then that makes the duplication less of a problem than it was when WorldEntityFetch was introduced:

One change is that Bevy has improved handling for Results, so we no longer expect to offer panicking versions of all APIs, and we do not need to restore the panicking many_entities and many_entities_mut.

Another change is the introduction of EntitySet and related traits in #16547. This lets us generalize over types that hold sets of unique Entitys, so we do not need a separate method for every such type.

Solution

Remove F: WorldEntityFetch parameter from World::get_entity() and related methods, and have it only accept Entity.

Introduce new methods to handle the various "many" cases:

get_many_entities takes an array and returns an array of EntityRef. These are read-only, so duplicates are okay.
iter_entities takes an iterator and returns an iterator of EntityRef. These are read-only, so duplicates are okay.
get_many_entities_mut takes an array and returns an array of EntityMut. It performs the O(N^2) duplicate check. This is acceptable because we expect N to be small for arrays.
get_many_entities_unique_mut takes a UniqueEntityArray and returns an array of EntityMut. UniqueEntityArray ensures there are no duplicates.
iter_entities_mut takes an EntitySet and returns an iterator of EntityMut. EntitySet ensures there are no duplicates.

Also rename the existing iter_entities(_mut) methods to iter_all_entities(_mut), to free up the name.

Migration Guide

World::entity(), World::entity_mut(), World::get_entity(), and World::get_entity_mut() now only take a single Entity instead of being generic and accepting collections. If you were passing a collection, you will need to call a different method.

If you were using a panicking variant, first replace it with get_entity(e).unwrap() or get_entity_mut(e).unwrap().

Then, replace:

World::get_entity::<[Entity; N]> -> World::get_many_entities()
World::get_entity_mut::<[Entity; N]> -> World::get_many_entities_mut()
World::get_entity::<&[Entity]> -> World::iter_entities().collect::<Vec<_>>()
World::get_entity_mut::<&[Entity]> -> This has no direct equivalent, as it performed an O(N^2) check that can be expensive for large slices. First convert the collection to an EntitySet, such as EntityHashSet, then call World::iter_entities(e).
World::get_entity::<&EntityHashSet> -> World::iter_entities(e).map(|e| (e.id(), e)).collect::<EntityHashMap<_>>()
World::get_entity_mut::<&EntityHashSet> -> World::iter_entities_mut(e).map(|e| (e.id(), e)).collect::<EntityHashMap<_>>()

In addition, World::iter_entities(_mut) has been renamed iter_all_entities(_mut) to make the name available for these new methods. If you were calling iter_entities or iter_entities_mut, the functionality is still available under the new name.

Add `get_many_entities` and `iter_entities` methods to handle lookups of multiple entities, instead.

Victoronz · 2025-03-14T19:59:31Z

crates/bevy_ecs/src/world/mod.rs

    }

    /// Returns an [`Entity`] iterator of current entities.
    ///
    /// This is useful in contexts where you only have read-only access to the [`World`].
    #[inline]
-    pub fn iter_entities(&self) -> impl Iterator<Item = EntityRef<'_>> + '_ {
+    pub fn iter_all_entities(&self) -> impl Iterator<Item = EntityRef<'_>> + '_ {


This can also be impl EntitySetIterator!

Yup, but that should be a separate PR (especially if I wind up reverting the name change and not even touching this line).

Victoronz · 2025-03-14T19:59:56Z

crates/bevy_ecs/src/world/mod.rs

@@ -966,7 +947,7 @@ impl World {
    }

    /// Returns a mutable iterator over all entities in the `World`.
-    pub fn iter_entities_mut(&mut self) -> impl Iterator<Item = EntityMut<'_>> + '_ {
+    pub fn iter_all_entities_mut(&mut self) -> impl Iterator<Item = EntityMut<'_>> + '_ {


Same here, impl EntitySetIterator.

Victoronz · 2025-03-14T20:00:05Z

Thanks for the PR!

Why make iter_entities the EntitySet version? Usually the naming conventions goes:
"I want to iterate it fully" -> no suffix
"I want to iterate a partial list, allowing duplicates" -> many suffix
"I want to iterate a subset" -> many_unique suffix
We could offer all of them, and retain the pattern.

For consistency, we also want both get_many_entities_unique and get_many_unique_mut, the impls can be shared the same way as in #18315!

When adding entities to a method name, does it make sense to follow human language? "I want to get many unique entities."
Meaning the name could be get_many_unique_entities_mut.
The mut suffix is already an exception to this, but I wonder what adjective placement makes more sense here.

chescock · 2025-03-15T00:23:13Z

Why make iter_entities the EntitySet version?

Honestly, I've been looking at these methods for so long that the words have lost all meaning :). It does seem weird that adding many to the name returns fewer things, but I guess that's already true for Query::iter_many.

So, your proposal relative to this is renaming:
iter_all_entities(_mut) => iter_entities(_mut) (meaning, revert that part)
iter_entities => iter_many_entities
iter_entities_mut => iter_many_unique_entities_mut
get_many_entities_unique_mut => get_many_unique_entities_mut
and adding:
get_many_unique_entities (which is the same as get_many_entities but with a stricter argument type, and so still seems a bit pointless to me)
iter_many_unique_entities (which is different from iter_many_entities because it returns impl EntitySetIterator instead of impl Iterator, so I can see how it could be useful)

The result is that we're adding all 8 combinations of (get|iter)_many(_unique)?_entities(_mut)? - except for iter_many_entities_mut, because there's no obviously correct way to check an iterator for duplicates.

Is that right? Sure, I can do that.

Victoronz · 2025-03-15T00:41:29Z

Why make iter_entities the EntitySet version?

Honestly, I've been looking at these methods for so long that the words have lost all meaning :). It does seem weird that adding many to the name returns fewer things, but I guess that's already true for Query::iter_many.

So, your proposal relative to this is renaming: iter_all_entities(_mut) => iter_entities(_mut) (meaning, revert that part) iter_entities => iter_many_entities iter_entities_mut => iter_many_unique_entities_mut get_many_entities_unique_mut => get_many_unique_entities_mut and adding: get_many_unique_entities (which is the same as get_many_entities but with a stricter argument type, and so still seems a bit pointless to me) iter_many_unique_entities (which is different from iter_many_entities because it returns impl EntitySetIterator instead of impl Iterator, so I can see how it could be useful)

The result is that we're adding all 8 combinations of (get|iter)_many(_unique)?_entities(_mut)? - except for iter_many_entities_mut, because there's no obviously correct way to check an iterator for duplicates.

Is that right? Sure, I can do that.

Yeah!

As an aside, for iter_many_entities_mut there is technically a way to do it, which is filtering the entity iterator in its next call with a HashSet of its previously yielded items.
But that still goes in the category of "Too expensive for an implicit cost", because you are then both doing the iteration and building a HashSet one step at a time...
So better left out here :)

chescock · 2025-03-15T14:51:46Z

crates/bevy_ecs/src/world/mod.rs

+    /// // Trying to access the same entity multiple times will fail.
+    /// assert!(world.get_many_entities_mut([id1, id1]).is_err());
+    /// ```
+    pub fn get_many_entities_mut<T: TrustedEntityBorrow, const N: usize>(


#18315 (comment)

it looks like I left a few as T: TrustedEntityBorrow. Should I switch those back to Entity, too?

Did you? I don't see them!

Here's one! I'll switch get_many_entities_mut to [Entity; N] and get_many_entities_unique_mut to UniqueEntityArray<N>.

…ntityFetch

Restrict arrays and `UniqueEntityArray`s from `T: TrustedEntityBorrow` to `Entity`.

iter_entities => iter_many_entities iter_entities_mut => iter_many_unique_entities_mut get_many_entities_unique_mut => get_many_unique_entities_mut

This reverts commit dccd52e.

ItsDoot

Two things:

Seems to be missing DeferredWorld::iter_many_entities as a counterpart to World::iter_many_entities.
We'll also need these functions on EntityFetcher which was added since.

Overall I'm alright with this change, the unique entity set stuff certainly helps compared to the original situation.

ItsDoot · 2025-05-10T04:25:35Z

crates/bevy_ecs/src/world/deferred_world.rs

-    /// - Returns [`EntityMutableFetchError::EntityDoesNotExist`] if any of the given `entities` do not exist in the world.
-    ///     - Only the first entity found to be missing will be returned.
-    /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the same entity is requested multiple times.
+    /// [`EntityDoesNotExistError`] if any entity does not exist in the world.
    ///


Suggested change

///

///

/// # Examples

///

ItsDoot · 2025-05-10T04:26:49Z

crates/bevy_ecs/src/world/deferred_world.rs

-    /// ## Single [`Entity`]
+    /// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`.
+    /// This will panic if the `entity` does not exist. Use [`DeferredWorld::get_entity_mut`] if you want
+    /// to check for entity existence instead of implicitly panic-ing.
    ///


Suggested change

///

///

/// # Examples

///

ItsDoot · 2025-05-10T04:32:50Z

crates/bevy_ecs/src/world/deferred_world.rs

+    pub fn get_many_entities_mut<const N: usize>(
+        &mut self,
+        entities: [Entity; N],
+    ) -> Result<[EntityMut<'_>; N], EntityMutableFetchError> {


I wonder if at some point we should provide a variant that returns [Result<EntityMut<'_>, EntityMutableFetchError>; N]. 🤔 Unfortunately std doesn't provide a transpose function for results <-> arrays, so we'd have to provide that function ourselves or make the user perform it themselves.

ItsDoot · 2025-05-10T04:37:54Z

crates/bevy_ecs/src/world/mod.rs

-    /// ## Single [`Entity`]
+    /// Retrieves an [`EntityRef`] that exposes read-only operations for the given `entity`.
+    /// This will panic if the `entity` does not exist. Use [`World::get_entity`] if you want
+    /// to check for entity existence instead of implicitly panic-ing.
    ///


Suggested change

///

///

/// # Examples

///

ItsDoot · 2025-05-10T04:38:26Z

crates/bevy_ecs/src/world/mod.rs

-    /// ## Single [`Entity`]
+    /// Retrieves an [`EntityWorldMut`] that exposes read and write operations for the given `entity`.
+    /// This will panic if the `entity` does not exist. Use [`World::get_entity_mut`] if you want
+    /// to check for entity existence instead of implicitly panic-ing.
    ///


Suggested change

///

///

/// # Examples

///

ItsDoot · 2025-05-10T04:39:58Z

crates/bevy_ecs/src/world/mod.rs

-    ///     - Only the first entity found to be missing will be returned.
-    /// - Returns [`EntityMutableFetchError::AliasedMutability`] if the same entity is requested multiple times.
+    /// [`EntityDoesNotExistError`] if any entity does not exist in the world.
+    pub fn get_many_unique_entities<const N: usize>(


chescock · 2025-05-10T16:45:35Z

Seems to be missing DeferredWorld::iter_many_entities as a counterpart to World::iter_many_entities.

I think the &World methods are covered by DeferredWorld: Deref<Target = World>.

We'll also need these functions on EntityFetcher which was added since.

Yeah, I got stalled on this PR once that got merged because I didn't really want three copies, and it seemed like there ought to be a way to combine World, DeferredWorld, and EntityFetcher using Deref or generics or something. But I haven't actually come up with a good way to do that, so maybe I should just update this PR with the third copy.

chescock added 2 commits March 9, 2025 09:54

Rename iter_entities to iter_all_entities.

dccd52e

Remove WorldEntityFetch.

ac73089

Add `get_many_entities` and `iter_entities` methods to handle lookups of multiple entities, instead.

chescock requested review from ItsDoot and Victoronz March 10, 2025 17:56

Victoronz reviewed Mar 14, 2025

View reviewed changes

chescock mentioned this pull request Mar 14, 2025

implement get_many_unique #18315

Merged

chescock commented Mar 15, 2025

View reviewed changes

chescock added 5 commits March 17, 2025 11:41

Merge remote-tracking branch 'remotes/origin/main' into remove-WorldE…

d0205a1

…ntityFetch

Update UniqueEntityArray to new path.

2a7233b

Restrict arrays and `UniqueEntityArray`s from `T: TrustedEntityBorrow` to `Entity`.

Rename

5d8f531

iter_entities => iter_many_entities iter_entities_mut => iter_many_unique_entities_mut get_many_entities_unique_mut => get_many_unique_entities_mut

Revert "Rename iter_entities to iter_all_entities."

311c454

This reverts commit dccd52e.

Add get_many_unique_entities and iter_many_unique_entities.

54150ef

chescock added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 24, 2025

ItsDoot reviewed May 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Restore separate methods for `World::get_many_entities` #18234

Restore separate methods for `World::get_many_entities` #18234

chescock commented Mar 10, 2025

Uh oh!

Victoronz Mar 14, 2025

Uh oh!

chescock Mar 15, 2025

Uh oh!

Victoronz Mar 14, 2025

Uh oh!

Victoronz commented Mar 14, 2025

Uh oh!

chescock commented Mar 15, 2025

Uh oh!

Victoronz commented Mar 15, 2025

Uh oh!

chescock Mar 15, 2025

Uh oh!

ItsDoot left a comment

Uh oh!

ItsDoot May 10, 2025

Uh oh!

ItsDoot May 10, 2025

Uh oh!

ItsDoot May 10, 2025

Uh oh!

ItsDoot May 10, 2025

Uh oh!

ItsDoot May 10, 2025

Uh oh!

ItsDoot May 10, 2025

Uh oh!

chescock commented May 10, 2025

Uh oh!

Uh oh!

-    ///
+    ///
+    /// # Examples
+    ///

Uh oh!

Restore separate methods for World::get_many_entities #18234

Are you sure you want to change the base?

Restore separate methods for World::get_many_entities #18234

Conversation

chescock commented Mar 10, 2025

Objective

Solution

Migration Guide

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Victoronz commented Mar 14, 2025

Uh oh!

chescock commented Mar 15, 2025

Uh oh!

Victoronz commented Mar 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ItsDoot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chescock commented May 10, 2025

Uh oh!

Uh oh!

Restore separate methods for `World::get_many_entities` #18234

Restore separate methods for `World::get_many_entities` #18234