GRIM: heightcheck fix

[tobiaspfaff]

In Sector::getBridgesTo(), the height check is effectively disabled, as the iterator is not reset. This may lead Manny to jump from high platforms to lower ones. This is especially visible in the mouse mod, which makes use of walkTo more extensively, but is a general bug which is why I'm posting this here.

[tobiaspfaff]

Actually, never mind. It seems the height check actually breaks stuff in some places. maybe put a comment in there, though.

[klusark]

Where does it break stuff? It does look like a bug to me, so if fixing that bug causes new bugs, we should probably fix those too.

[tobiaspfaff]

Not all sectors connect with two overlapping, parallel lines; E.g. at the beaver dam next to the tar pit, there is a triangle sector on the way up, which connects to a square sector with the pointy end.
The code tries to match the sides of the triangle, which then obviously have a height difference.

[Akz-]

The bridge building algorithm doesn't expect to find matching parallel lines on both sectors. It is enough that the sectors have intersecting area. In the above case the intersection would be the single point at the triangle sector's pointy end, and the correct bridge would be a line of length 0 at the intersection point. Sector.cpp:386 could be the problem, since it will discard any bridges that are less than 0.01 units wide.

[tobiaspfaff]

Ah, interesting! I missed the fact that bridges are shortened to the intersecting area. Removing the bridge lengthcheck (386) indeed makes these triangle-quad intersections work with the enabled height check.
Is it safe to remove this check, and why is it even there?

[Akz-]

I guess the check is supposed to prune unnecessary bridges at corners where two sectors are just barely touching. Considering the usual case where two sectors are connected by parallel edges the obvious bridge is the parallel edge, but if the length check is removed you will also get two almost zero length bridges at the corners. That is because the adjacent edges have one vertex overlapping the other sector, so the edges are then cut to the intersecting area and accepted as bridges.

The beaver dam case shows that such bridges can't always be considered unnecessary, though. Also the presence of any unnecessary bridges shouldn't affect the behavior of the pathfinding, so I think the check can be safely removed.

[somaen]

So, what's the conclusion on this?

[Akz-]

This can't be merged as is, because as mentioned above it breaks the pathfinding in certain areas. Disabling the bridge length check fixes this, but it in turn causes the pathfinding to occasionally produce some weirdly zig-zaggy sub-optimal paths, because of bugs in the pathfinding code. I have a fix for this stashed somewhere but it requires a bit more testing. I can look into that sometime soon once I finish the animation stuff I'm currently working on for EMI.

[klusark]

This commit was merged with modifications to it's commit message.

3ec2347