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: Register missing reflected types for bevy_render #6725

Closed
wants to merge 4 commits into from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Nov 22, 2022

Objective

Many types in bevy_render implemented Reflect but were not registered.

Solution

Register all types in bevy_render that impl Reflect.

This also registers additional dependent types (i.e. field types).

Note: Adding these dependent types would not be needed using something like #5781 😉


Changelog

  • Register missing bevy_render types in the TypeRegistry:
    • camera::RenderTarget
    • globals::GlobalsUniform
    • texture::Image
    • view::ComputedVisibility
    • view::Visibility
    • view::VisibleEntities
  • Register additional dependent types:
    • view::ComputedVisibilityFlags
    • Vec<Entity>

@MrGVSV MrGVSV added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types labels Nov 22, 2022
@inodentry
Copy link
Contributor

Ouch. Bevy really needs some more automated solution to the type registration problem ... forgetting to register types is a classic pitfall

Copy link
Contributor

@inodentry inodentry left a comment

Choose a reason for hiding this comment

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

A lot of these types are registered by the various sub-plugins, like CameraPlugin:
https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/camera/mod.rs#L23

Now you are just duplicating them here.

@MrGVSV
Copy link
Member Author

MrGVSV commented Nov 22, 2022

A lot of these types are registered by the various sub-plugins, like CameraPlugin:

https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/camera/mod.rs#L23

Now you are just duplicating them here.

Oof 🤦‍♂️

Great catch! I'll have to go through this again and check all the plugins to ensure everything is registered and then add the missing ones.

Yeah it'd be great if we had some way to catch this stuff.

@MrGVSV MrGVSV force-pushed the reflect-register-render branch 2 times, most recently from 537a209 to 32f123d Compare November 22, 2022 17:04
@MrGVSV MrGVSV added this to the 0.9.1 milestone Nov 22, 2022
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 23, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Nov 23, 2022
# Objective 

Many types in `bevy_render` implemented `Reflect` but were not registered.

## Solution

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like #5781 😉 

---

## Changelog

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
@bors
Copy link
Contributor

bors bot commented Nov 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Nov 23, 2022
# Objective 

Many types in `bevy_render` implemented `Reflect` but were not registered.

## Solution

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like #5781 😉 

---

## Changelog

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
@bors bors bot changed the title bevy_reflect: Register missing reflected types for bevy_render [Merged by Bors] - bevy_reflect: Register missing reflected types for bevy_render Nov 23, 2022
@bors bors bot closed this Nov 23, 2022
@sQu1rr sQu1rr mentioned this pull request Nov 25, 2022
cart pushed a commit to cart/bevy that referenced this pull request Dec 1, 2022
…yengine#6725)

Many types in `bevy_render` implemented `Reflect` but were not registered.

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like bevyengine#5781 😉

---

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
bors bot pushed a commit that referenced this pull request Dec 1, 2022
…_render` (#6725)" (#6811)

This had merge conflicts with the 0.9.1 release branch, so this had to be merged manually. Creating a PR for visibility + a final review.
bors bot pushed a commit that referenced this pull request Dec 1, 2022
…_render` (#6725)" (#6811)

This had merge conflicts with the 0.9.1 release branch, so this had to be merged manually. Creating a PR for visibility + a final review.
@MrGVSV MrGVSV mentioned this pull request Dec 3, 2022
3 tasks
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022
…yengine#6725)

Many types in `bevy_render` implemented `Reflect` but were not registered.

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like bevyengine#5781 😉

---

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
…yengine#6725)

# Objective

Many types in `bevy_render` implemented `Reflect` but were not registered.

## Solution

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like bevyengine#5781 😉

---

## Changelog

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…yengine#6725)

# Objective 

Many types in `bevy_render` implemented `Reflect` but were not registered.

## Solution

Register all types in `bevy_render` that impl `Reflect`.

This also registers additional dependent types (i.e. field types).

> Note: Adding these dependent types would not be needed using something like bevyengine#5781 😉 

---

## Changelog

- Register missing `bevy_render` types in the `TypeRegistry`:
  - `camera::RenderTarget`
  - `globals::GlobalsUniform`
  - `texture::Image`
  - `view::ComputedVisibility`
  - `view::Visibility`
  - `view::VisibleEntities`
- Register additional dependent types:
  - `view::ComputedVisibilityFlags`
  - `Vec<Entity>`
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 A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use 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: Done
Development

Successfully merging this pull request may close these issues.

4 participants