impl EntityBorrow for more types

[Victoronz]

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.

[chescock]

Is it really necessary to loosen the privacy here?

It looks like this is only needed so you can get an &Entity instead of an Entity to impl Borrow<Entity> for a few types. Since we're no longer using Borrow<Entity> internally, could we just leave out those impls?

Alternately, could we add a pub (crate) method to get &Entity so that it's clear nothing outside of this module modifies these fields?

[Victoronz]

right, there is a much simpler solution: I hadn't thought of just adding an entity method on UnsafeEntityCell itself for some reason. Should UnsafeEntityCell itself implement EntityBorrow?

[Victoronz]

Ah, I remembered why I didn't do this initially. Unless you return &Entity directly from a field, you will get a temporary lifetime error. So unless we increase the visibility here, or move the UnsafeEntityCell type to entity_ref.rs, we cannot impl Borrow<Entity> for the EntityRef types.
The reason why I added this impl is for the original purpose of Borrow: to allow its use in Map and Set types.
But I can omit it instead then.

[chescock]

I think it works if the method returns a reference to the original value:

impl UnsafeEntityCell<'_> {
    pub(crate) fn entity_ref(&self) -> &Entity {
        &self.entity
    }
}

impl Borrow<Entity> for EntityMut<'_> {
    fn borrow(&self) -> &Entity {
        self.0.entity_ref()
    }
}

But it's a moot point now that you removed them.

[Victoronz]

ahhh, you're right! Had I taken my train of thought just a bit further I could've realized, we can just impl Borrow<Entity> and EntityBorrow for UnsafeEntityCell...
I'll add them back.

[chescock]

I understand why we need to have this, but it's a little amusing since there can't possibly be two EntityMuts alive for the same entity. It's also not very useful with Query::iter_many, since you can never have an EntityMut that would match the query. Well, it can get combined with this other impls as part of the EntityPtr work and then it won't seem odd anymore.

[Victoronz]

You still can have its entity match another query! That is one thing the nesting_queries() test in entity_set.rs shows. But it is definitely more obscure.

[chescock]

Oh, I just meant EntityMut specifically. It has write access to all components, so it's going to conflict with any other query that matches the same archetypes. :)

[Victoronz]

I also meant EntityMut specifically! You can still query for an archetype without accessing any components :)

[chescock]

I suppose the danger here is that entities from different worlds that happen to have the same ID will compare equal. That seems unlikely to be a problem in practice, although it might be worth documenting somewhere.

[Victoronz]

Yeah, it is a bit unfortunate that we can't have both entity comparison/world comparison, but it seems to me that being able to compare them like Entity is a lot more valuable. I think I'll add some documentation to the types themselves.

[bushrat011899]

Looks good! I like seeing gaps in the API filled like this.