NavMeshQuery returns the same FindRandomPoint almost every time

[AqlaSolutions]

        for (int i = 0; i < 10; i++)
        {

            SVector3 startPt = new Vector3();
            int startRef = 0;
            while (!navMeshQuery.FindRandomPoint(ref startRef, ref startPt))
                Debug.WriteLine("Can't find random point");

            SVector3 endPt = new Vector3();
            int endRef = 0;

            while (!navMeshQuery.FindRandomPoint(ref endRef, ref endPt))
                Debug.WriteLine("Can't find random point");

            Debug.WriteLine("Testing finding path from " + startPt + " (" + startRef + ") to " + endPt + " (" + endRef + ")");
        }

Outputs: Testing finding path from (-5,907467; 12,95738; 68,52042) (424) to (-5,907467; 12,95738; 68,52042) (424) Testing finding path from (44,68903; 10,97468; 38,93972) (415) to (44,68903; 10,97468; 38,93972) (415) Testing finding path from (44,68903; 10,97468; 38,93972) (415) to (44,68903; 10,97468; 38,93972) (415) Testing finding path from (44,68903; 10,97468; 38,93972) (415) to (44,68903; 10,97468; 38,93972) (415)...

If I use different instances:

    for (int i = 0; i < 10; i++)
    {
            var navMeshQuery = new NavMeshQuery(_tiledNavMesh, 2048);

            SVector3 startPt = new Vector3();
            int startRef = 0;
            while (!navMeshQuery.FindRandomPoint(ref startRef, ref startPt))
                Debug.WriteLine("Can't find random point");

            navMeshQuery = new NavMeshQuery(_tiledNavMesh, 2048);

            SVector3 endPt = new Vector3();
            int endRef = 0;

            while (!navMeshQuery.FindRandomPoint(ref endRef, ref endPt))
                Debug.WriteLine("Can't find random point");

            Debug.WriteLine("Testing finding path from " + startPt + " (" + startRef + ") to " + endPt + " (" + endRef + ")");
        }

Testing finding path from (59,62428; 10,5812; 48,15485) (411) to (59,62428; 10,5812; 48,15485) (411) Testing finding path from (59,62428; 10,5812; 48,15485) (411) to (59,62428; 10,5812; 48,15485) (411) Testing finding path from (59,62428; 10,5812; 48,15485) (411) to (59,62428; 10,5812; 48,15485) (411) Testing finding path from (59,62428; 10,5812; 48,15485) (411) to (59,62428; 10,5812; 48,15485) (411) Testing finding path from (4,317558; 10,78998; 4,080609) (294) to (4,317558; 10,78998; 4,080609) (294) Testing finding path from (4,317558; 10,78998; 4,080609) (294) to (4,317558; 10,78998; 4,080609) (294) Testing finding path from (4,317558; 10,78998; 4,080609) (294) to (4,317558; 10,78998; 4,080609) (294) ...

You shouldn't create new Random object in every call because it uses Environment.TickCount seed.

[Robmaister]

Right, noticed this while fixing the other bug. I'll add overloads to accept a Random object.

[AqlaSolutions]

Wouldn't it be better to just keep one instance inside NavMeshQuery? Or it's supposed to be used in a multithreading environment?

[Robmaister]

IMO a Random object is something that's tangentially related to a query in that a handful of functions need one, but the rest of the class doesn't. It can also be useful for people who want to reproduce a state from a stored seed. Passing the random to the constructor of NavMeshQuery makes it a bit harder to keep track of where the random object is generating numbers.

[Robmaister]

Alright, that should do it. Reopen if you have any issues with the new overloads.

[AqlaSolutions]

There are no issues with the new overloads but there are still with old ones. The problem is not fixed. I'd recommend you to use "default" random instance passed to the constructor in case if the overload without rand parameter is used.

[Robmaister]

I'm not going to add any more members to NavMeshQuery, I'm already planning on slimming it down and possibly moving to a LINQ-esque API. I'll mention to not call it more than once at a time in the documentation for the old overloads, though.

[Robmaister]

@AqlaSolutions now that I'm revisiting this and cleaning up the pathfinding code, I ended up removing the Random parameter and taking one in the constructor mostly because it will reduce the possibility of thread-safety mistakes on the user's end.