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: Add DynamicTyped trait #15108

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Sep 9, 2024

Objective

Thanks to #7207, we now have a way to validate at the type-level that a reflected value is actually the type it says it is and not just a dynamic representation of that type.

dyn PartialReflect values might be a dynamic type, but dyn Reflect values are guaranteed to not be a dynamic type.

Therefore, we can start to add methods to Reflect that weren't really possible before. For example, we should now be able to always get a &'static TypeInfo, and not just an Option<&'static TypeInfo>.

Solution

Add the DynamicTyped trait.

This trait is similar to DynamicTypePath in that it provides a way to use the non-object-safe Typed trait in an object-safe way.

And since all types that derive Reflect will also derive Typed, we can safely add DynamicTyped as a supertrait of Reflect. This allows us to use it when just given a dyn Reflect trait object.

Testing

You can test locally by running:

cargo test --package bevy_reflect

Showcase

Reflect now has a supertrait of DynamicTyped, allowing TypeInfo to be retrieved from a dyn Reflect trait object without having to unwrap anything!

let value: Box<dyn Reflect> = Box::new(String::from("Hello!"));

// BEFORE
let info: &'static TypeInfo = value.get_represented_type_info().unwrap();

// AFTER
let info: &'static TypeInfo = value.reflect_type_info();

Migration Guide

Reflect now has a supertrait of DynamicTyped. If you were manually implementing Reflect and did not implement Typed, you will now need to do so.

@MrGVSV MrGVSV added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 9, 2024
@MrGVSV MrGVSV added this to the 0.15 milestone Sep 9, 2024
@alice-i-cecile alice-i-cecile requested a review from soqb September 9, 2024 21:10
@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Sep 9, 2024
@MrGVSV MrGVSV force-pushed the mrgvsv/reflect/dynamic-typed branch from 5c863d1 to 1bed9c8 Compare September 10, 2024 00:33
@SpecificProtagonist
Copy link
Contributor

Very reasonable.
Might make sense to mention it in this line too.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 13, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 13, 2024
Merged via the queue into bevyengine:main with commit 37443e0 Sep 13, 2024
27 checks passed
@MrGVSV MrGVSV deleted the mrgvsv/reflect/dynamic-typed branch September 13, 2024 18:07
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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants