Added support for shapeCast and merged raycastFirst and raycastAll into raycast

[MushAsterion]

• Requires to update Ammo.js to latest version. (which is not a breaking change)
• No custom IDL for Ammo.js required.

Deprecated RaycastResult for HitResult as it is now returned by shapeCast.
Added the following property for HitResult public API:

• distance: The distance at which the hit occurred from the starting point.

As shape may have lower hitFraction but be more distant than another hit with higher hitFraction (for example a side of the cylinder will be closest in distance than its other tip. (#5179)

Deprecated raycastFirst and raycastAll for a new function raycast defaulting to a "first" behavior, with possibility to use with option findAll = true to have all results.

raycast(start: Vec3, end: Vec3, options)

Added test and cast for shapes using shapeCast:

shapeCast(shape, startPosition: Vec3, startRotation: Vec3|Quat, endPosition; Vec3, options)

Where shape can either be a Ammo.btCollisionShape or an object defined as follows:

{
    type: "box"|"cylinder"|"capsule"|"cone"|"sphere",
    halfExtends: Vec3, // For type "box"
    radius: number, // For any type but "box"
    height: number, // For types "cylinder", "capsule" and "cone"
    axis: number // For types "cylinder", "capsule" and "cone"
}

And where available options are:

{
    sort: boolean,
    endRotation: Vec3|Quat,
    findAll: boolean, // Not available if different end transform
    filterCollisionGroup: number,
    filterCollisionMask: number,
    filterTags: string[]|string[][], // Not available if different end transform
    filterCallback: (entity: Entity) => boolean, // Not available if different end transform
    destroyShape: boolean // Forced true when shape is a configuration
}

Fixes #2094

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

[willeastcott]

So this is a utility function that basically calls the shape-specific functions. For the purpose of maintaining a nice and accessible API, should we:

1. just retain this 'all-in-one' function as public
2. just retain the shape-specific function as public
3. keep all public

[MushAsterion]

The advantage of "all-in-one" function is that you can actually use a JSON format for all the shape config meaning it can easily be assigned from a script attribute to edit it within the editor directly and avoid having to be only code-based. In another hand the shape-specific function allow users used to other engines to find the function easily (especially sphereCast which is really common).

[willeastcott]

Hmm, that's a great point. OK, fair enough. 😄

[willeastcott]

I wanted to unresolve this discussion and talk about it some more, @MushAsterion. I do still feel that this PR introduces a lot of additions to the API. 18 methods at the moment. And if shapeCastAll flavors get added, it takes it to 24. Plus the two existing raycasting functions, that'd be 26 functions for testing and casting. I think that's just too much. We could just have:

raycast (we could deprecate raycastFirst and raycastAll - note we don't _have_ to do that in this PR)
shapeCast
shapeTest

The all versus first could just be an option (maybe defaulting to first).

It just feels to me that the JSON approach is interesting, but it's very niche and, in reality, the vast majority of developers won't call the API like that.

[willeastcott]

You could potentially also drop shapeTest if start and end pos/rot of shapeCast where the same too maybe?

[MushAsterion]

shapeTestFirst is doing more work due to how Bullet/Ammo work and completely different from raycasting as raycasting is only a question of point while the closest point in a shape's position is not always the first to be detected since it depends on how the physics engine perform the test.

Once again the exposure of First/All methods is to match with what the engine has already implemented. I'm an external contributor so I don't want to step or change core project features. I personally find it handy for beginners to have the support for sure but as @willeastcott mentioned it's a very long list of new public methods which might also bring more confusion. I think it's a good solution to merge as much as possible to reduce public methods but I'm concerned about how easy it would be for people that are just looking to make a quick project without trying to get a deep understanding of the engine's API... I'll surely update to fit whatever they think is best anyway, the core features of the PR are already done and updating them is not a big deal

[LeXXik]

Good stuff! Many useful features are being added. Although, I do agree that the API is too verbose. We can simplify those, and move most of the customization stuff to options.

My vote goes for dropping -all/-first from the method name, and use an option setting.

shapeTestFirst seems a bit superfluous. We can just return everything and let user decide what to do with the results. If future Ammo adds a feature to allow collecting only the first intersection (which sounds strange to me), we can add it via an option. Shape and point collision tests are usually useful for getting all colliders they intersect, otherwise a sweep test is used.

About shape target rotation during sweep. Personally, I've never met a need to use it. Maybe it is useful in robotics, where Bullet engine is popular? No idea. Most of the other other physics engines don't allow it (Rapier, PhysX, Jolt). As such, I would actually move it to options as well.

Whatever the names are, but signatures could look something like:

raycast(start, end, opts = {});

shapecast(shape, pos, rot, dir = Vec3.ZERO, opts = {});

If dir is a zero vector, we do shape test. Otherwise it is a non-normalized cast direction (its normal for direction, its magnitude as cast distance). Shape properties can be part of the options, e.g. radius for radius-based shapes. We could also use the end position instead and check if it is same as start position, as Will suggested. Perhaps would be a better match to raycast then.

About destroying a shape used in sweep. If a shape is not auto-destroyed (e.g. via some option), then there should be a method to destroy it later. Never mind, it is for a user created one, which he is in control of.

[MushAsterion]

I'm not sure to understand the point of changing endPosition into dir while raycast has a end param, is there any particular reason to make it different?

Something like

shapecast(shape, start, end, opts = {})

Where options could have something like

{
    startRotation: Vec3|Quat,
    endRotation: Vec3|Quat,
    findAll: boolean, // Defaults to false, not possible if end transform !== start transform
    // ... other options
}

Would be more fitting with the suggested raycast signature.

[LeXXik]

Yep, something like that. I think starting rotation is probably not in options, as it is often used for non-spheres. Target rotation can be.

[MushAsterion]

Alright so based on feedback I just rewrote the PR. Let me know if it better fits now 🙃

[mvaligursky]

Nice PR! Two comments:

• does it work with the ammo version PlayCanvas uses by default? If not, we should update the version at least in the examples: https://github.com/playcanvas/engine/tree/main/examples/src/lib/ammo

• I'd love to have an example for this here in the physics section: https://github.com/playcanvas/engine/tree/main/examples - could be done in a separate PR as well, but it'd be awesome to have a test we can run before each engine release.

[MushAsterion]

Thanks! And thank you for your review.

Regarding your questions @mvaligursky,

• does it work with the ammo version PlayCanvas uses by default? If not, we should update the version at least in the examples: https://github.com/playcanvas/engine/tree/main/examples/src/lib/ammo

I tested it within the production version from online editor and re-checked after with engine-only to make sure it was working. Here is the project I made to do it: https://playcanvas.com/project/1034600.

• I'd love to have an example for this here in the physics section: https://github.com/playcanvas/engine/tree/main/examples - could be done in a separate PR as well, but it'd be awesome to have a test we can run before each engine release.

To be completely honest with you I'm far from being the best on creating full project, I'll leave this to someone else.

[willeastcott]

@MushAsterion I think it's OK to skip the example if you're not confident about putting one together. We can maybe open another issue requesting that...

[daredevildave]

Any update on this PR? Sounds like the latest version of ammo supports this PR, is that supported by the engine/editor?

[MushAsterion]

Editor must be updated to latest version of Ammo.js, I was multiple times mentioned by @willeastcott about it coming but still waiting for it.

[willeastcott]

@MushAsterion I just chatted to @slimbuck about this. He's busy working on a SuperSplat update at the moment but he'll deploy the latest ammo.js as soon as he's able.

[willeastcott]

@MushAsterion We just checked our current Ammo version. It has both convexSweepTest and ClosestConvexResultCallback. Is there anything else we might be missing?

[MushAsterion]

@MushAsterion We just checked our current Ammo version. It has both convexSweepTest and ClosestConvexResultCallback. Is there anything else we might be missing?

Not too sure, actually checking from my previous edits of original post it requires exposure of:

btCollisionObject on ClosestConvexResultCallback (for shapeCastFirst) and the ConvexResultCallback constructor (for shapeCastAll)

Just tested and actually everything works fine with latest Ammo version from Editor

[LeXXik]

LGTM with minor comments