From 4b2b1f51f946d858aa44dd65872cb3dbe068e272 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 22 Jun 2018 12:12:59 +0900
Subject: [PATCH] Apply some code review

---
 osu.Game/Audio/PreviewTrack.cs         | 21 ++-----
 osu.Game/Overlays/Direct/PlayButton.cs | 87 +++++++++++---------------
 2 files changed, 44 insertions(+), 64 deletions(-)

diff --git a/osu.Game/Audio/PreviewTrack.cs b/osu.Game/Audio/PreviewTrack.cs
index 50366e35eb..3c9122b941 100644
--- a/osu.Game/Audio/PreviewTrack.cs
+++ b/osu.Game/Audio/PreviewTrack.cs
@@ -22,15 +22,12 @@ namespace osu.Game.Audio
         public event Action Started;
 
         private Track track;
-        private bool wasPlaying;
+        private bool hasStarted;
 
         [BackgroundDependencyLoader]
         private void load()
         {
             track = GetTrack();
-
-            if (track != null)
-                track.Looping = false;
         }
 
         /// <summary>
@@ -58,7 +55,7 @@ namespace osu.Game.Audio
             base.Update();
 
             // Todo: Track currently doesn't signal its completion, so we have to handle it manually
-            if (track != null && wasPlaying && track.HasCompleted)
+            if (hasStarted && track.HasCompleted)
                 Stop();
         }
 
@@ -69,15 +66,12 @@ namespace osu.Game.Audio
         /// </summary>
         public void Start() => startDelegate = Schedule(() =>
         {
-            if (!IsLoaded)
-                return;
-
             if (track == null)
                 return;
 
-            if (wasPlaying)
+            if (hasStarted)
                 return;
-            wasPlaying = true;
+            hasStarted = true;
 
             track.Restart();
             Started?.Invoke();
@@ -90,15 +84,12 @@ namespace osu.Game.Audio
         {
             startDelegate?.Cancel();
 
-            if (!IsLoaded)
-                return;
-
             if (track == null)
                 return;
 
-            if (!wasPlaying)
+            if (!hasStarted)
                 return;
-            wasPlaying = false;
+            hasStarted = false;
 
             track.Stop();
             Stopped?.Invoke();
diff --git a/osu.Game/Overlays/Direct/PlayButton.cs b/osu.Game/Overlays/Direct/PlayButton.cs
index 4bf6cf2bf7..4b91a3d700 100644
--- a/osu.Game/Overlays/Direct/PlayButton.cs
+++ b/osu.Game/Overlays/Direct/PlayButton.cs
@@ -16,7 +16,7 @@ namespace osu.Game.Overlays.Direct
 {
     public class PlayButton : Container
     {
-        public readonly Bindable<bool> Playing = new Bindable<bool>();
+        public readonly BindableBool Playing = new BindableBool();
         public PreviewTrack Preview { get; private set; }
 
         private BeatmapSetInfo beatmapSet;
@@ -29,12 +29,9 @@ namespace osu.Game.Overlays.Direct
                 if (value == beatmapSet) return;
                 beatmapSet = value;
 
-                if (Preview != null)
-                {
-                    Preview.Stop();
-                    RemoveInternal(Preview);
-                    Preview = null;
-                }
+                Preview?.Stop();
+                Preview?.Expire();
+                Preview = null;
 
                 Playing.Value = false;
             }
@@ -51,15 +48,9 @@ namespace osu.Game.Overlays.Direct
             set
             {
                 if (value)
-                {
                     loadingAnimation.Show();
-                    icon.FadeOut(transition_duration * 5, Easing.OutQuint);
-                }
                 else
-                {
                     loadingAnimation.Hide();
-                    icon.FadeIn(transition_duration, Easing.OutQuint);
-                }
             }
         }
 
@@ -94,35 +85,7 @@ namespace osu.Game.Overlays.Direct
 
         protected override bool OnClick(InputState state)
         {
-            if (!Playing.Value)
-            {
-                if (Preview != null)
-                {
-                    Preview.Start();
-                    return true;
-                }
-
-                loading = true;
-
-                var loadingPreview = previewTrackManager.Get(beatmapSet);
-                loadingPreview.Started += () => Playing.Value = true;
-                loadingPreview.Stopped += () => Playing.Value = false;
-
-                LoadComponentAsync(Preview = loadingPreview, t =>
-                {
-                    if (Preview != loadingPreview)
-                        return;
-
-                    AddInternal(t);
-
-                    Preview.Start();
-
-                    loading = false;
-                });
-            }
-            else
-                Preview?.Stop();
-
+            Playing.Toggle();
             return true;
         }
 
@@ -141,17 +104,43 @@ namespace osu.Game.Overlays.Direct
 
         private void playingStateChanged(bool playing)
         {
-            if (playing && BeatmapSet == null)
-            {
-                Playing.Value = false;
-                return;
-            }
-
             icon.Icon = playing ? FontAwesome.fa_stop : FontAwesome.fa_play;
             icon.FadeColour(playing || IsHovered ? hoverColour : Color4.White, 120, Easing.InOutQuint);
 
-            if (!playing)
+            if (playing)
             {
+                if (BeatmapSet == null)
+                {
+                    Playing.Value = false;
+                    return;
+                }
+
+                if (Preview != null)
+                {
+                    Preview.Start();
+                    return;
+                }
+
+                loading = true;
+
+                LoadComponentAsync(Preview = previewTrackManager.Get(beatmapSet), preview =>
+                {
+                    // beatmapset may have changed.
+                    if (Preview != preview)
+                        return;
+
+                    AddInternal(preview);
+                    loading = false;
+                    preview.Stopped += () => Playing.Value = false;
+
+                    // user may have changed their mind.
+                    if (Playing)
+                        preview.Start();
+                });
+            }
+            else
+            {
+                Preview?.Stop();
                 loading = false;
             }
         }