Skip to content

Conversation

@Victoronz
Copy link
Contributor

@Victoronz Victoronz commented Apr 3, 2025

Objective

Newest installment of the #16547 series.

In #18319 we introduced Entity defaults to accomodate the most common use case for these types, however that resulted in the switch of the T and N generics of UniqueEntityArray.
Swapping generics might be somewhat acceptable for UniqueEntityArray, it is not at all acceptable for map and set types, which we would make generic over T: EntityEquivalent in #18408.

Leaving these defaults in place would result in a glaring inconsistency between these set collections and the others.

Additionally, the current standard in the engine is for "entity" to mean Entity. APIs could be changed to accept EntityEquivalent, however that is a separate and contentious discussion.

Solution

Name these set collections UniqueEntityEquivalent*, and retain the UniqueEntity* name for an alias of the Entity case.
While more verbose, this allows for all generics to be in proper order, full consistency between all set types*, and the "entity" name to be restricted to Entity.
On top of that, UniqueEntity* now always have 1 generic less, when previously this was not enforced for the default case.

*UniqueEntityIter<I: Iterator<T: EntityEquivalent>> is the sole exception to this. Aliases are unable to enforce bounds (lazy_type_alias is needed for this), so for this type, doing this split would be a mere suggestion, and in no way enforced.
Iterator types are rarely ever named, and this specific one is intended to be aliased when it sees more use, like we do for the corresponding set collection iterators.
Furthermore, the EntityEquivalent precursor Borrow<Entity> was used exactly because of such iterator bounds!
Because of that, we leave it as is.

While no migration guide for 0.15 users, for those that upgrade from main:
UniqueEntityVec<T> -> UniqueEntityEquivalentVec<T>
UniqueEntitySlice<T> -> UniqueEntityEquivalentSlice<T>
UniqueEntityArray<N, T> -> UniqueEntityEquivalentArray<T, N>

@Victoronz Victoronz added C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-ECS Entities, components, systems, and events labels Apr 3, 2025
@Victoronz
Copy link
Contributor Author

This impacts how people learn about this feature, and would be seen as a regression if done after the fact, so I think this should be done in 0.16.

@Victoronz Victoronz added this to the 0.16 milestone Apr 3, 2025
@Victoronz Victoronz requested a review from chescock April 3, 2025 01:23
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 3, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 3, 2025
Merged via the queue into bevyengine:main with commit 6a7fc9c Apr 3, 2025
34 checks passed
mockersf pushed a commit that referenced this pull request Apr 3, 2025
# Objective

Newest installment of the #16547 series.

In #18319 we introduced `Entity` defaults to accomodate the most common
use case for these types, however that resulted in the switch of the `T`
and `N` generics of `UniqueEntityArray`.
Swapping generics might be somewhat acceptable for `UniqueEntityArray`,
it is not at all acceptable for map and set types, which we would make
generic over `T: EntityEquivalent` in #18408.

Leaving these defaults in place would result in a glaring inconsistency
between these set collections and the others.

Additionally, the current standard in the engine is for "entity" to mean
`Entity`. APIs could be changed to accept `EntityEquivalent`, however
that is a separate and contentious discussion.

## Solution

Name these set collections `UniqueEntityEquivalent*`, and retain the
`UniqueEntity*` name for an alias of the `Entity` case.
While more verbose, this allows for all generics to be in proper order,
full consistency between all set types*, and the "entity" name to be
restricted to `Entity`.
On top of that, `UniqueEntity*` now always have 1 generic less, when
previously this was not enforced for the default case.

*`UniqueEntityIter<I: Iterator<T: EntityEquivalent>>` is the sole
exception to this. Aliases are unable to enforce bounds
(`lazy_type_alias` is needed for this), so for this type, doing this
split would be a mere suggestion, and in no way enforced.
Iterator types are rarely ever named, and this specific one is intended
to be aliased when it sees more use, like we do for the corresponding
set collection iterators.
Furthermore, the `EntityEquivalent` precursor `Borrow<Entity>` was used
exactly because of such iterator bounds!
Because of that, we leave it as is.

While no migration guide for 0.15 users, for those that upgrade from
main:
`UniqueEntityVec<T>` -> `UniqueEntityEquivalentVec<T>`
`UniqueEntitySlice<T>` -> `UniqueEntityEquivalentSlice<T>`
`UniqueEntityArray<N, T>` -> `UniqueEntityEquivalentArray<T, N>`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants