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

Doing #[reflect(ignore)] on fields in a struct still requires Default on that field #13851

Open
torsteingrindvik opened this issue Jun 14, 2024 · 4 comments
Labels
A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@torsteingrindvik
Copy link
Contributor

torsteingrindvik commented Jun 14, 2024

Bevy version

Latest: 1395e36

What you did

image

What went wrong

I couldn't ignore a field on my struct which I wanted to derive reflect on, since that field did not impl Default and I had no control over that field.

My expectation was that the field is ignored entirely with no limitations.

@torsteingrindvik torsteingrindvik added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 14, 2024
@MrGVSV MrGVSV added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 14, 2024
@MrGVSV
Copy link
Member

MrGVSV commented Jun 14, 2024

This is actually expected behavior since FromReflect (which is automatically derived alongside Reflect) requires Default for ignored fields.

There are two possible solutions besides implementing Default for Foo:

  1. Opt out of FromReflect by adding #[reflect(from_reflect = false)] to your Bar type
  2. Add a custom default function for Foo like #[reflect(ignore, default = "create_foo")], #[reflect(ignore, default = "Foo::new")], etc.

Hopefully that helps!

@MrGVSV
Copy link
Member

MrGVSV commented Jun 14, 2024

As for how we can avoid this in the future, I think we could update the macro code to make FromReflect opt-in. If any field is ignored, we can skip implementing FromReflect unless a default attribute is set for that field. I did something like this in #13432.

One possible issue is that this would mean that users won't be 100% sure their type implements FromReflect unless they try to use it.

@torsteingrindvik
Copy link
Contributor Author

As for how we can avoid this in the future, I think we could update the macro code to make FromReflect opt-in. If any field is ignored, we can skip implementing FromReflect unless a default attribute is set for that field. I did something like this in #13432.

One possible issue is that this would mean that users won't be 100% sure their type implements FromReflect unless they try to use it.

If this is a longer term solution, is the near term is it possible in the derive macro to detect the lacking default impl and report on it?

Opt out of FromReflect by adding #[reflect(from_reflect = false)] to your Bar type

If I was guided towards this in the error msg that would have helped me right away, which would be very nice

@MrGVSV
Copy link
Member

MrGVSV commented Jun 15, 2024

is it possible in the derive macro to detect the lacking default impl and report on it?

Unfortunately no. Macros don't have any type information available to them, including what traits a type implements.

Opt out of FromReflect by adding #[reflect(from_reflect = false)] to your Bar type

If I was guided towards this in the error msg that would have helped me right away, which would be very nice

Yeah ideally we would do this, but it's not possible to know whether or not a type implements Default via the macro. We'd have to generate this error based on the lack of default attributes when not opting out, which may be controversial compared to just returning None.

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-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
Status: Open
Development

No branches or pull requests

2 participants