From a1c72db5f62001aa62f994f5f24c463032a8ddce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 31 Aug 2019 17:01:12 +0200 Subject: [PATCH 1/7] Fix inconsistent sound effects on mod buttons Because HoverClickSounds.OnClick() does not fire upon right clicking on mod buttons, the sound effects that play on left and right click were inconsistent. Introduce HoverMouseUpSounds drawable that allows to play the click sound effect upon mouse up events for an arbitrary set of mouse buttons and use it on mod buttons. --- .../UserInterface/HoverMouseUpSounds.cs | 42 +++++++++++++++++++ osu.Game/Overlays/Mods/ModButton.cs | 3 +- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs diff --git a/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs b/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs new file mode 100644 index 0000000000..dcb443d5aa --- /dev/null +++ b/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs @@ -0,0 +1,42 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Audio.Sample; +using osu.Framework.Extensions; +using osu.Framework.Input.Events; +using osuTK.Input; + +namespace osu.Game.Graphics.UserInterface +{ + /// + /// Adds hover sounds to a drawable, as well as click sounds upon MouseUp events for selected mouse buttons. + /// Intended to be used for controls that can respond to clicks of buttons other than the left mouse button in place of . + /// + public class HoverMouseUpSounds : HoverSounds + { + private SampleChannel sampleClick; + private readonly List buttons; + + public HoverMouseUpSounds(List buttons, HoverSampleSet sampleSet = HoverSampleSet.Normal) + : base(sampleSet) + { + this.buttons = buttons; + } + + protected override bool OnMouseUp(MouseUpEvent e) + { + if (Contains(e.ScreenSpaceMousePosition) && buttons.Contains(e.Button)) + sampleClick?.Play(); + return base.OnMouseUp(e); + } + + [BackgroundDependencyLoader] + private void load(AudioManager audio) + { + sampleClick = audio.Samples.Get($@"UI/generic-select{SampleSet.GetDescription()}"); + } + } +} diff --git a/osu.Game/Overlays/Mods/ModButton.cs b/osu.Game/Overlays/Mods/ModButton.cs index 7b8745cf42..ba39360102 100644 --- a/osu.Game/Overlays/Mods/ModButton.cs +++ b/osu.Game/Overlays/Mods/ModButton.cs @@ -11,6 +11,7 @@ using osu.Game.Graphics.Sprites; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.UI; using System; +using System.Collections.Generic; using System.Linq; using osu.Framework.Graphics.Cursor; using osu.Framework.Input.Events; @@ -283,7 +284,7 @@ namespace osu.Game.Overlays.Mods Anchor = Anchor.TopCentre, Font = OsuFont.GetFont(size: 18) }, - new HoverClickSounds() + new HoverMouseUpSounds(new List { MouseButton.Left, MouseButton.Right }) }; Mod = mod; From 658e0edc3e7df2510953bc9acc11aac35b081590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 31 Aug 2019 20:16:16 +0200 Subject: [PATCH 2/7] Handle other button clicks in HoverClickSounds As suggested in review, remove previously introduced HoverMouseUpSounds and instead change effect playing logic in HoverClickSounds by moving it out of OnClick() to OnMouseUp(). Users of the class can either use the existing constructor to play the effect only on left click or use the newly introduced constructor with the MouseButton[] parameter to specify which button clicks should trigger the sound. --- .../UserInterface/HoverClickSounds.cs | 32 ++++++++++++-- .../UserInterface/HoverMouseUpSounds.cs | 42 ------------------- osu.Game/Overlays/Mods/ModButton.cs | 3 +- 3 files changed, 29 insertions(+), 48 deletions(-) delete mode 100644 osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index 70d988f60e..8fe20e3566 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -1,11 +1,13 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Extensions; using osu.Framework.Input.Events; +using osuTK.Input; namespace osu.Game.Graphics.UserInterface { @@ -16,16 +18,38 @@ namespace osu.Game.Graphics.UserInterface public class HoverClickSounds : HoverSounds { private SampleChannel sampleClick; + private readonly MouseButton[] buttons; + /// + /// Creates an instance that adds sounds on hover and left click only. + /// + /// Set of click samples to play. public HoverClickSounds(HoverSampleSet sampleSet = HoverSampleSet.Normal) - : base(sampleSet) + : this(new[] { MouseButton.Left }, sampleSet) { } - protected override bool OnClick(ClickEvent e) + /// + /// Creates an instance that adds sounds on hover and on click for any of the buttons specified. + /// + /// Array of button codes which should trigger the click sound. + /// Set of click samples to play. + public HoverClickSounds(MouseButton[] buttons, HoverSampleSet sampleSet = HoverSampleSet.Normal) + : base(sampleSet) { - sampleClick?.Play(); - return base.OnClick(e); + this.buttons = buttons; + } + + protected override bool OnMouseUp(MouseUpEvent e) + { + var index = Array.IndexOf(buttons, e.Button); + bool shouldPlayEffect = index > -1 && index < buttons.Length; + + // examine the button pressed first for short-circuiting + // in most usages it is more likely that another button was pressed than that the cursor left the drawable bounds + if (shouldPlayEffect && Contains(e.ScreenSpaceMousePosition)) + sampleClick?.Play(); + return base.OnMouseUp(e); } [BackgroundDependencyLoader] diff --git a/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs b/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs deleted file mode 100644 index dcb443d5aa..0000000000 --- a/osu.Game/Graphics/UserInterface/HoverMouseUpSounds.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using osu.Framework.Allocation; -using osu.Framework.Audio; -using osu.Framework.Audio.Sample; -using osu.Framework.Extensions; -using osu.Framework.Input.Events; -using osuTK.Input; - -namespace osu.Game.Graphics.UserInterface -{ - /// - /// Adds hover sounds to a drawable, as well as click sounds upon MouseUp events for selected mouse buttons. - /// Intended to be used for controls that can respond to clicks of buttons other than the left mouse button in place of . - /// - public class HoverMouseUpSounds : HoverSounds - { - private SampleChannel sampleClick; - private readonly List buttons; - - public HoverMouseUpSounds(List buttons, HoverSampleSet sampleSet = HoverSampleSet.Normal) - : base(sampleSet) - { - this.buttons = buttons; - } - - protected override bool OnMouseUp(MouseUpEvent e) - { - if (Contains(e.ScreenSpaceMousePosition) && buttons.Contains(e.Button)) - sampleClick?.Play(); - return base.OnMouseUp(e); - } - - [BackgroundDependencyLoader] - private void load(AudioManager audio) - { - sampleClick = audio.Samples.Get($@"UI/generic-select{SampleSet.GetDescription()}"); - } - } -} diff --git a/osu.Game/Overlays/Mods/ModButton.cs b/osu.Game/Overlays/Mods/ModButton.cs index ba39360102..f46555dc4b 100644 --- a/osu.Game/Overlays/Mods/ModButton.cs +++ b/osu.Game/Overlays/Mods/ModButton.cs @@ -11,7 +11,6 @@ using osu.Game.Graphics.Sprites; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.UI; using System; -using System.Collections.Generic; using System.Linq; using osu.Framework.Graphics.Cursor; using osu.Framework.Input.Events; @@ -284,7 +283,7 @@ namespace osu.Game.Overlays.Mods Anchor = Anchor.TopCentre, Font = OsuFont.GetFont(size: 18) }, - new HoverMouseUpSounds(new List { MouseButton.Left, MouseButton.Right }) + new HoverClickSounds(new[] { MouseButton.Left, MouseButton.Right }) }; Mod = mod; From c4dc34eefde8740e12bd2ed974f81841d862c878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 1 Sep 2019 13:10:11 +0200 Subject: [PATCH 3/7] Consolidate HoverClickSounds constructors As suggested in review, merge both HoverClickSounds constructors into one accepting optional arguments. Due to existing usages the parameter is added as second and supplied by name in ModButton. --- .../Graphics/UserInterface/HoverClickSounds.cs | 18 ++++++------------ osu.Game/Overlays/Mods/ModButton.cs | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index 8fe20e3566..7e6c0a0974 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -20,24 +20,18 @@ namespace osu.Game.Graphics.UserInterface private SampleChannel sampleClick; private readonly MouseButton[] buttons; - /// - /// Creates an instance that adds sounds on hover and left click only. - /// - /// Set of click samples to play. - public HoverClickSounds(HoverSampleSet sampleSet = HoverSampleSet.Normal) - : this(new[] { MouseButton.Left }, sampleSet) - { - } - /// /// Creates an instance that adds sounds on hover and on click for any of the buttons specified. /// - /// Array of button codes which should trigger the click sound. /// Set of click samples to play. - public HoverClickSounds(MouseButton[] buttons, HoverSampleSet sampleSet = HoverSampleSet.Normal) + /// + /// Array of button codes which should trigger the click sound. + /// If this optional parameter is omitted or set to null, the click sound will also be added on left click. + /// + public HoverClickSounds(HoverSampleSet sampleSet = HoverSampleSet.Normal, MouseButton[] buttons = null) : base(sampleSet) { - this.buttons = buttons; + this.buttons = buttons ?? new[] { MouseButton.Left }; } protected override bool OnMouseUp(MouseUpEvent e) diff --git a/osu.Game/Overlays/Mods/ModButton.cs b/osu.Game/Overlays/Mods/ModButton.cs index f46555dc4b..58892cd0dd 100644 --- a/osu.Game/Overlays/Mods/ModButton.cs +++ b/osu.Game/Overlays/Mods/ModButton.cs @@ -283,7 +283,7 @@ namespace osu.Game.Overlays.Mods Anchor = Anchor.TopCentre, Font = OsuFont.GetFont(size: 18) }, - new HoverClickSounds(new[] { MouseButton.Left, MouseButton.Right }) + new HoverClickSounds(buttons: new[] { MouseButton.Left, MouseButton.Right }) }; Mod = mod; From fc48b190fedb15f243e6087315567907cc80c166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 1 Sep 2019 13:32:53 +0200 Subject: [PATCH 4/7] Fix inaccurate xmldoc --- osu.Game/Graphics/UserInterface/HoverClickSounds.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index 7e6c0a0974..050e5a2835 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -26,7 +26,7 @@ namespace osu.Game.Graphics.UserInterface /// Set of click samples to play. /// /// Array of button codes which should trigger the click sound. - /// If this optional parameter is omitted or set to null, the click sound will also be added on left click. + /// If this optional parameter is omitted or set to null, the click sound will only be added on left click. /// public HoverClickSounds(HoverSampleSet sampleSet = HoverSampleSet.Normal, MouseButton[] buttons = null) : base(sampleSet) From 53c254c6a5ca0b59f1471a670beaf81d083f0450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 2 Sep 2019 19:01:36 +0200 Subject: [PATCH 5/7] Replace Array.IndexOf() with Contains() --- osu.Game/Graphics/UserInterface/HoverClickSounds.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index 050e5a2835..e64b9259f1 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; @@ -36,9 +36,7 @@ namespace osu.Game.Graphics.UserInterface protected override bool OnMouseUp(MouseUpEvent e) { - var index = Array.IndexOf(buttons, e.Button); - bool shouldPlayEffect = index > -1 && index < buttons.Length; - + bool shouldPlayEffect = buttons.Contains(e.Button); // examine the button pressed first for short-circuiting // in most usages it is more likely that another button was pressed than that the cursor left the drawable bounds if (shouldPlayEffect && Contains(e.ScreenSpaceMousePosition)) From 299d528654f823689e14ffc6fb56c9f2db55ae5f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Sep 2019 19:20:23 +0900 Subject: [PATCH 6/7] Simplify implementation --- osu.Game/Graphics/UserInterface/HoverClickSounds.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index e64b9259f1..38e6a82bab 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -36,11 +36,9 @@ namespace osu.Game.Graphics.UserInterface protected override bool OnMouseUp(MouseUpEvent e) { - bool shouldPlayEffect = buttons.Contains(e.Button); - // examine the button pressed first for short-circuiting - // in most usages it is more likely that another button was pressed than that the cursor left the drawable bounds - if (shouldPlayEffect && Contains(e.ScreenSpaceMousePosition)) + if (buttons.Contains(e.Button) && Contains(e.ScreenSpaceMousePosition)) sampleClick?.Play(); + return base.OnMouseUp(e); } From e98059267d4348a92e4d16fe6b75d9ebe45e3912 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Sep 2019 19:21:24 +0900 Subject: [PATCH 7/7] Improve xmldoc --- osu.Game/Graphics/UserInterface/HoverClickSounds.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs index 38e6a82bab..1fb73efa65 100644 --- a/osu.Game/Graphics/UserInterface/HoverClickSounds.cs +++ b/osu.Game/Graphics/UserInterface/HoverClickSounds.cs @@ -21,12 +21,12 @@ namespace osu.Game.Graphics.UserInterface private readonly MouseButton[] buttons; /// - /// Creates an instance that adds sounds on hover and on click for any of the buttons specified. + /// a container which plays sounds on hover and click for any specified s. /// /// Set of click samples to play. /// /// Array of button codes which should trigger the click sound. - /// If this optional parameter is omitted or set to null, the click sound will only be added on left click. + /// If this optional parameter is omitted or set to null, the click sound will only be played on left click. /// public HoverClickSounds(HoverSampleSet sampleSet = HoverSampleSet.Normal, MouseButton[] buttons = null) : base(sampleSet)