From 199bcd7fdb17519bb9c6d298b0459c260864f016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 25 Mar 2025 12:56:31 +0100 Subject: [PATCH 1/3] Improve input handling in beatmap card buttons This is in response to feedback in https://osu.ppy.sh/community/forums/topics/2056547?n=1. Upon examining the button further, there was indeed some rather weird... almost hysteresis in how the button behaved with respect to the area on the screen that activated it. Because of the following scourge of a method that continues to haunt us to this day: https://github.com/ppy/osu/blob/31487545d0d17c4337d4b4cc5d4afb3ba1dae838/osu.Game/Graphics/Containers/OsuClickableContainer.cs#L24-L25 the button would effectively only be activated by 80% of its drawable area when it was not hovered, because of the scale applied to the `content` container which `Container.Content` redirected to. This is resolved here by various rearrangements of paddings and sizes such that the clickable area of any of the buttons of the card is always the full top or bottom half of the button area. Also included are some cosmetic touch-ups which happened to be convenient like folding the loading spinner into the base `BeatmapCardIconButton`, adding loading support for the favourite button, using BDL more, and resolving some "virtual member call in constructor" inspections. --- .../Cards/Buttons/BeatmapCardIconButton.cs | 60 +++++++++---------- .../Drawables/Cards/Buttons/DownloadButton.cs | 15 +---- .../Cards/Buttons/FavouriteButton.cs | 7 ++- .../Cards/Buttons/GoToBeatmapButton.cs | 5 +- .../Cards/CollapsibleButtonContainer.cs | 9 +-- 5 files changed, 40 insertions(+), 56 deletions(-) diff --git a/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs b/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs index e78fd651fe..f9a1744f5c 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs @@ -9,6 +9,7 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Events; using osu.Game.Graphics.Containers; +using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osuTK; using osuTK.Graphics; @@ -43,61 +44,48 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons } } - private float iconSize; + protected SpriteIcon Icon { get; private set; } = null!; - public float IconSize + private Container content = null!; + private Container hover = null!; + private LoadingSpinner spinner = null!; + + [BackgroundDependencyLoader] + private void load(OverlayColourProvider colourProvider) { - get => iconSize; - set - { - iconSize = value; - Icon.Size = new Vector2(iconSize); - } - } + RelativeSizeAxes = Axes.Both; - protected readonly SpriteIcon Icon; - - protected override Container Content => content; - - private readonly Container content; - private readonly Box hover; - - protected BeatmapCardIconButton() - { - Origin = Anchor.Centre; - Anchor = Anchor.Centre; - - base.Content.Add(content = new Container + Add(content = new Container { RelativeSizeAxes = Axes.Both, Masking = true, - CornerRadius = BeatmapCard.CORNER_RADIUS, Scale = new Vector2(0.8f), Origin = Anchor.Centre, Anchor = Anchor.Centre, Children = new Drawable[] { - hover = new Box + hover = new Container { RelativeSizeAxes = Axes.Both, + CornerRadius = BeatmapCard.CORNER_RADIUS, + Masking = true, Colour = Color4.White.Opacity(0.1f), Blending = BlendingParameters.Additive, + Child = new Box { RelativeSizeAxes = Axes.Both, } }, Icon = new SpriteIcon { Origin = Anchor.Centre, Anchor = Anchor.Centre, - Scale = new Vector2(1.2f), + Size = new Vector2(14), + }, + spinner = new LoadingSpinner + { + Size = new Vector2(14), }, } }); - IconSize = 12; - } - - [BackgroundDependencyLoader] - private void load(OverlayColourProvider colourProvider) - { IdleColour = colourProvider.Light1; HoverColour = colourProvider.Content1; } @@ -127,8 +115,16 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons bool isHovered = IsHovered && Enabled.Value; hover.FadeTo(isHovered ? 1f : 0f, 500, Easing.OutQuint); - content.ScaleTo(isHovered ? 1 : 0.8f, 500, Easing.OutQuint); + content.ScaleTo(isHovered ? 0.9f : 0.8f, 500, Easing.OutQuint); Icon.FadeColour(isHovered ? HoverColour : IdleColour, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); + spinner.FadeColour(isHovered ? HoverColour : IdleColour, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); + } + + protected void SetLoading(bool isLoading) + { + Icon.Alpha = isLoading ? 0 : 1; + spinner.Alpha = isLoading ? 1 : 0; + Enabled.Value = !isLoading; } } } diff --git a/osu.Game/Beatmaps/Drawables/Cards/Buttons/DownloadButton.cs b/osu.Game/Beatmaps/Drawables/Cards/Buttons/DownloadButton.cs index 7f23b46150..96ec9d0731 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/Buttons/DownloadButton.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/Buttons/DownloadButton.cs @@ -8,10 +8,8 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Game.Online.API.Requests.Responses; using osu.Game.Configuration; -using osu.Game.Graphics.UserInterface; using osu.Game.Online; using osu.Game.Resources.Localisation.Web; -using osuTK; namespace osu.Game.Beatmaps.Drawables.Cards.Buttons { @@ -23,17 +21,11 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons private Bindable preferNoVideo = null!; - private readonly LoadingSpinner spinner; - [Resolved] private BeatmapModelDownloader beatmaps { get; set; } = null!; public DownloadButton(APIBeatmapSet beatmapSet) { - Icon.Icon = FontAwesome.Solid.Download; - - Content.Add(spinner = new LoadingSpinner { Size = new Vector2(IconSize) }); - this.beatmapSet = beatmapSet; } @@ -41,6 +33,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons private void load(OsuConfigManager config) { preferNoVideo = config.GetBindable(OsuSetting.PreferNoVideo); + Icon.Icon = FontAwesome.Solid.Download; } protected override void LoadComplete() @@ -64,8 +57,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons case DownloadState.Importing: Action = null; TooltipText = string.Empty; - spinner.Show(); - Icon.Hide(); + SetLoading(true); break; case DownloadState.LocallyAvailable: @@ -84,8 +76,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons Action = () => beatmaps.Download(beatmapSet, preferNoVideo.Value); this.FadeIn(BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); - spinner.Hide(); - Icon.Show(); + SetLoading(false); if (!beatmapSet.HasVideo) TooltipText = BeatmapsetsStrings.PanelDownloadAll; diff --git a/osu.Game/Beatmaps/Drawables/Cards/Buttons/FavouriteButton.cs b/osu.Game/Beatmaps/Drawables/Cards/Buttons/FavouriteButton.cs index f698185863..0b2aaf0bc3 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/Buttons/FavouriteButton.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/Buttons/FavouriteButton.cs @@ -53,19 +53,20 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons favouriteRequest?.Cancel(); favouriteRequest = new PostBeatmapFavouriteRequest(beatmapSet.OnlineID, actionType); - Enabled.Value = false; + SetLoading(true); + favouriteRequest.Success += () => { bool favourited = actionType == BeatmapFavouriteAction.Favourite; current.Value = new BeatmapSetFavouriteState(favourited, current.Value.FavouriteCount + (favourited ? 1 : -1)); - Enabled.Value = true; + SetLoading(false); }; favouriteRequest.Failure += e => { Logger.Error(e, $"Failed to {actionType.ToString().ToLowerInvariant()} beatmap: {e.Message}"); - Enabled.Value = true; + SetLoading(false); }; api.Queue(favouriteRequest); diff --git a/osu.Game/Beatmaps/Drawables/Cards/Buttons/GoToBeatmapButton.cs b/osu.Game/Beatmaps/Drawables/Cards/Buttons/GoToBeatmapButton.cs index 3df94bf233..e95ac94457 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/Buttons/GoToBeatmapButton.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/Buttons/GoToBeatmapButton.cs @@ -20,15 +20,14 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons public GoToBeatmapButton(APIBeatmapSet beatmapSet) { this.beatmapSet = beatmapSet; - - Icon.Icon = FontAwesome.Solid.AngleDoubleRight; - TooltipText = "Go to beatmap"; } [BackgroundDependencyLoader(true)] private void load(OsuGame? game) { Action = () => game?.PresentBeatmap(beatmapSet); + Icon.Icon = FontAwesome.Solid.AngleDoubleRight; + TooltipText = "Go to beatmap"; } protected override void LoadComplete() diff --git a/osu.Game/Beatmaps/Drawables/Cards/CollapsibleButtonContainer.cs b/osu.Game/Beatmaps/Drawables/Cards/CollapsibleButtonContainer.cs index a29724032e..5ab6e1a218 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/CollapsibleButtonContainer.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/CollapsibleButtonContainer.cs @@ -95,9 +95,6 @@ namespace osu.Game.Beatmaps.Drawables.Cards Child = buttons = new Container { RelativeSizeAxes = Axes.Both, - // Padding of 4 avoids touching the card borders when in the expanded (ie. showing difficulties) state. - // Left override allows the buttons to visually be wider and look better. - Padding = new MarginPadding(4) { Left = 2 }, Children = new BeatmapCardIconButton[] { new FavouriteButton(beatmapSet) @@ -106,7 +103,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, RelativeSizeAxes = Axes.Both, - Height = 0.48f, + Height = 0.5f, }, new DownloadButton(beatmapSet) { @@ -114,7 +111,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards Origin = Anchor.BottomCentre, State = { BindTarget = downloadTracker.State }, RelativeSizeAxes = Axes.Both, - Height = 0.48f, + Height = 0.5f, }, new GoToBeatmapButton(beatmapSet) { @@ -122,7 +119,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards Origin = Anchor.BottomCentre, State = { BindTarget = downloadTracker.State }, RelativeSizeAxes = Axes.Both, - Height = 0.48f, + Height = 0.5f, } } } From 491f28c451bea1f1f73be6a54210a74d92e87a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 25 Mar 2025 20:59:25 +0100 Subject: [PATCH 2/3] Fix tests --- .../Visual/Beatmaps/TestSceneBeatmapCardFavouriteButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardFavouriteButton.cs b/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardFavouriteButton.cs index c33033624a..81abe105f1 100644 --- a/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardFavouriteButton.cs +++ b/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardFavouriteButton.cs @@ -91,6 +91,6 @@ namespace osu.Game.Tests.Visual.Beatmaps } private void assertCorrectIcon(bool favourited) => AddAssert("icon correct", - () => this.ChildrenOfType().Single().Icon.Equals(favourited ? FontAwesome.Solid.Heart : FontAwesome.Regular.Heart)); + () => this.ChildrenOfType().First().Icon.Equals(favourited ? FontAwesome.Solid.Heart : FontAwesome.Regular.Heart)); } } From 2b471919e923db34ac0b0deb5c29e1190bf36b2d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Mar 2025 19:02:26 +0900 Subject: [PATCH 3/3] Remove loading spinner and fade out icon instead --- .../Drawables/Cards/Buttons/BeatmapCardIconButton.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs b/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs index f9a1744f5c..e4bcae281c 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs @@ -9,7 +9,6 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Events; using osu.Game.Graphics.Containers; -using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osuTK; using osuTK.Graphics; @@ -48,7 +47,6 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons private Container content = null!; private Container hover = null!; - private LoadingSpinner spinner = null!; [BackgroundDependencyLoader] private void load(OverlayColourProvider colourProvider) @@ -79,10 +77,6 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons Anchor = Anchor.Centre, Size = new Vector2(14), }, - spinner = new LoadingSpinner - { - Size = new Vector2(14), - }, } }); @@ -117,13 +111,11 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons hover.FadeTo(isHovered ? 1f : 0f, 500, Easing.OutQuint); content.ScaleTo(isHovered ? 0.9f : 0.8f, 500, Easing.OutQuint); Icon.FadeColour(isHovered ? HoverColour : IdleColour, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); - spinner.FadeColour(isHovered ? HoverColour : IdleColour, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); } protected void SetLoading(bool isLoading) { - Icon.Alpha = isLoading ? 0 : 1; - spinner.Alpha = isLoading ? 1 : 0; + Icon.FadeTo(isLoading ? 0.2f : 1, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint); Enabled.Value = !isLoading; } }