Commit Graph

8 Commits

Author SHA1 Message Date
Vic
10da4dc9ae Rename EntityBorrow/TrustedEntityBorrow to ContainsEntity/EntityEquivalent (#18470)
# Objective

Fixes #9367.

Yet another follow-up to #16547.

These traits were initially based on `Borrow<Entity>` because that trait
was what they were replacing, and it felt close enough in meaning.
However, they ultimately don't quite match: `borrow` always returns
references, whereas `EntityBorrow` always returns a plain `Entity`.
Additionally, `EntityBorrow` can imply that we are borrowing an `Entity`
from the ECS, which is not what it does.

Due to its safety contract, `TrustedEntityBorrow` is important an
important and widely used trait for `EntitySet` functionality.
In contrast, the safe `EntityBorrow` does not see much use, because even
outside of `EntitySet`-related functionality, it is a better idea to
accept `TrustedEntityBorrow` over `EntityBorrow`.

Furthermore, as #9367 points out, abstracting over returning `Entity`
from pointers/structs that contain it can skip some ergonomic friction.

On top of that, there are aspects of #18319 and #18408 that are relevant
to naming:
We've run into the issue that relying on a type default can switch
generic order. This is livable in some contexts, but unacceptable in
others.

To remedy that, we'd need to switch to a type alias approach: 
The "defaulted" `Entity` case becomes a
`UniqueEntity*`/`Entity*Map`/`Entity*Set` alias, and the base type
receives a more general name. `TrustedEntityBorrow` does not mesh
clearly with sensible base type names.

## Solution
Replace any `EntityBorrow` bounds with `TrustedEntityBorrow`.
+
Rename them as such:
`EntityBorrow` -> `ContainsEntity`
`TrustedEntityBorrow` -> `EntityEquivalent`

For `EntityBorrow` we produce a change in meaning; We designate it for
types that aren't necessarily strict wrappers around `Entity` or some
pointer to `Entity`, but rather any of the myriad of types that contain
a single associated `Entity`.
This pattern can already be seen in the common `entity`/`id` methods
across the engine.
We do not mean for `ContainsEntity` to be a trait that abstracts input
API (like how `AsRef<T>` is often used, f.e.), because eliding
`entity()` would be too implicit in the general case.

We prefix "Contains" to match the intuition of a struct with an `Entity`
field, like some contain a `length` or `capacity`.
It gives the impression of structure, which avoids the implication of a
relationship to the `ECS`.
`HasEntity` f.e. could be interpreted as "a currently live entity", 

As an input trait for APIs like #9367 envisioned, `TrustedEntityBorrow`
is a better fit, because it *does* restrict itself to strict wrappers
and pointers. Which is why we replace any
`EntityBorrow`/`ContainsEntity` bounds with
`TrustedEntityBorrow`/`EntityEquivalent`.

Here, the name `EntityEquivalent` is a lot closer to its actual meaning,
which is "A type that is both equivalent to an `Entity`, and forms the
same total order when compared".
Prior art for this is the
[`Equivalent`](https://docs.rs/hashbrown/latest/hashbrown/trait.Equivalent.html)
trait in `hashbrown`, which utilizes both `Borrow` and `Eq` for its one
blanket impl!

Given that we lose the `Borrow` moniker, and `Equivalent` can carry
various meanings, we expand on the safety comment of `EntityEquivalent`
somewhat. That should help prevent the confusion we saw in
[#18408](https://github.com/bevyengine/bevy/pull/18408#issuecomment-2742094176).

The new name meshes a lot better with the type aliasing approach in
#18408, by aligning with the base name `EntityEquivalentHashMap`.
For a consistent scheme among all set types, we can use this scheme for
the `UniqueEntity*` wrapper types as well!
This allows us to undo the switched generic order that was introduced to
`UniqueEntityArray` by its `Entity` default.

Even without the type aliases, I think these renames are worth doing!

## Migration Guide

Any use of `EntityBorrow` becomes `ContainsEntity`.
Any use of `TrustedEntityBorrow` becomes `EntityEquivalent`.
2025-03-30 10:24:00 +02:00
Joona Aalto
9165fb020a
Implement Serialize/Deserialize for entity collections (#17620)
# Objective

Follow-up to #17615.

Bevy's entity collection types like `EntityHashSet` no longer implement
serde's `Serialize` and `Deserialize` after becoming newtypes instead of
type aliases in #16912. This broke some types that support serde for me
in Avian.

I also missed creating const constructors for `EntityIndexMap` and
`EntityIndexSet` in #17615. Oops!

## Solution

Implement `Serialize` and `Deserialize` for Bevy's entity collection
types, and add const constructors for `EntityIndexMap` and
`EntityIndexSet`.

I didn't implement `ReflectSerialize` or `ReflectDeserialize` here,
because I had some trouble fixing the resulting errors, and they were
not implemented previously either.
2025-02-02 15:42:36 +00:00
Joona Aalto
59697f9ccc
Make EntityHashMap::new and EntityHashSet::new const (#17615)
# Objective

#16912 turned `EntityHashMap` and `EntityHashSet` into proper newtypes
instead of type aliases. However, this removed the ability to create
these collections in const contexts; previously, you could use
`EntityHashSet::with_hasher(EntityHash)`, but it doesn't exist anymore.

## Solution

Make `EntityHashMap::new` and `EntityHashSet::new` const methods.
2025-01-30 17:40:06 +00:00
Zachary Harrold
9bc0ae33c3
Move hashbrown and foldhash out of bevy_utils (#17460)
# Objective

- Contributes to #16877

## Solution

- Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils`
and into `bevy_platform_support`
- Refactored the above to match the layout of these types in `std`.
- Updated crates as required.

## Testing

- CI

---

## Migration Guide

- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::hash`:
  - `FixedState`
  - `DefaultHasher`
  - `RandomState`
  - `FixedHasher`
  - `Hashed`
  - `PassHash`
  - `PassHasher`
  - `NoOpHash`
- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::collections`:
  - `HashMap`
  - `HashSet`
- `bevy_utils::hashbrown` has been removed. Instead, import from
`bevy_platform_support::collections` _or_ take a dependency on
`hashbrown` directly.
- `bevy_utils::Entry` has been removed. Instead, import from
`bevy_platform_support::collections::hash_map` or
`bevy_platform_support::collections::hash_set` as appropriate.
- All of the above equally apply to `bevy::utils` and
`bevy::platform_support`.

## Notes

- I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils`
as they might be candidates for micro-crating. They can always be moved
into `bevy_platform_support` at a later date if desired.
2025-01-23 16:46:08 +00:00
Luc
93e5e6cb95
fix double comment characters (#17484)
# Objective
- Improve docs by removing duplicate comment characters
- Fixes #17483

## Solution
- Replaced `/// ///` with `///`
2025-01-21 23:24:05 +00:00
Vic
59657ed1e2
remove unsound DerefMut impls from EntityHashMap/EntityHashSet (#17450)
# Objective

Noticed while doing #17449, I had left these `DerefMut` impls in.
Obtaining mutable references to those inner iterator types allows for
`mem::swap`, which can be used to swap an incorrectly behaving instance
into the wrappers.

## Solution

Remove them!
2025-01-20 21:28:28 +00:00
Alice Cecile
5a9bc28502
Support non-Vec data structures in relations (#17447)
# Objective

The existing `RelationshipSourceCollection` uses `Vec` as the only
possible backing for our relationships. While a reasonable choice,
benchmarking use cases might reveal that a different data type is better
or faster.

For example:

- Not all relationships require a stable ordering between the
relationship sources (i.e. children). In cases where we a) have many
such relations and b) don't care about the ordering between them, a hash
set is likely a better datastructure than a `Vec`.
- The number of children-like entities may be small on average, and a
`smallvec` may be faster

## Solution

- Implement `RelationshipSourceCollection` for `EntityHashSet`, our
custom entity-optimized `HashSet`.
-~~Implement `DoubleEndedIterator` for `EntityHashSet` to make things
compile.~~
   -  This implementation was cursed and very surprising.
- Instead, by moving the iterator type on `RelationshipSourceCollection`
from an erased RPTIT to an explicit associated type we can add a trait
bound on the offending methods!
- Implement `RelationshipSourceCollection` for `SmallVec`

## Testing

I've added a pair of new tests to make sure this pattern compiles
successfully in practice!

## Migration Guide

`EntityHashSet` and `EntityHashMap` are no longer re-exported in
`bevy_ecs::entity` directly. If you were not using `bevy_ecs` / `bevy`'s
`prelude`, you can access them through their now-public modules,
`hash_set` and `hash_map` instead.

## Notes to reviewers

The `EntityHashSet::Iter` type needs to be public for this impl to be
allowed. I initially renamed it to something that wasn't ambiguous and
re-exported it, but as @Victoronz pointed out, that was somewhat
unidiomatic.

In
1a8564898f,
I instead made the `entity_hash_set` public (and its `entity_hash_set`)
sister public, and removed the re-export. I prefer this design (give me
module docs please), but it leads to a lot of churn in this PR.

Let me know which you'd prefer, and if you'd like me to split that
change out into its own micro PR.
2025-01-20 21:26:08 +00:00
Vic
8ac90ac542
make EntityHashMap and EntityHashSet proper types (#16912)
# Objective

`EntityHashMap` and `EntityHashSet` iterators do not implement
`EntitySetIterator`.

## Solution

Make them newtypes instead of aliases. The methods that create the
iterators can then produce their own newtypes that carry the `Hasher`
generic and implement `EntitySetIterator`. Functionality remains the
same otherwise.
There are some other small benefits, f.e. the removal of `with_hasher`
associated functions, and the ability to implement more traits
ourselves.

`MainEntityHashMap` and `MainEntityHashSet` are currently left as the
previous type aliases, because supporting general `TrustedEntityBorrow`
hashing is more complex. However, it can also be done.

## Testing

Pre-existing `EntityHashMap` tests.

## Migration Guide

Users of `with_hasher` and `with_capacity_and_hasher` on
`EntityHashMap`/`Set` must now use `new` and `with_capacity`
respectively.
If the non-newtyped versions are required, they can be obtained via
`Deref`, `DerefMut` or `into_inner` calls.
2024-12-20 20:55:45 +00:00