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

bevy_reflect: Allow #[reflect(skip_serializing)] on enum variant fields #6767

Closed
wants to merge 2 commits into from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Nov 26, 2022

Objective

Fixes #6721

Solution

Converts SerializationData to an enum with variants for structs (including tuple structs) and enums. The latter allows us to store the skipped field info on a per-variant level.


Changelog

  • Converted SerializationData to an enum
  • Added support for #[reflect(skip_serializing)] on enum variant fields

Migration Guide

  • SerializationData is now an enum. Custom implementations of this type data will need to be adjusted to account for this:
// OLD
let serialization_data = SerializationData::new([0, 2, 5].into_iter());

// NEW
let serialization_data = SerializationData::Struct(
  StructSerializationData::new([0, 2, 5].into_iter())
);

@MrGVSV MrGVSV added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types labels Nov 26, 2022
@MrGVSV MrGVSV force-pushed the reflect-skip-serializing-enum branch from fac4f5d to a9e5b8a Compare December 9, 2022 00:34
@MrGVSV MrGVSV force-pushed the reflect-skip-serializing-enum branch from a9e5b8a to 62aa538 Compare February 9, 2023 04:02
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just too minor readability comments.

Comment on lines +68 to +73
pub fn new<TName, TFields, TVariant>(ignored_variants: TVariant) -> Self
where
TName: Into<Cow<'static, str>>,
TFields: Iterator<Item = usize>,
TVariant: Iterator<Item = (TName, TFields)>,
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function signature 😅 . I see what you are doing here, but could it be possible to do the following instead of having a TFields parameter?

        TVariant: Iterator<Item = (TName, &[usize])>,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm having difficulties making it work. It's giving me errors saying it's missing type information.

I'll just leave it as is unless you or someone else has a better way of handling it haha

@MrGVSV MrGVSV force-pushed the reflect-skip-serializing-enum branch from 62aa538 to b509b55 Compare April 21, 2023 01:57
@bas-ie
Copy link
Contributor

bas-ie commented Oct 20, 2024

Backlog cleanup: closing due to inactivity, noting existing tracking issue #6721.

@bas-ie bas-ie closed this Oct 20, 2024
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
Status: Done
Development

Successfully merging this pull request may close these issues.

bevy_reflect: #[reflect(skip_serializing)] doesn't work on enum variant fields
3 participants