EntityMut::get_components_mut.

[chescock]

Objective

Provide a simple way to get mutable access to multiple components from an EntityMut at the cost of some performance.

This is the API from #13375, but with access checks added. Cart said these checks are "dissatisfying from a perf perspective because Access allocates" in #13375 (comment). But we haven't found a more performant approach, and there are use cases where doing these checks on each call is still fast enough.

Note that unchecked variants were added in #20265.

Solution

Add get_components_mut and into_components_mut to EntityMut and EntityWorldMut. Have them perform the access checks by calling QueryData::update_component_access before calling into_components_mut_unchecked.

[cBournhonesque]

I'm not sure if this is a good idea; the access checks are precisely the reason why #13375 was not merged.

Adding unsafe variants in #20265 is one thing (since there is no perf cost and the user has to opt-in to using unsafe), but adding this safe API will encourage a lot of users to start using it without necessarily putting thought into the perf implications.

I guess @cart should probably weigh in?

[chescock]

I'm not sure if this is a good idea; the access checks are precisely the reason why #13375 was not merged.

Adding unsafe variants in #20265 is one thing (since there is no perf cost and the user has to opt-in to using unsafe), but adding this safe API will encourage a lot of users to start using it without necessarily putting thought into the perf implications.

Yeah, I won't be too surprised if this gets closed, but I think it's worth considering, and I had the code ready. :)

There are plenty of cases where the perf cost won't matter, and there are users who want this functionality even with the cost. And if they wind up in a situation where the performance is a problem, they can switch to the unchecked variants to skip the runtime checks.

(And for code that's really performance sensitive, you even want to avoid the .get::<C>() methods since they do the hash lookup from TypeId to ComponentId on every call. You'll want to cache the ComponentId, either using queries or calling the _by_id methods manually.)

[EmileGr]

adding this safe API will encourage a lot of users to start using it without necessarily putting thought into the perf implications.

I can understand that users may gravitate towards it purely because it's marked as safe. That said, there's no excuse for users not profiling their code and Bevy should not design its API as if users cannot be trusted to do that. Simply document it as being much slower than the unchecked version and let users decide.

[alice-i-cecile]

I think this is worth doing, even with a relatively slow implementation. It's a useful helper, and frankly, perf-sensitive code shouldn't be using this path in the first place.

[chescock]

Closing in favor of #21780, which offers a more efficient allocation-free implementation of this API.