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

Really need Node destroy callback function, just like "OnDestroy" in Unity #707

Closed
D3ZAX opened this issue Apr 14, 2020 · 5 comments
Closed
Labels

Comments

@D3ZAX
Copy link

D3ZAX commented Apr 14, 2020

Describe the project you are working on:
Just test the engine for new game option

Describe the problem or limitation you are having in your project:
Can't call node clear function conveniently when consided the node can change it's parent. Must call every children's clear function for every class Inherited from Node or subclass of Node, since the condition that which child node will change its parent is not predictable.

If put the clear option in _exit_tree, then it will be triggered when one node change its parent, if there is some operation that can't fallback, such as destroy some node, then there is a problem.

And also, when serval node want to change parent at the same time with some opration that may destroy some of them or their children, it's really not so easy to judge whether one node is really destroyed or it's just removed from the tree.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Can put clear option in destroy callback function that just want to be triggered when the node is really destroyed such as destroy some related node.
And can write log to check whether the node is really destroyed.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Add a virtual function "_destroy" just like function "_ready", when one node to destroy, from every children node of it call it and then itself.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Not so easy.

Is there a reason why this should be core and not an add-on in the asset library?:
If this feather implemented in core, it will be so convient to manager node when many node can change there parent without data copy to keep state.
It's also one convient way to check node memory leak.

@clayjohn
Copy link
Member

I think what you are looking for is the tree_exiting signal see here for more details.

@hilfazer
Copy link

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Not so easy.

Hmm, i don't know:

func _notification(what):
	if what == NOTIFICATION_PREDELETE:
		# destroy stuff
		pass

@Jummit
Copy link

Jummit commented Apr 14, 2020

If you want to track creation / destruction of all nodes:

func _ready():
	get_tree().connect("node_added", self, "_on_Tree_node_added")
	get_tree().connect("node_removed", self, "_on_Tree_node_removed")

func _on_Tree_node_added(node):
	pass

func _on_Tree_node_removed(node):
	pass

If you only want to track destruction of the node the script is attached to:

func _notification(what):
	if what == NOTIFICATION_PREDELETE:
		pass

@willnationsdev
Copy link
Contributor

willnationsdev commented Apr 14, 2020

_exit_tree() is technically a different context than a destructor though. If the node has runtime-generated objects that it has instantiated within itself, they may need to be handled separately from just a tree exit scenario.

  • non-Node, non-Reference types (e.g. any custom Object type or some other class): you will have to manually perform cleanup of these types.
  • Nodes which are not presently in the node tree: if you have a parent node that micromanages swapping around various node trees for its children (for a state machine of some sort), then you need to manually clean up all internal node trees and not just the ones that are attached.

I can say, in my own experience, that I've wished there was a _predelete() callback available for objects, and if I understand correctly, I believe it would be somewhat trivial to implement in the engine, right? You just go to the object's predelete notification and add a has_method() and call() pair to execute it if it exists.

Technically speaking, if all you wanna do is react to the death of an object, but in order to do that, you have to trigger script API callback for every notification that could possibly happen, that seems like overkill to me. With that said, I consider it more of a quality of life / usability improvement rather than some great need for performance or anything. There's already a _init() constructor callback for any Object, but there's no corresponding dedicated callback for a destructor. It just feels weird.

Edit: Oh, and just to confirm, my point has almost nothing to do with OP's issue since what has been said already will already handle his issue I think. XD

@D3ZAX
Copy link
Author

D3ZAX commented Apr 15, 2020

Thank you guys! The function _notification works fine!

Maybe it should be add to beginners tutorial in document, for it's really eazy to ignore the parent class's function for Node for special callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants