Bug with nonzero InTargetValueRate values passed in to FMath::CriticallyDampedSmoothing()

Development Programming & Scripting
, ,
1

Problem:

  • In FMath::CriticallyDampedSmoothing() a factor of InTargetValueRate * InSmoothingTime is applied 2 times when it only needs to be applied once. This creates unexpected behaviour when nonzero values of InTargetValueRate are input.
  • The first time it is applied when creating AdjustedTarget:
    • // Target velocity turns into an offset to the position
    • T AdjustedTarget = InTargetValue + InTargetValueRate * InSmoothingTime;
  • It is then applied a second time when creating X0:
    • T X0 = InOutValue - AdjustedTarget;
    • // Target velocity turns into an offset to the position
    • X0 -= InTargetValueRate * InSmoothingTime;
  • This means that X0 has this factor applied twice since X0 already includes AdjustedTarget

Solution:

  • This line seems unneeded: X0 -= InTargetValueRate * InSmoothingTime;

Reasoning:

  • FMath::SpringDamperSmoothing() has a critically damped case with very similar code except it does not have this extra line
  • When providing the same values to FMath::SpringDamperSmoothing() and FMath::CriticallyDampedSmoothing() (where InTargetValueRate is nonzero and the SpringDamperSmoothing had a damping factor of 1) I found:
    • FMath::SpringDamperSmoothing() behaved as expected
    • FMath::CriticallyDampedSmoothing() behaved unexpectedly

[Attachment Removed]

2

Steps to Reproduce
If you pass in the same nonzero InTargetValueRate to FMath::CriticallyDampedSmoothing() and FMath::SpringDamperSmoothing() (where the latter has InDampingRatio = 1 to be comparable) you will get different behaviours.

The FMath::CriticallyDampedSmoothing() is the incorrect one.

[Attachment Removed]

3

Yes - you’re absolutely right, and thanks for the note about this.

I confirm that removing these lines is correct, as the behaviour has already been included in AdjustedTarget:

// Target velocity turns into an offset to the position

X0 -= InTargetValueRate * InSmoothingTime;

Sorry, I was a bit slow to respond. I spent some time checking the maths and adding tests for the different cases and functions. So far, I haven’t found any other problems!

[Attachment Removed]

4

Thanks for looking into this Danny! I’ll let our team know.

[Attachment Removed]