use entity set collections type aliases instead of defaults#18695
Merged
alice-i-cecile merged 2 commits intobevyengine:mainfrom Apr 3, 2025
Merged
Conversation
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. |
chescock
approved these changes
Apr 3, 2025
alice-i-cecile
approved these changes
Apr 3, 2025
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>`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Newest installment of the #16547 series.
In #18319 we introduced
Entitydefaults to accomodate the most common use case for these types, however that resulted in the switch of theTandNgenerics ofUniqueEntityArray.Swapping generics might be somewhat acceptable for
UniqueEntityArray, it is not at all acceptable for map and set types, which we would make generic overT: EntityEquivalentin #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 acceptEntityEquivalent, however that is a separate and contentious discussion.Solution
Name these set collections
UniqueEntityEquivalent*, and retain theUniqueEntity*name for an alias of theEntitycase.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_aliasis 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
EntityEquivalentprecursorBorrow<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>