efda7f3f9c
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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
|
90bb1adeb2
|
bevy_reflect: Contextual serialization error messages (#13888)
# Objective Reflection serialization can be difficult to debug. A lot of times a type fails to be serialized and the user is left wondering where that type came from. This is most often encountered with Bevy's scenes. Attempting to serialize all resources in the world will fail because some resources can't be serialized. For example, users will often get complaints about `bevy_utils::Instant` not registering `ReflectSerialize`. Well, `Instant` can't be serialized, so the only other option is to exclude the resource that contains it. But what resource contains it? This is where reflection serialization can get a little tricky (it's `Time<Real>` btw). ## Solution Add the `debug_stack` feature to `bevy_reflect`. When enabled, the reflection serializers and deserializers will keep track of the current type stack. And this stack will be used in error messages to help with debugging. Now, if we unknowingly try to serialize `Time<Real>`, we'll get the following error: ``` type `bevy_utils::Instant` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data` (stack: `bevy_time::time::Time<bevy_time::real::Real>` -> `bevy_time::real::Real` -> `bevy_utils::Instant`) ``` ### Implementation This makes use of `thread_local!` to manage an internal `TypeInfoStack` which holds a stack of `&'static TypeInfo`. We push to the stack before a type is (de)serialized and pop from the stack afterwards. Using a thread-local should be fine since we know two (de)serializers can't be running at the same time (and if they're running on separate threads, then we're still good). The only potential issue would be if a user went through one of the sub-serializers, like `StructSerializer`. However, I don't think many users are going through these types (I don't even know if we necessarily want to keep those public either, but we'll save that for a different PR). Additionally, this is just a debug feature that only affects error messages, so it wouldn't have any drastically negative effect. It would just result in the stack not being cleared properly if there were any errors. Lastly, this is not the most performant implementation since we now fetch the `TypeInfo` an extra time. But I figured that for a debug tool, it wouldn't matter too much. ### Feature This also adds a `debug` feature, which enables the `debug_stack` feature. I added it because I think we may want to potentially add more debug tools in the future, and this gives us a good framework for adding those. Users who want all debug features, present and future, can just set `debug`. If they only want this feature, then they can just use `debug_stack`. I also made the `debug` feature default to help capture the widest audience (i.e. the users who want this feature but don't know they do). However, if we think it's better as a non-default feature, I can change it! And if there's any bikeshedding around the name `debug_stack`, let me know! ## Testing Run the following command: ``` cargo test --package bevy_reflect --features debug_stack ``` --- ## Changelog - Added the `debug` and `debug_stack` features to `bevy_reflect` - Updated the error messages returned by the reflection serializers and deserializers to include more contextual information when the `debug_stack` or `debug` feature is enabled |
||
|
Gino Valente
|
ba3d9b3fb6
|
bevy_reflect: Refactor serde module (#15107)
# Objective
The `ser` and `de` modules in `bevy_reflect/serde` are very long and
difficult to navigate.
## Solution
Refactor both modules into many smaller modules that each have a single
primary focus (i.e. a `structs` module that only handles struct
serialization/deserialization).
I chose to keep the `ser` and `de` modules separate. We could have
instead broken it up kind (e.g. lists, maps, etc.), but I think this is
a little cleaner. Serialization and deserialization, while related, can
be very different. So keeping them separated makes sense for
organizational purposes.
That being said, if people disagree and think we should structure this a
different way, I am open to changing it.
Note that this PR's changes are mainly structural. There are a few
places I refactored code to reduce duplication and to make things a bit
cleaner, but these are largely cosmetic and shouldn't have any impact on
behavior.
### Other Details
This PR also hides a lot of the internal logic from being exported.
These were originally public, but it's unlikely they really saw any use
outside of these modules. In fact, you don't really gain anything by
using them outside of this module either.
By privatizing these fields and items, we also set ourselves up for more
easily changing internal logic around without involving a breaking
change.
I also chose not to mess around with tests since that would really blow
up the diff haha.
## Testing
You can test locally by running:
```
cargo test --package bevy_reflect --all-features
```
---
## Migration Guide
The fields on `ReflectSerializer` and `TypedReflectSerializer` are now
private. To instantiate, the corresponding constructor must be used:
```rust
// BEFORE
let serializer = ReflectSerializer {
value: &my_value,
registry: &type_registry,
};
// AFTER
let serializer = ReflectSerializer::new(&my_value, &type_registry);
```
Additionally, the following types are no longer public:
- `ArraySerializer`
- `EnumSerializer`
- `ListSerializer`
- `MapSerializer`
- `ReflectValueSerializer` (fully removed)
- `StructSerializer`
- `TupleSerializer`
- `TupleStructSerializer`
As well as the following traits:
- `DeserializeValue` (fully removed)
|