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

Fix crash when interacting with appended nodes not part of the edited scene #94916

Closed
wants to merge 1 commit into from

Conversation

yahkr
Copy link
Contributor

@yahkr yahkr commented Jul 29, 2024

closes #94844

The crash is an issue with how the engine handles the selection of nodes that have been appended to the main screen. It gets the deepest editable node and then climbs up until it reaches the parent of the edited scene, but for nodes appended to the main scene through plugins, this results in node becoming NULL, which is then checked for a specific class, causing a hard crash.

added:

// Prevent selection of nodes that exist outside the current edited scene.
if (!edited_scene->is_ancestor_of(node)) {
	continue;
}

related code:

if (node != edited_scene) {
	node = edited_scene->get_deepest_editable_node(node);
}

// Prevent selection of nodes not owned by the edited scene.
while (node && node != edited_scene->get_parent()) {
	Node *node_owner = node->get_owner();
	if (node_owner == edited_scene || node == edited_scene || (node_owner != nullptr && edited_scene->is_editable_instance(node_owner))) {
		break;
	}
	node = node->get_parent();
}

// Replace the node by the group if grouped
if (node->is_class("Node3D")) {

This change also prevents the pointless spam of bad ancestor checks prior to the crash:

ERROR: Condition "!is_ancestor_of(p_start_node)" is true. Returning: p_start_node
   at: get_deepest_editable_node (scene/main/node.cpp:2525)
ERROR: Condition "!is_ancestor_of(p_node)" is true. Returning: false
   at: is_editable_instance (scene/main/node.cpp:2518)
ERROR: Condition "!is_ancestor_of(p_node)" is true. Returning: false
   at: is_editable_instance (scene/main/node.cpp:2518)
ERROR: Condition "!is_ancestor_of(p_node)" is true. Returning: false
   at: is_editable_instance (scene/main/node.cpp:2518)


================================================================
handle_crash: Program crashed with signal 11

@yahkr yahkr requested a review from a team as a code owner July 29, 2024 13:45
@AThousandShips AThousandShips added bug topic:editor crash cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Jul 29, 2024
@AThousandShips AThousandShips added this to the 4.4 milestone Jul 29, 2024
@yahkr
Copy link
Contributor Author

yahkr commented Aug 19, 2024

looks like a pr got merged with this fix: #95420

@yahkr yahkr closed this Aug 19, 2024
@akien-mga akien-mga added archived and removed cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Aug 19, 2024
@akien-mga akien-mga removed this from the 4.4 milestone Aug 19, 2024
@yahkr yahkr deleted the 94844-fix branch December 4, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants