1
0
mirror of https://github.com/ppy/osu.git synced 2026-05-22 22:20:53 +08:00

Fix taiko swell ending samples playing at results sometimes

Closes https://github.com/ppy/osu/issues/32052.

Sooooo... this is going to be a rant...

To understand why this is going to require a rant, dear reader, please
do the following:

1. Read the issue thread and follow the reproduction scenario (download
   map linked, fire up autoplay, seek near end, wait for results, hear
   the sample spam).
2. Now exit out to song select, *hide the toolbar*, and attempt
   reproducing the issue again.
3. Depending on ambient mood, laugh or cry.

Now, *why on earth* would the *TOOLBAR* have any bearing on anything?

Well, the chain of failure is something like this:

- The toolbar hides for the duration of gameplay, naturally.
- When progressing to results, the toolbar gets automatically unhidden.
- This triggers invalidations on `ScrollingHitObjectContainer`. I'm not
  precisely sure which property it is that triggers the invalidations,
  but one clearly does. It may be position or size or whichever.
- When the invalidation is triggered on `layoutCache`, the next
  `Update()` call is going to recompute lifetimes for ALL hitobject
  entries.
- In case of swells, it happens that the calculated lifetime end of the
  swell is larger than what it actually ended up being determined as at
  the instant of judging the swell, and thus, the swell is *resurrected*,
  reassigned a DHO, and the DHO calls `UpdateState()` and plays the
  sample again despite the `samplePlayed` flag in `LegacySwell`, because
  all of that is ephemeral state that does not survive a hitobject
  getting resurrected.

Now I *could* just fix this locally to the swell, maybe, by having some
time lenience check, but the fact that hitobjects can be resurrected by
the *toolbar* appearing, of all possible causes in the world, feels
just completely wrong. So I'm adding a local check in SHOC to not
overwrite lifetime ends of judged object entries.

The reason why I'm making that check specific to end time is that I can
see valid reasons why you would want to recompute lifetime *start* even
on a judged object (such as playfield geometry changing in a significant
way). I can't think of a valid reason to do that to lifetime *end*.
This commit is contained in:
Bartłomiej Dach
2025-02-24 14:30:55 +01:00
Unverified
parent e9b8154090
commit 41db3c1501
@@ -247,7 +247,12 @@ namespace osu.Game.Rulesets.UI.Scrolling
// It is required that we set a lifetime end here to ensure that in scenarios like loading a Player instance to a seeked
// location in a beatmap doesn't churn every hit object into a DrawableHitObject. Even in a pooled scenario, the overhead
// of this can be quite crippling.
entry.LifetimeEnd = entry.HitObject.GetEndTime() + timeRange.Value;
//
// However, additionally do not attempt to alter lifetime of judged entries.
// This is to prevent freak accidents like objects suddenly becoming alive because of this estimate assigning a later lifetime
// than the object itself decided it should have when it underwent judgement.
if (!entry.Judged)
entry.LifetimeEnd = entry.HitObject.GetEndTime() + timeRange.Value;
}
private void updateLayoutRecursive(DrawableHitObject hitObject, double? parentHitObjectStartTime = null)