NavigationAgent's "link_reached" signal details should be improved

[venilark]

When setting up the NavigationLink3D, start position and end position are in relation to the NavigationLink3D itself, not global coordinates. When we get the details in the signal, we get the global coordinates of the current part of the link we are stepping on, whether it's start pos or end pos, so I did this

var link: MyJumpLink = details["owner"] as MyJumpLink
var node_position: int = link.get_node_position(details.location)
(this is the custom MyJumpLink class that extends NavigationLink3D)

but didn't work because the signal returns the closest point intersecting the Navmesh, not the object we set up manually
The code returns -1 because it is not exact enough so then I did this

if current_node_position.distance_to(start_node_position) < 0.5:
return 0
elif current_node_position.distance_to(end_node_position) < 0.5:
return 1
return -1

then if we need to get the other end (as a landing point for example)
func get_node_position_other_end(current_position: int) -> Vector3:
match current_position:
0:
return get_end_position()
_:
return get_start_position()

func get_end_position() -> Vector3:
return to_global(end_location)

func get_start_position() -> Vector3:
return to_global(start_location)

while this is working, I think it would be a good idea to add more parameters to the signal, so we know which part of the link we have reached, like "link_position_reached": "start"/"end", this way we could get the other one simply with the last two functions, without bothering about anything else

Also the signal documentation says

location: The start location of the link that was reached.

This is confusing, because if a link is bidirectional, you could be on the "end"

[smix8]

The function is_equal_approx() works with a small epsilon and that is always problematic when the distances can be unpredictably large.

Also @DarkKilauea.

[venilark]

I knew it later, that was the whole process because at first I thought both locations would be the same and the reason I had to do all this to get the information I needed.

[DarkKilauea]

When the link is created in the server, the end points are "snapped" to the surface of the mesh to avoid incongruities in the returned path (places where the path suddenly jumps to a position outside of the mesh). The idea was to avoid issues with users that used the returned path without any special logic to handle the link.

However, the distance the link will "search" for a polygon on the mesh to snap to is limited to the navigation/3d/default_link_connection_radius project setting (aka NavigationServer::map_get_link_connection_radius). You could check to see if the start or end point of the NavigationLink node is within that radius to determine which direction you are entering the link. Your final solution basically does that with the default value.

I agree that the documentation could be improved since "start" doesn't really describe what is happening. The signal returns the position of the link, as snapped to the mesh, that your path enters. From the perspective of the NavigationServer's underlying navigation mesh, a bidirectional link doesn't actually exist. Instead, all links are unidirectional and bidirectional links are duplicated, with a link running in both directions. This simplifies the path finding logic as it only has to consider a single case for whether to take a link.

In order to report which logical "end" of the NavigationLink node your path is entering, we'd have to map the underlying link back to the logical one and figure out which end we entered. This is certainly possible, but would require some overhead and potentially another array returned by the NavigationPathQueryResult itself that would only be useful in the case of a link. I'd like to find a more elegant solution that doesn't require the overhead or additional array if possible.

[smix8]

@venilark Please see pr godotengine/godot#73229 and let us know if this can solve this issue here.

[venilark]

sounds great
would it be merged for 4.0/next RC?

[smix8]

it is already merged.