polygon function to draw visible area #10
Conversation
feat(vector): orientation & intersection refactor(vector): use exponentiation operator refactor(vector): rename function orientation -> alignment feat(vector): polar_lt func to compare angle of vectors
| @@ -119,11 +148,11 @@ function vector:toPolar() | |||
| end | |||
|
|
|||
| function vector:len2() | |||
There was a problem hiding this comment.
best to make a separate pull request for these tweaks to keep changes atomic
Ah I feel silly, it is for calculating line of sight. This seems like something handy, but I'm not sure it belongs in hump rather than in a standalone library. |
| 3, 3, 3, -1, | ||
| -1, -1, -1, 3, | ||
| -1, -1, 3, -1 | ||
| }), |
There was a problem hiding this comment.
In tests it is often a good idea to be explicit: so pass vector:zero here and elsewhere you rely on it as default.
visible.lua
Outdated
| return vector.polar_lt(a.start, b.start, center) | ||
| end | ||
|
|
||
| function visible.polygon(segments, center) |
There was a problem hiding this comment.
The tricky thing with asking for a Vector object here is that it then is compatible only with Vector and not vector-light. People have their reasons for preferring vector-light. You could make it receive two arguments for center: center_or_x and y. Then you can interperet center_or_x as a vector if y is nil, as the x component of a vector from vector-light if y is not nil.
| @@ -0,0 +1,32 @@ | |||
| local visible = require("visible") | |||
There was a problem hiding this comment.
benchmark_visible.lua would be a more conventional filename for this
| -- < 0 -> counterclockwise | ||
| -- = 0 -> colinear | ||
| -- > 0 -> clockwise | ||
| local function alignment(a, b, c) |
There was a problem hiding this comment.
I'll need a bit more thorough a description of what this funtion represents, for example in the docs. From the name alone its purpose is not obvious.
| local vis = require("visible") | ||
|
|
||
|
|
||
| function testOrientation() |
There was a problem hiding this comment.
Thanks for adding tests. The tests for vector functions should have their own file, tests/vector.lua
visible.lua
Outdated
| do | ||
| local next_endpoints = {} | ||
| local missing_stops = {} | ||
| for i = 1, #segments, 4 do |
There was a problem hiding this comment.
In each loop here you clearly do a distinct operation. Name them and create local functions for them. it makes the code more modular, easier to build on and easier to read.
There was a problem hiding this comment.
For example, you might name this one collect_endpoints
visible.lua
Outdated
| -- return concave (frequently) polygon | ||
| if center == nil then center = vector.zero end | ||
| local endpoints = {} | ||
| do |
There was a problem hiding this comment.
This do seems to just form a block for no reason. I didn't actually know you could put a do statement alone without causing a syntaxerror.
visible.lua
Outdated
| vector.alignment(epj.start, epj.stop, epi.start) | ||
| * vector.alignment(epj.start, epj.stop, epi.stop) <= 0 then | ||
| local intersec = | ||
| vector.intersection(epi.start, epi.stop, epj.start, epj.stop) |
| lu.assertTrue(vec.alignment(vec(0, 1), vec(1, 1), vec(2, 1)) == 0) | ||
| end | ||
|
|
||
| function testIntersection() |
There was a problem hiding this comment.
This seems like the kind of function where you will want to test edge cases. does it work when they exactly intersect at a point? What does it return when they do not intersect at all?
|
Thanks for these reviews. I will see to them but not for several months. |
A visibility function to get the polygon of visible area from a list of segments (walls).
There are three new functions in vector.lua used by visibility function:
intersection,alignmentandpolar_ltI put a demonstration script, unit test script and performance timer script.