bevy_reflect: TypeInfo casting methods#13320
Merged
alice-i-cecile merged 3 commits intobevyengine:mainfrom Jul 14, 2024
Merged
Conversation
145a550 to
4c4302a
Compare
mweatherley
approved these changes
May 12, 2024
Contributor
mweatherley
left a comment
There was a problem hiding this comment.
Looks good! I don't have strong opinions about how exactly the errors are organized, but I think that the current scope of this PR is good :)
4c4302a to
004b3b6
Compare
alice-i-cecile
approved these changes
Jul 14, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 23, 2024
…e casting methods (#15235) # Objective #13320 added convenience methods for casting a `TypeInfo` into its respective variant: ```rust let info: &TypeInfo = <Vec<i32> as Typed>::type_info(); // We know `info` contains a `ListInfo`, so we can simply cast it: let list_info: &ListInfo = info.as_list().unwrap(); ``` This is especially helpful when you have already verified a type is a certain kind via `ReflectRef`, `ReflectMut`, `ReflectOwned`, or `ReflectKind`. As mentioned in that PR, though, it would be useful to add similar convenience methods to those types as well. ## Solution Added convenience casting methods to `ReflectRef`, `ReflectMut`, and `ReflectOwned`. With these methods, I was able to reduce our nesting in certain places throughout the crate. Additionally, I took this opportunity to move these types (and `ReflectKind`) to their own module to help clean up the `reflect` module. ## Testing You can test locally by running: ``` cargo test --package bevy_reflect --all-features ``` --- ## Showcase Convenience methods for casting `ReflectRef`, `ReflectMut`, and `ReflectOwned` into their respective variants has been added! This allows you to write cleaner code if you already know the kind of your reflected data: ```rust // BEFORE let ReflectRef::List(list) = list.reflect_ref() else { panic!("expected list"); }; // AFTER let list = list.reflect_ref().as_list().unwrap(); ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
This was referenced Oct 29, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
There are times when we might know the type of a
TypeInfoahead of time. Or we may have already checked it one way or another.In such cases, it's a bit cumbersome to have to pattern match every time we want to access the nested info:
Ideally, there would be a way to simply perform the cast down to
ListInfosince we already know it will succeed.Or even if we don't, perhaps we just want a cleaner way of exiting a function early (i.e. with the
?operator).Solution
Taking a bit from
mirror-mirror,TypeInfonow has methods for attempting a cast into the variant's info type.These new conversion methods return a
Resultwhere the error type is a newTypeInfoErrorenum.A
Resultwas chosen as the return type overOptionbecause if we do choose tounwrapit, the error message will give us some indication of what went wrong. In other words, it can truly replace those instances where we were panicking in theelsecase.Open Questions
ReflectRefand friends.ReflectRef,ReflectMut, andReflectOwned? 🤔try_as_***instead ofas_***since they return aResult?Testing
You can test locally by running:
Changelog
Added
TypeInfoErrorenumTypeInfo::kindmethodTypeInfo::as_structmethodTypeInfo::as_tuple_structmethodTypeInfo::as_tuplemethodTypeInfo::as_listmethodTypeInfo::as_arraymethodTypeInfo::as_mapmethodTypeInfo::as_enummethodTypeInfo::as_valuemethodVariantInfoErrorenumVariantInfo::variant_typemethodVariantInfo::as_unit_variantmethodVariantInfo::as_tuple_variantmethodVariantInfo::as_struct_variantmethod