Automatic registration of ECS types

[james7132]

Objective

Registering types is error prone. Forgetting to register is a common error when working with scenes. #4154 is proof that it's a huge UX footgun. It'd be nice if ECS types (Components, Resources, Bundles) automatically registered the types upon use.

Solution

Make the Component and Resource derive macro check for the reflect attribute. Add a hidden function to the traits that has an empty default impl that generates code to register the type if the type has a reflect attribute. For any reasonable type that implements Reflect via the derive macro, the reflect attribute should be present to allow reflecting the ECS trait (i.e. reflect(Component).

Use the TypeRegistryArc embedded within the World to initialize AppTypeRegistry, which now implements FromWorld instead of Default.

Gate all of this behind the bevy_ecs feature for bevy_reflect.

There should be no performance impact since these functions are only called when initializing new Components.

TODO: Do the same with bundles.

---

This PR is all sorts of cursed. There's a huge number of caveats here:

Major caveats (no easy mitigation or solution currently):

• This does not work with generic types, due to the type bounds requirements from the Reflect derive macro.
• Components or resources that are not used by initialized systems or added via bundles must still be manually registered. This may pose issues for components and resources added only after some other initialization (i.e. Plugin::finish), which may create some obtuse timing errors. This poses a major UX issue in working with a hypothetical editor where a developer declares a new component, but it doesn't show up until it's used by a registered system or manually registered.
• Manual implementations of the traits breaks automatic registration.

Minor caveats (not significant, already mitigated, and/or has potential solution):

• This potentially makes using a reflect attribute with a Component derived type without implementing Reflect a compile error with no good way to resolve it. This is largely mitigated by requiring reflect(Component), which is not really going to be valid without the Reflect derive macro.
• This doesn't actually work on derive(Reflect) but rather the attribute used by the Reflect derive macro. This is largely mitigated by requiring reflect(Component), which is not really going to be valid without the Reflect derive macro.
• Untyped components aren't automatically registered, if that makes sense at all.

This PR, however, allows us to almost transparently handle full top-down type registration for scenes in the steady state without manual type registrations, now that #4154 has been merged, so the UX wins here may be worth the caveats, at least until specialization or some ctor equivalent for global link-time registration becomes available.

---

Changelog

Changed: Components and Resources that implement their trait and Reflect via the derive macros, and reflect their corresponding trait, will be automatically registered in the World's AppTypeRegistry resource upon first use (i..e spawning, inserting a resource, adding a system that uses it).

Migration Guide

TODO

[alice-i-cecile]

Definitely valuable, definitely cursed. I would really like to find a solid way to make this work.

[james7132]

Found another very big blocker: generic types do not play well with this without generic type bounds. Found this out with State<S> which cannot be registered due to insufficient type bounds on S. Fast way out of this is to just ignore types with generics, but that creates yet another sharp edge to this feature.

[james7132]

Added in the generics exclusion for now and it seems to be building just fine sans the still broken tests from the AppTypeRegistry change. I tried to carve out an exception when there was a Reflect trait bound on all type parameters, but realized that anything that subtraits Reflect is likely to produce incorrect results. This really needs specialization to resolve cleanly.

[MrGVSV]

This really needs specialization to resolve cleanly.

Maybe add some autoderef magic to make it even more cursed 😄

[james7132]

I don't think that will work in a generic context like we have in Components::init_component::<T>. Even with the autoderef trick, it'll always disambiguate to the blanket impl on Component.

[james7132]

Fixed the trivial test errors, but it seems like this potentially makes all of the scenes APIs, dependent on ReflectResource and ReflectComponent, cause the RwLock on the TypeRegistryArc to become reentrant and may risk deadlocking.

EDIT: fixed this by switching to a read-lock on failing to acquire a write-lock, and then returning if the type is already registered, panicking if and only if it needs to register but can't.

[cBournhonesque]

This requirement is so that we don't automatically register to the World non-ECS-related Reflect types?

[james7132]

This was requested so that users could opt out of this, such as avoiding serialization in scenes. I personally think we should find other ways of opting out of each use case and just move towards registration by default.

[MrGVSV]

We could also consider an opt-out attribute. There could be other reasons to exclude a type from the registry besides opting out of scenes. For larger projects, you may only want to register types you know you’ll need (to save on memory and reduce startup times). You might also be using a third party crate that uses the registry for things you want to opt out of as well. Or maybe you don't want to "leak" type information to downstream crates.

Whatever the reason, we need a way to opt out of automatic registration since registration is intentionally a one-way process: you can't undo or remove a registration. This is so that guarantees about the registered type are never broken without the user being aware of it (unless, of course, a mischievous plugin goes nuclear and replaces the entire AppTypeRegistry resource but that's a whole separate issue haha).

[SkiFire13]

+1 for the opt-out attribute. It's more explicit, and allows this to work even when the user forgets to reflect Component/Resource

[cart]

I think the questions are:

• Can this be relied upon?
• Would we recommend that Bevy developers rely on this?

Things like this seem like serious footguns:

#[derive(Component, Reflect)]
#[reflect(Component)]
struct Foo;

app.add_systems(Update, foo);

fn foo(foos: Query<&Foo>) {}

During development, developer comments out system. Foo scenes stop working / editor loses track of Foo

// app.add_systems(Update, foo);

During development, developer changes query to something else, Foo scenes stop working / editor loses track of Foo

fn foo(bar: Query<&Bar>) {}

Plugin registers a system only if a setting is enabled. When setting is disabled, Foo scenes stop working / editor loses track of Foo

if self.should_foo {
  app.add_systems(Update, foo)
}

To protect against these scenarios, I think as a best practice we would still encourage people to manually register types. It definitely seems prudent to do that for built-in Bevy types. If its not something we're comfortable using in upstream plugins, can we really give it to users?

Which "recommended workflow" is better?

1. Is it better for developers in something like 80% percent of cases to forget to register (ex: don't know about registration yet or simply forgot), notice that a component isn't showing up in the editor / scene loads are failing, manually register, and then know with certainty that the registration will be there forever?
2. Or is it better for developers in something like 95% of cases to not register manually, and then every so often hit weird corner cases that don't work (and they won't understand why because they've never had to think about manual registration).

(1) Is predictable and trains people to think about registration at the cost of boilerplate and one-time friction when forgotten.
(2) Is unpredictable, trains people to not think about registration, is better ergonomically, and will cause weird / frustrating bugs when people fall through the gaps in the magic.