[Merged by Bors] - impl Reflect for std::collections::HashMap instead of only bevy::utils::HashMap (#7739)
#7782
Conversation
…omState> (bevyengine#7739) This changes the Map, Reflect, Typed, GetTypeRegistration and FromReflect impls for HashMap<K, V> to HashMap<K, V, S> as mentioned in a comment to bevyengine#7739 so that HashMaps with a State type other than RandomState can be reflected.
Using a similar macro to impl_reflect_for_veclike above, Reflect is now implemented for std::collections::HashMap as well as hashbrown::HashMap. Fixes bevyengine#7739.
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
13ros27
changed the title
Reflect for std::collections::HashMap instead of only bevy::utils::HashMapReflect for std::collections::HashMap instead of only bevy::utils::HashMap (#7739)
alice-i-cecile
added
C-Usability
|
If I remove the explicit |
|
I think we should probably stop using hashbrown directly then, I thought all utils was doing was swapping out the hasher, not the entire type. |
|
With a couple of changes to the example (which I think is just due to breaking changes), this also fixes #1044 (makes it compile). Specifically the changes are |
This can probably be done in a separate PR. I think this should at least solve the immediate issue of not being able to use the standard library |
Currently impossible, we're using the unstable/unexposed entry API of hashbrown for performance reasons. It also removes one std dependency on lower level crates that might be no_std compatible (i.e. potentially bevy_ecs). |
james7132
added
the
S-Ready-For-Final-Review
|
bors r+ |
…:utils::HashMap` (#7739) (#7782) # Objective Implement `Reflect` for `std::collections::HashMap<K, V, S>` as well as `hashbrown::HashMap<K, V, S>` rather than just for `hashbrown::HashMap<K, V, RandomState>`. Fixes #7739. ## Solution Rather than implementing on `HashMap<K, V>` I instead implemented most of the related traits on `HashMap<K, V, S> where S: BuildHasher + Send + Sync + 'static` and then `FromReflect` also needs the extra bound `S: Default` because it needs to use `with_capacity_and_hasher` so needs to be able to generate a default hasher. As the API of `hashbrown::HashMap` is identical to `collections::HashMap` making them both work just required creating an `impl_reflect_for_hashmap` macro like the `impl_reflect_for_veclike` above and then applying this to both HashMaps. --- ## Changelog `std::collections::HashMap` can now be reflected. Also more `State` generics than just `RandomState` can now be reflected for both `hashbrown::HashMap` and `collections::HashMap`
|
Build failed: |
|
bors retry |
…:utils::HashMap` (#7739) (#7782) # Objective Implement `Reflect` for `std::collections::HashMap<K, V, S>` as well as `hashbrown::HashMap<K, V, S>` rather than just for `hashbrown::HashMap<K, V, RandomState>`. Fixes #7739. ## Solution Rather than implementing on `HashMap<K, V>` I instead implemented most of the related traits on `HashMap<K, V, S> where S: BuildHasher + Send + Sync + 'static` and then `FromReflect` also needs the extra bound `S: Default` because it needs to use `with_capacity_and_hasher` so needs to be able to generate a default hasher. As the API of `hashbrown::HashMap` is identical to `collections::HashMap` making them both work just required creating an `impl_reflect_for_hashmap` macro like the `impl_reflect_for_veclike` above and then applying this to both HashMaps. --- ## Changelog `std::collections::HashMap` can now be reflected. Also more `State` generics than just `RandomState` can now be reflected for both `hashbrown::HashMap` and `collections::HashMap`
|
Pull request successfully merged into main. Build succeeded:
|
bors
bot
changed the title
Reflect for std::collections::HashMap instead of only bevy::utils::HashMap (#7739)Reflect for std::collections::HashMap instead of only bevy::utils::HashMap (#7739)
…:utils::HashMap` (bevyengine#7739) (bevyengine#7782) # Objective Implement `Reflect` for `std::collections::HashMap<K, V, S>` as well as `hashbrown::HashMap<K, V, S>` rather than just for `hashbrown::HashMap<K, V, RandomState>`. Fixes bevyengine#7739. ## Solution Rather than implementing on `HashMap<K, V>` I instead implemented most of the related traits on `HashMap<K, V, S> where S: BuildHasher + Send + Sync + 'static` and then `FromReflect` also needs the extra bound `S: Default` because it needs to use `with_capacity_and_hasher` so needs to be able to generate a default hasher. As the API of `hashbrown::HashMap` is identical to `collections::HashMap` making them both work just required creating an `impl_reflect_for_hashmap` macro like the `impl_reflect_for_veclike` above and then applying this to both HashMaps. --- ## Changelog `std::collections::HashMap` can now be reflected. Also more `State` generics than just `RandomState` can now be reflected for both `hashbrown::HashMap` and `collections::HashMap`
Objective
Implement
Reflectforstd::collections::HashMap<K, V, S>as well ashashbrown::HashMap<K, V, S>rather than just forhashbrown::HashMap<K, V, RandomState>. Fixes #7739.Solution
Rather than implementing on
HashMap<K, V>I instead implemented most of the related traits onHashMap<K, V, S> where S: BuildHasher + Send + Sync + 'staticand thenFromReflectalso needs the extra boundS: Defaultbecause it needs to usewith_capacity_and_hasherso needs to be able to generate a default hasher.As the API of
hashbrown::HashMapis identical tocollections::HashMapmaking them both work just required creating animpl_reflect_for_hashmapmacro like theimpl_reflect_for_veclikeabove and then applying this to both HashMaps.Changelog
std::collections::HashMapcan now be reflected. Also moreStategenerics than justRandomStatecan now be reflected for bothhashbrown::HashMapandcollections::HashMap