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

Add generic Lerp and Damp overloads #3939

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

smoogipoo
Copy link
Contributor

It's slower than the previous method, but it's on the level of transforms anyway.

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
LerpFloat 7.588 ns 0.0409 ns 0.0320 ns - - - -

@bdach
Copy link
Collaborator

bdach commented Oct 17, 2020

I see a big problem with this, API-wise. If you write Interpolation.Lerp(0, 10, 0.25f) in an expression, what should the result be?

You'd probably expect the answer to be 2.5, but with the way this is written right now, what you'll get is 2. This is of course due to generic type inference rules crossing with one of the worst characteristics of numeric literals. In the example above the compiler uses an Interpolation.Lerp<int>. This in particular means that at current we'd have to check every single game-side usage of Lerp to avoid breakage, and so would the consumers.

To look at it from a different side, this would allow using Lerp directly (without casts) on the following types:

  • ColourInfo
  • EdgeEffectParameters
  • SRGBColour
  • Colour4
  • byte, sbyte, short, ushort, int, uint, long, ulong, float, decimal
  • Vector2
  • RectangleF

That's a pretty big list which makes me think that maybe this should indeed be a thing and we should try and go through with this change for however much pain it might cause. Most things on the list are linearly-interpolable, it's just the integral types I have the biggest problem with, as it's a very easy to produce code with the new generic method that looks reasonable but actually does something different, just because there's no generic type spec / a literal annotation (be it f or d).

@smoogipoo
Copy link
Contributor Author

Ouch, I agree that's pretty bad. As an alternative, we could instead use the signatures:

T ValueAt(T start, T end, double amount);
T Dampen(T start, T final, double @base, double exponent);

And maybe marking the existing ones as obsolete?

@bdach
Copy link
Collaborator

bdach commented Oct 19, 2020

While that would avoid the need for checking all usages, it doesn't change fact that generics strongly work against this usage pattern. The generic will still bind to int if bare non-fractional literals are passed to start and final; even worse, this seemingly innocuous line is also completely wrong due to implicit conversions running after the generic call:

double value = Interpolation.Lerp(0, 10, 0.25d); // is actually 2...

I don't know whether this is salvageable to do the "expected" thing without doing

public static double Lerp(int start, int end, double amount) => Lerp<double>(start, final, amount);
public static double Lerp(long start, long end, double amount) => Lerp<double>(start, final, amount);
// you get the idea

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.

2 participants