Skip to content

Conversation

RolandCsibrei
Copy link
Contributor

@RolandCsibrei RolandCsibrei commented Aug 16, 2025

Already working examples (I need to tide up the code a bit):
http://localhost:1338/#KVQP83#203 = navmesh generation stuff
http://localhost:1338/#KVQP83#209 = obstacles
http://localhost:1338/#KVQP83#213 = offmesh
http://localhost:1338/#KVQP83#245 = move along navmesh with WASD
http://localhost:1338/#KVQP83#244 = raycast

@RolandCsibrei RolandCsibrei marked this pull request as draft August 16, 2025 22:03
@bjsplat
Copy link
Collaborator

bjsplat commented Aug 16, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 16, 2025

Reviewer - this PR has made changes to one or more package.json files.

@RolandCsibrei
Copy link
Contributor Author

RolandCsibrei commented Aug 16, 2025

Hello!
Finally it's here :-)

At this point we support all the V1 plugin functions + offMeshConnections. These are some enhacements beyond V1:

  • Inject custom TileCacheMeshProcess
  • Support Areas, Flags (this allows to set certain polys as water, sand, etc...)
  • Add userId support - each agent could have an userId. Some Recast functions can treat different users differently (some users can use the teleports, some must avoid the obstacle, some mustn't, etc...)
  • Implement all debug layers

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 16, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 16, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 17, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 24, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 24, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 25, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 25, 2025

Reviewer - this PR has made changes to one or more package.json files.

@RolandCsibrei
Copy link
Contributor Author

@ryantrem since the options parameter is for several functions the same I'll introduce a type for it

navMeshQuery: NavMeshQuery;
tileCache?: TileCache;
}>((resolve, _reject) => {
GenerateNavMeshWithWorker(meshes, parameters, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like GenerateNavMeshWithWorker can synchronously throw, but there is no try/catch with a call to _reject. I think that should be added if I'm reading the code correctly.

Copy link
Contributor Author

@RolandCsibrei RolandCsibrei Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not review the worker yet please.

// callback function to process the message from the worker
workerOptions.worker.onmessage = (e) => {
if ((e as any).data?.success === false) {
throw new Error(`Unable to generate navMesh: ${e}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do these errors go? I assume they just show up as console errors, and the promise on the other side of this never resolves or rejects?

Copy link
Contributor Author

@RolandCsibrei RolandCsibrei Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not review the worker yet please.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Reviewer - this PR has made changes to one or more package.json files.

@RolandCsibrei RolandCsibrei requested a review from ryantrem August 30, 2025 09:07
@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Reviewer - this PR has made changes to one or more package.json files.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 30, 2025

Reviewer - this PR has made changes to one or more package.json files.

@RolandCsibrei
Copy link
Contributor Author

RolandCsibrei commented Sep 2, 2025

@deltakosh @ryantrem @CedricGuillemet
Guys, I’d like to close the PR this week if possible. I’ll be on vacation for two weeks starting this weekend. Are there any other issues you’d like me to fix or change before then?

The UMD build always fails. I believe we need to update some of the build scripts to properly handle the recast-navigation-js dev dependency during the build process. I found some code that handles similar dependencies in generateDeclaration.ts (line 166), but I’m not sure if that’s the right place to make changes.

Thank you!

@ryantrem
Copy link
Member

ryantrem commented Sep 2, 2025

@deltakosh @ryantrem @CedricGuillemet Guys, I’d like to close the PR this week if possible. I’ll be on vacation for two weeks starting this weekend. Are there any other issues you’d like me to fix or change before then?

The UMD build always fails. I believe we need to update some of the build scripts to properly handle the recast-navigation-js dev dependency during the build process. I found some code that handles similar dependencies in generateDeclaration.ts (line 166), but I’m not sure if that’s the right place to make changes.

Thank you!

My main outstanding questions are:

  1. The top level API - if I'm understanding correctly, I think you've resolved this. Now you are just supposed to call functions like CreateNavigationPluginAsync and never directly instantiate the RecastNavigationJSPluginV2 class, correct? And no other setup is needed, just call one of the async factory functions and you are good to go, yes?
  2. Back compat confusion - there are a number of open comments where you mentioned you don't want to break back compat, but these are all on completely new files/APIs, so I'm unclear what back compat means to you in the context of this PR. In my mind, back compat was limited to implementing the existing INavigationEnginePlugin interface so that anyone currently using this interface can use the v2 implementation. I don't see net new files, module level exports, classes, class members as needing any kind of back compat (since they are net new). But maybe you have something in mind that I'm not understanding.

@RolandCsibrei
Copy link
Contributor Author

  1. The top level API - if I'm understanding correctly, I think you've resolved this. Now you are just supposed to call functions like CreateNavigationPluginAsync and never directly instantiate the RecastNavigationJSPluginV2 class, correct? And no other setup is needed, just call one of the async factory functions and you are good to go, yes?

You can calll the create function or you instantiate the plugin for backwards compatibility (in PG). This is where the exported let BjsRecast variable comes in role.

Back compat confusion - there are a number of open comments where you mentioned you don't want to break back compat, but these are all on completely new files/APIs, so I'm unclear what back compat means to you in the context of this PR. In my mind, back compat was limited to implementing the existing INavigationEnginePlugin interface so that anyone currently using this interface can use the v2 implementation. I don't see net new files, module level exports, classes, class members as needing any kind of back compat (since they are net new). But maybe you have something in mind that I'm not understanding.

There are onyl two files where backwards compatility playes role: the plugin class and the crowd class

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

Successfully merging this pull request may close these issues.

5 participants