rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

`FromReflect` Ergonomics

Open MrGVSV opened this issue 3 years ago • 2 comments

RENDERED

Derive FromReflect by default when deriving Reflect, add ReflectFromReflect type data, and make ReflectDeserializer easier to use.

MrGVSV avatar Jul 02 '22 19:07 MrGVSV

Have you considered using the “static type” instead of “real type”? It seems to be a direct translation from dynamic vs static typing.

I did, but chose not to use it. My reasoning was that "static" feels like we should know the exact type at compile time. However, a dyn Reflect could still be a "real type", even though it isn't statically known.

MrGVSV avatar Jul 04 '22 19:07 MrGVSV

Please try to split your lines at the sentence / clause level; it makes reviewing and suggestions much more pleasant :)

Okay I split bascially every sentence across multiple lines (where I could). Typora hates me now lol, but hopefully that's more readable/reviewable 🙂

MrGVSV avatar Jul 04 '22 21:07 MrGVSV

Any update on this? I am trying to deserialize some bevy basic types (eg Transform) but can't rewire the type from DynamicStruct, because it doesn't have ReflectFromReflect.

raffaeleragni avatar May 31 '23 21:05 raffaeleragni

Any update on this?

Unfortunately, there hasn't been much progress. While @cart has suggested more of an openness to merging FromReflect into the derive for Reflect, I haven't taken the time to update the RFC.

One reason I haven't updated it is because after coding the initial implementation in https://github.com/bevyengine/bevy/pull/6056, I just found the RFC to be overkill. The actual changes are only a few hundred lines and most of it is cleanup. On top of that, ReflectFromReflect, which was part of this RFC, got merged on its own pretty easily.

So I could update this, but I may just close it out in favor of just using https://github.com/bevyengine/bevy/pull/6056 to respond to any feedback. I'll probably rebase and update that PR relatively soon. Thoughts?


I am trying to deserialize some bevy basic types (eg Transform) but can't rewire the type from DynamicStruct, because it doesn't have ReflectFromReflect.

If you want to make a PR adding in a ReflectFromReflect registration, I'm sure that will be easily accepted (basically a four line change to cover both Transform and GlobalTransform).

In the meantime, however, you can just manually register it for all your known types:

app.register_type_data::<Transform, ReflectFromReflect>();

MrGVSV avatar May 31 '23 22:05 MrGVSV

Didn't know there was a workaround, that's actually great, thanks.

raffaeleragni avatar Jun 01 '23 11:06 raffaeleragni

Closing this out. See https://github.com/bevyengine/bevy/pull/6056#issuecomment-1575798530 for details.

MrGVSV avatar Jun 04 '23 23:06 MrGVSV