Improve findWallDistance()'s returned normal

[richard-fine]

The normal returned by findWallDistance() isn't really a normal, it's just a normalized vector from the hit point to the source point. The hit point is already returned by this function, and the source point was passed in, so it's easy for users to calculate this themselves if they want.

So, instead, return the bitangent to the actual wall itself, as it's more useful.

[richard-fine]

I'm actually not sure if this is correct or not... I made this change on my fork originally because I seemed to be having problems with the normal vector used to steer agents away from walls. But now that I think about it, the nearest wall point always should have a normal that points directly towards the source point, I think? So maybe this is unnecessary... but I'm not sure....

[hymerman]

Perhaps this is useful for when the nearest point is a corner?

[jswigart]

The nearest wall will not necessarily always have a perpendicular normal to the source point. That would perhaps be the case if you are looking for the nearest edge from within a convex polygon but walls are boundary edges of the broader navmesh structure, and will often be edges that your source point isn't projected directly onto.

The normal shouldn't be simply the direction to the hit point or it shouldn't be called normal. That should be the interior perpendicular direction of the edge if it's going to use the term.

[memononen]

If you hit a vertex then there are multiple normals. I chose to call it
normal, as in "outward facing unit vector". It indeed can be derived from
the hit point and current position.

On Wed, Nov 18, 2015 at 2:12 PM, jswigart notifications@github.com wrote:

The nearest wall will not necessarily always have a perpendicular normal
to the source point. That would perhaps be the case if you are looking for
the nearest edge from within a convex polygon but walls are boundary edges
of the broader navmesh structure, and will often be edges that your source
point isn't projected directly onto.

The normal shouldn't be simply the direction to the hit point or it
shouldn't be called normal. That should be the interior perpendicular
direction of the edge if it's going to use the term.

—
Reply to this email directly or view it on GitHub
#120 (comment)
.

[jakobbotsch]

The wall normal is very useful to have. For example, in many cases the points returned by findStraightPath is directly on top of the wall segment, in which case the normal returned by this function today is useless (as the direction cannot properly be computed).

I'm all for this change, however it is breaking. I'm not sure how we should deal with making breaking changes like this. @hymerman @grahamboree, what do you think?

[hymerman]

For this case in particular, if people think both the new and old behaviour is useful (which is what it sounds like), then the new behaviour should be added as a new parameter to the function.

More generally, we shouldn't be afraid to make breaking changes where we feel they're strictly better than the previous behaviour, and where it won't be too disruptive - as long as this is communicated to users, preferably via the code itself (i.e. let's not change the meaning of a parameter without also changing its type or at least its name!).

We'll eventually get round to sorting out official releases which we can tag using good ol' http://semver.org/ and release notes, to flag up non-backwards-compatible changes. Then we can treat the master branch as an unstable, "might change radically at any moment" branch, and encourage users to stick to tagged releases.

Does that sound reasonable?

[jakobbotsch]

Yes, that sounds completely reasonable and like a very good way to do it.

However, in this case I would prefer to change the behavior, not add new behavior; the old method of computation can easily be done by the user (simply compute the unit vector in the same way as the code used to do it). The new method is a lot more involved (it requires finding the nearest polygon side that actually constitutes a wall, and then computing its normal).

I think we should merge this change once we have the versioning set up.

[grahamboree]

I'm with @JanielS on this one. I'd rather return the more useful vector and let the user do the vector sub and normalize to get what we're already giving them (if they really want it in the first place).

Is the query point guaranteed to be on the same side of the vi -> vj line segment? If not, the normal direction will be flipped.

Re: versioning, I think what's been said is a smart way of approaching breaking changes. I also agree that we shouldn't be afraid to break things in the name of progress, so long as its clearly communicated to the users. Semantic versioning seems like a good way of communicating this.

[jakobbotsch]

I have merged this into the 2.0 branch as we talked about waaaaay back when.