wrap EntityIndexMap/Set slices as well #18134
Conversation
Victoronz
added
A-ECS
| @@ -197,6 +351,471 @@ where | |||
|
|
|||
| impl<V: Eq> Eq for EntityIndexMap<V> {} | |||
|
|
|||
| pub struct Slice<V, S = EntityHash>(PhantomData<S>, map::Slice<Entity, V>); | |||
There was a problem hiding this comment.
Why do you need the S parameter here (and in index_set::Slice)? I don't see any impls that use anything other than the default, so I don't think these can be created or used with any other type parameter.
There was a problem hiding this comment.
It is the same as with the iterator types: Technically not needed yet, but without it, the type alone doesn't convey why it exists!
Later on, when more trusted combinations of Hasher and Hash are introduced (MainEntity + EntityHash f.e.), this generic can carry proper semantic meaning.
| } | ||
| } | ||
|
|
||
| impl<V: PartialOrd> PartialOrd for Slice<V> { |
There was a problem hiding this comment.
Can you #[derive(PartialOrd, Ord, PartialEq, Eq, Hash)] for these? (And similarly for index_set::Slice)
There was a problem hiding this comment.
No, because of the S parameter, a derive would require that generic to implement these traits as well.
The functionality needed to derive here is called "perfect derive".
alice-i-cecile
added
S-Ready-For-Final-Review
Objective
Continuation of #17449.
#17449 implemented the wrapper types around
IndexMap/Setand co., however punted on the slice types.They are needed to support creating
EntitySetIterators from their slices, not just the base maps and sets.Solution
Add the wrappers, in the same vein as #17449 and #17589 before.
The
Index/IndexMutimplementations take up a lot of space, however they cannot be merged because we'd then get overlaps.They are simply named
Sliceto match theindexmapnaming scheme, but this means they cannot be differentiated properly until their modules are made public, which is already a follow-up mentioned in #17954.