Allow for geo-3d integration – Part 1.1: Code quality
#58
Conversation
|
This will be ready for review once #62 is merged |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 85f0871..0609981
| Severity | Location | Issue | Delete |
|---|---|---|---|
 |
src/shapes3d.rs:685 | Type mismatch compilation error |  |
✅ Files analyzed, no issues (11)
• src/bsp.rs
• src/csg.rs
• src/extrudes.rs
• src/io/svg.rs
• src/lib.rs
• src/metaballs.rs
• src/offset.rs
• src/plane.rs
• src/polygon.rs
• src/shapes2d.rs
• src/tests.rs
| let faces_vec: Vec<Vec<usize>> = faces.iter().map(|f| f.to_vec()).collect(); | ||
|
|
||
| Self::polyhedron(&pts, &faces_vec, metadata).scale(factor, factor, factor) | ||
| Self::polyhedron(&pts, &faces, metadata).scale(factor, factor, factor) |
There was a problem hiding this comment.
Type mismatch: The code passes &[[usize; 3]; 20] to CSG::polyhedron, which expects faces: &[Vec<usize>]. The variable faces is defined at line 662 as let faces: [[usize; 3]; 20] = [...].
When &faces is passed to polyhedron, it becomes &[[usize; 3]; 20] which can coerce to &[[usize; 3]] (slice of arrays), but cannot coerce to &[Vec<usize>] (slice of Vecs). These are fundamentally different types in Rust.
The old code (line 685-687 before this PR) correctly converted the array to Vec format:
let faces_vec: Vec<Vec<usize>> = faces.iter().map(|f| f.to_vec()).collect();
Self::polyhedron(&pts, &faces_vec, metadata)Removing this conversion breaks the type compatibility and will cause a compilation error.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
2 participants
This patch has code quality not related to organization and fixes a few tests.