Skip to content

Commit

Permalink
Fix issue with Option serialization (#10705)
Browse files Browse the repository at this point in the history
# Objective

- Fix #10499 

## Solution

- Use `.get_represented_type_info()` module path and type ident instead
of `.reflect_*` module path and type ident when serializing the `Option`
enum

---

## Changelog

- Fix serialization bug
- Add simple test
  - Add `serde_json` dev dependency
- Add `serde` with `derive` feature dev dependency (wouldn't compile for
me without it)

---------

Co-authored-by: hank <hank@hank.co.in>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 23, 2023
1 parent 48af029 commit e85af0e
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 7 deletions.
2 changes: 2 additions & 0 deletions crates/bevy_reflect/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ smol_str = { version = "0.2.0", optional = true }
ron = "0.8.0"
rmp-serde = "1.1"
bincode = "1.3"
serde_json = "1.0"
serde = { version = "1", features = ["derive"] }

[[example]]
name = "reflect_docs"
Expand Down
52 changes: 45 additions & 7 deletions crates/bevy_reflect/src/serde/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ impl<'a> Serialize for EnumSerializer<'a> {

match variant_type {
VariantType::Unit => {
if self.enum_value.reflect_module_path() == Some("core::option")
&& self.enum_value.reflect_type_ident() == Some("Option")
if type_info.type_path_table().module_path() == Some("core::option")
&& type_info.type_path_table().ident() == Some("Option")
{
serializer.serialize_none()
} else {
Expand Down Expand Up @@ -352,10 +352,9 @@ impl<'a> Serialize for EnumSerializer<'a> {
}
VariantType::Tuple if field_len == 1 => {
let field = self.enum_value.field_at(0).unwrap();
if self
.enum_value
.reflect_type_path()
.starts_with("core::option::Option")

if type_info.type_path_table().module_path() == Some("core::option")
&& type_info.type_path_table().ident() == Some("Option")
{
serializer.serialize_some(&TypedReflectSerializer::new(field, self.registry))
} else {
Expand Down Expand Up @@ -464,8 +463,8 @@ impl<'a> Serialize for ArraySerializer<'a> {

#[cfg(test)]
mod tests {
use crate as bevy_reflect;
use crate::serde::ReflectSerializer;
use crate::{self as bevy_reflect, Struct};
use crate::{Reflect, ReflectSerialize, TypeRegistry};
use bevy_utils::HashMap;
use ron::extensions::Extensions;
Expand Down Expand Up @@ -881,4 +880,43 @@ mod tests {

assert_eq!(expected, bytes);
}

#[test]
fn should_serialize_dynamic_option() {
#[derive(Default, Reflect)]
struct OtherStruct {
some: Option<SomeStruct>,
none: Option<SomeStruct>,
}

let value = OtherStruct {
some: Some(SomeStruct { foo: 999999999 }),
none: None,
};
let dynamic = value.clone_dynamic();
let reflect = dynamic.as_reflect();

let registry = get_registry();

let serializer = ReflectSerializer::new(reflect, &registry);

let mut buf = Vec::new();

let format = serde_json::ser::PrettyFormatter::with_indent(b" ");
let mut ser = serde_json::Serializer::with_formatter(&mut buf, format);

serializer.serialize(&mut ser).unwrap();

let output = std::str::from_utf8(&buf).unwrap();
let expected = r#"{
"bevy_reflect::serde::ser::tests::OtherStruct": {
"some": {
"foo": 999999999
},
"none": null
}
}"#;

assert_eq!(expected, output);
}
}

0 comments on commit e85af0e

Please sign in to comment.