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 new region select with selection when over geometry #87948

Conversation

ryevdokimov
Copy link
Contributor

@ryevdokimov ryevdokimov commented Feb 4, 2024

Fixes: #87949 Fixes #85736

You should be able to region select with the select tool even if you already have a node selected. All other tools already have the default behavior of remotely manipulating their respective gizmo when a node is selected.

Before:

2024-02-03.23-23-48.mp4

After:

2024-02-04.10-32-35.mp4

@ryevdokimov ryevdokimov requested a review from a team as a code owner February 4, 2024 15:44
@AThousandShips AThousandShips added this to the 3.6 milestone Feb 4, 2024
@AThousandShips AThousandShips modified the milestones: 3.6, 4.x Feb 4, 2024
@KoBeWi KoBeWi added bug and removed discussion labels Feb 4, 2024
@AThousandShips AThousandShips modified the milestones: 4.x, 4.3 Feb 4, 2024
@AThousandShips AThousandShips changed the title Fix bug that moves node during region select over geometry by defaulting to region select Fix new region select with selection when over geometry Feb 4, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Feb 4, 2024

This breaks selected node dragging:

qVmpxJZhHV.mp4

Dragging should still move a node when the clicked node is part of the current selection.

@ryevdokimov
Copy link
Contributor Author

Good catch. Working on it...

@ryevdokimov ryevdokimov force-pushed the always-default-to-region-select branch from 5445cdb to 92a8200 Compare February 4, 2024 17:58
@ryevdokimov
Copy link
Contributor Author

Should be fixed now. Had to add code to check whether or not the node(s) trying to be dragged are the ones that are selected.

@ryevdokimov ryevdokimov force-pushed the always-default-to-region-select branch from 92a8200 to 62852c6 Compare February 4, 2024 18:13
@ryevdokimov ryevdokimov force-pushed the always-default-to-region-select branch from 62852c6 to 6718a9e Compare February 4, 2024 18:34
@ryevdokimov
Copy link
Contributor Author

ryevdokimov commented Feb 4, 2024

Suggestions applied and tested. Thanks for the help.

@Cammymoop
Copy link
Contributor

It seems that both this approach and the one in #85740 could be desirable, probably due to the select mode tool wearing too many hats. I would prefer having the old functionality from before 3.5 restored (selecting and moving a node with a single click) but I could see myself using this style too.

Either way it looks like there's conflicting expectations on how the select tool should work in this instance, aside from the agreement that the current behavior is a bug.

@akien-mga akien-mga merged commit 710c56e into godotengine:master Feb 5, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Cammymoop This might warrant opening a proposal to discuss what should be the desired behavior, or whether it needs to be made configurable. If the behavior changed in 3.5 / 4.0 and most users are now familiar with the current behavior as being the intended one, changing it back would likely be a problem for more users than the 3.5 / 4.0 change was.

@ryevdokimov ryevdokimov deleted the always-default-to-region-select branch February 5, 2024 15:09
@ryevdokimov ryevdokimov restored the always-default-to-region-select branch February 16, 2024 19:33
@ryevdokimov ryevdokimov deleted the always-default-to-region-select branch February 16, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants