Commit Graph

32 Commits

Author SHA1 Message Date
Gino Valente
0265436fff
bevy_reflect: Rename UntypedReflectDeserializer to ReflectDeserializer (#12721)
# Objective

We have `ReflectSerializer` and `TypedReflectSerializer`. The former is
the one users will most often use since the latter takes a bit more
effort to deserialize.

However, our deserializers are named `UntypedReflectDeserializer` and
`TypedReflectDeserializer`. There is no obvious indication that
`UntypedReflectDeserializer` must be used with `ReflectSerializer` since
the names don't quite match up.

## Solution

Rename `UntypedReflectDeserializer` back to `ReflectDeserializer`
(initially changed as part of #5723).

Also update the docs for both deserializers (as they were pretty out of
date) and include doc examples.

I also updated the docs for the serializers, too, just so that
everything is consistent.

---

## Changelog

- Renamed `UntypedReflectDeserializer` to `ReflectDeserializer`
- Updated docs for `ReflectDeserializer`, `TypedReflectDeserializer`,
`ReflectSerializer`, and `TypedReflectSerializer`

## Migration Guide

`UntypedReflectDeserializer` has been renamed to `ReflectDeserializer`.
Usages will need to be updated accordingly.

```diff
- let reflect_deserializer = UntypedReflectDeserializer::new(&registry);
+ let reflect_deserializer = ReflectDeserializer::new(&registry);
```
2024-03-26 19:58:29 +00:00
Peter Hayman
d3e44325b4
Fix: deserialize DynamicEnum using index (#12464)
# Objective

- Addresses #12462
- When we serialize an enum, deserialize it, then reserialize it, the
correct variant should be selected.

## Solution

- Change `dynamic_enum.set_variant` to
`dynamic_enum.set_variant_with_index` in `EnumVisitor`
2024-03-14 05:15:20 +00:00
Gino Valente
4c47e31be6
bevy_reflect: Remove U32Visitor (#12433)
# Objective

The `U32Visitor` struct has been unused since its introduction in #6140.
It's made itself known now by causing a recent [CI
failure](https://github.com/bevyengine/bevy/actions/runs/8243333274/job/22543736624).

## Solution

Remove the unused `U32Visitor` struct.

Also removed `PrepassLightsViewFlush` as it was causing a [similar CI
failure](https://github.com/bevyengine/bevy/actions/runs/8243838066/job/22545103746?pr=12433#step:6:269)
on this PR.
2024-03-12 06:19:29 +00:00
radiish
2b7a3b2a55
reflect: treat proxy types correctly when serializing (#12024)
# Objective

- Fixes #12001.
- Note this PR doesn't change any feature flags, however flaky the issue
revealed they are.

## Solution

- Use `FromReflect` to convert proxy types to concrete ones in
`ReflectSerialize::get_serializable`.
- Use `get_represented_type_info() -> type_id()` to get the correct type
id to interact with the registry in
`bevy_reflect::serde::ser::get_serializable`.

---

## Changelog

- Registering `ReflectSerialize` now imposes additional `FromReflect`
and `TypePath` bounds.

## Migration Guide

- If `ReflectSerialize` is registered on a type, but `TypePath` or
`FromReflect` implementations are omitted (perhaps by
`#[reflect(type_path = false)` or `#[reflect(from_reflect = false)]`),
the traits must now be implemented.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-02-26 16:13:04 +00:00
eri
5f8f3b532c
Check cfg during CI and fix feature typos (#12103)
# Objective

- Add the new `-Zcheck-cfg` checks to catch more warnings
- Fixes #12091

## Solution

- Create a new `cfg-check` to the CI that runs `cargo check -Zcheck-cfg
--workspace` using cargo nightly (and fails if there are warnings)
- Fix all warnings generated by the new check

---

## Changelog

- Remove all redundant imports
- Fix cfg wasm32 targets
- Add 3 dead code exceptions (should StandardColor be unused?)
- Convert ios_simulator to a feature (I'm not sure if this is the right
way to do it, but the check complained before)

## Migration Guide

No breaking changes

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-02-25 15:19:27 +00:00
Doonv
189ceaf0d3
Replace or document ignored doctests (#11040)
# Objective

There are a lot of doctests that are `ignore`d for no documented reason.
And that should be fixed.

## Solution

I searched the bevy repo with the regex ` ```[a-z,]*ignore ` in order to
find all `ignore`d doctests. For each one of the `ignore`d doctests, I
did the following steps:
1. Attempt to remove the `ignored` attribute while still passing the
test. I did this by adding hidden dummy structs and imports.
2. If step 1 doesn't work, attempt to replace the `ignored` attribute
with the `no_run` attribute while still passing the test.
3. If step 2 doesn't work, keep the `ignored` attribute but add
documentation for why the `ignored` attribute was added.

---------

Co-authored-by: François <mockersf@gmail.com>
2024-01-01 16:50:56 +00:00
Tygyh
1568d4a415
Reorder impl to be the same as the trait (#11076)
# Objective

- Make the implementation order consistent between all sources to fit
the order in the trait.

## Solution

- Change the implementation order.
2023-12-24 17:43:55 +00:00
tygyh
fd308571c4
Remove unnecessary path prefixes (#10749)
# Objective

- Shorten paths by removing unnecessary prefixes

## Solution

- Remove the prefixes from many paths which do not need them. Finding
the paths was done automatically using built-in refactoring tools in
Jetbrains RustRover.
2023-11-28 23:43:40 +00:00
Hank Jordan
e85af0e366
Fix issue with Option serialization (#10705)
# Objective

- Fix #10499 

## Solution

- Use `.get_represented_type_info()` module path and type ident instead
of `.reflect_*` module path and type ident when serializing the `Option`
enum

---

## Changelog

- Fix serialization bug
- Add simple test
  - Add `serde_json` dev dependency
- Add `serde` with `derive` feature dev dependency (wouldn't compile for
me without it)

---------

Co-authored-by: hank <hank@hank.co.in>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2023-11-23 14:04:51 +00:00
Gino Valente
60773e6787
bevy_reflect: Fix ignored/skipped field order (#7575)
# Objective

Fixes #5101
Alternative to #6511

## Solution

Corrected the behavior for ignored fields in `FromReflect`, which was
previously using the incorrect field indexes.

Similarly, fields marked with `#[reflect(skip_serializing)]` no longer
break when using `FromReflect` after deserialization. This was done by
modifying `SerializationData` to store a function pointer that can later
be used to generate a default instance of the skipped field during
deserialization.

The function pointer points to a function generated by the derive macro
using the behavior designated by `#[reflect(default)]` (or just
`Default` if none provided). The entire output of the macro is now
wrapped in an [unnamed
constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant)
which keeps this behavior hygienic.

#### Rationale

The biggest downside to this approach is that it requires fields marked
`#[reflect(skip_serializing)]` to provide the ability to create a
default instance— either via a `Default` impl or by specifying a custom
one. While this isn't great, I think it might be justified by the fact
that we really need to create this value when using `FromReflect` on a
deserialized object. And we need to do this _during_ deserialization
because after that (at least for tuples and tuple structs) we lose
information about which field is which: _"is the value at index 1 in
this `DynamicTupleStruct` the actual value for index 1 or is it really
the value for index 2 since index 1 is skippable...?"_

#### Alternatives

An alternative would be to store `Option<Box<dyn Reflect>>` within
`DynamicTuple` and `DynamicTupleStruct` instead of just `Box<dyn
Reflect>`. This would allow us to insert "empty"/"missing" fields during
deserialization, thus saving the positional information of the skipped
fields. However, this may require changing the API of `Tuple` and
`TupleStruct` such that they can account for their dynamic counterparts
returning `None` for a skipped field. In practice this would probably
mean exposing the `Option`-ness of the dynamics onto implementors via
methods like `Tuple::drain` or `TupleStruct::field`.

Personally, I think requiring `Default` would be better than muddying up
the API to account for these special cases. But I'm open to trying out
this other approach if the community feels that it's better.

---

## Changelog

### Public Changes

#### Fixed

- The behaviors of `#[reflect(ignore)]` and
`#[reflect(skip_serializing)]` are no longer dependent on field order

#### Changed

- Fields marked with `#[reflect(skip_serializing)]` now need to either
implement `Default` or specify a custom default function using
`#[reflect(default = "path::to::some_func")]`
- Deserializing a type with fields marked `#[reflect(skip_serializing)]`
will now include that field initialized to its specified default value
- `SerializationData::new` now takes the new `SkippedField` struct along
with the skipped field index
- Renamed `SerializationData::is_ignored_field` to
`SerializationData::is_field_skipped`

#### Added

- Added `SkippedField` struct
- Added methods `SerializationData::generate_default` and
`SerializationData::iter_skipped`

### Internal Changes

#### Changed

- Replaced `members_to_serialization_denylist` and `BitSet<u32>` with
`SerializationDataDef`
- The `Reflect` derive is more hygienic as it now outputs within an
[unnamed
constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant)
- `StructField::index` has been split up into
`StructField::declaration_index` and `StructField::reflection_index`

#### Removed

- Removed `bitset` dependency

## Migration Guide

* Fields marked `#[reflect(skip_serializing)]` now must implement
`Default` or specify a custom default function with `#[reflect(default =
"path::to::some_func")]`
    ```rust
    #[derive(Reflect)]
    struct MyStruct {
      #[reflect(skip_serializing)]
      #[reflect(default = "get_foo_default")]
foo: Foo, // <- `Foo` does not impl `Default` so requires a custom
function
      #[reflect(skip_serializing)]
      bar: Bar, // <- `Bar` impls `Default`
    }
    
    #[derive(Reflect)]
    struct Foo(i32);
    
    #[derive(Reflect, Default)]
    struct Bar(i32);
    
    fn get_foo_default() -> Foo {
      Foo(123)
    }
    ```
* `SerializationData::new` has been changed to expect an iterator of
`(usize, SkippedField)` rather than one of just `usize`
    ```rust
    // BEFORE
    SerializationData::new([0, 3].into_iter());
    
    // AFTER
    SerializationData::new([
      (0, SkippedField::new(field_0_default_fn)),
      (3, SkippedField::new(field_3_default_fn)),
    ].into_iter());
    ```
* `Serialization::is_ignored_field` has been renamed to
`Serialization::is_field_skipped`
* Fields marked `#[reflect(skip_serializing)]` are now included in
deserialization output. This may affect logic that expected those fields
to be absent.
2023-10-22 12:43:31 +00:00
Gino Valente
01b910a148
bevy_reflect: Fix dynamic type serialization (#10103)
# Objective

Fixes #10086

## Solution

Instead of serializing via `DynamicTypePath::reflect_type_path`, now
uses the `TypePath` found on the `TypeInfo` returned by
`Reflect::get_represented_type_info`.

This issue was happening because the dynamic types implement `TypePath`
themselves and do not (and cannot) forward their proxy's `TypePath`
data. The solution was to access the proxy's type information in order
to get the correct `TypePath` data.

## Changed

- The `Debug` impl for `TypePathTable` now includes output for all
fields.
2023-10-16 23:31:16 +00:00
radiish
262846e702
reflect: TypePath part 2 (#8768)
# Objective

- Followup to #7184.
- ~Deprecate `TypeUuid` and remove its internal references.~ No longer
part of this PR.
- Use `TypePath` for the type registry, and (de)serialisation instead of
`std::any::type_name`.
- Allow accessing type path information behind proxies.

## Solution
- Introduce methods on `TypeInfo` and friends for dynamically querying
type path. These methods supersede the old `type_name` methods.
- Remove `Reflect::type_name` in favor of `DynamicTypePath::type_path`
and `TypeInfo::type_path_table`.
- Switch all uses of `std::any::type_name` in reflection, non-debugging
contexts to use `TypePath`.

---

## Changelog

- Added `TypePathTable` for dynamically accessing methods on `TypePath`
through `TypeInfo` and the type registry.
- Removed `type_name` from all `TypeInfo`-like structs.
- Added `type_path` and `type_path_table` methods to all `TypeInfo`-like
structs.
- Removed `Reflect::type_name` in favor of
`DynamicTypePath::reflect_type_path` and `TypeInfo::type_path`.
- Changed the signature of all `DynamicTypePath` methods to return
strings with a static lifetime.

## Migration Guide

- Rely on `TypePath` instead of `std::any::type_name` for all stability
guarantees and for use in all reflection contexts, this is used through
with one of the following APIs:
  - `TypePath::type_path` if you have a concrete type and not a value.
- `DynamicTypePath::reflect_type_path` if you have an `dyn Reflect`
value without a concrete type.
- `TypeInfo::type_path` for use through the registry or if you want to
work with the represented type of a `DynamicFoo`.
  
- Remove `type_name` from manual `Reflect` implementations.
- Use `type_path` and `type_path_table` in place of `type_name` on
`TypeInfo`-like structs.
- Use `get_with_type_path(_mut)` over `get_with_type_name(_mut)`.

## Note to reviewers

I think if anything we were a little overzealous in merging #7184 and we
should take that extra care here.

In my mind, this is the "point of no return" for `TypePath` and while I
think we all agree on the design, we should carefully consider if the
finer details and current implementations are actually how we want them
moving forward.

For example [this incorrect `TypePath` implementation for
`String`](3fea3c6c0b/crates/bevy_reflect/src/impls/std.rs (L90))
(note that `String` is in the default Rust prelude) snuck in completely
under the radar.
2023-10-09 19:33:03 +00:00
Joseph
8eb6ccdd87
Remove useless single tuples and trailing commas (#9720)
# Objective

Title
2023-09-08 21:46:54 +00:00
Gino Valente
aeeb20ec4c
bevy_reflect: FromReflect Ergonomics Implementation (#6056)
# Objective

**This implementation is based on
https://github.com/bevyengine/rfcs/pull/59.**

---

Resolves #4597

Full details and motivation can be found in the RFC, but here's a brief
summary.

`FromReflect` is a very powerful and important trait within the
reflection API. It allows Dynamic types (e.g., `DynamicList`, etc.) to
be formed into Real ones (e.g., `Vec<i32>`, etc.).

This mainly comes into play concerning deserialization, where the
reflection deserializers both return a `Box<dyn Reflect>` that almost
always contain one of these Dynamic representations of a Real type. To
convert this to our Real type, we need to use `FromReflect`.

It also sneaks up in other ways. For example, it's a required bound for
`T` in `Vec<T>` so that `Vec<T>` as a whole can be made `FromReflect`.
It's also required by all fields of an enum as it's used as part of the
`Reflect::apply` implementation.

So in other words, much like `GetTypeRegistration` and `Typed`, it is
very much a core reflection trait.

The problem is that it is not currently treated like a core trait and is
not automatically derived alongside `Reflect`. This makes using it a bit
cumbersome and easy to forget.

## Solution

Automatically derive `FromReflect` when deriving `Reflect`.

Users can then choose to opt-out if needed using the
`#[reflect(from_reflect = false)]` attribute.

```rust
#[derive(Reflect)]
struct Foo;

#[derive(Reflect)]
#[reflect(from_reflect = false)]
struct Bar;

fn test<T: FromReflect>(value: T) {}

test(Foo); // <-- OK
test(Bar); // <-- Panic! Bar does not implement trait `FromReflect`
```

#### `ReflectFromReflect`

This PR also automatically adds the `ReflectFromReflect` (introduced in
#6245) registration to the derived `GetTypeRegistration` impl— if the
type hasn't opted out of `FromReflect` of course.

<details>
<summary><h4>Improved Deserialization</h4></summary>

> **Warning**
> This section includes changes that have since been descoped from this
PR. They will likely be implemented again in a followup PR. I am mainly
leaving these details in for archival purposes, as well as for reference
when implementing this logic again.

And since we can do all the above, we might as well improve
deserialization. We can now choose to deserialize into a Dynamic type or
automatically convert it using `FromReflect` under the hood.

`[Un]TypedReflectDeserializer::new` will now perform the conversion and
return the `Box`'d Real type.

`[Un]TypedReflectDeserializer::new_dynamic` will work like what we have
now and simply return the `Box`'d Dynamic type.

```rust
// Returns the Real type
let reflect_deserializer = UntypedReflectDeserializer::new(&registry);
let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?;

let output: SomeStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?;

// Returns the Dynamic type
let reflect_deserializer = UntypedReflectDeserializer::new_dynamic(&registry);
let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?;

let output: DynamicStruct = reflect_deserializer.deserialize(&mut deserializer)?.take()?;
```

</details>

---

## Changelog

* `FromReflect` is now automatically derived within the `Reflect` derive
macro
* This includes auto-registering `ReflectFromReflect` in the derived
`GetTypeRegistration` impl
* ~~Renamed `TypedReflectDeserializer::new` and
`UntypedReflectDeserializer::new` to
`TypedReflectDeserializer::new_dynamic` and
`UntypedReflectDeserializer::new_dynamic`, respectively~~ **Descoped**
* ~~Changed `TypedReflectDeserializer::new` and
`UntypedReflectDeserializer::new` to automatically convert the
deserialized output using `FromReflect`~~ **Descoped**

## Migration Guide

* `FromReflect` is now automatically derived within the `Reflect` derive
macro. Items with both derives will need to remove the `FromReflect`
one.

  ```rust
  // OLD
  #[derive(Reflect, FromReflect)]
  struct Foo;
  
  // NEW
  #[derive(Reflect)]
  struct Foo;
  ```

If using a manual implementation of `FromReflect` and the `Reflect`
derive, users will need to opt-out of the automatic implementation.

  ```rust
  // OLD
  #[derive(Reflect)]
  struct Foo;
  
  impl FromReflect for Foo {/* ... */}
  
  // NEW
  #[derive(Reflect)]
  #[reflect(from_reflect = false)]
  struct Foo;
  
  impl FromReflect for Foo {/* ... */}
  ```

<details>
<summary><h4>Removed Migrations</h4></summary>

> **Warning**
> This section includes changes that have since been descoped from this
PR. They will likely be implemented again in a followup PR. I am mainly
leaving these details in for archival purposes, as well as for reference
when implementing this logic again.

* The reflect deserializers now perform a `FromReflect` conversion
internally. The expected output of `TypedReflectDeserializer::new` and
`UntypedReflectDeserializer::new` is no longer a Dynamic (e.g.,
`DynamicList`), but its Real counterpart (e.g., `Vec<i32>`).

  ```rust
let reflect_deserializer =
UntypedReflectDeserializer::new_dynamic(&registry);
  let mut deserializer = ron:🇩🇪:Deserializer::from_str(input)?;
  
  // OLD
let output: DynamicStruct = reflect_deserializer.deserialize(&mut
deserializer)?.take()?;
  
  // NEW
let output: SomeStruct = reflect_deserializer.deserialize(&mut
deserializer)?.take()?;
  ```

Alternatively, if this behavior isn't desired, use the
`TypedReflectDeserializer::new_dynamic` and
`UntypedReflectDeserializer::new_dynamic` methods instead:

  ```rust
  // OLD
  let reflect_deserializer = UntypedReflectDeserializer::new(&registry);
  
  // NEW
let reflect_deserializer =
UntypedReflectDeserializer::new_dynamic(&registry);
  ```

</details>

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2023-06-29 01:31:34 +00:00
Giacomo Stevanato
0f4d16aa3c
Don't ignore additional entries in UntypedReflectDeserializerVisitor (#7112)
# Objective

Currently when `UntypedReflectDeserializerVisitor` deserializes a
`Box<dyn Reflect>` it only considers the first entry of the map,
silently ignoring any additional entries. For example the following RON
data:

```json
{
    "f32": 1.23,
    "u32": 1,
}
```

is successfully deserialized as a `f32`, completly ignoring the `"u32":
1` part.

## Solution

`UntypedReflectDeserializerVisitor` was changed to check if any other
key could be deserialized, and in that case returns an error.

---

## Changelog

`UntypedReflectDeserializer` now errors on malformed inputs instead of
silently disgarding additional data.

## Migration Guide

If you were deserializing `Box<dyn Reflect>` values with multiple
entries (i.e. entries other than `"type": { /* fields */ }`) you should
remove them or deserialization will fail.
2023-06-26 19:32:27 +00:00
Gino Valente
75130bd5ec
bevy_reflect: Better proxies (#6971)
# Objective

> This PR is based on discussion from #6601

The Dynamic types (e.g. `DynamicStruct`, `DynamicList`, etc.) act as
both:
1. Dynamic containers which may hold any arbitrary data
2. Proxy types which may represent any other type

Currently, the only way we can represent the proxy-ness of a Dynamic is
by giving it a name.

```rust
// This is just a dynamic container
let mut data = DynamicStruct::default();

// This is a "proxy"
data.set_name(std::any::type_name::<Foo>());
```

This type name is the only way we check that the given Dynamic is a
proxy of some other type. When we need to "assert the type" of a `dyn
Reflect`, we call `Reflect::type_name` on it. However, because we're
only using a string to denote the type, we run into a few gotchas and
limitations.

For example, hashing a Dynamic proxy may work differently than the type
it proxies:

```rust
#[derive(Reflect, Hash)]
#[reflect(Hash)]
struct Foo(i32);

let concrete = Foo(123);
let dynamic = concrete.clone_dynamic();

let concrete_hash = concrete.reflect_hash();
let dynamic_hash = dynamic.reflect_hash();

// The hashes are not equal because `concrete` uses its own `Hash` impl
// while `dynamic` uses a reflection-based hashing algorithm
assert_ne!(concrete_hash, dynamic_hash);
```

Because the Dynamic proxy only knows about the name of the type, it's
unaware of any other information about it. This means it also differs on
`Reflect::reflect_partial_eq`, and may include ignored or skipped fields
in places the concrete type wouldn't.

## Solution

Rather than having Dynamics pass along just the type name of proxied
types, we can instead have them pass around the `TypeInfo`.

Now all Dynamic types contain an `Option<&'static TypeInfo>` rather than
a `String`:

```diff
pub struct DynamicTupleStruct {
-    type_name: String,
+    represented_type: Option<&'static TypeInfo>,
    fields: Vec<Box<dyn Reflect>>,
}
```

By changing `Reflect::get_type_info` to
`Reflect::represented_type_info`, hopefully we make this behavior a
little clearer. And to account for `None` values on these dynamic types,
`Reflect::represented_type_info` now returns `Option<&'static
TypeInfo>`.

```rust
let mut data = DynamicTupleStruct::default();

// Not proxying any specific type
assert!(dyn_tuple_struct.represented_type_info().is_none());

let type_info = <Foo as Typed>::type_info();
dyn_tuple_struct.set_represented_type(Some(type_info));
// Alternatively:
// let dyn_tuple_struct = foo.clone_dynamic();

// Now we're proxying `Foo`
assert!(dyn_tuple_struct.represented_type_info().is_some());
```

This means that we can have full access to all the static type
information for the proxied type. Future work would include
transitioning more static type information (trait impls, attributes,
etc.) over to the `TypeInfo` so it can actually be utilized by Dynamic
proxies.

### Alternatives & Rationale

> **Note** 
> These alternatives were written when this PR was first made using a
`Proxy` trait. This trait has since been removed.

<details>
<summary>View</summary>

#### Alternative: The `Proxy<T>` Approach

I had considered adding something like a `Proxy<T>` type where `T` would
be the Dynamic and would contain the proxied type information.

This was nice in that it allows us to explicitly determine whether
something is a proxy or not at a type level. `Proxy<DynamicStruct>`
proxies a struct. Makes sense.

The reason I didn't go with this approach is because (1) tuples, (2)
complexity, and (3) `PartialReflect`.

The `DynamicTuple` struct allows us to represent tuples at runtime. It
also allows us to do something you normally can't with tuples: add new
fields. Because of this, adding a field immediately invalidates the
proxy (e.g. our info for `(i32, i32)` doesn't apply to `(i32, i32,
NewField)`). By going with this PR's approach, we can just remove the
type info on `DynamicTuple` when that happens. However, with the
`Proxy<T>` approach, it becomes difficult to represent this behavior—
we'd have to completely control how we access data for `T` for each `T`.

Secondly, it introduces some added complexities (aside from the manual
impls for each `T`). Does `Proxy<T>` impl `Reflect`? Likely yes, if we
want to represent it as `dyn Reflect`. What `TypeInfo` do we give it?
How would we forward reflection methods to the inner type (remember, we
don't have specialization)? How do we separate this from Dynamic types?
And finally, how do all this in a way that's both logical and intuitive
for users?

Lastly, introducing a `Proxy` trait rather than a `Proxy<T>` struct is
actually more inline with the [Unique Reflect
RFC](https://github.com/bevyengine/rfcs/pull/56). In a way, the `Proxy`
trait is really one part of the `PartialReflect` trait introduced in
that RFC (it's technically not in that RFC but it fits well with it),
where the `PartialReflect` serves as a way for proxies to work _like_
concrete types without having full access to everything a concrete
`Reflect` type can do. This would help bridge the gap between the
current state of the crate and the implementation of that RFC.

All that said, this is still a viable solution. If the community
believes this is the better path forward, then we can do that instead.
These were just my reasons for not initially going with it in this PR.

#### Alternative: The Type Registry Approach

The `Proxy` trait is great and all, but how does it solve the original
problem? Well, it doesn't— yet!

The goal would be to start moving information from the derive macro and
its attributes to the generated `TypeInfo` since these are known
statically and shouldn't change. For example, adding `ignored: bool` to
`[Un]NamedField` or a list of impls.

However, there is another way of storing this information. This is, of
course, one of the uses of the `TypeRegistry`. If we're worried about
Dynamic proxies not aligning with their concrete counterparts, we could
move more type information to the registry and require its usage.

For example, we could replace `Reflect::reflect_hash(&self)` with
`Reflect::reflect_hash(&self, registry: &TypeRegistry)`.

That's not the _worst_ thing in the world, but it is an ergonomics loss.

Additionally, other attributes may have their own requirements, further
restricting what's possible without the registry. The `Reflect::apply`
method will require the registry as well now. Why? Well because the
`map_apply` function used for the `Reflect::apply` impls on `Map` types
depends on `Map::insert_boxed`, which (at least for `DynamicMap`)
requires `Reflect::reflect_hash`. The same would apply when adding
support for reflection-based diffing, which will require
`Reflect::reflect_partial_eq`.

Again, this is a totally viable alternative. I just chose not to go with
it for the reasons above. If we want to go with it, then we can close
this PR and we can pursue this alternative instead.

#### Downsides

Just to highlight a quick potential downside (likely needs more
investigation): retrieving the `TypeInfo` requires acquiring a lock on
the `GenericTypeInfoCell` used by the `Typed` impls for generic types
(non-generic types use a `OnceBox which should be faster). I am not sure
how much of a performance hit that is and will need to run some
benchmarks to compare against.

</details>

### Open Questions

1. Should we use `Cow<'static, TypeInfo>` instead? I think that might be
easier for modding? Perhaps, in that case, we need to update
`Typed::type_info` and friends as well?
2. Are the alternatives better than the approach this PR takes? Are
there other alternatives?

---

## Changelog

### Changed

- `Reflect::get_type_info` has been renamed to
`Reflect::represented_type_info`
- This method now returns `Option<&'static TypeInfo>` rather than just
`&'static TypeInfo`

### Added

- Added `Reflect::is_dynamic` method to indicate when a type is dynamic
- Added a `set_represented_type` method on all dynamic types

### Removed

- Removed `TypeInfo::Dynamic` (use `Reflect::is_dynamic` instead)
- Removed `Typed` impls for all dynamic types

## Migration Guide

- The Dynamic types no longer take a string type name. Instead, they
require a static reference to `TypeInfo`:

    ```rust
    #[derive(Reflect)]
    struct MyTupleStruct(f32, f32);
    
    let mut dyn_tuple_struct = DynamicTupleStruct::default();
    dyn_tuple_struct.insert(1.23_f32);
    dyn_tuple_struct.insert(3.21_f32);
    
    // BEFORE:
    let type_name = std::any::type_name::<MyTupleStruct>();
    dyn_tuple_struct.set_name(type_name);
    
    // AFTER:
    let type_info = <MyTupleStruct as Typed>::type_info();
    dyn_tuple_struct.set_represented_type(Some(type_info));
    ```

- `Reflect::get_type_info` has been renamed to
`Reflect::represented_type_info` and now also returns an
`Option<&'static TypeInfo>` (instead of just `&'static TypeInfo`):

    ```rust
    // BEFORE:
    let info: &'static TypeInfo = value.get_type_info();
    // AFTER:
let info: &'static TypeInfo = value.represented_type_info().unwrap();
    ```

- `TypeInfo::Dynamic` and `DynamicInfo` has been removed. Use
`Reflect::is_dynamic` instead:
   
    ```rust
    // BEFORE:
    if matches!(value.get_type_info(), TypeInfo::Dynamic) {
      // ...
    }
    // AFTER:
    if value.is_dynamic() {
      // ...
    }
    ```

---------

Co-authored-by: radiish <cb.setho@gmail.com>
2023-04-26 12:17:46 +00:00
张林伟
0d2cdb450d Fix beta clippy lints (#7154)
# Objective

- When I run `cargo run -p ci` for my pr locally using latest beta toolchain, the ci failed due to [uninlined_format_args](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) and [needless_lifetimes](https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes) lints

## Solution

- Fix lints according to clippy suggestions.
2023-01-11 09:51:22 +00:00
Giacomo Stevanato
871c80c103 Add TypeRegistrationDeserializer and remove BorrowedStr (#7094)
# Objective

This a follow-up to #6894, see https://github.com/bevyengine/bevy/pull/6894#discussion_r1045203113

The goal is to avoid cloning any string when getting a `&TypeRegistration` corresponding to a string which is being deserialized. As a bonus code duplication is also reduced.

## Solution

The manual deserialization of a string and lookup into the type registry has been moved into a separate `TypeRegistrationDeserializer` type, which implements `DeserializeSeed` with a `Visitor` that accepts any string with `visit_str`, even ones that may not live longer than that function call.
`BorrowedStr` has been removed since it's no longer used.

---

## Changelog

- The type `TypeRegistrationDeserializer` has been added, which simplifies getting a `&TypeRegistration` while deserializing a string.
2023-01-09 21:57:14 +00:00
Gino Valente
717def2ccf bevy_reflect: Fix deserialization with readers (#6894)
# Objective

Fixes #6891

## Solution

Replaces deserializing map keys as `&str` with deserializing them as `String`.

This bug seems to occur when using something like `File` or `BufReader` rather than bytes or a string directly (I only tested `File` and `BufReader` for `rmp-serde` and `serde_json`). This might be an issue with other `Read` impls as well (except `&[u8]` it seems).

We already had passing tests for Message Pack but none that use a `File` or `BufReader`. This PR also adds or modifies tests to check for this in the future.

This change was also based on [feedback](https://github.com/bevyengine/bevy/pull/4561#discussion_r957385136) I received in a previous PR.

---

## Changelog

- Fix bug where scene deserialization using certain readers could fail (e.g. `BufReader`, `File`, etc.)
2023-01-04 22:03:31 +00:00
Gino Valente
4e2374334f bevy_reflect: Fix binary deserialization not working for unit structs (#6722)
# Objective 

Fixes #6713

Binary deserialization is failing for unit structs as well as structs with all ignored/skipped fields.

## Solution

Add a check for the number of possible fields in a struct before deserializing. If empty, don't attempt to deserialize any fields (as there will be none).

Note: ~~This does not apply to enums as they do not properly handle skipped fields (see #6721).~~ Enums still do not properly handle skipped fields, but I decided to include the logic for it anyways to account for `#[reflect(ignore)]`'d fields in the meantime.

---

## Changelog

- Fix bug where deserializing unit structs would fail for non-self-describing formats
2022-11-23 00:01:36 +00:00
Gino Valente
97f7a1a99c bevy_reflect: Binary formats (#6140)
# Objective

Closes #5934

Currently it is not possible to de/serialize data to non-self-describing formats using reflection.

## Solution

Add support for non-self-describing de/serialization using reflection.

This allows us to use binary formatters, like [`postcard`](https://crates.io/crates/postcard):

```rust
#[derive(Reflect, FromReflect, Debug, PartialEq)]
struct Foo {
  data: String
}

let mut registry = TypeRegistry::new();
registry.register::<Foo>();

let input = Foo {
  data: "Hello world!".to_string()
};

// === Serialize! === //
let serializer = ReflectSerializer::new(&input, &registry);
let bytes: Vec<u8> = postcard::to_allocvec(&serializer).unwrap();

println!("{:?}", bytes); // Output: [129, 217, 61, 98, ...]

// === Deserialize! === //
let deserializer = UntypedReflectDeserializer::new(&registry);

let dynamic_output = deserializer
  .deserialize(&mut postcard::Deserializer::from_bytes(&bytes))
  .unwrap();

let output = <Foo as FromReflect>::from_reflect(dynamic_output.as_ref()).unwrap();

assert_eq!(expected, output); // OK!
```

#### Crates Tested

- ~~[`rmp-serde`](https://crates.io/crates/rmp-serde)~~ Apparently, this _is_ self-describing
- ~~[`bincode` v2.0.0-rc.1](https://crates.io/crates/bincode/2.0.0-rc.1) (using [this PR](https://github.com/bincode-org/bincode/pull/586))~~ This actually works for the latest release (v1.3.3) of [`bincode`](https://crates.io/crates/bincode) as well. You just need to be sure to use fixed-int encoding.
- [`postcard`](https://crates.io/crates/postcard)

## Future Work

Ideally, we would refactor the `serde` module, but I don't think I'll do that in this PR so as to keep the diff relatively small (and to avoid any painful rebases). This should probably be done once this is merged, though.

Some areas we could improve with a refactor:

* Split deserialization logic across multiple files
* Consolidate helper functions/structs
* Make the logic more DRY

---

## Changelog

- Add support for non-self-describing de/serialization using reflection.


Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2022-11-04 02:22:54 +00:00
Jakob Hellermann
e71c4d2802 fix nightly clippy warnings (#6395)
# Objective

- fix new clippy lints before they get stable and break CI

## Solution

- run `clippy --fix` to auto-fix machine-applicable lints
- silence `clippy::should_implement_trait` for `fn HandleId::default<T: Asset>`

## Changes
- always prefer `format!("{inline}")` over `format!("{}", not_inline)`
- prefer `Box::default` (or `Box::<T>::default` if necessary) over `Box::new(T::default())`
2022-10-28 21:03:01 +00:00
Gino Valente
19fc1f1ed2 bevy_reflect: Fix DynamicScene not respecting component registrations during serialization (#6288)
# Objective

When running the scene example, you might notice we end up printing out the following:
```ron
// ...
{
  "scene::ComponentB": (
    value: "hello",
    _time_since_startup: (
      secs: 0,
      nanos: 0,
    ),
  ),
},
// ...
```

We should not be printing out `_time_since_startup` as the field is marked with `#[reflect(skip_serializing)]`:

```rust
#[derive(Component, Reflect)]
#[reflect(Component)]
struct ComponentB {
  pub value: String,
  #[reflect(skip_serializing)]
  pub _time_since_startup: Duration,
}
```

This is because when we create the `DynamicScene`, we end up calling `Reflect::clone_value`:

82126697ee/crates/bevy_scene/src/dynamic_scene_builder.rs (L114-L114)

This results in non-Value types being cloned into Dynamic types, which means the `TypeId` returned from `reflected_value.type_id()` is not the same as the original component's. 

And this meant we were not able to locate the correct `TypeRegistration`.

## Solution

Use `TypeInfo::type_id()` instead of calling `Any::type_id()` on the value directly.

---

## Changelog

* Fix a bug introduced in `0.9.0-dev` where scenes disregarded component's type registrations
2022-10-24 14:53:12 +00:00
Gino Valente
d30d3e752a bevy_reflect: Improve serialization format even more (#5723)
> Note: This is rebased off #4561 and can be viewed as a competitor to that PR. See `Comparison with #4561` section for details.

# Objective

The current serialization format used by `bevy_reflect` is both verbose and error-prone. Taking the following structs[^1] for example:

```rust
// -- src/inventory.rs

#[derive(Reflect)]
struct Inventory {
  id: String,
  max_storage: usize,
  items: Vec<Item>
}

#[derive(Reflect)]
struct Item {
  name: String
}
```

Given an inventory of a single item, this would serialize to something like:

```rust
// -- assets/inventory.ron

{
  "type": "my_game::inventory::Inventory",
  "struct": {
    "id": {
      "type": "alloc::string::String",
      "value": "inv001",
    },
    "max_storage": {
      "type": "usize",
      "value": 10
    },
    "items": {
      "type": "alloc::vec::Vec<alloc::string::String>",
      "list": [
        {
          "type": "my_game::inventory::Item",
          "struct": {
            "name": {
              "type": "alloc::string::String",
              "value": "Pickaxe"
            },
          },
        },
      ],
    },
  },
}
```

Aside from being really long and difficult to read, it also has a few "gotchas" that users need to be aware of if they want to edit the file manually. A major one is the requirement that you use the proper keys for a given type. For structs, you need `"struct"`. For lists, `"list"`. For tuple structs, `"tuple_struct"`. And so on.

It also ***requires*** that the `"type"` entry come before the actual data. Despite being a map— which in programming is almost always orderless by default— the entries need to be in a particular order. Failure to follow the ordering convention results in a failure to deserialize the data.

This makes it very prone to errors and annoyances.


## Solution

Using #4042, we can remove a lot of the boilerplate and metadata needed by this older system. Since we now have static access to type information, we can simplify our serialized data to look like:

```rust
// -- assets/inventory.ron

{
  "my_game::inventory::Inventory": (
    id: "inv001",
    max_storage: 10,
    items: [
      (
        name: "Pickaxe"
      ),
    ],
  ),
}
```

This is much more digestible and a lot less error-prone (no more key requirements and no more extra type names).

Additionally, it is a lot more familiar to users as it follows conventional serde mechanics. For example, the struct is represented with `(...)` when serialized to RON.

#### Custom Serialization

Additionally, this PR adds the opt-in ability to specify a custom serde implementation to be used rather than the one created via reflection. For example[^1]:

```rust
// -- src/inventory.rs

#[derive(Reflect, Serialize)]
#[reflect(Serialize)]
struct Item {
  #[serde(alias = "id")]
  name: String
}
```

```rust
// -- assets/inventory.ron

{
  "my_game::inventory::Inventory": (
    id: "inv001",
    max_storage: 10,
    items: [
      (
        id: "Pickaxe"
      ),
    ],
  ),
},
```

By allowing users to define their own serialization methods, we do two things:

1. We give more control over how data is serialized/deserialized to the end user
2. We avoid having to re-define serde's attributes and forcing users to apply both (e.g. we don't need a `#[reflect(alias)]` attribute).

### Improved Formats

One of the improvements this PR provides is the ability to represent data in ways that are more conventional and/or familiar to users. Many users are familiar with RON so here are some of the ways we can now represent data in RON:

###### Structs

```js
{
  "my_crate::Foo": (
    bar: 123
  )
}
// OR
{
  "my_crate::Foo": Foo(
    bar: 123
  )
}
```

<details>
<summary>Old Format</summary>

```js
{
  "type": "my_crate::Foo",
  "struct": {
    "bar": {
      "type": "usize",
      "value": 123
    }
  }
}
```

</details>

###### Tuples

```js
{
  "(f32, f32)": (1.0, 2.0)
}
```

<details>
<summary>Old Format</summary>

```js
{
  "type": "(f32, f32)",
  "tuple": [
    {
      "type": "f32",
      "value": 1.0
    },
    {
      "type": "f32",
      "value": 2.0
    }
  ]
}
```

</details>

###### Tuple Structs

```js
{
  "my_crate::Bar": ("Hello World!")
}
// OR
{
  "my_crate::Bar": Bar("Hello World!")
}
```

<details>
<summary>Old Format</summary>

```js
{
  "type": "my_crate::Bar",
  "tuple_struct": [
    {
      "type": "alloc::string::String",
      "value": "Hello World!"
    }
  ]
}
```

</details>

###### Arrays

It may be a bit surprising to some, but arrays now also use the tuple format. This is because they essentially _are_ tuples (a sequence of values with a fixed size), but only allow for homogenous types. Additionally, this is how RON handles them and is probably a result of the 32-capacity limit imposed on them (both by [serde](https://docs.rs/serde/latest/serde/trait.Serialize.html#impl-Serialize-for-%5BT%3B%2032%5D) and by [bevy_reflect](https://docs.rs/bevy/latest/bevy/reflect/trait.GetTypeRegistration.html#impl-GetTypeRegistration-for-%5BT%3B%2032%5D)).

```js
{
  "[i32; 3]": (1, 2, 3)
}
```

<details>
<summary>Old Format</summary>

```js
{
  "type": "[i32; 3]",
  "array": [
    {
      "type": "i32",
      "value": 1
    },
    {
      "type": "i32",
      "value": 2
    },
    {
      "type": "i32",
      "value": 3
    }
  ]
}
```

</details>

###### Enums

To make things simple, I'll just put a struct variant here, but the style applies to all variant types:

```js
{
  "my_crate::ItemType": Consumable(
    name: "Healing potion"
  )
}
```

<details>
<summary>Old Format</summary>

```js
{
  "type": "my_crate::ItemType",
  "enum": {
    "variant": "Consumable",
    "struct": {
      "name": {
        "type": "alloc::string::String",
        "value": "Healing potion"
      }
    }
  }
}
```

</details>

### Comparison with #4561

This PR is a rebased version of #4561. The reason for the split between the two is because this PR creates a _very_ different scene format. You may notice that the PR descriptions for either PR are pretty similar. This was done to better convey the changes depending on which (if any) gets merged first. If #4561 makes it in first, I will update this PR description accordingly.

---

## Changelog

* Re-worked serialization/deserialization for reflected types
* Added `TypedReflectDeserializer` for deserializing data with known `TypeInfo`
* Renamed `ReflectDeserializer` to `UntypedReflectDeserializer` 
* ~~Replaced usages of `deserialize_any` with `deserialize_map` for non-self-describing formats~~ Reverted this change since there are still some issues that need to be sorted out (in a separate PR). By reverting this, crates like `bincode` can throw an error when attempting to deserialize non-self-describing formats (`bincode` results in `DeserializeAnyNotSupported`)
* Structs, tuples, tuple structs, arrays, and enums are now all de/serialized using conventional serde methods

## Migration Guide

* This PR reduces the verbosity of the scene format. Scenes will need to be updated accordingly:

```js
// Old format
{
  "type": "my_game::item::Item",
  "struct": {
    "id": {
      "type": "alloc::string::String",
      "value": "bevycraft:stone",
    },
    "tags": {
      "type": "alloc::vec::Vec<alloc::string::String>",
      "list": [
        {
          "type": "alloc::string::String",
          "value": "material"
        },
      ],
    },
}

// New format
{
  "my_game::item::Item": (
    id: "bevycraft:stone",
    tags: ["material"]
  )
}
```

[^1]: Some derives omitted for brevity.
2022-09-20 19:38:18 +00:00
Maksymilian Mozolewski
ac1aebed5e Add reflect(skip_serializing) which retains reflection but disables automatic serialization (#5250)
# Objective

- To address problems outlined in https://github.com/bevyengine/bevy/issues/5245

## Solution

- Introduce `reflect(skip_serializing)` on top of `reflect(ignore)` which disables automatic serialisation to scenes, but does not disable reflection of the field.

---

## Changelog
- Adds: 
  - `bevy_reflect::serde::type_data` module
  - `SerializationData` structure for describing which fields are to be/not to be ignored, automatically registers as type_data for struct-based types
  - the `skip_serialization` flag for `#[reflect(...)]`
 - Removes:
   - ability to ignore Enum variants in serialization, since that didn't work anyway   
 

## Migration Guide
- Change `#[reflect(ignore)]` to `#[reflect(skip_serializing)]` where disabling reflection is not the intended effect.
- Remove ignore/skip attributes from enum variants as these won't do anything anymore
2022-09-19 16:12:10 +00:00
Gino Valente
15826d6019 bevy_reflect: Reflect enums (#4761)
# Objective

> This is a revival of #1347. Credit for the original PR should go to @Davier.

Currently, enums are treated as `ReflectRef::Value` types by `bevy_reflect`. Obviously, there needs to be better a better representation for enums using the reflection API.

## Solution

Based on prior work from @Davier, an `Enum` trait has been added as well as the ability to automatically implement it via the `Reflect` derive macro. This allows enums to be expressed dynamically:

```rust
#[derive(Reflect)]
enum Foo {
  A,
  B(usize),
  C { value: f32 },
}

let mut foo = Foo::B(123);
assert_eq!("B", foo.variant_name());
assert_eq!(1, foo.field_len());

let new_value = DynamicEnum::from(Foo::C { value: 1.23 });
foo.apply(&new_value);
assert_eq!(Foo::C{value: 1.23}, foo);
```

### Features

#### Derive Macro

Use the `#[derive(Reflect)]` macro to automatically implement the `Enum` trait for enum definitions. Optionally, you can use `#[reflect(ignore)]` with both variants and variant fields, just like you can with structs. These ignored items will not be considered as part of the reflection and cannot be accessed via reflection.

```rust
#[derive(Reflect)]
enum TestEnum {
  A,
  // Uncomment to ignore all of `B`
  // #[reflect(ignore)]
  B(usize),
  C {
    // Uncomment to ignore only field `foo` of `C`
    // #[reflect(ignore)]
    foo: f32,
    bar: bool,
  },
}
```

#### Dynamic Enums

Enums may be created/represented dynamically via the `DynamicEnum` struct. The main purpose of this struct is to allow enums to be deserialized into a partial state and to allow dynamic patching. In order to ensure conversion from a `DynamicEnum` to a concrete enum type goes smoothly, be sure to add `FromReflect` to your derive macro.

```rust
let mut value = TestEnum::A;

// Create from a concrete instance
let dyn_enum = DynamicEnum::from(TestEnum::B(123));

value.apply(&dyn_enum);
assert_eq!(TestEnum::B(123), value);

// Create a purely dynamic instance
let dyn_enum = DynamicEnum::new("TestEnum", "A", ());

value.apply(&dyn_enum);
assert_eq!(TestEnum::A, value);
```

#### Variants

An enum value is always represented as one of its variants— never the enum in its entirety.

```rust
let value = TestEnum::A;
assert_eq!("A", value.variant_name());

// Since we are using the `A` variant, we cannot also be the `B` variant
assert_ne!("B", value.variant_name());
```

All variant types are representable within the `Enum` trait: unit, struct, and tuple.

You can get the current type like:

```rust
match value.variant_type() {
  VariantType::Unit => println!("A unit variant!"),
  VariantType::Struct => println!("A struct variant!"),
  VariantType::Tuple => println!("A tuple variant!"),
}
```

> Notice that they don't contain any values representing the fields. These are purely tags.

If a variant has them, you can access the fields as well:

```rust
let mut value = TestEnum::C {
  foo: 1.23,
  bar: false
};

// Read/write specific fields
*value.field_mut("bar").unwrap() = true;

// Iterate over the entire collection of fields
for field in value.iter_fields() {
  println!("{} = {:?}", field.name(), field.value());
}
```

#### Variant Swapping

It might seem odd to group all variant types under a single trait (why allow `iter_fields` on a unit variant?), but the reason this was done ~~is to easily allow *variant swapping*.~~ As I was recently drafting up the **Design Decisions** section, I discovered that other solutions could have been made to work with variant swapping. So while there are reasons to keep the all-in-one approach, variant swapping is _not_ one of them.

```rust
let mut value: Box<dyn Enum> = Box::new(TestEnum::A);
value.set(Box::new(TestEnum::B(123))).unwrap();
```

#### Serialization

Enums can be serialized and deserialized via reflection without needing to implement `Serialize` or `Deserialize` themselves (which can save thousands of lines of generated code). Below are the ways an enum can be serialized.

> Note, like the rest of reflection-based serialization, the order of the keys in these representations is important!

##### Unit

```json
{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "A"
  }
}
```

##### Tuple

```json
{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "B",
    "tuple": [
      {
        "type": "usize",
        "value": 123
      }
    ]
  }
}
```

<details>
<summary>Effects on Option</summary>

This ends up making `Option` look a little ugly:

```json
{
  "type": "core::option::Option<usize>",
  "enum": {
    "variant": "Some",
    "tuple": [
      {
        "type": "usize",
        "value": 123
      }
    ]
  }
}
```


</details>

##### Struct

```json
{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "C",
    "struct": {
      "foo": {
        "type": "f32",
        "value": 1.23
      },
      "bar": {
        "type": "bool",
        "value": false
      }
    }
  }
}
```

## Design Decisions

<details>
<summary><strong>View Section</strong></summary>

This section is here to provide some context for why certain decisions were made for this PR, alternatives that could have been used instead, and what could be improved upon in the future.

### Variant Representation

One of the biggest decisions was to decide on how to represent variants. The current design uses a "all-in-one" design where unit, tuple, and struct variants are all simultaneously represented by the `Enum` trait. This is not the only way it could have been done, though.

#### Alternatives

##### 1. Variant Traits

One way of representing variants would be to define traits for each variant, implementing them whenever an enum featured at least one instance of them. This would allow us to define variants like:

```rust
pub trait Enum: Reflect {
  fn variant(&self) -> Variant;
}

pub enum Variant<'a> {
    Unit,
    Tuple(&'a dyn TupleVariant),
    Struct(&'a dyn StructVariant),
}

pub trait TupleVariant {
  fn field_len(&self) -> usize;
  // ...
}
```

And then do things like:

```rust
fn get_tuple_len(foo: &dyn Enum) -> usize {
  match foo.variant() {
    Variant::Tuple(tuple) => tuple.field_len(),
    _ => panic!("not a tuple variant!")
  }
}
```

The reason this PR does not go with this approach is because of the fact that variants are not separate types. In other words, we cannot implement traits on specific variants— these cover the *entire* enum. This means we offer an easy footgun:

```rust
let foo: Option<i32> = None;
let my_enum = Box::new(foo) as Box<dyn TupleVariant>;
```

Here, `my_enum` contains `foo`, which is a unit variant. However, since we need to implement `TupleVariant` for `Option` as a whole, it's possible to perform such a cast. This is obviously wrong, but could easily go unnoticed. So unfortunately, this makes it not a good candidate for representing variants.

##### 2. Variant Structs

To get around the issue of traits necessarily needing to apply to both the enum and its variants, we could instead use structs that are created on a per-variant basis. This was also considered but was ultimately [[removed](71d27ab3c6) due to concerns about allocations.

 Each variant struct would probably look something like:

```rust
pub trait Enum: Reflect {
  fn variant_mut(&self) -> VariantMut;
}

pub enum VariantMut<'a> {
    Unit,
    Tuple(TupleVariantMut),
    Struct(StructVariantMut),
}

struct StructVariantMut<'a> {
  fields: Vec<&'a mut dyn Reflect>,
  field_indices: HashMap<Cow<'static, str>, usize>
}
```

This allows us to isolate struct variants into their own defined struct and define methods specifically for their use. It also prevents users from casting to it since it's not a trait. However, this is not an optimal solution. Both `field_indices` and `fields` will require an allocation (remember, a `Box<[T]>` still requires a `Vec<T>` in order to be constructed). This *might* be a problem if called frequently enough.

##### 3. Generated Structs

The original design, implemented by @Davier, instead generates structs specific for each variant. So if we had a variant path like `Foo::Bar`, we'd generate a struct named `FooBarWrapper`. This would be newtyped around the original enum and forward tuple or struct methods to the enum with the chosen variant.

Because it involved using the `Tuple` and `Struct` traits (which are also both bound on `Reflect`), this meant a bit more code had to be generated. For a single struct variant with one field, the generated code amounted to ~110LoC. However, each new field added to that variant only added ~6 more LoC.

In order to work properly, the enum had to be transmuted to the generated struct:

```rust
fn variant(&self) -> crate::EnumVariant<'_> {
  match self {
    Foo::Bar {value: i32} => {
      let wrapper_ref = unsafe { 
        std::mem::transmute::<&Self, &FooBarWrapper>(self) 
      };
      crate::EnumVariant::Struct(wrapper_ref as &dyn crate::Struct)
    }
  }
}
```

This works because `FooBarWrapper` is defined as `repr(transparent)`.

Out of all the alternatives, this would probably be the one most likely to be used again in the future. The reasons for why this PR did not continue to use it was because:

* To reduce generated code (which would hopefully speed up compile times)
* To avoid cluttering the code with generated structs not visible to the user
* To keep bevy_reflect simple and extensible (these generated structs act as proxies and might not play well with current or future systems)
* To avoid additional unsafe blocks
* My own misunderstanding of @Davier's code

That last point is obviously on me. I misjudged the code to be too unsafe and unable to handle variant swapping (which it probably could) when I was rebasing it. Looking over it again when writing up this whole section, I see that it was actually a pretty clever way of handling variant representation.

#### Benefits of All-in-One

As stated before, the current implementation uses an all-in-one approach. All variants are capable of containing fields as far as `Enum` is concerned. This provides a few benefits that the alternatives do not (reduced indirection, safer code, etc.).

The biggest benefit, though, is direct field access. Rather than forcing users to have to go through pattern matching, we grant direct access to the fields contained by the current variant. The reason we can do this is because all of the pattern matching happens internally. Getting the field at index `2` will automatically return `Some(...)` for the current variant if it has a field at that index or `None` if it doesn't (or can't).

This could be useful for scenarios where the variant has already been verified or just set/swapped (or even where the type of variant doesn't matter):

```rust
let dyn_enum: &mut dyn Enum = &mut Foo::Bar {value: 123};
// We know it's the `Bar` variant
let field = dyn_enum.field("value").unwrap();
```

Reflection is not a type-safe abstraction— almost every return value is wrapped in `Option<...>`. There are plenty of places to check and recheck that a value is what Reflect says it is. Forcing users to have to go through `match` each time they want to access a field might just be an extra step among dozens of other verification processes.

 Some might disagree, but ultimately, my view is that the benefit here is an improvement to the ergonomics and usability of reflected enums.

</details>

---

## Changelog

### Added

* Added `Enum` trait
* Added `Enum` impl to `Reflect` derive macro
* Added `DynamicEnum` struct
  * Added `DynamicVariant`
* Added `EnumInfo`
  * Added `VariantInfo`
    * Added `StructVariantInfo`
    * Added `TupleVariantInfo`
    * Added `UnitVariantInfo`
* Added serializtion/deserialization support for enums
  * Added `EnumSerializer`

* Added `VariantType`
* Added `VariantFieldIter`
* Added `VariantField`
* Added `enum_partial_eq(...)`
* Added `enum_hash(...)`

### Changed

* `Option<T>` now implements `Enum`
* `bevy_window` now depends on `bevy_reflect`
  * Implemented `Reflect` and `FromReflect` for `WindowId`
* Derive `FromReflect` on `PerspectiveProjection`
* Derive `FromReflect` on `OrthographicProjection`
* Derive `FromReflect` on `WindowOrigin`
* Derive `FromReflect` on `ScalingMode`
* Derive `FromReflect` on `DepthCalculation`


## Migration Guide

* Enums no longer need to be treated as values and usages of `#[reflect_value(...)]` can be removed or replaced by `#[reflect(...)]`
* Enums (including `Option<T>`) now take a different format when serializing. The format is described above, but this may cause issues for existing scenes that make use of enums. 

---

Also shout out to @nicopap for helping clean up some of the code here! It's a big feature so help like this is really appreciated!

Co-authored-by: Gino Valente <gino.valente.code@gmail.com>
2022-08-02 22:14:41 +00:00
James Liu
5498ef81fb bevy_reflect: support map insertion (#5173)
# Objective

This is a rebase of #3701 which is currently scheduled for 0.8 but is marked for adoption.

> Fixes https://github.com/bevyengine/bevy/discussions/3609

## Solution
> - add an `insert_boxed()` method on the `Map` trait
> - implement it for `HashMap` using a new `FromReflect` generic bound
> - add a `map_apply()` helper method to implement `Map::apply()`, that inserts new values instead of ignoring them


---

## Changelog
TODO

Co-authored-by: james7132 <contact@jamessliu.com>
2022-07-04 13:04:19 +00:00
Jakob Hellermann
218b0fd3b6 bevy_reflect: put serialize into external ReflectSerialize type (#4782)
builds on top of #4780 

# Objective

`Reflect` and `Serialize` are currently very tied together because `Reflect` has a `fn serialize(&self) -> Option<Serializable<'_>>` method. Because of that, we can either implement `Reflect` for types like `Option<T>` with `T: Serialize` and have `fn serialize` be implemented, or without the bound but having `fn serialize` return `None`.

By separating `ReflectSerialize` into a separate type (like how it already is for `ReflectDeserialize`, `ReflectDefault`), we could separately `.register::<Option<T>>()` and `.register_data::<Option<T>, ReflectSerialize>()` only if the type `T: Serialize`.

This PR does not change the registration but allows it to be changed in a future PR.

## Solution

- add the type
```rust
struct ReflectSerialize { .. }
impl<T: Reflect + Serialize> FromType<T> for ReflectSerialize { .. }
```

- remove `#[reflect(Serialize)]` special casing. 

- when serializing reflect value types, look for `ReflectSerialize` in the `TypeRegistry` instead of calling `value.serialize()`
2022-06-20 17:18:58 +00:00
MrGVSV
acbee7795d bevy_reflect: Reflect arrays (#4701)
# Objective

> ℹ️ **Note**: This is a rebased version of #2383. A large portion of it has not been touched (only a few minor changes) so that any additional discussion may happen here. All credit should go to @NathanSWard for their work on the original PR.

- Currently reflection is not supported for arrays.
- Fixes #1213

## Solution

* Implement reflection for arrays via the `Array` trait.
* Note, `Array` is different from `List` in the way that you cannot push elements onto an array as they are statically sized.
* Now `List` is defined as a sub-trait of `Array`.

---

## Changelog

* Added the `Array` reflection trait
* Allows arrays up to length 32 to be reflected via the `Array` trait

## Migration Guide

* The `List` trait now has the `Array` supertrait. This means that `clone_dynamic` will need to specify which version to use:
  ```rust
  // Before
  let cloned = my_list.clone_dynamic();
  // After
  let cloned = List::clone_dynamic(&my_list);
  ```
* All implementers of `List` will now need to implement `Array` (this mostly involves moving the existing methods to the `Array` impl)

Co-authored-by: NathanW <nathansward@comcast.net>
Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
2022-05-13 01:13:30 +00:00
Christopher Durham
a60fe30ada Avoid some format! into immediate format! (#2913)
# Objective

- Avoid usages of `format!` that ~immediately get passed to another `format!`. This avoids a temporary allocation and is just generally cleaner.

## Solution

- `bevy_derive::shader_defs` does a `format!("{}", val.to_string())`, which is better written as just `format!("{}", val)`
- `bevy_diagnostic::log_diagnostics_plugin` does a `format!("{:>}", format!(...))`, which is better written as `format!("{:>}", format_args!(...))`
- `bevy_ecs::schedule` does `tracing::info!(..., name = &*format!("{:?}", val))`, which is better written with the tracing shorthand `tracing::info!(..., name = ?val)`
- `bevy_reflect::reflect` does `f.write_str(&format!(...))`, which is better written as `write!(f, ...)` (this could also be written using `f.debug_tuple`, but I opted to maintain alt debug behavior)
- `bevy_reflect::serde::{ser, de}` do `serde::Error::custom(format!(...))`, which is better written as `Error::custom(format_args!(...))`, as `Error::custom` takes `impl Display` and just immediately calls `format!` again
2021-10-06 18:34:33 +00:00
TehPers
5e7456115a
Implement Reflect for tuples up to length 12 (#1218)
Add Reflect impls for tuples up to length 12
2021-01-07 19:50:09 -08:00
Carter Anderson
72b2fc9843
Bevy Reflection (#926)
Bevy Reflection
2020-11-27 16:39:59 -08:00