Skip to content

Commit

Permalink
Add reflect(skip_serializing) which retains reflection but disables…
Browse files Browse the repository at this point in the history
… automatic serialization (#5250)

# Objective

- To address problems outlined in #5245

## Solution

- Introduce `reflect(skip_serializing)` on top of `reflect(ignore)` which disables automatic serialisation to scenes, but does not disable reflection of the field.

---

## Changelog
- Adds: 
  - `bevy_reflect::serde::type_data` module
  - `ReflectSerializableWithData` trait for retrieving `SerializationData` which is implemented via macros
  - `SerializationData` structure for describing which fields are to be/not to be ignored
  - the `skip_serialization` flag for `#[reflect(...)]`
 - Removes:
   - ability to ignore Enum variants in serialization, since that didn't work anyway   
 

## Migration Guide
- Change `#[reflect(ignore)]` to `#[reflect(skip_serializing)]` where disabling reflection is not the intended effect.
- Remove ignore/skip attributes from enum variants as these won't do anything anymore
  • Loading branch information
makspll committed Sep 19, 2022
1 parent f2ad111 commit e33d7be
Show file tree
Hide file tree
Showing 18 changed files with 330 additions and 74 deletions.
1 change: 1 addition & 0 deletions crates/bevy_reflect/bevy_reflect_derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ syn = { version = "1.0", features = ["full"] }
proc-macro2 = "1.0"
quote = "1.0"
uuid = { version = "1.1", features = ["v4"] }
bit-set = "0.5.2"
92 changes: 61 additions & 31 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::container_attributes::ReflectTraits;
use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
use crate::utility::members_to_serialization_denylist;
use bit_set::BitSet;
use quote::quote;

use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME};
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Type, Variant};
use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Variant};

pub(crate) enum ReflectDerive<'a> {
Struct(ReflectStruct<'a>),
Expand Down Expand Up @@ -54,6 +56,7 @@ pub(crate) struct ReflectMeta<'a> {
/// ```
pub(crate) struct ReflectStruct<'a> {
meta: ReflectMeta<'a>,
serialization_denylist: BitSet<u32>,
fields: Vec<StructField<'a>>,
}

Expand Down Expand Up @@ -92,6 +95,7 @@ pub(crate) struct EnumVariant<'a> {
/// 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)]
Expand Down Expand Up @@ -125,18 +129,24 @@ impl<'a> ReflectDerive<'a> {
_ => continue,
}
}

let meta = ReflectMeta::new(&input.ident, &input.generics, traits);

if force_reflect_value {
return Ok(Self::Value(meta));
return Ok(Self::Value(ReflectMeta::new(
&input.ident,
&input.generics,
traits,
)));
}

return match &input.data {
Data::Struct(data) => {
let fields = Self::collect_struct_fields(&data.fields)?;
let meta = ReflectMeta::new(&input.ident, &input.generics, traits);
let reflect_struct = ReflectStruct {
meta,
fields: Self::collect_struct_fields(&data.fields)?,
serialization_denylist: members_to_serialization_denylist(
fields.iter().map(|v| v.attrs.ignore),
),
fields,
};

match data.fields {
Expand All @@ -146,10 +156,10 @@ impl<'a> ReflectDerive<'a> {
}
}
Data::Enum(data) => {
let reflect_enum = ReflectEnum {
meta,
variants: Self::collect_enum_variants(&data.variants)?,
};
let variants = Self::collect_enum_variants(&data.variants)?;
let meta = ReflectMeta::new(&input.ident, &input.generics, traits);

let reflect_enum = ReflectEnum { meta, variants };
Ok(Self::Enum(reflect_enum))
}
Data::Union(..) => Err(syn::Error::new(
Expand Down Expand Up @@ -246,6 +256,7 @@ impl<'a> ReflectMeta<'a> {
&self.bevy_reflect_path,
self.traits.idents(),
self.generics,
None,
)
}
}
Expand All @@ -256,19 +267,50 @@ impl<'a> ReflectStruct<'a> {
&self.meta
}

/// Get an iterator over the active fields.
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields.iter().filter(|field| !field.attrs.ignore)
/// Access the data about which fields should be ignored during serialization.
///
/// The returned bitset is a collection of indices obtained from the [`members_to_serialization_denylist`](crate::utility::members_to_serialization_denylist) function.
#[allow(dead_code)]
pub fn serialization_denylist(&self) -> &BitSet<u32> {
&self.serialization_denylist
}

/// Get an iterator over the ignored fields.
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields.iter().filter(|field| field.attrs.ignore)
/// Returns the `GetTypeRegistration` impl as a `TokenStream`.
///
/// Returns a specific implementation for structs and this method should be preffered over the generic [`get_type_registration`](crate::ReflectMeta) method
pub fn get_type_registration(&self) -> proc_macro2::TokenStream {
let reflect_path = self.meta.bevy_reflect_path();

crate::registration::impl_get_type_registration(
self.meta.type_name(),
reflect_path,
self.meta.traits().idents(),
self.meta.generics(),
Some(&self.serialization_denylist),
)
}

/// Get a collection of all active types.
pub fn active_types(&self) -> impl Iterator<Item = &Type> {
self.active_fields().map(|field| &field.data.ty)
/// Get a collection of types which are exposed to the reflection API
pub fn active_types(&self) -> Vec<syn::Type> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_active())
.map(|field| field.data.ty.clone())
.collect::<Vec<_>>()
}

/// Get an iterator of fields which are exposed to the reflection API
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_active())
}

