Mysterious behavior in _getSharedVerticesInOrder
Closed this issue · 4 comments
The zone group portals list is created by this function: https://github.com/donmccurdy/three-pathfinding/blob/master/src/Builder.js#L176
Some effort is made here to ensure that, for the pair of triangles given as input, given exactly two shared vertices (a, b), the list of vertex IDs for neither triangle has the form (a, c, b) or (b, c, a), having a and b on the "ends." It shifts the shared vertices to be contiguous in the array.
However, it seems to me that this won't be effective in actually causing the vertexIds in the outputted group polygon object to consistently have this property, because _getSharedVerticesInOrder will frequently run on the same triangle more than once, with different neighboring triangles. If a triangle's vertex list is first rearranged to (a, b, c) to accommodate a neighbor with shared vertices a and b, it might later be rearranged to (b, c, a) to accommodate a neighbor with shared vertices a and c.
Given this, what's the purpose of the work in _getSharedVerticesInOrder, anyway?
(Another note on this function: the check whether there are less than 2 shared vertices seems unnecessary -- that condition should be impossible unless there is malformed input (i.e. a geometry face with the same vertex multiple times) or a bug in the neighbor generation.)
Given this, what's the purpose of the work in _getSharedVerticesInOrder, anyway?
I'm not sure... this code was also in PatrolJS. Guessing a bit, but it looks like the string-pull implementation (Channel.js in this repository) depends on some concept of 'left' and 'right' portal vertices.
the check whether there are less than 2 shared vertices seems unnecessary
Let's replace that return [] with throwing an explicit error for now? If nothing hits the code path we can remove it eventually.
The code in Channel.js seems to care about the order of the elements in portals (the return value of _getSharedVerticesInOrder), but not about the order in vertexIds (the side effect of _getSharedVerticesInOrder) -- both of those seem like they will get somewhat unpredictably befuddled through repeated applications with different neighbors, but independently so.
Ok, no idea then! this should be covered by unit tests so I’m ok with trying to simplify it.