Merge pull request #10841 from bdach/fix-spectator-preview-crash

Add support for previewing tracks on spectator screen

osu.Game.Tests/Visual/Components/TestScenePreviewTrackManager.cs

@@ -167,6 +167,21 @@ namespace osu.Game.Tests.Visual.Components
             AddAssert("game not muted", () => audio.Tracks.AggregateVolume.Value != 0);
         }
 
+        [Test]
+        public void TestOwnerNotRegistered()
+        {
+            PreviewTrack track = null;
+
+            AddStep("get track", () => Add(new TestTrackOwner(track = getTrack(), registerAsOwner: false)));
+            AddUntilStep("wait for loaded", () => track.IsLoaded);
+
+            AddStep("start track", () => track.Start());
+            AddUntilStep("track is running", () => track.IsRunning);
+
+            AddStep("cancel from anyone", () => trackManager.StopAnyPlaying(this));
+            AddAssert("track stopped", () => !track.IsRunning);
+        }
+
         private TestPreviewTrack getTrack() => (TestPreviewTrack)trackManager.Get(null);
 
         private TestPreviewTrack getOwnedTrack()
@@ -181,10 +196,12 @@ namespace osu.Game.Tests.Visual.Components
         private class TestTrackOwner : CompositeDrawable, IPreviewTrackOwner
         {
             private readonly PreviewTrack track;
+            private readonly bool registerAsOwner;
 
-            public TestTrackOwner(PreviewTrack track)
+            public TestTrackOwner(PreviewTrack track, bool registerAsOwner = true)
             {
                 this.track = track;
+                this.registerAsOwner = registerAsOwner;
             }
 
             [BackgroundDependencyLoader]
@@ -196,7 +213,8 @@ namespace osu.Game.Tests.Visual.Components
             protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
             {
                 var dependencies = new DependencyContainer(base.CreateChildDependencies(parent));
-                dependencies.CacheAs<IPreviewTrackOwner>(this);
+                if (registerAsOwner)
+                    dependencies.CacheAs<IPreviewTrackOwner>(this);
                 return dependencies;
             }
         }

osu.Game/Audio/PreviewTrackManager.cs

@@ -11,6 +11,7 @@ using osu.Framework.Audio.Track;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.IO.Stores;
+using osu.Framework.Logging;
 using osu.Game.Beatmaps;
 
 namespace osu.Game.Audio
@@ -76,7 +77,7 @@ namespace osu.Game.Audio
         /// <param name="source">The <see cref="IPreviewTrackOwner"/> which may be the owner of the <see cref="PreviewTrack"/>.</param>
         public void StopAnyPlaying(IPreviewTrackOwner source)
         {
-            if (CurrentTrack == null || CurrentTrack.Owner != source)
+            if (CurrentTrack == null || (CurrentTrack.Owner != null && CurrentTrack.Owner != source))
                 return;
 
             CurrentTrack.Stop();
@@ -86,11 +87,12 @@ namespace osu.Game.Audio
         /// <summary>
         /// Creates the <see cref="TrackManagerPreviewTrack"/>.
         /// </summary>
-        protected virtual TrackManagerPreviewTrack CreatePreviewTrack(BeatmapSetInfo beatmapSetInfo, ITrackStore trackStore) => new TrackManagerPreviewTrack(beatmapSetInfo, trackStore);
+        protected virtual TrackManagerPreviewTrack CreatePreviewTrack(BeatmapSetInfo beatmapSetInfo, ITrackStore trackStore) =>
+            new TrackManagerPreviewTrack(beatmapSetInfo, trackStore);
 
         public class TrackManagerPreviewTrack : PreviewTrack
         {
-            [Resolved]
+            [Resolved(canBeNull: true)]
             public IPreviewTrackOwner Owner { get; private set; }
 
             private readonly BeatmapSetInfo beatmapSetInfo;
@@ -102,6 +104,12 @@ namespace osu.Game.Audio
                 this.trackManager = trackManager;
             }
 
+            protected override void LoadComplete()
+            {
+                base.LoadComplete();
+                Logger.Log($"A {nameof(PreviewTrack)} was created without a containing {nameof(IPreviewTrackOwner)}. An owner should be added for correct behaviour.");
+            }
+
             protected override Track GetTrack() => trackManager.Get($"https://b.ppy.sh/preview/{beatmapSetInfo?.OnlineBeatmapSetID}.mp3");
         }
 

osu.Game/Screens/Play/Spectator.cs

@@ -13,6 +13,7 @@ using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Screens;
+using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Graphics;
@@ -35,7 +36,8 @@ using osuTK;
 
 namespace osu.Game.Screens.Play
 {
-    public class Spectator : OsuScreen
+    [Cached(typeof(IPreviewTrackOwner))]
+    public class Spectator : OsuScreen, IPreviewTrackOwner
     {
         private readonly User targetUser;
 
@@ -62,6 +64,9 @@ namespace osu.Game.Screens.Play
         [Resolved]
         private RulesetStore rulesets { get; set; }
 
+        [Resolved]
+        private PreviewTrackManager previewTrackManager { get; set; }
+
         private Score score;
 
         private readonly object scoreLock = new object();
@@ -275,6 +280,7 @@ namespace osu.Game.Screens.Play
         {
             watchButton.Enabled.Value = false;
             beatmapPanelContainer.Clear();
+            previewTrackManager.StopAnyPlaying(this);
         }
 
         private void attemptStart()
@@ -326,7 +332,6 @@ namespace osu.Game.Screens.Play
         {
             if (state?.BeatmapID == null)
             {
-                beatmapPanelContainer.Clear();
                 onlineBeatmap = null;
                 return;
             }
@@ -359,6 +364,12 @@ namespace osu.Game.Screens.Play
             beatmaps.Download(onlineBeatmap);
         }
 
+        public override bool OnExiting(IScreen next)
+        {
+            previewTrackManager.StopAnyPlaying(this);
+            return base.OnExiting(next);
+        }
+
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);