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

Simplify gameplay pause sequence #26605

Merged
merged 3 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions osu.Game.Tests/Visual/Gameplay/TestScenePause.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,12 @@ public void TestPauseWithLargeOffset()

double currentTime = masterClock.CurrentTime;

bool goingForward = currentTime >= (masterClock.LastStopTime ?? lastStopTime);
bool goingForward = currentTime >= lastStopTime;

alwaysGoingForward &= goingForward;

if (!goingForward)
Logger.Log($"Went too far backwards (last stop: {lastStopTime:N1} current: {currentTime:N1})");

if (masterClock.LastStopTime != null)
lastStopTime = masterClock.LastStopTime.Value;
};
});

Expand All @@ -125,7 +122,7 @@ public void TestPauseResume()

resumeAndConfirm();

AddAssert("Resumed without seeking forward", () => Player.LastResumeTime, () => Is.LessThanOrEqualTo(Player.LastPauseTime));
AddAssert("continued playing forward", () => Player.LastResumeTime, () => Is.GreaterThanOrEqualTo(Player.LastPauseTime));

AddUntilStep("player playing", () => Player.LocalUserPlaying.Value);
}
Expand Down
11 changes: 3 additions & 8 deletions osu.Game/Beatmaps/FramedBeatmapClock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ public partial class FramedBeatmapClock : Component, IFrameBasedClock, IAdjustab
{
private readonly bool applyOffsets;

/// <summary>
/// The total frequency adjustment from pause transforms. Should eventually be handled in a better way.
/// </summary>
public readonly BindableDouble ExternalPauseFrequencyAdjust = new BindableDouble(1);

private readonly OffsetCorrectionClock? userGlobalOffsetClock;
private readonly OffsetCorrectionClock? platformOffsetClock;
private readonly OffsetCorrectionClock? userBeatmapOffsetClock;
Expand Down Expand Up @@ -69,13 +64,13 @@ public FramedBeatmapClock(bool applyOffsets, bool requireDecoupling, IClock? sou
{
// Audio timings in general with newer BASS versions don't match stable.
// This only seems to be required on windows. We need to eventually figure out why, with a bit of luck.
platformOffsetClock = new OffsetCorrectionClock(interpolatedTrack, ExternalPauseFrequencyAdjust) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 };
platformOffsetClock = new OffsetCorrectionClock(interpolatedTrack) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 };

// User global offset (set in settings) should also be applied.
userGlobalOffsetClock = new OffsetCorrectionClock(platformOffsetClock, ExternalPauseFrequencyAdjust);
userGlobalOffsetClock = new OffsetCorrectionClock(platformOffsetClock);

// User per-beatmap offset will be applied to this final clock.
finalClockSource = userBeatmapOffsetClock = new OffsetCorrectionClock(userGlobalOffsetClock, ExternalPauseFrequencyAdjust);
finalClockSource = userBeatmapOffsetClock = new OffsetCorrectionClock(userGlobalOffsetClock);
}
else
{
Expand Down
10 changes: 0 additions & 10 deletions osu.Game/Screens/Play/GameplayClockContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ public void Start()

isPaused.Value = false;

PrepareStart();

// The case which caused this to be added is FrameStabilityContainer, which manages its own current and elapsed time.
// Because we generally update our own current time quicker than children can query it (via Start/Seek/Update),
// this means that the first frame ever exposed to children may have a non-zero current time.
Expand All @@ -99,14 +97,6 @@ public void Start()
});
}

/// <summary>
/// When <see cref="Start"/> is called, this will be run to give an opportunity to prepare the clock at the correct
/// start location.
/// </summary>
protected virtual void PrepareStart()
{
}

/// <summary>
/// Seek to a specific time in gameplay.
/// </summary>
Expand Down
68 changes: 0 additions & 68 deletions osu.Game/Screens/Play/MasterGameplayClockContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using osu.Framework.Audio;
using osu.Framework.Audio.Track;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Logging;
using osu.Framework.Timing;
using osu.Game.Beatmaps;
Expand Down Expand Up @@ -60,17 +59,6 @@ public partial class MasterGameplayClockContainer : GameplayClockContainer, IBea

private readonly double skipTargetTime;

