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

Prevent selecting when a CanvasItem is selected #93671

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jun 27, 2024

#86804 made it possible to select CanvasItems when any tool is active. However it has an unwanted side-effect that you can select CanvasItems while another CanvasItem is already selected, if that CanvasItem can't be interacted with using the current tool. This PR prevents that, i.e. trying to move/rotate/scale a "locked" CanvasItem results in no-op. Note that "locked" includes actually locked nodes, nodes inside viewport or invisible nodes.

Fixes #92072

@akien-mga
Copy link
Member

CC @ryevdokimov

@ryevdokimov
Copy link
Contributor

This is not a hill I'm willing to die on, but I'm curious in your opinion why no-op is preferable to selection in this scenario? From my perspective if the user is trying to move or rotate something for example, why not let them without having to switch to the select tool or use the scene tree dock?

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 28, 2024

I think only Select tool should allow selecting. The exception is when you have nothing selected, so selecting is the only option to get started. Once you have a CanvasItem selected, showing a selection box can be unexpected and confusing (like imagine you try to move a node, but forgot to unlock it and wonder why your move tool is selecting things). I could maybe add a toaster message, similar to when you try to move a Control inside Container.

@ryevdokimov
Copy link
Contributor

Fair enough. I guess it depends on just what your expectations are and what you're used to. I'm personally in the camp of all tools should just always be able to select to prevent: #87608 and so the behavior is always consistent. No-op to me in general is more confusing, because it lacks any feedback on what is supposed to happen, but if this the direction we go I agree that a toaster message after repeated no-op could be helpful.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 29, 2024

vQk4pLFuYB.mp4

@akien-mga akien-mga merged commit 13ea24c into godotengine:master Jul 1, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the burn_the_box branch July 1, 2024 16:43
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.

Minor issue with tools when SubViewport is selected
3 participants