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

Making nodes enclosed within comment move with the comment node #54970

Merged

Conversation

theoway
Copy link
Contributor

@theoway theoway commented Nov 14, 2021

Link to proposal
WIP 🚧

Feature addition:
This PR adds the feature that binds the movement/positional changes of nodes enclosed with comment node they are enclosed in. Since the changes are made in GraphEdit module, this feature can be used in VisualScript/VisualShader editors and other modules that are built on top of GraphEdit. This will add to user experience as they don't have to select that particular bunch of nodes every time they want to move it.

Features:

  • Binding position changes of the enclosed nodes and comment node together.
  • Button to toggle on/off this movement feature

Demo

demo.mp4

Bugsquad edit: Closes godotengine/godot-proposals#3548

@theoway theoway force-pushed the moving_nodes_under_comment_node branch 2 times, most recently from 3b21818 to 2de2da1 Compare November 14, 2021 14:51
@Anaxan
Copy link

Anaxan commented Nov 14, 2021

Thoughts from a Visual Script User.

Looks pretty solid. I think it would help to organize and rearrange Visual Scripts.

Is it possible to add a way to identify a comment when zoomed out or looking on the minimap ? Maybe an option to adjust the size of the Comment Title or tint different comment backgrounds different colors. Different background shapes for different comments? Something that stands out from a distance.

Need a build to try out to give better feedback.

@theoway
Copy link
Contributor Author

theoway commented Nov 14, 2021

@Anaxan As far as changing the appearance of comment nodes for better readability is considered, it is out of scope of this PR, as it is only targeted towards making combined movement of comment nodes and the nodes enclosed within them.
I'll fix the build errors, so that you can test it out :)

@theoway
Copy link
Contributor Author

theoway commented Nov 15, 2021

@fire Should I add button/control to enable/disable the feature. Else, it is ready to be merged. Waiting for your reply..

@fire
Copy link
Member

fire commented Nov 15, 2021

I don’t see any reasoning to disable the feature.

@theoway
Copy link
Contributor Author

theoway commented Nov 15, 2021

I don’t see any reasoning to disable the feature.

@fire Neither do I. This PR is ready to be merged then.

@theoway theoway marked this pull request as ready for review November 15, 2021 14:18
@theoway theoway requested a review from a team as a code owner November 15, 2021 14:18
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I agree with the feature proposal, and the code changes seem fairly straightforward 👍

I suggest that maybe @Chaosus would want to give this a quick check as this should be useful for Visual Shaders too.

@Chaosus
Copy link
Member

Chaosus commented Nov 15, 2021

I did such things with comment nodes myself privately, and less elegant than here. I personally like it - it works as expected.

@akien-mga akien-mga merged commit 9c4c724 into godotengine:master Nov 15, 2021
@akien-mga
Copy link
Member

Thanks!

@theoway theoway deleted the moving_nodes_under_comment_node branch November 16, 2021 10:32
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.

Make nodes move together with the comment node that encloses them in GraphEdit
6 participants