-
-
Notifications
You must be signed in to change notification settings - Fork 227
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
Fixes issues where node preview does not toggle correctly #484
Conversation
- Fixed node previews not being hidden on mouse-over after loading a graph with node previews already enabled. - Fixed an issue where a node's preview didn't toggle visibility if another UI element was covering the node when the cursor entered/exited. - Fixed an issue where an incorrect preview was shown if the mouse exited the node before the preview was ready.
This looks pretty good, but I really don't like the gradient editor part (looks too specific, this hints it might break if value edit controls are modified). |
BTW, do you have this problem that when you create a new gradient cursor, it is shown in front of the preview? |
Yeah, ideally I was hoping Godot would have a signal indicating that a new node was added to the scene tree. Then I could just check for any new children down the tree in generic.gd rather than needing to manually put checks in the control script(s). Not sure of a clean way to future proof it without something generic like that though.
I didn't notice that, no. Thanks for fixing it. I must have always created the cursors before the preview in my testing. |
Actually it looks like the scene tree does emit a signal when nodes are added, I'll see if I can use that here to make it connect new children in a clean generic way. |
I don't think it's a good idea, the scene tree emits a signal whenever a node is added, I don't think we want all nodes to listen to that signal. But maybe I'm wrong... |
Yeah I'm not knowledgeable enough about the performance overhead of signals, so if that's a concern I can just leave it as it is. |
It looks like the signals we really want, Edit: Except that is only for direct children, so maybe still not applicable. |
Hmm I tried to implement it, and it seems to behave OK. Profiling did not reveal a problem with this. I'm surprise, I was expecting it to be worse (though it only happens when those preview are shown...). |
Yeah in the thread I posted above they talk about |
Fixes #481, #482, and #483
I've tested this on various nodes, trying to make sure it would work cleanly with any type of parameter. This is my first contribution, so apologies if there are any conflicts with design or code style.