Fix Container::pending_sort tracking

[aaronp64]

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 (#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 #92971

[akien-mga]

CC @WhalesState

[akien-mga]

Thanks!

[WhalesState]

I believe this is the correct fix instead of reverting the changes. anyway, this was able to fix the saving scene issue without forcing all the Containers in the project to call sort_children twice when the project starts.

[aaronp64]

Just want to clarify, this change does not revert #92645 - I just mentioned the previous issue to call out that we don't want to undo the fix for it. The difference from before the previous PR is that now pending_sort is set in _sort_children instead of _notification - since it's being done in the deferred call, multiple calls will not be queued up like they were before. I tested with the code sample provided in #92644, and the "SORT CHILDREN" message is only displayed once. If there are other cases that are behaving differently, we may still need to look into those.

The pending_sort issue isn't specific to ProgressDialog - it can be reproduced in gdscript if queue_sort() is called when removing/re-adding a Container, so I think we need to have a solution in Container for this.

[WhalesState]

I was really confused, thought that my changes was reverted.
So the issue only appears when removing then adding the same Container in a single frame, and what was making sort_children runs twice is because NOTIFICATION_THEME_CHANGED and NOTIFICATION_RESIZED are called before NOTIFICATION_ENTER_TREE, maybe we can return from NOTIFICATION_THEME_CHANGED and NOTIFICATION_RESIZED when the Container is not inside tree, and we revert all the changes. Let me see if this will fix both issues.

[WhalesState]

This didn't work, already _sort_children() will return when the node is outside the tree, so those notifications runs in this order when the node enters the tree.

VISIBILITY CHANGED
THEME CHANGED
ENTER TREE
SORT CHILDREN -> called by VISIBILITY CHANGED
SORT CHILDREN -> called by ENTER TREE

Maybe adding NOTIFICATION_PRE_ENTER_TREE can fix the double call issue, or we document that users will need to use container.call_deferred("notification", Container.NOTIFICATION_SORT_CHILDREN) to fix the resizing issue, which will rarely happen.