Resource hooks & immutable resources

[SpecificProtagonist]

Objective

Enables the resource derive macro to specify the attributes for

• hooks (fixes Support hooks on resource types #24159)
• immutability (fixes Immutable resources #24166)

Also fixes resource_scope overwriting change ticks and a few related doc comments.

Showcase

#[derive(Resource)]
#[component(immutable)]
#[component(on_insert = update_level, on_discard = update_level)]
struct Level(i32);

[github-actions]

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

[alice-i-cecile]

Drop this pr in the resources as entities release notes please!

[alice-i-cecile]

Needs a test please <3

[urben1680]

You should go through every API that offers mutable references to resources and add a <Mutability = Mutable> bound to them, I think that is missing. Like World::get_resource_mut. Untyped methods like World::get_resource_mut_by_id should fail if the resource is immutable.

[Trashtalk217]

ResMut shouldn't work on immutable resources. Is that implemented?

[Trashtalk217]

When I originally envisioned this, I called it #[resource(...)]. It doesn't really matter, but it might be easier to understand.

[SpecificProtagonist]

I do not have a strong opinion on this, happy to change it.

[Trashtalk217]

Me neither :)

[alice-i-cecile]

This looks good now; is there anything else you'd like to do beyond getting CI passing?

[urben1680]

Please don't forget to address my note: #24164 (review)

If you can mutate immutable resources via World/DeferredWorld/EntityWorldMut methods (and maybe others?) that would be bad.

[SpecificProtagonist]

Please don't forget to address my note

The dynamic methods refuse to return a MutUntyped with immutable component. The typed ones (incl. system param) can give you a ResMut, but it doesn't have any mutation methods for immutable resources. There are some fixed I still need to make:

This looks good now; is there anything else you'd like to do beyond getting CI passing?

• FilteredResourcesMut::get_mut_by_id_unchecked needs to refuse immutable components.
• typed methods granting ResMut should not silently fail on immutable components

[urben1680]

The typed ones (incl. system param) can give you a ResMut, but it doesn't have any mutation methods for immutable resources.

Seems not to be true, or you miss that I talk about World/DeferredWorld/EntityWorldMut methods, not system params / query data.

World::resource_mut returns a Mut<T> for mutable resources and panics for immutable ones currently I think. World::get_resource_mut returns None for immutable resources.
But For both I would actually prefer a bound on the generic type because that is how it is for fetching mutable references of components. For example World::get_mut.

[chescock]

ResMut shouldn't work on immutable resources. Is that implemented?

You cannot get a ResMut for an immutable resource (edit: found one place that still allows it: FilteredResourcesMut when used with a dynamic id).
I do want to also statically enforce this. Should mutability be required by

• ResMut, or
• all ResMut methods that allow mutation?

The standard library tends to go for the later (see e.g. HashMap).

Hmm. Let's do the latter to make generic code easier.

One disadvantage of allowing ResMut<ImmutableResource> to exist is that it creates an actual &mut ImmutableResource. Prohibiting &muts opens up some interesting possibilities, such as declaring that it's valid to obtain an &ImmutableResource from an UnsafeWorldCell without tracking access.

[alice-i-cecile]

We could do both even. No strong feelings here, but I would like to get this merged in a timely fashion.

[urben1680]

I think there are a handful of APIs that need <Mutability = Mutable> bounds:

• ParamBuilder::resource_mut
• TemplateContext::resource_mut
• World::get_resource_or_init
• World::get_resource_or_insert_with

I wonder if try_resource_scope does need to do extra care for immutable resources to ensure that the insert and change tick are equal.

[SpecificProtagonist]

We could do both even.

I'm not sure what you mean?

World::get_resource_or_init World::get_resource_or_insert_with

Thanks!

I wonder if try_resource_scope does need to do extra care for immutable resources to ensure that the insert and change tick are equal.

Resource scope doesn't care much about component mutability. But on main, the resource scope always set both added and changed to the value that changed had beforehand, making both wrong. Since I've had to touch that function anyways, I fixed it and added a test.

But For both I would actually prefer a bound on the generic type because that is how it is for fetching mutable references of components.

Note that a difference between ResMut and Mut is that the former knows whether the resource is mutable, the later doesn't. But no strong opinions from me here.

But I've added a commit that switches ResMut to require Mutability = Mutable.