Conversation
sci-code-711
left a comment
sci-code-711
left a comment
There was a problem hiding this comment.
Just looked at the line stuff so far. General codification is good just a few readability and test cases things
There was a problem hiding this comment.
Do you know why there are changes in this file? Can you also look if this file should even be included I the repo?
There was a problem hiding this comment.
I have a yellow squiggle under the compiler path. The comment is the message that comes up. It seem to only configure the c\c++ extentions in vscode so probably not?
sci-code-711
left a comment
sci-code-711
left a comment
There was a problem hiding this comment.
Could really do with some docstrings for the methods especially as things are getting a little more complicated with lines and planes and intersections
source/plane.hpp
Outdated
| Vector intersection(const Line &that) const; | ||
| }; | ||
|
|
||
| bool between(const Line line, const Plane &plane1, const Plane &plane2); |
There was a problem hiding this comment.
What does the between method do?
There was a problem hiding this comment.
Tells you whether a line goes between two planes. If the planes of the bounding box are perpendicular to the screen then it will never intersect them. In such a case you would need to know whether the line goes between the planes as to whether the clicking line intersects with the bounding box. At least, that is what I believe.
sci-code-711
left a comment
sci-code-711
left a comment
There was a problem hiding this comment.
All looks good. Only other thing I would want to do is go through the test cases and maybe add some more. Really good job on this though
|
Redundant if blocks should have been removed and I put all the unittests in a namespace. |
|
Approved |
2 participants
Add line and plane functionality to the code base