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

Fix theme access and improve UX in AnimationTree editor #82210

Merged

Conversation

YuriSizov
Copy link
Contributor

Fixes #81966. Fixes #82014.

This PR fixes an assortment of issues related to the AnimationTree editor and its subeditors. First of all, it fixes the aforementioned bugs (one of which is tangentially related, but can be reproduced in the same workflow, so I grabbed it along the way).

Theming issues

To properly fix the issue with the theme access I decided to do a proper cleanup and organize all the styles used by the state machine editor into their own type in the editor theme. As well as implement proper caching and bind used theme properties, for the first time in the editor codebase! At the core of the regression was the fact that animation editors used borrowed styles from GraphEdit and GraphNode, which were recently remade. And thus this fragile reliance was severed.

Having a proper pre-configured type helps to avoid such issues in the future, even if internally we still borrow styles. It also allows to clearly define what the state machine graph needs to be rendered. For example, the GraphNode rework removed one of the styles that only the state machine graph used, because that wasn't obvious. Also GraphNode in the editor had definitions which aren't related to that node at all, and that's not great.

Before

godot windows editor dev x86_64_2023-09-23_21-36-22

After

godot windows editor dev x86_64_2023-09-23_21-19-58

I also enabled antialiasing on the transition lines, it looks better and I don't think it would be too taxing.

More issues

Along the way I tried to improve some related code. This was mostly to make it cleaner and clearer, but there were also mistakes. Namely, we were sorting an empty array of possible class names, and only then populated it with data. That's probably not what the author has intended. Fixed now, but that means that classes in the context menu are now actually ordered.

In that same context menu we weren't properly handling the case when there was no animation player connect, or the animation player had no animations. I reworked the code to disable the submenu in this case:

godot windows editor dev x86_64_2023-09-23_21-33-46 godot windows editor dev x86_64_2023-09-23_21-20-12

I used the opportunity to also add tooltips to buttons rendered in the state machine graph. All the scaffolding for this was already in place, I just cleaned it up and utilized.

godot windows editor dev x86_64_2023-09-23_21-20-26

And then there was an error spam issue when selecting the tree root of an animation tree. It happened if you already have a BlendSpace1D or BlendSpace2D class selected, and then clicked on the resource picker's popup button. The issue here was with the check in getters of those classes. We reported errors when accessing blend points that haven't been formally added yet. But those points did exist as valid properties, which we would try to access as a part of some resource picker logic. So those getters would trigger for each one of them and print a bunch of errors.

2023-09-23_21-34-08

I changed the index check from the number of used points to the maximum number of points. All unused points should still be initialized and return proper default values, and errors don't seem to be useful here.

@akien-mga akien-mga merged commit 37d5e1e into godotengine:master Sep 24, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Broken icon in SceneTreeDialog Editor: AnimationTree node font color is black
3 participants