Require #[derive(Component)] for component types

[Frizi]

Rendered

Early discussion issue
Implementation PR

[cart]

I'd like it if the RFC compared "api design choices" a bit more:

• Derive DSLs vs manual impls
• Methods of defining storage type in Component trait

[alice-i-cecile]

This is compelling enough for me as is, and I agree with everything stated in it.

[Ixentus]

I agree with everything in this RFC about #[derive(Component)]. It offers a ton of benefits.

I don't like #[component(storage = "SparseSet")]. #[derive(Component)] is alright since most Components have derives anyway, but this storage one doesn't seem necessary. It looks like cart has some alternatives for this. Perhaps this change could be removed from the RFC and moved to a new issue, so the RFC can be accepted easily.

[alice-i-cecile]

@Ixentus the new component storage configuration approach is much nicer than the old strategy. It will remain an optional optimization; simply omitting any configuration will use the default table storage.

[wilk10]

Coming from a user perspective, I'd like to add that I really don't mind the (limited) extra boilerplate. More than once I found myself scanning through my structs wondering: "Is this a Component?", so besides the technical benefits listed above, I would enjoy the (already mentioned) visual indication at first glance as well.

[NathanSWard]

More than once I found myself scanning through my structs wondering: "Is this a Component?

I agree. This is one of the big upside to this RFC that is also really like.

Something else, I would like a world where change detection has an opt-out path, but that'll only be possibly once we have #[derive(Component)] and #[derive(Resource)]

[BoxyUwU]

I'm not sold on #[component(storage = "SparseSet")] but other than that I quite like this, personally I'd consider not having derive(Component) to be a blocker for relations

[NathanSWard]

I'm not sold on #[component(storage = "SparseSet")]

@BoxyUwU I'm curious about what part of this you're not sold on. Is it the static storage type, the syntax, etc...?

[BoxyUwU]

ah sorry yes should have clarified, it's just the syntax yeah. not a fan of using a string here and it's also a lot of text to write out. I'm not familiar with proc macros though so have no idea how actionable that is. Either way the proposed syntax seems totally workable and I wouldn't hate writing it :)

[Frizi]

I actually tried without the string, but that would be kinda non-standard. The attribute parser provided with syn crate only allows literals at that position. We could roll our own if that's really desired, but it's just more work and definitely less common pattern.

[Ixentus]

Thanks for that write-up @alice-i-cecile , I am now much happier with this DSL macro.

[cart]

I think this is probably good to go. I do wish this was a bit more prescriptive about implementation details (ex: what should the Storage associated type look like? How will that be consumed in things like Fetch impls? Will Component::Storage be constrained to a Storage trait?)

But i've also dragged my feet on this enough and we do have an implementation that we all like.