previously I worked on fixing issue #13646, back when the error message did not include the type at all. But that error message had room for improvement, so I included the feedback of @alice-i-cecile and @MrGVSV. The error message will now read `the given key (of type bevy_reflect::tests::Foo) does not support hashing` or 'the given key (of type bevy_reflect::DynamicStruct) does not support hashing' in case of a dynamic struct that represents a hashable struct i also added a new unit test for this new behaviour (`reflect_map_no_hash_dynamic`). Fixes #13646 (again) --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
This commit is contained in:
		
							parent
							
								
									b17292f9d1
								
							
						
					
					
						commit
						d45bcfd043
					
				| @ -596,6 +596,7 @@ mod tests { | |||||||
|         any::TypeId, |         any::TypeId, | ||||||
|         borrow::Cow, |         borrow::Cow, | ||||||
|         fmt::{Debug, Formatter}, |         fmt::{Debug, Formatter}, | ||||||
|  |         hash::Hash, | ||||||
|         marker::PhantomData, |         marker::PhantomData, | ||||||
|     }; |     }; | ||||||
| 
 | 
 | ||||||
| @ -756,7 +757,9 @@ mod tests { | |||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     #[test] |     #[test] | ||||||
|     #[should_panic(expected = "the given key bevy_reflect::tests::Foo does not support hashing")] |     #[should_panic(
 | ||||||
|  |         expected = "the given key of type `bevy_reflect::tests::Foo` does not support hashing" | ||||||
|  |     )] | ||||||
|     fn reflect_map_no_hash() { |     fn reflect_map_no_hash() { | ||||||
|         #[derive(Reflect)] |         #[derive(Reflect)] | ||||||
|         struct Foo { |         struct Foo { | ||||||
| @ -764,11 +767,50 @@ mod tests { | |||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         let foo = Foo { a: 1 }; |         let foo = Foo { a: 1 }; | ||||||
|  |         assert!(foo.reflect_hash().is_none()); | ||||||
| 
 | 
 | ||||||
|         let mut map = DynamicMap::default(); |         let mut map = DynamicMap::default(); | ||||||
|         map.insert(foo, 10u32); |         map.insert(foo, 10u32); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     #[test] | ||||||
|  |     #[should_panic(
 | ||||||
|  |         expected = "the dynamic type `bevy_reflect::DynamicStruct` (representing `bevy_reflect::tests::Foo`) does not support hashing" | ||||||
|  |     )] | ||||||
|  |     fn reflect_map_no_hash_dynamic_representing() { | ||||||
|  |         #[derive(Reflect, Hash)] | ||||||
|  |         #[reflect(Hash)] | ||||||
|  |         struct Foo { | ||||||
|  |             a: u32, | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         let foo = Foo { a: 1 }; | ||||||
|  |         assert!(foo.reflect_hash().is_some()); | ||||||
|  |         let dynamic = foo.clone_dynamic(); | ||||||
|  | 
 | ||||||
|  |         let mut map = DynamicMap::default(); | ||||||
|  |         map.insert(dynamic, 11u32); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     #[test] | ||||||
|  |     #[should_panic(
 | ||||||
|  |         expected = "the dynamic type `bevy_reflect::DynamicStruct` does not support hashing" | ||||||
|  |     )] | ||||||
|  |     fn reflect_map_no_hash_dynamic() { | ||||||
|  |         #[derive(Reflect, Hash)] | ||||||
|  |         #[reflect(Hash)] | ||||||
|  |         struct Foo { | ||||||
|  |             a: u32, | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         let mut dynamic = DynamicStruct::default(); | ||||||
|  |         dynamic.insert("a", 4u32); | ||||||
|  |         assert!(dynamic.reflect_hash().is_none()); | ||||||
|  | 
 | ||||||
|  |         let mut map = DynamicMap::default(); | ||||||
|  |         map.insert(dynamic, 11u32); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     #[test] |     #[test] | ||||||
|     fn reflect_ignore() { |     fn reflect_ignore() { | ||||||
|         #[derive(Reflect)] |         #[derive(Reflect)] | ||||||
|  | |||||||
| @ -197,12 +197,26 @@ impl MapInfo { | |||||||
| #[macro_export] | #[macro_export] | ||||||
| macro_rules! hash_error { | macro_rules! hash_error { | ||||||
|     ( $key:expr ) => {{ |     ( $key:expr ) => {{ | ||||||
|         let type_name = match (*$key).get_represented_type_info() { |         let type_path = (*$key).reflect_type_path(); | ||||||
|             None => "Unknown", |         if !$key.is_dynamic() { | ||||||
|             Some(s) => s.type_path(), |             format!( | ||||||
|         }; |                 "the given key of type `{}` does not support hashing", | ||||||
|         format!("the given key {} does not support hashing", type_name).as_str() |                 type_path | ||||||
|     }}; |             ) | ||||||
|  |         } else { | ||||||
|  |             match (*$key).get_represented_type_info() { | ||||||
|  |                 // Handle dynamic types that do not represent a type (i.e a plain `DynamicStruct`):
 | ||||||
|  |                 None => format!("the dynamic type `{}` does not support hashing", type_path), | ||||||
|  |                 // Handle dynamic types that do represent a type (i.e. a `DynamicStruct` proxying `Foo`):
 | ||||||
|  |                 Some(s) => format!( | ||||||
|  |                     "the dynamic type `{}` (representing `{}`) does not support hashing", | ||||||
|  |                     type_path, | ||||||
|  |                     s.type_path() | ||||||
|  |                 ), | ||||||
|  |             } | ||||||
|  |         } | ||||||
|  |         .as_str() | ||||||
|  |     }} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// An ordered mapping between reflected values.
 | /// An ordered mapping between reflected values.
 | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Wuketuke
						Wuketuke