Skip to content

Commit 9749caf

Browse files
EdwardChanCHEdwardChanCH
committed
Fixed 'Rename' and 'Batch Rename' ignoring non-top-level nodes and editing foreign nodes/ being available when it is not.
1 parent fa09dd1 commit 9749caf

3 files changed

Lines changed: 65 additions & 20 deletions

File tree

‎editor/docks/scene_tree_dock.cpp‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,8 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
615615
break;
616616
}
617617
if (editor_selection->get_selection().size() > 1) {
618-
if (!_validate_no_foreign()) {
618+
List<Node *> full_selection = editor_selection->get_full_selected_node_list();
619+
if (!_validate_no_foreign_selected(full_selection)) {
619620
break;
620621
}
621622
rename_dialog->popup_centered();
@@ -627,14 +628,16 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
627628
}
628629
Tree *tree = scene_tree->get_scene_tree();
629630
if (tree->is_anything_selected()) {
630-
if (!_validate_no_foreign()) {
631+
List<Node *> full_selection = editor_selection->get_full_selected_node_list();
632+
if (!_validate_no_foreign_selected(full_selection)) {
631633
break;
632634
}
633635
tree->grab_focus(!tree->has_focus(true));
634636
tree->edit_selected();
635637
}
636638
} break;
637639
case TOOL_REPARENT_TO_NEW_NODE:
640+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
638641
if (!_validate_no_foreign()) {
639642
break;
640643
}
@@ -708,6 +711,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
708711
} break;
709712
case TOOL_CUT:
710713
case TOOL_COPY: {
714+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
711715
if (!edited_scene || (p_tool == TOOL_CUT && !_validate_no_foreign())) {
712716
break;
713717
}
@@ -782,6 +786,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
782786
break;
783787
}
784788

789+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
785790
if (!_validate_no_foreign()) {
786791
break;
787792
}
@@ -800,10 +805,12 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
800805
break;
801806
}
802807

808+
// TODO: Bug, should be _validate_no_foreign_selected(full_selection). See PR https://github.com/godotengine/godot/pull/119909.
803809
if (!_validate_no_foreign()) {
804810
break;
805811
}
806812

813+
// TODO: Bug, should be _validate_no_instance_selected(full_selection). See PR https://github.com/godotengine/godot/pull/119909.
807814
if (!_validate_no_instance()) {
808815
break;
809816
}
@@ -883,6 +890,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
883890
break;
884891
}
885892

893+
// TODO: Bug, should be _validate_no_foreign_selected(full_selection). See PR https://github.com/godotengine/godot/pull/119910.
886894
if (!_validate_no_foreign()) {
887895
break;
888896
}
@@ -1055,6 +1063,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
10551063
break;
10561064
}
10571065

1066+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
10581067
if (!_validate_no_foreign()) {
10591068
break;
10601069
}
@@ -1164,6 +1173,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
11641173
return;
11651174
}
11661175

1176+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
11671177
if (!_validate_no_foreign()) {
11681178
break;
11691179
}
@@ -1311,6 +1321,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
13111321
}
13121322
} break;
13131323
case TOOL_OPEN_DOCUMENTATION: {
1324+
// TODO: Bug, should be get_full_selected_node_list(). See PR https://github.com/godotengine/godot/pull/119908.
13141325
const List<Node *> selection = editor_selection->get_top_selected_node_list();
13151326
for (const Node *node : selection) {
13161327
String class_name;
@@ -1378,6 +1389,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
13781389
break;
13791390
}
13801391

1392+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
13811393
if (!_validate_no_foreign()) {
13821394
break;
13831395
}
@@ -1413,6 +1425,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
14131425
break;
14141426
}
14151427

1428+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
14161429
if (!_validate_no_foreign()) {
14171430
break;
14181431
}
@@ -2402,9 +2415,12 @@ void SceneTreeDock::_node_prerenamed(Node *p_node, const String &p_new_name) {
24022415
}
24032416

24042417
bool SceneTreeDock::_validate_no_foreign() {
2405-
const List<Node *> selection = editor_selection->get_top_selected_node_list();
2418+
// Deprecated (see PR https://github.com/godotengine/godot/pull/119617).
2419+
return _validate_no_foreign_selected(editor_selection->get_top_selected_node_list());
2420+
}
24062421

2407-
for (Node *E : selection) {
2422+
bool SceneTreeDock::_validate_no_foreign_selected(const List<Node *> &p_selected) {
2423+
for (Node *E : p_selected) {
24082424
if (E != edited_scene && E->get_owner() != edited_scene) {
24092425
accept->set_text(TTR("Can't operate on nodes from a foreign scene!"));
24102426
accept->popup_centered();
@@ -2431,9 +2447,12 @@ bool SceneTreeDock::_validate_no_foreign() {
24312447
}
24322448

24332449
bool SceneTreeDock::_validate_no_instance() {
2434-
const List<Node *> selection = editor_selection->get_top_selected_node_list();
2450+
// Deprecated (see PR https://github.com/godotengine/godot/pull/119617).
2451+
return _validate_no_instance_selected(editor_selection->get_top_selected_node_list());
2452+
}
24352453

2436-
for (Node *E : selection) {
2454+
bool SceneTreeDock::_validate_no_instance_selected(const List<Node *> &p_selected) {
2455+
for (Node *E : p_selected) {
24372456
if (E != edited_scene && E->is_instance()) {
24382457
accept->set_text(TTR("This operation can't be done on instantiated scenes."));
24392458
accept->popup_centered();
@@ -3127,6 +3146,7 @@ void SceneTreeDock::_create() {
31273146
_do_create(parent);
31283147

31293148
} else if (current_option == TOOL_CHANGE_TYPE) {
3149+
// TODO: Bug, should be get_full_selected_node_list(). See PR https://github.com/godotengine/godot/pull/119909.
31303150
const List<Node *> selection = editor_selection->get_top_selected_node_list();
31313151
ERR_FAIL_COND(selection.is_empty());
31323152

@@ -3647,6 +3667,7 @@ void SceneTreeDock::_normalize_drop(Node *&to_node, int &to_pos, int p_type) {
36473667
}
36483668
}
36493669

3670+
// Same as get_top_selected_node_list() but returns an Array.
36503671
Array SceneTreeDock::_get_selection_array() {
36513672
const List<Node *> selection = editor_selection->get_top_selected_node_list();
36523673
TypedArray<Node> array;
@@ -3793,6 +3814,7 @@ void SceneTreeDock::_script_dropped(const String &p_file, NodePath p_to) {
37933814
}
37943815

37953816
void SceneTreeDock::_nodes_dragged(const Array &p_nodes, NodePath p_to, int p_type) {
3817+
// TODO: Deprecated, should be _validate_no_foreign_selected(editor_selection->get_top_selected_node_list()).
37963818
if (!_validate_no_foreign()) {
37973819
return;
37983820
}
@@ -3860,8 +3882,13 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
38603882
ERR_FAIL_COND(!EditorNode::get_singleton()->get_edited_scene());
38613883
menu->clear(false);
38623884

3885+
// TODO: Rename selection to top_selection?
3886+
// Returns only the top-level selected nodes (i.e. excludes any selected node whose parent is also selected).
3887+
// The first top-level node selected by the user is at the front of the list (i.e. not sorted in scene tree order).
38633888
const List<Node *> selection = editor_selection->get_top_selected_node_list();
3864-
List<Node *> full_selection = editor_selection->get_full_selected_node_list(); // Above method only returns nodes with common parent.
3889+
// Returns all selected nodes (list version of "get_selected_nodes").
3890+
// The first node selected by the user is at the front of the list (i.e. not sorted in scene tree order).
3891+
List<Node *> full_selection = editor_selection->get_full_selected_node_list();
38653892

38663893
scene_tree->get_scene_tree()->grab_focus(true);
38673894

@@ -3935,19 +3962,30 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
39353962
}
39363963

39373964
bool can_rename = true;
3965+
// TODO: Rename can_replace to can_change_type?
39383966
bool can_replace = true;
39393967

39403968
if (profile_allow_editing) {
3941-
for (Node *E : selection) {
3942-
if (E != edited_scene && (E->get_owner() != edited_scene || E->is_instance())) {
3969+
for (Node *E : full_selection) {
3970+
if (E != edited_scene && E->is_instance()) {
3971+
// E is a scene instance.
39433972
can_replace = false;
3944-
if (!E->is_instance()) {
3945-
can_rename = false;
3946-
}
3973+
// Allow TOOL_RENAME.
3974+
}
3975+
3976+
if (E != edited_scene && E->get_owner() != edited_scene) {
3977+
// E is a foreign node (editable children).
3978+
can_replace = false;
3979+
can_rename = false;
39473980
}
39483981

39493982
if (edited_scene->get_scene_inherited_state().is_valid()) {
3950-
if (E == edited_scene || edited_scene->get_scene_inherited_state()->find_node_by_path(edited_scene->get_path_to(E)) >= 0) {
3983+
if (E == edited_scene) {
3984+
// E is the root node in an inherited scene.
3985+
can_replace = false;
3986+
// Allow TOOL_RENAME. See _validate_no_foreign_selected().
3987+
} else if (edited_scene->get_scene_inherited_state()->find_node_by_path(edited_scene->get_path_to(E)) >= 0) {
3988+
// E is a child node in an inherited scene.
39513989
can_replace = false;
39523990
can_rename = false;
39533991
}
@@ -3975,6 +4013,7 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
39754013
}
39764014

39774015
if (selection.size() >= 1) {
4016+
// Note: This is correct, selection.front()->get() == edited_scene implies selection.size() == 1, so selection.front()->get() is the scene root.
39784017
if ((selection.front()->get() != edited_scene) && (can_rename || can_replace)) {
39794018
menu->add_shortcut(ED_GET_SHORTCUT("scene_tree/paste_node_as_replacement"), TOOL_PASTE_AS_REPLACEMENT);
39804019
if (!is_paste_icon_added) {
@@ -4117,8 +4156,9 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
41174156
END_SECTION()
41184157
}
41194158

4120-
if (profile_allow_editing && selection.size() > 1) {
4159+
if (profile_allow_editing && can_rename && full_selection.size() > 1) {
41214160
//this is not a commonly used action, it makes no sense for it to be where it was nor always present.
4161+
// TODO: Maybe group "batch_rename" with "rename"? See proposal #14626.
41224162
BEGIN_SECTION()
41234163
menu->add_icon_shortcut(get_editor_theme_icon(SNAME("Rename")), ED_GET_SHORTCUT("scene_tree/batch_rename"), TOOL_BATCH_RENAME);
41244164
END_SECTION()

‎editor/docks/scene_tree_dock.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,12 @@ class SceneTreeDock : public EditorDock {
252252
void _new_scene_from(const String &p_file);
253253
void _set_node_owner_recursive(Node *p_node, Node *p_owner, const HashMap<const Node *, Node *> &p_inverse_duplimap);
254254

255+
//[[deprecated("Use _validate_no_foreign_selected() instead (see PR https://github.com/godotengine/godot/pull/119617).")]]
255256
bool _validate_no_foreign();
257+
bool _validate_no_foreign_selected(const List<Node *> &p_selected);
258+
//[[deprecated("Use _validate_no_instance_selected() instead (see PR https://github.com/godotengine/godot/pull/119617).")]]
256259
bool _validate_no_instance();
260+
bool _validate_no_instance_selected(const List<Node *> &p_selected);
257261
void _selection_changed();
258262
void _update_script_button();
259263
void _queue_update_script_button();

‎editor/editor_data.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,16 +322,17 @@ class EditorSelection : public Object {
322322
void update(bool p_deferred = true);
323323
void clear();
324324

325-
// Returns only the top level selected nodes.
326-
// That is, if the selection includes some node and a child of that node, only the parent is returned.
325+
// Returns only the top-level selected nodes (i.e. excludes any selected node whose parent is also selected).
326+
// The first top-level node selected by the user is at the front of the list (i.e. not sorted in scene tree order).
327327
List<Node *> get_top_selected_node_list();
328-
// Same as get_top_selected_node_list but returns a copy in a TypedArray for binding to scripts.
328+
// Same as get_top_selected_node_list but returns a TypedArray for binding to scripts.
329329
TypedArray<Node> get_top_selected_nodes();
330-
// Returns all the selected nodes (list version of "get_selected_nodes").
330+
// Returns all selected nodes (list version of "get_selected_nodes").
331+
// The first node selected by the user is at the front of the list (i.e. not sorted in scene tree order).
331332
List<Node *> get_full_selected_node_list();
332-
// Same as get_full_selected_node_list but returns a copy in a TypedArray for binding to scripts.
333+
// Same as get_full_selected_node_list but returns a TypedArray for binding to scripts.
333334
TypedArray<Node> get_selected_nodes();
334-
// Returns the map of selected objects and their metadata.
335+
// Returns the map of all selected nodes and their metadata.
335336
HashMap<ObjectID, Object *> &get_selection() { return selection; }
336337

337338
~EditorSelection();

0 commit comments

Comments
 (0)