Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

bevy_reflect: TypeInfo casting methods #13320

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented May 10, 2024

Objective

There are times when we might know the type of a TypeInfo ahead 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:

if let TypeInfo::List(info) = <Vec<i32>>::type_info() {
  // ...
} else {
  panic!("expected list info");
}

Ideally, there would be a way to simply perform the cast down to ListInfo since 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, TypeInfo now has methods for attempting a cast into the variant's info type.

let info = <Vec<i32>>::type_info().as_list().unwrap();
// ...

These new conversion methods return a Result where the error type is a new TypeInfoError enum.

A Result was chosen as the return type over Option because if we do choose to unwrap it, 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 the else case.

Open Questions

  1. Should the error types instead be a struct? I chose an enum for future-proofing, but right now it only has one error state. Alternatively, we could make it a reflect-wide casting error so it could be used for similar methods on ReflectRef and friends.
  2. I was going to do it in a separate PR but should I just go ahead and add similar methods to ReflectRef, ReflectMut, and ReflectOwned? 馃
  3. Should we name these try_as_*** instead of as_*** since they return a Result?

Testing

You can test locally by running:

cargo test --package bevy_reflect

Changelog

Added

  • TypeInfoError enum
  • TypeInfo::kind method
  • TypeInfo::as_struct method
  • TypeInfo::as_tuple_struct method
  • TypeInfo::as_tuple method
  • TypeInfo::as_list method
  • TypeInfo::as_array method
  • TypeInfo::as_map method
  • TypeInfo::as_enum method
  • TypeInfo::as_value method
  • VariantInfoError enum
  • VariantInfo::variant_type method
  • VariantInfo::as_unit_variant method
  • VariantInfo::as_tuple_variant method
  • VariantInfo::as_struct_variant method

@MrGVSV MrGVSV added C-Enhancement A new feature A-Reflection Runtime information about types D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels May 10, 2024
@MrGVSV MrGVSV force-pushed the mrgvsv/reflect/type-info-casts branch from 145a550 to 4c4302a Compare May 10, 2024 20:35
Copy link
Contributor

@mweatherley mweatherley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

@MrGVSV MrGVSV force-pushed the mrgvsv/reflect/type-info-casts branch from 4c4302a to 004b3b6 Compare May 22, 2024 21:14
@MrGVSV MrGVSV added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Enhancement A new feature D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants