Skip to content

[Merged by Bors] - bevy_reflect: Improve serialization format even more #5723

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.

Sign up for GitHub

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

Closed
wants to merge 41 commits into from
Closed

[Merged by Bors] - bevy_reflect: Improve serialization format even more #5723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4230acd
Simplified reflection ser/de
MrGVSV Mar 20, 2022
32a83b9
Cleanup comment
MrGVSV Apr 22, 2022
e8bdefb
Updated example scene file
MrGVSV Apr 22, 2022
3dd5d0e
Fix clippy
MrGVSV Jun 1, 2022
f794ad2
Fix glam tests
MrGVSV Jun 1, 2022
f843355
Fixed tests
MrGVSV Jun 1, 2022
762fe4f
Remove TODO comment
MrGVSV Jun 11, 2022
31135e6
Fix test for CI
MrGVSV Jun 11, 2022
5e3ba1a
Fix CI test for glam serialization
MrGVSV Jun 12, 2022
8d19407
Replace deserialize_any with deserialize_map
MrGVSV Jun 27, 2022
83829eb
Remove unnecessary paths
MrGVSV Jun 27, 2022
54aca19
Fix PrettyConfig for tests
MrGVSV Jun 27, 2022
14105da
Rename ReflectDeserializer -> UntypedReflectDeserializer
MrGVSV Jun 28, 2022
6966a58
Add constructor to TypedReflectDeserializer
MrGVSV Jun 28, 2022
bafda23
Added test for simple value types
MrGVSV Jun 28, 2022
93b9679
Reorganize Visitor impls
MrGVSV Jun 28, 2022
d133167
Formatting
MrGVSV Jun 28, 2022
a44df72
Revert "Replace deserialize_any with deserialize_map"
MrGVSV Jun 29, 2022
88668ae
Remove "type" and "value" keys
MrGVSV Aug 8, 2022
5b88e74
Add nested enums to tests
MrGVSV Aug 8, 2022
bcb7a0a
Fix scene example
MrGVSV Aug 10, 2022
5d2c99e
Add more complex Option value to tests
MrGVSV Aug 17, 2022
733b580
Fix doc comment
MrGVSV Aug 17, 2022
db1227e
Apply review comment
MrGVSV Aug 29, 2022
dd9b319
Improved reflect serialization v2
MrGVSV Aug 14, 2022
94baee8
Improve error message for unknown fields on struct-likes
MrGVSV Aug 17, 2022
cb37f2e
Require 'static on all type info
MrGVSV Aug 17, 2022
98458ba
Improve error message
MrGVSV Aug 17, 2022
ca68a37
Update scene file
MrGVSV Aug 17, 2022
a2b6e2e
Allow Dynamic types to be serialized again
MrGVSV Aug 17, 2022
43fb3ab
Formatting
MrGVSV Aug 17, 2022
df99b96
Support Option types
MrGVSV Aug 18, 2022
509cb9f
Fix clippy errors
MrGVSV Aug 18, 2022
61a742b
Fix glam tests
MrGVSV Aug 18, 2022
070e509
Remove unnecessary Into
MrGVSV Sep 1, 2022
320d9e4
Fix getting registration for type twice
MrGVSV Sep 1, 2022
1d34ea5
Added Enum::variant_index
MrGVSV Sep 1, 2022
265cc30
Remove duplicate field_names/variant_names fields
MrGVSV Sep 1, 2022
6430af4
Simplify ExpectedValues struct
MrGVSV Sep 1, 2022
fe2ee1c
Fix doc error
MrGVSV Sep 5, 2022
8f6ed32
Pass in TypeRegistration to cut out redundant lookup
cart Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 24 additions & 44 deletions assets/scenes/load_scene_example.scn.ron
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,61 +3,41 @@
entity: 0,
components: [
{
"type": "bevy_transform::components::transform::Transform",
"struct": {
"translation": {
"type": "glam::f32::vec3::Vec3",
"value": (0.0, 0.0, 0.0),
},
"rotation": {
"type": "glam::f32::sse2::quat::Quat",
"value": (0.0, 0.0, 0.0, 1.0),
},
"scale": {
"type": "glam::f32::vec3::Vec3",
"value": (1.0, 1.0, 1.0),
},
},
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
},
{
"type": "scene::ComponentB",
"struct": {
"value": {
"type": "alloc::string::String",
"value": "hello",
},
},
"scene::ComponentB": (
value: "hello",
),
},
{
"type": "scene::ComponentA",
"struct": {
"x": {
"type": "f32",
"value": 1.0,
},
"y": {
"type": "f32",
"value": 2.0,
},
},
"scene::ComponentA": (
x: 1.0,
y: 2.0,
),
},
],
),
(
entity: 1,
components: [
{
"type": "scene::ComponentA",
"struct": {
"x": {
"type": "f32",
"value": 3.0,
},
"y": {
"type": "f32",
"value": 4.0,
},
},
"scene::ComponentA": (
x: 3.0,
y: 4.0,
),
},
],
),
Expand Down
29 changes: 23 additions & 6 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
enum_name_at,
enum_field_len,
enum_variant_name,
enum_variant_index,
enum_variant_type,
} = generate_impls(reflect_enum, &ref_index, &ref_name);

Expand Down Expand Up @@ -53,12 +54,13 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
}
});

let string_name = enum_name.to_string();
let typed_impl = impl_typed(
enum_name,
reflect_enum.meta().generics(),
quote! {
let variants = [#(#variant_info),*];
let info = #bevy_reflect_path::EnumInfo::new::<Self>(&variants);
let info = #bevy_reflect_path::EnumInfo::new::<Self>(#string_name, &variants);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing the static "short" type name in EnumInfo/StructInfo might be (and maybe should be) redundant with efforts like #5805. We might want to split that pr's impl into:

impl TypePath {
  fn short_type_name() -> &'static str;
  fn module() -> &'static str;
  fn type_path() -> &'static str;
}

That being said, this seems fine for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh that's a good thought for #5805 actually. But yeah when that lands we can probably make these constructors take T: Reflect + TypePath instead.

#bevy_reflect_path::TypeInfo::Enum(info)
},
bevy_reflect_path,
Expand Down Expand Up @@ -136,6 +138,14 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
}
}

#[inline]
fn variant_index(&self) -> usize {
match self {
#(#enum_variant_index,)*
_ => unreachable!(),
}
}

#[inline]
fn variant_type(&self) -> #bevy_reflect_path::VariantType {
match self {
Expand Down Expand Up @@ -254,6 +264,7 @@ struct EnumImpls {
enum_name_at: Vec<proc_macro2::TokenStream>,
enum_field_len: Vec<proc_macro2::TokenStream>,
enum_variant_name: Vec<proc_macro2::TokenStream>,
enum_variant_index: Vec<proc_macro2::TokenStream>,
enum_variant_type: Vec<proc_macro2::TokenStream>,
}

Expand All @@ -267,13 +278,21 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
let mut enum_name_at = Vec::new();
let mut enum_field_len = Vec::new();
let mut enum_variant_name = Vec::new();
let mut enum_variant_index = Vec::new();
let mut enum_variant_type = Vec::new();

for variant in reflect_enum.variants() {
for (variant_index, variant) in reflect_enum.variants().iter().enumerate() {
let ident = &variant.data.ident;
let name = ident.to_string();
let unit = reflect_enum.get_unit(ident);

enum_variant_name.push(quote! {
#unit{..} => #name
});
enum_variant_index.push(quote! {
#unit{..} => #variant_index
});

fn for_fields(
fields: &[StructField],
mut generate_for_field: impl FnMut(usize, usize, &StructField) -> proc_macro2::TokenStream,
Expand Down Expand Up @@ -301,9 +320,6 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
enum_field_len.push(quote! {
#unit{..} => #field_len
});
enum_variant_name.push(quote! {
#unit{..} => #name
});
enum_variant_type.push(quote! {
#unit{..} => #bevy_reflect_path::VariantType::#variant
});
Expand Down Expand Up @@ -342,7 +358,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
});

let field_ty = &field.data.ty;
quote! { #bevy_reflect_path::NamedField::new::<#field_ty, _>(#field_name) }
quote! { #bevy_reflect_path::NamedField::new::<#field_ty>(#field_name) }
});
let arguments = quote!(#name, &[ #(#argument),* ]);
add_fields_branch("Struct", "StructVariantInfo", arguments, field_len);
Expand All @@ -358,6 +374,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
enum_name_at,
enum_field_len,
enum_variant_name,
enum_variant_index,
enum_variant_type,
}
}
5 changes: 3 additions & 2 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
});

let string_name = struct_name.to_string();
let typed_impl = impl_typed(
struct_name,
reflect_struct.meta().generics(),
quote! {
let fields = [
#(#bevy_reflect_path::NamedField::new::<#field_types, _>(#field_names),)*
#(#bevy_reflect_path::NamedField::new::<#field_types>(#field_names),)*
];
let info = #bevy_reflect_path::StructInfo::new::<Self>(&fields);
let info = #bevy_reflect_path::StructInfo::new::<Self>(#string_name, &fields);
#bevy_reflect_path::TypeInfo::Struct(info)
},
bevy_reflect_path,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
});