/// <summary>
/// Stores the time at which the last <see cref="StopGameplayClock"/> call was triggered.
/// This is used to ensure we resume from that precise point in time, ignoring the proceeding frequency ramp.
///
/// Optimally, we'd have gameplay ramp down with the frequency, but I believe this was intentionally disabled
/// to avoid fails occurring after the pause screen has been shown.
///
/// In the future I want to change this.
/// </summary>
internal double? LastStopTime;

[Resolved]
private MusicController musicController { get; set; } = null!;

Expand Down Expand Up @@ -113,71 +101,17 @@ private double findEarliestStartTime()
return time;
}

protected override void StopGameplayClock()
{
LastStopTime = GameplayClock.CurrentTime;

if (IsLoaded)
{
// During normal operation, the source is stopped after performing a frequency ramp.
this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 0, 200, Easing.Out).OnComplete(_ =>
{
if (IsPaused.Value)
base.StopGameplayClock();
});
}
else
{
base.StopGameplayClock();

// If not yet loaded, we still want to ensure relevant state is correct, as it is used for offset calculations.
GameplayClock.ExternalPauseFrequencyAdjust.Value = 0;

// We must also process underlying gameplay clocks to update rate-adjusted offsets with the new frequency adjustment.
// Without doing this, an initial seek may be performed with the wrong offset.
GameplayClock.ProcessFrame();
}
}

public override void Seek(double time)
{
// Safety in case the clock is seeked while stopped.
LastStopTime = null;
elapsedValidationTime = null;

base.Seek(time);
}

protected override void PrepareStart()
{
if (LastStopTime != null)
{
Seek(LastStopTime.Value);
LastStopTime = null;
}
else
base.PrepareStart();
}

protected override void StartGameplayClock()
{
addAdjustmentsToTrack();

base.StartGameplayClock();

if (IsLoaded)
{
this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 1, 200, Easing.In);
}
else
{
// If not yet loaded, we still want to ensure relevant state is correct, as it is used for offset calculations.
GameplayClock.ExternalPauseFrequencyAdjust.Value = 1;

// We must also process underlying gameplay clocks to update rate-adjusted offsets with the new frequency adjustment.
// Without doing this, an initial seek may be performed with the wrong offset.
GameplayClock.ProcessFrame();
}
}

/// <summary>
Expand Down Expand Up @@ -273,7 +207,6 @@ private void addAdjustmentsToTrack()
musicController.ResetTrackAdjustments();

track.BindAdjustments(AdjustmentsFromMods);
track.AddAdjustment(AdjustableProperty.Frequency, GameplayClock.ExternalPauseFrequencyAdjust);
track.AddAdjustment(AdjustableProperty.Frequency, UserPlaybackRate);

speedAdjustmentsApplied = true;
Expand All @@ -285,7 +218,6 @@ private void removeAdjustmentsFromTrack()
return;

track.UnbindAdjustments(AdjustmentsFromMods);
track.RemoveAdjustment(AdjustableProperty.Frequency, GameplayClock.ExternalPauseFrequencyAdjust);
track.RemoveAdjustment(AdjustableProperty.Frequency, UserPlaybackRate);

speedAdjustmentsApplied = false;
Expand Down
14 changes: 3 additions & 11 deletions osu.Game/Screens/Play/OffsetCorrectionClock.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Bindables;
using osu.Framework.Timing;

namespace osu.Game.Screens.Play
{
public class OffsetCorrectionClock : FramedOffsetClock
{
private readonly BindableDouble pauseRateAdjust;

private double offset;

public new double Offset
Expand All @@ -28,10 +25,9 @@ public class OffsetCorrectionClock : FramedOffsetClock

public double RateAdjustedOffset => base.Offset;

public OffsetCorrectionClock(IClock source, BindableDouble pauseRateAdjust)
public OffsetCorrectionClock(IClock source)
: base(source)
{
this.pauseRateAdjust = pauseRateAdjust;
}

public override void ProcessFrame()
Expand All @@ -42,12 +38,8 @@ public override void ProcessFrame()

private void updateOffset()
{
// changing this during the pause transform effect will cause a potentially large offset to be suddenly applied as we approach zero rate.
if (pauseRateAdjust.Value == 1)
{
// we always want to apply the same real-time offset, so it should be adjusted by the difference in playback rate (from realtime) to achieve this.
base.Offset = Offset * Rate;
}
// we always want to apply the same real-time offset, so it should be adjusted by the difference in playback rate (from realtime) to achieve this.
base.Offset = Offset * Rate;
}
}
}
Loading