Merge pull request #14634 from peppy/fix-notification-sounds

Fix notification show/hide samples stacking indefinitely

osu.Game/Graphics/UserInterface/HoverSampleDebounceComponent.cs

@@ -15,11 +15,6 @@ namespace osu.Game.Graphics.UserInterface
     /// </summary>
     public abstract class HoverSampleDebounceComponent : CompositeDrawable
     {
-        /// <summary>
-        /// Length of debounce for hover sound playback, in milliseconds.
-        /// </summary>
-        public double HoverDebounceTime { get; } = 20;
-
         private Bindable<double?> lastPlaybackTime;
 
         [BackgroundDependencyLoader]
@@ -34,7 +29,7 @@ namespace osu.Game.Graphics.UserInterface
             if (e.HasAnyButtonPressed)
                 return false;
 
-            bool enoughTimePassedSinceLastPlayback = !lastPlaybackTime.Value.HasValue || Time.Current - lastPlaybackTime.Value >= HoverDebounceTime;
+            bool enoughTimePassedSinceLastPlayback = !lastPlaybackTime.Value.HasValue || Time.Current - lastPlaybackTime.Value >= OsuGameBase.SAMPLE_DEBOUNCE_TIME;
 
             if (enoughTimePassedSinceLastPlayback)
             {

osu.Game/Graphics/UserInterface/HoverSounds.cs

@@ -6,7 +6,6 @@ using osu.Framework.Audio;
 using osu.Framework.Audio.Sample;
 using osu.Framework.Extensions;
 using osu.Framework.Graphics;
-using osu.Game.Configuration;
 using osu.Framework.Utils;
 
 namespace osu.Game.Graphics.UserInterface
@@ -28,7 +27,7 @@ namespace osu.Game.Graphics.UserInterface
         }
 
         [BackgroundDependencyLoader]
-        private void load(AudioManager audio, SessionStatics statics)
+        private void load(AudioManager audio)
         {
             sampleHover = audio.Samples.Get($@"UI/{SampleSet.GetDescription()}-hover")
                           ?? audio.Samples.Get($@"UI/{HoverSampleSet.Default.GetDescription()}-hover");

osu.Game/OsuGameBase.cs

@@ -56,6 +56,11 @@ namespace osu.Game
 
         public const int SAMPLE_CONCURRENCY = 6;
 
+        /// <summary>
+        /// Length of debounce (in milliseconds) for commonly occuring sample playbacks that could stack.
+        /// </summary>
+        public const int SAMPLE_DEBOUNCE_TIME = 20;
+
         /// <summary>
         /// The maximum volume at which audio tracks should playback. This can be set lower than 1 to create some head-room for sound effects.
         /// </summary>

osu.Game/Overlays/NotificationOverlay.cs

@@ -9,6 +9,7 @@ using osu.Game.Overlays.Notifications;
 using osu.Framework.Graphics.Shapes;
 using osu.Game.Graphics.Containers;
 using osu.Framework.Allocation;
+using osu.Framework.Audio;
 using osu.Framework.Bindables;
 using osu.Framework.Localisation;
 using osu.Framework.Threading;
@@ -29,6 +30,9 @@ namespace osu.Game.Overlays
 
         private FlowContainer<NotificationSection> sections;
 
+        [Resolved]
+        private AudioManager audio { get; set; }
+
         [BackgroundDependencyLoader]
         private void load()
         {
@@ -98,14 +102,18 @@ namespace osu.Game.Overlays
 
         private int runningDepth;
 
-        private void notificationClosed() => updateCounts();
-
         private readonly Scheduler postScheduler = new Scheduler();
 
         public override bool IsPresent => base.IsPresent || postScheduler.HasPendingTasks;
 
         private bool processingPosts = true;
 
+        private double? lastSamplePlayback;
+
+        /// <summary>
+        /// Post a new notification for display.
+        /// </summary>
+        /// <param name="notification">The notification to display.</param>
         public void Post(Notification notification) => postScheduler.Add(() =>
         {
             ++runningDepth;
@@ -124,11 +132,13 @@ namespace osu.Game.Overlays
                 Show();
 
             updateCounts();
+            playDebouncedSample(notification.PopInSampleName);
         });
 
         protected override void Update()
         {
             base.Update();
+
             if (processingPosts)
                 postScheduler.Update();
         }
@@ -151,6 +161,24 @@ namespace osu.Game.Overlays
             this.FadeTo(0, TRANSITION_LENGTH, Easing.OutQuint);
         }
 
+        private void notificationClosed()
+        {
+            updateCounts();
+
+            // this debounce is currently shared between popin/popout sounds, which means one could potentially not play when the user is expecting it.
+            // popout is constant across all notification types, and should therefore be handled using playback concurrency instead, but seems broken at the moment.
+            playDebouncedSample("UI/overlay-pop-out");
+        }
+
+        private void playDebouncedSample(string sampleName)
+        {
+            if (lastSamplePlayback == null || Time.Current - lastSamplePlayback > OsuGameBase.SAMPLE_DEBOUNCE_TIME)
+            {
+                audio.Samples.Get(sampleName)?.Play();
+                lastSamplePlayback = Time.Current;
+            }
+        }
+
         private void updateCounts()
         {
             UnreadCount.Value = sections.Select(c => c.UnreadCount).Sum();

osu.Game/Overlays/Notifications/Notification.cs

@@ -3,20 +3,18 @@
 
 using System;
 using osu.Framework.Allocation;
-using osu.Framework.Audio;
-using osu.Framework.Audio.Sample;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Colour;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Effects;
-using osu.Game.Graphics;
-using osuTK;
-using osuTK.Graphics;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Input.Events;
+using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
+using osuTK;
+using osuTK.Graphics;
 
 namespace osu.Game.Overlays.Notifications
 {
@@ -42,10 +40,7 @@ namespace osu.Game.Overlays.Notifications
         /// </summary>
         public virtual bool DisplayOnTop => true;
 
-        private Sample samplePopIn;
+        public virtual string PopInSampleName => "UI/notification-pop-in";
-        private Sample samplePopOut;
-        protected virtual string PopInSampleName => "UI/notification-pop-in";
-        protected virtual string PopOutSampleName => "UI/overlay-pop-out"; // TODO: replace with a unique sample?
 
         protected NotificationLight Light;
         private readonly CloseButton closeButton;
@@ -114,7 +109,7 @@ namespace osu.Game.Overlays.Notifications
                         closeButton = new CloseButton
                         {
                             Alpha = 0,
-                            Action = () => Close(),
+                            Action = Close,
                             Anchor = Anchor.CentreRight,
                             Origin = Anchor.CentreRight,
                             Margin = new MarginPadding
@@ -127,13 +122,6 @@ namespace osu.Game.Overlays.Notifications
             });
         }
 
-        [BackgroundDependencyLoader]
-        private void load(AudioManager audio)
-        {
-            samplePopIn = audio.Samples.Get(PopInSampleName);
-            samplePopOut = audio.Samples.Get(PopOutSampleName);
-        }
-
         protected override bool OnHover(HoverEvent e)
         {
             closeButton.FadeIn(75);
@@ -158,8 +146,6 @@ namespace osu.Game.Overlays.Notifications
         {
             base.LoadComplete();
 
-            samplePopIn?.Play();
-
             this.FadeInFromZero(200);
             NotificationContent.MoveToX(DrawSize.X);
             NotificationContent.MoveToX(0, 500, Easing.OutQuint);
@@ -167,15 +153,12 @@ namespace osu.Game.Overlays.Notifications
 
         public bool WasClosed;
 
-        public virtual void Close(bool playSound = true)
+        public virtual void Close()
         {
             if (WasClosed) return;
 
             WasClosed = true;
 
-            if (playSound)
-                samplePopOut?.Play();
-
             Closed?.Invoke();
             this.FadeOut(100);
             Expire();

osu.Game/Overlays/Notifications/NotificationSection.cs

@@ -110,12 +110,7 @@ namespace osu.Game.Overlays.Notifications
 
         private void clearAll()
         {
-            bool first = true;
+            notifications.Children.ForEach(c => c.Close());
-            notifications.Children.ForEach(c =>
-            {
-                c.Close(first);
-                first = false;
-            });
         }
 
         protected override void Update()

osu.Game/Overlays/Notifications/ProgressNotification.cs

@@ -150,12 +150,12 @@ namespace osu.Game.Overlays.Notifications
             colourCancelled = colours.Red;
         }
 
-        public override void Close(bool playSound = true)
+        public override void Close()
         {
             switch (State)
             {
                 case ProgressNotificationState.Cancelled:
-                    base.Close(playSound);
+                    base.Close();
                     break;
 
                 case ProgressNotificationState.Active:

osu.Game/Overlays/Notifications/SimpleErrorNotification.cs

@@ -7,7 +7,7 @@ namespace osu.Game.Overlays.Notifications
 {
     public class SimpleErrorNotification : SimpleNotification
     {
-        protected override string PopInSampleName => "UI/error-notification-pop-in";
+        public override string PopInSampleName => "UI/error-notification-pop-in";
 
         public SimpleErrorNotification()
         {