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

`ScreenStack` likely removes screens one frame too early

Open smoogipoo opened this issue 10 months ago • 1 comments

ScreenStack implements its own children life updates:

https://github.com/ppy/osu-framework/blob/37e14754299553057b9c2d77412567c1794cdc1d/osu.Framework/Screens/ScreenStack.cs#L377-L395

When screens are exited they are .Expire()d, calculating their expiry time as LatestTransformEndTime:

https://github.com/ppy/osu-framework/blob/37e14754299553057b9c2d77412567c1794cdc1d/osu.Framework/Screens/ScreenStack.cs#L315-L319

Which means that the intended usage of screens is something like:

void OnExit()
{
    this.FadeOut(1000);
    // Expecting the screen to die after 1000ms.
}

The problem is that UpdateChildrenLife occurs prior to children being updated, which may cause unintended effects if the transforms aren't updated for the final time. It's likely fine for something like a FadeOut() because the final frame will already be at a low alpha value, but consider the following situation:

void OnExit()
{
    this.TransformBindableTo(audioVolumeBindable, 0, 1000);
}

In this case, the effect is not fine as it leaves the sample with >0 volume in the final frame.

This expiry process probably needs to be re-considered a bit, and this may even be an issue outside of screens too because the base UpdateChildrenLife() implementation works much the same way.

smoogipoo avatar Jan 31 '25 07:01 smoogipoo

If/when this is fixed, we should revisit adding a global sound fade on OsuScreen (as proposed here). We aren't doing this for now because there's a small chance it will never fully perform the fade.

peppy avatar Feb 03 '25 05:02 peppy