This node takes a curve and a point selection and allows you to set the specified (or all) points left/right or both handles to a given type.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Looks great! Just a few smaller comments.
- I wonder if it would make sense to have an NodeWarningType::Info message when there are no Bezier splines in the curve. Maybe it would help someone who expected it to work on poly splines or something.
- NodeGeometrySetCurveHandles would probably make sense, since we'll have other nodes that deal with handle types.
| source/blender/makesrna/intern/rna_nodetree.c | ||
|---|---|---|
| 9447–9466 | I really like your descriptions here! Two comments:
| |
| 9470–9471 |
| |
| source/blender/nodes/geometry/nodes/node_geo_curve_handles.cc | ||
| 16–21 ↗ | (On Diff #39788) | Generally the different modules / prefixes get newlines between them. |
| 55 ↗ | (On Diff #39788) | Thsee parentheses are unnecessary: (params.node().storage) -> params.node().storage |
| 68 ↗ | (On Diff #39788) | handlesd -> handles |
| 81–94 ↗ | (On Diff #39788) | Best to just do this once, rather than in the loop. Also, you can make it a separate function, then you can use return in the switch cases, which is a bit nicer. |
Comment Actions
I'll make a couple small changes when committing.
| source/blender/nodes/geometry/nodes/node_geo_curve_set_handles.cc | ||
|---|---|---|
| 68 | This function doesn't have a return for all possible values of type: BLI_assert_unreachable(); return BezierSpline::HandleType::Auto; | |
| 97 | I changed the variable names here a bit and checked the spline type and continued, just to decrease indentation a bit. | |
