jakobhellermann / bevy-inspector-egui

Inspector plugin for the bevy game engine
Apache License 2.0
1.19k stars 173 forks source link

Inspecting without mutating #8

Closed mvlabat closed 1 year ago

mvlabat commented 3 years ago

Hi!

I wanted to suggest adding an immutable variant of Inspectable trait, which would allow only viewing values (or probably just another trait function).

There are some resources, particular components or their fields in my game, that I would like to be able to view, but prohibit myself from editing. Is it something that you could consider supporting?

Btw, this is a really awesome crate, thanks for developing it. :)

jakobhellermann commented 3 years ago

Do you have mutable access to the value even if you don't want to modify it? If so, you could do this:

struct Immutable<T>(T);
impl<T: Inspectable> Inspectable for Immutable<T> {
    type Attributes = T::Attributes;

    fn ui(&mut self, ui: &mut egui::Ui, options: Self::Attributes, context: &Context) {
        ui.wrap(|ui| {
            ui.set_enabled(false);
            self.0.ui(ui, options, context);
        });
    }
}
jakobhellermann commented 3 years ago

Also, thanks for creating the bevy-egui crate, this one wouldn't be possible without it :)

mvlabat commented 3 years ago

Do you have mutable access to the value even if you don't want to modify it?

Ideally, I would like to avoid having mutable access to values if I don't want to modify them. Also, I'm afraid that wrapping resources in Immutable won't work since it implies storing those resources as Immutable<T>. For example, a plugin like InspectorPlugin::<Immutable<Data>>::new() will require an actual Immutable<Data> resource to exist.

But doing ui.set_enabled and making manual ui calls on structs that implement Inspectable trait seems like a good workaround though! I'm quite surprised it already works for such widgets as PointSelect out of the box.

MrGVSV commented 2 years ago

I just started using the crate (it's awesome), but was also wondering if an immutable Inspectable trait could be added.

When I add an inspector to view a resource in my game, it does so with mutable access. This seems to trigger Bevy's Change Detection, which can cause issues. A version that doesn't require &mut self would great!

jakobhellermann commented 2 years ago

An immutable version of the trait (or an immutable method on the same trait) is definitely worth considering, the downside is that the derive macro needs to generate a lot of duplicate code.

That said, if the only problem you have with the &mut access is change detection, that should only be triggered when the data is actually being modified. Is your resource.is_changed() always true?

MrGVSV commented 2 years ago

That said, if the only problem you have with the &mut access is change detection, that should only be triggered when the data is actually being modified.

I thought change detection happened for any DerefMut?

Is your resource.is_changed() always true?

Yes it is. Using this code:

#[derive(Inspectable, Default)]
struct DummyResource {
  foo: i32,
}

#[derive(Inspectable, Default)]
struct InspectorData {
  dummy: ResourceInspector<DummyResource>,
}

fn test_change(dummy: Res<DummyResource>) {
  if dummy.is_changed() {
    println!("Changed!");
  }
}

causes "Changed!" to be printed to the console every frame.

jakobhellermann commented 2 years ago

I thought change detection happened for any DerefMut?

It usually does, but for an InspectorPlugin<T> I worked around it so that the Res<T> will only be changed when it actually is.

This doesn't work through the ResourceInspector however.

I have a branch locally where I started adding an immutable variant, and it doesnt look as overly duplicate as I anticipated. I'll play around with it some more and may release the next version with that change.

Alternatively, bevy could expose APIs to allow dereferencing a ResMut without triggering its change detection, so that bevy-inspector-egui could fix the issue that way. This has the advantage that it also works with mutating the resource through the resource inspector while only triggering change detection if needed. Such a change would be pretty simple to implement, provided it is a feature that cart would want in bevy.

MrGVSV commented 2 years ago

Ah I see, so I should use a standard InspectorPlugin<T> to view the resource then?

Alternatively, bevy could expose APIs to allow dereferencing a ResMut without triggering its change detection

Yeah that would be a really nice change. Coming from Unity, we had things like SetValueWithoutNotify(...) to achieve this. I think it could be really helpful. But maybe there's a reason they haven't implemented something like this yet?

jakobhellermann commented 2 years ago

Yes, by default an InspectorPlugin<T> will create a resource of T::default() and display that.

But maybe there's a reason they haven't implemented something like this yet?

I'm pretty sure it is just a very niche use case with few legitimate reasons for existing, so it wasn't included from the start. As long as the method is properly documented as to when you would use it I see no harm in including it, unless bevy wants to 100% guarantee change detection being correct all the time.

MrGVSV commented 2 years ago

unless bevy wants to 100% guarantee change detection being correct all the time.

To be fair, it would be 100% guaranteed even with something like this. Opting out of change detection shouldn't count against that. Maybe I'll ask about it in the Discord and see what the thoughts are on it.

Thanks for the help!

jakobhellermann commented 1 year ago

Implemented in 0.16:


let env = InspectorUi::for_bevy(); // able to display Handles
env.ui_for_reflect_readonly(value, ui, type_registry);
// or
bevy_inspector_egui::reflect_inspector::ui_for_value_readonly(value, ui, type_registry) // without Handle support