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

[Merged by Bors] - bevy_reflect: Added get_boxed method to reflect_trait #4120

Closed
wants to merge 3 commits into from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Mar 6, 2022

Objective

Allow Box<dyn Reflect> to be converted into a Box<dyn MyTrait> using the #[reflect_trait] macro. The other methods get and get_mut only provide a reference to the reflected object.

Solution

Add a get_boxed method to the Reflect*** struct generated by the #[reflect_trait] macro. This method takes in a Box<dyn Reflect> and returns a Box<dyn MyTrait>.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 6, 2022
@MrGVSV MrGVSV changed the title Added get_boxed method Added get_boxed method to reflect_trait Mar 6, 2022
@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types C-Usability A simple quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Mar 6, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Implementation looks good, and this is clear and useful functionality. I would really like docs on this (and get_func and get_mut_funct), but I'm not immediately sure where they should go. Sticking them on the macro is waaay deep in the internals, and sticking them on the generated code is a bit odd.

I think that I would like it on both: it looks like we're generating a ReflectFoo type when this is used, and that should have proper docs.

Could you please add:

  • basic docs to reflect_trait function.
  • generated doc strings for the generated trait
  • generated doc strings for the methods

@MrGVSV
Copy link
Member Author

MrGVSV commented Mar 6, 2022

Implementation looks good, and this is clear and useful functionality. I would really like docs on this (and get_func and get_mut_funct), but I'm not immediately sure where they should go.

Sticking them on the macro is waaay deep in the internals, and sticking them on the generated code is a bit odd.

Haha I was actually going to ask about this. I think it makes sense to put them in the generated code. In that case it shows up in intellisense and other editor tools.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

LGTM! It's nice to slowly document reflection too :)

@MrGVSV MrGVSV changed the title Added get_boxed method to reflect_trait bevy_reflect: Added get_boxed method to reflect_trait Mar 28, 2022
Copy link
Contributor

@PROMETHIA-27 PROMETHIA-27 left a comment

Choose a reason for hiding this comment

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

LGTM!

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 20, 2022
# Objective

Allow `Box<dyn Reflect>` to be converted into a `Box<dyn MyTrait>` using the `#[reflect_trait]` macro. The other methods `get` and `get_mut` only provide a reference to the reflected object.

## Solution

Add a `get_boxed` method to the `Reflect***` struct generated by the `#[reflect_trait]` macro. This method takes in a `Box<dyn Reflect>` and returns a `Box<dyn MyTrait>`.


Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
@bors bors bot changed the title bevy_reflect: Added get_boxed method to reflect_trait [Merged by Bors] - bevy_reflect: Added get_boxed method to reflect_trait May 20, 2022
@bors bors bot closed this May 20, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Jun 7, 2022
…#4120)

# Objective

Allow `Box<dyn Reflect>` to be converted into a `Box<dyn MyTrait>` using the `#[reflect_trait]` macro. The other methods `get` and `get_mut` only provide a reference to the reflected object.

## Solution

Add a `get_boxed` method to the `Reflect***` struct generated by the `#[reflect_trait]` macro. This method takes in a `Box<dyn Reflect>` and returns a `Box<dyn MyTrait>`.


Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…#4120)

# Objective

Allow `Box<dyn Reflect>` to be converted into a `Box<dyn MyTrait>` using the `#[reflect_trait]` macro. The other methods `get` and `get_mut` only provide a reference to the reflected object.

## Solution

Add a `get_boxed` method to the `Reflect***` struct generated by the `#[reflect_trait]` macro. This method takes in a `Box<dyn Reflect>` and returns a `Box<dyn MyTrait>`.


Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
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 simple quality-of-life change that makes Bevy easier to use
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants