Skip to content

Commit b6a5ca6

Browse files
MrGVSVShatur
authored andcommitted
bevy_reflect: Fix binary deserialization not working for unit structs (bevyengine#6722) [skip ci]
Fixes bevyengine#6713 Binary deserialization is failing for unit structs as well as structs with all ignored/skipped fields. Add a check for the number of possible fields in a struct before deserializing. If empty, don't attempt to deserialize any fields (as there will be none). Note: ~~This does not apply to enums as they do not properly handle skipped fields (see bevyengine#6721).~~ Enums still do not properly handle skipped fields, but I decided to include the logic for it anyways to account for `#[reflect(ignore)]`'d fields in the meantime. --- - Fix bug where deserializing unit structs would fail for non-self-describing formats
1 parent 920543c commit b6a5ca6

File tree

2 files changed

+193
-18
lines changed

2 files changed

+193
-18
lines changed

crates/bevy_reflect/src/serde/de.rs

Lines changed: 126 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
341341
struct_info.field_names(),
342342
StructVisitor {
343343
struct_info,
344+
registration: self.registration,
344345
registry: self.registry,
345346
},
346347
)?;
@@ -411,6 +412,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
411412
enum_info.variant_names(),
412413
EnumVisitor {
413414
enum_info,
415+
registration: self.registration,
414416
registry: self.registry,
415417
},
416418
)?
@@ -439,6 +441,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
439441

440442
struct StructVisitor<'a> {
441443
struct_info: &'static StructInfo,
444+
registration: &'a TypeRegistration,
442445
registry: &'a TypeRegistry,
443446
}
444447

