From ae2fd2f2e1f4315d4c1b90a56d412fa7b54cc9f3 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 23 Apr 2021 18:46:59 +0900 Subject: [PATCH 1/3] Ensure source is set on reset --- osu.Game/Screens/Play/GameplayClockContainer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 5cd17d92c4..6677116399 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -100,6 +100,7 @@ namespace osu.Game.Screens.Play /// public virtual void Reset() { + ChangeSource(SourceClock); Seek(0); // Manually stop the source in order to not affect the IsPaused state. From fdb5490e51f1edf5b09aed6ecaf428e7ff03e8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 23 Apr 2021 21:56:08 +0200 Subject: [PATCH 2/3] Attempt to explain source initialisation better --- osu.Game/Screens/Play/GameplayClockContainer.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 6677116399..6d60c09521 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -63,8 +63,7 @@ namespace osu.Game.Screens.Play /// public virtual void Start() { - // Ensure that the source clock is set. - ChangeSource(SourceClock); + ensureSourceClockSet(); if (!AdjustableSource.IsRunning) { @@ -100,7 +99,7 @@ namespace osu.Game.Screens.Play /// public virtual void Reset() { - ChangeSource(SourceClock); + ensureSourceClockSet(); Seek(0); // Manually stop the source in order to not affect the IsPaused state. @@ -116,6 +115,15 @@ namespace osu.Game.Screens.Play /// The new source. protected void ChangeSource(IClock sourceClock) => AdjustableSource.ChangeSource(SourceClock = sourceClock); + /// + /// Ensures that the is set to . + /// This is usually done before a seek to avoid accidentally seeking only the adjustable source in decoupled mode, + /// but not the actual source clock. + /// That will pretty much only happen on the very first call of this method, as the source clock is passed in the constructor, + /// but it is not yet set on the adjustable source there. + /// + private void ensureSourceClockSet() => ChangeSource(SourceClock); + protected override void Update() { if (!IsPaused.Value) From e937b778f6a5ccb4840e9561467c81edfbfb4028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 24 Apr 2021 14:19:39 +0200 Subject: [PATCH 3/3] Fix potential failure in `ensureSourceClockSet()` `ensureSourceClockSet()` was intended to only run when the adjustable source hasn't been set at all yet. As it turns out permitting it to run unconditionally can break the state of the underlying interpolated clock. This is caused by the following factors: * While the decoupleable clock is running, its `CurrentTime` does not come from either the source clock, or the internal stopwatch; it is instead calculated using the base `InterpolatingFramedClock` logic. * A source change of a decoupleable clock seeks the provided source clock to the decoupleable's current time. * When an interpolating clock is seeked (decoupleable clock is also an interpolating one), its interpolation state (`{Last,Current}InterpolatedTime`) are reset to 0. * If the interpolating clock determines that its current time is too far away from the source's time (which was set when the source is changed), it will ignore the source and instead continue to use its current time until the source clock has caught up. Overall, the source change is not really necessary if a source is already there. The only reason to ensure it was set was to make sure the first seek of the gameplay clock wasn't performed in decoupled mode. Therefore, add a guard to make sure the source is only set if there isn't one already. --- osu.Game/Screens/Play/GameplayClockContainer.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 6d60c09521..1c8a3e51ac 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -116,13 +116,17 @@ namespace osu.Game.Screens.Play protected void ChangeSource(IClock sourceClock) => AdjustableSource.ChangeSource(SourceClock = sourceClock); /// - /// Ensures that the is set to . + /// Ensures that the is set to , if it hasn't been given a source yet. /// This is usually done before a seek to avoid accidentally seeking only the adjustable source in decoupled mode, /// but not the actual source clock. /// That will pretty much only happen on the very first call of this method, as the source clock is passed in the constructor, /// but it is not yet set on the adjustable source there. /// - private void ensureSourceClockSet() => ChangeSource(SourceClock); + private void ensureSourceClockSet() + { + if (AdjustableSource.Source == null) + ChangeSource(SourceClock); + } protected override void Update() {