Fix osu!catch fruit rotation being applied too late #29894
Conversation
peppy
force-pushed
the
fix-fruit-rotation
branch
from
d844ee8
to
3f44224
Compare
|
|
||
| ScalingContainer.RotateTo(startRotation).RotateTo(startRotation + 720, duration); | ||
| double preemptProgress = (Time.Current - (HitObject.StartTime - InitialLifetimeOffset)) / (HitObject.TimePreempt + 2000); |
There was a problem hiding this comment.
Why is this clamped in DrawableBanana (albeit only from above to 1, rather than from both sides), and not here? Interpolation.Lerp() does no clamping on its own, so it will extrapolate when given arguments outside [0;1].
There was a problem hiding this comment.
The clamping in DrawableBanana is because we assume that scale shouldn't continue to decrease after reaching the final value. For rotation, I want it to continue in both directions for sanity reasons (assume it's always spinning).
Also for bananas, they end up on the plate (where they shouldn't rotate) while droplets fly off the plate immediately, where I'd expect them to continue rotating (although they don't seem to for some reason, wasn't really interested in finding out why).
This is the same reason clamping is only applied to max not min for scale – to allow extrapolating values for the range which previously didn't animate.
I can add inline comments if this isn't clear.
There was a problem hiding this comment.
Inline comments could be useful because all that makes sense but is also very non-obvious.
There was a problem hiding this comment.
Will do. Also having talked through the above I realised that bananas will look wrong if you don't catch them, will fix that.
| // roughly matches osu-stable | ||
| float startRotation = RandomSingle(1) * 20; | ||
| double duration = HitObject.TimePreempt + 2000; | ||
| // Important to have this in UpdateInitialTransforms() to it is re-triggered by RefreshStateTransforms(). |
There was a problem hiding this comment.
Arguably, we could move all of these Random fetches to Update and nuke RefreshStateTransforms completely (seems only used in catch)...
There was a problem hiding this comment.
I think having it where it is right now is preferable. I'm not sure putting that in update would end well.
In passing
Untitled.Project.1.mp4
Untitled.Project.2.mp4
I still think we should do more hitobject animations using this style. Really gets rid of the mental overhead of transform / rewind gymnastics.