bevy: `Mut`/`ResMut` mutable access without triggering change detection

This feels like an easy footgun to avoid fixing a bad resource modelling

Something that I could find interesting is having a way to disable change detection on a field at the struct declaration

This is a much more interesting route IMO: it allows bypassing in an explicit way, defined on the struct itself. You can get the same effect with interior mutability, so I prefer the explicitness of @mockersf proposal.

I really don’t like the idea of allowing consumers to make changes in an untracked way: it seriously violates the guarantees provided in a way that seems virtually impossible to debug since it could be occurring anywhere in your code base.

As I still disagree on your assessment on the State issue, I also disagree on how we should fix it 😄

For disabling change detection, its cost is very light, even more so on resource. And to get completely free of it you would also have to disable it on the system

Something that I could find interesting is having a way to disable change detection on a field at the struct declaration

#[derive(Resource)]
struct MyResource {
    field1: u8,
    #[resource(untracked)]
    field2: u8
}

I’d rather not touch any macro stuff until #2254 lands, so for me I’d first do 2, and then do an opt-in get_untracked:

trait AllowGetUntracked {}

impl<'a, T: Component + AllowGetUntracked> Mut<'a, T> {
    fn get_untracked(&mut self) -> &mut T {
        &mut self.value
    }
}

Having a way to mutably access a struct without change detection is very useful in bevy-inspector-egui. Since egui expects e.g. &mut f32 for a slider widget, you have to give out a mutable reference which makes change detection not work.

To fix that I compied the Mut implementation and added get_mut_silent and mark_changed functions: https://github.com/jakobhellermann/bevy-inspector-egui/blob/3a0fd8970df3337a069778ecbeee47db7cd30961/src/plugin.rs#L242-L254.

This usecase wouldn’t be solved by #[resource(untracked)].

Hmm, while the macro approach looks interesting, I’m not sure how we can implement it. The only thing I can think of is generating a trait and implementing it on Mut<T> to add a acessor method that bypasses triggering change detection, but this causes a massive footgun where using field access instead of a method call suddenly re introduces change detection, and generating a trait fully implicitly is also rather ugly IMO. Furthermore, implementing this approach would require a full bypass anyways. So, I think we should just add the simple bypass now, and consider the much more complicated macro version after #2254 lands.

Personally, I still think an opt-in version of get_unchecked would be worth the trouble, so I’m opening this again. Interior mutability is not a very nice solution imo, as it breaks multiple very nice guarantees which rust makes and we rely on.

I’m pretty in favor of going the interior mutability route (or splitting up the resources into parts) paths.

Thanks everyone for discussing this issue with me, and now I feel ok to close this issue since I prefer the proposed paths over the get_unchecked one 😃

I think it’s because resources are often used for more complex things: assets, state, thread pools, input, events, …

Either splitting resources between observable/internal, or using interior mutability pattern sounds good to me 👍

Juxtapose with #1661.

And it’s not anywhere in your code base, it’s only where you explicitly use the get_unchecked method.

What about third-party plugins?

I disagree with this idea as written: it changes the API contract of change detection from reliable “this will trip on mutable access” to ambiguous “this will trip on mutable access unless it’s been opted out of”, which just screams “footgun” to me. Fancy macro looks more palatable, but even with an implementation like that: how would the user know it’s been opted out of, before just running into it and without going to the docs or source to look up specifically this?

Most importantly: what actual problem would this solve? If it’s about #2343, I strongly disagree with this being a solution. Are there any more concrete examples? I don’t like the idea of complicating an API contract just because, especially if we feel like we have to hide it (“Sure I can see this as being a potential footgun, however there are steps you can take to avoid this. E.g. not automatically exporting the associated trait, adding docs etc…”).

Wouldn’t this case be easily solved by just copying the float into a mutable temporary, giving a ref to that and writing back if it changed? I don’t see why get untracked is needed here.

The case of the float could be solved that way, but not every type that is inspectable is also Copy or Clone.