Skip to content

Commit

Permalink
impl EntityBorrow for more types (#16917)
Browse files Browse the repository at this point in the history
# Objective

Some types like `RenderEntity` and `MainEntity` are just wrappers around
`Entity`, so they should be able to implement
`EntityBorrow`/`TrustedEntityBorrow`. This allows using them with
`EntitySet` functionality.
The `EntityRef` family are more than direct wrappers around `Entity`,
but can still benefit from being unique in a collection.

## Solution

Implement `EntityBorrow` and `TrustedEntityBorrow` for simple `Entity`
newtypes and `EntityRef` types.
These impls are an explicit decision to have the `EntityRef` types
compare like just `Entity`.
`EntityWorldMut` is omitted from this impl, because it explicitly
contains a `&mut World` as well, and we do not ever use more than one at
a time.

Add `EntityBorrow` to the `bevy_ecs` prelude.

## Migration Guide

`NormalizedWindowRef::entity` has been replaced with an
`EntityBorrow::entity` impl.
  • Loading branch information
Victoronz authored Dec 24, 2024
1 parent 450b939 commit 5b899dc
Show file tree
Hide file tree
Showing 10 changed files with 307 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub mod prelude {
bundle::Bundle,
change_detection::{DetectChanges, DetectChangesMut, Mut, Ref},
component::{require, Component},
entity::{Entity, EntityMapper},
entity::{Entity, EntityBorrow, EntityMapper},
event::{Event, EventMutator, EventReader, EventWriter, Events},
name::{Name, NameOrEntity},
observer::{CloneEntityWithObserversExt, Observer, Trigger},
Expand Down
36 changes: 35 additions & 1 deletion crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use super::{QueryData, QueryFilter, ReadOnlyQueryData};
use crate::{
archetype::{Archetype, ArchetypeEntity, Archetypes},
bundle::Bundle,
component::Tick,
entity::{Entities, Entity, EntityBorrow, EntitySet, EntitySetIterator},
query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, StorageId},
storage::{Table, TableRow, Tables},
world::unsafe_world_cell::UnsafeWorldCell,
world::{
unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept,
FilteredEntityMut, FilteredEntityRef,
},
};
use alloc::vec::Vec;
use core::{
Expand Down Expand Up @@ -1105,6 +1109,36 @@ impl<'w, 's, D: QueryData, F: QueryFilter> FusedIterator for QueryIter<'w, 's, D
// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator for QueryIter<'w, 's, Entity, F> {}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator for QueryIter<'w, 's, EntityRef<'_>, F> {}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator for QueryIter<'w, 's, EntityMut<'_>, F> {}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator
for QueryIter<'w, 's, FilteredEntityRef<'_>, F>
{
}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter> EntitySetIterator
for QueryIter<'w, 's, FilteredEntityMut<'_>, F>
{
}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator
for QueryIter<'w, 's, EntityRefExcept<'_, B>, F>
{
}

// SAFETY: [`QueryIter`] is guaranteed to return every matching entity once and only once.
unsafe impl<'w, 's, F: QueryFilter, B: Bundle> EntitySetIterator
for QueryIter<'w, 's, EntityMutExcept<'_, B>, F>
{
}

impl<'w, 's, D: QueryData, F: QueryFilter> Debug for QueryIter<'w, 's, D, F> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
f.debug_struct("QueryIter").finish()
Expand Down
240 changes: 238 additions & 2 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::{
bundle::{Bundle, BundleId, BundleInfo, BundleInserter, DynamicBundle, InsertMode},
change_detection::MutUntyped,
component::{Component, ComponentId, ComponentTicks, Components, Mutable, StorageType},
entity::{Entities, Entity, EntityCloneBuilder, EntityLocation},
entity::{
Entities, Entity, EntityBorrow, EntityCloneBuilder, EntityLocation, TrustedEntityBorrow,
},
event::Event,
observer::Observer,
query::{Access, ReadOnlyQueryData},
Expand All @@ -17,7 +19,13 @@ use bevy_ptr::{OwningPtr, Ptr};
use bevy_utils::{HashMap, HashSet};
#[cfg(feature = "track_change_detection")]
use core::panic::Location;
use core::{any::TypeId, marker::PhantomData, mem::MaybeUninit};
use core::{
any::TypeId,
cmp::Ordering,
hash::{Hash, Hasher},
marker::PhantomData,
mem::MaybeUninit,
};
use thiserror::Error;

use super::{unsafe_world_cell::UnsafeEntityCell, Ref, ON_REMOVE, ON_REPLACE};
Expand Down Expand Up @@ -369,6 +377,44 @@ impl<'a> TryFrom<&'a FilteredEntityMut<'_>> for EntityRef<'a> {
}
}

impl PartialEq for EntityRef<'_> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl Eq for EntityRef<'_> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl PartialOrd for EntityRef<'_> {
/// [`EntityRef`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl Ord for EntityRef<'_> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl Hash for EntityRef<'_> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl EntityBorrow for EntityRef<'_> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl TrustedEntityBorrow for EntityRef<'_> {}

/// Provides mutable access to a single entity and all of its components.
///
/// Contrast with [`EntityWorldMut`], which allows adding and removing components,
Expand Down Expand Up @@ -869,6 +915,44 @@ impl<'a> TryFrom<&'a mut FilteredEntityMut<'_>> for EntityMut<'a> {
}
}

impl PartialEq for EntityMut<'_> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl Eq for EntityMut<'_> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl PartialOrd for EntityMut<'_> {
/// [`EntityMut`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl Ord for EntityMut<'_> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl Hash for EntityMut<'_> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl EntityBorrow for EntityMut<'_> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl TrustedEntityBorrow for EntityMut<'_> {}

/// A mutable reference to a particular [`Entity`], and the entire world.
///
/// This is essentially a performance-optimized `(Entity, &mut World)` tuple,
Expand Down Expand Up @@ -2969,6 +3053,44 @@ impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a> {
}
}

