Make resources usable as components by moving IsResource off of the required components path

[alice-i-cecile]

Objective

One of my goals with resources-as-components (#20934) was to support patterns where data can be used as either a resource or a component fluidly.
Query for the component type once, and hit it everywhere.

This is prominently used in leafwing-input-manager, where many users asked for the ability to register InputMaps and ActionStates globally, for games where there's no natural "player" entity to store them.

Unfortunately (as I discovered during upgrading this crate), resources-as-components broke this pattern badly.
There were two bugs:

1. Components inserted onto ordinary entities that happen to be resources are registered as resources (Inserting a resource as a component should not make it available as a resource #24591). This is surprising!
2. Inserting a second copy of an entity that happens to be a resource removes

Fixes #24591. Fixes #24592.

Solution

Both of these problems stem from the strategy used to enforce uniqueness: IsResource's on_insert hook, which is inserted as a required component via the Resource derive macro.

This meant that every usage of the resource type was subject to the resource uniqueness constraints, even if it was not being used as a resource.
And at the same time, it meant that every usage of that type was recording as the canonical resource location.

To fix this, there are basically two steps:

1. Stop inserting IsResource as a required component.
2. Add IsResource during initialization via another approach.

I opted to achieve 2 by adding a World::get_or_insert_resource_entity helper,
and warning extensively that it must be used at all of the attractive nuisance methods where you might bypass it.
Initially this was just a bit of boilerplate copied around, but that managed to miss the reflection paths and I decided to make it Enterprise Grade.

This fixes both bugs above, since the whole IsResource path is only exercised when you're using the type as, you know,
a component.
As a bonus, the uniqueness guarantees are now robust to manual Resource impls. I don't know why you would do that, but hey!

This results in a single extra archetype move on a very cold path (resource insertion).
I think that's well worth it to avoid regressing semantics and to allow resources-as-components to actually live up to its promise here.

We could decide that this is an intended breakage and close this PR.
If we do so, I will be sad, and also we need to extend the migration guide for resources-as-components to clearly note this limitation.

Testing

Two new tests: checking for both regressions reported.

[alice-i-cecile]

I decided to beef up the docs here while I was at it: this is the most natural home for explaining the machinery of resources-as-components.

[cart]

I'd like to make the case that a Resource should always be a unique component:

1. Resource semantically means "there is only one of this thing". It is "unique". Our docs describe it that way:

   /// A type that can be inserted into a [World] as a singleton.
   /// You can access resource data in systems using the [Res] and [ResMut] system parameters
   /// Only one resource of each type can be stored in a [World] at any given time.

   We've at various points discussed how Res could just be Single (I don't think we should actually do that, just bringing it up to make a point). If we allow "non-resource resources", Single<R: Resource> queries would start panicking.

2. Deriving Resource is saying "this thing is a Resource". Allowing "non-resource resources" feels odd to me.

3. Calling something a Component or a Resource is a form of (statically enforced) documentation. A Component derive is a way to say "you can have zero to many of this thing, you can add it to whatever entities you want, you should Query / entity.insert / entity.get it, etc. Resource is a way to say "this is a unique thing", you should Res / ResMut / init_resource it. If we allow Resources to also not be "resources", thats means that people can't make any of the "resource-like" assumptions about it (ex: to get all instances of a type, they need to check both Res and Query for it cannot rely on Res / they need to Query for it , the resource might not be initialized but a Component instance of it might exist, etc). All of these assumptions that we get statically for free now require documentation to be authored (and read!).

The cost of more predictable, self-documenting APIs is that someone that wants both a "local" InputMap and a "global" InputMap must define separate types (ex: an InputMap Resource and a LocalInputMap Component) . I think there is a strong argument to be made that those things should be separate types, as they have very different behaviors and usage constraints.

[cart]

Definitely worth discussing though. The added flexibility of not needing to define separate types / being able to write one InputMap logic handler that runs for everything is compelling.

But in that specific case, I don't think you could just write a single InputMap query, as the Res<InputMap> likely needs to be processed first, so you'd be doing an IsResource query anyway

[alice-i-cecile]

Yeah, in the specific case of InputMap / ActionState this actually doesn't help me much (other than reducing bugginess). There's two resources which need to interact with each other, so I can't actually reduce the code.

I'm fine if "resources are always distinct" is the direction you want to take things, but that cuts against the Resource: Component bound that you pushed for in the original PR. My original design did not do that, using a wrapper instead, and these footguns were far less prevalant.

I feel like the current approach is trying to have it both ways and doing it badly. If Resource implies Component, the type should genuinely be usable as such. Telling people to read the docs and "just don't insert this as a component" feels weak when we could simply statically enforce it!

[cart]

I feel like the current approach is trying to have it both ways and doing it badly. If Resource implies Component, the type should genuinely be usable as such. Telling people to read the docs and "just don't insert this as a component" feels weak when we could simply statically enforce it!

I think it logically works in the context of "Resource is a unique-constrained Component". It can do everything a component can do (ex: entity.get, Query, etc), with the added / layered on constraint of uniqueness. Supertraits are allowed to layer on new behaviors.

Telling people to read the docs and "just don't insert this as a component"

Spawning (or inserting) a Resource as a Component is allowed. It just shouldn't be done if it would result in a second instance of the Resource. I think thats a pretty understandable and fair constraint, given that Resource means "unique".

[Diddykonga]

After thinking about it, I agree with cart in the first part, where Resource is adding an invariant on top of Component, so there is no two ways to use it, only one. (This then makes less sense to have bifurcated API's but eh)

The second part, I would like to expand possibly to allow keeping the new value, and discarding the old one.

[amtep]

I ran into this when updating moonshine_util (not my crate). Unlike alice, I thought it was an intended breaking change :)

moonshine_util has an "Expect" query mechanic which enforces that a component is accompanied by another component. It has a Resource+Component ExpectDeferred to temporarily turn off those checks, for example during game load. Insert it as a resource to turn off checks globally, insert it as a component to turn off checks for that entity.

[FreeQueue]

I think the cleaner model may be to make uniqueness an explicit component constraint, rather than implicitly tying it to Resource.

Resource should mean “this type has one canonical instance in the resource namespace and can be accessed through Res<T> / ResMut<T> / insert_resource”. That does not necessarily imply that the same type cannot also have local component instances.

For example, ExpectDeferred being both a resource and a component seems like a valid pattern: as a resource it disables checks globally, while as a component it disables checks for one entity.

If Bevy wants to express world-level uniqueness for components, I would prefer that to be modeled directly:

#[derive(Component)]
#[component(unique)]
struct Foo;

Then a type that is both a resource and forbidden from having local instances could say so explicitly:

#[derive(Resource)]
#[component(unique)]
struct Foo;

So Resource and unique component become orthogonal concepts: resources are unique as resources, while #[component(unique)] controls uniqueness in the component namespace.

[FreeQueue]

More broadly, I’d be excited to see Bevy eventually grow a more general component constraint system, where unique is just one constraint alongside things like mutually exclusive component groups for modeling state machines.

[chescock]

I have nothing to add to the debate about whether or not we should do this, but I'm happy to approve (and nitpick) the implementation :).

[chescock]

If we make resources usable as components, then we probably want to make it possible to set the storage type to Table. Currently #[derive(Resource)] sets it to SparseSet and does not allow it to be overridden.

[chescock]

I think get_or_spawn_resource_entity lets you combine the two insert branches here.

GitHub is being difficult about showing suggestions on unchanged lines, but you can replace this if let with an unconditional let entity = self.get_or_spawn_resource_entity(resource_id); and then delete everything after the return as unreachable.

[chescock]

Given how many places you were able to simplify, it might be worth adding get_or_spawn_resource_entity even if we don't do the rest of this change!

[chescock]

It might be worth emphasizing in this doc comment that the entity won't necessarily have the resource component on it, and definitely won't if it was just spawned.

[jnhyatt]

I'll throw my two cents in -- as a user, I find it surprising to hear that the intended behavior of "Resources are Components" is "only one of these no matter where it is". The apparent intent from my perspective was "Resources can now do Component things", which includes being inserted onto entities. Uniqueness is very surprising.

Overall I agree with the sentiment that uniqueness should be orthogonal -- unique components that aren't resources would be a cool feature, and components that exist as singletons in the world instead of on entities are more what I expected from this feature.

[Jengamon]

adding my pair of extinct pennies, I think "Resource" being used as equivalent to "unique Component" (Resources are not usable as Components) is a more communicable restriction than "Resource can be a unique Component (but can also be used as a Component)".

I am not really sold on the value of Resources usable as Components that a newtype-wrapper with Deref wouldn't solve for, and personally value the clear Resource / Component logical split that Resources being unusable as Components gives.

[amtep]

I recently realized that this is a breaking change either way. A program that in 0.18 uses a type as both Component and Resource will probably have a system that queries the Component. In 0.19 that query will also catch the instance that was inserted as a Resource, possibly leading to unintended effects.