apply suggestions

- refactor `SongProgress`
- make`UpdateProgress` more readable
- enable NRT on new classes
- refactor `TestSceneSongProgress` to use `GameplayClockContainer`

osu.Game.Tests/Visual/Gameplay/TestSceneSongProgress.cs

@@ -4,11 +4,12 @@
 #nullable disable
 
 using System.Collections.Generic;
+using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Testing;
-using osu.Framework.Timing;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Screens.Play;
 using osu.Game.Screens.Play.HUD;
@@ -19,44 +20,43 @@ namespace osu.Game.Tests.Visual.Gameplay
     [TestFixture]
     public class TestSceneSongProgress : SkinnableHUDComponentTestScene
     {
-        private DefaultSongProgress defaultProgress;
+        private DefaultSongProgress progress => this.ChildrenOfType<DefaultSongProgress>().Single();
+        private GameplayClockContainer gameplayClockContainer;
+        private const double gameplay_start_time = -2000;
 
-        private readonly List<SongProgress> progresses = new List<SongProgress>();
+        [BackgroundDependencyLoader]
-
+        private void load()
-        private readonly StopwatchClock clock;
-        private readonly FramedClock framedClock;
-
-        [Cached]
-        private readonly GameplayClock gameplayClock;
-
-        public TestSceneSongProgress()
         {
-            clock = new StopwatchClock();
+            var working = CreateWorkingBeatmap(Ruleset.Value);
-            gameplayClock = new GameplayClock(framedClock = new FramedClock(clock));
+            working.LoadTrack();
+            Add(gameplayClockContainer = new MasterGameplayClockContainer(working, gameplay_start_time));
+            Dependencies.CacheAs(gameplayClockContainer);
+            Dependencies.CacheAs(gameplayClockContainer.GameplayClock);
         }
 
         [SetUpSteps]
         public void SetupSteps()
         {
-            AddStep("reset clock", clock.Reset);
+            AddStep("reset clock", () => gameplayClockContainer.Reset(false));
         }
 
         [Test]
         public void TestDisplay()
         {
             AddStep("display max values", displayMaxValues);
-            AddStep("start", clock.Start);
+            AddStep("seek to intro", () => gameplayClockContainer.Seek(gameplay_start_time));
-            AddStep("stop", clock.Stop);
+            AddStep("start", gameplayClockContainer.Start);
+            AddStep("stop", gameplayClockContainer.Stop);
         }
 
         [Test]
         public void TestToggleSeeking()
         {
-            AddStep("allow seeking", () => defaultProgress.AllowSeeking.Value = true);
+            AddStep("allow seeking", () => progress.AllowSeeking.Value = true);
-            AddStep("hide graph", () => defaultProgress.ShowGraph.Value = false);
+            AddStep("hide graph", () => progress.ShowGraph.Value = false);
-            AddStep("disallow seeking", () => defaultProgress.AllowSeeking.Value = false);
+            AddStep("disallow seeking", () => progress.AllowSeeking.Value = false);
-            AddStep("allow seeking", () => defaultProgress.AllowSeeking.Value = true);
+            AddStep("allow seeking", () => progress.AllowSeeking.Value = true);
-            AddStep("show graph", () => defaultProgress.ShowGraph.Value = true);
+            AddStep("show graph", () => progress.ShowGraph.Value = true);
         }
 
         private void displayMaxValues()
@@ -65,48 +65,25 @@ namespace osu.Game.Tests.Visual.Gameplay
             for (double i = 0; i < 5000; i++)
                 objects.Add(new HitObject { StartTime = i });
 
-            replaceObjects(objects);
+            setObjects(objects);
         }
 
-        private void replaceObjects(List<HitObject> objects)
+        private void setObjects(List<HitObject> objects)
         {
-            defaultProgress.RequestSeek = pos => clock.Seek(pos);
+            this.ChildrenOfType<SongProgress>().ForEach(progress => progress.Objects = objects);
-
-            foreach (var p in progresses)
-            {
-                p.Objects = objects;
-            }
         }
 
-        protected override void Update()
+        protected override Drawable CreateDefaultImplementation() => new DefaultSongProgress
         {
-            base.Update();
+            RelativeSizeAxes = Axes.X,
-            framedClock.ProcessFrame();
+            Anchor = Anchor.BottomLeft,
-        }
+            Origin = Anchor.BottomLeft,
+        };
 
-        protected override Drawable CreateDefaultImplementation()
+        protected override Drawable CreateLegacyImplementation() => new LegacySongProgress
         {
-            defaultProgress = new DefaultSongProgress
+            Anchor = Anchor.Centre,
-            {
+            Origin = Anchor.Centre,
-                RelativeSizeAxes = Axes.X,
+        };
-                Anchor = Anchor.BottomLeft,
-                Origin = Anchor.BottomLeft,
-            };
-
-            progresses.Add(defaultProgress);
-            return defaultProgress;
-        }
-
-        protected override Drawable CreateLegacyImplementation()
-        {
-            var progress = new LegacySongProgress
-            {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-            };
-
-            progresses.Add(progress);
-            return progress;
-        }
     }
 }

osu.Game/Screens/Play/HUD/DefaultSongProgress.cs

@@ -49,6 +49,9 @@ namespace osu.Game.Screens.Play.HUD
 
         protected override bool BlockScrollInput => false;
 
+        [Resolved(canBeNull: true)]
+        private GameplayClock gameplayClock { get; set; }
+
         [Resolved(canBeNull: true)]
         private Player player { get; set; }
 
