Fix NavigationServer3D.get_closest_point_to_segment() with use_collision

[permelin]

When calling NavigationServer3D.get_closest_point_to_segment() with the parameter use_collision set to true, it always returns (0, 0, 0).

The bug can be seen here, where closest_point_d is incorrectly set to the same value as d on the first line, which hinders the else if below it from ever being true.

godot/modules/navigation/nav_map.cpp

Lines 620 to 628 in e96ad5a

	const real_t d = closest_point_d = p_from.distance_to(inters);
	if (use_collision == false) {
	closest_point = inters;
	use_collision = true;
	closest_point_d = d;
	} else if (closest_point_d > d) {
	closest_point = inters;
	closest_point_d = d;
	}

It's obvious that closest_point_d and closest_point should always be set together, so I can't imagine what the intention behind that first line could have been.

(I find the way these lines are written to be confusing with unnecessary duplication, but I resisted the urge to refactor them.)

[Scony]

Haven't tested but the change looks correct. I've checked with blame and it looks like the problem was there form the very beginning.

If you have some more time then I'd recommend adding a unit test covering that code path. There are unit tests for map already so it should be a matter of copy-pasting and modifying a bit.

[permelin]

I've checked with blame and it looks like the problem was there form the very beginning.

Well, if you had written tests instead of this TODO, the bug would have been found earlier. 😁

godot/tests/servers/test_navigation_server_3d.h

Line 700 in e96ad5a

// TODO: Test map_get_closest_point_to_segment() with p_use_collision=true as well.

If you have some more time then I'd recommend adding a unit test covering that code path.

I've added a couple of tests. The subcase looks awkward, but I don't know what else to do.

[Scony]

Good point. Maybe I was hesitant about that test case because it was giving me the wrong results :)
Anyway, now the change looks perfect.

[akien-mga]

Thanks!