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 Camera2D is enabled when dragging scene files to the CanvasItemEditor #87743

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

WhalesState
Copy link
Contributor

Fixes: #87704

scene/2d/camera_2d.cpp Outdated Show resolved Hide resolved
@KoBeWi
Copy link
Member

KoBeWi commented Feb 18, 2024

This bug potentially affects all nodes that rely on is_part_of_edited_scene() and can be part of a drag preview, so the change should be done in is_part_of_edited_scene() I think.

@WhalesState
Copy link
Contributor Author

WhalesState commented Feb 18, 2024

This bug potentially affects all nodes that rely on is_part_of_edited_scene() and can be part of a drag preview, so the change should be done in is_part_of_edited_scene() I think.

Yes, but the main issue is that the dragged scenes is instantiated and is added to the editor's root while dragging, this behavior will result in more issues like running the tool scripts while dragging, especially if it's a heavy scene.

is_part_of_edited_scene() works as expected, but while dragging it will return false because a scene instance will be added to the editor's root before dropping it to the edited scene.

I have tried to stop the dragged scene from proccessing, but it didn't work as expected, maybe we can add an internal preview node to the edited scene and we prevent it from being saved instead of adding it to the editor's root.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 18, 2024

Which is why I suggested that the change can be done in is_part_of_edited_scene(), so that it checks whether the node is under scene root viewport, not the scene itself. While that would make it technically incorrect, the only nodes under the root viewport are the scene and the drag previews, so it shouldn't have any negative impact.

@WhalesState
Copy link
Contributor Author

Is it fine to add this get_tree()->get_edited_scene_root()->get_viewport() == get_viewport() ?

bool Node::is_part_of_edited_scene() const {
	return Engine::get_singleton()->is_editor_hint() && is_inside_tree() && get_tree()->get_edited_scene_root() && 
			(get_tree()->get_edited_scene_root() == this || get_tree()->get_edited_scene_root()->get_viewport() == get_viewport() || get_tree()->get_edited_scene_root()->is_ancestor_of(this));
}

@KoBeWi
Copy link
Member

KoBeWi commented Feb 18, 2024

It reminded me of this old issue: #39221

The best condition would be: Engine::get_singleton()->is_editor_hint() && is_inside_tree() && EditorNode::get_singleton()->get_scene_root()->is_ancestor_of(this), but since this is Node code, the EditorNode can't be used. The problem with your check is that if a scene is empty, the issue will still be happening. Though it's still less likely to appear.

I'd suggest this for now:

bool Node::is_part_of_edited_scene() const {
	return Engine::get_singleton()->is_editor_hint() && is_inside_tree() && get_tree()->get_edited_scene_root() && 
			get_tree()->get_edited_scene_root()->get_parent()->is_ancestor_of(this);
}

@akien-mga akien-mga merged commit 0f0515a into godotengine:master Feb 18, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@WhalesState WhalesState deleted the camera2d branch February 18, 2024 23:21
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.

Weird camera bug
3 participants