Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polygon stdlib function #4300

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

guptaarnav
Copy link
Collaborator

Implements polygon in the kcl stdlib to close #4261

There seems to be an issue with edge/face selection of the polygon, but I don't think its related to this implementation (please correct me if i'm wrong). Upon looking into what commands are sent via the debug console, it appears that when selecting an edge/face, the engine selects that edge/face, then immediately clears the selection, then consistently selects a specific edge/face instead of the targeted edge/face. This seems to be happening for any selection on other entities, as well, i.e. when the user selects some entity, the engine will select, then clear selection, then reselect in rapid succession.

Copy link

qa-wolf bot commented Oct 25, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented Oct 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Oct 25, 2024 2:00am

@jtran
Copy link
Collaborator

jtran commented Oct 26, 2024

@adamchalmers, is it possible to implement this stdlib function in KCL? I know we have some infrastructure for that like the below, but I don't think we actually use it.

pub trait KclStdLibFn: StdLibFn {

@guptaarnav, I don't want to make you redo this PR. I only thought of this now. Our internal users wrote an ngon() function in KCL.

@guptaarnav
Copy link
Collaborator Author

guptaarnav commented Oct 26, 2024

@adamchalmers, is it possible to implement this stdlib function in KCL? I know we have some infrastructure for that like the below, but I don't think we actually use it.

pub trait KclStdLibFn: StdLibFn {

@guptaarnav, I don't want to make you redo this PR. I only thought of this now. Our internal users wrote an ngon() function in KCL.

@jtran, i'm not sure i understand, i thought my PR was implementing polygon as a stdlib function in kcl. what's different in what you're alluding to? is the internal ngon() function a kcl library function that users can import into their project to use rather than something implemented in the std language itself as I implemented?

p.s. i don't mind redoing the PR as long i understand what should be changed

@jtran
Copy link
Collaborator

jtran commented Oct 26, 2024

It's theoretically possible to implement a KCL stdlib function in KCL, as opposed to in Rust. I.e. the function is actually implemented with KCL code. There's some code to support this, like what I linked above. But I don't think we actually use it. So there may be more missing. If there's a lot missing (I'm guessing there is), it's probably not worth going that route now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KCL: Polygon function
2 participants