Checking if multiple editing widgets have changed the given data
I needed this in my own editor so I just implemented it.
It turned out to be a bit more advanced since I needed at least two editing groups (one for updating data version, another for updating realtime preview which takes more time).
Is anyone else in need of this feature?
P.S. I also added two new parts to the demo - "data version" menu "button" that increments with every change and a "Widgets/Change checks" section that also features masking behavior.
Hello and thanks for submitting that.
I am not sure why you would need to do that in the core library instead of at user level. Every widget already return 'true' when the underlying value has changed, so you can follow this pattern:
bool changed1 = false;
bool changed2 = false;
..
changed1 |= ImGui::RadioButton(..);
changed1 |= ImGui::RadioButton(..);
...
changed2 |= ImGui::SliderFloat("Group 2 value...").
etc. Did you overlook that possibility, by any chance?
I basically do exactly what Omar suggests in my own in-game editor. It's not particularly onerous, either, since I just have wrappers around the various widgets to automatically set the changed state for me.
Having it handled in the core library is a small convenience, but not a huge win as far as I'm concerned.
Every widget already return 'true' when the underlying value has changed, so you can follow this pattern:
I started with exactly that usage pattern when I discovered I had an array editing widget in the middle (which accepts a "void EditItem(size_t, T&)" callback), which made this information difficult to pass across all these functions.
There are of course workarounds to that (such as using custom wrappers and objects with virtual interfaces) but none seem to be as convenient as having an interface like this.
--> bool EditItem(size_t, T&) ?
I don't find the idea uninteresting but I feel it wouldn't cope with all situations nor be as flexible as tracking it manually. In elaborate ui some actions may constitute a change, some not, at which point the user would have to keep changing masks and then the code becomes more verbose.
Similarly to the situation you have currently, your elaborate widgets could as well alter the ChangeMask etc. in a way that render them incompatible with what the calling code expect? So that communication may be expected if your editor gets really fancy.
I would like to sit on that idea, keep it in mind and see how it would improve existing code-base. In the code-bases I've seen returning the bool around made sense. Perhaps you can also look into passing that bool around, if that's not too tricky, and see if you think the idea is still worth it or not?
--> bool EditItem(size_t, T&) ?
I've also considered that but it wouldn't support passing 2 states. Though there's the option to use uint32_t, but I'd need a wrapper for every single control that I plan to use to return uint32_t or convert all the values in usage code.
In elaborate ui some actions may constitute a change, some not, at which point the user would have to keep changing masks and then the code becomes more verbose (and subtly slower) ?
I'm not sure about that, I never got as far as using all 32 slots. 2 is all I need right now. If something significantly more complex is necessary, then masks may not be a better choice, who knows. All I can say is, they helped in my use case.
if your editor gets really fancy
This one won't - it's been designed a while ago, I'm just swapping the UI code to reduce maintenance workload. :)
Perhaps you can also look into passing that bool around, if that's not too tricky, and see if you think the idea is still worth it or not?
It's not that it's tricky, I'd just need to modify my widgets in a way that breaks existing code or fork/expand their contents into the new usage code which would considerably increase complexity. This feature requires none of that, it's basically a drop-in replacement for merging return values that results in less code.
I would like to sit on that idea, keep it in mind and see how it would improve existing code-base.
Sounds good.
FYI, while I don't know what the appropriate api for this is (haven't given it much time), to support functions like IsItemEdited(), IsItemDeactivatedAfterEdit(), internals have since then been modified to call an internal MarkItemEdited() function which is not unlike the TriggerChangeCheck() function of your PR.
If you are still using it, I would suggest rebasing the PR and moving the code from your TriggerChangeCheck() function MarkItemEdited(). At least, that's the direction it would go in.