Commit graph

3 commits

Author SHA1 Message Date
Gino Valente
cd1737ecca bevy_reflect: Improved documentation (#7148)
# Objective

`bevy_reflect` can be a moderately complex crate to try and understand. It has many moving parts, a handful of gotchas, and a few subtle contracts that aren't immediately obvious to users and even other contributors.

The current README does an okay job demonstrating how the crate can be used. However, the crate's actual documentation should give a better overview of the crate, its inner-workings, and show some of its own examples.

## Solution

Added crate-level documentation that attempts to summarize the main parts of `bevy_reflect` into small sections.

This PR also updates the documentation for:
- `Reflect`
- `FromReflect`
- The reflection subtraits
- Other important types and traits
- The reflection macros (including the derive macros)
- Crate features

### Open Questions

1. ~~Should I update the docs for the Dynamic types? I was originally going to, but I'm getting a little concerned about the size of this PR 😅~~ Decided to not do this in this PR. It'll be better served from its own PR.
2. Should derive macro documentation be moved to the trait itself? This could improve visibility and allow for better doc links, but could also clutter up the trait's documentation (as well as not being on the actual derive macro's documentation).

### TODO

- [ ] ~~Document Dynamic types (?)~~ I think this should be done in a separate PR.
- [x] Document crate features
- [x] Update docs for `GetTypeRegistration`
- [x] Update docs for `TypeRegistration`
- [x] Update docs for `derive_from_reflect`
- [x] Document `reflect_trait`
- [x] Document `impl_reflect_value`
- [x] Document `impl_from_reflect_value`

---

## Changelog

- Updated documentation across the `bevy_reflect` crate
- Removed `#[module]` helper attribute for `Reflect` derives (this is not currently used)

## Migration Guide

- Removed `#[module]` helper attribute for `Reflect` derives. If your code is relying on this attribute, please replace it with either `#[reflect]` or `#[reflect_value]` (dependent on use-case).


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2023-02-18 20:42:01 +00:00
Gino Valente
229d6c686f bevy_reflect: Simplify take-or-else-from_reflect operation (#6566)
# Objective

There are times where we want to simply take an owned `dyn Reflect` and cast it to a type `T`.

Currently, this involves doing:

```rust
let value = value.take::<T>().unwrap_or_else(|value| {
  T::from_reflect(&*value).unwrap_or_else(|| {
    panic!(
      "expected value of type {} to convert to type {}.",
      value.type_name(),
      std::any::type_name::<T>()
    )
  })
});
```

This is a common operation that could be easily be simplified.

## Solution

Add the `FromReflect::take_from_reflect` method. This first tries to `take` the value, calling `from_reflect` iff that fails.

```rust
let value = T::take_from_reflect(value).unwrap_or_else(|value| {
  panic!(
    "expected value of type {} to convert to type {}.",
    value.type_name(),
    std::any::type_name::<T>()
  )
});
```

Based on suggestion from @soqb on [Discord](https://discord.com/channels/691052431525675048/1002362493634629796/1041046880316043374).

---

## Changelog

- Add `FromReflect::take_from_reflect` method
2023-01-11 16:25:37 +00:00
Gino Valente
63f1a9dec8 bevy_reflect: Add ReflectFromReflect (v2) (#6245)
# Objective

Resolves #4597 (based on the work from #6056 and a refresh of #4147)

When using reflection, we may often end up in a scenario where we have a Dynamic representing a certain type. Unfortunately, we can't just call `MyType::from_reflect` as we do not have knowledge of the concrete type (`MyType`) at runtime.

Such scenarios happen when we call `Reflect::clone_value`, use the reflection deserializers, or create the Dynamic type ourselves.

## Solution

Add a `ReflectFromReflect` type data struct.

This struct allows us to easily convert Dynamic representations of our types into their respective concrete instances.

```rust
#[derive(Reflect, FromReflect)]
#[reflect(FromReflect)] // <- Register `ReflectFromReflect`
struct MyStruct(String);

let type_id = TypeId::of::<MyStruct>();

// Register our type
let mut registry = TypeRegistry::default();
registry.register::<MyStruct>();

// Create a concrete instance
let my_struct = MyStruct("Hello world".to_string());

// `Reflect::clone_value` will generate a `DynamicTupleStruct` for tuple struct types
let dynamic_value: Box<dyn Reflect> = my_struct.clone_value();
assert!(!dynamic_value.is::<MyStruct>());

// Get the `ReflectFromReflect` type data from the registry
let rfr: &ReflectFromReflect = registry
  .get_type_data::<ReflectFromReflect>(type_id)
  .unwrap();

// Call `FromReflect::from_reflect` on our Dynamic value
let concrete_value: Box<dyn Reflect> = rfr.from_reflect(&dynamic_value);
assert!(concrete_value.is::<MyStruct>());
```

### Why this PR?

###### Why now?

The three main reasons I closed #4147 were that:

1. Registering `ReflectFromReflect` is clunky (deriving `FromReflect` *and* registering `ReflectFromReflect`)
2. The ecosystem and Bevy itself didn't seem to pay much attention to deriving `FromReflect`
3. I didn't see a lot of desire from the community for such a feature

However, as time has passed it seems 2 and 3 are not really true anymore. Bevy is internally adding lots more `FromReflect` derives, which should make this feature all the more useful. Additionally, I have seen a growing number of people look for something like `ReflectFromReflect`.

I think 1 is still an issue, but not a horrible one. Plus it could be made much, much better using #6056. And I think splitting this feature out of #6056 could lead to #6056 being adopted sooner (or at least make the need more clear to users).

###### Why not just re-open #4147?

The main reason is so that this PR can garner more attention than simply re-opening the old one. This helps bring fresh eyes to the PR for potentially more perspectives/reviews.

---

## Changelog

* Added `ReflectFromReflect`

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2022-12-11 17:52:48 +00:00