Point-and-click deletion of lofts, shells, and offset planes #4898
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…-of-Lofts-and-Offset-Planes
pierremtb
changed the title
|
@nadr0 @franknoirot this isn't 100% ready yet but I figured you'd want to be in the loop |
This reverts commit 214763c.
…-of-Lofts-and-Offset-Planes
…-of-Lofts-and-Offset-Planes
| }), | ||
| ) | ||
| .await?; | ||
| let id: uuid::Uuid = exec_state.next_uuid(); | ||
| let resp = args | ||
| .send_modeling_cmd( | ||
| id, | ||
| ModelingCmd::from(mcmd::Loft { | ||
| section_ids: sketches.iter().map(|group| group.id).collect(), | ||
| base_curve_index, | ||
| bez_approximate_rational, | ||
| tolerance: LengthUnit(tolerance.unwrap_or(default_tolerance(&args.ctx.settings.units))), | ||
| v_degree, | ||
| }), | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
Discussion started with the engine team to see if we could get an explicit solid_id field as part of the Loft command, this way it we could keep batching this one. See c9f7336
There was a problem hiding this comment.
This is the PR we need merged https://github.com/KittyCAD/engine/pull/2969
|
Taking this back to draft until the changes started at KittyCAD/modeling-api@f95d35d cascade down to engine. See #4898 (comment) |
Closes #4662
Supports:
Atm we can't select internal faces of a shell so scene selection deletion of shells is left out here.
TODO before merge:
Review note:
As mentioned in the code there's a weird case were we can't delete a loft by selection right after we create it, will make a new issue about as I'm tired of over rotating on this PR and want it merged. In the same vein, I wanted to migrate the Delete selection command to a promise actor thing like we have for newer commands, but doing so (see commit history) had a side effect on a the error toast and something else that was breaking existing selection test. Also something for another PR, I'd like to have this merged 😅