Skip to content

Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes #119617

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.

Sign up for GitHub

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

Open
EdwardChanCH wants to merge 1 commit into
base: master
Choose a base branch
from
+73 −57
Open

Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes #119617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 66 additions & 51 deletions editor/docks/scene_tree_dock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}
if (editor_selection->get_selection().size() > 1) {
if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
if (!_validate_no_foreign_selected(editor_selection->get_full_selected_node_list())) {
break;
}
rename_dialog->popup_centered();
Expand All @@ -627,7 +627,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
}
Tree *tree = scene_tree->get_scene_tree();
if (tree->is_anything_selected()) {
if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
if (!_validate_no_foreign_selected(editor_selection->get_full_selected_node_list())) {
break;
}
tree->grab_focus(!tree->has_focus(true));
Expand Down Expand Up @@ -708,11 +708,11 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
} break;
case TOOL_CUT:
case TOOL_COPY: {
if (!edited_scene || (p_tool == TOOL_CUT && !_validate_no_foreign_selected(editor_selection->get_top_selected_node_list()))) {
List<Node *> selection = editor_selection->get_top_selected_node_list();
if (!edited_scene || (p_tool == TOOL_CUT && !_validate_no_foreign_selected(selection))) {
break;
}

List<Node *> selection = editor_selection->get_top_selected_node_list();
if (selection.is_empty()) {
break;
}
Expand Down Expand Up @@ -782,11 +782,12 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
List<Node *> selection = editor_selection->get_top_selected_node_list();
if (!_validate_no_foreign_selected(selection)) {
break;
}

for (const Node *node : editor_selection->get_top_selected_node_list()) {
for (const Node *node : selection) {
if (node == edited_scene) {
EditorNode::get_singleton()->show_warning(TTR("This operation can't be done on the tree root."));
return;
Expand Down Expand Up @@ -816,9 +817,9 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
}

Node *selected = scene_tree->get_selected();
const List<Node *> &top_node_list = editor_selection->get_top_selected_node_list();
if (!selected && !top_node_list.is_empty()) {
selected = top_node_list.front()->get();
const List<Node *> &selection = editor_selection->get_top_selected_node_list();
if (!selected && !selection.is_empty()) {
selected = selection.front()->get();
}

if (selected) {
Expand Down Expand Up @@ -892,6 +893,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
bool MOVING_DOWN = (p_tool == TOOL_MOVE_DOWN);
bool MOVING_UP = !MOVING_DOWN;

// TODO: Bug, see PR https://github.com/godotengine/godot/pull/119910.
List<Node *> selection = editor_selection->get_full_selected_node_list();
selection.sort_custom<Node::Comparator>(); // sort by index
if (MOVING_DOWN) {
Expand Down Expand Up @@ -1057,13 +1059,13 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
List<Node *> selection = editor_selection->get_top_selected_node_list();
if (!_validate_no_foreign_selected(selection)) {
break;
}

List<Node *> nodes = editor_selection->get_top_selected_node_list();
HashSet<Node *> nodeset;
for (Node *E : nodes) {
for (Node *E : selection) {
nodeset.insert(E);
}
reparent_dialog->set_current(nodeset);
Expand All @@ -1074,10 +1076,10 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

List<Node *> nodes = editor_selection->get_top_selected_node_list();
ERR_FAIL_COND(nodes.size() != 1);
List<Node *> selection = editor_selection->get_top_selected_node_list();
ERR_FAIL_COND(selection.size() != 1);

Node *node = nodes.front()->get();
Node *node = selection.front()->get();
Node *root = get_tree()->get_edited_scene_root();

if (node == root) {
Expand Down Expand Up @@ -1160,32 +1162,32 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

List<Node *> remove_list = editor_selection->get_top_selected_node_list();
List<Node *> selection = editor_selection->get_top_selected_node_list();

if (remove_list.is_empty()) {
if (selection.is_empty()) {
return;
}

if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
if (!_validate_no_foreign_selected(selection)) {
break;
}

bool allow_ask_delete_tracks = EDITOR_GET("docks/scene_tree/ask_before_deleting_related_animation_tracks").operator bool();
bool has_tracks_to_delete = allow_ask_delete_tracks && _has_tracks_to_delete(edited_scene, remove_list);
bool has_tracks_to_delete = allow_ask_delete_tracks && _has_tracks_to_delete(edited_scene, selection);
if (p_confirm_override && !has_tracks_to_delete) {
_delete_confirm();
} else {
String msg;
if (remove_list.size() > 1) {
if (selection.size() > 1) {
bool any_children = false;
for (List<Node *>::ConstIterator itr = remove_list.begin(); !any_children && itr != remove_list.end(); ++itr) {
for (List<Node *>::ConstIterator itr = selection.begin(); !any_children && itr != selection.end(); ++itr) {
any_children = (*itr)->get_child_count() > 0;
}

msg = vformat(any_children ? TTR("Delete %d nodes and any children?") : TTR("Delete %d nodes?"), remove_list.size());
msg = vformat(any_children ? TTR("Delete %d nodes and any children?") : TTR("Delete %d nodes?"), selection.size());
} else {
if (!p_confirm_override) {
Node *node = remove_list.front()->get();
Node *node = selection.front()->get();
if (node == editor_data->get_edited_scene_root()) {
msg = vformat(TTR("Delete the root node \"%s\"?"), node->get_name());
} else if (!node->is_instance() && node->get_child_count() > 0) {
Expand Down Expand Up @@ -1379,11 +1381,11 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
const List<Node *> selection = editor_selection->get_top_selected_node_list();
if (!_validate_no_foreign_selected(selection)) {
break;
}

const List<Node *> selection = editor_selection->get_top_selected_node_list();
const List<Node *>::Element *e = selection.front();
if (e) {
Node *node = e->get();
Expand Down Expand Up @@ -1414,11 +1416,11 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
break;
}

if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
const List<Node *> selection = editor_selection->get_top_selected_node_list();
if (!_validate_no_foreign_selected(selection)) {
break;
}

const List<Node *> selection = editor_selection->get_top_selected_node_list();
const List<Node *>::Element *e = selection.front();
if (e) {
Node *node = e->get();
Expand Down Expand Up @@ -2654,9 +2656,9 @@ void SceneTreeDock::_do_reparent(Node *p_new_parent, int p_position_in_parent, V
}

void SceneTreeDock::_script_created(Ref<Script> p_script) {
const List<Node *> &selected = editor_selection->get_top_selected_node_list();
const List<Node *> &selection = editor_selection->get_top_selected_node_list();

if (selected.is_empty()) {
if (selection.is_empty()) {
return;
}

Expand All @@ -2665,8 +2667,8 @@ void SceneTreeDock::_script_created(Ref<Script> p_script) {
}

EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton();
undo_redo->create_action(TTR("Attach Script"), UndoRedo::MERGE_DISABLE, selected.front()->get());
for (Node *E : selected) {
undo_redo->create_action(TTR("Attach Script"), UndoRedo::MERGE_DISABLE, selection.front()->get());
for (Node *E : selection) {
Ref<Script> existing = E->get_script();
undo_redo->add_do_method(InspectorDock::get_singleton(), "store_script_properties", E);
undo_redo->add_undo_method(InspectorDock::get_singleton(), "store_script_properties", E);
Expand Down Expand Up @@ -2861,23 +2863,23 @@ void SceneTreeDock::_toggle_editable_children(Node *p_node) {
}

void SceneTreeDock::_delete_confirm(bool p_cut) {
List<Node *> remove_list = editor_selection->get_top_selected_node_list();
List<Node *> selection = editor_selection->get_top_selected_node_list();

if (remove_list.is_empty()) {
if (selection.is_empty()) {
return;
}

bool entire_scene = false;

for (const Node *E : remove_list) {
for (const Node *E : selection) {
if (E == edited_scene) {
entire_scene = true;
break;
}
}

if (!entire_scene) {
for (const Node *E : remove_list) {
for (const Node *E : selection) {
// `move_child` + `get_index` doesn't really work for internal nodes.
ERR_FAIL_COND_MSG(E->is_internal(), "Trying to remove internal node, this is not supported.");
}
Expand All @@ -2886,7 +2888,7 @@ void SceneTreeDock::_delete_confirm(bool p_cut) {
EditorNode::get_singleton()->hide_unused_editors(this);

EditorUndoRedoManager *undo_redo = EditorUndoRedoManager::get_singleton();
undo_redo->create_action(p_cut ? TTR("Cut Node(s)") : TTR("Remove Node(s)"), UndoRedo::MERGE_DISABLE, remove_list.front()->get());
undo_redo->create_action(p_cut ? TTR("Cut Node(s)") : TTR("Remove Node(s)"), UndoRedo::MERGE_DISABLE, selection.front()->get());

if (entire_scene) {
undo_redo->add_do_method(EditorNode::get_singleton(), "set_edited_scene", (Object *)nullptr);
Expand All @@ -2896,7 +2898,7 @@ void SceneTreeDock::_delete_confirm(bool p_cut) {
undo_redo->add_undo_reference(edited_scene);
} else {
// Delete nodes.
for (Node *n : remove_list) {
for (Node *n : selection) {
if (!n->is_inside_tree() || !n->get_parent()) {
continue;
}
Expand All @@ -2923,11 +2925,11 @@ void SceneTreeDock::_delete_confirm(bool p_cut) {
}

if (delete_tracks_checkbox->is_pressed() || p_cut) {
remove_list.sort_custom<Node::Comparator>(); // Sort nodes to keep positions.
selection.sort_custom<Node::Comparator>(); // Sort nodes to keep positions.
HashMap<Node *, NodePath> path_renames;

// Delete from animations.
for (Node *n : remove_list) {
for (Node *n : selection) {
if (!n->is_inside_tree() || !n->get_parent()) {
continue;
}
Expand Down Expand Up @@ -3644,6 +3646,7 @@ void SceneTreeDock::_normalize_drop(Node *&to_node, int &to_pos, int p_type) {
}
}

// Same as get_top_selected_node_list() but returns an Array.
Array SceneTreeDock::_get_selection_array() {
const List<Node *> selection = editor_selection->get_top_selected_node_list();
TypedArray<Node> array;
Expand Down Expand Up @@ -3790,12 +3793,12 @@ void SceneTreeDock::_script_dropped(const String &p_file, NodePath p_to) {
}

void SceneTreeDock::_nodes_dragged(const Array &p_nodes, NodePath p_to, int p_type) {
if (!_validate_no_foreign_selected(editor_selection->get_top_selected_node_list())) {
const List<Node *> selection = editor_selection->get_top_selected_node_list();

if (!_validate_no_foreign_selected(selection)) {
return;
}

const List<Node *> selection = editor_selection->get_top_selected_node_list();

if (selection.is_empty()) {
return; //nothing to reparent
}
Expand Down Expand Up @@ -3932,19 +3935,29 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
}

bool can_rename = true;
bool can_replace = true;
bool can_replace = true; // TODO: Rename can_replace to can_change_type?

if (profile_allow_editing) {
for (Node *E : full_selection) {
if (E != edited_scene && (E->get_owner() != edited_scene || E->is_instance())) {
if (E != edited_scene && E->is_instance()) {
// E is a scene instance.
can_replace = false;
if (!E->is_instance()) {
can_rename = false;
}
// Allow TOOL_RENAME.
}

if (E != edited_scene && E->get_owner() != edited_scene) {
// E is a foreign node (editable children).
can_replace = false;
can_rename = false;
}

if (edited_scene->get_scene_inherited_state().is_valid()) {
if (E == edited_scene || edited_scene->get_scene_inherited_state()->find_node_by_path(edited_scene->get_path_to(E)) >= 0) {
if (E == edited_scene) {
// E is the root node in an inherited scene.
can_replace = false;
// Allow TOOL_RENAME. See _validate_no_foreign_selected().
} else if (edited_scene->get_scene_inherited_state()->find_node_by_path(edited_scene->get_path_to(E)) >= 0) {
// E is a child node in an inherited scene.
can_replace = false;
can_rename = false;
}
Expand Down Expand Up @@ -3972,6 +3985,7 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
}

if (selection.size() >= 1) {
// Note: This is correct, selection.front()->get() == edited_scene implies selection.size() == 1, so selection.front()->get() is the scene root.
if ((selection.front()->get() != edited_scene) && (can_rename || can_replace)) {
menu->add_shortcut(ED_GET_SHORTCUT("scene_tree/paste_node_as_replacement"), TOOL_PASTE_AS_REPLACEMENT);
if (!is_paste_icon_added) {
Expand Down Expand Up @@ -4114,8 +4128,9 @@ void SceneTreeDock::_tree_rmb(const Vector2 &p_menu_pos) {
END_SECTION()
}

if (profile_allow_editing && selection.size() > 1) {
if (profile_allow_editing && can_rename && full_selection.size() > 1) {
//this is not a commonly used action, it makes no sense for it to be where it was nor always present.
// TODO: Maybe group "batch_rename" with "rename"? See PR https://github.com/godotengine/godot/pull/119978.
BEGIN_SECTION()
menu->add_icon_shortcut(get_editor_theme_icon(SNAME("Rename")), ED_GET_SHORTCUT("scene_tree/batch_rename"), TOOL_BATCH_RENAME);
END_SECTION()
Expand Down Expand Up @@ -4532,8 +4547,8 @@ List<Node *> SceneTreeDock::paste_nodes(bool p_paste_as_sibling) {
}

void SceneTreeDock::paste_node_as_replacement() {
List<Node *> selected_node_list = editor_selection->get_top_selected_node_list();
for (Node *selected : selected_node_list) {
List<Node *> selection = editor_selection->get_top_selected_node_list();
for (Node *selected : selection) {
Node *clipboard_node = node_clipboard.front()->get();
HashMap<const Node *, Node *> duplimap;
Node *new_node = clipboard_node->duplicate_from_editor(duplimap);
Expand Down
13 changes: 7 additions & 6 deletions editor/editor_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,16 +322,17 @@ class EditorSelection : public Object {
void update(bool p_deferred = true);
void clear();

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

~EditorSelection();
Expand Down
Loading