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

Deleting GraphNodes when handling delete_nodes_request causes connection visuals to leak #36716

Closed
Zylann opened this issue Mar 1, 2020 · 2 comments · Fixed by #86158
Closed
Assignees
Milestone

Comments

@Zylann
Copy link
Contributor

Zylann commented Mar 1, 2020

Godot 3.2
Windows 10 64 bits

I followed the way Godot's graph editors implement the delete_nodes_request:

func _on_GraphEdit_delete_nodes_request():
	var to_delete = []
	
	for i in _graph_edit.get_child_count():
		var node = _graph_edit.get_child(i)
		if node is GraphNode:
			if node.selected:
				to_delete.append(node)
	
	for node in to_delete:
		node.free() # queue_free also works

It properly removes the node, however it causes connections to remain visible, until the graph gets panned around or redrawn (which will then make them disappear):

image

When redrawing, an error is logged once:

ERROR: Node not found: B.
   At: scene/main/node.cpp:1381

Note: I think this never happens in Godot's graph-based editors, because those actually rebuild the entire graph each time a node is deleted... which is very far from the way I implement mine.
This happens even if I use queue_free().

Test project:
GraphEditDeleteRequest.zip

If it's required to remove those connections manually, it would be really nice to have a function to do so.
Edit: indeed, it must be done manually.

void remove_connections_from_and_to(GraphEdit *graph_edit, StringName node_name) {
	// Get copy of connection list
	List<GraphEdit::Connection> connections;
	graph_edit->get_connection_list(&connections);

	for (List<GraphEdit::Connection>::Element *E = connections.front(); E; E = E->next()) {
		const GraphEdit::Connection &con = E->get();
		if (con.from == node_name || con.to == node_name) {
			graph_edit->disconnect_node(con.from, con.from_port, con.to, con.to_port);
		}
	}
}
@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 14, 2023

Still reproduceable in 4.2 beta 6. cc @Geometror

Here's an updated project: graph-connections-break.zip

@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 14, 2023
@Geometror Geometror self-assigned this Jan 2, 2024
@Geometror
Copy link
Member

Geometror commented Jan 5, 2024

This is fixed in #86158.
The following works until then, but it's hacky and shouldn't be used:

for node in to_delete:
	_graph_edit.remove_child(node)
	node.queue_free()
_graph_edit.get_node("_connection_layer").queue_redraw()

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

Successfully merging a pull request may close this issue.

5 participants