Added support for `shapeCast` and merged `raycastFirst` and `raycastAll` into `raycast`

[MushAsterion]

• [x] Requires to update Ammo.js to latest version. (which is not a breaking change)
• [X] 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.

[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...

[willeastcott]

Something else occurred to me. We have already have raycastFirst and raycastAll. Should this new API map to this? So we'd have:

• boxCastFirst and boxCastAll
• coneCastFirst and coneCastAll
• capsuleCastFirst and capsuleCastAll
• cylinderCastFirst and cylinderCastAll
• shapeCastFirst and shapeCastAll
• sphereCastFirst and sphereCastAll

[willeastcott]

@MushAsterion By the way, I'm sorry if this review seems a bit overly rigorous and sometimes pedantic! Just want to make sure we get everything perfect as possible - but I'm super-excited to get this merged! 😄

[willeastcott]

I have another question. I see how to specify the shape we want to cast and where to position/orient it initially. But how do you specify the direction and/or distance you want to cast the shape? Can you maybe give a code example showing, say, how you'd do a sphere cast (radius 1) from world space position [1, 2, 3] 10 units in the direction [1, 1, 1]?

[kungfooman]

But how do you specify the direction and/or distance you want to cast the shape?

As far as I understand this, it just does "in place testing". I think what you are asking for could be achieved via:

pc.app.systems.rigidbody.dynamicsWorld.convexSweepTest

[willeastcott]

As far as I understand this, it just does "in place testing". I think what you are asking for could be achieved via:

Oh. Well, in that case, it's not casting, right? I thought casting is analogous to sweeping. I'd say names like sphereTest are more accurate than sphereCast, IMHO.

[willeastcott]

I should point out that the original poster on the forum was asking for spherecasting that is analogous to raycasting. i.e. a swept sphere. And I should probably also point out that Unity's API does sweeping when using this naming. Check it out:

https://docs.unity3d.com/ScriptReference/Physics.SphereCast.html

[MushAsterion]

That's a good point, I renamed the existing functions and am adding the proper functions for casting using sweeping then!

[MushAsterion]

But how do you specify the direction and/or distance you want to cast the shape?

As far as I understand this, it just does "in place testing". I think what you are asking for could be achieved via:

pc.app.systems.rigidbody.dynamicsWorld.convexSweepTest

Problem is that the current Ammo version doesn't expose the btCollisionObject on ClosestConvexResultCallback (for shapeCastFirst) and the ConvexResultCallback is not exposed at all (for shapeCastAll).

I however added boxCast and sphereCast function based on shapeTest. For other shapes it would require to expose more of the Bullet API...

[kungfooman]

Thank you for your quick changes! I think now the only thing thats left is what @willeastcott wrote about, to achieve a unified API:

boxCastFirst and boxCastAll

Since Ammo seems to have no proper sweeping API boxCastFirst needs to be implemented using a distance-squared-check of the boxCastAll results?

[MushAsterion]

Thank you for your quick changes! I think now the only thing thats left is what @willeastcott wrote about, to achieve a unified API:

boxCastFirst and boxCastAll

Since Ammo seems to have no proper sweeping API boxCastFirst needs to be implemented using a distance-squared-check of the boxCastAll results?

I updated the functions to be boxCastAll and sphereCastAll, however I have no idea how Ammo found the points when performing shape casting therefore nothing proves the closest point from start will be the actual first result so I'm not confident in adding something that could lead in wrong results to the code. For it to be proper boxCastFirst/sphereCastFirst it will require real sweeping I think.

[LeXXik]

Thank you for the PR!

Convex cast or collision tests are useful in kinematics, when the developer wants to know if the spatial variables (position/orientation) of the rigidbody are ok for the next simulation step or we have a contact and we ought to do something. It is also often used for camera position test to prevent it from clipping through. Camera is using a sphere, which is fine, but character is usually a capsule, which we cannot do without upgrading Ammo (or unless we do some custom convex hull shape mumbo-jumbo, which we shouldn't). However, since we are offering boxCastAll, I would assume it is using a sweep test (from the name), and I would wonder how to orient the shape I am sweeping. Obviously we cannot change orientations with a collision test, so I would rename it to boxTestAll or something.

As for the first contact - testing the result distance from the start, suggested by @kungfooman, is a solid suggestion, and works fine. We use the method in production no problem. The closest contact to start is first in distance and. as a result, first in time (even if it was added to the contacts list sometime in the middle). You can add boxTestFirst, which does boxTestAll internally and simply finds the closest result from the list. Just add a comment for the future contributor that it should be changed to convex sweep test once Ammo version gets updated, as it would be more performant.

[MushAsterion]

Alright thank you for your feedback.

I removed boxCallAll until Ammo get upgraded to include shape rotation.

Regarding the first result I don't see how would the engine know which tip my start point would be so the point be on the good side of a shape being in the middle of the cast. Do you have an example with it working on all directions?

[LeXXik]

Regarding the first result I don't see how would the engine know which tip my start point would be so the point be on the good side of a shape being in the middle of the cast. Do you have an example with it working on all directions?

Ah, you are right. I recall now, I was adding some additional logic to filter the test. I think we can go without it, then. As long as we show all contacts, the developer will have an opportunity to select the one needed.

[willeastcott]

Sorry I've been a bit quiet this week - I've been travelling. Anyway, so it sounds like we would want to use this to do shape casting:

https://github.com/kripken/ammo.js/blob/main/ammo.idl#L888

You say:

Problem is that the current Ammo version doesn't expose the btCollisionObject on ClosestConvexResultCallback (for shapeCastFirst) and the ConvexResultCallback is not exposed at all (for shapeCastAll).

Are you saying we need to grab the latest ammo.js build from here:

https://github.com/kripken/ammo.js/tree/main/builds

...replace the version currently added to the Editor? We can absolutely do that if this requires it.

My concern is that we deploy the current implementation of sphereCastAll that you have here, and then we update the implementation later to use convexSweepTest and the returned results differ somehow. I would rather we update ammo.js and have the initial implementation using convexSweepTest personally.

[willeastcott]

Also, one other minor thing. Whenever you update any changes to public API in the PR, please update your original PR description to reflect the latest API state. It's really useful to be able to clearly see at a glance how a PR will alter the public API.

[MushAsterion]

Are you saying we need to grab the latest ammo.js build from here:

https://github.com/kripken/ammo.js/tree/main/builds

...replace the version currently added to the Editor? We can absolutely do that if this requires it.

I don't specially mean update the version as it will be a real breaking change as many functions changed since the version that you use. I only mean expose the property and class needed:

• ClosestConvexResultCallback#m_hitCollisionObject ([Const] attribute btCollisionObject m_hitCollisionObject;)
• ConvexResultCallback, problem is that the class is only meant to be an interface which means we have no real way to have all the results with the current version and from my tests changing the addSingleResult from ClosestConvexResultCallback doesn't change the results function...

My concern is that we deploy the current implementation of sphereCastAll that you have here, and then we update the implementation later to use convexSweepTest and the returned results differ somehow. I would rather we update ammo.js and have the initial implementation using convexSweepTest personally.

I got the point and yes result order will probably differ as it will check for a full shape (capsule) at once instead of starting at a point and going to another.

Also, one other minor thing. Whenever you update any changes to public API in the PR, please update your original PR description to reflect the latest API state. It's really useful to be able to clearly see at a glance how a PR will alter the public API.

It's updated, sorry for that

[willeastcott]

I don't specially mean update the version as it will be a real breaking change as many functions changed since the version that you use.

Really?? In the past, we have been able to occasionally update the version of Ammo available in the Editor to the latest build in the Ammo.js repo. It's always been backwards compatible until now. Are you completely sure that taking the latest build will cause breakage to existing projects?

[MushAsterion]

Well when I was trying to use latest version on NodeJS it was showing me some errors, or maybe I did anything wrong on my end, possible as well, I'm far from being an expert... But yeah from the Bullet docs some classes doesn't exist anymore and it in fact include the ConvexResultCallback, unless Ammo is not matching completely the Bullet API?

[MushAsterion]

Oh after looking at some other classes within the IDL file I found it's possible to expose the addSingleResult function from ClosestConvexResultCallback by adding: float addSingleResult([Ref] LocalConvexResult convexResult, bool normalInWorldSpace);

This way it's possible by modifying the idl file only!

[MushAsterion]

Really?? In the past, we have been able to occasionally update the version of Ammo available in the Editor to the latest build in the Ammo.js repo. It's always been backwards compatible until now. Are you completely sure that taking the latest build will cause breakage to existing projects?

Ok so I was just idiot and importing it the wrong way. It's in fact all backwards compatible and works well with latest build... 🤦

[MushAsterion]

Added all shapeCastFirst and linked primitive shape function using convexSweepTest with proper assertion on _shapeCastFirst. All it needs now is exposure of Ammo.ClosestConvexResultCallback.#get_m_hitCollisionObject for it to work.

[willeastcott]

Ok so I was just idiot and importing it the wrong way. It's in fact all backwards compatible and works well with latest build... 🤦

LOL. That's great that you managed to implement 'true' shape casting. So you now have the 'First' functions defined and I have a couple of questions:

1. Did you want to also provide 'All' functions (that return all hits)?
2. From an API perspective, would it be better to have shapeCastFirst and shapeCastAll or to have shapeCast(arg1...argN, findAll = false)?
3. Anything else you think is important to include in this PR before it gets merged?

It feels like we're getting very close with this now! 🚀

[MushAsterion]

1. It would be nice but I'm not even sure it's possible with Ammo.js as ConvexResultCallback is a virtual class and ClosestConvexResultCallback is limited to closest.
2. As you mentionned in a previous comment the First/All match the existing names for raycastFirst/raycastAll so I guess it's better this way. But for sure if we get to implement a All version for sphereCast and a First version for sphereTest it can still be used to reduce number of lines and keep the First/All functions for consistency.
3. Only the shapeCastAll function if we go for it from (1)

[MushAsterion]

Just made T and C from shapeTest and shapeCast lowercase to keep consistency with raycast.

[willeastcott]

Sorry to be annoying after you've made the change @MushAsterion, but raycast is a word, whereas boxcast is not. I would stick with boxCast etc. We could alias raycast to rayCast....but I'm not entirely sure we should since, like I say, it is actually a word. I do also note that Unity used a small 'c' for Raycast and a capital 'C' for BoxCast. If we did standardize, I would go with rayCast though.

[MushAsterion]

Oh alright my bad. That's reverted.

[MushAsterion]

Hello @willeastcott, I'm getting back to this PR,

I added all the shapeTestFirst functions which work based on the first returned result, while there is no documentation on this result being the closest it still provide users with a computation that is slightly faster than the All counterpart (shorter on PC side, Ammo is still computing all results).

I also went ahead and tested for shapeCastAll however it does not work by updating the Ammo.ClosestConvexResultCallback#addSingleResult and as Ammo.ConvexResultCallback is only a virtual class there is no real class provided to handle this situation making it impossible to add.