Skip to content

bevy_reflect: TypeInfo casting methods#13320

Merged
alice-i-cecile merged 3 commits intobevyengine:mainfrom
MrGVSV:mrgvsv/reflect/type-info-casts
Jul 14, 2024
Merged

bevy_reflect: TypeInfo casting methods#13320
alice-i-cecile merged 3 commits intobevyengine:mainfrom
MrGVSV:mrgvsv/reflect/type-info-casts

Conversation

@MrGVSV
Copy link
Copy Markdown
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-Feature A new feature, making something new possible 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
Copy Markdown
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
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 14, 2024
Merged via the queue into bevyengine:main with commit e512cb6 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>
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-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants