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

Do not remove ragdoll collision layers after simulation disabled #34313

Closed

Conversation

Acvarium
Copy link
Contributor

Those two lines that I removed clear collision layers and masks for each bone, if simulation was disabled.
With that collision detection doesn't work any more.
Here is how it works right now:
https://youtu.be/H0wkiiwg4bI

And here is how it has to work, and is without those lines:
https://youtu.be/m2KjrekxD0Y

(in videos the act of shooting enable a simulation, and a keypress disable it and returns character to default position)

If game developer want to clear collision layers, he/she can do that in the script.

@AndreaCatania
Copy link
Contributor

After the deactivation the physical skeleton are supposed to remain inplace.
you can see that this function _stop_physics_simulation not only change the layer and the mask but also change the body mode to static.

The correct approach is to add another state (Kinematic):

  • Active (The bones are fully simulated by the physics)
  • Kinematic (The physical bones are moved by the animation)
  • Deactivated (The physical bones are fully deactivated)

@Acvarium
Copy link
Contributor Author

About, "The physical bones are moved by the animation", as I can see, it doesn't work at all. For bones to move with animation with simulation disabled, I added a workaround in game script, in form of 3 lines, that go thru each physic bone and correct it's transform to transform of a corresponding animation bone

@AndreaCatania
Copy link
Contributor

AndreaCatania commented Dec 14, 2019 via email

@AndreaCatania
Copy link
Contributor

This feature got already integrated by this one #36008

@aaronfranke
Copy link
Member

@AndreaCatania Does that mean that this PR should be closed?

@Acvarium If this still desired, it needs to be rebased on the latest master branch.

@AndreaCatania
Copy link
Contributor

Yep, it's already implemented.

@akien-mga
Copy link
Member

Closing then. Thanks for the contribution nevertheless!

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

Successfully merging this pull request may close these issues.

5 participants