NavMoveTo always fails when stopOnTolerance==true

[RyanChapman2x]

I traced the bug and have my own fix, but thought I'd post it here in case it causes any unintended consequences.

Here's the offending code from NavMoveTo.cs lines 103-118, in moveToBlackboardKey():

// set new destination
agent.destination = destination;

bool destinationChanged = (agent.destination - lastDestination).sqrMagnitude > (DESTINATION_CHANGE_THRESHOLD * DESTINATION_CHANGE_THRESHOLD); //(destination - agent.destination).sqrMagnitude > (DESTINATION_CHANGE_THRESHOLD * DESTINATION_CHANGE_THRESHOLD);
bool distanceChanged = Mathf.Abs(agent.remainingDistance - lastDistance) > DESTINATION_CHANGE_THRESHOLD;

// check if we are already at our goal and stop the task
if (lastDistance < this.tolerance)
{
    if (stopOnTolerance || (!destinationChanged && !distanceChanged))
    {
        // reached the goal
        stopAndCleanUp(true);
        return;
    }
}

...

lastDestination = agent.destination;
lastDistance = agent.remainingDistance;

When agent.destination is changed, agent doesn't immediately update the path. Instead, it takes a frame or so to generate the new result (see: NavMeshAgent.destination documentation).

But immediately after changing the destination, this code is querying parameters that rely on the path having been generated. In particular, agent.remainingDistance will return 0 if the path hasn't been generated yet. And it will always return 0 after changing the destination because the destination is changed only two lines before it.

At the end of this method, the following line is called: lastDistance = agent.remainingDistance; Since agent.remainingDistance always returns 0 after the destination is changed, lastDistance is also always set to 0 after the destination is changed.

That's fine for the first pass through this method. It's when the method gets called on the second tick that the problem starts:

lastDistance will equal 0, since it was set on the last tick, before the path was ready. Which means that the conditional statement if (lastDistance < this.tolerance) will always evaluate to true on that second tick, because 0 is always smaller than any tolerance. Then the next conditional, if (stopOnTolerance || (!destinationChanged && !distanceChanged)) will also always evaluate to true if stopOnTolerance = true. These conditions cause the code to exit, apparently assuming that since the tolerance was met, then agent must have reached its destination (but in reality it hasn't even started to move yet).

To fix this problem, I changed the following line of code:

lastDistance = agent.remainingDistance;

to:

if (agent.pathPending)
{
    lastDistance = 99999999.0f;
}
else
{
    lastDistance = agent.remainingDistance;
}

agent.pathPending will return true if the path isn't yet set, meaning that we shouldn't trust the value of agent.remainingDistance, since it will return a false value of 0. So instead of using the unreliable agent.remainingDistance to set lastDistance, we instead set lastDistance to its default starting value of 99999999.0f.

That means that when we come around to the second tick, the conditional if (lastDistance < this.tolerance) does NOT trigger accidentally and we do NOT prematurely exit the navigation.

[meniku]

Hi thanks, I'll do some tests and include your fix

[meniku]

Thanks, I've added a test scene for the NavMoveTo and tried out your workaround. It definitly makes things better, thanks a lot. The fix is in the master, still has to be merged to 2.0-dev

[meniku]

merged to 2.0-dev as well