jakobhellermann / bevy-inspector-egui

Inspector plugin for the bevy game engine
Apache License 2.0
1.12k stars 166 forks source link

new derive InspectorOptions doesn't allow fields to refer to each other. #111

Open maxwellodri opened 1 year ago

maxwellodri commented 1 year ago

For example:

#[derive(Reflect, InspectorOptions]
struct MyAbility {
    #[inspector(min = 0, max = 10)]
    max_charges: u8,
    #[inspector(min = 0, max = self.max_charges)]
    current_charges: u8,
}

In the previous version this worked fine.

jakobhellermann commented 1 year ago

Oh wow, I didn't even know that worked before. But it makes sense, as the Inspectable impl was just a function with self being MyAbility, and the options were created on every ui call.

In the new version InspectorOptions is just a struct that is stored once in the TypeRegistry associated to MyAbility, so it cannot depend on a particular instance. This could maybe be made to work by not storing NumberOptions for current_charges but instead Fn(&MyAbility) -> NumberOptions that will be evaluated when walking the Reflect to display it, but that would be complicated.

An alternative solution would be to have something like this

#[derive(Reflect, InspectorOptions)
#[inspector(validate = |ability| ability.current_charges <= ability.max_charges)]
struct MyAbility {
    #[inspector(min = 0, max = 10)]
    max_charges: u8,
    #[inspector(min = 0, max = self.max_charges)]
    current_charges: u8,
}

which would only allow changes that pass the validate function.

This would be more flexible and useful for e.g. the Msaa type:

#[inspector(validate = |msaa| msaa.samples.is_power_of_two())]

How important is this feature to you?

maxwellodri commented 1 year ago

Definitely a nice QOL to have but not critical, mostly for making sure you don't create invalid state like with Msaa, I like the closure idea personally.