ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update code mods for extrude so that new top-level constants are created (#2549)

Browse Source 
* Make sketch and extrude produce separate top-level constants

* Fix most tests

* Add a breaking test for sketch on face AST mod

* Use `extrude` instead of `part`

* Implement @Irev-Dev's branch changes from https://github.com/KittyCAD/modeling-app/pull/2472

* Get extrude on face working

* Update incorrect sketch on face test

* Update unit tests

* Fix up E2E test changes

* fmt

* Fix a couple of goofed up test updates

* More specific names for paths to node sent to modelingMachine

* Bump down playwright workers for now

* Slightly more explicit type coercion

* Update snapshot tests

* Missed one other new flow test that wasn't updated to use "sketch001"

* Typo

* Damn missed one more sorry

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* Re-run CI

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* I think the multiple sketches test reverted from under me

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Frank Noirot
2024-06-04 13:57:01 -04:00
committed by GitHub
parent e46aca4992
commit c9800a58d0
17 changed files with 319 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
src/clientSideScene/sceneEntities.ts
View File

@ -93,7 +93,10 @@ import { createGridHelper, orthoScale, perspScale } from './helpers'
import { Models } from '@kittycad/lib'
import { uuidv4 } from 'lib/utils'
import { SegmentOverlayPayload, SketchDetails } from 'machines/modelingMachine'
import { EngineCommandManager } from 'lang/std/engineConnection'
import {
ArtifactMapCommand,
EngineCommandManager,
} from 'lang/std/engineConnection'
import {
getRectangleCallExpressions,
updateRectangleSketch,
@ -759,14 +762,6 @@ export class SceneEntities {
_ast = parse(recast(_ast))
console.log('onClick', {
sketchInit: sketchInit,
_ast,
x,
y,
truncatedAst,
})
// Update the primary AST and unequip the rectangle tool
await kclManager.executeAstMock(_ast)
sceneInfra.modelingSend({ type: 'CancelSketch' })
@ -1422,6 +1417,30 @@ export class SceneEntities {
return ['plane', entity_id]
}
const artifact = this.engineCommandManager.artifactMap[entity_id]
// If we clicked on an extrude wall, we climb up the parent Id
// to get the sketch profile's face ID. If we clicked on an endcap,
// we already have it.
const targetId =
'additionalData' in artifact &&
artifact.additionalData?.type === 'cap'
? entity_id
: artifact.parentId
// tsc cannot infer that target can have extrusions
// from the commandType (why?) so we need to cast it
const target = this.engineCommandManager.artifactMap?.[
targetId || ''
] as ArtifactMapCommand & { extrusions?: string[] }
// TODO: We get the first extrusion command ID,
// which is fine while backend systems only support one extrusion.
// but we need to more robustly handle resolving to the correct extrusion
// if there are multiple.
const extrusions =
this.engineCommandManager.artifactMap?.[
target?.extrusions?.[0] || ''
]
if (artifact?.commandType !== 'solid3d_get_extrusion_face_info')
return ['other', entity_id]
@ -1429,10 +1448,13 @@ export class SceneEntities {
if (!faceInfo?.origin || !faceInfo?.z_axis || !faceInfo?.y_axis)
return ['other', entity_id]
const { z_axis, y_axis, origin } = faceInfo
const pathToNode = getNodePathFromSourceRange(
const sketchPathToNode = getNodePathFromSourceRange(
kclManager.ast,
artifact.range
)
const extrudePathToNode = extrusions?.range
? getNodePathFromSourceRange(kclManager.ast, extrusions.range)
: []
sceneInfra.modelingSend({
type: 'Select default plane',
@ -1443,7 +1465,8 @@ export class SceneEntities {
position: [origin.x, origin.y, origin.z].map(
(num) => num / sceneInfra._baseUnitMultiplier
) as [number, number, number],
extrudeSegmentPathToNode: pathToNode,
sketchPathToNode,
extrudePathToNode,
cap:
artifact?.additionalData?.type === 'cap'
? artifact.additionalData.info
@ -1455,7 +1478,6 @@ export class SceneEntities {
}
const faceResult = await checkExtrudeFaceClick()
console.log('faceResult', faceResult)
if (faceResult[0] === 'face') return
if (!args || !args.intersects?.[0]) return

3
src/components/ModelingMachineProvider.tsx
View File

@ -453,7 +453,8 @@ export const ModelingMachineProvider = ({
const { modifiedAst, pathToNode: pathToNewSketchNode } =
sketchOnExtrudedFace(
kclManager.ast,
data.extrudeSegmentPathToNode,
data.sketchPathToNode,
data.extrudePathToNode,
kclManager.programMemory,
data.cap
)

113
src/lang/modifyAst.test.ts
View File

@ -125,7 +125,7 @@ describe('Testing addSketchTo', () => {
'yz'
)
const str = recast(result.modifiedAst)
expect(str).toBe(`const part001 = startSketchOn('YZ')
expect(str).toBe(`const sketch001 = startSketchOn('YZ')
|> startProfileAt('default', %)
|> line('default', %)
`)
@ -291,14 +291,25 @@ describe('testing sketchOnExtrudedFace', () => {
|> extrude(5 + 7, %)`
const ast = parse(code)
const programMemory = await enginelessExecutor(ast)
const snippet = `line([9.7, 9.19], %)`
const range: [number, number] = [
code.indexOf(snippet),
code.indexOf(snippet) + snippet.length,
const segmentSnippet = `line([9.7, 9.19], %)`
const segmentRange: [number, number] = [
code.indexOf(segmentSnippet),
code.indexOf(segmentSnippet) + segmentSnippet.length,
]
const pathToNode = getNodePathFromSourceRange(ast, range)
const segmentPathToNode = getNodePathFromSourceRange(ast, segmentRange)
const extrudeSnippet = `extrude(5 + 7, %)`
const extrudeRange: [number, number] = [
code.indexOf(extrudeSnippet),
code.indexOf(extrudeSnippet) + extrudeSnippet.length,
]
const extrudePathToNode = getNodePathFromSourceRange(ast, extrudeRange)
const { modifiedAst } = sketchOnExtrudedFace(ast, pathToNode, programMemory)
const { modifiedAst } = sketchOnExtrudedFace(
ast,
segmentPathToNode,
extrudePathToNode,
programMemory
)
const newCode = recast(modifiedAst)
expect(newCode).toContain(`const part001 = startSketchOn('-XZ')
|> startProfileAt([3.58, 2.06], %)
@ -306,7 +317,7 @@ describe('testing sketchOnExtrudedFace', () => {
|> line([8.62, -9.57], %)
|> close(%)
|> extrude(5 + 7, %)
const part002 = startSketchOn(part001, 'seg01')`)
const sketch001 = startSketchOn(part001, 'seg01')`)
})
test('it should be able to extrude on close segments', async () => {
const code = `const part001 = startSketchOn('-XZ')
@ -317,14 +328,25 @@ const part002 = startSketchOn(part001, 'seg01')`)
|> extrude(5 + 7, %)`
const ast = parse(code)
const programMemory = await enginelessExecutor(ast)
const snippet = `close(%)`
const range: [number, number] = [
code.indexOf(snippet),
code.indexOf(snippet) + snippet.length,
const segmentSnippet = `close(%)`
const segmentRange: [number, number] = [
code.indexOf(segmentSnippet),
code.indexOf(segmentSnippet) + segmentSnippet.length,
]
const pathToNode = getNodePathFromSourceRange(ast, range)
const segmentPathToNode = getNodePathFromSourceRange(ast, segmentRange)
const extrudeSnippet = `extrude(5 + 7, %)`
const extrudeRange: [number, number] = [
code.indexOf(extrudeSnippet),
code.indexOf(extrudeSnippet) + extrudeSnippet.length,
]
const extrudePathToNode = getNodePathFromSourceRange(ast, extrudeRange)
const { modifiedAst } = sketchOnExtrudedFace(ast, pathToNode, programMemory)
const { modifiedAst } = sketchOnExtrudedFace(
ast,
segmentPathToNode,
extrudePathToNode,
programMemory
)
const newCode = recast(modifiedAst)
expect(newCode).toContain(`const part001 = startSketchOn('-XZ')
|> startProfileAt([3.58, 2.06], %)
@ -332,7 +354,7 @@ const part002 = startSketchOn(part001, 'seg01')`)
|> line([8.62, -9.57], %)
|> close(%, 'seg01')
|> extrude(5 + 7, %)
const part002 = startSketchOn(part001, 'seg01')`)
const sketch001 = startSketchOn(part001, 'seg01')`)
})
test('it should be able to extrude on start-end caps', async () => {
const code = `const part001 = startSketchOn('-XZ')
@ -343,16 +365,23 @@ const part002 = startSketchOn(part001, 'seg01')`)
|> extrude(5 + 7, %)`
const ast = parse(code)
const programMemory = await enginelessExecutor(ast)
const snippet = `startProfileAt([3.58, 2.06], %)`
const range: [number, number] = [
code.indexOf(snippet),
code.indexOf(snippet) + snippet.length,
const sketchSnippet = `startProfileAt([3.58, 2.06], %)`
const sketchRange: [number, number] = [
code.indexOf(sketchSnippet),
code.indexOf(sketchSnippet) + sketchSnippet.length,
]
const pathToNode = getNodePathFromSourceRange(ast, range)
const sketchPathToNode = getNodePathFromSourceRange(ast, sketchRange)
const extrudeSnippet = `extrude(5 + 7, %)`
const extrudeRange: [number, number] = [
code.indexOf(extrudeSnippet),
code.indexOf(extrudeSnippet) + extrudeSnippet.length,
]
const extrudePathToNode = getNodePathFromSourceRange(ast, extrudeRange)
const { modifiedAst } = sketchOnExtrudedFace(
ast,
pathToNode,
sketchPathToNode,
extrudePathToNode,
programMemory,
'end'
)
@ -363,7 +392,47 @@ const part002 = startSketchOn(part001, 'seg01')`)
|> line([8.62, -9.57], %)
|> close(%)
|> extrude(5 + 7, %)
const part002 = startSketchOn(part001, 'END')`)
const sketch001 = startSketchOn(part001, 'END')`)
})
test('it should ensure that the new sketch is inserted after the extrude', async () => {
const code = `const sketch001 = startSketchOn('-XZ')
|> startProfileAt([3.29, 7.86], %)
|> line([2.48, 2.44], %)
|> line([2.66, 1.17], %)
|> line([3.75, 0.46], %)
|> line([4.99, -0.46], %)
|> line([3.3, -2.12], %)
|> line([2.16, -3.33], %)
|> line([0.85, -3.08], %)
|> line([-0.18, -3.36], %)
|> line([-3.86, -2.73], %)
|> line([-17.67, 0.85], %)
|> close(%)
const part001 = extrude(5 + 7, sketch001)`
const ast = parse(code)
const programMemory = await enginelessExecutor(ast)
const segmentSnippet = `line([4.99, -0.46], %)`
const segmentRange: [number, number] = [
code.indexOf(segmentSnippet),
code.indexOf(segmentSnippet) + segmentSnippet.length,
]
const segmentPathToNode = getNodePathFromSourceRange(ast, segmentRange)
const extrudeSnippet = `extrude(5 + 7, sketch001)`
const extrudeRange: [number, number] = [
code.indexOf(extrudeSnippet),
code.indexOf(extrudeSnippet) + extrudeSnippet.length,
]
const extrudePathToNode = getNodePathFromSourceRange(ast, extrudeRange)
const { modifiedAst } = sketchOnExtrudedFace(
ast,
segmentPathToNode,
extrudePathToNode,
programMemory
)
const newCode = recast(modifiedAst)
expect(newCode).toContain(`const part001 = extrude(5 + 7, sketch001)
const sketch002 = startSketchOn(part001, 'seg01')`)
})
})

54
src/lang/modifyAst.ts
View File

@ -34,6 +34,7 @@ import {
} from './std/sketchcombos'
import { DefaultPlaneStr } from 'clientSideScene/sceneEntities'
import { isOverlap, roundOff } from 'lib/utils'
import { KCL_DEFAULT_CONSTANT_PREFIXES } from 'lib/constants'
import { ConstrainInfo } from './std/stdTypes'
export function startSketchOnDefault(
@ -42,7 +43,8 @@ export function startSketchOnDefault(
name = ''
): { modifiedAst: Program; id: string; pathToNode: PathToNode } {
const _node = { ...node }
const _name = name || findUniqueName(node, 'part')
const _name =
name || findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SKETCH)
const startSketchOn = createCallExpressionStdLib('startSketchOn', [
createLiteral(axis),
@ -109,7 +111,8 @@ export function addSketchTo(
name = ''
): { modifiedAst: Program; id: string; pathToNode: PathToNode } {
const _node = { ...node }
const _name = name || findUniqueName(node, 'part')
const _name =
name || findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SKETCH)
const startSketchOn = createCallExpressionStdLib('startSketchOn', [
createLiteral(axis.toUpperCase()),
@ -242,7 +245,7 @@ export function mutateObjExpProp(
export function extrudeSketch(
node: Program,
pathToNode: PathToNode,
shouldPipe = true,
shouldPipe = false,
distance = createLiteral(4) as Value
): {
modifiedAst: Program
@ -293,12 +296,22 @@ export function extrudeSketch(
pathToExtrudeArg,
}
}
const name = findUniqueName(node, 'part')
// We're not creating a pipe expression,
// but rather a separate constant for the extrusion
const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.EXTRUDE)
const VariableDeclaration = createVariableDeclaration(name, extrudeCall)
_node.body.splice(_node.body.length, 0, VariableDeclaration)
const sketchIndexInPathToNode =
pathToDecleration.findIndex((a) => a[0] === 'body') + 1
const sketchIndexInBody = pathToDecleration[
sketchIndexInPathToNode
][0] as number
_node.body.splice(sketchIndexInBody + 1, 0, VariableDeclaration)
const pathToExtrudeArg: PathToNode = [
['body', ''],
[_node.body.length, 'index'],
[sketchIndexInBody + 1, 'index'],
['declarations', 'VariableDeclaration'],
[0, 'index'],
['init', 'VariableDeclarator'],
@ -306,7 +319,7 @@ export function extrudeSketch(
[0, 'index'],
]
return {
modifiedAst: node,
modifiedAst: _node,
pathToNode: [...pathToNode.slice(0, -1), [-1, 'index']],
pathToExtrudeArg,
}
@ -314,31 +327,41 @@ export function extrudeSketch(
export function sketchOnExtrudedFace(
node: Program,
pathToNode: PathToNode,
sketchPathToNode: PathToNode,
extrudePathToNode: PathToNode,
programMemory: ProgramMemory,
cap: 'none' | 'start' | 'end' = 'none'
): { modifiedAst: Program; pathToNode: PathToNode } {
let _node = { ...node }
const newSketchName = findUniqueName(node, 'part')
const newSketchName = findUniqueName(
node,
KCL_DEFAULT_CONSTANT_PREFIXES.SKETCH
)
const { node: oldSketchNode } = getNodeFromPath<VariableDeclarator>(
_node,
pathToNode,
sketchPathToNode,
'VariableDeclarator',
true
)
const oldSketchName = oldSketchNode.id.name
const { node: expression } = getNodeFromPath<CallExpression>(
_node,
pathToNode,
sketchPathToNode,
'CallExpression'
)
const { node: extrudeVarDec } = getNodeFromPath<VariableDeclarator>(
_node,
extrudePathToNode,
'VariableDeclarator'
)
const extrudeName = extrudeVarDec.id?.name
let _tag = ''
if (cap === 'none') {
const { modifiedAst, tag } = addTagForSketchOnFace(
{
previousProgramMemory: programMemory,
pathToNode,
pathToNode: sketchPathToNode,
node: _node,
},
expression.callee.name
@ -352,13 +375,16 @@ export function sketchOnExtrudedFace(
const newSketch = createVariableDeclaration(
newSketchName,
createCallExpressionStdLib('startSketchOn', [
createIdentifier(oldSketchName),
createIdentifier(extrudeName ? extrudeName : oldSketchName),
createLiteral(_tag),
]),
'const'
)
const expressionIndex = pathToNode[1][0] as number
const expressionIndex = Math.max(
sketchPathToNode[1][0] as number,
extrudePathToNode[1][0] as number
)
_node.body.splice(expressionIndex + 1, 0, newSketch)
const newpathToNode: PathToNode = [
['body', ''],

97
src/lang/std/engineConnection.ts
View File

@ -12,22 +12,43 @@ let lastMessage = ''
// TODO(paultag): This ought to be tweakable.
const pingIntervalMs = 10000
interface CommandInfo {
commandType: CommandTypes
range: SourceRange
pathToNode: PathToNode
parentId?: string
additionalData?:
| {
type: 'cap'
info: 'start' | 'end'
}
| {
type: 'batch-ids'
ids: string[]
info?: null
}
}
type CommandTypes = Models['ModelingCmd_type']['type'] | 'batch'
type CommandInfo =
| {
commandType: 'extrude'
// commandType: CommandTypes
range: SourceRange
pathToNode: PathToNode
/// uuid of the entity to extrude
target: string
parentId?: string
}
| {
commandType: 'start_path'
// commandType: CommandTypes
range: SourceRange
pathToNode: PathToNode
/// uuid of the entity that have been extruded
extrusions: string[]
parentId?: string
}
| {
commandType: CommandTypes
range: SourceRange
pathToNode: PathToNode
parentId?: string
additionalData?:
| {
type: 'cap'
info: 'start' | 'end'
}
| {
type: 'batch-ids'
ids: string[]
info?: null
}
}
function isHighlightSetEntity_type(
data: any
@ -38,13 +59,13 @@ function isHighlightSetEntity_type(
type WebSocketResponse = Models['WebSocketResponse_type']
type OkWebSocketResponseData = Models['OkWebSocketResponseData_type']
interface ResultCommand extends CommandInfo {
type ResultCommand = CommandInfo & {
type: 'result'
data: any
raw: WebSocketResponse
headVertexId?: string
}
interface FailedCommand extends CommandInfo {
type FailedCommand = CommandInfo & {
type: 'failed'
errors: Models['FailureWebSocketResponse_type']['errors']
}
@ -57,12 +78,14 @@ interface ResolveCommand {
data?: Models['OkModelingCmdResponse_type']
errors?: Models['FailureWebSocketResponse_type']['errors']
}
interface PendingCommand extends CommandInfo {
type PendingCommand = CommandInfo & {
type: 'pending'
promise: Promise<any>
resolve: (val: ResolveCommand) => void
}
export type ArtifactMapCommand = ResultCommand | PendingCommand | FailedCommand
/**
* The ArtifactMap is a client-side representation of the artifacts that
* have been sent to the server-side engine. It is used to keep track of
@ -72,7 +95,7 @@ interface PendingCommand extends CommandInfo {
* lines of KCL code that generated them.
*/
export interface ArtifactMap {
[commandId: string]: ResultCommand | PendingCommand | FailedCommand
[commandId: string]: ArtifactMapCommand
}
interface NewTrackArgs {
@ -988,8 +1011,6 @@ class EngineConnection extends EventTarget {
export type EngineCommand = Models['WebSocketRequest_type']
type ModelTypes = Models['OkModelingCmdResponse_type']['type']
type CommandTypes = Models['ModelingCmd_type']['type'] | 'batch'
type UnreliableResponses = Extract<
Models['OkModelingCmdResponse_type'],
{ type: 'highlight_set_entity' | 'camera_drag_move' }
@ -1814,9 +1835,12 @@ export class EngineCommandManager extends EventTarget {
if (command.type === 'solid3d_get_extrusion_face_info') {
const edgeArtifact = this.artifactMap[command.edge_id]
// edges's parent id is to the original "start_path" artifact
if (edgeArtifact?.parentId) return edgeArtifact.parentId
if (edgeArtifact && edgeArtifact.parentId) {
return edgeArtifact.parentId
}
}
if (command.type === 'close_path') return command.path_id
if (command.type === 'extrude') return command.target
// handle other commands that have a parent here
}
const pathToNode = ast
@ -1831,6 +1855,33 @@ export class EngineCommandManager extends EventTarget {
promise,
resolve,
}
if (command.type === 'extrude') {
this.artifactMap[id] = {
range: range || [0, 0],
pathToNode,
type: 'pending',
commandType: 'extrude',
parentId: getParentId(),
promise,
target: command.target,
resolve,
}
const target = this.artifactMap[command.target]
if (target.commandType === 'start_path') {
// tsc cannot infer that target can have extrusions
// from the commandType (why?) so we need to cast it
const typedTarget = target as (
| PendingCommand
| ResultCommand
| FailedCommand
) & { extrusions?: string[] }
if (typedTarget?.extrusions?.length) {
typedTarget.extrusions.push(id)
} else {
typedTarget.extrusions = [id]
}
}
}
return promise
}
async handlePendingBatchCommand(

8
src/lib/constants.ts
View File

@ -44,5 +44,13 @@ export const RELEVANT_FILE_TYPES = [
] as const
/** The default name for a tutorial project */
export const ONBOARDING_PROJECT_NAME = 'Tutorial Project $nn'
/**
* The default starting constant name for various modeling operations.
* These are used to generate unique names for new objects.
* */
export const KCL_DEFAULT_CONSTANT_PREFIXES = {
SKETCH: 'sketch',
EXTRUDE: 'extrude',
} as const
/** The default KCL length expression */
export const KCL_DEFAULT_LENGTH = `5`

5
src/machines/modelingMachine.ts
View File

@ -145,7 +145,8 @@ export type ModelingMachineEvent =
| {
type: 'extrudeFace'
position: [number, number, number]
extrudeSegmentPathToNode: PathToNode
sketchPathToNode: PathToNode
extrudePathToNode: PathToNode
cap: 'start' | 'end' | 'none'
faceId: string
}
@ -883,7 +884,7 @@ export const modelingMachine = createMachine(
const { modifiedAst, pathToExtrudeArg } = extrudeSketch(
ast,
pathToNode,
true,
false,
'variableName' in distance
? distance.variableIdentifierAst
: distance.valueAst