let string_name = struct_name.to_string();
let typed_impl = impl_typed(
struct_name,
reflect_struct.meta().generics(),
quote! {
let fields = [
#(#bevy_reflect_path::UnnamedField::new::<#field_types>(#field_idents),)*
];
let info = #bevy_reflect_path::TupleStructInfo::new::<Self>(&fields);
let info = #bevy_reflect_path::TupleStructInfo::new::<Self>(#string_name, &fields);
#bevy_reflect_path::TypeInfo::TupleStruct(info)
},
bevy_reflect_path,
Expand Down
5 changes: 1 addition & 4 deletions crates/bevy_reflect/bevy_reflect_derive/src/utility.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ where
let mut bitset = BitSet::default();

member_iter.fold(0, |next_idx, member| match member {
ReflectIgnoreBehavior::IgnoreAlways => {
bitset.insert(next_idx);
next_idx
}
ReflectIgnoreBehavior::IgnoreAlways => next_idx,
ReflectIgnoreBehavior::IgnoreSerialization => {
bitset.insert(next_idx);
next_idx + 1
Expand Down
51 changes: 48 additions & 3 deletions crates/bevy_reflect/src/enums/dynamic_enum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl From<()> for DynamicVariant {
pub struct DynamicEnum {
name: String,
variant_name: String,
variant_index: usize,
variant: DynamicVariant,
}

Expand All @@ -96,6 +97,30 @@ impl DynamicEnum {
) -> Self {
Self {
name: name.into(),
variant_index: 0,
variant_name: variant_name.into(),
variant: variant.into(),
}
}

/// Create a new [`DynamicEnum`] with a variant index to represent an enum at runtime.
///
/// # Arguments
///
/// * `name`: The type name of the enum
/// * `variant_index`: The index of the variant to set
/// * `variant_name`: The name of the variant to set
/// * `variant`: The variant data
///
pub fn new_with_index<I: Into<String>, V: Into<DynamicVariant>>(
name: I,
variant_index: usize,
variant_name: I,
variant: V,
) -> Self {
Self {
name: name.into(),
variant_index,
variant_name: variant_name.into(),
variant: variant.into(),
}
Expand All @@ -117,6 +142,18 @@ impl DynamicEnum {
self.variant = variant.into();
}

/// Set the current enum variant represented by this struct along with its variant index.
pub fn set_variant_with_index<I: Into<String>, V: Into<DynamicVariant>>(
&mut self,
variant_index: usize,
name: I,
variant: V,
) {
self.variant_index = variant_index;
self.variant_name = name.into();
self.variant = variant.into();
}

/// Create a [`DynamicEnum`] from an existing one.
///
/// This is functionally the same as [`DynamicEnum::from_ref`] except it takes an owned value.
Expand All @@ -129,8 +166,9 @@ impl DynamicEnum {
/// This is functionally the same as [`DynamicEnum::from`] except it takes a reference.
pub fn from_ref<TEnum: Enum>(value: &TEnum) -> Self {
match value.variant_type() {
VariantType::Unit => DynamicEnum::new(
VariantType::Unit => DynamicEnum::new_with_index(
value.type_name(),
value.variant_index(),
value.variant_name(),
DynamicVariant::Unit,
),
Expand All @@ -139,8 +177,9 @@ impl DynamicEnum {
for field in value.iter_fields() {
data.insert_boxed(field.value().clone_value());
}
DynamicEnum::new(
DynamicEnum::new_with_index(
value.type_name(),
value.variant_index(),
value.variant_name(),
DynamicVariant::Tuple(data),
)
Expand All @@ -151,8 +190,9 @@ impl DynamicEnum {
let name = field.name().unwrap();
data.insert_boxed(name, field.value().clone_value());
}
DynamicEnum::new(
DynamicEnum::new_with_index(
value.type_name(),
value.variant_index(),
value.variant_name(),
DynamicVariant::Struct(data),
)
Expand Down Expand Up @@ -226,6 +266,10 @@ impl Enum for DynamicEnum {
&self.variant_name
}

fn variant_index(&self) -> usize {
self.variant_index
}

fn variant_type(&self) -> VariantType {
match &self.variant {
DynamicVariant::Unit => VariantType::Unit,
Expand All @@ -237,6 +281,7 @@ impl Enum for DynamicEnum {
fn clone_dynamic(&self) -> DynamicEnum {
Self {
name: self.name.clone(),
variant_index: self.variant_index,
variant_name: self.variant_name.clone(),
variant: self.variant.clone(),
}
Expand Down
Loading