b01de70bdd
59 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
theotherphil
|
3b25b38bdc
|
deny(missing_docs) for bevy_reflect (#19481)
## Objective Deny missing docs in bevy_reflect, towards https://github.com/bevyengine/bevy/issues/3492. ## Solution Add the missing docs! ## Testing N/A |
||
|
SpecificProtagonist
|
a266e7e642
|
More uninlined_format_args fixes (#19396)
# Objective There are several uninlined format args (seems to be in more formatting macros and in more crates) that are not detected on stable, but are on nightly. ## Solution Fix them. |
||
|
Zachary Harrold
|
63e78fe489
|
Deprecated Begone! 0.16 Cleanup (#19108)
# Objective A fair few items were deprecated in 0.16. Let's delete them now that we're in the 0.17 development cycle! ## Solution - Deleted items marked deprecated in 0.16. ## Testing - CI --- ## Notes I'm making the assumption that _everything_ deprecated in 0.16 should be removed in 0.17. That may be a false assumption in certain cases. Please check the items to be removed to see if there are any exceptions we should keep around for another cycle! |
||
|
Carter Anderson
|
e9a0ef49f9
|
Rename bevy_platform_support to bevy_platform (#18813)
# Objective The goal of `bevy_platform_support` is to provide a set of platform agnostic APIs, alongside platform-specific functionality. This is a high traffic crate (providing things like HashMap and Instant). Especially in light of https://github.com/bevyengine/bevy/discussions/18799, it deserves a friendlier / shorter name. Given that it hasn't had a full release yet, getting this change in before Bevy 0.16 makes sense. ## Solution - Rename `bevy_platform_support` to `bevy_platform`. |
||
|
Gino Valente
|
c2854a2a05
|
bevy_reflect: Deprecate PartialReflect::clone_value (#18284)
# Objective #13432 added proper reflection-based cloning. This is a better method than cloning via `clone_value` for reasons detailed in the description of that PR. However, it may not be immediately apparent to users why one should be used over the other, and what the gotchas of `clone_value` are. ## Solution This PR marks `PartialReflect::clone_value` as deprecated, with the deprecation notice pointing users to `PartialReflect::reflect_clone`. However, it also suggests using a new method introduced in this PR: `PartialReflect::to_dynamic`. `PartialReflect::to_dynamic` is essentially a renaming of `PartialReflect::clone_value`. By naming it `to_dynamic`, we make it very obvious that what's returned is a dynamic type. The one caveat to this is that opaque types still use `reflect_clone` as they have no corresponding dynamic type. Along with changing the name, the method is now optional, and comes with a default implementation that calls out to the respective reflection subtrait method. This was done because there was really no reason to require manual implementors provide a method that almost always calls out to a known set of methods. Lastly, to make this default implementation work, this PR also did a similar thing with the `clone_dynamic ` methods on the reflection subtraits. For example, `Struct::clone_dynamic` has been marked deprecated and is superseded by `Struct::to_dynamic_struct`. This was necessary to avoid the "multiple names in scope" issue. ### Open Questions This PR maintains the original signature of `clone_value` on `to_dynamic`. That is, it takes `&self` and returns `Box<dyn PartialReflect>`. However, in order for this to work, it introduces a panic if the value is opaque and doesn't override the default `reflect_clone` implementation. One thing we could do to avoid the panic would be to make the conversion fallible, either returning `Option<Box<dyn PartialReflect>>` or `Result<Box<dyn PartialReflect>, ReflectCloneError>`. This makes using the method a little more involved (i.e. users have to either unwrap or handle the rare possibility of an error), but it would set us up for a world where opaque types don't strictly need to be `Clone`. Right now this bound is sort of implied by the fact that `clone_value` is a required trait method, and the default behavior of the macro is to use `Clone` for opaque types. Alternatively, we could keep the signature but make the method required. This maintains that implied bound where manual implementors must provide some way of cloning the value (or YOLO it and just panic), but also makes the API simpler to use. Finally, we could just leave it with the panic. It's unlikely this would occur in practice since our macro still requires `Clone` for opaque types, and thus this would only ever be an issue if someone were to manually implement `PartialReflect` without a valid `to_dynamic` or `reflect_clone` method. ## Testing You can test locally using the following command: ``` cargo test --package bevy_reflect --all-features ``` --- ## Migration Guide `PartialReflect::clone_value` is being deprecated. Instead, use `PartialReflect::to_dynamic` if wanting to create a new dynamic instance of the reflected value. Alternatively, use `PartialReflect::reflect_clone` to attempt to create a true clone of the underlying value. Similarly, the following methods have been deprecated and should be replaced with these alternatives: - `Array::clone_dynamic` → `Array::to_dynamic_array` - `Enum::clone_dynamic` → `Enum::to_dynamic_enum` - `List::clone_dynamic` → `List::to_dynamic_list` - `Map::clone_dynamic` → `Map::to_dynamic_map` - `Set::clone_dynamic` → `Set::to_dynamic_set` - `Struct::clone_dynamic` → `Struct::to_dynamic_struct` - `Tuple::clone_dynamic` → `Tuple::to_dynamic_tuple` - `TupleStruct::clone_dynamic` → `TupleStruct::to_dynamic_tuple_struct` |
||
|
raldone01
|
1b7db895b7
|
Harden proc macro path resolution and add integration tests. (#17330)
This pr uses the `extern crate self as` trick to make proc macros behave the same way inside and outside bevy. # Objective - Removes noise introduced by `crate as` in the whole bevy repo. - Fixes #17004. - Hardens proc macro path resolution. ## TODO - [x] `BevyManifest` needs cleanup. - [x] Cleanup remaining `crate as`. - [x] Add proper integration tests to the ci. ## Notes - `cargo-manifest-proc-macros` is written by me and based/inspired by the old `BevyManifest` implementation and [`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate). - What do you think about the new integration test machinery I added to the `ci`? More and better integration tests can be added at a later stage. The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates. ## Testing - Needs RA test - Needs testing from other users - Others need to run at least `cargo run -p ci integration-test` and verify that they work. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Zachary Harrold
|
9bc0ae33c3
|
Move hashbrown and foldhash out of bevy_utils (#17460)
# Objective - Contributes to #16877 ## Solution - Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils` and into `bevy_platform_support` - Refactored the above to match the layout of these types in `std`. - Updated crates as required. ## Testing - CI --- ## Migration Guide - The following items were moved out of `bevy_utils` and into `bevy_platform_support::hash`: - `FixedState` - `DefaultHasher` - `RandomState` - `FixedHasher` - `Hashed` - `PassHash` - `PassHasher` - `NoOpHash` - The following items were moved out of `bevy_utils` and into `bevy_platform_support::collections`: - `HashMap` - `HashSet` - `bevy_utils::hashbrown` has been removed. Instead, import from `bevy_platform_support::collections` _or_ take a dependency on `hashbrown` directly. - `bevy_utils::Entry` has been removed. Instead, import from `bevy_platform_support::collections::hash_map` or `bevy_platform_support::collections::hash_set` as appropriate. - All of the above equally apply to `bevy::utils` and `bevy::platform_support`. ## Notes - I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils` as they might be candidates for micro-crating. They can always be moved into `bevy_platform_support` at a later date if desired. |
||
|
Zachary Harrold
|
a64446b77e
|
Create bevy_platform_support Crate (#17250)
# Objective - Contributes to #16877 ## Solution - Initial creation of `bevy_platform_support` crate. - Moved `bevy_utils::Instant` into new `bevy_platform_support` crate. - Moved `portable-atomic`, `portable-atomic-util`, and `critical-section` into new `bevy_platform_support` crate. ## Testing - CI --- ## Showcase Instead of needing code like this to import an `Arc`: ```rust #[cfg(feature = "portable-atomic")] use portable_atomic_util::Arc; #[cfg(not(feature = "portable-atomic"))] use alloc::sync::Arc; ``` We can now use: ```rust use bevy_platform_support::sync::Arc; ``` This applies to many other types, but the goal is overall the same: allowing crates to use `std`-like types without the boilerplate of conditional compilation and platform-dependencies. ## Migration Guide - Replace imports of `bevy_utils::Instant` with `bevy_platform_support::time::Instant` - Replace imports of `bevy::utils::Instant` with `bevy::platform_support::time::Instant` ## Notes - `bevy_platform_support` hasn't been reserved on `crates.io` - ~~`bevy_platform_support` is not re-exported from `bevy` at this time. It may be worthwhile exporting this crate, but I am unsure of a reasonable name to export it under (`platform_support` may be a bit wordy for user-facing).~~ - I've included an implementation of `Instant` which is suitable for `no_std` platforms that are not Wasm for the sake of eliminating feature gates around its use. It may be a controversial inclusion, so I'm happy to remove it if required. - There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`, etc.) which should be added to this crate. I have kept the initial scope small to demonstrate utility without making this too unwieldy. --------- Co-authored-by: TimJentzsch <TimJentzsch@users.noreply.github.com> Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Zachary Harrold
|
bf765e61b5
|
Add no_std support to bevy_reflect (#16256)
# Objective - Contributes to #15460 ## Solution - Added `std` feature (enabled by default) ## Testing - CI - `cargo check -p bevy_reflect --no-default-features --target "x86_64-unknown-none"` - UEFI demo application runs with this branch of `bevy_reflect`, allowing `derive(Reflect)` ## Notes - The [`spin`](https://crates.io/crates/spin) crate has been included to provide `RwLock` and `Once` (as an alternative to `OnceLock`) when the `std` feature is not enabled. Another alternative may be more desirable, please provide feedback if you have a strong opinion here! - Certain items (`Box`, `String`, `ToString`) provided by `alloc` have been added to `__macro_exports` as a way to avoid `alloc` vs `std` namespacing. I'm personally quite annoyed that we can't rely on `alloc` as a crate name in `std` environments within macros. I'd love an alternative to my approach here, but I suspect it's the least-bad option. - I would've liked to have an `alloc` feature (for allocation-free `bevy_reflect`), unfortunately, `erased_serde` unconditionally requires access to `Box`. Maybe one day we could design around this, but for now it just means `bevy_reflect` requires `alloc`. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Torstein Grindvik
|
9f5f5d3d41
|
bevy_reflect: get_represented_kind_info APIs for reflected kinds (#14380)
# Objective Fixes #14378 --------- Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com> Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Clar Fon
|
e79bc7811d
|
Fix *most* clippy lints (#15906)
# Objective Another clippy-lint fix: the goal is so that `ci lints` actually displays the problems that a contributor caused, and not a bunch of existing stuff in the repo. (when run on nightly) ## Solution This fixes all but the `clippy::needless_lifetimes` lint, which will result in substantially more fixes and be in other PR(s). I also explicitly allow `non_local_definitions` since it is [not working correctly, but will be fixed](https://github.com/rust-lang/rust/issues/131643). A few things were manually fixed: for example, some places had an explicitly defined `div_ceil` function that was used, which is no longer needed since this function is stable on unsigned integers. Also, empty lines in doc comments were handled individually. ## Testing I ran `cargo clippy --workspace --all-targets --all-features --fix --allow-staged` with the `clippy::needless_lifetimes` lint marked as `allow` in `Cargo.toml` to avoid fixing that too. It now passes with all but the listed lint. |
||
|
Gino Valente
|
397f20e835
|
bevy_reflect: Generic parameter info (#15475)
# Objective Currently, reflecting a generic type provides no information about the generic parameters. This means that you can't get access to the type of `T` in `Foo<T>` without creating custom type data (we do this for [`ReflectHandle`](https://docs.rs/bevy/0.14.2/bevy/asset/struct.ReflectHandle.html#method.asset_type_id)). ## Solution This PR makes it so that generic type parameters and generic const parameters are tracked in a `Generics` struct stored on the `TypeInfo` for a type. For example, `struct Foo<T, const N: usize>` will store `T` and `N` as a `TypeParamInfo` and `ConstParamInfo`, respectively. The stored information includes: - The name of the generic parameter (i.e. `T`, `N`, etc.) - The type of the generic parameter (remember that we're dealing with monomorphized types, so this will actually be a concrete type) - The default type/value, if any (e.g. `f32` in `T = f32` or `10` in `const N: usize = 10`) ### Caveats The only requirement for this to work is that the user does not opt-out of the automatic `TypePath` derive with `#[reflect(type_path = false)]`. Doing so prevents the macro code from 100% knowing that the generic type implements `TypePath`. This in turn means the generated `Typed` impl can't add generics to the type. There are two solutions for this—both of which I think we should explore in a future PR: 1. We could just not use `TypePath`. This would mean that we can't store the `Type` of the generic, but we can at least store the `TypeId`. 2. We could provide a way to opt out of the automatic `Typed` derive with a `#[reflect(typed = false)]` attribute. This would allow users to manually implement `Typed` to add whatever generic information they need (e.g. skipping a parameter that can't implement `TypePath` while the rest can). I originally thought about making `Generics` an enum with `Generic`, `NonGeneric`, and `Unavailable` variants to signify whether there are generics, no generics, or generics that cannot be added due to opting out of `TypePath`. I ultimately decided against this as I think it adds a bit too much complexity for such an uncommon problem. Additionally, user's don't necessarily _have_ to know the generics of a type, so just skipping them should generally be fine for now. ## Testing You can test locally by running: ``` cargo test --package bevy_reflect ``` --- ## Showcase You can now access generic parameters via `TypeInfo`! ```rust #[derive(Reflect)] struct MyStruct<T, const N: usize>([T; N]); let generics = MyStruct::<f32, 10>::type_info().generics(); // Get by index: let t = generics.get(0).unwrap(); assert_eq!(t.name(), "T"); assert!(t.ty().is::<f32>()); assert!(!t.is_const()); // Or by name: let n = generics.get_named("N").unwrap(); assert_eq!(n.name(), "N"); assert!(n.ty().is::<usize>()); assert!(n.is_const()); ``` You can even access parameter defaults: ```rust #[derive(Reflect)] struct MyStruct<T = String, const N: usize = 10>([T; N]); let generics = MyStruct::<f32, 5>::type_info().generics(); let GenericInfo::Type(info) = generics.get_named("T").unwrap() else { panic!("expected a type parameter"); }; let default = info.default().unwrap(); assert!(default.is::<String>()); let GenericInfo::Const(info) = generics.get_named("N").unwrap() else { panic!("expected a const parameter"); }; let default = info.default().unwrap(); assert_eq!(default.downcast_ref::<usize>().unwrap(), &10); ``` |
||
|
Zachary Harrold
|
d70595b667
|
Add core and alloc over std Lints (#15281)
# Objective - Fixes #6370 - Closes #6581 ## Solution - Added the following lints to the workspace: - `std_instead_of_core` - `std_instead_of_alloc` - `alloc_instead_of_core` - Used `cargo +nightly fmt` with [item level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A) to split all `use` statements into single items. - Used `cargo clippy --workspace --all-targets --all-features --fix --allow-dirty` to _attempt_ to resolve the new linting issues, and intervened where the lint was unable to resolve the issue automatically (usually due to needing an `extern crate alloc;` statement in a crate root). - Manually removed certain uses of `std` where negative feature gating prevented `--all-features` from finding the offending uses. - Used `cargo +nightly fmt` with [crate level use formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A) to re-merge all `use` statements matching Bevy's previous styling. - Manually fixed cases where the `fmt` tool could not re-merge `use` statements due to conditional compilation attributes. ## Testing - Ran CI locally ## Migration Guide The MSRV is now 1.81. Please update to this version or higher. ## Notes - This is a _massive_ change to try and push through, which is why I've outlined the semi-automatic steps I used to create this PR, in case this fails and someone else tries again in the future. - Making this change has no impact on user code, but does mean Bevy contributors will be warned to use `core` and `alloc` instead of `std` where possible. - This lint is a critical first step towards investigating `no_std` options for Bevy. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Clar Fon
|
efda7f3f9c
|
Simpler lint fixes: makes ci lints work but disables a lint for now (#15376)
Takes the first two commits from #15375 and adds suggestions from this comment: https://github.com/bevyengine/bevy/pull/15375#issuecomment-2366968300 See #15375 for more reasoning/motivation. ## Rebasing (rerunning) ```rust git switch simpler-lint-fixes git reset --hard main cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "rustfmt" cargo clippy --workspace --all-targets --all-features --fix cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate cargo fmt --all git add --update git commit --message "clippy" git cherry-pick e6c0b94f6795222310fb812fa5c4512661fc7887 ``` |
||
|
Gino Valente
|
4d0961cc8a
|
bevy_reflect: Add ReflectRef/ReflectMut/ReflectOwned convenience 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> |
||
|
Gino Valente
|
3892adcb47
|
bevy_reflect: Add Type type (#14838)
# Objective Closes #7622. I was working on adding support for reflecting generic functions and found that I wanted to use an argument's `TypeId` for hashing and comparison, but its `TypePath` for debugging and error messaging. While I could just keep them separate, place them in a tuple or a local struct or something, I think I see an opportunity to make a dedicate type for this. Additionally, we can use this type to clean up some duplication amongst the type info structs in a manner similar to #7622. ## Solution Added the `Type` type. This should be seen as the most basic representation of a type apart from `TypeId`. It stores both the `TypeId` of the type as well as its `TypePathTable`. The `Hash` and `PartialEq` implementations rely on the `TypeId`, while the `Debug` implementation relies on the `TypePath`. This makes it especially useful as a key in a `HashMap` since we get the speed of the `TypeId` hashing/comparisons with the readability of `TypePath`. With this type, we're able to reduce the duplication across the type info structs by removing individual fields for `TypeId` and `TypePathTable`, replacing them with a single `Type` field. Similarly, we can remove many duplicate methods and replace it with a macro that delegates to the stored `Type`. ### Caveats It should be noted that this type is currently 3x larger than `TypeId`. On my machine, it's 48 bytes compared to `TypeId`'s 16. While this doesn't matter for `TypeInfo` since it would contain that data regardless, it is something to keep in mind when using elsewhere. ## Testing All tests should pass as normal: ``` cargo test --package bevy_reflect ``` --- ## Showcase `bevy_reflect` now exports a `Type` struct. This type contains both the `TypeId` and the `TypePathTable` of the given type, allowing it to be used like `TypeId` but have the debuggability of `TypePath`. ```rust // We can create this for any type implementing `TypePath`: let ty = Type::of::<String>(); // It has `Hash` and `Eq` impls powered by `TypeId`, making it useful for maps: let mut map = HashMap::<Type, i32>::new(); map.insert(ty, 25); // And it has a human-readable `Debug` representation: let debug = format!("{:?}", map); assert_eq!(debug, "{alloc::string::String: 25}"); ``` ## Migration Guide Certain type info structs now only return their item types as `Type` instead of exposing direct methods on them. The following methods have been removed: - `ArrayInfo::item_type_path_table` - `ArrayInfo::item_type_id` - `ArrayInfo::item_is` - `ListInfo::item_type_path_table` - `ListInfo::item_type_id` - `ListInfo::item_is` - `SetInfo::value_type_path_table` - `SetInfo::value_type_id` - `SetInfo::value_is` - `MapInfo::key_type_path_table` - `MapInfo::key_type_id` - `MapInfo::key_is` - `MapInfo::value_type_path_table` - `MapInfo::value_type_id` - `MapInfo::value_is` Instead, access the `Type` directly using one of the new methods: - `ArrayInfo::item_ty` - `ListInfo::item_ty` - `SetInfo::value_ty` - `MapInfo::key_ty` - `MapInfo::value_ty` For example: ```rust // BEFORE let type_id = array_info.item_type_id(); // AFTER let type_id = array_info.item_ty().id(); ``` |
||
|
radiish
|
6ab8767d3b
|
reflect: implement the unique reflect rfc (#7207)
# Objective
- Implements the [Unique Reflect
RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md).
## Solution
- Implements the RFC.
- This implementation differs in some ways from the RFC:
- In the RFC, it was suggested `Reflect: Any` but `PartialReflect:
?Any`. During initial implementation I tried this, but we assume the
`PartialReflect: 'static` in a lot of places and the changes required
crept out of the scope of this PR.
- `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn
Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn
PartialReflect>>` since the method takes by value and otherwise there
would be no way to recover the type. `as_full` and `as_full_mut` both
still return `Option<&(mut) dyn Reflect>`.
---
## Changelog
- Added `PartialReflect`.
- `Reflect` is now a subtrait of `PartialReflect`.
- Moved most methods on `Reflect` to the new `PartialReflect`.
- Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect}`.
- Added `PartialReflect::{try_as_reflect, try_as_reflect_mut,
try_into_reflect}`.
- Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut,
try_downcast, try_take}` supplementing the methods on `dyn Reflect`.
## Migration Guide
- Most instances of `dyn Reflect` should be changed to `dyn
PartialReflect` which is less restrictive, however trait bounds should
generally stay as `T: Reflect`.
- The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect, try_as_reflect, try_as_reflect_mut,
try_into_reflect}` methods as well as `Reflect::{as_reflect,
as_reflect_mut, into_reflect}` will need to be implemented for manual
implementors of `Reflect`.
## Future Work
- This PR is designed to be followed up by another "Unique Reflect Phase
2" that addresses the following points:
- Investigate making serialization revolve around `Reflect` instead of
`PartialReflect`.
- [Remove the `try_*` methods on `dyn PartialReflect` since they are
stop
gaps](https://github.com/bevyengine/bevy/pull/7207#discussion_r1083476050).
- Investigate usages like `ReflectComponent`. In the places they
currently use `PartialReflect`, should they be changed to use `Reflect`?
- Merging this opens the door to lots of reflection features we haven't
been able to implement.
- We could re-add [the `Reflectable`
trait](
|
||
|
SpecificProtagonist
|
ab255aefc6
|
Implement FromIterator/IntoIterator for dynamic types (#14250)
# Objective Implement FromIterator/IntoIterator for dynamic types where missing Note: - can't impl `IntoIterator` for `&Array` & co because of orphan rules - `into_iter().collect()` is a no-op for `Vec`s because of specialization --- ## Migration Guide - Change `DynamicArray::from_vec` to `DynamicArray::from_iter` |
||
|
Gino Valente
|
99c9218b56
|
bevy_reflect: Feature-gate function reflection (#14174)
# Objective Function reflection requires a lot of macro code generation in the form of several `all_tuples!` invocations, as well as impls generated in the `Reflect` derive macro. Seeing as function reflection is currently a bit more niche, it makes sense to gate it all behind a feature. ## Solution Add a `functions` feature to `bevy_reflect`, which can be enabled in Bevy using the `reflect_functions` feature. ## Testing You can test locally by running: ``` cargo test --package bevy_reflect ``` That should ensure that everything still works with the feature disabled. To test with the feature on, you can run: ``` cargo test --package bevy_reflect --features functions ``` --- ## Changelog - Moved function reflection behind a Cargo feature (`bevy/reflect_functions` and `bevy_reflect/functions`) - Add `IntoFunction` export in `bevy_reflect::prelude` ## Internal Migration Guide > [!important] > Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on `main` during this cycle, and is not a breaking change coming from 0.14. Function reflection is now gated behind a feature. To use function reflection, enable the feature: - If using `bevy_reflect` directly, enable the `functions` feature - If using `bevy`, enable the `reflect_functions` feature |
||
|
Lura
|
856b39d821
|
Apply Clippy lints regarding lazy evaluation and closures (#14015)
# Objective - Lazily evaluate [default](https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_or_default)~~/[or](https://rust-lang.github.io/rust-clippy/master/index.html#/or_fun_call)~~ values where it makes sense - ~~`unwrap_or(foo())` -> `unwrap_or_else(|| foo())`~~ - `unwrap_or(Default::default())` -> `unwrap_or_default()` - etc. - Avoid creating [redundant closures](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure), even for [method calls](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure_for_method_calls) - `map(|something| something.into())` -> `map(Into:into)` ## Solution - Apply Clippy lints: - ~~[or_fun_call](https://rust-lang.github.io/rust-clippy/master/index.html#/or_fun_call)~~ - [unwrap_or_default](https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_or_default) - [redundant_closure_for_method_calls](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure_for_method_calls) ([redundant closures](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure) is already enabled) ## Testing - Tested on Windows 11 (`stable-x86_64-pc-windows-gnu`, 1.79.0) - Bevy compiles without errors or warnings and examples seem to work as intended - `cargo clippy` ✅ - `cargo run -p ci -- compile` ✅ --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Gino Valente
|
276dd04001
|
bevy_reflect: Function reflection (#13152)
# Objective
We're able to reflect types sooooooo... why not functions?
The goal of this PR is to make functions callable within a dynamic
context, where type information is not readily available at compile
time.
For example, if we have a function:
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
```
And two `Reflect` values we've already validated are `i32` types:
```rust
let left: Box<dyn Reflect> = Box::new(2_i32);
let right: Box<dyn Reflect> = Box::new(2_i32);
```
We should be able to call `add` with these values:
```rust
// ?????
let result: Box<dyn Reflect> = add.call_dynamic(left, right);
```
And ideally this wouldn't just work for functions, but methods and
closures too!
Right now, users have two options:
1. Manually parse the reflected data and call the function themselves
2. Rely on registered type data to handle the conversions for them
For a small function like `add`, this isn't too bad. But what about for
more complex functions? What about for many functions?
At worst, this process is error-prone. At best, it's simply tedious.
And this is assuming we know the function at compile time. What if we
want to accept a function dynamically and call it with our own
arguments?
It would be much nicer if `bevy_reflect` could alleviate some of the
problems here.
## Solution
Added function reflection!
This adds a `DynamicFunction` type to wrap a function dynamically. This
can be called with an `ArgList`, which is a dynamic list of
`Reflect`-containing `Arg` arguments. It returns a `FunctionResult`
which indicates whether or not the function call succeeded, returning a
`Reflect`-containing `Return` type if it did succeed.
Many functions can be converted into this `DynamicFunction` type thanks
to the `IntoFunction` trait.
Taking our previous `add` example, this might look something like
(explicit types added for readability):
```rust
fn add(left: i32, right: i32) -> i32 {
left + right
}
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
And it also works on closures:
```rust
let add = |left: i32, right: i32| left + right;
let mut function: DynamicFunction = add.into_function();
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
let result: Return = function.call(args).unwrap();
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
As well as methods:
```rust
#[derive(Reflect)]
struct Foo(i32);
impl Foo {
fn add(&mut self, value: i32) {
self.0 += value;
}
}
let mut foo = Foo(2);
let mut function: DynamicFunction = Foo::add.into_function();
let args: ArgList = ArgList::new().push_mut(&mut foo).push_owned(2_i32);
function.call(args).unwrap();
assert_eq!(foo.0, 4);
```
### Limitations
While this does cover many functions, it is far from a perfect system
and has quite a few limitations. Here are a few of the limitations when
using `IntoFunction`:
1. The lifetime of the return value is only tied to the lifetime of the
first argument (useful for methods). This means you can't have a
function like `(a: i32, b: &i32) -> &i32` without creating the
`DynamicFunction` manually.
2. Only 15 arguments are currently supported. If the first argument is a
(mutable) reference, this number increases to 16.
3. Manual implementations of `Reflect` will need to implement the new
`FromArg`, `GetOwnership`, and `IntoReturn` traits in order to be used
as arguments/return types.
And some limitations of `DynamicFunction` itself:
1. All arguments share the same lifetime, or rather, they will shrink to
the shortest lifetime.
2. Closures that capture their environment may need to have their
`DynamicFunction` dropped before accessing those variables again (there
is a `DynamicFunction::call_once` to make this a bit easier)
3. All arguments and return types must implement `Reflect`. While not a
big surprise coming from `bevy_reflect`, this implementation could
actually still work by swapping `Reflect` out with `Any`. Of course,
that makes working with the arguments and return values a bit harder.
4. Generic functions are not supported (unless they have been manually
monomorphized)
And general, reflection gotchas:
1. `&str` does not implement `Reflect`. Rather, `&'static str`
implements `Reflect` (the same is true for `&Path` and similar types).
This means that `&'static str` is considered an "owned" value for the
sake of generating arguments. Additionally, arguments and return types
containing `&str` will assume it's `&'static str`, which is almost never
the desired behavior. In these cases, the only solution (I believe) is
to use `&String` instead.
### Followup Work
This PR is the first of two PRs I intend to work on. The second PR will
aim to integrate this new function reflection system into the existing
reflection traits and `TypeInfo`. The goal would be to register and call
a reflected type's methods dynamically.
I chose not to do that in this PR since the diff is already quite large.
I also want the discussion for both PRs to be focused on their own
implementation.
Another followup I'd like to do is investigate allowing common container
types as a return type, such as `Option<&[mut] T>` and `Result<&[mut] T,
E>`. This would allow even more functions to opt into this system. I
chose to not include it in this one, though, for the same reasoning as
previously mentioned.
### Alternatives
One alternative I had considered was adding a macro to convert any
function into a reflection-based counterpart. The idea would be that a
struct that wraps the function would be created and users could specify
which arguments and return values should be `Reflect`. It could then be
called via a new `Function` trait.
I think that could still work, but it will be a fair bit more involved,
requiring some slightly more complex parsing. And it of course is a bit
more work for the user, since they need to create the type via macro
invocation.
It also makes registering these functions onto a type a bit more
complicated (depending on how it's implemented).
For now, I think this is a fairly simple, yet powerful solution that
provides the least amount of friction for users.
---
## Showcase
Bevy now adds support for storing and calling functions dynamically
using reflection!
```rust
// 1. Take a standard Rust function
fn add(left: i32, right: i32) -> i32 {
left + right
}
// 2. Convert it into a type-erased `DynamicFunction` using the `IntoFunction` trait
let mut function: DynamicFunction = add.into_function();
// 3. Define your arguments from reflected values
let args: ArgList = ArgList::new().push_owned(2_i32).push_owned(2_i32);
// 4. Call the function with your arguments
let result: Return = function.call(args).unwrap();
// 5. Extract the return value
let value: Box<dyn Reflect> = result.unwrap_owned();
assert_eq!(value.take::<i32>().unwrap(), 4);
```
## Changelog
#### TL;DR
- Added support for function reflection
- Added a new `Function Reflection` example:
|
||
|
Gino Valente
|
5db52663b3
|
bevy_reflect: Custom attributes (#11659)
# Objective As work on the editor starts to ramp up, it might be nice to start allowing types to specify custom attributes. These can be used to provide certain functionality to fields, such as ranges or controlling how data is displayed. A good example of this can be seen in [`bevy-inspector-egui`](https://github.com/jakobhellermann/bevy-inspector-egui) with its [`InspectorOptions`](https://docs.rs/bevy-inspector-egui/0.22.1/bevy_inspector_egui/struct.InspectorOptions.html): ```rust #[derive(Reflect, Default, InspectorOptions)] #[reflect(InspectorOptions)] struct Slider { #[inspector(min = 0.0, max = 1.0)] value: f32, } ``` Normally, as demonstrated in the example above, these attributes are handled by a derive macro and stored in a corresponding `TypeData` struct (i.e. `ReflectInspectorOptions`). Ideally, we would have a good way of defining this directly via reflection so that users don't need to create and manage a whole proc macro just to allow these sorts of attributes. And note that this doesn't have to just be for inspectors and editors. It can be used for things done purely on the code side of things. ## Solution Create a new method for storing attributes on fields via the `Reflect` derive. These custom attributes are stored in type info (e.g. `NamedField`, `StructInfo`, etc.). ```rust #[derive(Reflect)] struct Slider { #[reflect(@0.0..=1.0)] value: f64, } let TypeInfo::Struct(info) = Slider::type_info() else { panic!("expected struct info"); }; let field = info.field("value").unwrap(); let range = field.get_attribute::<RangeInclusive<f64>>().unwrap(); assert_eq!(*range, 0.0..=1.0); ``` ## TODO - [x] ~~Bikeshed syntax~~ Went with a type-based approach, prefixed by `@` for ease of parsing and flexibility - [x] Add support for custom struct/tuple struct field attributes - [x] Add support for custom enum variant field attributes - [x] ~~Add support for custom enum variant attributes (maybe?)~~ ~~Will require a larger refactor. Can be saved for a future PR if we really want it.~~ Actually, we apparently still have support for variant attributes despite not using them, so it was pretty easy to add lol. - [x] Add support for custom container attributes - [x] Allow custom attributes to store any reflectable value (not just `Lit`) - [x] ~~Store attributes in registry~~ This PR used to store these in attributes in the registry, however, it has since switched over to storing them in type info - [x] Add example ## Bikeshedding > [!note] > This section was made for the old method of handling custom attributes, which stored them by name (i.e. `some_attribute = 123`). The PR has shifted away from that, to a more type-safe approach. > > This section has been left for reference. There are a number of ways we can syntactically handle custom attributes. Feel free to leave a comment on your preferred one! Ideally we want one that is clear, readable, and concise since these will potentially see _a lot_ of use. Below is a small, non-exhaustive list of them. Note that the `skip_serializing` reflection attribute is added to demonstrate how each case plays with existing reflection attributes. <details> <summary>List</summary> ##### 1. `@(name = value)` > The `@` was chosen to make them stand out from other attributes and because the "at" symbol is a subtle pneumonic for "attribute". Of course, other symbols could be used (e.g. `$`, `#`, etc.). ```rust #[derive(Reflect)] struct Slider { #[reflect(@(min = 0.0, max = 1.0), skip_serializing)] #[[reflect(@(bevy_editor::hint = "Range: 0.0 to 1.0"))] value: f32, } ``` ##### 2. `@name = value` > This is my personal favorite. ```rust #[derive(Reflect)] struct Slider { #[reflect(@min = 0.0, @max = 1.0, skip_serializing)] #[[reflect(@bevy_editor::hint = "Range: 0.0 to 1.0")] value: f32, } ``` ##### 3. `custom_attr(name = value)` > `custom_attr` can be anything. Other possibilities include `with` or `tag`. ```rust #[derive(Reflect)] struct Slider { #[reflect(custom_attr(min = 0.0, max = 1.0), skip_serializing)] #[[reflect(custom_attr(bevy_editor::hint = "Range: 0.0 to 1.0"))] value: f32, } ``` ##### 4. `reflect_attr(name = value)` ```rust #[derive(Reflect)] struct Slider { #[reflect(skip_serializing)] #[reflect_attr(min = 0.0, max = 1.0)] #[[reflect_attr(bevy_editor::hint = "Range: 0.0 to 1.0")] value: f32, } ``` </details> --- ## Changelog - Added support for custom attributes on reflected types (i.e. `#[reflect(@Foo::new("bar")]`) |
||
|
rmsthebest
|
278380394f
|
Avoid bevy_reflect::List::iter wrapping in release mode (#13271)
# Objective Fixes #13230 ## Solution Uses solution described in #13230 They mention a worry about adding a branch, but I'm not sure there is one. This code ```Rust #[no_mangle] pub fn next_if_some(num: i32, b: Option<bool>) -> i32 { num + b.is_some() as i32 } ``` produces this assembly with opt-level 3 ```asm next_if_some: xor eax, eax cmp sil, 2 setne al add eax, edi ret ``` ## Testing Added test from #13230, tagged it as ignore as it is only useful in release mode and very slow if you accidentally invoke it in debug mode. --- ## Changelog Iterationg of ListIter will no longer overflow and wrap around ## Migration Guide |
||
|
Brezak
|
9c4ac7c297
|
Finish the work on try_apply (#12646)
# Objective Finish the `try_apply` implementation started in #6770 by @feyokorenhof. Supersedes and closes #6770. Closes #6182 ## Solution Add `try_apply` to `Reflect` and implement it in all the places that implement `Reflect`. --- ## Changelog Added `try_apply` to `Reflect`. --------- Co-authored-by: Feyo Korenhof <feyokorenhof@gmail.com> Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> |
||
|
Doonv
|
054134fba2
|
Add ReflectKind (#11664)
# Objective Fix https://github.com/bevyengine/bevy/issues/11657 ## Solution Add a `ReflectKind` enum, add `Reflect::reflect_kind` which returns a `ReflectKind`, and add `kind` method implementions to `ReflectRef`, `ReflectMut`, and `ReflectOwned`, which returns a `ReflectKind`. I also changed `AccessError` to use this new struct instead of it's own `TypeKind` struct. --- ## Changelog - Added `ReflectKind`, an enumeration over the kinds of a reflected type without its data. - Added `Reflect::reflect_kind` (with default implementation) - Added implementation for the `kind` method on `ReflectRef`, `ReflectMut`, and `ReflectOwned` which gives their kind without any information, as a `ReflectKind` |
||
|
Doonv
|
56076b7b0c
|
Improve DynamicStruct::insert (#11068)
# Objective I wanted to pass in a `String` to `DynamicStruct::insert_boxed` but it took in a &str. That's fine but I also saw that it immediately converted the `&str` to a `String`. Which is wasteful. ## Solution I made `DynamicStruct::insert_boxed` take in a `impl Into<Cow<str>>`. Same for `DynamicStruct::insert`. --- ## Changelog - `DynamicStruct::insert_boxed` and `DynamicStruct::insert` now support taking in anything that implements `impl Into<Cow<str>>`. |
||
|
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. |
||
|
davier
|
55402bdf2e
|
Fix debug printing for dynamic types (#10740)
# Objective
Printing `DynamicStruct` with a debug format does not show the contained
type anymore. For instance, in `examples/reflection/reflection.rs`,
adding `dbg!(&reflect_value);` to line 96 will print:
```rust
[examples/reflection/reflection.rs:96] &reflect_value = DynamicStruct(bevy_reflect::DynamicStruct {
a: 4,
nested: DynamicStruct(bevy_reflect::DynamicStruct {
b: 8,
}),
})
```
## Solution
Show the represented type instead (`reflection::Foo` and
`reflection::Bar` in this case):
```rust
[examples/reflection/reflection.rs:96] &reflect_value = DynamicStruct(reflection::Foo {
a: 4,
nested: DynamicStruct(reflection::Bar {
b: 8,
}),
})
```
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
|
||
|
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. |
||
|
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`](
|
||
|
Rob Parrett
|
a788e31ad5
|
Fix CI for Rust 1.72 (#9562)
# Objective [Rust 1.72.0](https://blog.rust-lang.org/2023/08/24/Rust-1.72.0.html) is now stable. # Notes - `let-else` formatting has arrived! - I chose to allow `explicit_iter_loop` due to https://github.com/rust-lang/rust-clippy/issues/11074. We didn't hit any of the false positives that prevent compilation, but fixing this did produce a lot of the "symbol soup" mentioned, e.g. `for image in &mut *image_events {`. Happy to undo this if there's consensus the other way. --------- Co-authored-by: François <mockersf@gmail.com> |
||
|
Carter Anderson
|
8b9d88f4d0
|
Reflect now requires DynamicTypePath. Remove Reflect::get_type_path() (#8764)
Followup to #7184 This makes `Reflect: DynamicTypePath` which allows us to remove `Reflect::get_type_path`, reducing unnecessary codegen and simplifying `Reflect` implementations. |
||
|
radiish
|
1efc762924
|
reflect: stable type path v2 (#7184)
# Objective
- Introduce a stable alternative to
[`std::any::type_name`](https://doc.rust-lang.org/std/any/fn.type_name.html).
- Rewrite of #5805 with heavy inspiration in design.
- On the path to #5830.
- Part of solving #3327.
## Solution
- Add a `TypePath` trait for static stable type path/name information.
- Add a `TypePath` derive macro.
- Add a `impl_type_path` macro for implementing internal and foreign
types in `bevy_reflect`.
---
## Changelog
- Added `TypePath` trait.
- Added `DynamicTypePath` trait and `get_type_path` method to `Reflect`.
- Added a `TypePath` derive macro.
- Added a `bevy_reflect::impl_type_path` for implementing `TypePath` on
internal and foreign types in `bevy_reflect`.
- Changed `bevy_reflect::utility::(Non)GenericTypeInfoCell` to
`(Non)GenericTypedCell<T>` which allows us to be generic over both
`TypeInfo` and `TypePath`.
- `TypePath` is now a supertrait of `Asset`, `Material` and
`Material2d`.
- `impl_reflect_struct` needs a `#[type_path = "..."]` attribute to be
specified.
- `impl_reflect_value` needs to either specify path starting with a
double colon (`::core::option::Option`) or an `in my_crate::foo`
declaration.
- Added `bevy_reflect_derive::ReflectTypePath`.
- Most uses of `Ident` in `bevy_reflect_derive` changed to use
`ReflectTypePath`.
## Migration Guide
- Implementors of `Asset`, `Material` and `Material2d` now also need to
derive `TypePath`.
- Manual implementors of `Reflect` will need to implement the new
`get_type_path` method.
## Open Questions
- [x] ~This PR currently does not migrate any usages of
`std::any::type_name` to use `bevy_reflect::TypePath` to ease the review
process. Should it?~ Migration will be left to a follow-up PR.
- [ ] This PR adds a lot of `#[derive(TypePath)]` and `T: TypePath` to
satisfy new bounds, mostly when deriving `TypeUuid`. Should we make
`TypePath` a supertrait of `TypeUuid`? [Should we remove `TypeUuid` in
favour of
`TypePath`?](
|
||
|
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> |
||
|
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>
|
||
|
Gino Valente
|
02fbf16c80 |
bevy_reflect: Add Reflect::into_reflect (#6502)
# Objective
Using `Reflect` we can easily switch between a specific reflection trait object, such as a `dyn Struct`, to a `dyn Reflect` object via `Reflect::as_reflect` or `Reflect::as_reflect_mut`.
```rust
fn do_something(value: &dyn Reflect) {/* ... */}
let foo: Box<dyn Struct> = Box::new(Foo::default());
do_something(foo.as_reflect());
```
However, there is no way to convert a _boxed_ reflection trait object to a `Box<dyn Reflect>`.
## Solution
Add a `Reflect::into_reflect` method which allows converting a boxed reflection trait object back into a boxed `Reflect` trait object.
```rust
fn do_something(value: Box<dyn Reflect>) {/* ... */}
let foo: Box<dyn Struct> = Box::new(Foo::default());
do_something(foo.into_reflect());
```
---
## Changelog
- Added `Reflect::into_reflect`
|
||
|
Hennadii Chernyshchyk
|
feebbc5ea9 |
Add reflect_owned (#6494)
# Objective There is no way to gen an owned value of `Reflect`. ## Solution Add it! This was originally a part of #6421, but @MrGVSV asked me to create a separate for it to implement reflect diffing. --- ## Changelog ### Added - `Reflect::reflect_owned` to get an owned version of `Reflect`. |
||
|
Carter Anderson
|
e5905379de |
Use new let-else syntax where possible (#6463)
# Objective Let-else syntax is now stable! ## Solution Use it where possible! |
||
|
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, ®istry); let bytes: Vec<u8> = postcard::to_allocvec(&serializer).unwrap(); println!("{:?}", bytes); // Output: [129, 217, 61, 98, ...] // === Deserialize! === // let deserializer = UntypedReflectDeserializer::new(®istry); 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> |
||
|
Gino Valente
|
a658bfef19 |
bevy_reflect: Reflect doc comments (#6234)
# Objective Resolves #6197 Make it so that doc comments can be retrieved via reflection. ## Solution Adds the new `documentation` feature to `bevy_reflect` (disabled by default). When enabled, documentation can be found using `TypeInfo::doc` for reflected types: ```rust /// Some struct. /// /// # Example /// /// ```ignore /// let some_struct = SomeStruct; /// ``` #[derive(Reflect)] struct SomeStruct; let info = <SomeStruct as Typed>::type_info(); assert_eq!( Some(" Some struct.\n\n # Example\n\n ```ignore\n let some_struct = SomeStruct;\n ```"), info.docs() ); ``` ### Notes for Reviewers The bulk of the files simply added the same 16 lines of code (with slightly different documentation). Most of the real changes occur in the `bevy_reflect_derive` files as well as in the added tests. --- ## Changelog * Added `documentation` feature to `bevy_reflect` * Added `TypeInfo::docs` method (and similar methods for all info types) |
||
|
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. |
||
|
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 |
||
|
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](
|
||
|
Afonso Lage
|
40982cd0a2 |
Make reflect_partial_eq return more accurate results (#5210)
# Objective Closes #5204 ## Solution - Followed @nicopap suggestion on https://github.com/bevyengine/bevy/pull/4761#discussion_r903982224 ## Changelog - [x] Updated [struct_trait]( |
||
|
PROMETHIA-27
|
c27a3cff6d |
Make Reflect safe to implement (#5010)
# Objective Currently, `Reflect` is unsafe to implement because of a contract in which `any` and `any_mut` must return `self`, or `downcast` will cause UB. This PR makes `Reflect` safe, makes `downcast` not use unsafe, and eliminates this contract. ## Solution This PR adds a method to `Reflect`, `any`. It also renames the old `any` to `as_any`. `any` now takes a `Box<Self>` and returns a `Box<dyn Any>`. --- ## Changelog ### Added: - `any()` method - `represents()` method ### Changed: - `Reflect` is now a safe trait - `downcast()` is now safe - The old `any` is now called `as_any`, and `any_mut` is now `as_mut_any` ## Migration Guide - Reflect derives should not have to change anything - Manual reflect impls will need to remove the `unsafe` keyword, add `any()` implementations, and rename the old `any` and `any_mut` to `as_any` and `as_mut_any`. - Calls to `any`/`any_mut` must be changed to `as_any`/`as_mut_any` ## Points of discussion: - Should renaming `any` be avoided and instead name the new method `any_box`? - ~~Could there be a performance regression from avoiding the unsafe? I doubt it, but this change does seem to introduce redundant checks.~~ - ~~Could/should `is` and `type_id()` be implemented differently? For example, moving `is` onto `Reflect` as an `fn(&self, TypeId) -> bool`~~ Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com> |
||
|
Gino Valente
|
e6f34ba47f |
bevy_reflect: Add statically available type info for reflected types (#4042)
# Objective > Resolves #4504 It can be helpful to have access to type information without requiring an instance of that type. Especially for `Reflect`, a lot of the gathered type information is known at compile-time and should not necessarily require an instance. ## Solution Created a dedicated `TypeInfo` enum to store static type information. All types that derive `Reflect` now also implement the newly created `Typed` trait: ```rust pub trait Typed: Reflect { fn type_info() -> &'static TypeInfo; } ``` > Note: This trait was made separate from `Reflect` due to `Sized` restrictions. If you only have access to a `dyn Reflect`, just call `.get_type_info()` on it. This new trait method on `Reflect` should return the same value as if you had called it statically. If all you have is a `TypeId` or type name, you can get the `TypeInfo` directly from the registry using the `TypeRegistry::get_type_info` method (assuming it was registered). ### Usage Below is an example of working with `TypeInfo`. As you can see, we don't have to generate an instance of `MyTupleStruct` in order to get this information. ```rust #[derive(Reflect)] struct MyTupleStruct(usize, i32, MyStruct); let info = MyTupleStruct::type_info(); if let TypeInfo::TupleStruct(info) = info { assert!(info.is::<MyTupleStruct>()); assert_eq!(std::any::type_name::<MyTupleStruct>(), info.type_name()); assert!(info.field_at(1).unwrap().is::<i32>()); } else { panic!("Expected `TypeInfo::TupleStruct`"); } ``` ### Manual Implementations It's not recommended to manually implement `Typed` yourself, but if you must, you can use the `TypeInfoCell` to automatically create and manage the static `TypeInfo`s for you (which is very helpful for blanket/generic impls): ```rust use bevy_reflect::{Reflect, TupleStructInfo, TypeInfo, UnnamedField}; use bevy_reflect::utility::TypeInfoCell; struct Foo<T: Reflect>(T); impl<T: Reflect> Typed for Foo<T> { fn type_info() -> &'static TypeInfo { static CELL: TypeInfoCell = TypeInfoCell::generic(); CELL.get_or_insert::<Self, _>(|| { let fields = [UnnamedField:🆕:<T>()]; let info = TupleStructInfo:🆕:<Self>(&fields); TypeInfo::TupleStruct(info) }) } } ``` ## Benefits One major benefit is that this opens the door to other serialization methods. Since we can get all the type info at compile time, we can know how to properly deserialize something like: ```rust #[derive(Reflect)] struct MyType { foo: usize, bar: Vec<String> } // RON to be deserialized: ( type: "my_crate::MyType", // <- We now know how to deserialize the rest of this object value: { // "foo" is a value type matching "usize" "foo": 123, // "bar" is a list type matching "Vec<String>" with item type "String" "bar": ["a", "b", "c"] } ) ``` Not only is this more compact, but it has better compatibility (we can change the type of `"foo"` to `i32` without having to update our serialized data). Of course, serialization/deserialization strategies like this may need to be discussed and fully considered before possibly making a change. However, we will be better equipped to do that now that we can access type information right from the registry. ## Discussion Some items to discuss: 1. Duplication. There's a bit of overlap with the existing traits/structs since they require an instance of the type while the type info structs do not (for example, `Struct::field_at(&self, index: usize)` and `StructInfo::field_at(&self, index: usize)`, though only `StructInfo` is accessible without an instance object). Is this okay, or do we want to handle it in another way? 2. Should `TypeInfo::Dynamic` be removed? Since the dynamic types don't have type information available at runtime, we could consider them `TypeInfo::Value`s (or just even just `TypeInfo::Struct`). The intention with `TypeInfo::Dynamic` was to keep the distinction from these dynamic types and actual structs/values since users might incorrectly believe the methods of the dynamic type's info struct would map to some contained data (which isn't possible statically). 4. General usefulness of this change, including missing/unnecessary parts. 5. Possible changes to the scene format? (One possible issue with changing it like in the example above might be that we'd have to be careful when handling generic or trait object types.) ## Compile Tests I ran a few tests to compare compile times (as suggested [here](https://github.com/bevyengine/bevy/pull/4042#discussion_r876408143)). I toggled `Reflect` and `FromReflect` derive macros using `cfg_attr` for both this PR ( |
||
|
Gino Valente
|
2f5591ff8c |
bevy_reflect: Improve debug formatting for reflected types (#4218)
# Objective
Debugging reflected types can be somewhat frustrating since all `dyn Reflect` trait objects return something like `Reflect(core::option::Option<alloc::string::String>)`.
It would be much nicer to be able to see the actual value— or even use a custom `Debug` implementation.
## Solution
Added `Reflect::debug` which allows users to customize the debug output. It sets defaults for all `ReflectRef` subtraits and falls back to `Reflect(type_name)` if no `Debug` implementation was registered.
To register a custom `Debug` impl, users can add `#[reflect(Debug)]` like they can with other traits.
### Example
Using the following structs:
```rust
#[derive(Reflect)]
pub struct Foo {
a: usize,
nested: Bar,
#[reflect(ignore)]
_ignored: NonReflectedValue,
}
#[derive(Reflect)]
pub struct Bar {
value: Vec2,
tuple_value: (i32, String),
list_value: Vec<usize>,
// We can't determine debug formatting for Option<T> yet
unknown_value: Option<String>,
custom_debug: CustomDebug
}
#[derive(Reflect)]
#[reflect(Debug)]
struct CustomDebug;
impl Debug for CustomDebug {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "This is a custom debug!")
}
}
pub struct NonReflectedValue {
_a: usize,
}
```
We can do:
```rust
let value = Foo {
a: 1,
_ignored: NonReflectedValue { _a: 10 },
nested: Bar {
value: Vec2::new(1.23, 3.21),
tuple_value: (123, String::from("Hello")),
list_value: vec![1, 2, 3],
unknown_value: Some(String::from("World")),
custom_debug: CustomDebug
},
};
let reflected_value: &dyn Reflect = &value;
println!("{:#?}", reflected_value)
```
Which results in:
```rust
Foo {
a: 2,
nested: Bar {
value: Vec2(
1.23,
3.21,
),
tuple_value: (
123,
"Hello",
),
list_value: [
1,
2,
3,
],
unknown_value: Reflect(core::option::Option<alloc::string::String>),
custom_debug: This is a custom debug!,
},
}
```
Notice that neither `Foo` nor `Bar` implement `Debug`, yet we can still deduce it. This might be a concern if we're worried about leaking internal values. If it is, we might want to consider a way to exclude fields (possibly with a `#[reflect(hide)]` macro) or make it purely opt in (as opposed to the default implementation automatically handled by ReflectRef subtraits).
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
|
||
|
MrGVSV
|
15acd6f45d |
bevy_reflect: Small refactor and default Reflect methods (#4739)
# Objective Quick followup to #4712. While updating some [other PRs](https://github.com/bevyengine/bevy/pull/4218), I realized the `ReflectTraits` struct could be improved. The issue with the current implementation is that `ReflectTraits::get_xxx_impl(...)` returns just the _logic_ to the corresponding `Reflect` trait method, rather than the entire function. This makes it slightly more annoying to manage since the variable names need to be consistent across files. For example, `get_partial_eq_impl` uses a `value` variable. But the name "value" isn't defined in the `get_partial_eq_impl` method, it's defined in three other methods in a completely separate file. It's not likely to cause any bugs if we keep it as it is since differing variable names will probably just result in a compile error (except in very particular cases). But it would be useful to someone who wanted to edit/add/remove a method. ## Solution Made `get_hash_impl`, `get_partial_eq_impl` and `get_serialize_impl` return the entire method implementation for `reflect_hash`, `reflect_partial_eq`, and `serializable`, respectively. As a result of this, those three `Reflect` methods were also given default implementations. This was fairly simple to do since all three could just be made to return `None`. --- ## Changelog * Small cleanup/refactor to `ReflectTraits` in `bevy_reflect_derive` * Gave `Reflect::reflect_hash`, `Reflect::reflect_partial_eq`, and `Reflect::serializable` default implementations |
||
|
MrGVSV
|
5047e1f08e |
bevy_reflect: Add as_reflect and as_reflect_mut (#4350)
# Objective
Trait objects that have `Reflect` as a supertrait cannot be upcast to a `dyn Reflect`.
Attempting something like:
```rust
trait MyTrait: Reflect {
// ...
}
fn foo(value: &dyn MyTrait) {
let reflected = value as &dyn Reflect; // Error!
// ...
}
```
Results in `error[E0658]: trait upcasting coercion is experimental`.
The reason this is important is that a lot of `bevy_reflect` methods require a `&dyn Reflect`. This is trivial with concrete types, but if we don't know the concrete type (we only have the trait object), we can't use these methods. For example, we couldn't create a `ReflectSerializer` for the type since it expects a `&dyn Reflect` value— even though we should be able to.
## Solution
Add `as_reflect` and `as_reflect_mut` to `Reflect` to allow upcasting to a `dyn Reflect`:
```rust
trait MyTrait: Reflect {
// ...
}
fn foo(value: &dyn MyTrait) {
let reflected = value.as_reflect();
// ...
}
```
## Alternatives
We could defer this type of logic to the crate/user. They can add these methods to their trait in the same exact way we do here. The main benefit of doing it ourselves is it makes things convenient for them (especially when using the derive macro).
We could also create an `AsReflect` trait with a blanket impl over all reflected types, however, I could not get that to work for trait objects since they aren't sized.
---
## Changelog
- Added trait method `Reflect::as_reflect(&self)`
- Added trait method `Reflect::as_reflect_mut(&mut self)`
## Migration Guide
- Manual implementors of `Reflect` will need to add implementations for the methods above (this should be pretty easy as most cases just need to return `self`)
|
||
|
Carter Anderson
|
b3a1db60f2 |
Proper prehashing (#3963)
For some keys, it is too expensive to hash them on every lookup. Historically in Bevy, we have regrettably done the "wrong" thing in these cases (pre-computing hashes, then re-hashing them) because Rust's built in hashed collections don't give us the tools we need to do otherwise. Doing this is "wrong" because two different values can result in the same hash. Hashed collections generally get around this by falling back to equality checks on hash collisions. You can't do that if the key _is_ the hash. Additionally, re-hashing a hash increase the odds of collision! #3959 needs pre-hashing to be viable, so I decided to finally properly solve the problem. The solution involves two different changes: 1. A new generalized "pre-hashing" solution in bevy_utils: `Hashed<T>` types, which store a value alongside a pre-computed hash. And `PreHashMap<K, V>` (which uses `Hashed<T>` internally) . `PreHashMap` is just an alias for a normal HashMap that uses `Hashed<T>` as the key and a new `PassHash` implementation as the Hasher. 2. Replacing the `std::collections` re-exports in `bevy_utils` with equivalent `hashbrown` impls. Avoiding re-hashes requires the `raw_entry_mut` api, which isn't stabilized yet (and may never be ... `entry_ref` has favor now, but also isn't available yet). If std's HashMap ever provides the tools we need, we can move back to that. The latest version of `hashbrown` adds support for the `entity_ref` api, so we can move to that in preparation for an std migration, if thats the direction they seem to be going in. Note that adding hashbrown doesn't increase our dependency count because it was already in our tree. In addition to providing these core tools, I also ported the "table identity hashing" in `bevy_ecs` to `raw_entry_mut`, which was a particularly egregious case. The biggest outstanding case is `AssetPathId`, which stores a pre-hash. We need AssetPathId to be cheaply clone-able (and ideally Copy), but `Hashed<AssetPath>` requires ownership of the AssetPath, which makes cloning ids way more expensive. We could consider doing `Hashed<Arc<AssetPath>>`, but cloning an arc is still a non-trivial expensive that needs to be considered. I would like to handle this in a separate PR. And given that we will be re-evaluating the Bevy Assets implementation in the very near future, I'd prefer to hold off until after that conversation is concluded. |