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

Undo redo Leak #34

Closed
qarmin opened this issue Dec 12, 2019 · 5 comments
Closed

Undo redo Leak #34

qarmin opened this issue Dec 12, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@qarmin
Copy link

qarmin commented Dec 12, 2019

Undo Redo needs to be freed to prevent leak

WARNING: cleanup: ObjectDB Instances still exist!
   At: core/object.cpp:2069.
Leaked instance: UndoRedo:2088
Orphan StringName: UndoRedo
StringName: 1 unclaimed string names at exit.

To prevent leak it is enough to add at the Global.gd class in _exit_tree undo_redo.free():

diff --git a/Scripts/Global.gd b/Scripts/Global.gd
index 1d0f7fb..041ac7b 100644
--- a/Scripts/Global.gd
+++ b/Scripts/Global.gd
@@ -457,3 +457,6 @@ func blend_image_with_color(image : Image, color : Color, interpolate_factor : f
                        else: #If color is transparent - if it's the eraser
                                blended_image.set_pixel(xx, yy, Color(0, 0, 0, 0))
        return blended_image
+
+func _exit_tree():
+       undo_redo.free()
@OverloadedOrama
Copy link
Member

Hello there. I was expecting objects would be freed automatically on exit_tree(). Or is it just an UndoRedo thing? Also, would there be any difference if I added it in the Main.gd script instead of Global.gd?

@qarmin
Copy link
Author

qarmin commented Dec 12, 2019

All nodes and objects needs to be freed at the end of life.
Only references(child of Reference node like ImageTexture or BoxShape) are counted and are deleted when no one use it.
Nodes should be deleted with queue_free() and objects with free().
You can check which objects and nodes leaks when you opening godot with verbose flag in terminal godot -v or in monitors inside Godot editor(this only show leaked nodes)

Description of exit_tree function from docs - https://docs.godotengine.org/en/latest/classes/class_node.html#class-node-method-exit-tree

Called when the node is about to leave the SceneTree (e.g. upon freeing, scene changing, or after calling remove_child in a script). If the node has children, its _exit_tree callback will be called last, after all its children have left the tree.

That means, that node is out of scene, but it can be deleted or in any moment node can back to scene

@qarmin
Copy link
Author

qarmin commented Dec 12, 2019

Also when I paint something then also Image node is leaking

WARNING: cleanup: ObjectDB Instances still exist!
   At: core/object.cpp:2069.
Leaked instance: UndoRedo:2088
Leaked instance: Image:2135 - Resource name:  Path: 
Orphan StringName: UndoRedo
Orphan StringName: Image
StringName: 2 unclaimed string names at exit.

To get more precise info about node, you can set unique name to all node(I will create issue in Godot repository to automate this)
e.g.

var image : Image.new()
image.name = "Image-Tanks.gd"

@OverloadedOrama
Copy link
Member

I implemented your fix for the UndoRedo leak in a4b7fe2. I'm not sure how to handle the Image node leaking though. I tried running godot -v in terminal, and run the project, but I didn't get any "Leaked instance" messages. I'm using Godot 3.1.2

@qarmin
Copy link
Author

qarmin commented Dec 13, 2019

In Pixelorama master branch I can't reproduce leak in Godot 3.1.2 and Godot 3.2 beta 3, so probably the latest commit fixes also this leak.

@qarmin qarmin closed this as completed Dec 13, 2019
@OverloadedOrama OverloadedOrama added the bug Something isn't working label Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants