Bitecs-ammo-shape

[keianhzo]

This PR adds support for ammo-shape, heightfield, trimesh and box-collider components.

[netpro2k]

This is getting close but I think there are some changes that need to be made. The biggest thing I see is that I would expect the deprecated shape-specific components to only be a concept at the GLTF inflator level. After that they should all end up as just PhysicsShape components and be treated the same in the rest of the code.

Also unless I am misremembering how the physics system works, FIT.ALL should not need any special handling from us at the bitecs level, its still 1 PhysicsShape to 1 shapeId in the physics system. What FIT.ALL does is inform the thing generating for example a Trimesh shape that it should traverse the hierarchy. I would eventually like to get rid of it as a concept.

I banged my head against the offset stuff as well in my PR and I don't think I fully fixed it. That said I want to make sure we don't leak too much of the current issues in the physics system into our new code.

Its worth looking at where I ended up as well. Its definitely a good deal more hardcoded and specific to just what I needed to get object handling going, but it might be helpful anyway 4e7ce59#diff-ec2d8825470197103fe423076d6fa5c0b5c995b4ff3c17a40998595e24c1d553R2

[netpro2k]

Pretty confused what this is doing., though I suspect the confusion is rooted in the way its handled in the physics system itself... I don't see code like this in the old "shape-helper" component though. I did have to do a slight hack like this in the branch I am working on but I am confused why the various depercated components have influence here.

Also I am like 95% sure the existing physics components don't all work quite correctly. Most scenes do not have collisions so I would rather er on the side of cleaner code than 100% perfect reproduction of existing scenes. Offsets are definitely one of the areas I think we can majorly clean things up.

[keianhzo]

The reason for this is that when adding objects without a Rigid body, we default to the closest parent rigid body and if we don't set offsets the shape will use the parent transform so the shapes will be incorrectly positioned. This is usually the same if you add an ammo-shape to a scene.

For low density scenes the old loader adds a Fit.ALL Mesh collider in traverseMeshesAndAddShapes for all the environment meshes but it's not respecting the users design in case there are ammo-shapes. For high density scenes the old loader only sets a floor Box collider and all the ammo shapes are wrongly placed at the scene origin.

[netpro2k]

We should check if this is correctly handled in the old code. If not (probably true) I doubt anyone is actually using the components. We might just keep the old broken behavior and can think through how to simplify. Worth noting that even in the aframe code we only actually have a shape-helper component. trimesh, box-collider, and heightfield are only in gtlf-component-mappings.

[keianhzo]

Yes, this is also broken in the old code and I agree that probably people don't use it. I thought it would be worth fixing but if it's not that simplifies the code and we can remove all those tag components.

[netpro2k]

Yeah if its not a regression I think we should fix it by thinking a bit more about what we want to be configurable in Blender.

[netpro2k]

This is looking much better. The legacy stuff is nicely contained off to the edges and we can start to see a nicer system starting to poke through under some of the noise. Made a few stylistic comments but I don't think anything major there.

The change in modelLoader for Object3D check is actually a bug though and I think needs to be reverted.

The main issue I can see from a usability perspective is the auto generation of shapes for scenes. I think given the reality of existing scenes we probably need something a bit smarter even though its going to carry over some legacy complexity. It should hopefully be able to be localized in just sceneLoadingSystem so it wont be too bad.

I only just glanced at the MediaFrame changes. Didn't realize you got all that working with new objects, very nice. I do think splitting that off into its own PR makes sense after all, since it probably deserves its own review process.

[netpro2k]

Seems suspicious we have updateRigidiBody, updateRigidBodyOptions, updateBody, and updateBodyOptions. I think updateBodyOptions was something I added for use in our bitECS systems (did basically what updateBody did, but slightly more specific), I think directly passing in entity id makes some sense. So we can use your function instead. Not sure where I land on this function also updating the rigidbody params via updateRigidbodyParams since I kind of want to slowly move towards that being the source of truth. As in, you set all the rigidbody properties how you like, and then call updateRigidbody(eid). Not super important to change that right now but we should at least try and cut down the number of very similar functions.

[keianhzo]

Yeah, it's a bit unfortunate that we have to go back and forth between bitECS and Ammo to keep everything in sync. As the entry point for the physics life cycle is the physics system, I wonder it would make sense to encapsulate all the component life cycle there and let that system manage it all. Instead of going through the hops of adding a component and have that intermediate bit-physics-system to map that to the physics system, just call the physics system and it will add/update the component under the hood and keep everything in sync.

At least this will save some method calls and also remove that bit-physics bridge system.

[netpro2k]

getBodyFromRigidBody create a new object so I don't thin we need to worry about mutating curretnPrams, we can skip the {} here.

But I also wonder if we can just skip the intermediate object all together, we have params and we want to overwrite any properties in RigidBody that params contains. I think we can get rid of getBodyFromRigidBody and updateRigidBody and updateRigidBody and just have a bunch of lines that look like
if(params.linerDampining !== undefined) Rigidbody.linearDamping[eid] = params.linearDampining.

[netpro2k]

Ah, though I did miss that we are exporting getBodyFromRigidBody

[netpro2k]

This is looking much simpler now!

Asked to merged privately