Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obstacle queue not processing batches of updates #312

Open
DVLP opened this issue Apr 8, 2024 · 11 comments
Open

Obstacle queue not processing batches of updates #312

DVLP opened this issue Apr 8, 2024 · 11 comments

Comments

@DVLP
Copy link

DVLP commented Apr 8, 2024

First of all thanks for making a good JS wrapper for Recast. It's the most usable one and with good examples too!

Now the issue. If I understood the system correctly tileCache.update(navMesh) needs to be called to process the queue of up to 64 added obstacles. So I wrote this code to add multiple boxes to a test level (earlier passed maxObstacles: 1000 option accordingly)

for (var i = 0; i < 1000; i++) {
      const x = Math.random() * 500 - 250
      const z = Math.random() * 500 - 250
      const y = 0
      const box = tileCache.addBoxObstacle(
        { x, y, z },
        { x: 2, y: 2, z: 2 },
        Math.random()
      )

      if (i % 64 === 0) {
        let upToDate = false
        while (!upToDate) {
          const result = tileCache.update(navMesh)
          upToDate = result.upToDate;
        }
      }
    }
}

With this code only a few boxes are seen by the navmesh helper. The rest is only visible by the TileCacheHelper with no effect on the navmesh. I updated navmesh helper after adding all the obstacles to make sure it's up to date.

Then I commented out the part that limits execution of tileCache.update(navMesh) to each 64th pass. Now it's processing the update after each call to tileCache.addBoxObstacle.

      // if (i % 64 === 0) {
        let upToDate = false
        while (!upToDate) {
          const result = tileCache.update(navMesh)
          upToDate = result.upToDate;
        }
      //}

With this code every box is now correctly seen by the navmesh and the agents. It seems like there's an issue with the queue system or I just use the library incorrectly

@isaac-mason
Copy link
Owner

Hey @DVLP,

It's entirely possible I've mis-documented the specifics of the obstacle queue behaviour.

I tried to be clever and add more documentation than recastnavigation itself by reading the c++ implementation.

I'll take a second pass!

@isaac-mason
Copy link
Owner

I've made a related change that could help here: d490a5c

It's possible for these APIs (addCylinderObstacle, addBoxObstacle, removeObstacle) to fail. If they do fail, the c++ DetourTileCache APIs return a status code explaining why.

This isn't currently exposed, but that change (pending release) exposes them.

@DVLP
Copy link
Author

DVLP commented Apr 10, 2024

Thanks for looking into this. I'll try later to see if it actually comes back with success false when running this in a loop without updating tileCache every time

@DVLP
Copy link
Author

DVLP commented Apr 12, 2024

Found some time to check this again with the lastest version.

This is a result when running navmesh.update() (recursively until it's done) on each 64th iteration when adding boxes in a loop
image

It looks like the queue size is around 25-26 but that's not completely reliable. When running a full update on each 25th iteration I was still getting gaps. btw the result of addBoxObstacle is always a success.

When running on every 23rd iteration I'd still get some boxes not registered. On each 22nd or 21st I'd get some artifacts like only half of the box would become a hole in the navmesh.
image

Running the update on each 20th iteration seems to be the most reliable without leaving artifacts (except of some small corners cut but that's a different issue).

btw I'm also having an issue with artifacts when moving a box around that is larger than the tiles and on one side it's not creating a hole in the tiles

@DVLP
Copy link
Author

DVLP commented Apr 12, 2024

Here's my setup code

    function tileCacheFullUpdate(tileCache, navMesh) {
      let upToDate = false
      while (!upToDate) {
        const result = tileCache.update(navMesh)
        upToDate = result.upToDate
      }
    }

    for (var i = 0; i < 1000; i++) {
      const obj = new THREE.Mesh(new THREE.BoxGeometry(5,5,5), new THREE.MeshBasicMaterial({ color: 0xFF1133, wireframe: true }))
      scene.add(obj)

      const x = (i % 10) * 10
      const z = -200 + Math.floor(i / 10) * 10
      const y = 0

      obj.position.set(x, y, z)
      obj.geometry.computeBoundingBox()
      const bbox = obj.geometry.boundingBox
      const extents = bbox.max.clone().sub(bbox.min).divideScalar(2)

      const result = tileCache.addBoxObstacle(
        obj.position,
        extents,
        obj.rotation.y,
      )
      if (!result.success) throw new Error('addBoxObstacle failed')

      // 20 seems to be the highest reliable interval
      if (i % 20 === 0) {
        tileCacheFullUpdate(tileCache, navMesh)
      }
    }

@isaac-mason
Copy link
Owner

Thanks for looking into this more!

btw the result of addBoxObstacle is always a success.

Noted! This may help narrow down the issue.

@isaac-mason
Copy link
Owner

isaac-mason commented Apr 13, 2024

I'll create a storybook example like yours as a testbed to investigate, and so we've got an example of correct usage when we get a handle on this.

@isaac-mason
Copy link
Owner

isaac-mason commented Apr 13, 2024

I've added a storybook now, see: 56f1855 https://recast-navigation-js.isaacmason.com/?path=/story/tilecache-obstacles--many-obstacles-example

I think it's better to check the returned status, the tile cache queue size isn't exposed anywhere and could change. If the status is BUFFER_TOO_SMALL I do a full tile cache update then retry adding the obstacle.

let obstacles = 0;
for (let x = -10; x < 10; x += 2) {
  for (let z = -10; z < 10; z += 2) {
    obstacles += 1;

    const obstaclePosition = new Vector3(x, 0, z);

    const createObstacle = () => {
      if (obstacles % 2 === 0) {
        return tileCache.addBoxObstacle(
          obstaclePosition,
          boxObstacleSize,
          0.2
        );
      } else {
        return tileCache.addCylinderObstacle(
          obstaclePosition,
          addCylinderObstacleRadius,
          1
        );
      }
    };

    const result = createObstacle();

    if (
      !result.success &&
      statusDetail(result.status, Raw.Detour.BUFFER_TOO_SMALL)
    ) {
      fullTileCacheUpdate();

      createObstacle();
    }
  }
}

@isaac-mason
Copy link
Owner

isaac-mason commented Apr 13, 2024

I was also able to reproduce the issue with tile cache calls returning a successful status but not updating the navmesh as expected. In that storybook, if I change cs from 0.1 to 0.05, all tile cache calls (addBoxObstacle, addCylinderObstacle, update) are successful but many of the obstacles aren't reflected on the navmesh.

@DVLP
Copy link
Author

DVLP commented May 2, 2024

This may be something worth raising with the main library https://github.com/recastnavigation/recastnavigation

@isaac-mason
Copy link
Owner

Yep agreed. Ideally we'd also create a reproduction with the c++ library directly to confirm it's not just an issue with the javascript bindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants