-
Notifications
You must be signed in to change notification settings - Fork 334
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
Fix component reflection such as to not force the component to impl Default
#7439
Comments
I ran into this myself when adding |
We discussed this internally quite a bit internally The gist of it that we do need guaranteed defaults for anything that's potentially ui editable (for the value to start out with). Overall, the force to default was introduced as a simplification so that anyone querying a fallback can rely on it being there and treat it as an hard error when it isn't. |
@kpreid Edit: to clarify, yes in this case there should always be a fallback provider that computes the proper default. But the design choice here was to protect against what happens if there is none. E.g. someone adds a new visualizer that doesn't specify the fallback provider. |
Have some work in progress that allows to generate placeholders fully generically so we need lot less of those pointless looking defaults and can actually make use of the fact there's always a placeholder 🙂 since we still want to use |
Currently, the codegen'd component reflection always emit a call to
C::default()
, forcing the component toimpl Default
. Sometime this really is meaning less (e.g what's a defaultEntityPath
orComponentName
?).Funnily the
ComponentReflection
struct actually has anOption
. The codegen should therefore emitNone
if the component doesn't haveDefault
(which the codegen knows from theattr.rust.derive
attribute).Edit: the codegen doesn't always know about
Default
if it's manually implemented. So that should be controlled with a new fbs attributeattr.rust.no_placeholder
The text was updated successfully, but these errors were encountered: