-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bevy_reflect: Fix dynamic type serialization #10103
bevy_reflect: Fix dynamic type serialization #10103
Conversation
5946fb2
to
fce22cc
Compare
@raffaeleragni, if you feel equipped, your review / approval would be appreciated here :) |
This also has a change to |
fce22cc
to
44afc01
Compare
Pointing to the fork & branch. (my branch is https://github.com/raffaeleragni/bevy_sync/tree/bevy_0_12) |
I see in the new test DynamicStruct is asserted directly, but how do you bring back the TestStruct from deserialization instead? Without knowing its type at compile time, that is. |
My test runs fine too now, thanks. |
As an extra, but this could be unrelated to this PR and require a new one, not sure: https://github.com/raffaeleragni/bevy_sync/blob/bevy_0_12/src/proto_serde.rs#L148
|
This seems possibly unrelated. Could you open a new issue? |
Sure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! i like this direction.
@MrGVSV those CI failures look real; failing bevy_reflect test. |
318fb2e
to
7d1d802
Compare
# Objective Fixes bevyengine#10086 ## Solution Instead of serializing via `DynamicTypePath::reflect_type_path`, now uses the `TypePath` found on the `TypeInfo` returned by `Reflect::get_represented_type_info`. This issue was happening because the dynamic types implement `TypePath` themselves and do not (and cannot) forward their proxy's `TypePath` data. The solution was to access the proxy's type information in order to get the correct `TypePath` data. ## Changed - The `Debug` impl for `TypePathTable` now includes output for all fields.
# Objective Fixes bevyengine#10086 ## Solution Instead of serializing via `DynamicTypePath::reflect_type_path`, now uses the `TypePath` found on the `TypeInfo` returned by `Reflect::get_represented_type_info`. This issue was happening because the dynamic types implement `TypePath` themselves and do not (and cannot) forward their proxy's `TypePath` data. The solution was to access the proxy's type information in order to get the correct `TypePath` data. ## Changed - The `Debug` impl for `TypePathTable` now includes output for all fields.
Objective
Fixes #10086
Solution
Instead of serializing via
DynamicTypePath::reflect_type_path
, now uses theTypePath
found on theTypeInfo
returned byReflect::get_represented_type_info
.This issue was happening because the dynamic types implement
TypePath
themselves and do not (and cannot) forward their proxy'sTypePath
data. The solution was to access the proxy's type information in order to get the correctTypePath
data.Changed
Debug
impl forTypePathTable
now includes output for all fields.