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 method dialog label #75844

Merged
merged 1 commit into from
Apr 10, 2023
Merged

Fix method dialog label #75844

merged 1 commit into from
Apr 10, 2023

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Apr 9, 2023

In the method pick dialog, the label that appears when no methods are found could be at wrong position (probably due to being child of Tree).
image
Also if you selected an empty script, the label did not appear due to empty root item:
image
With this PR the label is centered in the dialog and the "Attached Script" item only appears if there are any script methods available to pick.
image

@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 10, 2023

This is probably rooted in something unrelated, but because the label is directly parented to a Window node, it doesn't always work. If you open a dialog where it is supposed to be immediately visible, you can see it. But if you open a dialog where there are valid methods, and then filter it, it does not appear. This is repeatable, and you can try it with either case in any order, and it will still happen.

It is also possible to "cure" the editor and make the label appear correctly until you restart, if you close the dialog in a state where the label is supposed to be visible. Like, if you filter the dialog so it is supposed to appear (and it doesn't), then close it and open it again, the label will start appearing correctly after that.

Edit: And it breaks again if you try to connect a signal that has no matching methods.


Seems to be some desync issue with the visibility propagation when we try to immediately hide it, I think. Still looking into it, but just wanted to leave a comment so we don't merge this in the meantime and get unexpected reports.

@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 10, 2023

I think this is related to the fact that we never update parent's visibility for windows when the node itself is hidden. Because this is explicitly disabled in CanvasItem::_window_visibility_changed(). I'll try removing the check, it should be safe.

Edit: Done #75890.

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the issue with the window, this seems to work as expected and I don't see any issues with the layout or input propagation from parenting the label directly to the window.

@YuriSizov YuriSizov merged commit 8d88715 into godotengine:master Apr 10, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@KoBeWi KoBeWi deleted the shy_label branch April 10, 2023 14:51
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.3.

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.

2 participants