Unified EntityPtr Types

TL;DR

We can reduce code duplication for the following sets of types because they differ only in semantics:

• EntityRef, EntityMut, and EntityWorldMut
• FilteredEntityRef, FilteredEntityMut
• EntityRefExcept, EntityMutExcept

What they look like currently:

pub struct EntityRef<'w>(UnsafeEntityCell<'w>);
pub struct EntityMut<'w>(UnsafeEntityCell<'w>);

// This isn't *exactly* what it is in-engine, but it's equivalent.
pub struct EntityWorldMut<'w>(UnsafeEntityCell<'w>); 

What they will look like afterwards:

pub struct EntityPtr<'w, L: ReadAccess>(UnsafeEntityCell<'w>, PhantomData<L>);

pub type EntityRef<'w> = EntityPtr<'w, ReadOnly>;
pub type EntityMut<'w> = EntityPtr<'w, Mutable>;
pub type EnttiyWorldMut<'w> = EntityPtr<'w, WorldMutable>;

Access Levels

These types do not differ memory-wise, but they do semantically. If we take a look at the three aforementioned types, we can see that there's three different "access levels" for safely accessing an UnsafeEntityCell:

1. Read only access
2. Mutable access
3. Structurally mutable access (can insert/remove components, and despawn the entity)

We can define some marker traits and structs to put these into rust:

pub trait ReadAccess /* sealed */ {}

pub struct ReadOnly;

impl ReadAccess for ReadOnly {}

// Since we also have two different levels of mutability
pub trait MutableAccess: ReadAccess {}

pub struct Mutable;

impl ReadAccess for Mutable {}
impl MutableAccess for Mutable {}

pub struct WorldMutable;

impl ReadAccess for WorldMutable {}
impl MutableAccess for WorldMutable {}

The above set of traits and types should cover all the different kinds of access bounds:

• L: ReadAccess - for when some kind of read access is needed
• L = ReadOnly - for when precisely read-only access is needed
• L: MutableAccess - for when some kind of mutable access is needed
• L = Mutable - for when precisely non-world-mutable access is needed
• L = WorldMutable - for when precisely world-mutable access is needed

Downgrading access

Because each higher level of access implies the levels below it, we can safely allow downgrading higher access level pointers to lower access level:

// Mutable access implies we have read-only access.
impl<'w> From<EntityPtr<'w, Mutable>> for EntityPtr<'w, ReadOnly> {
    // ...
}

// World-mutable access implies we have mutable access.
impl<'w> From<EntityPtr<'w, WorldMutable>> for EntityPtr<'w, Mutable> {
    // ...
}

Minimum access level

When working generically with some pointer we are only given read access:

// All operations within this `impl` block must only read.
impl<'w, L: ReadAccess> EntityPtr<'w, L> {
    // Because we can safely read the entity ID at any access level,
    // we write the function once rather than for every specific access level.
    pub fn id(&self) -> Entity {
        // ...
    }
    
    // Same with the entity's location
    pub fn location(&self) -> EntityLocation {
        // ...
    }
    
    // ...
}

Downgrading arbitrary access levels

We can combine the two previous concepts for implementing conversions from &EntityPtr<ANY> -> EntityPtr<ReadOnly> with only one From implementation:

// A read-only reference to an EntityPtr of arbitrary access level
// can be downgraded into an EntityPtr with read-only access.
impl<'w, L: ReadAccess> From<&'w EntityPtr<'_, L>> for EntityPtr<'w, ReadOnly> {
    // ...
}

// A mutable reference to an EntityPtr of arbitrary *mutable* access level
// can be downgraded into an EntityPtr with mutable access.
// This is equivalent to reborrowing.
impl<'w, L: MutableAccess> From<&'w mut EntityPtr<'_, L>> for EntityPtr<'w, Mutable> {
    // ...
}

Amendment: Consider adding Immutable access type + Deref/DerefMut impls for downgrading

