osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

Interpolation functions should be documented/refactored to better define behaviour beyond extents

Open peppy opened this issue 5 years ago • 1 comments

As noticed in https://github.com/ppy/osu/pull/10659, it is quite easy to assume (incorrectly) that interpolation functions clamp at their start/endTime extents (give the names of parameters it sounds like clamping may be enforced).

I'd propose that, unless we use these functions in a way which requires it somewhere, clamping is enforced. If that's not agreed upon, at very least documenting this behaviour in xmldoc may help avoid future similar cases.

If it is decided that behaviour is changed, local changes in the aforementioned PR can be reverted.

peppy avatar Nov 02 '20 13:11 peppy

As a data point, this is already done locally in many places in the framework:

https://github.com/ppy/osu-framework/blob/c765605199c2cb23223cb38d5d809781337d02c8/osu.Framework/Graphics/Transforms/TransformBindable.cs#L24-L30

https://github.com/ppy/osu-framework/blob/c765605199c2cb23223cb38d5d809781337d02c8/osu.Framework/Graphics/Transforms/TransformCustom.cs#L171-L177

https://github.com/ppy/osu-framework/blob/c765605199c2cb23223cb38d5d809781337d02c8/osu.Framework/Graphics/TransformableExtensions.cs#L766-L772

On the other side I can see how not clamping can be sometimes useful, but at that point it's not really interpolation anymore rather than extrapolation. And it doesn't really work for some easing types, either (something like OutElastic will not extrapolate well, I imagine).

As for my opinion I would probably make it a default-but-opt-out behaviour by having a clamp param defaulting to true. That change might hurt though, as it's at the level of "too many usages to actually check if every single one behaves correctly".

bdach avatar Nov 02 '20 17:11 bdach