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.

Sign up for GitHub

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

Jump to bottom

NOTIFICATION_SORT_CHILDREN is being called twice on start for all Containers. #92644

Closed
WhalesState opened this issue Jun 1, 2024 · 0 comments · Fixed by #92645
Closed

NOTIFICATION_SORT_CHILDREN is being called twice on start for all Containers. #92644

WhalesState opened this issue Jun 1, 2024 · 0 comments · Fixed by #92645
Labels
bug topic:gui
Milestone
4.3

Comments

@WhalesState
Copy link
Contributor

WhalesState commented Jun 1, 2024 •
edited
Loading

Tested versions

4.2 - 4.3.

System information

win 10

Issue description

Removing this line will fix the issue.

godot/scene/gui/container.cpp

Line 181 in 705b7a0

pending_sort = false;

all editor containers still looks and behaves as expected after removing this line.

Edit: it happens because NOTIFICATION_RESIZED is called before NOTIFICATION_ENTER_TREE.

Steps to reproduce

Create a new container node, and attach a script to it.

extends Container


func _notification(what: int):
	if what == NOTIFICATION_SORT_CHILDREN:
		print("SORT CHILDREN")
	elif what == NOTIFICATION_RESIZED:
		print("RESIZED")
	elif what == NOTIFICATION_ENTER_TREE:
		print("ENTER TREE")

and run the scene, it will print SORT CHILDREN twice.

Minimal reproduction project (MRP)

N/A
@WhalesState WhalesState mentioned this issue Jun 1, 2024
Merged
@akien-mga akien-mga added this to the 4.3 milestone Jun 7, 2024
aaronp64 added a commit to aaronp64/godot that referenced this issue Jun 11, 2024
@aaronp64
Fix Container::pending_sort tracking
5a25637 
When Container::queue_sort() is called, pending_sort is set to true to indicate when a call to _sort_children() is queued, to avoid queueing multiple calls. Container::_sort_children() sets pending_sort back to false when finished, but did not do this when the container was not inside the tree.  This would allow cases where queue_sort() could be called just before removing from the tree, causing _sort_children() to never reset pending_sort, preventing any future queue_sort() calls from queueing again.

One case where this happened was with the "Saving Scene" progress bar in the editor - when saving for the first time (or the first time the progress bar popup otherwise appeared in the editor), _sort_children() would be called successfully.  After the progress bar popup was hidden, then shown again on future saves, _sort_children() would not be called again, resulting in the progress bar not taking up as much space as it should.

