Skip to content

Allow creating a VisualShader node group from a selection - #123773

Open
Geometror wants to merge 1 commit into
godotengine:masterfrom
Geometror:make-group-by-sel
Open

Geometror wants to merge 1 commit into
godotengine:masterfrom
Geometror:make-group-by-sel

Conversation

@Geometror

Copy link
Copy Markdown
Member

This is a follow-up to #99404, adding one important UX improvement that speeds up node group creation significantly.
As always, this was a bit more sophisticated than anticipated.

Animation

Detailed changes:

  • Adds a context menu option ("Make Group") for creating a group based on the current selection in the VisualShader editor
    • Replaces the selection after validating (to prevent cycles or forbidden nodes moving in) with a node group instance (everything stays connected, so the operation is seamless); undo/redo works of course
    • Multiple connections from the same output port share one group port; group ports are sorted based on the position of the source nodes to minimize "entanglement"; group port names are based on the inner port they are connected to (with a fallback to that node's caption).
  • Refactoring along the way:
    • Made NodeMenuOptions and ConnectionMenuOptions enum class to avoid the MAKE_GROUP name clash (+ we want to move to enum class in general)
    • Refactor node deletion a bit (use LocalVector instead of List)

Notes:

  • A remaining TODO is to add a shortcut (Ctrl + G) for this, but that might conflict with grouping in the 2D/3D editor (need to investigate)
  • I'd really like to deprecate VisualShaderNodeGroupBase since it's a relic from the past. Ideally the Expression node should be overhauled to use the same mechanisms for port management as node groups.
  • I have a refactor/cleanup PR for the port expansion code in the works which should get rid of ~300 LOC.

AI Disclosure: AI was used as part of a review pass at the end; resulting changes were done manually or in the form of surgical instructions.

@LiveTrower

Copy link
Copy Markdown
Contributor

I was just about to make a proposal about this.

HashSet<int> selected_nodes;
Vector<String> unsupported_nodes_captions;

for (int i = 0; i < graph->get_child_count(); i++) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this use iterate_children?

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.

3 participants