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 copy-paste and cut-paste in GridMap #95322

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

Nodragem
Copy link
Contributor

@Nodragem Nodragem commented Aug 9, 2024

fix #52608 which also makes "paste selects" option useful again.

The idea of the fix is to block the propagation of the click once the Gridmap has consumed it.

@Nodragem Nodragem requested a review from a team as a code owner August 9, 2024 09:46
@AThousandShips AThousandShips added this to the 4.4 milestone Aug 9, 2024
@AThousandShips AThousandShips added 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 9, 2024
@Nodragem
Copy link
Contributor Author

Nodragem commented Aug 9, 2024

is it too late to get into 4.3? knowing it is a small, uncontroversial fix with big usability improvement?

@AThousandShips
Copy link
Member

We are only merging critical regressions right now, it'll hopefully make it into 4.3.1

@Nodragem
Copy link
Contributor Author

Nodragem commented Aug 9, 2024

that would be great, thanks!

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Note that I still find the copy/paste UX in the GridMap editor to be suboptimal (I'd expect to be able to press Ctrl + V to confirm copying or cutting and be able to do it multiple times), but this will do for now.

@akien-mga akien-mga changed the title Fix copy-paste and cut-paste in Gridmap Fix copy-paste and cut-paste in GridMap Aug 16, 2024
@akien-mga akien-mga merged commit 9393388 into godotengine:master Aug 16, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@Nodragem
Copy link
Contributor Author

Nodragem commented Aug 18, 2024

Thank you for merging my contribution!

Tested locally, it works as expected.

Note that I still find the copy/paste UX in the GridMap editor to be suboptimal (I'd expect to be able to press Ctrl + V to confirm copying or cutting and be able to do it multiple times), but this will do for now.

I am using it and I think it works pretty well, but I agree that these are not really copy/paste and cut/paste functions.

The current behaviours could probably be better named as "Duplicate Selection" (already the case*) and "Move Selection"; and their shorcuts could be a one key shorcut. Furthermore, I would probably change the option "Paste Selects" to something clearer like "Keep Original selected", and group the "Move Selection", "Duplicate Selection", "Keep Original Selected" together is the menu.

But, what I would really like to work on next, is to surface all this tools in a bottom panel and show them as icons, see this proposal:
godotengine/godot-proposals#10405

@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 16, 2024
@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 19, 2025
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.

Duplicating gridmap area unselects the gridmap node, hence stops the user from further editing
4 participants