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

Allow to remove signal connections using Delete #82821

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

Madalaski
Copy link
Contributor

@Madalaski Madalaski commented Oct 4, 2023

This pull request improves usability with the Node connection panel, to allow the user to quickly delete connections by pressing the DEL key.

The _rmb_pressed method in the ConnectionsDock class has been renamed to _gui_input to reflect the new handling of other inputs.

This fixes the usability issue outlined in #82616.

@fire fire requested a review from a team October 5, 2023 16:16
Copy link
Contributor

@MewPurPur MewPurPur left a comment

Choose a reason for hiding this comment

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

Comment style nitpicks. I don't see an issue in the rest, other than I don't think connection needs to be initialized as it's only used once.

editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

Please amend the commit message so it's meaningful, "Initial Commit" isn't a very good description for this change ;)
See https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

@Madalaski
Copy link
Contributor Author

Comment style nitpicks. I don't see an issue in the rest, other than I don't think connection needs to be initialized as it's only used once.

Cheers, should've been more careful with the comments. I got an error when I removed the connection initialisation. From the looks of it, connection uses a Variant() operation to convert get_metadata()'s return to Connection and then an implicit conversion in the method call into ConnectionData.

Please amend the commit message so it's meaningful, "Initial Commit" isn't a very good description for this change ;) See https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

Done! 😄

@MewPurPur
Copy link
Contributor

MewPurPur commented Oct 12, 2023

Makes sense then to keep it then

@clayjohn clayjohn requested review from dalexeev and removed request for MewPurPur January 15, 2024 06:10
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.h Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
@dalexeev
Copy link
Member

Please squash commits into one. To do this you need a local git client, GitHub web interface doesn't support it.

editor/connections_dialog.cpp Outdated Show resolved Hide resolved
editor/connections_dialog.cpp Outdated Show resolved Hide resolved
Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

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

Looks good to me. Note that when the Script workspace is open, you can press Delete regardless of the mouse cursor position. But when a 2D or 3D workspace is open, the cursor must be inside the Connection Editor, otherwise deleting a node in the Scene Tree will be triggered. This is not directly related to this PR, just a note.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 17, 2024
@YuriSizov YuriSizov changed the title Allowing the DEL Key to remove Signal connections Allow to remove signal connections using Delete Jan 17, 2024
@YuriSizov YuriSizov merged commit a0fec56 into godotengine:master Jan 17, 2024
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks! And congrats on your first merged Godot pull request!

@Madalaski Madalaski deleted the connections_delete_key branch January 18, 2024 00:24
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.

Pressing DEL on selected signals should delete them
7 participants