Selection dry-run validation for Shell (#4775)

* WIP: mess with shell selection validation Will eventually fix #4711 * Update from main * WIP: not working yet * Working loft dry run validator * Clean up shell (still not working) * Bump kittycad-modeling-cmds * Clean up * Add logging * Add proper object_id and face_id mapping, still not working for shell * Fix faceId * A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores) * Trigger CI * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores) * Working validation after engine merge; Clean up * Fix codespell * Add pw test * More clean up * Back to basics * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Trigger CI * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Trigger CI * Clean up * Fix tests * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Trigger CI * Remove kcl-samples --------- Co-authored-by: Adam Chalmers <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
KittyCAD · Jan 17, 2025 · 0592d3b · 0592d3b
1 parent 31e4d60
commit 0592d3b
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 2 deletions.
diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts
@@ -1503,6 +1503,7 @@ shellPointAndClickCapCases.forEach(({ shouldPreselect }) => {
         await clickOnCap()
         await page.waitForTimeout(500)
         await cmdBar.progressCmdBar()
+        await page.waitForTimeout(500)
         await cmdBar.progressCmdBar()
         await cmdBar.expectState({
           stage: 'review',
@@ -1523,6 +1524,7 @@ shellPointAndClickCapCases.forEach(({ shouldPreselect }) => {
       await test.step(`Go through the command bar flow with a preselected face (cap)`, async () => {
         await toolbar.shellButton.click()
         await cmdBar.progressCmdBar()
+        await page.waitForTimeout(500)
         await cmdBar.progressCmdBar()
         await cmdBar.expectState({
           stage: 'review',
@@ -1604,6 +1606,7 @@ extrude001 = extrude(40, sketch001)
     await page.waitForTimeout(500)
     await page.keyboard.up('Shift')
     await cmdBar.progressCmdBar()
+    await page.waitForTimeout(500)
     await cmdBar.progressCmdBar()
     await cmdBar.expectState({
       stage: 'review',
@@ -1727,3 +1730,61 @@ shellSketchOnFacesCases.forEach((initialCode, index) => {
     })
   })
 })
+
+test(`Shell dry-run validation rejects sweeps`, async ({
+  context,
+  page,
+  homePage,
+  scene,
+  editor,
+  toolbar,
+  cmdBar,
+}) => {
+  const initialCode = `sketch001 = startSketchOn('YZ')
+  |> circle({
+       center = [0, 0],
+       radius = 500
+     }, %)
+sketch002 = startSketchOn('XZ')
+  |> startProfileAt([0, 0], %)
+  |> xLine(-2000, %)
+sweep001 = sweep({ path = sketch002 }, sketch001)
+`
+  await context.addInitScript((initialCode) => {
+    localStorage.setItem('persistCode', initialCode)
+  }, initialCode)
+  await page.setBodyDimensions({ width: 1000, height: 500 })
+  await homePage.goToModelingScene()
+  await scene.waitForExecutionDone()
+
+  // One dumb hardcoded screen pixel value
+  const testPoint = { x: 500, y: 250 }
+  const [clickOnSweep] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
+
+  await test.step(`Confirm sweep exists`, async () => {
+    await toolbar.closePane('code')
+    await scene.expectPixelColor([231, 231, 231], testPoint, 15)
+  })
+
+  await test.step(`Go through the Shell flow and fail validation with a toast`, async () => {
+    await toolbar.shellButton.click()
+    await cmdBar.expectState({
+      stage: 'arguments',
+      currentArgKey: 'selection',
+      currentArgValue: '',
+      headerArguments: {
+        Selection: '',
+        Thickness: '',
+      },
+      highlightedHeaderArg: 'selection',
+      commandName: 'Shell',
+    })
+    await clickOnSweep()
+    await page.waitForTimeout(500)
+    await cmdBar.progressCmdBar()
+    await expect(
+      page.getByText('Unable to shell with the provided selection')
+    ).toBeVisible()
+    await page.waitForTimeout(1000)
+  })
+})
diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts
@@ -9,7 +9,11 @@ import { Selections } from 'lib/selections'
 import { kclManager } from 'lib/singletons'
 import { err } from 'lib/trap'
 import { modelingMachine, SketchTool } from 'machines/modelingMachine'
-import { loftValidator, revolveAxisValidator } from './validators'
+import {
+  loftValidator,
+  revolveAxisValidator,
+  shellValidator,
+} from './validators'
 
 type OutputFormat = Models['OutputFormat_type']
 type OutputTypeKey = OutputFormat['type']
@@ -351,12 +355,13 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
         selectionTypes: ['cap', 'wall'],
         multiple: true,
         required: true,
-        skip: false,
+        validation: shellValidator,
       },
       thickness: {
         inputType: 'kcl',
         defaultValue: KCL_DEFAULT_LENGTH,
         required: true,
+        // TODO: add dry-run validation on thickness param
       },
     },
   },

diff --git a/src/lib/commandBarConfigs/validators.ts b/src/lib/commandBarConfigs/validators.ts
@@ -153,3 +153,57 @@ export const loftValidator = async ({
     return 'Unable to loft with selected sketches'
   }
 }
+
+export const shellValidator = async ({
+  data,
+}: {
+  data: { selection: Selections }
+}): Promise<boolean | string> => {
+  if (!isSelections(data.selection)) {
+    return 'Unable to shell, selections are missing'
+  }
+
+  // No validation on the faces, filtering is done upstream and we have the dry run validation just below
+  const face_ids = data.selection.graphSelections.flatMap((s) =>
+    s.artifact ? s.artifact.id : []
+  )
+
+  // We don't have the concept of solid3ds in TS yet.
+  // So we're listing out the sweeps as if they were solids and taking the first one, just like in Rust for Shell:
+  // https://github.com/KittyCAD/modeling-app/blob/e61fff115b9fa94aaace6307b1842cc15d41655e/src/wasm-lib/kcl/src/std/shell.rs#L237-L238
+  // TODO: This is one cheap way to make sketch-on-face supported now but will likely fail multiple solids
+  const object_id = engineCommandManager.artifactGraph
+    .values()
+    .find((v) => v.type === 'sweep')?.pathId
+
+  if (!object_id) {
+    return "Unable to shell, couldn't find the solid"
+  }
+
+  const shellCommand = async () => {
+    // TODO: figure out something better than an arbitrarily small value
+    const DEFAULT_THICKNESS: Models['LengthUnit_type'] = 1e-9
+    const DEFAULT_HOLLOW = false
+    const cmdArgs = {
+      face_ids,
+      object_id,
+      hollow: DEFAULT_HOLLOW,
+      shell_thickness: DEFAULT_THICKNESS,
+    }
+    return await engineCommandManager.sendSceneCommand({
+      type: 'modeling_cmd_req',
+      cmd_id: uuidv4(),
+      cmd: {
+        type: 'solid3d_shell_face',
+        ...cmdArgs,
+      },
+    })
+  }
+
+  const attemptShell = await dryRunWrapper(shellCommand)
+  if (attemptShell?.success) {
+    return true
+  }
+
+  return 'Unable to shell with the provided selection'
+}