[BrushGraph 5/7] Add complex node fields#64
[BrushGraph 5/7] Add complex node fields#64maxmmitchell wants to merge 3 commits intobrush-graph/4-nodesfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements UI fields for various brush graph nodes, including behavior, color, family, and texture layers. Feedback highlights missing unit conversions for angle ranges in polar target nodes and performance concerns regarding allocations within the response curve's draw loop. Other suggestions include removing unused variables, consolidating duplicated logic for source types, and ensuring developer comments are consistently available across all behavior node types.
| if (nodeCase == ProtoBrushBehavior.Node.NodeCase.TARGET_NODE || | ||
| nodeCase == ProtoBrushBehavior.Node.NodeCase.POLAR_TARGET_NODE) { |
There was a problem hiding this comment.
The developerComment field is currently only displayed for TARGET_NODE and POLAR_TARGET_NODE. Since developerComment is a common property of the NodeData.Behavior class, it should likely be available for all behavior node types to ensure consistency and allow users to document any node in the graph.
There was a problem hiding this comment.
developerComment exists at the Behavior level in proto, not at the node level. Therefore it only makes sense to have one comment per behavior, so the other nodes have no need for comments, it would just be duplicated and changing it would need to update all the nodes -- no good. This does present an interesting challenge for terminal nodes which take multiple inputs, thereby representing multiple behaviors -- perhaps they should surface multiple comment fields, one per input. Though each input could technically represent several behaviors, so this too is brittle. Needs more thought on how to best display.
618cc67 to
d78bbc2
Compare
4b2402e to
6f34b89
Compare
d78bbc2 to
7d9bfce
Compare
Description
This is the fifth PR in the Brush Graph stack. It completes the node field editors by adding the more complex ones.
Details
Dependencies
brush-graph/4-nodes