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

EditorInterface.get_edited_scene_root().queue_free() will crash the editor. #90176

Closed
ryevdokimov opened this issue Apr 3, 2024 · 9 comments
Closed

Comments

@ryevdokimov
Copy link
Contributor

ryevdokimov commented Apr 3, 2024

Tested versions

v4.3.dev5.mono.official [89f70e9]

System information

Godot v4.3.dev5.mono - Windows 10.0.22631 - Vulkan (Forward+) - integrated AMD Radeon(TM) Graphics (Advanced Micro Devices, Inc.; 31.0.24002.92) - AMD Ryzen 7 PRO 6850U with Radeon Graphics (16 Threads)

Issue description

Might be trying something dumb here, but trying to delete the root node via the title method to go back to this in the editor:

image

Get a crash instead.

ERROR: Cannot get path of node as it is not in a scene tree.
at: (scene/main/node.cpp:2188)
ERROR: Condition "!is_inside_tree()" is true. Returning: false
at: can_process (scene/main/node.cpp:791)

Steps to reproduce

File->Run

extends EditorScript

func _run() -> void:
	EditorInterface.get_edited_scene_root().queue_free()

Minimal reproduction project (MRP)

N/A

@AThousandShips
Copy link
Member

AThousandShips commented Apr 3, 2024

The best we could do here I'd say is prevent it being freed, otherwise this is a case in my opinion if destructive use of the engine, I'd say you should expect issues if you try to delete stuff that the editor handles

We could theoretically add some detection if scene is deleted and close the tab, but that'd be assuming malicious use, and how would we handle saving, connections, etc.

@ryevdokimov
Copy link
Contributor Author

Maybe.

Deleting the root node isn't outside normal usage of the engine.

image

Doing this will work and take you to the screenshot in the issue. I'm basically looking for a reverse of this (if/when it gets implemented): #90136, to build out an undo/redo system for plugins.

@AThousandShips
Copy link
Member

Deleting the root node isn't outside normal usage of the engine.

Deleting the root node not through the editor isn't though, they're fundamentally different

@ryevdokimov
Copy link
Contributor Author

Well yeah, it crashes the editor lol. I'll agree with that point. It would be nice if I could delete the root node via scripts, but at a minimum, sure it shouldn't crash.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 3, 2024

Related: #26409

@AThousandShips
Copy link
Member

So what would the solution be? To just complain and close the tab without saving I assume? There's no way to recover the node when deleted

@ryevdokimov
Copy link
Contributor Author

I don't see why not. You also don't have a way to recover nodes you delete when you use remove_child(), nor does it prompt you either. At least not without programming in that functionality.

@AThousandShips
Copy link
Member

Then the solution is to just recover gracefully, as opposed to crashing, and not trying to fix anything, that's a good solution, but people might not like it (also remove_child doesn't delete anything, so not really comparable)

@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Apr 3, 2024

Fair point, I suppose queue_free() on children is comparable and the result looks the same.

I think this comment in the issue referenced above puts it pretty aptly: #26409 (comment)

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

No branches or pull requests

3 participants