Properly show detach script button when script is added via inspector #108122

Kazuo256 · 2025-06-29T19:19:33Z

The SceneTreeDock was not tracking script changes in selected nodes in any capacity as far as I could assess. To do
that, my solution essentially connects the "script_changed" signal from selected nodes to
"SceneTreeDock::_update_script_button()" whenever the selection changes. It actually queues the update to make sure it
happens only once no matter how many nodes are selected.

However, only connecting that signal would leave previously selected nodes with a signal connection that should no
longer exist. To properly disconnect previously selected nodes, we have to store the list of currently selected nodes so
we can disconnect them when the selection changes.

KoBeWi · 2025-07-03T14:19:59Z

editor/scene_tree_dock.cpp

+	for (int i = 0; i < node_previous_selection.size(); i++) {
+		Node *node = node_previous_selection.get(i);


Suggested change

for (int i = 0; i < node_previous_selection.size(); i++) {

Node *node = node_previous_selection.get(i);

for (Node *node : node_previous_selection) {

KoBeWi · 2025-07-03T14:21:16Z

editor/scene_tree_dock.h

@@ -135,6 +135,7 @@ class SceneTreeDock : public VBoxContainer {

 	EditorData *editor_data = nullptr;
 	EditorSelection *editor_selection = nullptr;
+	List<Node *> node_previous_selection;


Suggested change

List<Node *> node_previous_selection;

LocalVector<Node *> node_previous_selection;

No reason to use a List.
Also remember to reserve() before adding elements.

KoBeWi · 2025-07-03T14:23:16Z

You can't just connect script_changed to _update_script_button(), because it will update the button for every selected node. It should happen only once. You can add something like _queue_update_script_button().

Kazuo256 · 2025-07-08T02:34:45Z

You can't just connect script_changed to _update_script_button(), because it will update the button for every selected node. It should happen only once. You can add something like _queue_update_script_button().

Makes sense. Do you have a suggestion for where to check for queued updates? My first guess would under the NOTIFICATION_PROCESS case in the _notification() method.

KoBeWi · 2025-07-08T10:32:55Z

The usual pattern is

void queue_update() {
	if (is_update_queued) {
		return
	}
	is_update_queued = true
	call_deferred("actual_update")
}

Fixes godotengine#107059. The SceneTreeDock was not tracking script changes in selected nodes in any capacity as far as I could assess. To do that, my solution essentially connects the "script_changed" signal from selected nodes to "SceneTreeDock::_update_script_button()" whenever the selection changes. It actually queues the update to make sure it happens only once no matter how many nodes are selected. However, only connecting that signal would leave previously selected nodes with a signal connection that should no longer exist. To properly disconnect previously selected nodes, we have to store the list of currently selected nodes so we can disconnect them when the selection changes.

Kazuo256 · 2025-07-09T04:43:06Z

I've implemented the requested changes. One thing I realized is that clicking the script button in the scene tree dock to remove the script will call _update_script_button twice (regardless of how many nodes are selected). One because of the callback I added to the script_changed signal, and another as part of the usual logic of the remove script button. Is that OK or should I also adjust the script button to queue the update?

KoBeWi · 2025-07-09T12:50:48Z

should I also adjust the script button to queue the update?

Now that we have a queue method, it can be used more. So yes.

KoBeWi · 2025-07-09T12:58:53Z

editor/docks/scene_tree_dock.cpp

+	}
+
+	update_script_button_queued = true;
+
+	callable_mp(this, &SceneTreeDock::_update_script_button).call_deferred();


Suggested change

}

update_script_button_queued = true;

callable_mp(this, &SceneTreeDock::_update_script_button).call_deferred();

}

update_script_button_queued = true;

callable_mp(this, &SceneTreeDock::_update_script_button).call_deferred();

Unnecessary empty lines.

KoBeWi · 2025-07-09T12:59:39Z

editor/docks/scene_tree_dock.cpp

@@ -2911,6 +2923,18 @@ void SceneTreeDock::_selection_changed() {
 		_push_item(nullptr);
 	}

+	// Track script changes in selected nodes


Suggested change

// Track script changes in selected nodes

// Track script changes in selected nodes.

Also this comment should be when connecting the signal I think.

KoBeWi · 2025-07-09T13:01:49Z

editor/docks/scene_tree_dock.h

@@ -135,6 +135,8 @@ class SceneTreeDock : public VBoxContainer {

 	EditorData *editor_data = nullptr;
 	EditorSelection *editor_selection = nullptr;
+	LocalVector<Node *> node_previous_selection;
+	bool update_script_button_queued;


Suggested change

bool update_script_button_queued;

bool update_script_button_queued = false;

KoBeWi · 2025-07-09T13:02:15Z

editor/docks/scene_tree_dock.cpp

@@ -4653,6 +4677,7 @@ SceneTreeDock::SceneTreeDock(Node *p_scene_root, EditorSelection *p_editor_selec
 	edited_scene = nullptr;
 	editor_data = &p_editor_data;
 	editor_selection = p_editor_selection;
+	update_script_button_queued = false;


Unnecessary.
You might as well delete edited_scene above.

Kazuo256 requested a review from a team as a code owner June 29, 2025 19:19

Kazuo256 changed the title ~~Show detach script button when added via inspector~~ Properly show detach script button when script is added via inspector Jun 30, 2025

Chaosus added bug topic:editor labels Jun 30, 2025

Chaosus added this to the 4.5 milestone Jun 30, 2025

KoBeWi reviewed Jul 3, 2025

View reviewed changes

Kazuo256 force-pushed the fix-107059 branch from f763613 to 5b07c6e Compare July 9, 2025 04:02

Kazuo256 force-pushed the fix-107059 branch from 5b07c6e to a00caa9 Compare July 9, 2025 04:38

KoBeWi reviewed Jul 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Properly show detach script button when script is added via inspector #108122

Properly show detach script button when script is added via inspector #108122

Kazuo256 commented Jun 29, 2025 •

edited

Loading

Uh oh!

KoBeWi Jul 3, 2025

Uh oh!

Kazuo256 Jul 9, 2025

Uh oh!

KoBeWi Jul 3, 2025

Uh oh!

Kazuo256 Jul 9, 2025

Uh oh!

KoBeWi commented Jul 3, 2025

Uh oh!

Kazuo256 commented Jul 8, 2025

Uh oh!

KoBeWi commented Jul 8, 2025

Uh oh!

Kazuo256 commented Jul 9, 2025

Uh oh!

KoBeWi commented Jul 9, 2025

Uh oh!

KoBeWi Jul 9, 2025

Uh oh!

KoBeWi Jul 9, 2025

Uh oh!

KoBeWi Jul 9, 2025

Uh oh!

KoBeWi Jul 9, 2025 •

edited

Loading

Uh oh!

Uh oh!

		for (int i = 0; i < node_previous_selection.size(); i++) {
		Node *node = node_previous_selection.get(i);

	for (int i = 0; i < node_previous_selection.size(); i++) {
	Node *node = node_previous_selection.get(i);
	for (Node *node : node_previous_selection) {

	List<Node *> node_previous_selection;
	LocalVector<Node *> node_previous_selection;

	// Track script changes in selected nodes
	// Track script changes in selected nodes.

	bool update_script_button_queued;
	bool update_script_button_queued = false;

Uh oh!

Properly show detach script button when script is added via inspector #108122

Are you sure you want to change the base?

Properly show detach script button when script is added via inspector #108122

Conversation

Kazuo256 commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KoBeWi Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Kazuo256 Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Kazuo256 Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

KoBeWi commented Jul 3, 2025

Uh oh!

Kazuo256 commented Jul 8, 2025

Uh oh!

KoBeWi commented Jul 8, 2025

Uh oh!

Kazuo256 commented Jul 9, 2025

Uh oh!

KoBeWi commented Jul 9, 2025

Uh oh!

KoBeWi Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

KoBeWi Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kazuo256 commented Jun 29, 2025 •

edited

Loading

KoBeWi Jul 9, 2025 •

edited

Loading