Don't remove the owner immediately

[KoBeWi]

Closes #22840

The question is whether setting owner to null immediately in NOTIFICATION_PREDELETE was done for a reason or the order doesn't matter (but some code might still rely on the old order). I did a quick test and didn't notice any unusual behavior.

Also here's a test code:

func _ready():
	var se1f = self
	$Node2D.connect("tree_exiting", Callable(se1f, "exiting"), [$Node2D])
	$Node2D.connect("tree_exited", Callable(se1f, "exited"), [$Node2D])
	$Node2D.free()

func exiting(node):
	print(node.owner) #prints node; before it was null

func exited(node):
	print(node.owner) #prints null

[akien-mga]

Are you sure about this? From a quick look at remove_child() and the follow-up call to _propagate_validate_owner(), it doesn't seem strictly equivalent to the while loop above removing all owners.

[KoBeWi]

The node loses its parent, _propagate_validate_owner will search for current owner in node's ancestors and will remove the owner if it's not found (which will happen, because the parent was removed).

Although this probably needs to be tested with instanced scenes, because they have different owner...

[KoBeWi]

Ok, instanced scenes are broken. Here's a test case (replace code in OP):

$Node2D/Node.connect("tree_exiting", Callable(self, StringName("exiting")), [$Node2D/Node])
$Node2D/Node.connect("tree_exited", Callable(self, StringName("exited")), [$Node2D/Node])
$Node2D.free()

Node2D is an instanced scene with a Node child. Seems like Node will never lose its owner (prints the node in both cases, while previously it was both nulls).

Not sure how to fix this other than by forcing the removal of owner inside remove_child (which would require some hacks probably).

[mhilbrunner]

PR meeting: reduz says the original code is indeed fishy, but we need to research/review and make sure to test it, also maybe @RandomShaper could have a look

[RandomShaper]

I checked this trying to suggest some changes, but in the end I've needed to check the issue deeply and that has lead to a very different approach I've opened a PR for: #55512.

[akien-mga]

Superseded by #55512.