Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
maxwellodri opened this issue Jan 7, 2023 · 2 comments

Comments

@maxwellodri
Copy link

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
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants