Skip to content
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

ReflectDeserializer does not error when deserializing incomplete enum value #12357

Open
UkoeHB opened this issue Mar 7, 2024 · 3 comments
Open
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior

Comments

@UkoeHB
Copy link
Contributor

UkoeHB commented Mar 7, 2024

Bevy version

v0.13.0

What you did

#[derive(Reflect)]
enum E {
    A,
    B{ a: usize, b: usize },
}
  1. Initialized an enum to E::A.
  2. Created a reflected E::B with only one field by deserializing it from JSON using TypedReflectDeserializer.
  3. Applied the incomplete reflected E::B to the enum with value E::A, using Reflect::apply.

What went wrong

This panic triggered. Basically, the reflect machinery cannot apply an incomplete enum variant to a different enum variant.

This is a bug because I expect the deserialize-from-JSON step to catch this kind of error so you never hit the panic. Note that, in contrast to enums, if you reflect-deserialize a struct with missing fields, then a serialization error will occur.

Additional information

You can avoid this panic by using FromReflect::from_reflect instead of Reflect::apply, which will fail without panicking.

If you don't want FromReflect::from_reflect to fail when a field is missing, use #[reflect(default)] on potentially-missing fields.

@UkoeHB UkoeHB added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types and removed S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@UkoeHB UkoeHB changed the title Reflect::apply panics when applying incomplete enum value ReflectDeserializer does not error when deserializing incomplete enum value Mar 7, 2024
@mrchantey
Copy link
Contributor

mrchantey commented Mar 13, 2024

Not sure if its related but im also currently tracking down a bug for scene serde where enums are breaking in weird ways.

Code
#[derive(Reflect, Component)]
#[reflect(Component)]
enum Fizz {
  B(f32),  // this is ok
  A,
}

#[derive(Reflect, Component)]
#[reflect(Component)]
enum Bazz {
  A,       // this fails
  B(f32),
}

fn test() -> Result<()> {
  do_another_test(Bazz::B(0.5))?;
  do_another_test(Fizz::B(0.5))?;
  Ok(())
}


fn do_another_test<T: Reflect + GetTypeRegistration + Component>(val: T) -> Result<()> {
	let mut world = World::new();
	world.spawn(val);
	let registry = AppTypeRegistry::default();
	registry.write().register::<T>();
	world.insert_resource(registry.clone());
	let scene1 = DynamicScene::from_world(&world);

	let serialized1 =
		ron::ser::to_string(&SceneSerializer::new(&scene1, &registry))?;

	let mut deserializer = ron::de::Deserializer::from_str(&serialized1)?;
	let scene2 = SceneDeserializer {
		type_registry: &registry.read(),
	}
	.deserialize(&mut deserializer)?;

	let serialized2 =
		ron::ser::to_string(&SceneSerializer::new(&scene2, &registry))?;

	expect(serialized1).to_be(serialized2)?;
	Ok(())
}

output:

Expected: (resources:{},entities:{4294967296:(components:{\"my_crate::Bazz\":A(0.5)})})
Received: (resources:{},entities:{4294967296:(components:{\"my_crate::Bazz\":B(0.5)})})

@mrchantey
Copy link
Contributor

@UkoeHB
Copy link
Contributor Author

UkoeHB commented Mar 14, 2024

@mrchantey not sure if they are related, will need to test out the fix from #12464 to see. My problem doesn't seem to be deserialization, it's applying reflected enums to existing structs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

3 participants