Implement interaction groups test mode and cofficient combine rule #741
Conversation
Ughuuu
commented
Sep 22, 2024
Ughuuu
commented
- Continues Add InteractionGroups::test_or, increase to 64 bits #170
- Fixes Feature: Add support for custom friction and bounce CombineRules #622
|
Ok, added this mainly to stop using my fork and to try to use rapier default in godot-rapier. This way I hope to no longer need to maintain that and use this repo as it is. (if this gets merged) |
Ughuuu
force-pushed
the
Add-interaction-or,-increase-to-64-bit-and-add-more-rules-take-2
branch
3 times, most recently
from
b05dc45 to
1d0db7e
Compare
update fix docs cargo format fix update feedback update Update CHANGELOG.md ignore line for error docs fix update
Ughuuu
force-pushed
the
Add-interaction-or,-increase-to-64-bit-and-add-more-rules-take-2
branch
from
bb2b387 to
0db7c72
Compare
|
Fixed the doc test from CI. |
| @@ -27,8 +29,10 @@ impl CoefficientCombineRule { | |||
|
|
|||
| match effective_rule { | |||
| 0 => (coeff1 + coeff2) / 2.0, | |||
| 1 => coeff1.min(coeff2), | |||
| 1 => coeff1.min(coeff2).abs(), | |||
There was a problem hiding this comment.
(outside of this PR's scope) @sebcrozet why does this function takes u8 rather than CoefficientCombineRule ?
There was a problem hiding this comment.
Should it be max(0.0) ?
I think there is some use-case for this in godot but that would be worth clarifying by @Ughuuu.
(outside of this PR's scope) @sebcrozet why does this function takes u8 rather than CoefficientCombineRule ?
I don’t think there is a particular reason for this.
There was a problem hiding this comment.
Not sure about the godot use case, this is just the exact code they have right now, so for parity I have that. Their use case is that they also have negative coefficients, and use this with absorbent field.
https://github.com/godotengine/godot/blob/0eadbdb5d0709e4e557e52377fa075d3e2f0ad1f/scene/resources/physics_material.h#L57
Then they do abs here, so the negative one becomes positive:
https://github.com/godotengine/godot/blob/0eadbdb5d0709e4e557e52377fa075d3e2f0ad1f/modules/godot_physics_3d/godot_body_pair_3d.cpp#L259
There was a problem hiding this comment.
Should i add a comment in code around that match explaining that the coefficient can be negative in some cases?
There was a problem hiding this comment.
Should i add a comment in code around that match explaining that the coefficient can be negative in some cases?
Yes ; you can add a link to godot documentation so the reader knows where this comes from 👍
| @@ -27,8 +29,10 @@ impl CoefficientCombineRule { | |||
|
|
|||
| match effective_rule { | |||
| 0 => (coeff1 + coeff2) / 2.0, | |||
| 1 => coeff1.min(coeff2), | |||
| 1 => coeff1.min(coeff2).abs(), | |||
There was a problem hiding this comment.
Should it be max(0.0) ?
I think there is some use-case for this in godot but that would be worth clarifying by @Ughuuu.
(outside of this PR's scope) @sebcrozet why does this function takes u8 rather than CoefficientCombineRule ?
I don’t think there is a particular reason for this.
Co-authored-by: Sébastien Crozet <[email protected]>
Co-authored-by: Sébastien Crozet <[email protected]>
