Avoid popping up dialogs excessively in the Animation editor #84208
Conversation
There was a problem hiding this comment.
Structurally, the code seems sensible, but it seems to have introduced a regression where it crashes if you press one of the arrows to modify the blend times of any of the animations. This seems to be due to the Tree being blocked in tree::_range_click_timeout and functions not properly checking if it's safe to update or not.
YuriSizov
force-pushed
the
editor-animation-that-pops
branch
from
4d7922e
to
6035588
Compare
|
Oops, silly mistake, checks need to be in both new methods. Though I have to say, the fact that changing the times goes into the method that could potentially rebuild the tree (or popup the window before) is problematic. Maybe some refactoring is in order. |
|
It's not a regression in this PR so I'm okay with approving this, but you're right about the way its structured in regards to it rebuilding the tree through every spinbox update is bad and probably does need refactoring. If for no other reason than the fact that the result of how this is structured means spinboxes don't work how they work in other parts of the app (usually you can click and hold a spinbox and move the mouse up and down to alter them). |
|
Thanks! |
Quick refactoring to fix #83167. Some other codestyle changes as well, such as initializing primitive types in the header, removing unneeded ClassDB binds, and moving some commands around.
Whether we should at all allow to pop windows which are already visible I don't know. It was also possible in Godot 3. It should not lead to issues anyway, I don't think. I, for one, cannot reproduce the particular issue from the clip in the linked report. But there may be some timing issues, I can imagine.