Search Unity

Possibly found a bug in NavMeshBuildSource?

Discussion in 'Navigation' started by RandoX1X, Apr 6, 2017.

  1. RandoX1X

    RandoX1X

    Joined:
    Apr 6, 2017
    Posts:
    4
    Hey there!

    I was really excited for Unity 5.6 primarily because of the runtime NavMesh builder, because I'm creating a game that makes use of a Voxel Engine and pre-baking NavMesh was never an option for me, because game's maps are being unpredictably modified during gameplay.

    Using your LocalNavMeshBuilder example I was able to plug the new solution in and get it to work without much problem. Since I'm developing for mobile I'm always optimizing code the best I can and with LocalNavMeshBuilder I noticed there's a room for improvement in a form of pre-caching NavMeshBuildSources instead of recollecting them on each rebuild. I've done exactly that and came across an issue that's possibly a bug. I've got 2 types of NavMeshBuildSources in my game:

    1. first is for parts of the Voxel map, that have their Mesh procedurally updated during gameplay, but don't move
    2. second type is for gameplay obstacles that move around, but don't have their mesh updated.

    Reusing the cached NavMeshBuildSources for #1 is perfectly fine, when Mesh gets updated NavMesh gets updated accordingly on rebuild. Case 2 however doesn't really work on pre-cached sources as I assumed it would - before each NavMesh rebuild I'm updating their transform matrix, but it's not being reflected in the rebuilt NavMesh (position stays as it was in the first build). For now the work around I ended up doing is creating new instances of those sources with the exact same settings, but updated transform, and replacing them in the sources array, which kind of defeats the purpose of pre-caching them, since I'm forced to constantly create new copies at runtime anyway. It's not the end of the world, cause luckily the case 1 sources outnumber the case 2, so I'm still seeing 80% speed increase on an average UpdateNavMesh() iteration compared to the original implementation, but thought i'd let you guys know anyway.

    Stay awesome, my United folks!
     
  2. Jakob_Unity

    Jakob_Unity

    Joined:
    Dec 25, 2011
    Posts:
    269
    First a disclaimer: The examples in the repository are not optimized for speed - but serve as inspiration.
    To keep the example minimal - I decided to simply extract sources every time (and there are other limitations : only default agent type is supported - no support for phys. colliders etc.).

    That said - caching the NavMeshBuildSources instead of extracting them every update is a good idea!

    To modify the `NavMeshSourceTag` component to cache the sources - I advice using OnEnable/OnDisable to add/remove the NavMeshBuildSources to a static list, which the LocalNavMeshBuilder then uses (instead of `NavMeshSourceTag.Collect()` )

    Now since the NavMeshBuildSource is simply a snapshot of the input geometry - it doesn't know about transformations for the input geometry. This means you must track of the transforms explicitly - then update the matrix of the build source.

    I've tried this - and I don't see any problems. My guess is that the problem you see related to case #2 : you're not updating the build source transform to follow transform of the obstacle gameobject?
     
  3. RandoX1X

    RandoX1X

    Joined:
    Apr 6, 2017
    Posts:
    4
    Hey, thanks for replying!

    That's exactly what I was trying at first, I created a helper class for the moving objects that had a reference to its own Source object and prior to every NavMesh rebuild I iterated through them and updated the source's transform.

    Here's the code if you want to check it out:
    https://pastebin.com/GTurgUys

    UpdatePosition() method is the one I tried at first and did not work, CloneSource() is the work around I mentioned and what I'm now calling while iterating through these helpers before each NavMesh rebuild.

    Edit: captured 2 short videos showing the issue, this one is using the UpdatePosition() method:
    https://www.dropbox.com/s/bb09b80elbfi6k2/UpdatePosition.mov?dl=0
    and here's the CloneSource():
    https://www.dropbox.com/s/ab2102qu8h2ufbt/CloneSource.mov?dl=0

    So as you can see, the only difference between the 2 is that the 2nd one creates new instance of the Source with the exact same settings, while both of them update to current transform value.
     
    Last edited: Apr 10, 2017
  4. Jakob_Unity

    Jakob_Unity

    Joined:
    Dec 25, 2011
    Posts:
    269
    Also the signatures of the two functions differ - one updates in-place the other one returns the new struct.
    It's difficult to tell what's wrong w/o the calling context. Are you updating the navmesh repeatedly in both cases ? Maybe look at where/how you populate the List you pass to the navmesh build/update call.

    The snippet below seems to work for me - it's just updating the transform property after initialization.
    The navmesh updates correctly, following the transform of the script GO.

    Code (csharp):
    1.  
    2. // TestTransformChange.cs
    3. using System.Collections;
    4. using System.Collections.Generic;
    5. using UnityEngine;
    6. using UnityEngine.AI;
    7.  
    8. public class TestTransformChange : MonoBehaviour
    9. {  
    10.     public NavMeshBuildSource m_Source;
    11.     NavMeshDataInstance m_Instance;
    12.     NavMeshData m_Data;
    13.  
    14.     void OnEnable () {
    15.         var plane = GameObject.CreatePrimitive (PrimitiveType.Plane);
    16.         plane.SetActive (false);
    17.  
    18.         m_Source.shape = NavMeshBuildSourceShape.Mesh;
    19.         m_Source.sourceObject = plane.GetComponent<MeshFilter> ().sharedMesh;
    20.         m_Source.transform = transform.localToWorldMatrix;
    21.  
    22.         m_Data = new NavMeshData (0);
    23.         m_Instance = NavMesh.AddNavMeshData (m_Data);
    24.     }
    25.  
    26.     void OnDisable () {
    27.         m_Instance.Remove ();
    28.     }
    29.  
    30.     void Update () {
    31.         m_Source.transform = transform.localToWorldMatrix;
    32.         var bounds = new Bounds (Vector3.zero, 1000.0f * Vector3.one);
    33.         var sources = new List<NavMeshBuildSource> ();
    34.         sources.Add (m_Source);
    35.         NavMeshBuilder.UpdateNavMeshData (m_Data, NavMesh.GetSettingsByIndex (0), sources, bounds);
    36.     }
    37.  
    38.     void OnDrawGizmos () {
    39.         Gizmos.color = Color.red;
    40.         Gizmos.DrawWireMesh (m_Source.sourceObject as Mesh, m_Source.transform.GetColumn (3), Quaternion.identity);
    41.     }
    42. }
    43.  
     
    Last edited: Apr 10, 2017
  5. RandoX1X

    RandoX1X

    Joined:
    Apr 6, 2017
    Posts:
    4
    Thanks for replying again.

    Calling context is exactly the same in both cases, the only difference is which of those 2 methods is in use. I tried getting my version closer to your test case just to see if I stumble upon anything (moving from Start enumerator to update, using UpdateNavMeshData instead of the Async version, creating new array on each update) but nothing changed.

    Here's what's happening my end: https://pastebin.com/Sd8sYqii
    At the bottom there you have the UpdatePositions() method which iterates through helper array and calls either CloneSource() or UpdatePosition() (from the earlier paste, https://pastebin.com/GTurgUys), nothing else changes and first one works, second one doesn't. Do you notice anything being done not as intended?

    I'm using Unity 5.6.0f3 in case it matters.
     
  6. Jakob_Unity

    Jakob_Unity

    Joined:
    Dec 25, 2011
    Posts:
    269
    From your loop :
    Code (csharp):
    1. sourcesArray[sourcePositionUpdaters[i].index] = sourcePositionUpdaters[i].CloneSource(); // <- This works fine,                 creates new instance of the source and replaces the old one in the sources array at the same index
    2. //sourcePositionUpdaters[i].UpdatePosition();// <- This doesn't work (updating existing sources transform)
    only the first line can work - it's the only one that updates the entry in the sourcesArray (m_Sources). The other one (commented out) just updates its own public field. It never writes to elements in m_Sources.
     
  7. RandoX1X

    RandoX1X

    Joined:
    Apr 6, 2017
    Posts:
    4
    Right, because they're structs, therefore value types, my bad.

    Thanks and sorry for wasting your time!