hashing error in bevy_reflect now includes the type (bevyengine#13646) (#13691)

# Objective
If you try to add an object to the hashmap that is not capable of
hashing, the program panics. For easier debugging, the type for that
object should be included in the error message.

Fixes #13646.

## Solution
initially i tried calling std::any::type_name_of_val. this had the
problem that it would print something like dyn Box<dyn Reflect>, not
helpful. But since these objects all implement Reflect, i used
Reflect::type_path() instead. Previously, the error message was part of
a constant called HASH_ERROR. i changed that to a macro called
hash_error to print the type of that object more easily

## Testing
i adapted the unit test reflect_map_no_hash to expect the type in that
panic aswell

since this is my first contribution, please let me know if i have done
everything properly
This commit is contained in:
Wuketuke 2024-06-05 19:41:23 +00:00 committed by GitHub
parent fb3a560a1c
commit 2eb9d5cc38
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 18 additions and 6 deletions

View file

@ -756,7 +756,7 @@ mod tests {
}
#[test]
#[should_panic(expected = "the given key does not support hashing")]
#[should_panic(expected = "the given key bevy_reflect::tests::Foo does not support hashing")]
fn reflect_map_no_hash() {
#[derive(Reflect)]
struct Foo {

View file

@ -194,7 +194,16 @@ impl MapInfo {
}
}
const HASH_ERROR: &str = "the given key does not support hashing";
#[macro_export]
macro_rules! hash_error {
( $key:expr ) => {{
let type_name = match (*$key).get_represented_type_info() {
None => "Unknown",
Some(s) => s.type_path(),
};
format!("the given key {} does not support hashing", type_name).as_str()
}};
}
/// An ordered mapping between reflected values.
#[derive(Default)]
@ -233,13 +242,13 @@ impl DynamicMap {
impl Map for DynamicMap {
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
self.indices
.get(&key.reflect_hash().expect(HASH_ERROR))
.get(&key.reflect_hash().expect(hash_error!(key)))
.map(|index| &*self.values.get(*index).unwrap().1)
}
fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
self.indices
.get(&key.reflect_hash().expect(HASH_ERROR))
.get(&key.reflect_hash().expect(hash_error!(key)))
.cloned()
.map(move |index| &mut *self.values.get_mut(index).unwrap().1)
}
@ -285,7 +294,10 @@ impl Map for DynamicMap {
key: Box<dyn Reflect>,
mut value: Box<dyn Reflect>,
) -> Option<Box<dyn Reflect>> {
match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) {
match self
.indices
.entry(key.reflect_hash().expect(hash_error!(key)))
{
Entry::Occupied(entry) => {
let (_old_key, old_value) = self.values.get_mut(*entry.get()).unwrap();
std::mem::swap(old_value, &mut value);
@ -302,7 +314,7 @@ impl Map for DynamicMap {
fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let index = self
.indices
.remove(&key.reflect_hash().expect(HASH_ERROR))?;
.remove(&key.reflect_hash().expect(hash_error!(key)))?;
let (_key, value) = self.values.remove(index);
Some(value)
}