@@ -463,6 +466,18 @@ impl<'a, 'de> Visitor<'de> for StructVisitor<'a> {
463466
let mut index = 0usize;
464467
let mut output = DynamicStruct::default();
465468

469+
let ignored_len = self
470+
.registration
471+
.data::<SerializationData>()
472+
.map(|data| data.len())
473+
.unwrap_or(0);
474+
let field_len = self.struct_info.field_len().saturating_sub(ignored_len);
475+
476+
if field_len == 0 {
477+
// Handle unit structs and ignored fields
478+
return Ok(output);
479+
}
480+
466481
while let Some(value) = seq.next_element_seed(TypedReflectDeserializer {
467482
registration: self
468483
.struct_info
@@ -501,6 +516,21 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> {
501516
let mut index = 0usize;
502517
let mut tuple_struct = DynamicTupleStruct::default();
503518

519+
let ignored_len = self
520+
.registration
521+
.data::<SerializationData>()
522+
.map(|data| data.len())
523+
.unwrap_or(0);
524+
let field_len = self
525+
.tuple_struct_info
526+
.field_len()
527+
.saturating_sub(ignored_len);
528+
529+
if field_len == 0 {
530+
// Handle unit structs and ignored fields
531+
return Ok(tuple_struct);
532+
}
533+
504534
let get_field_registration = |index: usize| -> Result<&'a TypeRegistration, V::Error> {
505535
let field = self.tuple_struct_info.field_at(index).ok_or_else(|| {
506536
de::Error::custom(format_args!(
@@ -675,6 +705,7 @@ impl<'a, 'de> Visitor<'de> for MapVisitor<'a> {
675705

676706
struct EnumVisitor<'a> {
677707
enum_info: &'static EnumInfo,
708+
registration: &'a TypeRegistration,
678709
registry: &'a TypeRegistry,
679710
}
680711

@@ -701,6 +732,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> {
701732
struct_info.field_names(),
702733
StructVariantVisitor {
703734
struct_info,
735+
registration: self.registration,
704736
registry: self.registry,
705737
},
706738
)?
@@ -722,6 +754,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> {
722754
tuple_info.field_len(),
723755
TupleVariantVisitor {
724756
tuple_info,
757+
registration: self.registration,
725758
registry: self.registry,
726759
},
727760
)?
@@ -787,6 +820,7 @@ impl<'de> DeserializeSeed<'de> for VariantDeserializer {
787820

788821
struct StructVariantVisitor<'a> {
789822
struct_info: &'static StructVariantInfo,
823+
registration: &'a TypeRegistration,
790824
registry: &'a TypeRegistry,
791825
}
792826

@@ -811,6 +845,18 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> {
811845
let mut index = 0usize;
812846
let mut output = DynamicStruct::default();
813847

848+
let ignored_len = self
849+
.registration
850+
.data::<SerializationData>()
851+
.map(|data| data.len())
852+
.unwrap_or(0);
853+
let field_len = self.struct_info.field_len().saturating_sub(ignored_len);
854+
855+
if field_len == 0 {
856+
// Handle all fields being ignored
857+
return Ok(output);
858+
}
859+
814860
while let Some(value) = seq.next_element_seed(TypedReflectDeserializer {
815861
registration: self
816862
.struct_info
@@ -831,6 +877,7 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> {
831877

832878
struct TupleVariantVisitor<'a> {
833879
tuple_info: &'static TupleVariantInfo,
880+
registration: &'a TypeRegistration,
834881
registry: &'a TypeRegistry,
835882
}
836883

@@ -845,6 +892,18 @@ impl<'a, 'de> Visitor<'de> for TupleVariantVisitor<'a> {
845892
where
846893
V: SeqAccess<'de>,
847894
{
895+
let ignored_len = self
896+
.registration
897+
.data::<SerializationData>()
898+
.map(|data| data.len())
899+
.unwrap_or(0);
900+
let field_len = self.tuple_info.field_len().saturating_sub(ignored_len);
901+
902+
if field_len == 0 {
903+
// Handle all fields being ignored
904+
return Ok(DynamicTuple::default());
905+
}
906+
848907
visit_tuple(&mut seq, self.tuple_info, self.registry)
849908
}
850909
}
@@ -1011,10 +1070,15 @@ mod tests {
10111070
map_value: HashMap<u8, usize>,
10121071
struct_value: SomeStruct,
10131072
tuple_struct_value: SomeTupleStruct,
1073+
unit_struct: SomeUnitStruct,
10141074
unit_enum: SomeEnum,
10151075
newtype_enum: SomeEnum,
10161076
tuple_enum: SomeEnum,
10171077
struct_enum: SomeEnum,
1078+
ignored_struct: SomeIgnoredStruct,
1079+
ignored_tuple_struct: SomeIgnoredTupleStruct,
1080+
ignored_struct_variant: SomeIgnoredEnum,
1081+
ignored_tuple_variant: SomeIgnoredEnum,
10181082
custom_deserialize: CustomDeserialize,
10191083
}
10201084

@@ -1026,6 +1090,18 @@ mod tests {
10261090
#[derive(Reflect, FromReflect, Debug, PartialEq)]
10271091
struct SomeTupleStruct(String);
10281092

1093+
#[derive(Reflect, FromReflect, Debug, PartialEq)]
1094+
struct SomeUnitStruct;
1095+
1096+
#[derive(Reflect, FromReflect, Debug, PartialEq)]
1097+
struct SomeIgnoredStruct {
1098+
#[reflect(ignore)]
1099+
ignored: i32,
1100+
}
1101+
1102+
#[derive(Reflect, FromReflect, Debug, PartialEq)]
1103+
struct SomeIgnoredTupleStruct(#[reflect(ignore)] i32);
1104+
10291105
#[derive(Reflect, FromReflect, Debug, PartialEq, Deserialize)]
10301106
struct SomeDeserializableStruct {
10311107
foo: i64,
@@ -1050,14 +1126,27 @@ mod tests {
10501126
Struct { foo: String },
10511127
}
10521128

1129+
#[derive(Reflect, FromReflect, Debug, PartialEq)]
1130+
enum SomeIgnoredEnum {
1131+
Tuple(#[reflect(ignore)] f32, #[reflect(ignore)] f32),
1132+
Struct {
1133+
#[reflect(ignore)]
1134+
foo: String,
1135+
},
1136+
}
1137+
10531138
fn get_registry() -> TypeRegistry {
10541139
let mut registry = TypeRegistry::default();
10551140
registry.register::<MyStruct>();
10561141
registry.register::<SomeStruct>();
10571142
registry.register::<SomeTupleStruct>();
1143+
registry.register::<SomeUnitStruct>();
1144+
registry.register::<SomeIgnoredStruct>();
1145+
registry.register::<SomeIgnoredTupleStruct>();
10581146
registry.register::<CustomDeserialize>();
10591147
registry.register::<SomeDeserializableStruct>();
10601148
registry.register::<SomeEnum>();
1149+
registry.register::<SomeIgnoredEnum>();
10611150
registry.register::<i8>();
10621151
registry.register::<String>();
10631152
registry.register::<i64>();
@@ -1090,12 +1179,19 @@ mod tests {
10901179
map_value: map,
10911180
struct_value: SomeStruct { foo: 999999999 },
10921181
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
1182+
unit_struct: SomeUnitStruct,
10931183
unit_enum: SomeEnum::Unit,
10941184
newtype_enum: SomeEnum::NewType(123),
10951185
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
10961186
struct_enum: SomeEnum::Struct {
10971187
foo: String::from("Struct variant value"),
10981188
},
1189+
ignored_struct: SomeIgnoredStruct { ignored: 0 },
1190+
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
1191+
ignored_struct_variant: SomeIgnoredEnum::Struct {
1192+
foo: String::default(),
1193+
},
1194+
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
10991195
custom_deserialize: CustomDeserialize {
11001196
value: 100,
11011197
inner_struct: SomeDeserializableStruct { foo: 101 },
@@ -1125,12 +1221,17 @@ mod tests {
11251221
foo: 999999999,
11261222
),
11271223
tuple_struct_value: ("Tuple Struct"),
1224+
unit_struct: (),
11281225
unit_enum: Unit,
11291226
newtype_enum: NewType(123),
11301227
tuple_enum: Tuple(1.23, 3.21),
11311228
struct_enum: Struct(
11321229
foo: "Struct variant value",
11331230
),
1231+
ignored_struct: (),
1232+
ignored_tuple_struct: (),
1233+
ignored_struct_variant: Struct(),
1234+
ignored_tuple_variant: Tuple(),
11341235
custom_deserialize: (
11351236
value: 100,
11361237
renamed: (
@@ -1337,12 +1438,19 @@ mod tests {
13371438
map_value: map,
13381439
struct_value: SomeStruct { foo: 999999999 },
13391440
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
1441+
unit_struct: SomeUnitStruct,
13401442
unit_enum: SomeEnum::Unit,
13411443
newtype_enum: SomeEnum::NewType(123),
13421444
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
13431445
struct_enum: SomeEnum::Struct {
13441446
foo: String::from("Struct variant value"),
13451447
},
1448+
ignored_struct: SomeIgnoredStruct { ignored: 0 },
1449+
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
1450+
ignored_struct_variant: SomeIgnoredEnum::Struct {
1451+
foo: String::default(),
1452+
},
1453+
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
13461454
custom_deserialize: CustomDeserialize {
13471455
value: 100,
13481456
inner_struct: SomeDeserializableStruct { foo: 101 },
@@ -1363,7 +1471,8 @@ mod tests {
13631471
108, 101, 32, 83, 116, 114, 117, 99, 116, 0, 0, 0, 0, 1, 0, 0, 0, 123, 0, 0, 0, 0, 0,
13641472
0, 0, 2, 0, 0, 0, 164, 112, 157, 63, 164, 112, 77, 64, 3, 0, 0, 0, 20, 0, 0, 0, 0, 0,
13651473
0, 0, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97,
1366-
108, 117, 101, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0, 0,
1474+
108, 117, 101, 1, 0, 0, 0, 0, 0, 0, 0, 100, 0, 0, 0, 0, 0, 0, 0, 101, 0, 0, 0, 0, 0, 0,
1475+
0,
13671476
];
13681477

13691478
let deserializer = UntypedReflectDeserializer::new(&registry);
@@ -1392,12 +1501,19 @@ mod tests {
13921501
map_value: map,
13931502
struct_value: SomeStruct { foo: 999999999 },
13941503
tuple_struct_value: SomeTupleStruct(String::from("Tuple Struct")),
1504+
unit_struct: SomeUnitStruct,
13951505
unit_enum: SomeEnum::Unit,
13961506
newtype_enum: SomeEnum::NewType(123),
13971507
tuple_enum: SomeEnum::Tuple(1.23, 3.21),
13981508
struct_enum: SomeEnum::Struct {
13991509
foo: String::from("Struct variant value"),
14001510
},
1511+
ignored_struct: SomeIgnoredStruct { ignored: 0 },
1512+
ignored_tuple_struct: SomeIgnoredTupleStruct(0),
1513+
ignored_struct_variant: SomeIgnoredEnum::Struct {
1514+
foo: String::default(),
1515+
},
1516+
ignored_tuple_variant: SomeIgnoredEnum::Tuple(0.0, 0.0),
14011517
custom_deserialize: CustomDeserialize {
14021518
value: 100,
14031519
inner_struct: SomeDeserializableStruct { foo: 101 },
@@ -1409,14 +1525,15 @@ mod tests {
14091525
let input = vec![
14101526
129, 217, 40, 98, 101, 118, 121, 95, 114, 101, 102, 108, 101, 99, 116, 58, 58, 115,
14111527
101, 114, 100, 101, 58, 58, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121,
1412-
83, 116, 114, 117, 99, 116, 158, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111, 114,
1413-
108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0, 1, 2,
1414-
149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172, 84, 117,
1415-
112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 164, 85, 110, 105, 116, 129, 167, 78,
1416-
101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146, 202, 63, 157,
1417-
112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116, 145, 180, 83,
1418-
116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97, 108, 117,
1419-
101, 146, 100, 145, 101,
1528+
83, 116, 114, 117, 99, 116, 220, 0, 19, 123, 172, 72, 101, 108, 108, 111, 32, 119, 111,
1529+
114, 108, 100, 33, 145, 123, 146, 202, 64, 73, 15, 219, 205, 5, 57, 149, 254, 255, 0,
1530+
1, 2, 149, 254, 255, 0, 1, 2, 129, 64, 32, 145, 206, 59, 154, 201, 255, 145, 172, 84,
1531+
117, 112, 108, 101, 32, 83, 116, 114, 117, 99, 116, 144, 164, 85, 110, 105, 116, 129,
1532+
167, 78, 101, 119, 84, 121, 112, 101, 123, 129, 165, 84, 117, 112, 108, 101, 146, 202,
1533+
63, 157, 112, 164, 202, 64, 77, 112, 164, 129, 166, 83, 116, 114, 117, 99, 116, 145,
1534+
180, 83, 116, 114, 117, 99, 116, 32, 118, 97, 114, 105, 97, 110, 116, 32, 118, 97, 108,
1535+
117, 101, 144, 144, 129, 166, 83, 116, 114, 117, 99, 116, 144, 129, 165, 84, 117, 112,
1536+
108, 101, 144, 146, 100, 145, 101,
14201537
];
14211538

14221539
let deserializer = UntypedReflectDeserializer::new(&registry);

0 commit comments

Comments
 (0)