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

fix #ifdef for old damping method in btRigidBody #2748

Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

As discussed in godotengine/godot#19182 I've made some fixes to be able to use the old damping method.

When BT_USE_OLD_DAMPING_METHOD is defined:

  • setDamping allows any non-negative value instead of clamping between 0 and 1
  • Velocity changes use the old damping equation

@pouleyKetchoupp
Copy link
Contributor Author

CC @AndreaCatania

m_angularVelocity *= GEN_clamped((btScalar(1.) - timeStep * m_angularDamping), (btScalar)btScalar(0.0), (btScalar)btScalar(1.0));
#ifdef BT_USE_OLD_DAMPING_METHOD
m_linearVelocity *= btClamped((btScalar(1.0) - timeStep * m_linearDamping), btScalar(0.0), btScalar(1.0));
m_angularVelocity *= btClamped((btScalar(1.0) - timeStep * m_angularDamping), btScalar(0.0), btScalar(1.0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use max here, because it can't be more than 1, so we save one if condition.

m_linearVelocity *= btMax(btScalar(1.0) - timeStep * m_linearDamping, btScalar(0.0));
m_angularVelocity *= btMax(btScalar(1.0) - timeStep * m_angularDamping, btScalar(0.0));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've just pushed this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super!

@pouleyKetchoupp pouleyKetchoupp force-pushed the fix-old-damping-method branch from 29bb2c4 to 760960f Compare April 15, 2020 14:02
@erwincoumans
Copy link
Member

thanks, looks good to me.

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.

3 participants