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 Container pending_sort and "Saving Scene" ProgressDialog size. #93030

Closed

Conversation

WhalesState
Copy link
Contributor

@WhalesState WhalesState commented Jun 11, 2024

Fixes: #92971

Since both issues are closed, and each one produces the other, it's better to fix both at the same time.

If this ever leads to any other issues that can't be fixed by deferring a sort children notification call, please tag me.

@WhalesState WhalesState requested a review from a team as a code owner June 11, 2024 12:20
@akien-mga
Copy link
Member

I'm confused, we already merged fixes for these issues. Your branch seems to be based off an earlier version of the master branch, so you don't have #93008. I'm not sure why it doesn't raise a merge conflict.

@akien-mga akien-mga added this to the 4.3 milestone Jun 11, 2024
@WhalesState
Copy link
Contributor Author

WhalesState commented Jun 11, 2024

I have restored the branch after deletion, then i pressed update branch because it was already merged before so there was no commits ahead master, and when i opened the branch i didn't find the changes was reverted, so i just pressed undo last commit and i force pushed the changes. I also was confused because the last commit wasn't my fix, there was like 60 other commits.

@WhalesState

This comment was marked as outdated.

@WhalesState WhalesState marked this pull request as draft June 11, 2024 13:09
@WhalesState

This comment was marked as outdated.

@WhalesState WhalesState marked this pull request as ready for review June 11, 2024 14:35
@WhalesState
Copy link
Contributor Author

WhalesState commented Jun 11, 2024

I can't reproduce #92971, seems like it only happens with ProgressDialog.
We will need a MRP if it happened again after merging this pull request.

I have tried the same code in _ready(), _enter_tree() and _init() and it only calls sort children once every time i add the VBoxContainer -> Container2 as a child.

image

@WhalesState
Copy link
Contributor Author

Sorry about the confusion, I just thought my fix was reverted because i used to see that happens when a pull request causes regression.
This also fixes the "Save Scene" dialog issue because i think the issue is not related to the Container class.

@akien-mga
Copy link
Member

To clarify, #92971 was already fixed by #93008.

What bug is this PR aiming to solve?

@WhalesState
Copy link
Contributor Author

Nothing, I reproduced the issue with this script to understand, Sorry -_-

extends Control


func _ready():
	var container = Container2.new()
	container.set_anchors_and_offsets_preset(PRESET_FULL_RECT)
	var popup := PopupPanel.new()
	popup.exclusive = true
	add_child(popup)
	popup.add_child(container)
	popup.size = Vector2(340, 256)
	popup.popup_centered()
	popup.hide()
	popup.remove_child(container)
	await get_tree().create_timer(1).timeout
	popup.add_child(container)
	popup.popup_centered()


class Container2:
	extends VBoxContainer
	
	
	func _init():
		for i in range(5):
			var button = Button.new()
			button.text = "Button %s" % i
			add_child(button)

@KoBeWi KoBeWi removed this from the 4.3 milestone Jun 11, 2024
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.

Save progress popup's content is too small
3 participants