diff --git a/docs/kcl/std.json b/docs/kcl/std.json index a2b13c29d..5ff450f9a 100644 --- a/docs/kcl/std.json +++ b/docs/kcl/std.json @@ -27979,9 +27979,33 @@ }, { "name": "tag", - "type": "String", + "type": "SketchOnFaceTag", "schema": { - "type": "string", + "description": "A tag for sketch on face.", + "anyOf": [ + { + "oneOf": [ + { + "description": "The start face as in before you extruded. This could also be known as the bottom face. But we do not call it bottom because it would be the top face if you extruded it in the opposite direction or flipped the camera.", + "type": "string", + "enum": [ + "start" + ] + }, + { + "description": "The end face after you extruded. This could also be known as the top face. But we do not call it top because it would be the bottom face if you extruded it in the opposite direction or flipped the camera.", + "type": "string", + "enum": [ + "end" + ] + } + ] + }, + { + "description": "A string tag for the face you want to sketch on.", + "type": "string" + } + ], "nullable": true }, "required": true diff --git a/docs/kcl/std.md b/docs/kcl/std.md index 07175263d..6d63ed6e0 100644 --- a/docs/kcl/std.md +++ b/docs/kcl/std.md @@ -5149,7 +5149,7 @@ Start a sketch on a specific plane or face. ``` -startSketchOn(data: SketchData, tag: String) -> SketchSurface +startSketchOn(data: SketchData, tag: SketchOnFaceTag) -> SketchSurface ``` #### Arguments @@ -5239,7 +5239,11 @@ startSketchOn(data: SketchData, tag: String) -> SketchSurface }, } ``` -* `tag`: `String` +* `tag`: `SketchOnFaceTag` - A tag for sketch on face. +``` +"start" | "end" | +string +``` #### Returns diff --git a/src/wasm-lib/kcl/src/executor.rs b/src/wasm-lib/kcl/src/executor.rs index 959705810..36533c9ea 100644 --- a/src/wasm-lib/kcl/src/executor.rs +++ b/src/wasm-lib/kcl/src/executor.rs @@ -862,7 +862,7 @@ impl ExtrudeSurface { pub fn get_name(&self) -> String { match self { - ExtrudeSurface::ExtrudePlane(ep) => ep.name.clone(), + ExtrudeSurface::ExtrudePlane(ep) => ep.name.to_string(), } } diff --git a/src/wasm-lib/kcl/src/fs/mod.rs b/src/wasm-lib/kcl/src/fs/mod.rs index 6a76a0a87..b321bb523 100644 --- a/src/wasm-lib/kcl/src/fs/mod.rs +++ b/src/wasm-lib/kcl/src/fs/mod.rs @@ -8,12 +8,11 @@ pub use local::FileManager; #[cfg(target_arch = "wasm32")] #[cfg(not(test))] pub mod wasm; +use anyhow::Result; #[cfg(target_arch = "wasm32")] #[cfg(not(test))] pub use wasm::FileManager; -use anyhow::Result; - #[async_trait::async_trait(?Send)] pub trait FileSystem: Clone { /// Read a file from the local file system. diff --git a/src/wasm-lib/kcl/src/std/extrude.rs b/src/wasm-lib/kcl/src/std/extrude.rs index 71ff71d78..686da0ea3 100644 --- a/src/wasm-lib/kcl/src/std/extrude.rs +++ b/src/wasm-lib/kcl/src/std/extrude.rs @@ -123,7 +123,10 @@ async fn inner_extrude(length: f64, sketch_group: Box, args: Args) } Ok(Box::new(ExtrudeGroup { - id, + // Ok so you would think that the id would be the id of the extrude group, + // that we passed in to the function, but it's actually the id of the + // sketch group. + id: sketch_group.id, value: new_value, height: length, position: sketch_group.position, diff --git a/src/wasm-lib/kcl/src/std/mod.rs b/src/wasm-lib/kcl/src/std/mod.rs index f81e27bd0..aa13d5558 100644 --- a/src/wasm-lib/kcl/src/std/mod.rs +++ b/src/wasm-lib/kcl/src/std/mod.rs @@ -20,9 +20,9 @@ use parse_display::{Display, FromStr}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use self::kcl_stdlib::KclStdLibFn; +use self::{kcl_stdlib::KclStdLibFn, sketch::SketchOnFaceTag}; use crate::{ - ast::types::{parse_json_number_as_f64, parse_json_value_as_string}, + ast::types::parse_json_number_as_f64, docs::StdLibFn, engine::EngineManager, errors::{KclError, KclErrorDetails}, @@ -406,7 +406,9 @@ impl Args { } } - fn get_data_and_optional_tag(&self) -> Result<(T, Option), KclError> { + fn get_data_and_optional_tag( + &self, + ) -> Result<(T, Option), KclError> { let first_value = self .args .first() @@ -426,8 +428,13 @@ impl Args { })?; if let Some(second_value) = self.args.get(1) { - let tag = parse_json_value_as_string(&second_value.get_json_value()?); - Ok((data, tag)) + let tag: SketchOnFaceTag = serde_json::from_value(second_value.get_json_value()?).map_err(|e| { + KclError::Type(KclErrorDetails { + message: format!("Failed to deserialize SketchOnFaceTag from JSON: {}", e), + source_ranges: vec![self.source_range], + }) + })?; + Ok((data, Some(tag))) } else { Ok((data, None)) } diff --git a/src/wasm-lib/kcl/src/std/sketch.rs b/src/wasm-lib/kcl/src/std/sketch.rs index 21e96bda0..e5d247571 100644 --- a/src/wasm-lib/kcl/src/std/sketch.rs +++ b/src/wasm-lib/kcl/src/std/sketch.rs @@ -4,6 +4,7 @@ use anyhow::Result; use derive_docs::stdlib; use kittycad::types::{Angle, ModelingCmd, Point3D}; use kittycad_execution_plan_macros::ExecutionPlanValue; +use parse_display::{Display, FromStr}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -813,7 +814,7 @@ impl SketchSurface { /// Start a sketch on a specific plane or face. pub async fn start_sketch_on(args: Args) -> Result { - let (data, tag): (SketchData, Option) = args.get_data_and_optional_tag()?; + let (data, tag): (SketchData, Option) = args.get_data_and_optional_tag()?; match inner_start_sketch_on(data, tag, args).await? { SketchSurface::Plane(plane) => Ok(MemoryItem::Plane(plane)), @@ -825,7 +826,11 @@ pub async fn start_sketch_on(args: Args) -> Result { #[stdlib { name = "startSketchOn", }] -async fn inner_start_sketch_on(data: SketchData, tag: Option, args: Args) -> Result { +async fn inner_start_sketch_on( + data: SketchData, + tag: Option, + args: Args, +) -> Result { match data { SketchData::Plane(plane_data) => { let plane = start_sketch_on_plane(plane_data, args).await?; @@ -838,26 +843,72 @@ async fn inner_start_sketch_on(data: SketchData, tag: Option, args: Args source_ranges: vec![args.source_range], })); }; - let face = start_sketch_on_face(extrude_group, &tag, args).await?; + let face = start_sketch_on_face(extrude_group, tag, args).await?; Ok(SketchSurface::Face(face)) } } } -async fn start_sketch_on_face(extrude_group: Box, tag: &str, args: Args) -> Result, KclError> { - let extrude_plane = extrude_group - .value - .iter() - .find_map(|extrude_surface| match extrude_surface { - ExtrudeSurface::ExtrudePlane(extrude_plane) if extrude_plane.name == tag => Some(extrude_plane), - ExtrudeSurface::ExtrudePlane(_) => None, - }) - .ok_or_else(|| { +/// A tag for sketch on face. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, FromStr, Display)] +#[ts(export)] +#[serde(rename_all = "snake_case", untagged)] +#[display("{0}")] +pub enum SketchOnFaceTag { + StartOrEnd(StartOrEnd), + /// A string tag for the face you want to sketch on. + String(String), +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, FromStr, Display)] +#[ts(export)] +#[serde(rename_all = "snake_case")] +#[display(style = "snake_case")] +pub enum StartOrEnd { + /// The start face as in before you extruded. This could also be known as the bottom + /// face. But we do not call it bottom because it would be the top face if you + /// extruded it in the opposite direction or flipped the camera. + #[serde(rename = "start", alias = "START")] + Start, + /// The end face after you extruded. This could also be known as the top + /// face. But we do not call it top because it would be the bottom face if you + /// extruded it in the opposite direction or flipped the camera. + #[serde(rename = "end", alias = "END")] + End, +} + +async fn start_sketch_on_face( + extrude_group: Box, + tag: SketchOnFaceTag, + args: Args, +) -> Result, KclError> { + let extrude_plane_id = match tag { + SketchOnFaceTag::String(ref s) => extrude_group + .value + .iter() + .find_map(|extrude_surface| match extrude_surface { + ExtrudeSurface::ExtrudePlane(extrude_plane) if extrude_plane.name == *s => Some(extrude_plane.face_id), + ExtrudeSurface::ExtrudePlane(_) => None, + }) + .ok_or_else(|| { + KclError::Type(KclErrorDetails { + message: format!("Expected a face with the tag `{}`", tag), + source_ranges: vec![args.source_range], + }) + })?, + SketchOnFaceTag::StartOrEnd(StartOrEnd::Start) => extrude_group.start_cap_id.ok_or_else(|| { KclError::Type(KclErrorDetails { - message: format!("Expected a face with the tag `{}`", tag), + message: "Expected a start face to sketch on".to_string(), source_ranges: vec![args.source_range], }) - })?; + })?, + SketchOnFaceTag::StartOrEnd(StartOrEnd::End) => extrude_group.end_cap_id.ok_or_else(|| { + KclError::Type(KclErrorDetails { + message: "Expected an end face to sketch on".to_string(), + source_ranges: vec![args.source_range], + }) + })?, + }; // Enter sketch mode on the face. let id = uuid::Uuid::new_v4(); @@ -866,7 +917,7 @@ async fn start_sketch_on_face(extrude_group: Box, tag: &str, args: ModelingCmd::EnableSketchMode { animated: false, ortho: false, - entity_id: extrude_plane.face_id, + entity_id: extrude_plane_id, }, ) .await?; @@ -1645,4 +1696,43 @@ mod tests { let data: PlaneData = serde_json::from_str(&str_json).unwrap(); assert_eq!(data, PlaneData::NegXZ); } + + #[test] + fn test_deserialize_sketch_on_face_tag() { + let data = "start"; + let mut str_json = serde_json::to_string(&data).unwrap(); + assert_eq!(str_json, "\"start\""); + + str_json = "\"end\"".to_string(); + let data: crate::std::sketch::SketchOnFaceTag = serde_json::from_str(&str_json).unwrap(); + assert_eq!( + data, + crate::std::sketch::SketchOnFaceTag::StartOrEnd(crate::std::sketch::StartOrEnd::End) + ); + + str_json = "\"thing\"".to_string(); + let data: crate::std::sketch::SketchOnFaceTag = serde_json::from_str(&str_json).unwrap(); + assert_eq!(data, crate::std::sketch::SketchOnFaceTag::String("thing".to_string())); + + str_json = "\"END\"".to_string(); + let data: crate::std::sketch::SketchOnFaceTag = serde_json::from_str(&str_json).unwrap(); + assert_eq!( + data, + crate::std::sketch::SketchOnFaceTag::StartOrEnd(crate::std::sketch::StartOrEnd::End) + ); + + str_json = "\"start\"".to_string(); + let data: crate::std::sketch::SketchOnFaceTag = serde_json::from_str(&str_json).unwrap(); + assert_eq!( + data, + crate::std::sketch::SketchOnFaceTag::StartOrEnd(crate::std::sketch::StartOrEnd::Start) + ); + + str_json = "\"START\"".to_string(); + let data: crate::std::sketch::SketchOnFaceTag = serde_json::from_str(&str_json).unwrap(); + assert_eq!( + data, + crate::std::sketch::SketchOnFaceTag::StartOrEnd(crate::std::sketch::StartOrEnd::Start) + ); + } } diff --git a/src/wasm-lib/tests/executor/main.rs b/src/wasm-lib/tests/executor/main.rs index c898d7c3c..ae84f4d78 100644 --- a/src/wasm-lib/tests/executor/main.rs +++ b/src/wasm-lib/tests/executor/main.rs @@ -91,6 +91,62 @@ const part002 = startSketchOn(part001, "here") twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face.png", &result, 0.999); } +#[tokio::test(flavor = "multi_thread")] +async fn serial_test_sketch_on_face_start() { + let code = r#"fn cube = (pos, scale) => { + const sg = startSketchOn('XY') + |> startProfileAt(pos, %) + |> line([0, scale], %) + |> line([scale, 0], %) + |> line([0, -scale], %) + + return sg +} +const part001 = cube([0,0], 20) + |> close(%) + |> extrude(20, %) + +const part002 = startSketchOn(part001, "start") + |> startProfileAt([0, 0], %) + |> line([0, 10], %) + |> line([10, 0], %) + |> line([0, -10], %) + |> close(%) + |> extrude(5, %) +"#; + + let result = execute_and_snapshot(code).await.unwrap(); + twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face_start.png", &result, 0.999); +} + +#[tokio::test(flavor = "multi_thread")] +async fn serial_test_sketch_on_face_end() { + let code = r#"fn cube = (pos, scale) => { + const sg = startSketchOn('XY') + |> startProfileAt(pos, %) + |> line([0, scale], %) + |> line([scale, 0], %) + |> line([0, -scale], %) + + return sg +} +const part001 = cube([0,0], 20) + |> close(%) + |> extrude(20, %) + +const part002 = startSketchOn(part001, "END") + |> startProfileAt([0, 0], %) + |> line([0, 10], %) + |> line([10, 0], %) + |> line([0, -10], %) + |> close(%) + |> extrude(5, %) +"#; + + let result = execute_and_snapshot(code).await.unwrap(); + twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face_end.png", &result, 0.999); +} + #[tokio::test(flavor = "multi_thread")] async fn serial_test_execute_with_function_sketch() { let code = r#"fn box = (h, l, w) => { diff --git a/src/wasm-lib/tests/executor/outputs/sketch_on_face_end.png b/src/wasm-lib/tests/executor/outputs/sketch_on_face_end.png new file mode 100644 index 000000000..c51b5d312 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/sketch_on_face_end.png differ diff --git a/src/wasm-lib/tests/executor/outputs/sketch_on_face_start.png b/src/wasm-lib/tests/executor/outputs/sketch_on_face_start.png new file mode 100644 index 000000000..894b3db10 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/sketch_on_face_start.png differ