From 3a0b2508d42b1d09da1aa30bbc356d88b5b4f76d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 15:01:30 +0900 Subject: [PATCH 01/12] Fix possible nullrefs --- osu.Game/Overlays/Direct/PanelDownloadButton.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Direct/PanelDownloadButton.cs b/osu.Game/Overlays/Direct/PanelDownloadButton.cs index ed44f1e960..1b3657f010 100644 --- a/osu.Game/Overlays/Direct/PanelDownloadButton.cs +++ b/osu.Game/Overlays/Direct/PanelDownloadButton.cs @@ -45,7 +45,7 @@ namespace osu.Game.Overlays.Direct [BackgroundDependencyLoader(true)] private void load(OsuGame game, BeatmapManager beatmaps) { - if (BeatmapSet.Value.OnlineInfo.Availability?.DownloadDisabled ?? false) + if (BeatmapSet.Value?.OnlineInfo?.Availability?.DownloadDisabled ?? false) { button.Enabled.Value = false; button.TooltipText = "this beatmap is currently not available for download."; @@ -62,7 +62,7 @@ namespace osu.Game.Overlays.Direct break; case DownloadState.LocallyAvailable: - game.PresentBeatmap(BeatmapSet.Value); + game?.PresentBeatmap(BeatmapSet.Value); break; default: From eb14dbcd77ffc141ff717f4307df40028027d451 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 15:01:45 +0900 Subject: [PATCH 02/12] Initial implementation of rearrangeable playlist --- .../TestSceneDrawableRoomPlaylist.cs | 86 +++++ .../Screens/Multi/DrawableRoomPlaylist.cs | 81 ++++ .../Screens/Multi/DrawableRoomPlaylistItem.cs | 347 ++++++++++++++++++ 3 files changed, 514 insertions(+) create mode 100644 osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs create mode 100644 osu.Game/Screens/Multi/DrawableRoomPlaylist.cs create mode 100644 osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs new file mode 100644 index 0000000000..b906ce82e9 --- /dev/null +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -0,0 +1,86 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Utils; +using osu.Game.Beatmaps; +using osu.Game.Online.Multiplayer; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Screens.Multi; +using osuTK; + +namespace osu.Game.Tests.Visual.Multiplayer +{ + public class TestSceneDrawableRoomPlaylist : OsuTestScene + { + [Resolved] + private BeatmapManager beatmapManager { get; set; } + + [Resolved] + private RulesetStore rulesetStore { get; set; } + + private DrawableRoomPlaylist playlist; + + [Test] + public void TestItemsCanNotBeRemovedOrSelectedFromNonEditableAndNonSelectablePlaylist() + { + createPlaylist(false, false); + } + + [Test] + public void TestItemsCanBeRemovedFromEditablePlaylist() + { + createPlaylist(true, false); + } + + [Test] + public void TestItemsCanBeSelectedInSelectablePlaylist() + { + createPlaylist(false, true); + } + + [Test] + public void TestItemsCanBeSelectedAndRemovedFromEditableAndSelectablePlaylist() + { + createPlaylist(true, true); + } + + private void createPlaylist(bool allowEdit, bool allowSelection) => AddStep("create playlist", () => + { + Child = playlist = new DrawableRoomPlaylist(allowEdit, allowSelection) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Size = new Vector2(500, 300) + }; + + var beatmapSets = beatmapManager.GetAllUsableBeatmapSets(); + var rulesets = rulesetStore.AvailableRulesets.ToList(); + + for (int i = 0; i < 20; i++) + { + var set = beatmapSets[RNG.Next(0, beatmapSets.Count)]; + var beatmap = set.Beatmaps[RNG.Next(0, set.Beatmaps.Count)]; + + beatmap.BeatmapSet = set; + beatmap.Metadata = set.Metadata; + + playlist.Items.Add(new PlaylistItem + { + Beatmap = { Value = beatmap }, + Ruleset = { Value = rulesets[RNG.Next(0, rulesets.Count)] }, + RequiredMods = + { + new OsuModHardRock(), + new OsuModDoubleTime(), + new OsuModAutoplay() + } + }); + } + }); + } +} diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs new file mode 100644 index 0000000000..b42a8575ec --- /dev/null +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs @@ -0,0 +1,81 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using osu.Framework.Bindables; +using osu.Framework.Extensions.IEnumerableExtensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Graphics.Containers; +using osu.Game.Online.Multiplayer; +using osuTK; + +namespace osu.Game.Screens.Multi +{ + public class DrawableRoomPlaylist : RearrangeableListContainer + { + public readonly Bindable SelectedItem = new Bindable(); + + private readonly bool allowEdit; + private readonly bool allowSelection; + + public DrawableRoomPlaylist(bool allowEdit, bool allowSelection) + { + this.allowEdit = allowEdit; + this.allowSelection = allowSelection; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + SelectedItem.BindValueChanged(item => + { + if (item.OldValue != null && ItemMap.TryGetValue(item.OldValue, out var oldItem)) + ((DrawableRoomPlaylistItem)oldItem).Deselect(); + + if (item.NewValue != null && ItemMap.TryGetValue(item.NewValue, out var newItem)) + ((DrawableRoomPlaylistItem)newItem).Select(); + }, true); + + Items.ItemsRemoved += items => + { + if (items.Any(i => i == SelectedItem.Value)) + SelectedItem.Value = null; + }; + } + + protected override ScrollContainer CreateScrollContainer() => new OsuScrollContainer + { + ScrollbarVisible = false + }; + + protected override FillFlowContainer> CreateListFillFlowContainer() => new FillFlowContainer> + { + LayoutDuration = 200, + LayoutEasing = Easing.OutQuint, + Spacing = new Vector2(0, 2) + }; + + protected override RearrangeableListItem CreateDrawable(PlaylistItem item) => new DrawableRoomPlaylistItem(item, allowEdit, allowSelection) + { + RequestSelection = requestSelection, + RequestDeletion = requestDeletion + }; + + private void requestSelection(PlaylistItem item) => SelectedItem.Value = item; + + private void requestDeletion(PlaylistItem item) + { + if (SelectedItem.Value == item) + { + if (Items.Count == 1) + SelectedItem.Value = null; + else + SelectedItem.Value = Items.GetNext(item) ?? Items[^2]; + } + + Items.Remove(item); + } + } +} diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs new file mode 100644 index 0000000000..d2788a5c4b --- /dev/null +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs @@ -0,0 +1,347 @@ +// 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.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Colour; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Input.Events; +using osu.Framework.Threading; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Drawables; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; +using osu.Game.Graphics.UserInterface; +using osu.Game.Online.Chat; +using osu.Game.Online.Multiplayer; +using osu.Game.Overlays.Direct; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Mods; +using osu.Game.Screens.Play.HUD; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Screens.Multi +{ + public class DrawableRoomPlaylistItem : RearrangeableListItem + { + public Action RequestSelection; + public Action RequestDeletion; + + private Container maskingContainer; + private Container difficultyIconContainer; + private LinkFlowContainer beatmapText; + private LinkFlowContainer authorText; + private ItemHandle handle; + private ModDisplay modDisplay; + + private readonly Bindable beatmap = new Bindable(); + private readonly Bindable ruleset = new Bindable(); + private readonly BindableList requiredMods = new BindableList(); + + private readonly PlaylistItem item; + private readonly bool allowEdit; + private readonly bool allowSelection; + + public DrawableRoomPlaylistItem(PlaylistItem item, bool allowEdit, bool allowSelection) + : base(item) + { + this.item = item; + this.allowEdit = allowEdit; + this.allowSelection = allowSelection; + + RelativeSizeAxes = Axes.X; + Height = 50; + + beatmap.BindTo(item.Beatmap); + ruleset.BindTo(item.Ruleset); + requiredMods.BindTo(item.RequiredMods); + } + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + InternalChild = new GridContainer + { + RelativeSizeAxes = Axes.Both, + Content = new[] + { + new Drawable[] + { + new Container + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + AutoSizeAxes = Axes.Both, + Alpha = allowEdit ? 1 : 0, + Child = handle = new ItemHandle + { + Size = new Vector2(12), + AlwaysPresent = true, + Alpha = 0, + }, + }, + maskingContainer = new Container + { + RelativeSizeAxes = Axes.Both, + Masking = true, + CornerRadius = 10, + BorderColour = colours.Yellow, + Children = new Drawable[] + { + new Box // A transparent box that forces the border to be drawn if the panel background is opaque + { + RelativeSizeAxes = Axes.Both, + Alpha = 0, + AlwaysPresent = true + }, + new PanelBackground + { + RelativeSizeAxes = Axes.Both, + Beatmap = { BindTarget = beatmap } + }, + new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Left = 8 }, + Spacing = new Vector2(8, 0), + Direction = FillDirection.Horizontal, + Children = new Drawable[] + { + difficultyIconContainer = new Container + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + }, + new FillFlowContainer + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + beatmapText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, + new FillFlowContainer + { + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Horizontal, + Spacing = new Vector2(10, 0), + Children = new Drawable[] + { + authorText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, + modDisplay = new ModDisplay + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + Scale = new Vector2(0.4f), + DisplayUnrankedText = false, + ExpansionMode = ExpansionMode.AlwaysExpanded + } + } + } + } + } + } + }, + new Container + { + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + AutoSizeAxes = Axes.Both, + X = -18, + Children = new Drawable[] + { + new IconButton + { + Icon = FontAwesome.Solid.MinusSquare, + Alpha = allowEdit ? 1 : 0, + Action = () => RequestDeletion?.Invoke(Model), + }, + new PanelDownloadButton(item.Beatmap.Value.BeatmapSet) + { + Size = new Vector2(50, 30), + Alpha = allowEdit ? 0 : 1 + } + } + } + } + } + }, + }, + ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, + }; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + beatmap.BindValueChanged(_ => scheduleRefresh()); + ruleset.BindValueChanged(_ => scheduleRefresh()); + + requiredMods.ItemsAdded += _ => scheduleRefresh(); + requiredMods.ItemsRemoved += _ => scheduleRefresh(); + + refresh(); + } + + private ScheduledDelegate scheduledRefresh; + + private void scheduleRefresh() + { + scheduledRefresh?.Cancel(); + scheduledRefresh = Schedule(refresh); + } + + private void refresh() + { + difficultyIconContainer.Child = new DifficultyIcon(beatmap.Value, ruleset.Value) { Size = new Vector2(32) }; + + beatmapText.Clear(); + beatmapText.AddLink(item.Beatmap.ToString(), LinkAction.OpenBeatmap, item.Beatmap.Value.OnlineBeatmapID.ToString()); + + authorText.Clear(); + + if (item.Beatmap?.Value?.Metadata?.Author != null) + { + authorText.AddText("mapped by "); + authorText.AddUserLink(item.Beatmap.Value?.Metadata.Author); + } + + modDisplay.Current.Value = requiredMods.ToArray(); + } + + protected override bool IsDraggableAt(Vector2 screenSpacePos) => handle.HandlingDrag; + + protected override bool OnHover(HoverEvent e) + { + handle.UpdateHoverState(true); + return base.OnHover(e); + } + + protected override void OnHoverLost(HoverLostEvent e) => handle.UpdateHoverState(false); + + protected override bool OnClick(ClickEvent e) + { + if (allowSelection) + RequestSelection?.Invoke(Model); + return true; + } + + public void Select() => maskingContainer.BorderThickness = 5; + + public void Deselect() => maskingContainer.BorderThickness = 0; + + // For now, this is the same implementation as in PanelBackground, but supports a beatmap info rather than a working beatmap + private class PanelBackground : Container // todo: should be a buffered container (https://github.com/ppy/osu-framework/issues/3222) + { + public readonly Bindable Beatmap = new Bindable(); + + public PanelBackground() + { + InternalChildren = new Drawable[] + { + new UpdateableBeatmapBackgroundSprite + { + RelativeSizeAxes = Axes.Both, + FillMode = FillMode.Fill, + Beatmap = { BindTarget = Beatmap } + }, + new Container + { + Depth = -1, + RelativeSizeAxes = Axes.Both, + // This makes the gradient not be perfectly horizontal, but diagonal at a ~40° angle + Shear = new Vector2(0.8f, 0), + Alpha = 0.5f, + Children = new[] + { + // The left half with no gradient applied + new Box + { + RelativeSizeAxes = Axes.Both, + RelativePositionAxes = Axes.Both, + Colour = Color4.Black, + Width = 0.4f, + }, + // Piecewise-linear gradient with 3 segments to make it appear smoother + new Box + { + RelativeSizeAxes = Axes.Both, + RelativePositionAxes = Axes.Both, + Colour = ColourInfo.GradientHorizontal(Color4.Black, new Color4(0f, 0f, 0f, 0.9f)), + Width = 0.05f, + X = 0.4f, + }, + new Box + { + RelativeSizeAxes = Axes.Both, + RelativePositionAxes = Axes.Both, + Colour = ColourInfo.GradientHorizontal(new Color4(0f, 0f, 0f, 0.9f), new Color4(0f, 0f, 0f, 0.1f)), + Width = 0.2f, + X = 0.45f, + }, + new Box + { + RelativeSizeAxes = Axes.Both, + RelativePositionAxes = Axes.Both, + Colour = ColourInfo.GradientHorizontal(new Color4(0f, 0f, 0f, 0.1f), new Color4(0, 0, 0, 0)), + Width = 0.05f, + X = 0.65f, + }, + } + } + }; + } + } + + private class ItemHandle : SpriteIcon + { + public bool HandlingDrag { get; private set; } + private bool isHovering; + + public ItemHandle() + { + Margin = new MarginPadding { Horizontal = 5 }; + + Icon = FontAwesome.Solid.Bars; + } + + protected override bool OnMouseDown(MouseDownEvent e) + { + base.OnMouseDown(e); + + HandlingDrag = true; + UpdateHoverState(isHovering); + + return false; + } + + protected override void OnMouseUp(MouseUpEvent e) + { + base.OnMouseUp(e); + + HandlingDrag = false; + UpdateHoverState(isHovering); + } + + public void UpdateHoverState(bool hovering) + { + isHovering = hovering; + + if (isHovering || HandlingDrag) + this.FadeIn(100); + else + this.FadeOut(100); + } + } + } +} From eb75d26c8f5c3365fda1965401f4df77fb900078 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 15:36:16 +0900 Subject: [PATCH 03/12] Extract common rearrangeable list design --- .../OsuRearrangeableListContainer.cs | 26 +++ .../Containers/OsuRearrangeableListItem.cs | 149 ++++++++++++++++++ osu.Game/Overlays/Music/Playlist.cs | 12 +- osu.Game/Overlays/Music/PlaylistItem.cs | 121 ++------------ 4 files changed, 190 insertions(+), 118 deletions(-) create mode 100644 osu.Game/Graphics/Containers/OsuRearrangeableListContainer.cs create mode 100644 osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs diff --git a/osu.Game/Graphics/Containers/OsuRearrangeableListContainer.cs b/osu.Game/Graphics/Containers/OsuRearrangeableListContainer.cs new file mode 100644 index 0000000000..47aed1c500 --- /dev/null +++ b/osu.Game/Graphics/Containers/OsuRearrangeableListContainer.cs @@ -0,0 +1,26 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; + +namespace osu.Game.Graphics.Containers +{ + public abstract class OsuRearrangeableListContainer : RearrangeableListContainer + { + /// + /// Whether any item is currently being dragged. Used to hide other items' drag handles. + /// + private readonly BindableBool playlistDragActive = new BindableBool(); + + protected override ScrollContainer CreateScrollContainer() => new OsuScrollContainer(); + + protected sealed override RearrangeableListItem CreateDrawable(TModel item) => CreateOsuDrawable(item).With(d => + { + d.PlaylistDragActive.BindTo(playlistDragActive); + }); + + protected abstract OsuRearrangeableListItem CreateOsuDrawable(TModel item); + } +} diff --git a/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs b/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs new file mode 100644 index 0000000000..f75a3330e2 --- /dev/null +++ b/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs @@ -0,0 +1,149 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Input.Events; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Graphics.Containers +{ + public abstract class OsuRearrangeableListItem : RearrangeableListItem + { + public const float FADE_DURATION = 100; + + /// + /// Whether any item is currently being dragged. Used to hide other items' drag handles. + /// + public readonly BindableBool PlaylistDragActive = new BindableBool(); + + private Color4 handleColour = Color4.White; + + protected Color4 HandleColour + { + get => handleColour; + set + { + if (handleColour == value) + return; + + handleColour = value; + + if (handle != null) + handle.Colour = value; + } + } + + private PlaylistItemHandle handle; + + protected OsuRearrangeableListItem(TModel item) + : base(item) + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + } + + [BackgroundDependencyLoader] + private void load() + { + InternalChild = new GridContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Content = new[] + { + new[] + { + new Container + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + AutoSizeAxes = Axes.Both, + Padding = new MarginPadding { Horizontal = 5 }, + Child = handle = new PlaylistItemHandle + { + Size = new Vector2(12), + Colour = HandleColour, + AlwaysPresent = true, + Alpha = 0 + } + }, + CreateContent() + } + }, + ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, + RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) } + }; + } + + protected override bool OnDragStart(DragStartEvent e) + { + if (!base.OnDragStart(e)) + return false; + + PlaylistDragActive.Value = true; + return true; + } + + protected override void OnDragEnd(DragEndEvent e) + { + PlaylistDragActive.Value = false; + base.OnDragEnd(e); + } + + protected override bool IsDraggableAt(Vector2 screenSpacePos) => handle.HandlingDrag; + + protected override bool OnHover(HoverEvent e) + { + handle.UpdateHoverState(IsDragged || !PlaylistDragActive.Value); + return base.OnHover(e); + } + + protected override void OnHoverLost(HoverLostEvent e) => handle.UpdateHoverState(false); + + protected abstract Drawable CreateContent(); + + private class PlaylistItemHandle : SpriteIcon + { + public bool HandlingDrag { get; private set; } + private bool isHovering; + + public PlaylistItemHandle() + { + Icon = FontAwesome.Solid.Bars; + } + + protected override bool OnMouseDown(MouseDownEvent e) + { + base.OnMouseDown(e); + + HandlingDrag = true; + UpdateHoverState(isHovering); + + return false; + } + + protected override void OnMouseUp(MouseUpEvent e) + { + base.OnMouseUp(e); + + HandlingDrag = false; + UpdateHoverState(isHovering); + } + + public void UpdateHoverState(bool hovering) + { + isHovering = hovering; + + if (isHovering || HandlingDrag) + this.FadeIn(FADE_DURATION); + else + this.FadeOut(FADE_DURATION); + } + } + } +} diff --git a/osu.Game/Overlays/Music/Playlist.cs b/osu.Game/Overlays/Music/Playlist.cs index 1ba568443d..621a533dd6 100644 --- a/osu.Game/Overlays/Music/Playlist.cs +++ b/osu.Game/Overlays/Music/Playlist.cs @@ -12,17 +12,12 @@ using osuTK; namespace osu.Game.Overlays.Music { - public class Playlist : RearrangeableListContainer + public class Playlist : OsuRearrangeableListContainer { public Action RequestSelection; public readonly Bindable SelectedSet = new Bindable(); - /// - /// Whether any item is currently being dragged. Used to hide other items' drag handles. - /// - private readonly BindableBool playlistDragActive = new BindableBool(); - public new MarginPadding Padding { get => base.Padding; @@ -33,15 +28,12 @@ namespace osu.Game.Overlays.Music public BeatmapSetInfo FirstVisibleSet => Items.FirstOrDefault(i => ((PlaylistItem)ItemMap[i]).MatchingFilter); - protected override RearrangeableListItem CreateDrawable(BeatmapSetInfo item) => new PlaylistItem(item) + protected override OsuRearrangeableListItem CreateOsuDrawable(BeatmapSetInfo item) => new PlaylistItem(item) { SelectedSet = { BindTarget = SelectedSet }, - PlaylistDragActive = { BindTarget = playlistDragActive }, RequestSelection = set => RequestSelection?.Invoke(set) }; - protected override ScrollContainer CreateScrollContainer() => new OsuScrollContainer(); - protected override FillFlowContainer> CreateListFillFlowContainer() => new SearchContainer> { Spacing = new Vector2(0, 3), diff --git a/osu.Game/Overlays/Music/PlaylistItem.cs b/osu.Game/Overlays/Music/PlaylistItem.cs index 0569261867..8cafbc694a 100644 --- a/osu.Game/Overlays/Music/PlaylistItem.cs +++ b/osu.Game/Overlays/Music/PlaylistItem.cs @@ -14,36 +14,27 @@ using osu.Framework.Localisation; using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.Containers; -using osuTK; using osuTK.Graphics; namespace osu.Game.Overlays.Music { - public class PlaylistItem : RearrangeableListItem, IFilterable + public class PlaylistItem : OsuRearrangeableListItem, IFilterable { - private const float fade_duration = 100; - - public BindableBool PlaylistDragActive = new BindableBool(); - public readonly Bindable SelectedSet = new Bindable(); public Action RequestSelection; - private PlaylistItemHandle handle; private TextFlowContainer text; private IEnumerable titleSprites; private ILocalisedBindableString titleBind; private ILocalisedBindableString artistBind; - private Color4 hoverColour; + private Color4 selectedColour; private Color4 artistColour; public PlaylistItem(BeatmapSetInfo item) : base(item) { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - Padding = new MarginPadding { Left = 5 }; FilterTerms = item.Metadata.SearchableTerms; @@ -52,42 +43,12 @@ namespace osu.Game.Overlays.Music [BackgroundDependencyLoader] private void load(OsuColour colours, LocalisationManager localisation) { - hoverColour = colours.Yellow; + selectedColour = colours.Yellow; artistColour = colours.Gray9; - - InternalChild = new GridContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Content = new[] - { - new Drawable[] - { - handle = new PlaylistItemHandle - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - Size = new Vector2(12), - Colour = colours.Gray5, - AlwaysPresent = true, - Alpha = 0 - }, - text = new OsuTextFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Padding = new MarginPadding { Left = 5 }, - }, - } - }, - ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, - RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) } - }; + HandleColour = colours.Gray5; titleBind = localisation.GetLocalisedString(new LocalisedString((Model.Metadata.TitleUnicode, Model.Metadata.Title))); artistBind = localisation.GetLocalisedString(new LocalisedString((Model.Metadata.ArtistUnicode, Model.Metadata.Artist))); - - artistBind.BindValueChanged(_ => recreateText(), true); } protected override void LoadComplete() @@ -100,10 +61,18 @@ namespace osu.Game.Overlays.Music return; foreach (Drawable s in titleSprites) - s.FadeColour(set.NewValue == Model ? hoverColour : Color4.White, fade_duration); + s.FadeColour(set.NewValue == Model ? selectedColour : Color4.White, FADE_DURATION); }, true); + + artistBind.BindValueChanged(_ => recreateText(), true); } + protected override Drawable CreateContent() => text = new OsuTextFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + }; + private void recreateText() { text.Clear(); @@ -125,31 +94,6 @@ namespace osu.Game.Overlays.Music return true; } - protected override bool OnDragStart(DragStartEvent e) - { - if (!base.OnDragStart(e)) - return false; - - PlaylistDragActive.Value = true; - return true; - } - - protected override void OnDragEnd(DragEndEvent e) - { - PlaylistDragActive.Value = false; - base.OnDragEnd(e); - } - - protected override bool IsDraggableAt(Vector2 screenSpacePos) => handle.HandlingDrag; - - protected override bool OnHover(HoverEvent e) - { - handle.UpdateHoverState(IsDragged || !PlaylistDragActive.Value); - return base.OnHover(e); - } - - protected override void OnHoverLost(HoverLostEvent e) => handle.UpdateHoverState(false); - public IEnumerable FilterTerms { get; } private bool matching = true; @@ -168,44 +112,5 @@ namespace osu.Game.Overlays.Music } public bool FilteringActive { get; set; } - - private class PlaylistItemHandle : SpriteIcon - { - public bool HandlingDrag { get; private set; } - private bool isHovering; - - public PlaylistItemHandle() - { - Icon = FontAwesome.Solid.Bars; - } - - protected override bool OnMouseDown(MouseDownEvent e) - { - base.OnMouseDown(e); - - HandlingDrag = true; - UpdateHoverState(isHovering); - - return false; - } - - protected override void OnMouseUp(MouseUpEvent e) - { - base.OnMouseUp(e); - - HandlingDrag = false; - UpdateHoverState(isHovering); - } - - public void UpdateHoverState(bool hovering) - { - isHovering = hovering; - - if (isHovering || HandlingDrag) - this.FadeIn(fade_duration); - else - this.FadeOut(fade_duration); - } - } } } From 5e871f9838b16f5319cfa5886dc4a81c93137641 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 15:36:47 +0900 Subject: [PATCH 04/12] Make room playlist use the common rearrangeable design --- .../TestSceneDrawableRoomPlaylist.cs | 16 +- .../Screens/Multi/DrawableRoomPlaylist.cs | 10 +- .../Screens/Multi/DrawableRoomPlaylistItem.cs | 263 +++++++----------- 3 files changed, 112 insertions(+), 177 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index b906ce82e9..393fd281d4 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -1,6 +1,8 @@ // 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.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -17,6 +19,12 @@ namespace osu.Game.Tests.Visual.Multiplayer { public class TestSceneDrawableRoomPlaylist : OsuTestScene { + public override IReadOnlyList RequiredTypes => new[] + { + typeof(DrawableRoomPlaylist), + typeof(DrawableRoomPlaylistItem) + }; + [Resolved] private BeatmapManager beatmapManager { get; set; } @@ -26,25 +34,25 @@ namespace osu.Game.Tests.Visual.Multiplayer private DrawableRoomPlaylist playlist; [Test] - public void TestItemsCanNotBeRemovedOrSelectedFromNonEditableAndNonSelectablePlaylist() + public void TestNonEditableNonSelectable() { createPlaylist(false, false); } [Test] - public void TestItemsCanBeRemovedFromEditablePlaylist() + public void TestEditable() { createPlaylist(true, false); } [Test] - public void TestItemsCanBeSelectedInSelectablePlaylist() + public void TestSelectable() { createPlaylist(false, true); } [Test] - public void TestItemsCanBeSelectedAndRemovedFromEditableAndSelectablePlaylist() + public void TestEditableSelectable() { createPlaylist(true, true); } diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs index b42a8575ec..021a8ca176 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs @@ -12,7 +12,7 @@ using osuTK; namespace osu.Game.Screens.Multi { - public class DrawableRoomPlaylist : RearrangeableListContainer + public class DrawableRoomPlaylist : OsuRearrangeableListContainer { public readonly Bindable SelectedItem = new Bindable(); @@ -45,10 +45,10 @@ namespace osu.Game.Screens.Multi }; } - protected override ScrollContainer CreateScrollContainer() => new OsuScrollContainer + protected override ScrollContainer CreateScrollContainer() => base.CreateScrollContainer().With(d => { - ScrollbarVisible = false - }; + d.ScrollbarVisible = false; + }); protected override FillFlowContainer> CreateListFillFlowContainer() => new FillFlowContainer> { @@ -57,7 +57,7 @@ namespace osu.Game.Screens.Multi Spacing = new Vector2(0, 2) }; - protected override RearrangeableListItem CreateDrawable(PlaylistItem item) => new DrawableRoomPlaylistItem(item, allowEdit, allowSelection) + protected override OsuRearrangeableListItem CreateOsuDrawable(PlaylistItem item) => new DrawableRoomPlaylistItem(item, allowEdit, allowSelection) { RequestSelection = requestSelection, RequestDeletion = requestDeletion diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs index d2788a5c4b..2bc593d779 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; @@ -28,7 +29,7 @@ using osuTK.Graphics; namespace osu.Game.Screens.Multi { - public class DrawableRoomPlaylistItem : RearrangeableListItem + public class DrawableRoomPlaylistItem : OsuRearrangeableListItem { public Action RequestSelection; public Action RequestDeletion; @@ -37,7 +38,6 @@ namespace osu.Game.Screens.Multi private Container difficultyIconContainer; private LinkFlowContainer beatmapText; private LinkFlowContainer authorText; - private ItemHandle handle; private ModDisplay modDisplay; private readonly Bindable beatmap = new Bindable(); @@ -55,9 +55,6 @@ namespace osu.Game.Screens.Multi this.allowEdit = allowEdit; this.allowSelection = allowSelection; - RelativeSizeAxes = Axes.X; - Height = 50; - beatmap.BindTo(item.Beatmap); ruleset.BindTo(item.Ruleset); requiredMods.BindTo(item.RequiredMods); @@ -66,118 +63,10 @@ namespace osu.Game.Screens.Multi [BackgroundDependencyLoader] private void load(OsuColour colours) { - InternalChild = new GridContainer - { - RelativeSizeAxes = Axes.Both, - Content = new[] - { - new Drawable[] - { - new Container - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - AutoSizeAxes = Axes.Both, - Alpha = allowEdit ? 1 : 0, - Child = handle = new ItemHandle - { - Size = new Vector2(12), - AlwaysPresent = true, - Alpha = 0, - }, - }, - maskingContainer = new Container - { - RelativeSizeAxes = Axes.Both, - Masking = true, - CornerRadius = 10, - BorderColour = colours.Yellow, - Children = new Drawable[] - { - new Box // A transparent box that forces the border to be drawn if the panel background is opaque - { - RelativeSizeAxes = Axes.Both, - Alpha = 0, - AlwaysPresent = true - }, - new PanelBackground - { - RelativeSizeAxes = Axes.Both, - Beatmap = { BindTarget = beatmap } - }, - new FillFlowContainer - { - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Left = 8 }, - Spacing = new Vector2(8, 0), - Direction = FillDirection.Horizontal, - Children = new Drawable[] - { - difficultyIconContainer = new Container - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - AutoSizeAxes = Axes.Both, - }, - new FillFlowContainer - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - AutoSizeAxes = Axes.Both, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - beatmapText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, - new FillFlowContainer - { - AutoSizeAxes = Axes.Both, - Direction = FillDirection.Horizontal, - Spacing = new Vector2(10, 0), - Children = new Drawable[] - { - authorText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, - modDisplay = new ModDisplay - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - AutoSizeAxes = Axes.Both, - Scale = new Vector2(0.4f), - DisplayUnrankedText = false, - ExpansionMode = ExpansionMode.AlwaysExpanded - } - } - } - } - } - } - }, - new Container - { - Anchor = Anchor.CentreRight, - Origin = Anchor.CentreRight, - AutoSizeAxes = Axes.Both, - X = -18, - Children = new Drawable[] - { - new IconButton - { - Icon = FontAwesome.Solid.MinusSquare, - Alpha = allowEdit ? 1 : 0, - Action = () => RequestDeletion?.Invoke(Model), - }, - new PanelDownloadButton(item.Beatmap.Value.BeatmapSet) - { - Size = new Vector2(50, 30), - Alpha = allowEdit ? 0 : 1 - } - } - } - } - } - }, - }, - ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, - }; + if (!allowEdit) + HandleColour = HandleColour.Opacity(0); + + maskingContainer.BorderColour = colours.Yellow; } protected override void LoadComplete() @@ -193,6 +82,95 @@ namespace osu.Game.Screens.Multi refresh(); } + protected override Drawable CreateContent() => maskingContainer = new Container + { + RelativeSizeAxes = Axes.X, + Height = 50, + Masking = true, + CornerRadius = 10, + Children = new Drawable[] + { + new Box // A transparent box that forces the border to be drawn if the panel background is opaque + { + RelativeSizeAxes = Axes.Both, + Alpha = 0, + AlwaysPresent = true + }, + new PanelBackground + { + RelativeSizeAxes = Axes.Both, + Beatmap = { BindTarget = beatmap } + }, + new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Left = 8 }, + Spacing = new Vector2(8, 0), + Direction = FillDirection.Horizontal, + Children = new Drawable[] + { + difficultyIconContainer = new Container + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + }, + new FillFlowContainer + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + beatmapText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, + new FillFlowContainer + { + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Horizontal, + Spacing = new Vector2(10, 0), + Children = new Drawable[] + { + authorText = new LinkFlowContainer { AutoSizeAxes = Axes.Both }, + modDisplay = new ModDisplay + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + Scale = new Vector2(0.4f), + DisplayUnrankedText = false, + ExpansionMode = ExpansionMode.AlwaysExpanded + } + } + } + } + } + } + }, + new Container + { + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + AutoSizeAxes = Axes.Both, + X = -18, + Children = new Drawable[] + { + new IconButton + { + Icon = FontAwesome.Solid.MinusSquare, + Alpha = allowEdit ? 1 : 0, + Action = () => RequestDeletion?.Invoke(Model), + }, + new PanelDownloadButton(item.Beatmap.Value.BeatmapSet) + { + Size = new Vector2(50, 30), + Alpha = allowEdit ? 0 : 1 + } + } + } + } + }; + private ScheduledDelegate scheduledRefresh; private void scheduleRefresh() @@ -219,16 +197,6 @@ namespace osu.Game.Screens.Multi modDisplay.Current.Value = requiredMods.ToArray(); } - protected override bool IsDraggableAt(Vector2 screenSpacePos) => handle.HandlingDrag; - - protected override bool OnHover(HoverEvent e) - { - handle.UpdateHoverState(true); - return base.OnHover(e); - } - - protected override void OnHoverLost(HoverLostEvent e) => handle.UpdateHoverState(false); - protected override bool OnClick(ClickEvent e) { if (allowSelection) @@ -302,46 +270,5 @@ namespace osu.Game.Screens.Multi }; } } - - private class ItemHandle : SpriteIcon - { - public bool HandlingDrag { get; private set; } - private bool isHovering; - - public ItemHandle() - { - Margin = new MarginPadding { Horizontal = 5 }; - - Icon = FontAwesome.Solid.Bars; - } - - protected override bool OnMouseDown(MouseDownEvent e) - { - base.OnMouseDown(e); - - HandlingDrag = true; - UpdateHoverState(isHovering); - - return false; - } - - protected override void OnMouseUp(MouseUpEvent e) - { - base.OnMouseUp(e); - - HandlingDrag = false; - UpdateHoverState(isHovering); - } - - public void UpdateHoverState(bool hovering) - { - isHovering = hovering; - - if (isHovering || HandlingDrag) - this.FadeIn(100); - else - this.FadeOut(100); - } - } } } From 1909ea2bd3a17cd3d6a877dd4b01c1585d4f631c Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 16:09:15 +0900 Subject: [PATCH 05/12] Add a way to hide the drag handle --- .../Containers/OsuRearrangeableListItem.cs | 17 +++++++++++++++-- .../Screens/Multi/DrawableRoomPlaylistItem.cs | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs b/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs index f75a3330e2..29553954fe 100644 --- a/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs +++ b/osu.Game/Graphics/Containers/OsuRearrangeableListItem.cs @@ -23,6 +23,9 @@ namespace osu.Game.Graphics.Containers private Color4 handleColour = Color4.White; + /// + /// The colour of the drag handle. + /// protected Color4 HandleColour { get => handleColour; @@ -38,6 +41,11 @@ namespace osu.Game.Graphics.Containers } } + /// + /// Whether the drag handle should be shown. + /// + protected virtual bool ShowDragHandle => true; + private PlaylistItemHandle handle; protected OsuRearrangeableListItem(TModel item) @@ -50,6 +58,8 @@ namespace osu.Game.Graphics.Containers [BackgroundDependencyLoader] private void load() { + Container handleContainer; + InternalChild = new GridContainer { RelativeSizeAxes = Axes.X, @@ -58,7 +68,7 @@ namespace osu.Game.Graphics.Containers { new[] { - new Container + handleContainer = new Container { Anchor = Anchor.Centre, Origin = Anchor.Centre, @@ -78,6 +88,9 @@ namespace osu.Game.Graphics.Containers ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) } }; + + if (!ShowDragHandle) + handleContainer.Alpha = 0; } protected override bool OnDragStart(DragStartEvent e) @@ -107,7 +120,7 @@ namespace osu.Game.Graphics.Containers protected abstract Drawable CreateContent(); - private class PlaylistItemHandle : SpriteIcon + public class PlaylistItemHandle : SpriteIcon { public bool HandlingDrag { get; private set; } private bool isHovering; diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs index 2bc593d779..7156839dfc 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs @@ -34,6 +34,8 @@ namespace osu.Game.Screens.Multi public Action RequestSelection; public Action RequestDeletion; + protected override bool ShowDragHandle => allowEdit; + private Container maskingContainer; private Container difficultyIconContainer; private LinkFlowContainer beatmapText; From d6768bba628c2feafac4c28b5719cc83ed83e48a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 16:09:30 +0900 Subject: [PATCH 06/12] Make test playlist items unique --- .../Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index 393fd281d4..d43bf9edea 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -79,6 +79,7 @@ namespace osu.Game.Tests.Visual.Multiplayer playlist.Items.Add(new PlaylistItem { + ID = i, Beatmap = { Value = beatmap }, Ruleset = { Value = rulesets[RNG.Next(0, rulesets.Count)] }, RequiredMods = From afd3e4604b5649e4f27e52afa6869e64249e8e2a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 16:48:23 +0900 Subject: [PATCH 07/12] Fix race condition causing selection to be reset incorrectly --- osu.Game/Screens/Multi/DrawableRoomPlaylist.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs index 021a8ca176..01f173db4e 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Linq; using osu.Framework.Bindables; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; @@ -38,11 +37,11 @@ namespace osu.Game.Screens.Multi ((DrawableRoomPlaylistItem)newItem).Select(); }, true); - Items.ItemsRemoved += items => + Items.ItemsRemoved += items => Schedule(() => { - if (items.Any(i => i == SelectedItem.Value)) + if (!Items.Contains(SelectedItem.Value)) SelectedItem.Value = null; - }; + }); } protected override ScrollContainer CreateScrollContainer() => base.CreateScrollContainer().With(d => From 6a466ea2f584e8c8c74ed8e92414ea00c5532dce Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 16:48:30 +0900 Subject: [PATCH 08/12] Improve test cases --- .../TestSceneDrawableRoomPlaylist.cs | 148 +++++++++++++++++- 1 file changed, 147 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index d43bf9edea..267496bba2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -7,17 +7,22 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Graphics.Containers; +using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu.Mods; using osu.Game.Screens.Multi; using osuTK; +using osuTK.Input; namespace osu.Game.Tests.Visual.Multiplayer { - public class TestSceneDrawableRoomPlaylist : OsuTestScene + public class TestSceneDrawableRoomPlaylist : ManualInputManagerTestScene { public override IReadOnlyList RequiredTypes => new[] { @@ -37,26 +42,167 @@ namespace osu.Game.Tests.Visual.Multiplayer public void TestNonEditableNonSelectable() { createPlaylist(false, false); + + moveToItem(0); + assertHandleVisibility(0, false); + assertDeleteButtonVisibility(0, false); } [Test] public void TestEditable() { createPlaylist(true, false); + + moveToItem(0); + assertHandleVisibility(0, true); + assertDeleteButtonVisibility(0, true); } [Test] public void TestSelectable() { createPlaylist(false, true); + + moveToItem(0); + assertHandleVisibility(0, false); + assertDeleteButtonVisibility(0, false); + + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("item 0 is selected", () => playlist.SelectedItem.Value == playlist.Items[0]); } [Test] public void TestEditableSelectable() { createPlaylist(true, true); + + moveToItem(0); + assertHandleVisibility(0, true); + assertDeleteButtonVisibility(0, true); + + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("item 0 is selected", () => playlist.SelectedItem.Value == playlist.Items[0]); } + [Test] + public void TestSelectionNotLostAfterRearrangement() + { + createPlaylist(true, true); + + moveToItem(0); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + moveToDragger(0); + AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left)); + moveToDragger(1, new Vector2(0, 5)); + AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left)); + + AddAssert("item 1 is selected", () => playlist.SelectedItem.Value == playlist.Items[1]); + } + + [Test] + public void TestItemRemovedOnDeletion() + { + PlaylistItem selectedItem = null; + + createPlaylist(true, true); + + moveToItem(0); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("retrieve selection", () => selectedItem = playlist.SelectedItem.Value); + + moveToDeleteButton(0); + AddStep("click delete button", () => InputManager.Click(MouseButton.Left)); + + AddAssert("item removed", () => !playlist.Items.Contains(selectedItem)); + } + + [Test] + public void TestNextItemSelectedAfterDeletion() + { + createPlaylist(true, true); + + moveToItem(0); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + moveToDeleteButton(0); + AddStep("click delete button", () => InputManager.Click(MouseButton.Left)); + + AddAssert("item 0 is selected", () => playlist.SelectedItem.Value == playlist.Items[0]); + } + + [Test] + public void TestLastItemSelectedAfterLastItemDeleted() + { + createPlaylist(true, true); + + AddWaitStep("wait for flow", 5); // Items may take 1 update frame to flow. A wait count of 5 is guaranteed to result in the flow being updated as desired. + AddStep("scroll to bottom", () => playlist.ChildrenOfType>().First().ScrollToEnd(false)); + + moveToItem(19); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + moveToDeleteButton(19); + AddStep("click delete button", () => InputManager.Click(MouseButton.Left)); + + AddAssert("item 18 is selected", () => playlist.SelectedItem.Value == playlist.Items[18]); + } + + [Test] + public void TestSelectionResetWhenAllItemsDeleted() + { + createPlaylist(true, true); + + AddStep("remove all but one item", () => + { + playlist.Items.RemoveRange(1, playlist.Items.Count - 1); + }); + + moveToItem(0); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + moveToDeleteButton(0); + AddStep("click delete button", () => InputManager.Click(MouseButton.Left)); + + AddAssert("selected item is null", () => playlist.SelectedItem.Value == null); + } + + // Todo: currently not possible due to bindable list shortcomings (https://github.com/ppy/osu-framework/issues/3081) + // [Test] + public void TestNextItemSelectedAfterExternalDeletion() + { + createPlaylist(true, true); + + moveToItem(0); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("remove item 0", () => playlist.Items.RemoveAt(0)); + + AddAssert("item 0 is selected", () => playlist.SelectedItem.Value == playlist.Items[0]); + } + + private void moveToItem(int index, Vector2? offset = null) + => AddStep($"move mouse to item {index}", () => InputManager.MoveMouseTo(playlist.ChildrenOfType>().ElementAt(index), offset)); + + private void moveToDragger(int index, Vector2? offset = null) => AddStep($"move mouse to dragger {index}", () => + { + var item = playlist.ChildrenOfType>().ElementAt(index); + InputManager.MoveMouseTo(item.ChildrenOfType.PlaylistItemHandle>().Single(), offset); + }); + + private void moveToDeleteButton(int index, Vector2? offset = null) => AddStep($"move mouse to delete button {index}", () => + { + var item = playlist.ChildrenOfType>().ElementAt(index); + InputManager.MoveMouseTo(item.ChildrenOfType().ElementAt(0), offset); + }); + + private void assertHandleVisibility(int index, bool visible) + => AddAssert($"handle {index} {(visible ? "is" : "is not")} visible", + () => (playlist.ChildrenOfType.PlaylistItemHandle>().ElementAt(index).Alpha > 0) == visible); + + private void assertDeleteButtonVisibility(int index, bool visible) + => AddAssert($"delete button {index} {(visible ? "is" : "is not")} visible", () => (playlist.ChildrenOfType().ElementAt(2 + index * 2).Alpha > 0) == visible); + private void createPlaylist(bool allowEdit, bool allowSelection) => AddStep("create playlist", () => { Child = playlist = new DrawableRoomPlaylist(allowEdit, allowSelection) From aceba8791c2e3bbbb1a541351257900ae3a56fa0 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 16:55:05 +0900 Subject: [PATCH 09/12] Clean up selection handling --- .../TestSceneDrawableRoomPlaylist.cs | 8 ++- .../Screens/Multi/DrawableRoomPlaylist.cs | 12 +--- .../Screens/Multi/DrawableRoomPlaylistItem.cs | 63 +++++++++---------- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index 267496bba2..c93d59acdf 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -46,6 +46,9 @@ namespace osu.Game.Tests.Visual.Multiplayer moveToItem(0); assertHandleVisibility(0, false); assertDeleteButtonVisibility(0, false); + + AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddAssert("no item selected", () => playlist.SelectedItem.Value == null); } [Test] @@ -56,6 +59,9 @@ namespace osu.Game.Tests.Visual.Multiplayer moveToItem(0); assertHandleVisibility(0, true); assertDeleteButtonVisibility(0, true); + + AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddAssert("no item selected", () => playlist.SelectedItem.Value == null); } [Test] @@ -165,7 +171,7 @@ namespace osu.Game.Tests.Visual.Multiplayer moveToDeleteButton(0); AddStep("click delete button", () => InputManager.Click(MouseButton.Left)); - AddAssert("selected item is null", () => playlist.SelectedItem.Value == null); + AddAssert("no item selected", () => playlist.SelectedItem.Value == null); } // Todo: currently not possible due to bindable list shortcomings (https://github.com/ppy/osu-framework/issues/3081) diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs index 01f173db4e..b139c61166 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylist.cs @@ -28,15 +28,7 @@ namespace osu.Game.Screens.Multi { base.LoadComplete(); - SelectedItem.BindValueChanged(item => - { - if (item.OldValue != null && ItemMap.TryGetValue(item.OldValue, out var oldItem)) - ((DrawableRoomPlaylistItem)oldItem).Deselect(); - - if (item.NewValue != null && ItemMap.TryGetValue(item.NewValue, out var newItem)) - ((DrawableRoomPlaylistItem)newItem).Select(); - }, true); - + // Scheduled since items are removed and re-added upon rearrangement Items.ItemsRemoved += items => Schedule(() => { if (!Items.Contains(SelectedItem.Value)) @@ -58,7 +50,7 @@ namespace osu.Game.Screens.Multi protected override OsuRearrangeableListItem CreateOsuDrawable(PlaylistItem item) => new DrawableRoomPlaylistItem(item, allowEdit, allowSelection) { - RequestSelection = requestSelection, + SelectedItem = { BindTarget = SelectedItem }, RequestDeletion = requestDeletion }; diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs index 7156839dfc..b1e69e4c4f 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs @@ -31,9 +31,10 @@ namespace osu.Game.Screens.Multi { public class DrawableRoomPlaylistItem : OsuRearrangeableListItem { - public Action RequestSelection; public Action RequestDeletion; + public readonly Bindable SelectedItem = new Bindable(); + protected override bool ShowDragHandle => allowEdit; private Container maskingContainer; @@ -75,6 +76,8 @@ namespace osu.Game.Screens.Multi { base.LoadComplete(); + SelectedItem.BindValueChanged(selected => maskingContainer.BorderThickness = selected.NewValue == Model ? 5 : 0); + beatmap.BindValueChanged(_ => scheduleRefresh()); ruleset.BindValueChanged(_ => scheduleRefresh()); @@ -84,6 +87,32 @@ namespace osu.Game.Screens.Multi refresh(); } + private ScheduledDelegate scheduledRefresh; + + private void scheduleRefresh() + { + scheduledRefresh?.Cancel(); + scheduledRefresh = Schedule(refresh); + } + + private void refresh() + { + difficultyIconContainer.Child = new DifficultyIcon(beatmap.Value, ruleset.Value) { Size = new Vector2(32) }; + + beatmapText.Clear(); + beatmapText.AddLink(item.Beatmap.ToString(), LinkAction.OpenBeatmap, item.Beatmap.Value.OnlineBeatmapID.ToString()); + + authorText.Clear(); + + if (item.Beatmap?.Value?.Metadata?.Author != null) + { + authorText.AddText("mapped by "); + authorText.AddUserLink(item.Beatmap.Value?.Metadata.Author); + } + + modDisplay.Current.Value = requiredMods.ToArray(); + } + protected override Drawable CreateContent() => maskingContainer = new Container { RelativeSizeAxes = Axes.X, @@ -173,43 +202,13 @@ namespace osu.Game.Screens.Multi } }; - private ScheduledDelegate scheduledRefresh; - - private void scheduleRefresh() - { - scheduledRefresh?.Cancel(); - scheduledRefresh = Schedule(refresh); - } - - private void refresh() - { - difficultyIconContainer.Child = new DifficultyIcon(beatmap.Value, ruleset.Value) { Size = new Vector2(32) }; - - beatmapText.Clear(); - beatmapText.AddLink(item.Beatmap.ToString(), LinkAction.OpenBeatmap, item.Beatmap.Value.OnlineBeatmapID.ToString()); - - authorText.Clear(); - - if (item.Beatmap?.Value?.Metadata?.Author != null) - { - authorText.AddText("mapped by "); - authorText.AddUserLink(item.Beatmap.Value?.Metadata.Author); - } - - modDisplay.Current.Value = requiredMods.ToArray(); - } - protected override bool OnClick(ClickEvent e) { if (allowSelection) - RequestSelection?.Invoke(Model); + SelectedItem.Value = Model; return true; } - public void Select() => maskingContainer.BorderThickness = 5; - - public void Deselect() => maskingContainer.BorderThickness = 0; - // For now, this is the same implementation as in PanelBackground, but supports a beatmap info rather than a working beatmap private class PanelBackground : Container // todo: should be a buffered container (https://github.com/ppy/osu-framework/issues/3222) { From 32fd713a9d4c4d40a5066e155efc463c207e3701 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 17:57:25 +0900 Subject: [PATCH 10/12] Use test beatmap instead of beatmap manager --- .../Multiplayer/TestSceneDrawableRoomPlaylist.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index c93d59acdf..4ba802730f 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -9,14 +9,15 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; -using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; using osu.Game.Screens.Multi; +using osu.Game.Tests.Beatmaps; using osuTK; using osuTK.Input; @@ -218,22 +219,13 @@ namespace osu.Game.Tests.Visual.Multiplayer Size = new Vector2(500, 300) }; - var beatmapSets = beatmapManager.GetAllUsableBeatmapSets(); - var rulesets = rulesetStore.AvailableRulesets.ToList(); - for (int i = 0; i < 20; i++) { - var set = beatmapSets[RNG.Next(0, beatmapSets.Count)]; - var beatmap = set.Beatmaps[RNG.Next(0, set.Beatmaps.Count)]; - - beatmap.BeatmapSet = set; - beatmap.Metadata = set.Metadata; - playlist.Items.Add(new PlaylistItem { ID = i, - Beatmap = { Value = beatmap }, - Ruleset = { Value = rulesets[RNG.Next(0, rulesets.Count)] }, + Beatmap = { Value = new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, + Ruleset = { Value = new OsuRuleset().RulesetInfo }, RequiredMods = { new OsuModHardRock(), From fbc5ffbadbd27e5856b8c94c6d046a3513b5df16 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 17:59:47 +0900 Subject: [PATCH 11/12] Remove now unused members --- .../Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index 4ba802730f..d2d02412cd 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -5,15 +5,12 @@ using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; -using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; -using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; using osu.Game.Screens.Multi; @@ -31,12 +28,6 @@ namespace osu.Game.Tests.Visual.Multiplayer typeof(DrawableRoomPlaylistItem) }; - [Resolved] - private BeatmapManager beatmapManager { get; set; } - - [Resolved] - private RulesetStore rulesetStore { get; set; } - private DrawableRoomPlaylist playlist; [Test] From 9eaf37258794cdec6c0bc6c9dedad6713551d54e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 14 Feb 2020 21:17:07 +0900 Subject: [PATCH 12/12] Immediately update selected state --- osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs index b1e69e4c4f..1c35d8a53f 100644 --- a/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/Multi/DrawableRoomPlaylistItem.cs @@ -76,7 +76,7 @@ namespace osu.Game.Screens.Multi { base.LoadComplete(); - SelectedItem.BindValueChanged(selected => maskingContainer.BorderThickness = selected.NewValue == Model ? 5 : 0); + SelectedItem.BindValueChanged(selected => maskingContainer.BorderThickness = selected.NewValue == Model ? 5 : 0, true); beatmap.BindValueChanged(_ => scheduleRefresh()); ruleset.BindValueChanged(_ => scheduleRefresh());