Skip to content

Early version of the recast-navigation-js navigation plugin #16612

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

Closed

Conversation

RolandCsibrei
Copy link
Contributor

@RolandCsibrei RolandCsibrei commented May 15, 2025

#X5XCVT#873

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 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.

@RolandCsibrei RolandCsibrei marked this pull request as draft May 15, 2025 19:17
@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have made possible changes to the playground.
You can test the snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/

The snapshot playground with the CDN snapshot (only when available):

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/?snapshot=refs/pull/16612/merge

Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly.

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have changed file(s) that made possible changes to the sandbox.
You can test the sandbox snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16612/merge/

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have changed file(s) that made possible changes to the sandbox.
You can test the sandbox snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16612/merge/

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have made possible changes to the playground.
You can test the snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/

The snapshot playground with the CDN snapshot (only when available):

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/?snapshot=refs/pull/16612/merge

Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly.

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have changed file(s) that made possible changes to the sandbox.
You can test the sandbox snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16612/merge/

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

You have made possible changes to the playground.
You can test the snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/

The snapshot playground with the CDN snapshot (only when available):

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/?snapshot=refs/pull/16612/merge

Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly.

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 16, 2025

You have made possible changes to the playground.
You can test the snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/

The snapshot playground with the CDN snapshot (only when available):

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16612/merge/?snapshot=refs/pull/16612/merge

Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly.

@bjsplat
Copy link
Collaborator

bjsplat commented May 16, 2025

You have changed file(s) that made possible changes to the sandbox.
You can test the sandbox snapshot here:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16612/merge/

@bjsplat
Copy link
Collaborator

bjsplat commented May 16, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented May 16, 2025

/**
*
*/
export interface INavigationEnginePlugin extends INavigationEnginePlugin0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

why extending INavigationEnginePlugin0 and not simply adding members/methods to INavigationEnginePlugin ?

@@ -392,7 +493,7 @@ export interface IAgentParameters {
/**
* Configures the navigation mesh creation
*/
export interface INavMeshParameters {
export interface INavMeshParameters0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need a name change as members are just added?

/**
* If set to true, intermediate objects used during the generation process will be retained.
* This is useful for debugging purposes or if you want to process the intermediate data in any other way.
* For example you can use it to trace countours, calculate heightfields, etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are functions used to get contours, heightfields exposed by the plugin?

*/
public constructor(recastInjection: any = Recast) {
public constructor(recastInjection: any = Recast, recastGeneratorsInjection: any = RecastGenerators) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to log the version here or in recast js to make the difference between old and new navigation


private _worker: Nullable<Worker> = null;

// TODO: Nullable?
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, when plugin just constructed but no navmesh. I'm wondering if it's possible to delegate this storage to recast.js instead.

@@ -70,7 +178,17 @@ export class RecastJSPlugin implements INavigationEnginePlugin {
*/
public setWorkerURL(workerURL: string | URL): boolean {
if (window && window.Worker) {
this._worker = new Worker(workerURL);
this._worker = new Worker(workerURL, {
Copy link
Contributor

Choose a reason for hiding this comment

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

got an error when testing PG #TN7KNN#2

Copy link

github-actions bot commented Jul 3, 2025

This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale".

@github-actions github-actions bot added the stale label Jul 3, 2025
@deltakosh
Copy link
Contributor

Should we close it for now @RolandCsibrei ?

@RolandCsibrei
Copy link
Contributor Author

RolandCsibrei commented Jul 18, 2025

Should we close it for now @RolandCsibrei ?

Greetings, Masters of the babylon.js Council!

I must confess — while the silence from the Council gave me time to shape the plugin in solitude, the answers came only after the stars had shifted. Now, as fate would have it, my duties lie elsewhere, and the pull of my current responsibilities is... strong.

Yet know this: my commitment to the plugin has not wavered. I shall return to this path and address your wise comments in the second week of August. The contribution shall be completed — as foreseen.

Thank you for your patience. May the render loop remain smooth, and your shaders unbroken.

Sorry, I just can't help myself to use AI to generate this kind of responses :D

As @CedricGuillemet has confirmed, It'll be put the addons.

Have a nice summer my friends!

@sebavan
Copy link
Member

sebavan commented Jul 18, 2025

This is probably one of the best github comment I have seen :-)

image

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

Successfully merging this pull request may close these issues.

5 participants