-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: Update on_unimplemented
attributes
#15110
bevy_reflect: Update on_unimplemented
attributes
#15110
Conversation
I'm not entirely sure if I should add some to the reflection subtraits like Since those aren't used as bounds in the derive macro (or almost at all really), I think the normal Thoughts here? Are they worth adding or is that overkill? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely better wording than my original messaging, good choices! I hadn't considered the way some IDEs only show the initial error statement.
26c6457
to
d8836fd
Compare
d8836fd
to
31f8f2c
Compare
Objective
Some of the new compile error messages are a little unclear (at least to me). For example:
While the annotation makes it clear that
FromReflect
is missing, it's not very clear from the main error message.My IDE lists errors with only their message immediately present:
This makes it hard to tell at a glance why my code isn't compiling.
Solution
Updated all
on_unimplemented
attributes inbevy_reflect
to mention the relevant trait—either the actual trait or the one users actually need to implement—as well as a small snippet of what not implementing them means.For example, failing to implement
TypePath
now mentions missing aTypePath
implementation. And failing to implementDynamicTypePath
now also mentions missing aTypePath
implementation, since that's the actual trait users need to implement (i.e. they shouldn't implementDynamicTypePath
directly).Lastly, I also added some missing
on_unimplemented
attributes forMaybeTyped
andRegisterForReflection
(which you can see in the image above).Here's how this looks in my IDE now:
Testing
You can test by adding the following code and verifying the compile errors are correct: