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

Rationalize Node removals and deletions #55512

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Dec 1, 2021

  • Regarding preservation of owner, free() and remove_child() now behave the same: owner is kept during tree_exiting signal and null during tree_exited.
  • tree_exited is emitted in the same order as tree_exiting.
  • Superfluous code to handle cancelation of ownership is removed.

Tested with the following composite scene (full project, where exact ownerships can be checked: Minimal.zip:

image

Main's code:

extends Node2D

func _ready():
	var se1f = self
	$A.connect(\"tree_exiting\", Callable(se1f, \"exiting\"), [$A])
	$A.connect(\"tree_exited\", Callable(se1f, \"exited\"), [$A])
	$A/Node.connect(\"tree_exiting\", Callable(se1f, \"exiting\"), [$A/Node])
	$A/Node.connect(\"tree_exited\", Callable(se1f, \"exited\"), [$A/Node])
	$A/Node/B.connect(\"tree_exiting\", Callable(se1f, \"exiting\"), [$A/Node/B])
	$A/Node/B.connect(\"tree_exited\", Callable(se1f, \"exited\"), [$A/Node/B])
	$A/Node/B/B2.connect(\"tree_exiting\", Callable(se1f, \"exiting\"), [$A/Node/B/B2])
	$A/Node/B/B2.connect(\"tree_exited\", Callable(se1f, \"exited\"), [$A/Node/B/B2])
	$A/Node/B/C.connect(\"tree_exiting\", Callable(se1f, \"exiting\"), [$A/Node/B/C])
	$A/Node/B/C.connect(\"tree_exited\", Callable(se1f, \"exited\"), [$A/Node/B/C])
	# Uncomment either, never both
	#remove_child($A)
	$A.free()

func exiting(node):
	prints(\"exiting\", node.name, node.owner) #prints node; before it was null

func exited(node):
	prints(\"exited\", node.name, node.owner) #prints null

Results with current code (before this PR)

With $A.free():

exiting C Node2D:[Node2D:29192356323]
exiting B2 B:[Node2D:29242687969]
exiting B Node2D:[Node2D:29192356323]
exiting Node [Object:null]
exiting A [Object:null]
exited B2 B:[Node2D:29242687969]
exited C [Object:null]
exited B [Object:null]
exited Node [Object:null]
exited A [Object:null]

With remove_child($A):

exiting C Node2D:[Node2D:29192356324]
exiting B2 B:[Node2D:29242687969]
exiting B Node2D:[Node2D:29192356324]
exiting Node A:[Node2D:29209133539]
exiting A Node2D:[Node2D:29192356324]
exited B2 B:[Node2D:29242687969]
exited C [Object:null]
exited B [Object:null]
exited Node A:[Node2D:29209133539]
exited A [Object:null]

Results with this PR

With $A.free() or remove_child($A) (same results in both cases):

# tree_exiting preserves owners
exiting C Node2D:[Node2D:29192356600]
exiting B2 B:[Node2D:29242688246]
exiting B Node2D:[Node2D:29192356600]
exiting Node A:[Node2D:29209133817]
exiting A Node2D:[Node2D:29192356600]

# - Owners are already cleared by tree_exited
# - tree_exited comes in tree reverse order, like tree_exiting
exited C [Object:null]
exited B2 [Object:null]
exited B [Object:null]
exited Node [Object:null]
exited A [Object:null]

Supersedes #39835.
Fixes #22840.

NOTE: A non compat breaking subset of this PR for 3.x has been submitted separately: #55514.

- Regarding preservation of owner, `free()` and `remove_child()` now behave the same: owner is kept during `tree_exiting` signal and `null` during `tree_exited`.
- `tree_exited` is emitted in the same order as `tree_exiting`.
- Superfluous code to handle cancelation of ownership is removed.
@akien-mga akien-mga merged commit 91b5c35 into godotengine:master Jan 5, 2022
@akien-mga
Copy link
Member

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.

Owner variable is null on tree_exiting signal
2 participants