impl PartialEq for FilteredEntityRef<'_> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl Eq for FilteredEntityRef<'_> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl PartialOrd for FilteredEntityRef<'_> {
/// [`FilteredEntityRef`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl Ord for FilteredEntityRef<'_> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl Hash for FilteredEntityRef<'_> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl EntityBorrow for FilteredEntityRef<'_> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl TrustedEntityBorrow for FilteredEntityRef<'_> {}

/// Provides mutable access to a single entity and some of its components defined by the contained [`Access`].
///
/// To define the access when used as a [`QueryData`](crate::query::QueryData),
Expand Down Expand Up @@ -3258,6 +3380,44 @@ impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a> {
}
}

impl PartialEq for FilteredEntityMut<'_> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl Eq for FilteredEntityMut<'_> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl PartialOrd for FilteredEntityMut<'_> {
/// [`FilteredEntityMut`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl Ord for FilteredEntityMut<'_> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl Hash for FilteredEntityMut<'_> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl EntityBorrow for FilteredEntityMut<'_> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl TrustedEntityBorrow for FilteredEntityMut<'_> {}

/// Error type returned by [`TryFrom`] conversions from filtered entity types
/// ([`FilteredEntityRef`]/[`FilteredEntityMut`]) to full-access entity types
/// ([`EntityRef`]/[`EntityMut`]).
Expand Down Expand Up @@ -3361,6 +3521,44 @@ where
}
}

impl<B: Bundle> PartialEq for EntityRefExcept<'_, B> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl<B: Bundle> Eq for EntityRefExcept<'_, B> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl<B: Bundle> PartialOrd for EntityRefExcept<'_, B> {
/// [`EntityRefExcept`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl<B: Bundle> Ord for EntityRefExcept<'_, B> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl<B: Bundle> Hash for EntityRefExcept<'_, B> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl<B: Bundle> EntityBorrow for EntityRefExcept<'_, B> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl<B: Bundle> TrustedEntityBorrow for EntityRefExcept<'_, B> {}

/// Provides mutable access to all components of an entity, with the exception
/// of an explicit set.
///
Expand Down Expand Up @@ -3464,6 +3662,44 @@ where
}
}

impl<B: Bundle> PartialEq for EntityMutExcept<'_, B> {
fn eq(&self, other: &Self) -> bool {
self.entity() == other.entity()
}
}

impl<B: Bundle> Eq for EntityMutExcept<'_, B> {}

#[expect(clippy::non_canonical_partial_ord_impl)]
impl<B: Bundle> PartialOrd for EntityMutExcept<'_, B> {
/// [`EntityMutExcept`]'s comparison trait implementations match the underlying [`Entity`],
/// and cannot discern between different worlds.
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.entity().partial_cmp(&other.entity())
}
}

impl<B: Bundle> Ord for EntityMutExcept<'_, B> {
fn cmp(&self, other: &Self) -> Ordering {
self.entity().cmp(&other.entity())
}
}

impl<B: Bundle> Hash for EntityMutExcept<'_, B> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.entity().hash(state);
}
}

impl<B: Bundle> EntityBorrow for EntityMutExcept<'_, B> {
fn entity(&self) -> Entity {
self.id()
}
}

// SAFETY: This type represents one Entity. We implement the comparison traits based on that Entity.
unsafe impl<B: Bundle> TrustedEntityBorrow for EntityMutExcept<'_, B> {}

fn bundle_contains_component<B>(components: &Components, query_id: ComponentId) -> bool
where
B: Bundle,
Expand Down
8 changes: 7 additions & 1 deletion crates/bevy_ecs/src/world/unsafe_world_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
bundle::Bundles,
change_detection::{MaybeUnsafeCellLocation, MutUntyped, Ticks, TicksMut},
component::{ComponentId, ComponentTicks, Components, Mutable, StorageType, Tick, TickCells},
entity::{Entities, Entity, EntityLocation},
entity::{Entities, Entity, EntityBorrow, EntityLocation},
observer::Observers,
prelude::Component,
query::{DebugCheckedUnwrap, ReadOnlyQueryData},
Expand Down Expand Up @@ -1183,3 +1183,9 @@ unsafe fn get_ticks(
StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_ticks(entity),
}
}

impl EntityBorrow for UnsafeEntityCell<'_> {
fn entity(&self) -> Entity {
self.id()
}
}
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{
change_detection::DetectChanges,
component::{Component, ComponentId, Mutable},
entity::Entity,
entity::{Entity, EntityBorrow},
event::EventReader,
prelude::{require, With},
query::Has,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera_driver_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
renderer::RenderContext,
view::ExtractedWindows,
};
use bevy_ecs::{prelude::QueryState, world::World};
use bevy_ecs::{entity::EntityBorrow, prelude::QueryState, world::World};
use bevy_utils::HashSet;
use wgpu::{LoadOp, Operations, RenderPassColorAttachment, RenderPassDescriptor, StoreOp};

Expand Down
Loading

0 comments on commit 5b899dc

Please sign in to comment.