mirror of
https://github.com/bevyengine/bevy
synced 2024-11-10 07:04:33 +00:00
57a175f546
# Objective - Fixes #10629 - `UntypedAssetId` and `AssetId` (along with `UntypedHandle` and `Handle`) do not hash to the same values when pointing to the same `Asset`. Additionally, comparison and conversion between these types does not follow idiomatic Rust standards. ## Solution - Added new unit tests to validate/document expected behaviour - Added trait implementations to make working with Un/Typed values more ergonomic - Ensured hashing and comparison between Un/Typed values is consistent - Removed `From` trait implementations that panic, and replaced them with `TryFrom` --- ## Changelog - Ensured `Handle::<A>::hash` and `UntypedHandle::hash` will produce the same value for the same `Asset` - Added non-panicing `Handle::<A>::try_typed` - Added `PartialOrd` to `UntypedHandle` to match `Handle<A>` (this will return `None` for `UntypedHandles` for differing `Asset` types) - Added `TryFrom<UntypedHandle>` for `Handle<A>` - Added `From<Handle<A>>` for `UntypedHandle` - Removed panicing `From<Untyped...>` implementations. These are currently unused within the Bevy codebase, and shouldn't be used externally, hence removal. - Added cross-implementations of `PartialEq` and `PartialOrd` for `UntypedHandle` and `Handle<A>` allowing direct comparison when `TypeId`'s match. - Near-identical changes to `AssetId` and `UntypedAssetId` ## Migration Guide If you relied on any of the panicing `From<Untyped...>` implementations, simply call the existing `typed` methods instead. Alternatively, use the new `TryFrom` implementation instead to directly expose possible mistakes. ## Notes I've made these changes since `Handle` is such a fundamental type to the entire `Asset` ecosystem within Bevy, and yet it had pretty unclear behaviour with no direct testing. The fact that hashing untyped vs typed versions of the same handle would produce different values is something I expect to cause a very subtle bug for someone else one day. I haven't included it in this PR to avoid any controversy, but I also believe the `typed_unchecked` methods should be removed from these types, or marked as `unsafe`. The `texture_atlas` example currently uses it, and I believe it is a bad choice. The performance gained by not type-checking before conversion would be entirely dwarfed by the act of actually loading an asset and creating its handle anyway. If an end user is in a tight loop repeatedly calling `typed_unchecked` on an `UntypedHandle` for the entire runtime of their application, I think the small performance drop caused by that safety check is ~~a form of cosmic justice~~ reasonable. |
||
|---|---|---|
| .. | ||
| bevy_a11y | ||
| bevy_animation | ||
| bevy_app | ||
| bevy_asset | ||
| bevy_audio | ||
| bevy_core | ||
| bevy_core_pipeline | ||
| bevy_derive | ||
| bevy_diagnostic | ||
| bevy_dylib | ||
| bevy_dynamic_plugin | ||
| bevy_ecs | ||
| bevy_ecs_compile_fail_tests | ||
| bevy_encase_derive | ||
| bevy_gilrs | ||
| bevy_gizmos | ||
| bevy_gltf | ||
| bevy_hierarchy | ||
| bevy_input | ||
| bevy_internal | ||
| bevy_log | ||
| bevy_macro_utils | ||
| bevy_macros_compile_fail_tests | ||
| bevy_math | ||
| bevy_mikktspace | ||
| bevy_pbr | ||
| bevy_ptr | ||
| bevy_reflect | ||
| bevy_reflect_compile_fail_tests | ||
| bevy_render | ||
| bevy_scene | ||
| bevy_sprite | ||
| bevy_tasks | ||
| bevy_text | ||
| bevy_time | ||
| bevy_transform | ||
| bevy_ui | ||
| bevy_utils | ||
| bevy_window | ||
| bevy_winit | ||