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

add test checking that types are registered (as far as possible) #6061

Closed

Conversation

jakobhellermann
Copy link
Contributor

Objective

  1. every type that is used somewhere in a reflected type should be registered
  2. every component/resource that is registered should include their respective ReflectResource/ReflectComponent type data

Solution

  • Luckily both objectives can be checked at runtime, so add a test that does exactly that
  • register the missing types

Note: this does not lint for types that implement Reflect but are not registered. This can't be done at runtime, but maybe as a script running on rustdoc output.

.register_type::<bevy_math::Quat>()
.register_type::<bevy_math::Rect>()
.register_type::<Option<bevy_math::Rect>>()
.register_type::<Option<bevy_math::Vec2>>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose to only register Option<Vec2> here. We could in theory register Option<T>, Vec<T> etc. for every conceivable type, but since there's no need to do this where the types are defined and it would introduce a lot of unnecessary TypeInfo into the binary I opted to only register those that are currently in use.

@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types C-Testing A change that impacts how we test Bevy or how users test their apps C-Code-Quality A section of code that is hard to understand or change labels Sep 22, 2022
@mockersf
Copy link
Member

seems related to #5781

@jakobhellermann
Copy link
Contributor Author

seems related to #5781

Yes, that would make half of this PR obsolete.

@janhohenheim janhohenheim added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Sep 14, 2024
@richchurcher
Copy link
Contributor

Backlog cleanup: closing due to inactivity, and seemingly partially covered by the merge of #5781 earlier this year. See also #15030 with similar goals.

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-Code-Quality A section of code that is hard to understand or change C-Testing A change that impacts how we test Bevy or how users test their apps S-Adopt-Me The original PR author has no intent to complete this work. Pick me up!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants