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

Call add_child after set_rect to fix size bug #80968

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

rakkarage
Copy link
Contributor

closes: #80234

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

There is some odd preference for adding a child first and then initializing its properties (I tend to use it myself .-.), but recently I noticed that it affects sizing. I think we should re-evaluate this "convention". Even if the sizing problems are a bug, initializing stuff after adding it to tree is kinda backwards tbh (and in some cases probably less efficient).

@YuriSizov
Copy link
Contributor

Even if the sizing problems are a bug, initializing stuff after adding it to tree is kinda backwards tbh

Well, it is possible that the context of the parent would affect how some things are resolved when it comes to sizing. Probably not with containers, but with anchors when setting a preset it may work differently, I think.

In this case, btw, I'm not even sure it makes sense to have a button control. We could just draw an icon and detect clicks over specific areas with a bit of lower-level code. That should avoid messing with control sizing all together. But that's not important for this PR.

@YuriSizov
Copy link
Contributor

Could you please amend your commit message to follow the project's preferred style? The title of the PR is a good option.

@rakkarage rakkarage force-pushed the alternative-icon-scale branch from 79e80d8 to e93d2a0 Compare August 25, 2023 14:11
@YuriSizov YuriSizov merged commit 6da4ad1 into godotengine:master Aug 25, 2023
@YuriSizov
Copy link
Contributor

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.

Alternative Tile 'Plus' Icon incorrectly sized for small tile sizes
4 participants