Skip to content

Commit 2eb9d5c

Browse files
authored
hashing error in bevy_reflect now includes the type (#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
1 parent fb3a560 commit 2eb9d5c

File tree

2 files changed

+18
-6
lines changed

2 files changed

+18
-6
lines changed

crates/bevy_reflect/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,7 @@ mod tests {
756756
}
757757

758758
#[test]
759-
#[should_panic(expected = "the given key does not support hashing")]
759+
#[should_panic(expected = "the given key bevy_reflect::tests::Foo does not support hashing")]
760760
fn reflect_map_no_hash() {
761761
#[derive(Reflect)]
762762
struct Foo {

crates/bevy_reflect/src/map.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,16 @@ impl MapInfo {
194194
}
195195
}
196196

197-
const HASH_ERROR: &str = "the given key does not support hashing";
197+
#[macro_export]
198+
macro_rules! hash_error {
199+
( $key:expr ) => {{
200+
let type_name = match (*$key).get_represented_type_info() {
201+
None => "Unknown",
202+
Some(s) => s.type_path(),
203+
};
204+
format!("the given key {} does not support hashing", type_name).as_str()
205+
}};
206+
}
198207

199208
/// An ordered mapping between reflected values.
200209
#[derive(Default)]
@@ -233,13 +242,13 @@ impl DynamicMap {
233242
impl Map for DynamicMap {
234243
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
235244
self.indices
236-
.get(&key.reflect_hash().expect(HASH_ERROR))
245+
.get(&key.reflect_hash().expect(hash_error!(key)))
237246
.map(|index| &*self.values.get(*index).unwrap().1)
238247
}
239248

240249
fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
241250
self.indices
242-
.get(&key.reflect_hash().expect(HASH_ERROR))
251+
.get(&key.reflect_hash().expect(hash_error!(key)))
243252
.cloned()
244253
.map(move |index| &mut *self.values.get_mut(index).unwrap().1)
245254
}
@@ -285,7 +294,10 @@ impl Map for DynamicMap {
285294
key: Box<dyn Reflect>,
286295
mut value: Box<dyn Reflect>,
287296
) -> Option<Box<dyn Reflect>> {
288-
match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) {
297+
match self
298+
.indices
299+
.entry(key.reflect_hash().expect(hash_error!(key)))
300+
{
289301
Entry::Occupied(entry) => {
290302
let (_old_key, old_value) = self.values.get_mut(*entry.get()).unwrap();
291303
std::mem::swap(old_value, &mut value);
@@ -302,7 +314,7 @@ impl Map for DynamicMap {
302314
fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
303315
let index = self
304316
.indices
305-
.remove(&key.reflect_hash().expect(HASH_ERROR))?;
317+
.remove(&key.reflect_hash().expect(hash_error!(key)))?;
306318
let (_key, value) = self.values.remove(index);
307319
Some(value)
308320
}

0 commit comments

Comments
 (0)