Skip to content
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

Also update the center of mass of dynamic bodies on syncToPhysics #50

Conversation

Elettrotecnica
Copy link

This enables the use case of forcefully repositioning a dynamic body without interrupting the physics.

I need this behavior for a feature where I respawn (reposition) an ammo dynamic entity when this leaves the playing area, and I think it makes sense in general.

…cToPhysics manually on an entity

This enables the use case of forcefully repositioning a dynamic body without interrupting the physics.
@diarmidmackenzie
Copy link
Member

Can you help me understand the use case better?

In the sweeper example, we have entities that leave the playing area and respawn. This is handled by removing the shape and body components, repositioning the object3D, then adding them back.

How are you re-positioning dynamic objects in your use case, and why is that better?

Perhaps we should make some updates to the notes here to explain the best way to move dynamic objects (while also emphasizing that in most cases dynamic objects should be moved by the physics engine).
https://github.com/c-frame/aframe-physics-system/blob/master/AmmoDriver.md#ammo-body-type

@Elettrotecnica
Copy link
Author

My use case is somewhat similar, but addresses an unintended behavior: I have items that because of unintended clipping may end up out of my playing area. This could happen, for instance, if I squeeze them too much against a corner of my scene.

Another situation, which seems to affect this specific version of Ammo/Bullet is that sometimes too small shapes may not collide correctly with the floor and would start falling indefinitely.

This is something I can live with, as it happens only in a minority of circumstances. The way I mitigate it is to make sure shapes are "big enough" and with a trick that you can see in this "bound-to-entity" component I have implemented: [1]

Basically, every second I check that the entity is "inside the scene" and if it is not I reposition it inside.

I could replace the ammo syncToPhysics lines with the trick in recycleBoxAmmo from your link, however, I find it unnecessary to destroy and recreate the whole ammo enchilada when I really just need to reposition it and go on exactly with the same behavior.

Note that this affects only syncToPhysics behavior when one calls it manually, e.g. like in my code. The beforeStep function where this is called automatically by the component will only invoke it on kinematic objects [2].

If you want I could try and see if the same approach can be applied also to the rain of entity example, e.g. if it really necessary to remove/reset the body and shape when recycling.

@diarmidmackenzie
Copy link
Member

diarmidmackenzie commented Oct 12, 2023

The 2nd answer here suggests that the object needs to be explicitly recorded as kinematic within Ammo/Bullet for the updated position to take:

"...the only solution I found to "teleport" an object is to:

  • set the body collision flag to KINEMATIC (2) - otherwise bullet just ignores the motionstate update
  • apply the new position (via syncToPhysics() when using aframe physics)
  • reset the body velocity, so the object won't bounce right after the teleport
  • restore the original flags (via updateCollisionFlags() when using aframe physics)"

It sounds as though that's different from what you are seeing, though? I wonder why.

Perhaps Piotr was actually hitting the line of code that you are proposing changing, didn't realize, and assumed it was something inside Ammo...?

I'm nervous enough about your approach (not knowing anything about the internals of Ammo, and not having the inclination to do a load of testing) that I wouldn't want to recommend it as the best way to move dynamic objects.

But if it seems to be working for you, I don't have an objection to this specific code change going in.

@Elettrotecnica
Copy link
Author

The way I see it, the StackOverflow question acknowledges that it does not work to move a dynamic body via syncToPhysics in current implementation of the component.

The trick of setting the body to kinematic is used to circumvent this, as these bodies are not influenced by the overall physics and won't be moved automatically: the changes from the simulation have the precedence on dynamic bodies and they would immediately overwrite any change from us.

IMHO, my change addresses this this limitation, because by moving the center of mass of the rigid body, both the graphical and the physical representations will be "in agreement" about where the object is currently.

I understand if you want to be careful with changes is these deep mechanics, I have this in my fork and does the trick for me, but you can leave the PR open for more feedback if you prefer.

@diarmidmackenzie
Copy link
Member

You may be right.

My issue with your interpretation is what Piotr says in the first point: "...otherwise bullet just ignores the motionstate update" - he attributes the issue to bullet rather than aframe-physics-system.

But it's possible he didn't fully understand why the update wasn't working.

I'm happy to merge the PR as is. What I'm not going to do at this point (which I had been considering) is update the documentation & examples to recommend this technique as a best practice for moving dynamic objects.

@diarmidmackenzie
Copy link
Member

Would you be able to make sure your branch is up-to-date with main, and then rebuild the files in dist, so they can be updated alongside the source code change as part of this PR?

@Elettrotecnica Elettrotecnica force-pushed the sync-center-of-mass-of-dynamic-objects branch from 2589e01 to 9f91b5f Compare October 14, 2023 22:36
@Elettrotecnica
Copy link
Author

Done, I have merged latest master with this branch and regenerated the distribution file

@diarmidmackenzie diarmidmackenzie merged commit fc7834e into c-frame:master Oct 16, 2023
4 of 6 checks passed
@diarmidmackenzie
Copy link
Member

Perfect. Thank you so much.

@Elettrotecnica Elettrotecnica deleted the sync-center-of-mass-of-dynamic-objects branch December 28, 2023 11:34
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.

2 participants