Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to download beatmaps from context menu of beatmap cards #31468

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add ability to download beatmaps from context menu of beatmap cards #31468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions osu.Game/Beatmaps/Drawables/Cards/BeatmapCard.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Collections.Generic;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
Expand All @@ -24,6 +25,8 @@ public abstract partial class BeatmapCard : OsuClickableContainer, IHasContextMe

protected const float WIDTH = 345;

public CollapsibleButtonContainer ButtonContainer { get; set; } = null!;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really really dislike this. This is how multiplayer ended up in the place it currently is, where random things are exposed for the purpose of inheriting screens setting them that eventually goes completely out of control.

Let's not make the same mistake again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not 100% optimal, but also can't read your mind 😅.

I also tried an Action RequestDownload kinda flow but it felt way too verbose for what this is. Would that work better?

What's your preference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoogipoo prod on this one, need some direction before attempting something else that doesn't work for you.

Copy link
Contributor

@smoogipoo smoogipoo Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I thought your reply was in response to the comment below.

Here's two suggestions:

  1. Make ContextMenuItems virtual and duplicate the implementation.
  2. Add abstract DownloadButton? GetDownloadButton() and implement in every place.


public IBindable<bool> Expanded { get; }

public readonly APIBeatmapSet BeatmapSet;
Expand Down Expand Up @@ -103,9 +106,19 @@ public static BeatmapCard Create(APIBeatmapSet beatmapSet, BeatmapCardSize size,
}
}

public MenuItem[] ContextMenuItems => new MenuItem[]
public MenuItem[] ContextMenuItems
{
new OsuMenuItem(ContextMenuStrings.ViewBeatmap, MenuItemType.Highlighted, Action),
};
get
{
var menuItems = new List<MenuItem>();

if (ButtonContainer.DownloadButton.Enabled.Value)
menuItems.Add(new OsuMenuItem(ContextMenuStrings.DownloadBeatmap, MenuItemType.Highlighted, () => ButtonContainer.DownloadButton.TriggerClick()));
Copy link
Member

@Joehuu Joehuu Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never said this question, but what makes menu items highlighted?

Relevant search: https://github.com/search?q=repo%3Appy%2Fosu%20MenuItemType.Highlighted&type=code

I thought the highlighting was to indicate the left-click behavior, but this makes it more inconsistent right now.

The "play" button on beatmap panels in song select was a precedent to these "view" items that I made. Other menus that don't show the left-click menu item highlight something else.


menuItems.Add(new OsuMenuItem(ContextMenuStrings.ViewBeatmap, MenuItemType.Standard, Action));

return menuItems.ToArray();
}
}
}
}
5 changes: 2 additions & 3 deletions osu.Game/Beatmaps/Drawables/Cards/BeatmapCardExtra.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public partial class BeatmapCardExtra : BeatmapCard
private readonly BeatmapCardContent content;

private BeatmapCardThumbnail thumbnail = null!;
private CollapsibleButtonContainer buttonContainer = null!;

private GridContainer statisticsContainer = null!;

Expand Down Expand Up @@ -74,7 +73,7 @@ private void load(BeatmapSetOverlay? beatmapSetOverlay)
Spacing = new Vector2(1)
}
},
buttonContainer = new CollapsibleButtonContainer(BeatmapSet)
ButtonContainer = new CollapsibleButtonContainer(BeatmapSet)
{
X = height - CORNER_RADIUS,
Width = WIDTH - height + CORNER_RADIUS,
Expand Down Expand Up @@ -318,7 +317,7 @@ protected override void UpdateState()

bool showDetails = IsHovered || Expanded.Value;

buttonContainer.ShowDetails.Value = showDetails;
ButtonContainer.ShowDetails.Value = showDetails;
thumbnail.Dimmed.Value = showDetails;
}
}
Expand Down
8 changes: 3 additions & 5 deletions osu.Game/Beatmaps/Drawables/Cards/BeatmapCardNano.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public override float Width
base.Width = value;

if (LoadState >= LoadState.Ready)
buttonContainer.Width = value;
ButtonContainer.Width = value;
}
}

Expand All @@ -38,8 +38,6 @@ public override float Width
[Cached]
private readonly BeatmapCardContent content;

private CollapsibleButtonContainer buttonContainer = null!;

private FillFlowContainer idleBottomContent = null!;
private BeatmapCardDownloadProgressBar downloadProgressBar = null!;

Expand All @@ -66,7 +64,7 @@ private void load()
Height = height,
Children = new Drawable[]
{
buttonContainer = new CollapsibleButtonContainer(BeatmapSet)
ButtonContainer = new CollapsibleButtonContainer(BeatmapSet)
{
Width = Width,
FavouriteState = { BindTarget = FavouriteState },
Expand Down Expand Up @@ -163,7 +161,7 @@ protected override void UpdateState()

bool showDetails = IsHovered;

buttonContainer.ShowDetails.Value = showDetails;
ButtonContainer.ShowDetails.Value = showDetails;
}
}
}
5 changes: 2 additions & 3 deletions osu.Game/Beatmaps/Drawables/Cards/BeatmapCardNormal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public partial class BeatmapCardNormal : BeatmapCard
private readonly BeatmapCardContent content;

private BeatmapCardThumbnail thumbnail = null!;
private CollapsibleButtonContainer buttonContainer = null!;

private FillFlowContainer<BeatmapCardStatistic> statisticsContainer = null!;

Expand Down Expand Up @@ -75,7 +74,7 @@ private void load()
Spacing = new Vector2(1)
}
},
buttonContainer = new CollapsibleButtonContainer(BeatmapSet)
ButtonContainer = new CollapsibleButtonContainer(BeatmapSet)
{
X = HEIGHT - CORNER_RADIUS,
Width = WIDTH - HEIGHT + CORNER_RADIUS,
Expand Down Expand Up @@ -286,7 +285,7 @@ protected override void UpdateState()

bool showDetails = IsHovered || Expanded.Value;

buttonContainer.ShowDetails.Value = showDetails;
ButtonContainer.ShowDetails.Value = showDetails;
thumbnail.Dimmed.Value = showDetails;

statisticsContainer.FadeTo(showDetails ? 1 : 0, TRANSITION_DURATION, Easing.OutQuint);
Expand Down
4 changes: 3 additions & 1 deletion osu.Game/Beatmaps/Drawables/Cards/CollapsibleButtonContainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public partial class CollapsibleButtonContainer : Container

private readonly BeatmapDownloadTracker downloadTracker;

public DownloadButton DownloadButton { get; private set; }

private float buttonsExpandedWidth;

public float ButtonsExpandedWidth
Expand Down Expand Up @@ -108,7 +110,7 @@ public CollapsibleButtonContainer(APIBeatmapSet beatmapSet)
RelativeSizeAxes = Axes.Both,
Height = 0.48f,
},
new DownloadButton(beatmapSet)
DownloadButton = new DownloadButton(beatmapSet)
{
Anchor = Anchor.BottomCentre,
Origin = Anchor.BottomCentre,
Expand Down
5 changes: 5 additions & 0 deletions osu.Game/Localisation/ContextMenuStrings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ public static class ContextMenuStrings
/// </summary>
public static LocalisableString ViewProfile => new TranslatableString(getKey(@"view_profile"), @"View profile");

/// <summary>
/// "Download beatmap"
/// </summary>
public static LocalisableString DownloadBeatmap => new TranslatableString(getKey(@"download_beatmap"), @"Download beatmap");

/// <summary>
/// "View beatmap"
/// </summary>
Expand Down
Loading