This issue used to be avoided by setting pending_sort to false immediately on NOTIFICATION_ENTER_TREE - however, this would allow multiple calls to be queued at the same time when entering the tree (godotengine#92644).  The multiple calls was fixed recently by removing this assignment, but this also made possible the case where pending_sort is never reset.

This change sets pending_sort back to false in _sort_children() whether or not it's in the tree.  Since this is done in a deferred call, it should still avoid the previous issue of multiple calls being queued at once on entering the tree.

Fixes godotengine#92971
@aaronp64 aaronp64 mentioned this issue Jun 11, 2024
Merged
@WhalesState WhalesState mentioned this issue Jun 11, 2024
Closed
MewPurPur pushed a commit to MewPurPur/godot that referenced this issue Jul 11, 2024
@aaronp64 @MewPurPur
Fix Container::pending_sort tracking
f9b12fa 
When Container::queue_sort() is called, pending_sort is set to true to indicate when a call to _sort_children() is queued, to avoid queueing multiple calls. Container::_sort_children() sets pending_sort back to false when finished, but did not do this when the container was not inside the tree.  This would allow cases where queue_sort() could be called just before removing from the tree, causing _sort_children() to never reset pending_sort, preventing any future queue_sort() calls from queueing again.

One case where this happened was with the "Saving Scene" progress bar in the editor - when saving for the first time (or the first time the progress bar popup otherwise appeared in the editor), _sort_children() would be called successfully.  After the progress bar popup was hidden, then shown again on future saves, _sort_children() would not be called again, resulting in the progress bar not taking up as much space as it should.

This issue used to be avoided by setting pending_sort to false immediately on NOTIFICATION_ENTER_TREE - however, this would allow multiple calls to be queued at the same time when entering the tree (godotengine#92644).  The multiple calls was fixed recently by removing this assignment, but this also made possible the case where pending_sort is never reset.

This change sets pending_sort back to false in _sort_children() whether or not it's in the tree.  Since this is done in a deferred call, it should still avoid the previous issue of multiple calls being queued at once on entering the tree.

Fixes godotengine#92971
sorascode pushed a commit to sorascode/godot-soras-version that referenced this issue Jul 22, 2024
@aaronp64 @sorascode
Fix Container::pending_sort tracking
1ff99a7 
When Container::queue_sort() is called, pending_sort is set to true to indicate when a call to _sort_children() is queued, to avoid queueing multiple calls. Container::_sort_children() sets pending_sort back to false when finished, but did not do this when the container was not inside the tree.  This would allow cases where queue_sort() could be called just before removing from the tree, causing _sort_children() to never reset pending_sort, preventing any future queue_sort() calls from queueing again.

One case where this happened was with the "Saving Scene" progress bar in the editor - when saving for the first time (or the first time the progress bar popup otherwise appeared in the editor), _sort_children() would be called successfully.  After the progress bar popup was hidden, then shown again on future saves, _sort_children() would not be called again, resulting in the progress bar not taking up as much space as it should.

This issue used to be avoided by setting pending_sort to false immediately on NOTIFICATION_ENTER_TREE - however, this would allow multiple calls to be queued at the same time when entering the tree (godotengine#92644).  The multiple calls was fixed recently by removing this assignment, but this also made possible the case where pending_sort is never reset.

This change sets pending_sort back to false in _sort_children() whether or not it's in the tree.  Since this is done in a deferred call, it should still avoid the previous issue of multiple calls being queued at once on entering the tree.

Fixes godotengine#92971
2nafish117 pushed a commit to 2nafish117/godot that referenced this issue Aug 5, 2024
@aaronp64 @2nafish117
Fix Container::pending_sort tracking
242e793 
When Container::queue_sort() is called, pending_sort is set to true to indicate when a call to _sort_children() is queued, to avoid queueing multiple calls. Container::_sort_children() sets pending_sort back to false when finished, but did not do this when the container was not inside the tree.  This would allow cases where queue_sort() could be called just before removing from the tree, causing _sort_children() to never reset pending_sort, preventing any future queue_sort() calls from queueing again.

One case where this happened was with the "Saving Scene" progress bar in the editor - when saving for the first time (or the first time the progress bar popup otherwise appeared in the editor), _sort_children() would be called successfully.  After the progress bar popup was hidden, then shown again on future saves, _sort_children() would not be called again, resulting in the progress bar not taking up as much space as it should.

This issue used to be avoided by setting pending_sort to false immediately on NOTIFICATION_ENTER_TREE - however, this would allow multiple calls to be queued at the same time when entering the tree (godotengine#92644).  The multiple calls was fixed recently by removing this assignment, but this also made possible the case where pending_sort is never reset.

This change sets pending_sort back to false in _sort_children() whether or not it's in the tree.  Since this is done in a deferred call, it should still avoid the previous issue of multiple calls being queued at once on entering the tree.

Fixes godotengine#92971
chryan pushed a commit to chryan/godot that referenced this issue Aug 6, 2024
@aaronp64 @chryan
Fix Container::pending_sort tracking
08ad793 
When Container::queue_sort() is called, pending_sort is set to true to indicate when a call to _sort_children() is queued, to avoid queueing multiple calls. Container::_sort_children() sets pending_sort back to false when finished, but did not do this when the container was not inside the tree.  This would allow cases where queue_sort() could be called just before removing from the tree, causing _sort_children() to never reset pending_sort, preventing any future queue_sort() calls from queueing again.

One case where this happened was with the "Saving Scene" progress bar in the editor - when saving for the first time (or the first time the progress bar popup otherwise appeared in the editor), _sort_children() would be called successfully.  After the progress bar popup was hidden, then shown again on future saves, _sort_children() would not be called again, resulting in the progress bar not taking up as much space as it should.

This issue used to be avoided by setting pending_sort to false immediately on NOTIFICATION_ENTER_TREE - however, this would allow multiple calls to be queued at the same time when entering the tree (godotengine#92644).  The multiple calls was fixed recently by removing this assignment, but this also made possible the case where pending_sort is never reset.

This change sets pending_sort back to false in _sort_children() whether or not it's in the tree.  Since this is done in a deferred call, it should still avoid the previous issue of multiple calls being queued at once on entering the tree.

Fixes godotengine#92971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug topic:gui
Projects
None yet
Milestone
4.3
Development

Successfully merging a pull request may close this issue.

Fix NOTIFICATION_SORT_CHILDREN is called twice on startup mounirtohami/pixel-engine
3 participants
@akien-mga @WhalesState @AThousandShips