-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Bind remaining theme properties to their respective classes #81551
Bind remaining theme properties to their respective classes #81551
Conversation
22722a9
to
f40364a
Compare
Added some fixes for There was also a weird bit of code that I ignored before, that fetched theme properties of So I reworked it to bind properties on the correct class, and then fetch the values using virtual methods that the extending class implements. Should be safe and properly separated now. |
This adds binds for GraphEdit/GraphElement/GraphNode, which were skipped before due to a rework. This also adds binds for Window, which was skipped before due to a complicated code organization. Also adds theme cache entries/direct cache access to a few places that previously missed it. Some theme properties are now exposed to other classes via friendships or public getters for convenience. This removes all string-based theme access from scene/ classes.
f40364a
to
fe00027
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primarily reviewed the GraphEdit
related stuff. The changes beyond the theme caching make sense to me. Nice work!
Thanks for reviews! |
Follow-up to #81312 and #79311 et al.
This PR adds binds for
GraphEdit
/GraphElement
/GraphNode
, which were skipped before due to a rework. Mostly this is straight-forward implementation of the theme cache and the new binds. But I changed one thing. I noticed thatGraphEdit
uses theport
icon of theGraphNode
type, but it doesn't fetch it through specific graph nodes, instead it only reads it from the theme. This means that if individual graph nodes have overrides or themes applied directly to them, we don't account for that. That is despite all related code operating in a context of specific graph nodes. So I adjusted these parts to use theport
icon relevant to each node. (cc @Geometror).This also adds binds for
Window
, which was skipped before due to a complicated code organization. I've used the power of friendship 🌈🙌 to access the cache directly fromViewport
, which should do it for now. I still want to move embedded window drawing code back intoWindow
at some point.Finally, this adds theme cache entries/direct cache access to a few places that previously missed it. Some theme properties are now exposed to other classes via friendships or public getters for convenience. (cc @KoBeWi for
ColorPicker
/ColorMode
related changes).This removes all string-based theme access from
scene/
classes. Everything is cached and statically typed now.