Proposed by @bushRAT on the Bevy Discord, revised based on discussion with @quartermeister and @Diddykonga.

Originally proposed was to add Deref/DerefMut implementations for WorldMutable -> Mutable -> ReadOnly. However this would be unsound as ReadOnly is (currently) Copy + Clone, so someone could do:

fn duplicate<'w>(ptr: &mut EntityPtr<'w, Mutable>) -> (&mut EntityPtr<'w, Mutable>, EntityPtr<'w, ReadOnly>) {
    (*ptr).clone()
}

// oops just caused aliased mutability!
let (ptr, dup) = duplicate(ptr);

To combat this we could introduce a 4th access type, Immutable, alongside our three other access types (ReadOnly, Mutable, and WorldMutable). This enables the removal of the ReadAccess and MutableAccess traits and we would instead provide Deref/DerefMut impls, likely leading to a cleaner implementation overall.

First a look at the types in code:

// Access types; no traits needed as we'll use them directly
pub struct ReadOnly;
pub struct Immutable;
pub struct Mutable;
pub struct WorldMutable;

pub struct EntityPtr<'w, Access = ReadOnly>(UnsafeEntityCell<'w>, PhantomData<Access>);

pub type EntityView<'w> = EntityPtr<'w, ReadOnly>;
pub type EntityRef<'w> = EntityPtr<'w, Immutable>;
pub type EntityMut<'w> = EntityPtr<'w, Mutable>;
pub type EntityWorldMut<'w> = EntityPtr<'w, WorldMutable>;

The first thing

We should be able to safely implement the following:

impl<'w> Deref for EntityPtr<'w, WorldMutable> {
    type Target = EntityPtr<'w, Mutable>;
    
    fn deref(&self) -> &Self::Target {
        // EntityPtrs are layout-compatible and ideally #[repr(transparent)],
        // so this transmute should be safe.
        std::mem::transmute(self)
    }
}

impl<'w> DerefMut for EntityPtr<'w, WorldMutable> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        // Likewise as above
        std::mem::transmute(self)
    }
}

Tackling this change systematically

This changeset in its entirety will likely involve >3000 line changes when including file re-organization. Therefore it's prudent to make sure we push these changes incrementally in as small portions as reasonably possible.

Re-organization of entity_ref.rs

This is a big file (presently 4485 lines), and is a bit slow for Rust-Analyzer to process after an edit even on my (doot) well-specced machine, so we should take the following steps:

1. Move DynamicComponentFetch to entity_fetch.rs
   • We will likely unify this trait with WorldEntityFetch at some point, anyways.
2. Move FilteredEntityRef/FilteredEntityMut, TryFromFilteredError and associated tests into a new file entity_ref_filtered.rs
3. Move EntityRefExcept/EntityMutExcept, fn bundle_contains_component, and associated tests into a new file entity_ref_except.rs

With all that done entity_ref.rs will be down to ~3180 lines.

Change EntityWorldMut to hold just an UnsafeEntityCell

pub struct EntityWorldMut<'w> {
-   world: &'w mut World,
-   entity: Entity,
-   location: EntityLocation,
+   cell: UnsafeEntityCell<'w>,
}

// then fix up all uses of the now missing fields...

In isolation this might seem like a downgrade as we'll go from holding an explicit &mut World to an UnsafeWorldCell (contained in the UnsafeEntityCell), but it's required for the next step / the overall plan.

Finally, implement this document

1. Add new EntityPtr<L: ReadAccess> type and change EntityRef from struct to type alias.
2. Change EntityMut into a type alias.
3. Change EntityWorldMut into a type alias.
4. Add new FilteredEntityPtr<L: ReadAccess> type and change FilteredEntityRef/FilteredEntityMut into type aliases.
5. Add new EntityPtrExcept<L: ReadAccess> type and change EntityRefExcept/EntityMutExcept into type aliases.

These can be done as separate PRs, but TBD which changes should be grouped together into individual PRs.