@@ -178,10 +181,12 @@ namespace osu.Game.Screens.Play.HUD
             bar.EndTime = LastHitTime;
         }
 
-        protected override void UpdateProgress(double progress, double time, bool isIntro)
+        protected override void UpdateProgress(double progress, bool isIntro)
         {
-            bar.CurrentTime = time;
+            bar.CurrentTime = gameplayClock?.CurrentTime ?? Time.Current;
-            graph.Progress = (int)(graph.ColumnCount * progress);
+
+            if (!isIntro)
+                graph.Progress = (int)(graph.ColumnCount * progress);
         }
 
         protected override void Update()

osu.Game/Screens/Play/HUD/SongProgress.cs

@@ -1,16 +1,11 @@
 // 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.
 
-#nullable disable
-
-using System;
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Timing;
-using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.UI;
 using osu.Game.Skinning;
@@ -21,20 +16,14 @@ namespace osu.Game.Screens.Play.HUD
     {
         public bool UsesFixedAnchor { get; set; }
 
-        [Resolved(canBeNull: true)]
+        [Resolved]
-        private GameplayClock gameplayClock { get; set; }
+        private GameplayClockContainer? gameplayClockContainer { get; set; }
 
         [Resolved(canBeNull: true)]
-        private GameplayClockContainer gameplayClockContainer { get; set; }
+        private DrawableRuleset? drawableRuleset { get; set; }
 
-        [Resolved(canBeNull: true)]
+        private IClock? referenceClock;
-        private DrawableRuleset drawableRuleset { get; set; }
+        private IEnumerable<HitObject>? objects;
-
-        [Resolved(canBeNull: true)]
-        private IBindable<WorkingBeatmap> beatmap { get; set; }
-
-        private IClock referenceClock;
-        private IEnumerable<HitObject> objects;
 
         public IEnumerable<HitObject> Objects
         {
@@ -46,9 +35,7 @@ namespace osu.Game.Screens.Play.HUD
         //TODO: this isn't always correct (consider mania where a non-last object may last for longer than the last in the list).
         protected double LastHitTime => objects.LastOrDefault()?.GetEndTime() ?? 0;
 
-        protected double FirstEventTime { get; private set; }
+        protected abstract void UpdateProgress(double progress, bool isIntro);
-
-        protected abstract void UpdateProgress(double progress, double time, bool isIntro);
         protected abstract void UpdateObjects(IEnumerable<HitObject> objects);
 
         [BackgroundDependencyLoader]
@@ -59,11 +46,6 @@ namespace osu.Game.Screens.Play.HUD
                 Objects = drawableRuleset.Objects;
                 referenceClock = drawableRuleset.FrameStableClock;
             }
-
-            if (beatmap != null)
-            {
-                FirstEventTime = beatmap.Value.Storyboard.EarliestEventTime ?? 0;
-            }
         }
 
         protected override void Update()
@@ -73,17 +55,22 @@ namespace osu.Game.Screens.Play.HUD
             if (objects == null)
                 return;
 
-            double gameplayTime = gameplayClockContainer?.GameplayClock.CurrentTime ?? gameplayClock?.CurrentTime ?? Time.Current;
+            // The reference clock is used to accurately tell the playfield's time. This is obtained from the drawable ruleset.
-            double frameStableTime = referenceClock?.CurrentTime ?? gameplayTime;
+            // However, if no drawable ruleset is available (i.e. used in tests), we fall back to either the gameplay clock container or this drawable's own clock.
+            double gameplayTime = referenceClock?.CurrentTime ?? gameplayClockContainer?.GameplayClock.CurrentTime ?? Time.Current;
 
-            if (frameStableTime < FirstHitTime)
+            if (gameplayTime < FirstHitTime)
             {
-                double earliest = Math.Min(FirstEventTime, gameplayClockContainer?.StartTime ?? 0);
+                double earliest = gameplayClockContainer?.StartTime ?? 0;
-                UpdateProgress((frameStableTime - earliest) / (FirstHitTime - earliest), gameplayTime, true);
+                double introDuration = FirstHitTime - earliest;
+                double currentIntroTime = gameplayTime - earliest;
+                UpdateProgress(currentIntroTime / introDuration, true);
             }
             else
             {
-                UpdateProgress((frameStableTime - FirstHitTime) / (LastHitTime - FirstHitTime), gameplayTime, false);
+                double duration = LastHitTime - FirstHitTime;
+                double currentTime = gameplayTime - FirstHitTime;
+                UpdateProgress(currentTime / duration, false);
             }
         }
     }

osu.Game/Skinning/LegacySongProgress.cs

@@ -1,8 +1,6 @@
 // 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.
 
-#nullable disable
-
 using System.Collections.Generic;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
@@ -17,7 +15,7 @@ namespace osu.Game.Skinning
 {
     public class LegacySongProgress : SongProgress
     {
-        private CircularProgress pie;
+        private CircularProgress? pie;
 
         [BackgroundDependencyLoader]
         private void load()
@@ -72,8 +70,11 @@ namespace osu.Game.Skinning
         {
         }
 
-        protected override void UpdateProgress(double progress, double time, bool isIntro)
+        protected override void UpdateProgress(double progress, bool isIntro)
         {
+            if (pie == null)
+                return;
+
             if (isIntro)
             {
                 pie.Scale = new Vector2(-1, 1);