/// Get an iterator of fields which are ignored by the reflection API
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
.iter()
.filter(move |field| field.attrs.ignore.is_ignored())
}

/// The complete set of fields in this struct.
Expand All @@ -284,17 +326,6 @@ impl<'a> ReflectEnum<'a> {
&self.meta
}

/// Get an iterator over the active variants.
pub fn active_variants(&self) -> impl Iterator<Item = &EnumVariant<'a>> {
self.variants.iter().filter(|variant| !variant.attrs.ignore)
}

/// Get an iterator over the ignored variants.
#[allow(dead_code)]
pub fn ignored_variants(&self) -> impl Iterator<Item = &EnumVariant<'a>> {
self.variants.iter().filter(|variant| variant.attrs.ignore)
}

/// Returns the given ident as a qualified unit variant of this enum.
pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream {
let name = self.meta.type_name;
Expand All @@ -304,7 +335,6 @@ impl<'a> ReflectEnum<'a> {
}

/// The complete set of variants in this enum.
#[allow(dead_code)]
pub fn variants(&self) -> &[EnumVariant<'a>] {
&self.variants
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(crate) fn get_variant_constructors(
let mut variant_names = Vec::with_capacity(variant_count);
let mut variant_constructors = Vec::with_capacity(variant_count);

for variant in reflect_enum.active_variants() {
for variant in reflect_enum.variants() {
let ident = &variant.data.ident;
let name = ident.to_string();
let variant_constructor = reflect_enum.get_unit(ident);
Expand All @@ -38,7 +38,7 @@ pub(crate) fn get_variant_constructors(
let mut reflect_index: usize = 0;
let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| {
let field_ident = ident_or_index(field.data.ident.as_ref(), declar_index);
let field_value = if field.attrs.ignore {
let field_value = if field.attrs.ignore.is_ignored() {
quote! { Default::default() }
} else {
let error_repr = field.data.ident.as_ref().map_or_else(
Expand Down
51 changes: 45 additions & 6 deletions crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,47 @@ use quote::ToTokens;
use syn::spanned::Spanned;
use syn::{Attribute, Lit, Meta, NestedMeta};

pub(crate) static IGNORE_ATTR: &str = "ignore";
pub(crate) static IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing";
pub(crate) static IGNORE_ALL_ATTR: &str = "ignore";

pub(crate) static DEFAULT_ATTR: &str = "default";

/// Stores data about if the field should be visible via the Reflect and serialization interfaces
///
/// Note the relationship between serialization and reflection is such that a member must be reflected in order to be serialized.
/// In boolean logic this is described as: `is_serialized -> is_reflected`, this means we can reflect something without serializing it but not the other way round.
/// The `is_reflected` predicate is provided as `self.is_active()`
#[derive(Default, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ReflectIgnoreBehavior {
/// Don't ignore, appear to all systems
#[default]
None,
/// Ignore when serializing but not when reflecting
IgnoreSerialization,
/// Ignore both when serializing and reflecting
IgnoreAlways,
}

impl ReflectIgnoreBehavior {
/// Returns `true` if the ignoring behaviour implies member is included in the reflection API, and false otherwise.
pub fn is_active(self) -> bool {
match self {
ReflectIgnoreBehavior::None | ReflectIgnoreBehavior::IgnoreSerialization => true,
ReflectIgnoreBehavior::IgnoreAlways => false,
}
}

/// The exact logical opposite of `self.is_active()` returns true iff this member is not part of the reflection API whatsover (neither serialized nor reflected)
pub fn is_ignored(self) -> bool {
!self.is_active()
}
}

/// A container for attributes defined on a reflected type's field.
#[derive(Default)]
pub(crate) struct ReflectFieldAttr {
/// Determines if this field should be ignored.
pub ignore: bool,
/// Determines how this field should be ignored if at all.
pub ignore: ReflectIgnoreBehavior,
/// Sets the default behavior of this field.
pub default: DefaultBehavior,
}
Expand Down Expand Up @@ -65,9 +98,15 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result<ReflectFieldAttr,
/// 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_ATTR) => {
args.ignore = true;
Ok(())
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")))
}
Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => {
args.default = DefaultBehavior::Default;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
let mut enum_variant_name = Vec::new();
let mut enum_variant_type = Vec::new();

for variant in reflect_enum.active_variants() {
for variant in reflect_enum.variants() {
let ident = &variant.data.ident;
let name = ident.to_string();
let unit = reflect_enum.get_unit(ident);
Expand All @@ -281,7 +281,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
let mut constructor_argument = Vec::new();
let mut reflect_idx = 0;
for field in fields.iter() {
if field.attrs.ignore {
if field.attrs.ignore.is_ignored() {
// Ignored field
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
bevy_reflect_path,
);

let get_type_registration_impl = reflect_struct.meta().get_type_registration();
let get_type_registration_impl = reflect_struct.get_type_registration();
let (impl_generics, ty_generics, where_clause) =
reflect_struct.meta().generics().split_for_impl();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use syn::{Index, Member};
pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path();
let struct_name = reflect_struct.meta().type_name();
let get_type_registration_impl = reflect_struct.meta().get_type_registration();
let get_type_registration_impl = reflect_struct.get_type_registration();

let field_idents = reflect_struct
.active_fields()
Expand Down
11 changes: 11 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/registration.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Contains code related specifically to Bevy's type registration.

use bit_set::BitSet;
use proc_macro2::Ident;
use quote::quote;
use syn::{Generics, Path};
Expand All @@ -10,14 +11,24 @@ pub(crate) fn impl_get_type_registration(
bevy_reflect_path: &Path,
registration_data: &[Ident],
generics: &Generics,
serialization_denylist: Option<&BitSet<u32>>,
) -> proc_macro2::TokenStream {
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
let serialization_data = serialization_denylist.map(|denylist| {
let denylist = denylist.into_iter().map(|v| v as usize);
quote! {
let ignored_indices = [#(#denylist),*].into_iter();
registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::serde::SerializationData::new(ignored_indices));
}
});

quote! {
#[allow(unused_mut)]
impl #impl_generics #bevy_reflect_path::GetTypeRegistration for #type_name #ty_generics #where_clause {
fn get_type_registration() -> #bevy_reflect_path::TypeRegistration {
let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>();
registration.insert::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());
#serialization_data
#(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)*
registration
}
Expand Down
41 changes: 41 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/utility.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! General-purpose utility functions for internal usage within this crate.

use crate::field_attributes::ReflectIgnoreBehavior;
use bevy_macro_utils::BevyManifest;
use bit_set::BitSet;
use proc_macro2::{Ident, Span};
use syn::{Member, Path};

Expand Down Expand Up @@ -96,3 +98,42 @@ impl<T> ResultSifter<T> {
}
}
}

/// Converts an iterator over ignore behaviour of members to a bitset of ignored members.
///
/// Takes into account the fact that always ignored (non-reflected) members are skipped.
///
/// # Example
/// ```rust,ignore
/// 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
/// }
/// ```
/// Would convert to the `0b01` bitset (i.e second field is NOT serialized)
///
pub(crate) fn members_to_serialization_denylist<T>(member_iter: T) -> BitSet<u32>
where
T: Iterator<Item = ReflectIgnoreBehavior>,
{
let mut bitset = BitSet::default();

member_iter.fold(0, |next_idx, member| match member {
ReflectIgnoreBehavior::IgnoreAlways => {
bitset.insert(next_idx);
next_idx
}
ReflectIgnoreBehavior::IgnoreSerialization => {
bitset.insert(next_idx);
next_idx + 1
}
ReflectIgnoreBehavior::None => next_idx + 1,
});

bitset
}
Loading

0 comments on commit e33d7be

Please sign in to comment.