impl Reflect for std::collections::HashMap instead of only bevy::utils::HashMap
#7739
Labels
A-Reflection
Runtime information about types
C-Bug
An unexpected or incorrect behavior
D-Trivial
Nice and easy! A great choice to get started with Bevy
Comments
PROMETHIA-27
added
C-Bug
|
Wonder if we could just make the implementation generic over the Hasher provided the entire HashMap meets certain trait requirements. |
PROMETHIA-27
added
the
D-Trivial
|
I'd be interested in having a go at this, if its still free |
|
Should still be! Feel free to file a PR whenever you're ready. |
13ros27
added a commit
to 13ros27/bevy
that referenced
this issue
Feb 21, 2023
…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.
13ros27
added a commit
to 13ros27/bevy
that referenced
this issue
Feb 21, 2023
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.
bors bot
pushed a commit
that referenced
this issue
Feb 27, 2023
…: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`
Shfty
pushed a commit
to shfty-rust/bevy
that referenced
this issue
Mar 19, 2023
…: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`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I've seen this pop up a few times for people getting used to
Reflectin the discord;the std impls in
bevy_reflectaren't general enough, and only work onbevy::utils::HashMap. This is also the cause of #1044.The text was updated successfully, but these errors were encountered: