Skip to content

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

Open
EdwardChanCH wants to merge 1 commit into
godotengine:masterfrom
EdwardChanCH:fix_missing_rename_node_button
Open

Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes#119617
EdwardChanCH wants to merge 1 commit into
godotengine:masterfrom
EdwardChanCH:fix_missing_rename_node_button

Conversation

@EdwardChanCH
Copy link
Copy Markdown
Contributor

@EdwardChanCH EdwardChanCH commented May 21, 2026

Closes #119593
Closes #119645

Related PRs:

Changes:

  • "Rename" now correctly appears when right-clicking the root node of an inherited scene.
    • Had to untangle some nested conditions, but there should be no unlisted changes.
  • "Rename" and "Batch Rename" are now correctly hidden when any yellow foreign node is selected.
  • "Alert!" pop-up now correctly shows up when trying to rename yellow foreign nodes with F2.

@EdwardChanCH EdwardChanCH requested a review from a team as a code owner May 21, 2026 02:02
@EdwardChanCH EdwardChanCH changed the title Fix missing "Rename" context menu button in inherited scene root. Fix missing "Rename" context menu button in inherited scene root May 21, 2026
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

In the future, a follow-up PR could sync the Rename button (shown/hidden) with the color of the node name (white/yellow):

} else if (p_part_of_subscene) {
if (valid_types.is_empty()) {
_set_item_custom_color(p_item, get_theme_color(SNAME("warning_color"), EditorStringName(Editor)));
}

@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

Need more time for this fix, the rabbit hole gets deeper and deeper...

@EdwardChanCH EdwardChanCH changed the title Fix missing "Rename" context menu button in inherited scene root Fix "Rename", "Batch Rename", and "Change Type" incorrectly shown/hidden May 25, 2026
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 70111b5 to b9dca42 Compare May 25, 2026 20:31
@EdwardChanCH EdwardChanCH changed the title Fix "Rename", "Batch Rename", and "Change Type" incorrectly shown/hidden [WIP] Fix "Rename", "Batch Rename", and "Change Type" incorrectly shown/hidden May 28, 2026
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from b9dca42 to 7dcd02e Compare May 28, 2026 17:25
@EdwardChanCH EdwardChanCH changed the title [WIP] Fix "Rename", "Batch Rename", and "Change Type" incorrectly shown/hidden [WIP] Fix "Rename", "Batch Rename", "Change Type", "Move Up/Down" incorrectly shown and done May 29, 2026
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch 2 times, most recently from 9524599 to 5e03c40 Compare May 29, 2026 07:53
@ajreckof
Copy link
Copy Markdown
Member

Hi #74236 seems unrelated to the other issue except for the fact that they are in the same part of the editor if so please make a separate PR to fix it

@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

Hi #74236 seems unrelated to the other issue except for the fact that they are in the same part of the editor if so please make a separate PR to fix it

It is related, unfortunately. "Change Type" (along with the others) requires the new _validate_no_foreign_selected() to prevent editing foreign child nodes in the selection.

But I plan to split this into separate PRs (one per issue) once I get everything fixed and tested (would require duplicating some shared changes). Thanks for the reminder though.

		case TOOL_CHANGE_TYPE: {
			if (!profile_allow_editing) {
				break;
			}

			List<Node *> full_selection = editor_selection->get_full_selected_node_list();

			if (!_validate_no_foreign_selected(full_selection)) { // New
				break;
			}

			if (!_validate_no_instance_selected(full_selection)) { // New
				break;
			}

@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch 2 times, most recently from b6c2a0b to 6bd3e82 Compare May 31, 2026 03:06
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented May 31, 2026

Triple checked all the related code and locally tested all the new code, finally!
Will split this into smaller, more focused PRs.

Bug triage results:

  • Confusing selection with full_selection in scene_tree_dock.cpp.
  • Misusing get_top_selected_node_list() for get_full_selected_node_list().
  • Incorrect usage of _validate_no_foreign(), _validate_no_instance() to check full_selection.

Split PRs:

@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 6bd3e82 to 52e10bd Compare May 31, 2026 04:27
@EdwardChanCH EdwardChanCH changed the title [WIP] Fix "Rename", "Batch Rename", "Change Type", "Move Up/Down" incorrectly shown and done [WIP] Fix "Rename", "Batch Rename", "Move Up/Down" incorrectly shown and done May 31, 2026
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 52e10bd to edbc9a7 Compare May 31, 2026 05:55
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from edbc9a7 to e15ea5e Compare May 31, 2026 06:23
@EdwardChanCH EdwardChanCH changed the title [WIP] Fix "Rename", "Batch Rename", "Move Up/Down" incorrectly shown and done [WIP] Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes May 31, 2026
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch 2 times, most recently from f19d469 to 2334a20 Compare May 31, 2026 07:58
@EdwardChanCH EdwardChanCH changed the title [WIP] Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes Fix "Rename" and "Batch Rename" ignoring non-top-level nodes and editing foreign nodes May 31, 2026
@EdwardChanCH EdwardChanCH marked this pull request as ready for review May 31, 2026 08:20
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

Cleaned up the code and undid some unnecessary changes. Ready for review!

@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 2334a20 to d4a05dc Compare June 2, 2026 05:47
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Jun 2, 2026

Updated comments. In sync with #119909 and #119910.

@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch 3 times, most recently from 9749caf to 99df8b9 Compare June 3, 2026 19:50
@EdwardChanCH
Copy link
Copy Markdown
Contributor Author

EdwardChanCH commented Jun 3, 2026

Rebased after #119909.

  • Cleaned up duplicated editor_selection->get_top_selected_node_list() calls from #119909.
  • Acted on comments about renaming variables to selection = editor_selection->get_top_selected_node_list().

@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 99df8b9 to 00344c0 Compare June 3, 2026 19:57
…iting foreign nodes/ being available when it is not.
@EdwardChanCH EdwardChanCH force-pushed the fix_missing_rename_node_button branch from 00344c0 to d56c4f6 Compare June 3, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Rename" and "Batch Rename" can sometimes edit yellow foreign nodes "Rename" and "Batch Rename" incorrectly shown/hidden

3 participants