diff --git a/crates/bevy_pbr/src/pbr_material.rs b/crates/bevy_pbr/src/pbr_material.rs index 8e48acf5821c66..68e58425702f61 100644 --- a/crates/bevy_pbr/src/pbr_material.rs +++ b/crates/bevy_pbr/src/pbr_material.rs @@ -177,26 +177,6 @@ pub struct StandardMaterial { /// which can be done via `cull_mode`. pub double_sided: bool, - /// Whether to cull the "front", "back" or neither side of a mesh. - /// If set to `None`, the two sides of the mesh are visible. - /// - /// Defaults to `Some(Face::Back)`. - /// In bevy, the order of declaration of a triangle's vertices - /// in [`Mesh`] defines the triangle's front face. - /// - /// When a triangle is in a viewport, - /// if its vertices appear counter-clockwise from the viewport's perspective, - /// then the viewport is seeing the triangle's front face. - /// Conversly, if the vertices appear clockwise, you are seeing the back face. - /// - /// In short, in bevy, front faces winds counter-clockwise. - /// - /// Your 3D editing software should manage all of that. - /// - /// [`Mesh`]: bevy_render::mesh::Mesh - // TODO: include this in reflection somehow (maybe via remote types like serde https://serde.rs/remote-derive.html) - #[reflect(ignore)] - pub cull_mode: Option, /// Whether to apply only the base color to this material. /// @@ -225,6 +205,26 @@ pub struct StandardMaterial { /// /// [z-fighting]: https://en.wikipedia.org/wiki/Z-fighting pub depth_bias: f32, + /// Whether to cull the "front", "back" or neither side of a mesh. + /// If set to `None`, the two sides of the mesh are visible. + /// + /// Defaults to `Some(Face::Back)`. + /// In bevy, the order of declaration of a triangle's vertices + /// in [`Mesh`] defines the triangle's front face. + /// + /// When a triangle is in a viewport, + /// if its vertices appear counter-clockwise from the viewport's perspective, + /// then the viewport is seeing the triangle's front face. + /// Conversly, if the vertices appear clockwise, you are seeing the back face. + /// + /// In short, in bevy, front faces winds counter-clockwise. + /// + /// Your 3D editing software should manage all of that. + /// + /// [`Mesh`]: bevy_render::mesh::Mesh + // TODO: include this in reflection somehow (maybe via remote types like serde https://serde.rs/remote-derive.html) + #[reflect(ignore)] + pub cull_mode: Option, } impl Default for StandardMaterial { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 92b2035e1939dd..0944316ebc6863 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,5 +1,5 @@ use crate::container_attributes::ReflectTraits; -use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; +use crate::field_attributes::{ReflectFieldAttr, ReflectFieldAttrParser}; use crate::utility::members_to_serialization_denylist; use bit_set::BitSet; use quote::quote; @@ -100,9 +100,6 @@ pub(crate) struct EnumVariant<'a> { pub data: &'a Variant, /// The fields within this variant. pub fields: EnumVariantFields<'a>, - /// The reflection-based attributes on the variant. - #[allow(dead_code)] - pub attrs: ReflectFieldAttr, /// The index of this variant within the enum. #[allow(dead_code)] pub index: usize, @@ -195,7 +192,8 @@ impl<'a> ReflectDerive<'a> { return match &input.data { Data::Struct(data) => { - let fields = Self::collect_struct_fields(&data.fields)?; + let mut field_parser = ReflectFieldAttrParser::new_struct(); + let fields = Self::collect_struct_fields(&data.fields, &mut field_parser)?; let reflect_struct = ReflectStruct { meta, serialization_denylist: members_to_serialization_denylist( @@ -223,12 +221,15 @@ impl<'a> ReflectDerive<'a> { }; } - fn collect_struct_fields(fields: &'a Fields) -> Result>, syn::Error> { + fn collect_struct_fields( + fields: &'a Fields, + field_parser: &mut ReflectFieldAttrParser, + ) -> Result>, syn::Error> { let sifter: utility::ResultSifter> = fields .iter() .enumerate() .map(|(index, field)| -> Result { - let attrs = parse_field_attrs(&field.attrs)?; + let attrs = field_parser.parse(&field.attrs)?; Ok(StructField { index, attrs, @@ -252,7 +253,8 @@ impl<'a> ReflectDerive<'a> { .iter() .enumerate() .map(|(index, variant)| -> Result { - let fields = Self::collect_struct_fields(&variant.fields)?; + let mut field_parser = ReflectFieldAttrParser::new_enum_variant(); + let fields = Self::collect_struct_fields(&variant.fields, &mut field_parser)?; let fields = match variant.fields { Fields::Named(..) => EnumVariantFields::Named(fields), @@ -261,7 +263,6 @@ impl<'a> ReflectDerive<'a> { }; Ok(EnumVariant { fields, - attrs: parse_field_attrs(&variant.attrs)?, data: variant, index, #[cfg(feature = "documentation")] diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index 0e19b8429e581a..d2b0315de62da4 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -5,6 +5,7 @@ //! the derive helper attribute for `Reflect`, which looks like: `#[reflect(ignore)]`. use crate::REFLECT_ATTRIBUTE_NAME; +use proc_macro2::Span; use quote::ToTokens; use syn::spanned::Spanned; use syn::{Attribute, Lit, Meta, NestedMeta}; @@ -69,85 +70,158 @@ pub(crate) enum DefaultBehavior { Func(syn::ExprPath), } -/// Parse all field attributes marked "reflect" (such as `#[reflect(ignore)]`). -pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result { - let mut args = ReflectFieldAttr::default(); - let mut errors: Option = None; - - let attrs = attrs - .iter() - .filter(|a| a.path.is_ident(REFLECT_ATTRIBUTE_NAME)); - for attr in attrs { - let meta = attr.parse_meta()?; - if let Err(err) = parse_meta(&mut args, &meta) { - if let Some(ref mut error) = errors { - error.combine(err); - } else { - errors = Some(err); - } +/// Helper struct for parsing field attributes on structs, tuple structs, and enum variants. +pub(crate) struct ReflectFieldAttrParser { + /// Indicates whether the fields being parsed are part of an enum variant. + is_variant: bool, + /// The [`Span`] for the last `#[reflect(ignore)]` attribute, if any. + last_ignored: Option, + /// The [`Span`] for the last `#[reflect(skip_serializing)]` attribute, if any. + last_skipped: Option, +} + +impl ReflectFieldAttrParser { + /// Create a new parser for struct and tuple struct fields. + pub fn new_struct() -> Self { + Self { + is_variant: false, + last_ignored: None, + last_skipped: None, } } - if let Some(error) = errors { - Err(error) - } else { - Ok(args) + /// Create a new parser for enum variant struct fields. + pub fn new_enum_variant() -> Self { + Self { + is_variant: true, + last_ignored: None, + last_skipped: None, + } } -} -/// Recursively parses attribute metadata for things like `#[reflect(ignore)]` and `#[reflect(default = "foo")]` -fn parse_meta(args: &mut ReflectFieldAttr, meta: &Meta) -> Result<(), syn::Error> { - match meta { - Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { - (args.ignore == ReflectIgnoreBehavior::None) - .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreSerialization) - .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) - } - Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { - (args.ignore == ReflectIgnoreBehavior::None) - .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreAlways) - .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) + /// Parse all field attributes marked "reflect" (such as `#[reflect(ignore)]`). + pub fn parse(&mut self, attrs: &[Attribute]) -> Result { + let mut args = ReflectFieldAttr::default(); + let mut errors: Option = None; + + let attrs = attrs + .iter() + .filter(|a| a.path.is_ident(REFLECT_ATTRIBUTE_NAME)); + for attr in attrs { + let meta = attr.parse_meta()?; + if let Err(err) = self.parse_meta(&mut args, &meta) { + Self::combine_error(err, &mut errors); + } } - Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { - args.default = DefaultBehavior::Default; - Ok(()) + + self.check_ignore_order(&args, &mut errors); + self.check_skip_order(&args, &mut errors); + + if let Some(error) = errors { + Err(error) + } else { + Ok(args) } - Meta::Path(path) => Err(syn::Error::new( - path.span(), - format!("unknown attribute parameter: {}", path.to_token_stream()), - )), - Meta::NameValue(pair) if pair.path.is_ident(DEFAULT_ATTR) => { - let lit = &pair.lit; - match lit { - Lit::Str(lit_str) => { - args.default = DefaultBehavior::Func(lit_str.parse()?); + } + + /// Recursively parses attribute metadata for things like `#[reflect(ignore)]` and `#[reflect(default = "foo")]` + fn parse_meta(&mut self, args: &mut ReflectFieldAttr, meta: &Meta) -> Result<(), syn::Error> { + match meta { + Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { + if args.ignore == ReflectIgnoreBehavior::None { + args.ignore = ReflectIgnoreBehavior::IgnoreSerialization; + self.last_skipped = Some(path.span()); Ok(()) + } else { + Err(syn::Error::new_spanned(path, format!("only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } - err => { - Err(syn::Error::new( - err.span(), - format!("expected a string literal containing the name of a function, but found: {}", err.to_token_stream()), - )) + } + Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { + if args.ignore == ReflectIgnoreBehavior::None { + args.ignore = ReflectIgnoreBehavior::IgnoreAlways; + self.last_ignored = Some(path.span()); + Ok(()) + } else { + Err(syn::Error::new_spanned(path, format!("only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } } - } - Meta::NameValue(pair) => { - let path = &pair.path; - Err(syn::Error::new( + Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { + args.default = DefaultBehavior::Default; + Ok(()) + } + Meta::Path(path) => Err(syn::Error::new( path.span(), format!("unknown attribute parameter: {}", path.to_token_stream()), - )) + )), + Meta::NameValue(pair) if pair.path.is_ident(DEFAULT_ATTR) => { + let lit = &pair.lit; + match lit { + Lit::Str(lit_str) => { + args.default = DefaultBehavior::Func(lit_str.parse()?); + Ok(()) + } + err => { + Err(syn::Error::new( + err.span(), + format!("expected a string literal containing the name of a function, but found: {}", err.to_token_stream()), + )) + } + } + } + Meta::NameValue(pair) => { + let path = &pair.path; + Err(syn::Error::new( + path.span(), + format!("unknown attribute parameter: {}", path.to_token_stream()), + )) + } + Meta::List(list) if !list.path.is_ident(REFLECT_ATTRIBUTE_NAME) => { + Err(syn::Error::new(list.path.span(), "unexpected property")) + } + Meta::List(list) => { + for nested in &list.nested { + if let NestedMeta::Meta(meta) = nested { + self.parse_meta(args, meta)?; + } + } + Ok(()) + } } - Meta::List(list) if !list.path.is_ident(REFLECT_ATTRIBUTE_NAME) => { - Err(syn::Error::new(list.path.span(), "unexpected property")) + } + + /// Verifies `#[reflect(ignore)]` attributes are always last in the type definition. + fn check_ignore_order(&self, args: &ReflectFieldAttr, errors: &mut Option) { + if args.ignore.is_active() { + if let Some(span) = self.last_ignored { + if self.is_variant { + Self::combine_error(syn::Error::new(span, format!("fields marked with `#[reflect({IGNORE_ALL_ATTR})]` must come last in variant definition")), errors); + } else { + Self::combine_error(syn::Error::new(span, format!("fields marked with `#[reflect({IGNORE_ALL_ATTR})]` must come last in type definition")), errors); + } + } } - Meta::List(list) => { - for nested in &list.nested { - if let NestedMeta::Meta(meta) = nested { - parse_meta(args, meta)?; + } + + /// Verifies `#[reflect(skip_serializing)]` attributes are always last in the type definition, + /// but before `#[reflect(ignore)]` attributes. + fn check_skip_order(&self, args: &ReflectFieldAttr, errors: &mut Option) { + if args.ignore == ReflectIgnoreBehavior::None { + if let Some(span) = self.last_skipped { + if self.is_variant { + Self::combine_error(syn::Error::new(span, format!("fields marked with `#[reflect({IGNORE_SERIALIZATION_ATTR})]` must come last in variant definition (but before any fields marked `#[reflect({IGNORE_ALL_ATTR})]`)")), errors); + } else { + Self::combine_error(syn::Error::new(span, format!("fields marked with `#[reflect({IGNORE_SERIALIZATION_ATTR})]` must come last in type definition (but before any fields marked `#[reflect({IGNORE_ALL_ATTR})]`)")), errors); } } - Ok(()) + } + } + + /// Set or combine the given error into an optionally existing error. + fn combine_error(err: syn::Error, errors: &mut Option) { + if let Some(error) = errors { + error.combine(err); + } else { + *errors = Some(err); } } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index d9e44754c9f547..cc6c2139e672f1 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -108,11 +108,11 @@ impl ResultSifter { /// pub struct HelloWorld { /// reflected_field: u32 // index: 0 /// -/// #[reflect(ignore)] -/// non_reflected_field: u32 // index: N/A (not 1!) -/// /// #[reflect(skip_serializing)] /// non_serialized_field: u32 // index: 1 +/// +/// #[reflect(ignore)] +/// non_reflected_field: u32 // index: N/A (not 2!) /// } /// ``` /// Would convert to the `0b01` bitset (i.e second field is NOT serialized) diff --git a/crates/bevy_reflect/src/enums/mod.rs b/crates/bevy_reflect/src/enums/mod.rs index e8c32e73ef9c56..fe6d2198732731 100644 --- a/crates/bevy_reflect/src/enums/mod.rs +++ b/crates/bevy_reflect/src/enums/mod.rs @@ -290,9 +290,9 @@ mod tests { A, B, C { + bar: bool, #[reflect(ignore)] foo: f32, - bar: bool, }, } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 6c1c3c8dc320e1..6231cb2e4be1bb 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -310,14 +310,14 @@ mod tests { #[reflect(PartialEq)] struct Foo { a: u32, + b: Vec, + c: HashMap, + d: Bar, + e: (i32, Vec, Bar), + f: Vec<(Baz, HashMap)>, + g: [u32; 2], #[reflect(ignore)] - _b: u32, - c: Vec, - d: HashMap, - e: Bar, - f: (i32, Vec, Bar), - g: Vec<(Baz, HashMap)>, - h: [u32; 2], + _h: u32, } #[derive(Reflect, Eq, PartialEq, Clone, Debug, FromReflect)] @@ -338,39 +338,39 @@ mod tests { let mut foo = Foo { a: 1, - _b: 1, - c: vec![1, 2], - d: hash_map, - e: Bar { x: 1 }, - f: (1, vec![1, 2], Bar { x: 1 }), - g: vec![(Baz("string".to_string()), hash_map_baz)], - h: [2; 2], + b: vec![1, 2], + c: hash_map, + d: Bar { x: 1 }, + e: (1, vec![1, 2], Bar { x: 1 }), + f: vec![(Baz("string".to_string()), hash_map_baz)], + g: [2; 2], + _h: 1, }; let mut foo_patch = DynamicStruct::default(); foo_patch.insert("a", 2u32); - foo_patch.insert("b", 2u32); // this should be ignored + foo_patch.insert("_h", 2u32); // this should be ignored let mut list = DynamicList::default(); list.push(3isize); list.push(4isize); list.push(5isize); - foo_patch.insert("c", List::clone_dynamic(&list)); + foo_patch.insert("b", List::clone_dynamic(&list)); let mut map = DynamicMap::default(); map.insert(2usize, 3i8); map.insert(3usize, 4i8); - foo_patch.insert("d", map); + foo_patch.insert("c", map); let mut bar_patch = DynamicStruct::default(); bar_patch.insert("x", 2u32); - foo_patch.insert("e", bar_patch.clone_dynamic()); + foo_patch.insert("d", bar_patch.clone_dynamic()); let mut tuple = DynamicTuple::default(); tuple.insert(2i32); tuple.insert(list); tuple.insert(bar_patch); - foo_patch.insert("f", tuple); + foo_patch.insert("e", tuple); let mut composite = DynamicList::default(); composite.push({ @@ -391,10 +391,10 @@ mod tests { }); tuple }); - foo_patch.insert("g", composite); + foo_patch.insert("f", composite); let array = DynamicArray::from_vec(vec![2u32, 2u32]); - foo_patch.insert("h", array); + foo_patch.insert("g", array); foo.apply(&foo_patch); @@ -408,13 +408,13 @@ mod tests { let expected_foo = Foo { a: 2, - _b: 1, - c: vec![3, 4, 5], - d: hash_map, - e: Bar { x: 2 }, - f: (2, vec![3, 4, 5], Bar { x: 2 }), - g: vec![(Baz("new_string".to_string()), hash_map_baz.clone())], - h: [2; 2], + b: vec![3, 4, 5], + c: hash_map, + d: Bar { x: 2 }, + e: (2, vec![3, 4, 5], Bar { x: 2 }), + f: vec![(Baz("new_string".to_string()), hash_map_baz.clone())], + g: [2; 2], + _h: 1, }; assert_eq!(foo, expected_foo); @@ -428,13 +428,13 @@ mod tests { let expected_new_foo = Foo { a: 2, - _b: 0, - c: vec![3, 4, 5], - d: hash_map, - e: Bar { x: 2 }, - f: (2, vec![3, 4, 5], Bar { x: 2 }), - g: vec![(Baz("new_string".to_string()), hash_map_baz)], - h: [2; 2], + b: vec![3, 4, 5], + c: hash_map, + d: Bar { x: 2 }, + e: (2, vec![3, 4, 5], Bar { x: 2 }), + f: vec![(Baz("new_string".to_string()), hash_map_baz)], + g: [2; 2], + _h: 0, }; assert_eq!(new_foo, expected_new_foo); @@ -445,14 +445,14 @@ mod tests { #[derive(Reflect)] struct Foo { a: u32, + b: Vec, + c: HashMap, + d: Bar, + e: String, + f: (i32, Vec, Bar), + g: [u32; 2], #[reflect(ignore)] - _b: u32, - c: Vec, - d: HashMap, - e: Bar, - f: String, - g: (i32, Vec, Bar), - h: [u32; 2], + _h: u32, } #[derive(Reflect, Serialize, Deserialize)] @@ -466,13 +466,13 @@ mod tests { hash_map.insert(2, 2); let foo = Foo { a: 1, - _b: 1, - c: vec![1, 2], - d: hash_map, - e: Bar { x: 1 }, - f: "hi".to_string(), - g: (1, vec![1, 2], Bar { x: 1 }), - h: [2; 2], + b: vec![1, 2], + c: hash_map, + d: Bar { x: 1 }, + e: "hi".to_string(), + f: (1, vec![1, 2], Bar { x: 1 }), + g: [2; 2], + _h: 1, }; let mut registry = TypeRegistry::default(); diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 3355ed38d0b115..c9434b6097e702 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -22,10 +22,10 @@ mod tests { #[reflect(PartialEq)] struct TestStruct { a: i32, - #[reflect(ignore)] b: i32, #[reflect(skip_serializing)] c: i32, + #[reflect(ignore)] d: i32, } @@ -34,9 +34,9 @@ mod tests { let test_struct = TestStruct { a: 3, - b: 4, - c: 5, - d: 6, + b: 6, + c: 4, + d: 5, }; let serializer = ReflectSerializer::new(&test_struct, ®istry); @@ -45,7 +45,7 @@ mod tests { let mut expected = DynamicStruct::default(); expected.insert("a", 3); - expected.insert("d", 6); + expected.insert("b", 6); let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); let reflect_deserializer = UntypedReflectDeserializer::new(®istry); @@ -64,9 +64,9 @@ mod tests { #[reflect(PartialEq)] struct TestStruct( i32, - #[reflect(ignore)] i32, - #[reflect(skip_serializing)] i32, i32, + #[reflect(skip_serializing)] i32, + #[reflect(ignore)] i32, ); let mut registry = TypeRegistry::default(); @@ -80,7 +80,7 @@ mod tests { let mut expected = DynamicTupleStruct::default(); expected.insert(3); - expected.insert(6); + expected.insert(4); let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); let reflect_deserializer = UntypedReflectDeserializer::new(®istry); diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 82bf0c894661b1..9c379cbe1845d7 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -91,18 +91,18 @@ pub struct Camera { /// If this is set to `true`, this camera will be rendered to its specified [`RenderTarget`]. If `false`, this /// camera will not be rendered. pub is_active: bool, - /// Computed values for this camera, such as the projection matrix and the render target size. - #[reflect(ignore)] - pub computed: ComputedCameraValues, - /// The "target" that this camera will render to. - #[reflect(ignore)] - pub target: RenderTarget, /// If this is set to `true`, the camera will use an intermediate "high dynamic range" render texture. /// Warning: we are still working on this feature. If MSAA is enabled, there will be artifacts in /// some cases. When rendering with WebGL, this will crash if MSAA is enabled. /// See for details. // TODO: resolve the issues mentioned in the doc comment above, then remove the warning. pub hdr: bool, + /// Computed values for this camera, such as the projection matrix and the render target size. + #[reflect(ignore)] + pub computed: ComputedCameraValues, + /// The "target" that this camera will render to. + #[reflect(ignore)] + pub target: RenderTarget, } impl Default for Camera {