From c6e9a6cd5a3d990b2c72fab3f82148cd65dfb6fe Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 15 Jan 2021 14:28:49 +0900 Subject: [PATCH 01/49] Make most common BPM more accurate --- osu.Game/Beatmaps/Beatmap.cs | 38 +++++++++++++++++++ osu.Game/Beatmaps/BeatmapManager.cs | 2 +- .../ControlPoints/ControlPointInfo.cs | 7 ---- osu.Game/Beatmaps/IBeatmap.cs | 5 +++ osu.Game/Screens/Edit/EditorBeatmap.cs | 2 + osu.Game/Screens/Play/GameplayBeatmap.cs | 2 + osu.Game/Screens/Select/BeatmapInfoWedge.cs | 2 +- 7 files changed, 49 insertions(+), 9 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index be2006e67a..410fd5e92e 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -48,6 +48,44 @@ namespace osu.Game.Beatmaps public virtual IEnumerable GetStatistics() => Enumerable.Empty(); + public double GetMostCommonBeatLength() + { + // The last playable time in the beatmap - the last timing point extends to this time. + // Note: This is more accurate and may present different results because osu-stable didn't have the ability to calculate slider durations in this context. + double lastTime = HitObjects.LastOrDefault()?.GetEndTime() ?? ControlPointInfo.TimingPoints.LastOrDefault()?.Time ?? 0; + + var beatLengthsAndDurations = + // Construct a set of (beatLength, duration) tuples for each individual timing point. + ControlPointInfo.TimingPoints.Select((t, i) => + { + if (t.Time > lastTime) + return (beatLength: t.BeatLength, 0); + + var nextTime = i == ControlPointInfo.TimingPoints.Count - 1 ? lastTime : ControlPointInfo.TimingPoints[i + 1].Time; + return (beatLength: t.BeatLength, duration: nextTime - t.Time); + }) + // Aggregate durations into a set of (beatLength, duration) tuples for each beat length + .GroupBy(t => t.beatLength) + .Select(g => (beatLength: g.Key, duration: g.Sum(t => t.duration))) + // And if there are no timing points, use a default. + .DefaultIfEmpty((TimingControlPoint.DEFAULT_BEAT_LENGTH, 0)); + + // Find the single beat length with the maximum aggregate duration. + double maxDurationBeatLength = double.NegativeInfinity; + double maxDuration = double.NegativeInfinity; + + foreach (var (beatLength, duration) in beatLengthsAndDurations) + { + if (duration > maxDuration) + { + maxDuration = duration; + maxDurationBeatLength = beatLength; + } + } + + return 60000 / maxDurationBeatLength; + } + IBeatmap IBeatmap.Clone() => Clone(); public Beatmap Clone() diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 42418e532b..0d4cc38ac3 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -451,7 +451,7 @@ namespace osu.Game.Beatmaps // TODO: this should be done in a better place once we actually need to dynamically update it. beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate().StarRating ?? 0; beatmap.BeatmapInfo.Length = calculateLength(beatmap); - beatmap.BeatmapInfo.BPM = beatmap.ControlPointInfo.BPMMode; + beatmap.BeatmapInfo.BPM = beatmap.GetMostCommonBeatLength(); beatmapInfos.Add(beatmap.BeatmapInfo); } diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index e8a91e4001..5cc60a5758 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -101,13 +101,6 @@ namespace osu.Game.Beatmaps.ControlPoints public double BPMMinimum => 60000 / (TimingPoints.OrderByDescending(c => c.BeatLength).FirstOrDefault() ?? TimingControlPoint.DEFAULT).BeatLength; - /// - /// Finds the mode BPM (most common BPM) represented by the control points. - /// - [JsonIgnore] - public double BPMMode => - 60000 / (TimingPoints.GroupBy(c => c.BeatLength).OrderByDescending(grp => grp.Count()).FirstOrDefault()?.FirstOrDefault() ?? TimingControlPoint.DEFAULT).BeatLength; - /// /// Remove all s and return to a pristine state. /// diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 8f27e0b0e9..aaca8f7658 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -47,6 +47,11 @@ namespace osu.Game.Beatmaps /// IEnumerable GetStatistics(); + /// + /// Finds the most common beat length represented by the control points in this beatmap. + /// + double GetMostCommonBeatLength(); + /// /// Creates a shallow-clone of this beatmap and returns it. /// diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 165d2ba278..0e9008ba68 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -84,6 +84,8 @@ namespace osu.Game.Screens.Edit public IEnumerable GetStatistics() => PlayableBeatmap.GetStatistics(); + public double GetMostCommonBeatLength() => PlayableBeatmap.GetMostCommonBeatLength(); + public IBeatmap Clone() => (EditorBeatmap)MemberwiseClone(); private IList mutableHitObjects => (IList)PlayableBeatmap.HitObjects; diff --git a/osu.Game/Screens/Play/GameplayBeatmap.cs b/osu.Game/Screens/Play/GameplayBeatmap.cs index 64894544f4..53c1360bfa 100644 --- a/osu.Game/Screens/Play/GameplayBeatmap.cs +++ b/osu.Game/Screens/Play/GameplayBeatmap.cs @@ -39,6 +39,8 @@ namespace osu.Game.Screens.Play public IEnumerable GetStatistics() => PlayableBeatmap.GetStatistics(); + public double GetMostCommonBeatLength() => PlayableBeatmap.GetMostCommonBeatLength(); + public IBeatmap Clone() => PlayableBeatmap.Clone(); private readonly Bindable lastJudgementResult = new Bindable(); diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index 04c1f6efe4..abcd697d85 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -391,7 +391,7 @@ namespace osu.Game.Screens.Select if (Precision.AlmostEquals(bpmMin, bpmMax)) return $"{bpmMin:0}"; - return $"{bpmMin:0}-{bpmMax:0} (mostly {beatmap.ControlPointInfo.BPMMode:0})"; + return $"{bpmMin:0}-{bpmMax:0} (mostly {beatmap.GetMostCommonBeatLength():0})"; } private OsuSpriteText[] getMapper(BeatmapMetadata metadata) From 24e991a5ef9c1797e4bebddf1d0a2f623845a40f Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 15 Jan 2021 14:32:06 +0900 Subject: [PATCH 02/49] Actually return beat length and not BPM --- osu.Game/Beatmaps/Beatmap.cs | 2 +- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- osu.Game/Screens/Select/BeatmapInfoWedge.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 410fd5e92e..4e2e9eb96d 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -83,7 +83,7 @@ namespace osu.Game.Beatmaps } } - return 60000 / maxDurationBeatLength; + return maxDurationBeatLength; } IBeatmap IBeatmap.Clone() => Clone(); diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 0d4cc38ac3..b934ac556d 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -451,7 +451,7 @@ namespace osu.Game.Beatmaps // TODO: this should be done in a better place once we actually need to dynamically update it. beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate().StarRating ?? 0; beatmap.BeatmapInfo.Length = calculateLength(beatmap); - beatmap.BeatmapInfo.BPM = beatmap.GetMostCommonBeatLength(); + beatmap.BeatmapInfo.BPM = 60000 / beatmap.GetMostCommonBeatLength(); beatmapInfos.Add(beatmap.BeatmapInfo); } diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index abcd697d85..86cb561bc7 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -391,7 +391,7 @@ namespace osu.Game.Screens.Select if (Precision.AlmostEquals(bpmMin, bpmMax)) return $"{bpmMin:0}"; - return $"{bpmMin:0}-{bpmMax:0} (mostly {beatmap.GetMostCommonBeatLength():0})"; + return $"{bpmMin:0}-{bpmMax:0} (mostly {60000 / beatmap.GetMostCommonBeatLength():0})"; } private OsuSpriteText[] getMapper(BeatmapMetadata metadata) From 27ffc9844520036e81a652d9d8d7b45b7b5845be Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 18 Jan 2021 10:48:12 +0300 Subject: [PATCH 03/49] Implement WebOverlay component --- .../Online/TestSceneFullscreenOverlay.cs | 4 +- osu.Game/Overlays/BeatmapListingOverlay.cs | 86 ++++++++----------- osu.Game/Overlays/BeatmapSetOverlay.cs | 74 ++++++---------- osu.Game/Overlays/ChangelogOverlay.cs | 56 +++--------- osu.Game/Overlays/DashboardOverlay.cs | 56 ++---------- osu.Game/Overlays/FullscreenOverlay.cs | 26 +++++- osu.Game/Overlays/NewsOverlay.cs | 62 +++---------- osu.Game/Overlays/RankingsOverlay.cs | 81 +++-------------- osu.Game/Overlays/UserProfileOverlay.cs | 13 ++- osu.Game/Overlays/WebOverlay.cs | 50 +++++++++++ 10 files changed, 193 insertions(+), 315 deletions(-) create mode 100644 osu.Game/Overlays/WebOverlay.cs diff --git a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs index 8f20bcdcc1..f8b059e471 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs @@ -41,7 +41,7 @@ namespace osu.Game.Tests.Visual.Online private class TestFullscreenOverlay : FullscreenOverlay { public TestFullscreenOverlay() - : base(OverlayColourScheme.Pink, null) + : base(OverlayColourScheme.Pink) { Children = new Drawable[] { @@ -52,6 +52,8 @@ namespace osu.Game.Tests.Visual.Online }, }; } + + protected override OverlayHeader CreateHeader() => null; } } } diff --git a/osu.Game/Overlays/BeatmapListingOverlay.cs b/osu.Game/Overlays/BeatmapListingOverlay.cs index 0c9c995dd6..ae1667d403 100644 --- a/osu.Game/Overlays/BeatmapListingOverlay.cs +++ b/osu.Game/Overlays/BeatmapListingOverlay.cs @@ -15,98 +15,82 @@ using osu.Framework.Graphics.Textures; using osu.Framework.Input.Events; using osu.Game.Audio; using osu.Game.Beatmaps; -using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; -using osu.Game.Graphics.UserInterface; using osu.Game.Overlays.BeatmapListing; using osu.Game.Overlays.BeatmapListing.Panels; using osuTK; +using osuTK.Graphics; namespace osu.Game.Overlays { - public class BeatmapListingOverlay : FullscreenOverlay + public class BeatmapListingOverlay : WebOverlay { [Resolved] private PreviewTrackManager previewTrackManager { get; set; } private Drawable currentContent; - private LoadingLayer loadingLayer; private Container panelTarget; private FillFlowContainer foundContent; private NotFoundDrawable notFoundContent; - - private OverlayScrollContainer resultScrollContainer; + private BeatmapListingFilterControl filterControl; public BeatmapListingOverlay() - : base(OverlayColourScheme.Blue, new BeatmapListingHeader()) + : base(OverlayColourScheme.Blue) { } - private BeatmapListingFilterControl filterControl; - [BackgroundDependencyLoader] private void load() { - Children = new Drawable[] + Child = new FillFlowContainer { - new Box + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Children = new Drawable[] { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background6 - }, - resultScrollContainer = new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new ReverseChildIDFillFlowContainer + filterControl = new BeatmapListingFilterControl + { + TypingStarted = onTypingStarted, + SearchStarted = onSearchStarted, + SearchFinished = onSearchFinished, + }, + new Container { AutoSizeAxes = Axes.Y, RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical, Children = new Drawable[] { - Header, - filterControl = new BeatmapListingFilterControl + new Box { - TypingStarted = onTypingStarted, - SearchStarted = onSearchStarted, - SearchFinished = onSearchFinished, + RelativeSizeAxes = Axes.Both, + Colour = ColourProvider.Background4, }, - new Container + panelTarget = new Container { AutoSizeAxes = Axes.Y, RelativeSizeAxes = Axes.X, + Padding = new MarginPadding { Horizontal = 20 }, Children = new Drawable[] { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background4, - }, - panelTarget = new Container - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Padding = new MarginPadding { Horizontal = 20 }, - Children = new Drawable[] - { - foundContent = new FillFlowContainer(), - notFoundContent = new NotFoundDrawable(), - } - } - }, - }, - } + foundContent = new FillFlowContainer(), + notFoundContent = new NotFoundDrawable(), + } + } + }, }, - }, - loadingLayer = new LoadingLayer(true) + } }; } + protected override BeatmapListingHeader CreateHeader() => new BeatmapListingHeader(); + + protected override Color4 GetBackgroundColour() => ColourProvider.Background6; + private void onTypingStarted() { // temporary until the textbox/header is updated to always stay on screen. - resultScrollContainer.ScrollToStart(); + ScrollFlow.ScrollToStart(); } protected override void OnFocus(FocusEvent e) @@ -125,7 +109,7 @@ namespace osu.Game.Overlays previewTrackManager.StopAnyPlaying(this); if (panelTarget.Any()) - loadingLayer.Show(); + Loading.Show(); } private Task panelLoadDelegate; @@ -173,7 +157,7 @@ namespace osu.Game.Overlays private void addContentToPlaceholder(Drawable content) { - loadingLayer.Hide(); + Loading.Hide(); lastFetchDisplayedTime = Time.Current; var lastContent = currentContent; @@ -256,7 +240,7 @@ namespace osu.Game.Overlays bool shouldShowMore = panelLoadDelegate?.IsCompleted != false && Time.Current - lastFetchDisplayedTime > time_between_fetches - && (resultScrollContainer.ScrollableExtent > 0 && resultScrollContainer.IsScrolledToEnd(pagination_scroll_distance)); + && (ScrollFlow.ScrollableExtent > 0 && ScrollFlow.IsScrolledToEnd(pagination_scroll_distance)); if (shouldShowMore) filterControl.FetchNextPage(); diff --git a/osu.Game/Overlays/BeatmapSetOverlay.cs b/osu.Game/Overlays/BeatmapSetOverlay.cs index bbec62a85a..10953415ed 100644 --- a/osu.Game/Overlays/BeatmapSetOverlay.cs +++ b/osu.Game/Overlays/BeatmapSetOverlay.cs @@ -6,7 +6,6 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; @@ -16,10 +15,11 @@ using osu.Game.Overlays.BeatmapSet.Scores; using osu.Game.Overlays.Comments; using osu.Game.Rulesets; using osuTK; +using osuTK.Graphics; namespace osu.Game.Overlays { - public class BeatmapSetOverlay : FullscreenOverlay // we don't provide a standard header for now. + public class BeatmapSetOverlay : WebOverlay // we don't provide a standard header for now. { public const float X_PADDING = 40; public const float Y_PADDING = 25; @@ -36,55 +36,40 @@ namespace osu.Game.Overlays // receive input outside our bounds so we can trigger a close event on ourselves. public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; - private readonly Box background; - public BeatmapSetOverlay() - : base(OverlayColourScheme.Blue, null) + : base(OverlayColourScheme.Blue) { - OverlayScrollContainer scroll; Info info; CommentsSection comments; - Children = new Drawable[] + Child = new FillFlowContainer { - background = new Box + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Spacing = new Vector2(0, 20), + Children = new Drawable[] { - RelativeSizeAxes = Axes.Both - }, - scroll = new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new ReverseChildIDFillFlowContainer + new BeatmapSetLayoutSection { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - Spacing = new Vector2(0, 20), - Children = new[] + Child = new ReverseChildIDFillFlowContainer { - new BeatmapSetLayoutSection + AutoSizeAxes = Axes.Y, + RelativeSizeAxes = Axes.X, + Direction = FillDirection.Vertical, + Children = new Drawable[] { - Child = new ReverseChildIDFillFlowContainer - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - Header = new Header(), - info = new Info() - } - }, - }, - new ScoresContainer - { - Beatmap = { BindTarget = Header.Picker.Beatmap } - }, - comments = new CommentsSection() + Header = new Header(), + info = new Info() + } }, }, - }, + new ScoresContainer + { + Beatmap = { BindTarget = Header.Picker.Beatmap } + }, + comments = new CommentsSection() + } }; Header.BeatmapSet.BindTo(beatmapSet); @@ -94,16 +79,13 @@ namespace osu.Game.Overlays Header.Picker.Beatmap.ValueChanged += b => { info.Beatmap = b.NewValue; - - scroll.ScrollToStart(); + ScrollFlow.ScrollToStart(); }; } - [BackgroundDependencyLoader] - private void load() - { - background.Colour = ColourProvider.Background6; - } + protected override OverlayHeader CreateHeader() => null; + + protected override Color4 GetBackgroundColour() => ColourProvider.Background6; protected override void PopOutComplete() { diff --git a/osu.Game/Overlays/ChangelogOverlay.cs b/osu.Game/Overlays/ChangelogOverlay.cs index c7e9a86fa4..5d99887053 100644 --- a/osu.Game/Overlays/ChangelogOverlay.cs +++ b/osu.Game/Overlays/ChangelogOverlay.cs @@ -11,22 +11,18 @@ using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics.Containers; using osu.Game.Input.Bindings; using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays.Changelog; +using osuTK.Graphics; namespace osu.Game.Overlays { - public class ChangelogOverlay : FullscreenOverlay + public class ChangelogOverlay : WebOverlay { public readonly Bindable Current = new Bindable(); - private Container content; - private SampleChannel sampleBack; private List builds; @@ -34,45 +30,14 @@ namespace osu.Game.Overlays protected List Streams; public ChangelogOverlay() - : base(OverlayColourScheme.Purple, new ChangelogHeader()) + : base(OverlayColourScheme.Purple) { } [BackgroundDependencyLoader] private void load(AudioManager audio) { - Children = new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background4, - }, - new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new ReverseChildIDFillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - Header.With(h => - { - h.ListingSelected = ShowListing; - h.Build.BindTarget = Current; - }), - content = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - } - }, - }, - }, - }; + Header.Build.BindTarget = Current; sampleBack = audio.Samples.Get(@"UI/generic-select-soft"); @@ -85,6 +50,13 @@ namespace osu.Game.Overlays }); } + protected override ChangelogHeader CreateHeader() => new ChangelogHeader + { + ListingSelected = ShowListing, + }; + + protected override Color4 GetBackgroundColour() => ColourProvider.Background4; + public void ShowListing() { Current.Value = null; @@ -198,16 +170,16 @@ namespace osu.Game.Overlays private void loadContent(ChangelogContent newContent) { - content.FadeTo(0.2f, 300, Easing.OutQuint); + Content.FadeTo(0.2f, 300, Easing.OutQuint); loadContentCancellation?.Cancel(); LoadComponentAsync(newContent, c => { - content.FadeIn(300, Easing.OutQuint); + Content.FadeIn(300, Easing.OutQuint); c.BuildSelected = ShowBuild; - content.Child = c; + Child = c; }, (loadContentCancellation = new CancellationTokenSource()).Token); } } diff --git a/osu.Game/Overlays/DashboardOverlay.cs b/osu.Game/Overlays/DashboardOverlay.cs index 03c320debe..3722d36388 100644 --- a/osu.Game/Overlays/DashboardOverlay.cs +++ b/osu.Game/Overlays/DashboardOverlay.cs @@ -7,29 +7,18 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Overlays.Dashboard; using osu.Game.Overlays.Dashboard.Friends; namespace osu.Game.Overlays { - public class DashboardOverlay : FullscreenOverlay + public class DashboardOverlay : WebOverlay { private CancellationTokenSource cancellationToken; - private Container content; - private LoadingLayer loading; - private OverlayScrollContainer scrollFlow; - public DashboardOverlay() - : base(OverlayColourScheme.Purple, new DashboardOverlayHeader - { - Anchor = Anchor.TopCentre, - Origin = Anchor.TopCentre, - Depth = -float.MaxValue - }) + : base(OverlayColourScheme.Purple) { } @@ -40,45 +29,16 @@ namespace osu.Game.Overlays { apiState.BindTo(api.State); apiState.BindValueChanged(onlineStateChanged, true); - - Children = new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background5 - }, - scrollFlow = new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new FillFlowContainer - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - Header, - content = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y - } - } - } - }, - loading = new LoadingLayer(true), - }; } protected override void LoadComplete() { base.LoadComplete(); - Header.Current.BindValueChanged(onTabChanged); } + protected override DashboardOverlayHeader CreateHeader() => new DashboardOverlayHeader(); + private bool displayUpdateRequired = true; protected override void PopIn() @@ -102,21 +62,21 @@ namespace osu.Game.Overlays private void loadDisplay(Drawable display) { - scrollFlow.ScrollToStart(); + ScrollFlow.ScrollToStart(); LoadComponentAsync(display, loaded => { if (API.IsLoggedIn) - loading.Hide(); + Loading.Hide(); - content.Child = loaded; + Child = loaded; }, (cancellationToken = new CancellationTokenSource()).Token); } private void onTabChanged(ValueChangedEvent tab) { cancellationToken?.Cancel(); - loading.Show(); + Loading.Show(); if (!API.IsLoggedIn) { diff --git a/osu.Game/Overlays/FullscreenOverlay.cs b/osu.Game/Overlays/FullscreenOverlay.cs index 6f56d95929..d65213f573 100644 --- a/osu.Game/Overlays/FullscreenOverlay.cs +++ b/osu.Game/Overlays/FullscreenOverlay.cs @@ -6,6 +6,7 @@ using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Effects; +using osu.Framework.Graphics.Shapes; using osu.Game.Graphics.Containers; using osu.Game.Online.API; using osuTK.Graphics; @@ -27,9 +28,13 @@ namespace osu.Game.Overlays [Cached] protected readonly OverlayColourProvider ColourProvider; - protected FullscreenOverlay(OverlayColourScheme colourScheme, T header) + protected override Container Content => content; + + private readonly Container content; + + protected FullscreenOverlay(OverlayColourScheme colourScheme) { - Header = header; + Header = CreateHeader(); ColourProvider = new OverlayColourProvider(colourScheme); @@ -47,6 +52,19 @@ namespace osu.Game.Overlays Type = EdgeEffectType.Shadow, Radius = 10 }; + + base.Content.AddRange(new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = GetBackgroundColour() + }, + content = new Container + { + RelativeSizeAxes = Axes.Both + } + }); } [BackgroundDependencyLoader] @@ -58,6 +76,10 @@ namespace osu.Game.Overlays Waves.FourthWaveColour = ColourProvider.Dark3; } + protected abstract T CreateHeader(); + + protected virtual Color4 GetBackgroundColour() => ColourProvider.Background5; + public override void Show() { if (State.Value == Visibility.Visible) diff --git a/osu.Game/Overlays/NewsOverlay.cs b/osu.Game/Overlays/NewsOverlay.cs index 5820d405d4..268d2bb6a2 100644 --- a/osu.Game/Overlays/NewsOverlay.cs +++ b/osu.Game/Overlays/NewsOverlay.cs @@ -2,67 +2,22 @@ // See the LICENCE file in the repository root for full licence text. using System.Threading; -using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics.UserInterface; using osu.Game.Overlays.News; using osu.Game.Overlays.News.Displays; namespace osu.Game.Overlays { - public class NewsOverlay : FullscreenOverlay + public class NewsOverlay : WebOverlay { private readonly Bindable article = new Bindable(null); - private Container content; - private LoadingLayer loading; - private OverlayScrollContainer scrollFlow; - public NewsOverlay() - : base(OverlayColourScheme.Purple, new NewsHeader()) + : base(OverlayColourScheme.Purple) { } - [BackgroundDependencyLoader] - private void load() - { - Children = new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background5, - }, - scrollFlow = new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - Header.With(h => - { - h.ShowFrontPage = ShowFrontPage; - }), - content = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - } - }, - }, - }, - loading = new LoadingLayer(true), - }; - } - protected override void LoadComplete() { base.LoadComplete(); @@ -71,6 +26,11 @@ namespace osu.Game.Overlays article.BindValueChanged(onArticleChanged); } + protected override NewsHeader CreateHeader() => new NewsHeader + { + ShowFrontPage = ShowFrontPage + }; + private bool displayUpdateRequired = true; protected override void PopIn() @@ -107,7 +67,7 @@ namespace osu.Game.Overlays private void onArticleChanged(ValueChangedEvent e) { cancellationToken?.Cancel(); - loading.Show(); + Loading.Show(); if (e.NewValue == null) { @@ -122,11 +82,11 @@ namespace osu.Game.Overlays protected void LoadDisplay(Drawable display) { - scrollFlow.ScrollToStart(); + ScrollFlow.ScrollToStart(); LoadComponentAsync(display, loaded => { - content.Child = loaded; - loading.Hide(); + Child = loaded; + Loading.Hide(); }, (cancellationToken = new CancellationTokenSource()).Token); } diff --git a/osu.Game/Overlays/RankingsOverlay.cs b/osu.Game/Overlays/RankingsOverlay.cs index 25350e310a..89853e9044 100644 --- a/osu.Game/Overlays/RankingsOverlay.cs +++ b/osu.Game/Overlays/RankingsOverlay.cs @@ -4,96 +4,41 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; using osu.Game.Overlays.Rankings; using osu.Game.Users; using osu.Game.Rulesets; using osu.Game.Online.API; using System.Threading; -using osu.Game.Graphics.UserInterface; using osu.Game.Online.API.Requests; using osu.Game.Overlays.Rankings.Tables; namespace osu.Game.Overlays { - public class RankingsOverlay : FullscreenOverlay + public class RankingsOverlay : WebOverlay { protected Bindable Country => Header.Country; protected Bindable Scope => Header.Current; - private readonly OverlayScrollContainer scrollFlow; - private readonly Container contentContainer; - private readonly LoadingLayer loading; - private readonly Box background; - private APIRequest lastRequest; private CancellationTokenSource cancellationToken; [Resolved] private IAPIProvider api { get; set; } - public RankingsOverlay() - : base(OverlayColourScheme.Green, new RankingsOverlayHeader - { - Anchor = Anchor.TopCentre, - Origin = Anchor.TopCentre, - Depth = -float.MaxValue - }) - { - loading = new LoadingLayer(true); + [Resolved] + private Bindable ruleset { get; set; } - Children = new Drawable[] - { - background = new Box - { - RelativeSizeAxes = Axes.Both - }, - scrollFlow = new OverlayScrollContainer - { - RelativeSizeAxes = Axes.Both, - ScrollbarVisible = false, - Child = new FillFlowContainer - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - Header, - new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Children = new Drawable[] - { - contentContainer = new Container - { - Anchor = Anchor.TopCentre, - Origin = Anchor.TopCentre, - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Margin = new MarginPadding { Bottom = 10 } - }, - } - } - } - } - }, - loading - }; + public RankingsOverlay() + : base(OverlayColourScheme.Green) + { } [BackgroundDependencyLoader] private void load() { - background.Colour = ColourProvider.Background5; } - [Resolved] - private Bindable ruleset { get; set; } - protected override void LoadComplete() { base.LoadComplete(); @@ -129,6 +74,8 @@ namespace osu.Game.Overlays Scheduler.AddOnce(loadNewContent); } + protected override RankingsOverlayHeader CreateHeader() => new RankingsOverlayHeader(); + public void ShowCountry(Country requested) { if (requested == null) @@ -147,7 +94,7 @@ namespace osu.Game.Overlays private void loadNewContent() { - loading.Show(); + Loading.Show(); cancellationToken?.Cancel(); lastRequest?.Cancel(); @@ -218,19 +165,19 @@ namespace osu.Game.Overlays private void loadContent(Drawable content) { - scrollFlow.ScrollToStart(); + ScrollFlow.ScrollToStart(); if (content == null) { - contentContainer.Clear(); - loading.Hide(); + Clear(); + Loading.Hide(); return; } LoadComponentAsync(content, loaded => { - loading.Hide(); - contentContainer.Child = loaded; + Loading.Hide(); + Child = loaded; }, (cancellationToken = new CancellationTokenSource()).Token); } diff --git a/osu.Game/Overlays/UserProfileOverlay.cs b/osu.Game/Overlays/UserProfileOverlay.cs index 81027667fa..c29df72501 100644 --- a/osu.Game/Overlays/UserProfileOverlay.cs +++ b/osu.Game/Overlays/UserProfileOverlay.cs @@ -15,6 +15,7 @@ using osu.Game.Overlays.Profile; using osu.Game.Overlays.Profile.Sections; using osu.Game.Users; using osuTK; +using osuTK.Graphics; namespace osu.Game.Overlays { @@ -29,10 +30,14 @@ namespace osu.Game.Overlays public const float CONTENT_X_MARGIN = 70; public UserProfileOverlay() - : base(OverlayColourScheme.Pink, new ProfileHeader()) + : base(OverlayColourScheme.Pink) { } + protected override ProfileHeader CreateHeader() => new ProfileHeader(); + + protected override Color4 GetBackgroundColour() => ColourProvider.Background6; + public void ShowUser(int userId) => ShowUser(new User { Id = userId }); public void ShowUser(User user, bool fetchOnline = true) @@ -72,12 +77,6 @@ namespace osu.Game.Overlays Origin = Anchor.TopCentre, }; - Add(new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourProvider.Background6 - }); - Add(sectionsContainer = new ProfileSectionsContainer { ExpandableHeader = Header, diff --git a/osu.Game/Overlays/WebOverlay.cs b/osu.Game/Overlays/WebOverlay.cs new file mode 100644 index 0000000000..aca767e32f --- /dev/null +++ b/osu.Game/Overlays/WebOverlay.cs @@ -0,0 +1,50 @@ +// 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.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Graphics.UserInterface; + +namespace osu.Game.Overlays +{ + public abstract class WebOverlay : FullscreenOverlay + where T : OverlayHeader + { + protected override Container Content => content; + + protected readonly OverlayScrollContainer ScrollFlow; + protected readonly LoadingLayer Loading; + private readonly Container content; + + protected WebOverlay(OverlayColourScheme colourScheme) + : base(colourScheme) + { + FillFlowContainer flow = new FillFlowContainer + { + AutoSizeAxes = Axes.Y, + RelativeSizeAxes = Axes.X, + Direction = FillDirection.Vertical + }; + + if (Header != null) + flow.Add(Header.With(h => h.Depth = -float.MaxValue)); + + flow.Add(content = new Container + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y + }); + + base.Content.AddRange(new Drawable[] + { + ScrollFlow = new OverlayScrollContainer + { + RelativeSizeAxes = Axes.Both, + ScrollbarVisible = false, + Child = flow + }, + Loading = new LoadingLayer(true) + }); + } + } +} From 6e34ab5d152f37d19702ff305c2a53323e89ef75 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 18 Jan 2021 11:13:38 +0300 Subject: [PATCH 04/49] Rename WebOverlay to OnlineOverlay --- osu.Game/Overlays/BeatmapListingOverlay.cs | 2 +- osu.Game/Overlays/BeatmapSetOverlay.cs | 2 +- osu.Game/Overlays/ChangelogOverlay.cs | 2 +- osu.Game/Overlays/DashboardOverlay.cs | 2 +- osu.Game/Overlays/NewsOverlay.cs | 2 +- osu.Game/Overlays/{WebOverlay.cs => OnlineOverlay.cs} | 4 ++-- osu.Game/Overlays/RankingsOverlay.cs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) rename osu.Game/Overlays/{WebOverlay.cs => OnlineOverlay.cs} (91%) diff --git a/osu.Game/Overlays/BeatmapListingOverlay.cs b/osu.Game/Overlays/BeatmapListingOverlay.cs index ae1667d403..eafb7e95d5 100644 --- a/osu.Game/Overlays/BeatmapListingOverlay.cs +++ b/osu.Game/Overlays/BeatmapListingOverlay.cs @@ -23,7 +23,7 @@ using osuTK.Graphics; namespace osu.Game.Overlays { - public class BeatmapListingOverlay : WebOverlay + public class BeatmapListingOverlay : OnlineOverlay { [Resolved] private PreviewTrackManager previewTrackManager { get; set; } diff --git a/osu.Game/Overlays/BeatmapSetOverlay.cs b/osu.Game/Overlays/BeatmapSetOverlay.cs index 10953415ed..872621801a 100644 --- a/osu.Game/Overlays/BeatmapSetOverlay.cs +++ b/osu.Game/Overlays/BeatmapSetOverlay.cs @@ -19,7 +19,7 @@ using osuTK.Graphics; namespace osu.Game.Overlays { - public class BeatmapSetOverlay : WebOverlay // we don't provide a standard header for now. + public class BeatmapSetOverlay : OnlineOverlay // we don't provide a standard header for now. { public const float X_PADDING = 40; public const float Y_PADDING = 25; diff --git a/osu.Game/Overlays/ChangelogOverlay.cs b/osu.Game/Overlays/ChangelogOverlay.cs index 5d99887053..5200b567ff 100644 --- a/osu.Game/Overlays/ChangelogOverlay.cs +++ b/osu.Game/Overlays/ChangelogOverlay.cs @@ -19,7 +19,7 @@ using osuTK.Graphics; namespace osu.Game.Overlays { - public class ChangelogOverlay : WebOverlay + public class ChangelogOverlay : OnlineOverlay { public readonly Bindable Current = new Bindable(); diff --git a/osu.Game/Overlays/DashboardOverlay.cs b/osu.Game/Overlays/DashboardOverlay.cs index 3722d36388..39a23fe3d4 100644 --- a/osu.Game/Overlays/DashboardOverlay.cs +++ b/osu.Game/Overlays/DashboardOverlay.cs @@ -13,7 +13,7 @@ using osu.Game.Overlays.Dashboard.Friends; namespace osu.Game.Overlays { - public class DashboardOverlay : WebOverlay + public class DashboardOverlay : OnlineOverlay { private CancellationTokenSource cancellationToken; diff --git a/osu.Game/Overlays/NewsOverlay.cs b/osu.Game/Overlays/NewsOverlay.cs index 268d2bb6a2..08e8331dd3 100644 --- a/osu.Game/Overlays/NewsOverlay.cs +++ b/osu.Game/Overlays/NewsOverlay.cs @@ -9,7 +9,7 @@ using osu.Game.Overlays.News.Displays; namespace osu.Game.Overlays { - public class NewsOverlay : WebOverlay + public class NewsOverlay : OnlineOverlay { private readonly Bindable article = new Bindable(null); diff --git a/osu.Game/Overlays/WebOverlay.cs b/osu.Game/Overlays/OnlineOverlay.cs similarity index 91% rename from osu.Game/Overlays/WebOverlay.cs rename to osu.Game/Overlays/OnlineOverlay.cs index aca767e32f..b44ccc32c9 100644 --- a/osu.Game/Overlays/WebOverlay.cs +++ b/osu.Game/Overlays/OnlineOverlay.cs @@ -7,7 +7,7 @@ using osu.Game.Graphics.UserInterface; namespace osu.Game.Overlays { - public abstract class WebOverlay : FullscreenOverlay + public abstract class OnlineOverlay : FullscreenOverlay where T : OverlayHeader { protected override Container Content => content; @@ -16,7 +16,7 @@ namespace osu.Game.Overlays protected readonly LoadingLayer Loading; private readonly Container content; - protected WebOverlay(OverlayColourScheme colourScheme) + protected OnlineOverlay(OverlayColourScheme colourScheme) : base(colourScheme) { FillFlowContainer flow = new FillFlowContainer diff --git a/osu.Game/Overlays/RankingsOverlay.cs b/osu.Game/Overlays/RankingsOverlay.cs index 89853e9044..f6bbac4407 100644 --- a/osu.Game/Overlays/RankingsOverlay.cs +++ b/osu.Game/Overlays/RankingsOverlay.cs @@ -14,7 +14,7 @@ using osu.Game.Overlays.Rankings.Tables; namespace osu.Game.Overlays { - public class RankingsOverlay : WebOverlay + public class RankingsOverlay : OnlineOverlay { protected Bindable Country => Header.Country; From c6b0f3c247aa54a64d9e61c47643a09ea91ec177 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 18 Jan 2021 21:22:50 +0300 Subject: [PATCH 05/49] Implement TabbableOnlineOverlay component --- osu.Game/Overlays/DashboardOverlay.cs | 92 ++----------------- osu.Game/Overlays/RankingsOverlay.cs | 46 ++-------- osu.Game/Overlays/TabbableOnlineOverlay.cs | 101 +++++++++++++++++++++ 3 files changed, 116 insertions(+), 123 deletions(-) create mode 100644 osu.Game/Overlays/TabbableOnlineOverlay.cs diff --git a/osu.Game/Overlays/DashboardOverlay.cs b/osu.Game/Overlays/DashboardOverlay.cs index 39a23fe3d4..83ad8faf1c 100644 --- a/osu.Game/Overlays/DashboardOverlay.cs +++ b/osu.Game/Overlays/DashboardOverlay.cs @@ -2,115 +2,35 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Threading; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Game.Online.API; using osu.Game.Overlays.Dashboard; using osu.Game.Overlays.Dashboard.Friends; namespace osu.Game.Overlays { - public class DashboardOverlay : OnlineOverlay + public class DashboardOverlay : TabbableOnlineOverlay { - private CancellationTokenSource cancellationToken; - public DashboardOverlay() : base(OverlayColourScheme.Purple) { } - private readonly IBindable apiState = new Bindable(); - - [BackgroundDependencyLoader] - private void load(IAPIProvider api) - { - apiState.BindTo(api.State); - apiState.BindValueChanged(onlineStateChanged, true); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - Header.Current.BindValueChanged(onTabChanged); - } - protected override DashboardOverlayHeader CreateHeader() => new DashboardOverlayHeader(); - private bool displayUpdateRequired = true; - - protected override void PopIn() + protected override void CreateDisplayToLoad(DashboardOverlayTabs tab) { - base.PopIn(); - - // We don't want to create a new display on every call, only when exiting from fully closed state. - if (displayUpdateRequired) - { - Header.Current.TriggerChange(); - displayUpdateRequired = false; - } - } - - protected override void PopOutComplete() - { - base.PopOutComplete(); - loadDisplay(Empty()); - displayUpdateRequired = true; - } - - private void loadDisplay(Drawable display) - { - ScrollFlow.ScrollToStart(); - - LoadComponentAsync(display, loaded => - { - if (API.IsLoggedIn) - Loading.Hide(); - - Child = loaded; - }, (cancellationToken = new CancellationTokenSource()).Token); - } - - private void onTabChanged(ValueChangedEvent tab) - { - cancellationToken?.Cancel(); - Loading.Show(); - - if (!API.IsLoggedIn) - { - loadDisplay(Empty()); - return; - } - - switch (tab.NewValue) + switch (tab) { case DashboardOverlayTabs.Friends: - loadDisplay(new FriendDisplay()); + LoadDisplay(new FriendDisplay()); break; case DashboardOverlayTabs.CurrentlyPlaying: - loadDisplay(new CurrentlyPlayingDisplay()); + LoadDisplay(new CurrentlyPlayingDisplay()); break; default: - throw new NotImplementedException($"Display for {tab.NewValue} tab is not implemented"); + throw new NotImplementedException($"Display for {tab} tab is not implemented"); } } - - private void onlineStateChanged(ValueChangedEvent state) => Schedule(() => - { - if (State.Value == Visibility.Hidden) - return; - - Header.Current.TriggerChange(); - }); - - protected override void Dispose(bool isDisposing) - { - cancellationToken?.Cancel(); - base.Dispose(isDisposing); - } } } diff --git a/osu.Game/Overlays/RankingsOverlay.cs b/osu.Game/Overlays/RankingsOverlay.cs index f6bbac4407..6cd72d6e2c 100644 --- a/osu.Game/Overlays/RankingsOverlay.cs +++ b/osu.Game/Overlays/RankingsOverlay.cs @@ -8,20 +8,18 @@ using osu.Game.Overlays.Rankings; using osu.Game.Users; using osu.Game.Rulesets; using osu.Game.Online.API; -using System.Threading; using osu.Game.Online.API.Requests; using osu.Game.Overlays.Rankings.Tables; namespace osu.Game.Overlays { - public class RankingsOverlay : OnlineOverlay + public class RankingsOverlay : TabbableOnlineOverlay { protected Bindable Country => Header.Country; protected Bindable Scope => Header.Current; private APIRequest lastRequest; - private CancellationTokenSource cancellationToken; [Resolved] private IAPIProvider api { get; set; } @@ -34,11 +32,6 @@ namespace osu.Game.Overlays { } - [BackgroundDependencyLoader] - private void load() - { - } - protected override void LoadComplete() { base.LoadComplete(); @@ -54,6 +47,8 @@ namespace osu.Game.Overlays Scheduler.AddOnce(loadNewContent); }); + // Unbind events from scope so base class event will not be called + Scope.UnbindEvents(); Scope.BindValueChanged(_ => { // country filtering is only valid for performance scope. @@ -70,8 +65,6 @@ namespace osu.Game.Overlays Scheduler.AddOnce(loadNewContent); }); - - Scheduler.AddOnce(loadNewContent); } protected override RankingsOverlayHeader CreateHeader() => new RankingsOverlayHeader(); @@ -92,16 +85,13 @@ namespace osu.Game.Overlays Show(); } - private void loadNewContent() + protected override void CreateDisplayToLoad(RankingsScope tab) { - Loading.Show(); - - cancellationToken?.Cancel(); lastRequest?.Cancel(); if (Scope.Value == RankingsScope.Spotlights) { - loadContent(new SpotlightsLayout + LoadDisplay(new SpotlightsLayout { Ruleset = { BindTarget = ruleset } }); @@ -113,12 +103,12 @@ namespace osu.Game.Overlays if (request == null) { - loadContent(null); + LoadDisplay(Empty()); return; } - request.Success += () => Schedule(() => loadContent(createTableFromResponse(request))); - request.Failure += _ => Schedule(() => loadContent(null)); + request.Success += () => Schedule(() => LoadDisplay(createTableFromResponse(request))); + request.Failure += _ => Schedule(() => LoadDisplay(Empty())); api.Queue(request); } @@ -163,29 +153,11 @@ namespace osu.Game.Overlays return null; } - private void loadContent(Drawable content) - { - ScrollFlow.ScrollToStart(); - - if (content == null) - { - Clear(); - Loading.Hide(); - return; - } - - LoadComponentAsync(content, loaded => - { - Loading.Hide(); - Child = loaded; - }, (cancellationToken = new CancellationTokenSource()).Token); - } + private void loadNewContent() => OnTabChanged(Scope.Value); protected override void Dispose(bool isDisposing) { lastRequest?.Cancel(); - cancellationToken?.Cancel(); - base.Dispose(isDisposing); } } diff --git a/osu.Game/Overlays/TabbableOnlineOverlay.cs b/osu.Game/Overlays/TabbableOnlineOverlay.cs new file mode 100644 index 0000000000..cbcf3cd96e --- /dev/null +++ b/osu.Game/Overlays/TabbableOnlineOverlay.cs @@ -0,0 +1,101 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Threading; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Online.API; + +namespace osu.Game.Overlays +{ + public abstract class TabbableOnlineOverlay : OnlineOverlay + where THeader : TabControlOverlayHeader + { + private readonly IBindable apiState = new Bindable(); + + private CancellationTokenSource cancellationToken; + private bool displayUpdateRequired = true; + + protected TabbableOnlineOverlay(OverlayColourScheme colourScheme) + : base(colourScheme) + { + } + + [BackgroundDependencyLoader] + private void load(IAPIProvider api) + { + apiState.BindTo(api.State); + apiState.BindValueChanged(onlineStateChanged, true); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + Header.Current.BindValueChanged(tab => OnTabChanged(tab.NewValue)); + } + + protected override void PopIn() + { + base.PopIn(); + + // We don't want to create a new display on every call, only when exiting from fully closed state. + if (displayUpdateRequired) + { + Header.Current.TriggerChange(); + displayUpdateRequired = false; + } + } + + protected override void PopOutComplete() + { + base.PopOutComplete(); + LoadDisplay(Empty()); + displayUpdateRequired = true; + } + + protected void LoadDisplay(Drawable display) + { + ScrollFlow.ScrollToStart(); + + LoadComponentAsync(display, loaded => + { + if (API.IsLoggedIn) + Loading.Hide(); + + Child = loaded; + }, (cancellationToken = new CancellationTokenSource()).Token); + } + + protected void OnTabChanged(TEnum tab) + { + cancellationToken?.Cancel(); + Loading.Show(); + + if (!API.IsLoggedIn) + { + LoadDisplay(Empty()); + return; + } + + CreateDisplayToLoad(tab); + } + + protected abstract void CreateDisplayToLoad(TEnum tab); + + private void onlineStateChanged(ValueChangedEvent state) => Schedule(() => + { + if (State.Value == Visibility.Hidden) + return; + + Header.Current.TriggerChange(); + }); + + protected override void Dispose(bool isDisposing) + { + cancellationToken?.Cancel(); + base.Dispose(isDisposing); + } + } +} From 5261c0184919c79973c08a7bc43466adf4f0d3ba Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 20 Jan 2021 19:43:42 +0900 Subject: [PATCH 06/49] Tie JoinRoom() and PartRoom() together --- .../Multiplayer/StatefulMultiplayerClient.cs | 154 +++++++++++------- 1 file changed, 92 insertions(+), 62 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index f0e11b2b8b..18a63171c4 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -4,7 +4,6 @@ #nullable enable using System; -using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Threading.Tasks; @@ -109,30 +108,54 @@ namespace osu.Game.Online.Multiplayer }); } + private readonly object joinOrLeaveTaskLock = new object(); + private Task? joinOrLeaveTask; + /// /// Joins the for a given API . /// /// The API . public async Task JoinRoom(Room room) { - if (Room != null) - throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); + Task? lastTask; + Task newTask; - Debug.Assert(room.RoomID.Value != null); + lock (joinOrLeaveTaskLock) + { + lastTask = joinOrLeaveTask; + joinOrLeaveTask = newTask = Task.Run(async () => + { + if (lastTask != null) + await lastTask; - apiRoom = room; - playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; + // Should be thread-safe since joinOrLeaveTask is locked on in both JoinRoom() and LeaveRoom(). + if (Room != null) + throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); - Room = await JoinRoom(room.RoomID.Value.Value); + Debug.Assert(room.RoomID.Value != null); - Debug.Assert(Room != null); + // Join the server-side room. + var joinedRoom = await JoinRoom(room.RoomID.Value.Value); + Debug.Assert(joinedRoom != null); - var users = await getRoomUsers(); - Debug.Assert(users != null); + // Populate users. + Debug.Assert(joinedRoom.Users != null); + await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)); - await Task.WhenAll(users.Select(PopulateUser)); + // Update the stored room (must be done on update thread for thread-safety). + await scheduleAsync(() => + { + Room = joinedRoom; + apiRoom = room; + playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; + }); - updateLocalRoomSettings(Room.Settings); + // Update room settings. + await updateLocalRoomSettings(joinedRoom.Settings); + }); + } + + await newTask; } /// @@ -142,21 +165,35 @@ namespace osu.Game.Online.Multiplayer /// The joined . protected abstract Task JoinRoom(long roomId); - public virtual Task LeaveRoom() + public virtual async Task LeaveRoom() { - Scheduler.Add(() => + Task? lastTask; + Task newTask; + + lock (joinOrLeaveTaskLock) { - if (Room == null) - return; + lastTask = joinOrLeaveTask; + joinOrLeaveTask = newTask = Task.Run(async () => + { + if (lastTask != null) + await lastTask; - apiRoom = null; - Room = null; - CurrentMatchPlayingUserIds.Clear(); + // Should be thread-safe since joinOrLeaveTask is locked on in both JoinRoom() and LeaveRoom(). + if (Room == null) + return; - RoomUpdated?.Invoke(); - }, false); + await scheduleAsync(() => + { + apiRoom = null; + Room = null; + CurrentMatchPlayingUserIds.Clear(); - return Task.CompletedTask; + RoomUpdated?.Invoke(); + }); + }); + } + + await newTask; } /// @@ -432,27 +469,6 @@ namespace osu.Game.Online.Multiplayer /// The to populate. protected async Task PopulateUser(MultiplayerRoomUser multiplayerUser) => multiplayerUser.User ??= await userLookupCache.GetUserAsync(multiplayerUser.UserID); - /// - /// Retrieve a copy of users currently in the joined in a thread-safe manner. - /// This should be used whenever accessing users from outside of an Update thread context (ie. when not calling ). - /// - /// A copy of users in the current room, or null if unavailable. - private Task?> getRoomUsers() - { - var tcs = new TaskCompletionSource?>(); - - // at some point we probably want to replace all these schedule calls with Room.LockForUpdate. - // for now, as this would require quite some consideration due to the number of accesses to the room instance, - // let's just add a manual schedule for the non-scheduled usages instead. - Scheduler.Add(() => - { - var users = Room?.Users.ToList(); - tcs.SetResult(users); - }, false); - - return tcs.Task; - } - /// /// Updates the local room settings with the given . /// @@ -460,34 +476,28 @@ namespace osu.Game.Online.Multiplayer /// This updates both the joined and the respective API . /// /// The new to update from. - private void updateLocalRoomSettings(MultiplayerRoomSettings settings) + private Task updateLocalRoomSettings(MultiplayerRoomSettings settings) => scheduleAsync(() => { if (Room == null) return; - Scheduler.Add(() => - { - if (Room == null) - return; + Debug.Assert(apiRoom != null); - Debug.Assert(apiRoom != null); + // Update a few properties of the room instantaneously. + Room.Settings = settings; + apiRoom.Name.Value = Room.Settings.Name; - // Update a few properties of the room instantaneously. - Room.Settings = settings; - apiRoom.Name.Value = Room.Settings.Name; + // The playlist update is delayed until an online beatmap lookup (below) succeeds. + // In-order for the client to not display an outdated beatmap, the playlist is forcefully cleared here. + apiRoom.Playlist.Clear(); - // The playlist update is delayed until an online beatmap lookup (below) succeeds. - // In-order for the client to not display an outdated beatmap, the playlist is forcefully cleared here. - apiRoom.Playlist.Clear(); + RoomUpdated?.Invoke(); - RoomUpdated?.Invoke(); + var req = new GetBeatmapSetRequest(settings.BeatmapID, BeatmapSetLookupType.BeatmapId); + req.Success += res => updatePlaylist(settings, res); - var req = new GetBeatmapSetRequest(settings.BeatmapID, BeatmapSetLookupType.BeatmapId); - req.Success += res => updatePlaylist(settings, res); - - api.Queue(req); - }, false); - } + api.Queue(req); + }); private void updatePlaylist(MultiplayerRoomSettings settings, APIBeatmapSet onlineSet) { @@ -534,5 +544,25 @@ namespace osu.Game.Online.Multiplayer else CurrentMatchPlayingUserIds.Remove(userId); } + + private Task scheduleAsync(Action action) + { + var tcs = new TaskCompletionSource(); + + Scheduler.Add(() => + { + try + { + action(); + tcs.SetResult(true); + } + catch (Exception ex) + { + tcs.SetException(ex); + } + }); + + return tcs.Task; + } } } From 5ff76be052a73b95e4aa2b562dfb88055939018b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 20 Jan 2021 19:43:51 +0900 Subject: [PATCH 07/49] Fix potential test failures due to timing --- .../Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs index 7a3845cbf3..6de5704410 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs @@ -101,7 +101,7 @@ namespace osu.Game.Tests.Visual.Multiplayer }); }); - AddAssert("multiplayer room joined", () => roomContainer.Client.Room != null); + AddUntilStep("multiplayer room joined", () => roomContainer.Client.Room != null); } [Test] @@ -133,7 +133,7 @@ namespace osu.Game.Tests.Visual.Multiplayer }); }); - AddAssert("multiplayer room joined", () => roomContainer.Client.Room != null); + AddUntilStep("multiplayer room joined", () => roomContainer.Client.Room != null); } private TestMultiplayerRoomManager createRoomManager() From e005a1cc9fc72413fe5a6c4592eb867cde860c14 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 20 Jan 2021 20:16:34 +0900 Subject: [PATCH 08/49] Remove unnecessary condition blocking the part --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 50dc8f661c..5d18521eac 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -143,9 +143,6 @@ namespace osu.Game.Online.Multiplayer return; } - if (Room == null) - return; - await base.LeaveRoom(); await connection.InvokeAsync(nameof(IMultiplayerServer.LeaveRoom)); } From 6b139d4cf312cb75970a1555dab546d65f8ba898 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 20 Jan 2021 20:21:07 +0900 Subject: [PATCH 09/49] Reset task post-execution --- .../Multiplayer/StatefulMultiplayerClient.cs | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 18a63171c4..80162eae59 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -155,7 +155,19 @@ namespace osu.Game.Online.Multiplayer }); } - await newTask; + try + { + await newTask; + } + finally + { + // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. + lock (joinOrLeaveTask) + { + if (joinOrLeaveTask == newTask) + joinOrLeaveTask = null; + } + } } /// @@ -193,7 +205,19 @@ namespace osu.Game.Online.Multiplayer }); } - await newTask; + try + { + await newTask; + } + finally + { + // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. + lock (joinOrLeaveTask) + { + if (joinOrLeaveTask == newTask) + joinOrLeaveTask = null; + } + } } /// From 76e1f6e57bbd27297a320b64cbb27dcbdad6c85a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 21 Jan 2021 12:45:44 +0900 Subject: [PATCH 10/49] Fix locking on incorrect object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 80162eae59..f2b5a44fcf 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -162,7 +162,7 @@ namespace osu.Game.Online.Multiplayer finally { // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. - lock (joinOrLeaveTask) + lock (joinOrLeaveTaskLock) { if (joinOrLeaveTask == newTask) joinOrLeaveTask = null; @@ -212,7 +212,7 @@ namespace osu.Game.Online.Multiplayer finally { // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. - lock (joinOrLeaveTask) + lock (joinOrLeaveTaskLock) { if (joinOrLeaveTask == newTask) joinOrLeaveTask = null; From 65ece1aa722b74e6590b4b3d208a10141238164d Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Thu, 21 Jan 2021 07:50:41 +0300 Subject: [PATCH 11/49] Mark OverlayHeader as NotNull in FullscreenOverlay --- osu.Game/Overlays/FullscreenOverlay.cs | 8 ++++--- osu.Game/Overlays/OnlineOverlay.cs | 32 ++++++++++++-------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/osu.Game/Overlays/FullscreenOverlay.cs b/osu.Game/Overlays/FullscreenOverlay.cs index d65213f573..d0a0c994aa 100644 --- a/osu.Game/Overlays/FullscreenOverlay.cs +++ b/osu.Game/Overlays/FullscreenOverlay.cs @@ -1,6 +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 JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; @@ -16,9 +17,9 @@ namespace osu.Game.Overlays public abstract class FullscreenOverlay : WaveOverlayContainer, INamedOverlayComponent where T : OverlayHeader { - public virtual string IconTexture => Header?.Title.IconTexture ?? string.Empty; - public virtual string Title => Header?.Title.Title ?? string.Empty; - public virtual string Description => Header?.Title.Description ?? string.Empty; + public virtual string IconTexture => Header.Title.IconTexture ?? string.Empty; + public virtual string Title => Header.Title.Title ?? string.Empty; + public virtual string Description => Header.Title.Description ?? string.Empty; public T Header { get; } @@ -76,6 +77,7 @@ namespace osu.Game.Overlays Waves.FourthWaveColour = ColourProvider.Dark3; } + [NotNull] protected abstract T CreateHeader(); protected virtual Color4 GetBackgroundColour() => ColourProvider.Background5; diff --git a/osu.Game/Overlays/OnlineOverlay.cs b/osu.Game/Overlays/OnlineOverlay.cs index b44ccc32c9..4a7318d065 100644 --- a/osu.Game/Overlays/OnlineOverlay.cs +++ b/osu.Game/Overlays/OnlineOverlay.cs @@ -19,29 +19,27 @@ namespace osu.Game.Overlays protected OnlineOverlay(OverlayColourScheme colourScheme) : base(colourScheme) { - FillFlowContainer flow = new FillFlowContainer - { - AutoSizeAxes = Axes.Y, - RelativeSizeAxes = Axes.X, - Direction = FillDirection.Vertical - }; - - if (Header != null) - flow.Add(Header.With(h => h.Depth = -float.MaxValue)); - - flow.Add(content = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y - }); - base.Content.AddRange(new Drawable[] { ScrollFlow = new OverlayScrollContainer { RelativeSizeAxes = Axes.Both, ScrollbarVisible = false, - Child = flow + Child = new FillFlowContainer + { + AutoSizeAxes = Axes.Y, + RelativeSizeAxes = Axes.X, + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + Header.With(h => h.Depth = -float.MaxValue), + content = new Container + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y + } + } + } }, Loading = new LoadingLayer(true) }); From 964976f604e9071e929f90cdd934ab104cf20bdb Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 25 Jan 2021 20:41:51 +0900 Subject: [PATCH 12/49] Use a task chain and fix potential misordering of events --- .../Online/Multiplayer/MultiplayerClient.cs | 11 +- .../Multiplayer/StatefulMultiplayerClient.cs | 123 +++++------------- .../Multiplayer/TestMultiplayerClient.cs | 2 + osu.Game/Utils/TaskChain.cs | 30 +++++ 4 files changed, 70 insertions(+), 96 deletions(-) create mode 100644 osu.Game/Utils/TaskChain.cs diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 5d18521eac..8573adc94a 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -134,17 +134,12 @@ namespace osu.Game.Online.Multiplayer return connection.InvokeAsync(nameof(IMultiplayerServer.JoinRoom), roomId); } - public override async Task LeaveRoom() + protected override Task LeaveRoomInternal() { if (!isConnected.Value) - { - // even if not connected, make sure the local room state can be cleaned up. - await base.LeaveRoom(); - return; - } + return Task.FromCanceled(new CancellationToken(true)); - await base.LeaveRoom(); - await connection.InvokeAsync(nameof(IMultiplayerServer.LeaveRoom)); + return connection.InvokeAsync(nameof(IMultiplayerServer.LeaveRoom)); } public override Task TransferHost(int userId) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index f2b5a44fcf..94122aeff5 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -108,67 +108,38 @@ namespace osu.Game.Online.Multiplayer }); } - private readonly object joinOrLeaveTaskLock = new object(); - private Task? joinOrLeaveTask; + private readonly TaskChain joinOrLeaveTaskChain = new TaskChain(); /// /// Joins the for a given API . /// /// The API . - public async Task JoinRoom(Room room) + public async Task JoinRoom(Room room) => await joinOrLeaveTaskChain.Add(async () => { - Task? lastTask; - Task newTask; + if (Room != null) + throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); - lock (joinOrLeaveTaskLock) + Debug.Assert(room.RoomID.Value != null); + + // Join the server-side room. + var joinedRoom = await JoinRoom(room.RoomID.Value.Value); + Debug.Assert(joinedRoom != null); + + // Populate users. + Debug.Assert(joinedRoom.Users != null); + await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)); + + // Update the stored room (must be done on update thread for thread-safety). + await scheduleAsync(() => { - lastTask = joinOrLeaveTask; - joinOrLeaveTask = newTask = Task.Run(async () => - { - if (lastTask != null) - await lastTask; + Room = joinedRoom; + apiRoom = room; + playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; + }); - // Should be thread-safe since joinOrLeaveTask is locked on in both JoinRoom() and LeaveRoom(). - if (Room != null) - throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); - - Debug.Assert(room.RoomID.Value != null); - - // Join the server-side room. - var joinedRoom = await JoinRoom(room.RoomID.Value.Value); - Debug.Assert(joinedRoom != null); - - // Populate users. - Debug.Assert(joinedRoom.Users != null); - await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)); - - // Update the stored room (must be done on update thread for thread-safety). - await scheduleAsync(() => - { - Room = joinedRoom; - apiRoom = room; - playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; - }); - - // Update room settings. - await updateLocalRoomSettings(joinedRoom.Settings); - }); - } - - try - { - await newTask; - } - finally - { - // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. - lock (joinOrLeaveTaskLock) - { - if (joinOrLeaveTask == newTask) - joinOrLeaveTask = null; - } - } - } + // Update room settings. + await updateLocalRoomSettings(joinedRoom.Settings); + }); /// /// Joins the with a given ID. @@ -177,48 +148,24 @@ namespace osu.Game.Online.Multiplayer /// The joined . protected abstract Task JoinRoom(long roomId); - public virtual async Task LeaveRoom() + public async Task LeaveRoom() => await joinOrLeaveTaskChain.Add(async () => { - Task? lastTask; - Task newTask; + if (Room == null) + return; - lock (joinOrLeaveTaskLock) + await scheduleAsync(() => { - lastTask = joinOrLeaveTask; - joinOrLeaveTask = newTask = Task.Run(async () => - { - if (lastTask != null) - await lastTask; + apiRoom = null; + Room = null; + CurrentMatchPlayingUserIds.Clear(); - // Should be thread-safe since joinOrLeaveTask is locked on in both JoinRoom() and LeaveRoom(). - if (Room == null) - return; + RoomUpdated?.Invoke(); + }); - await scheduleAsync(() => - { - apiRoom = null; - Room = null; - CurrentMatchPlayingUserIds.Clear(); + await LeaveRoomInternal(); + }); - RoomUpdated?.Invoke(); - }); - }); - } - - try - { - await newTask; - } - finally - { - // The task will be awaited in the future, so reset it so that the user doesn't get into a permanently faulted state if anything fails. - lock (joinOrLeaveTaskLock) - { - if (joinOrLeaveTask == newTask) - joinOrLeaveTask = null; - } - } - } + protected abstract Task LeaveRoomInternal(); /// /// Change the current settings. diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 7fbc770351..a79183fdab 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -98,6 +98,8 @@ namespace osu.Game.Tests.Visual.Multiplayer return Task.FromResult(room); } + protected override Task LeaveRoomInternal() => Task.CompletedTask; + public override Task TransferHost(int userId) => ((IMultiplayerClient)this).HostChanged(userId); public override async Task ChangeSettings(MultiplayerRoomSettings settings) diff --git a/osu.Game/Utils/TaskChain.cs b/osu.Game/Utils/TaskChain.cs new file mode 100644 index 0000000000..b397b0c45b --- /dev/null +++ b/osu.Game/Utils/TaskChain.cs @@ -0,0 +1,30 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +#nullable enable + +using System; +using System.Threading.Tasks; + +namespace osu.Game.Utils +{ + /// + /// A chain of s that run sequentially. + /// + public class TaskChain + { + private readonly object currentTaskLock = new object(); + private Task? currentTask; + + public Task Add(Func taskFunc) + { + lock (currentTaskLock) + { + currentTask = currentTask == null + ? taskFunc() + : currentTask.ContinueWith(_ => taskFunc()).Unwrap(); + return currentTask; + } + } + } +} From bb44fcfe31c78f77321714c466724847d27492ce Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 25 Jan 2021 20:58:02 +0900 Subject: [PATCH 13/49] Prevent some data races --- .../Multiplayer/StatefulMultiplayerClient.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 94122aeff5..0e736ed7c6 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -6,6 +6,7 @@ using System; using System.Diagnostics; using System.Linq; +using System.Threading; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -148,12 +149,15 @@ namespace osu.Game.Online.Multiplayer /// The joined . protected abstract Task JoinRoom(long roomId); - public async Task LeaveRoom() => await joinOrLeaveTaskChain.Add(async () => + public Task LeaveRoom() { if (Room == null) - return; + return Task.FromCanceled(new CancellationToken(true)); - await scheduleAsync(() => + // Leaving rooms is expected to occur instantaneously whilst the operation is finalised in the background. + // However a few members need to be reset immediately to prevent other components from entering invalid states whilst the operation hasn't yet completed. + // For example, if a room was left and the user immediately pressed the "create room" button, then the user could be taken into the lobby if the value of Room is not reset in time. + var scheduledReset = scheduleAsync(() => { apiRoom = null; Room = null; @@ -162,8 +166,12 @@ namespace osu.Game.Online.Multiplayer RoomUpdated?.Invoke(); }); - await LeaveRoomInternal(); - }); + return joinOrLeaveTaskChain.Add(async () => + { + await scheduledReset; + await LeaveRoomInternal(); + }); + } protected abstract Task LeaveRoomInternal(); From c17774e23c6b770d07cd2d9ea059ca1bcc4dff1a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 25 Jan 2021 20:58:05 +0900 Subject: [PATCH 14/49] Add xmldoc --- osu.Game/Utils/TaskChain.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Utils/TaskChain.cs b/osu.Game/Utils/TaskChain.cs index b397b0c45b..64d523bd3d 100644 --- a/osu.Game/Utils/TaskChain.cs +++ b/osu.Game/Utils/TaskChain.cs @@ -16,6 +16,11 @@ namespace osu.Game.Utils private readonly object currentTaskLock = new object(); private Task? currentTask; + /// + /// Adds a new task to the end of this . + /// + /// The task creation function. + /// The awaitable . public Task Add(Func taskFunc) { lock (currentTaskLock) From 8c3b0a316737eb359c4d00e03001ab7167ed0633 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 26 Jan 2021 22:47:37 +0900 Subject: [PATCH 15/49] Fix TaskChain performing the action in-line, add test --- osu.Game.Tests/NonVisual/TaskChainTest.cs | 83 +++++++++++++++++++++++ osu.Game/Utils/TaskChain.cs | 28 ++++++-- 2 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 osu.Game.Tests/NonVisual/TaskChainTest.cs diff --git a/osu.Game.Tests/NonVisual/TaskChainTest.cs b/osu.Game.Tests/NonVisual/TaskChainTest.cs new file mode 100644 index 0000000000..d561fb4c1b --- /dev/null +++ b/osu.Game.Tests/NonVisual/TaskChainTest.cs @@ -0,0 +1,83 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Game.Utils; + +namespace osu.Game.Tests.NonVisual +{ + [TestFixture] + public class TaskChainTest + { + private TaskChain taskChain; + private int currentTask; + + [SetUp] + public void Setup() + { + taskChain = new TaskChain(); + currentTask = 0; + } + + [Test] + public async Task TestChainedTasksRunSequentially() + { + var task1 = addTask(); + var task2 = addTask(); + var task3 = addTask(); + + task3.mutex.Set(); + task2.mutex.Set(); + task1.mutex.Set(); + + await Task.WhenAll(task1.task, task2.task, task3.task); + + Assert.That(task1.task.Result, Is.EqualTo(1)); + Assert.That(task2.task.Result, Is.EqualTo(2)); + Assert.That(task3.task.Result, Is.EqualTo(3)); + } + + [Test] + public async Task TestChainedTaskWithIntermediateCancelRunsInSequence() + { + var task1 = addTask(); + var task2 = addTask(); + var task3 = addTask(); + + // Cancel task2, allow task3 to complete. + task2.cancellation.Cancel(); + task2.mutex.Set(); + task3.mutex.Set(); + + // Allow task3 to potentially complete. + Thread.Sleep(1000); + + // Allow task1 to complete. + task1.mutex.Set(); + + // Wait on both tasks. + await Task.WhenAll(task1.task, task3.task); + + Assert.That(task1.task.Result, Is.EqualTo(1)); + Assert.That(task2.task.IsCompleted, Is.False); + Assert.That(task3.task.Result, Is.EqualTo(2)); + } + + private (Task task, ManualResetEventSlim mutex, CancellationTokenSource cancellation) addTask() + { + var mutex = new ManualResetEventSlim(false); + var cancellationSource = new CancellationTokenSource(); + var completionSource = new TaskCompletionSource(); + + taskChain.Add(() => + { + mutex.Wait(CancellationToken.None); + completionSource.SetResult(Interlocked.Increment(ref currentTask)); + }, cancellationSource.Token); + + return (completionSource.Task, mutex, cancellationSource); + } + } +} diff --git a/osu.Game/Utils/TaskChain.cs b/osu.Game/Utils/TaskChain.cs index 64d523bd3d..2bc2c00e28 100644 --- a/osu.Game/Utils/TaskChain.cs +++ b/osu.Game/Utils/TaskChain.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Threading; using System.Threading.Tasks; namespace osu.Game.Utils @@ -19,15 +20,32 @@ namespace osu.Game.Utils /// /// Adds a new task to the end of this . /// - /// The task creation function. + /// The action to be executed. + /// The for this task. Does not affect further tasks in the chain. /// The awaitable . - public Task Add(Func taskFunc) + public Task Add(Action action, CancellationToken cancellationToken = default) { lock (currentTaskLock) { - currentTask = currentTask == null - ? taskFunc() - : currentTask.ContinueWith(_ => taskFunc()).Unwrap(); + // Note: Attaching the cancellation token to the continuation could lead to re-ordering of tasks in the chain. + // Therefore, the cancellation token is not used to cancel the continuation but only the run of each task. + if (currentTask == null) + { + currentTask = Task.Run(() => + { + cancellationToken.ThrowIfCancellationRequested(); + action(); + }, CancellationToken.None); + } + else + { + currentTask = currentTask.ContinueWith(_ => + { + cancellationToken.ThrowIfCancellationRequested(); + action(); + }, CancellationToken.None); + } + return currentTask; } } From 085115cba538483ed2bbbfdac848a70a6ffdabb8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 26 Jan 2021 22:49:01 +0900 Subject: [PATCH 16/49] Make threading even more thread safe --- osu.Game/Extensions/TaskExtensions.cs | 4 +- .../Multiplayer/StatefulMultiplayerClient.cs | 82 ++++++++++++------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/osu.Game/Extensions/TaskExtensions.cs b/osu.Game/Extensions/TaskExtensions.cs index 4138c2757a..24f0188cf0 100644 --- a/osu.Game/Extensions/TaskExtensions.cs +++ b/osu.Game/Extensions/TaskExtensions.cs @@ -21,9 +21,9 @@ namespace osu.Game.Extensions /// Whether errors should be logged as errors visible to users, or as debug messages. /// Logging as debug will essentially silence the errors on non-release builds. /// - public static void CatchUnobservedExceptions(this Task task, bool logAsError = false) + public static Task CatchUnobservedExceptions(this Task task, bool logAsError = false) { - task.ContinueWith(t => + return task.ContinueWith(t => { Exception? exception = t.Exception?.AsSingular(); if (logAsError) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 0e736ed7c6..3d8ab4b4c7 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -110,37 +110,49 @@ namespace osu.Game.Online.Multiplayer } private readonly TaskChain joinOrLeaveTaskChain = new TaskChain(); + private CancellationTokenSource? joinCancellationSource; /// /// Joins the for a given API . /// /// The API . - public async Task JoinRoom(Room room) => await joinOrLeaveTaskChain.Add(async () => + public async Task JoinRoom(Room room) { - if (Room != null) - throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); + var cancellationSource = new CancellationTokenSource(); - Debug.Assert(room.RoomID.Value != null); - - // Join the server-side room. - var joinedRoom = await JoinRoom(room.RoomID.Value.Value); - Debug.Assert(joinedRoom != null); - - // Populate users. - Debug.Assert(joinedRoom.Users != null); - await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)); - - // Update the stored room (must be done on update thread for thread-safety). await scheduleAsync(() => { - Room = joinedRoom; - apiRoom = room; - playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; - }); + joinCancellationSource?.Cancel(); + joinCancellationSource = cancellationSource; + }, CancellationToken.None); - // Update room settings. - await updateLocalRoomSettings(joinedRoom.Settings); - }); + await joinOrLeaveTaskChain.Add(async () => + { + if (Room != null) + throw new InvalidOperationException("Cannot join a multiplayer room while already in one."); + + Debug.Assert(room.RoomID.Value != null); + + // Join the server-side room. + var joinedRoom = await JoinRoom(room.RoomID.Value.Value); + Debug.Assert(joinedRoom != null); + + // Populate users. + Debug.Assert(joinedRoom.Users != null); + await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)); + + // Update the stored room (must be done on update thread for thread-safety). + await scheduleAsync(() => + { + Room = joinedRoom; + apiRoom = room; + playlistItemId = room.Playlist.SingleOrDefault()?.ID ?? 0; + }, cancellationSource.Token); + + // Update room settings. + await updateLocalRoomSettings(joinedRoom.Settings, cancellationSource.Token); + }, cancellationSource.Token); + } /// /// Joins the with a given ID. @@ -151,14 +163,15 @@ namespace osu.Game.Online.Multiplayer public Task LeaveRoom() { - if (Room == null) - return Task.FromCanceled(new CancellationToken(true)); - // Leaving rooms is expected to occur instantaneously whilst the operation is finalised in the background. // However a few members need to be reset immediately to prevent other components from entering invalid states whilst the operation hasn't yet completed. // For example, if a room was left and the user immediately pressed the "create room" button, then the user could be taken into the lobby if the value of Room is not reset in time. var scheduledReset = scheduleAsync(() => { + // The join may have not completed yet, so certain tasks that either update the room or reference the room should be cancelled. + // This includes the setting of Room itself along with the initial update of the room settings on join. + joinCancellationSource?.Cancel(); + apiRoom = null; Room = null; CurrentMatchPlayingUserIds.Clear(); @@ -169,7 +182,7 @@ namespace osu.Game.Online.Multiplayer return joinOrLeaveTaskChain.Add(async () => { await scheduledReset; - await LeaveRoomInternal(); + await LeaveRoomInternal().CatchUnobservedExceptions(); }); } @@ -455,7 +468,8 @@ namespace osu.Game.Online.Multiplayer /// This updates both the joined and the respective API . /// /// The new to update from. - private Task updateLocalRoomSettings(MultiplayerRoomSettings settings) => scheduleAsync(() => + /// The to cancel the update. + private Task updateLocalRoomSettings(MultiplayerRoomSettings settings, CancellationToken cancellationToken = default) => scheduleAsync(() => { if (Room == null) return; @@ -473,10 +487,17 @@ namespace osu.Game.Online.Multiplayer RoomUpdated?.Invoke(); var req = new GetBeatmapSetRequest(settings.BeatmapID, BeatmapSetLookupType.BeatmapId); - req.Success += res => updatePlaylist(settings, res); + + req.Success += res => + { + if (cancellationToken.IsCancellationRequested) + return; + + updatePlaylist(settings, res); + }; api.Queue(req); - }); + }, cancellationToken); private void updatePlaylist(MultiplayerRoomSettings settings, APIBeatmapSet onlineSet) { @@ -524,12 +545,15 @@ namespace osu.Game.Online.Multiplayer CurrentMatchPlayingUserIds.Remove(userId); } - private Task scheduleAsync(Action action) + private Task scheduleAsync(Action action, CancellationToken cancellationToken = default) { var tcs = new TaskCompletionSource(); Scheduler.Add(() => { + if (cancellationToken.IsCancellationRequested) + return; + try { action(); From 248989b3ebe9de5a1b24341774670be6533825d3 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Jan 2021 01:20:50 +0900 Subject: [PATCH 17/49] wip --- osu.Game.Tests/NonVisual/TaskChainTest.cs | 38 ++++++++++++++++-- .../Multiplayer/StatefulMultiplayerClient.cs | 2 +- osu.Game/Utils/TaskChain.cs | 39 +++++++------------ 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/osu.Game.Tests/NonVisual/TaskChainTest.cs b/osu.Game.Tests/NonVisual/TaskChainTest.cs index d561fb4c1b..0a56468818 100644 --- a/osu.Game.Tests/NonVisual/TaskChainTest.cs +++ b/osu.Game.Tests/NonVisual/TaskChainTest.cs @@ -4,6 +4,7 @@ using System.Threading; using System.Threading.Tasks; using NUnit.Framework; +using osu.Game.Extensions; using osu.Game.Utils; namespace osu.Game.Tests.NonVisual @@ -13,14 +14,22 @@ namespace osu.Game.Tests.NonVisual { private TaskChain taskChain; private int currentTask; + private CancellationTokenSource globalCancellationToken; [SetUp] public void Setup() { + globalCancellationToken = new CancellationTokenSource(); taskChain = new TaskChain(); currentTask = 0; } + [TearDown] + public void TearDown() + { + globalCancellationToken?.Cancel(); + } + [Test] public async Task TestChainedTasksRunSequentially() { @@ -65,17 +74,40 @@ namespace osu.Game.Tests.NonVisual Assert.That(task3.task.Result, Is.EqualTo(2)); } + [Test] + public async Task TestChainedTaskDoesNotCompleteBeforeChildTasks() + { + var mutex = new ManualResetEventSlim(false); + + var task = taskChain.Add(async () => + { + await Task.Run(() => mutex.Wait(globalCancellationToken.Token)).CatchUnobservedExceptions(); + }); + + // Allow task to potentially complete + Thread.Sleep(1000); + + Assert.That(task.IsCompleted, Is.False); + + // Allow the task to complete. + mutex.Set(); + + await task; + } + private (Task task, ManualResetEventSlim mutex, CancellationTokenSource cancellation) addTask() { var mutex = new ManualResetEventSlim(false); - var cancellationSource = new CancellationTokenSource(); var completionSource = new TaskCompletionSource(); + var cancellationSource = new CancellationTokenSource(); + var token = CancellationTokenSource.CreateLinkedTokenSource(cancellationSource.Token, globalCancellationToken.Token); + taskChain.Add(() => { - mutex.Wait(CancellationToken.None); + mutex.Wait(globalCancellationToken.Token); completionSource.SetResult(Interlocked.Increment(ref currentTask)); - }, cancellationSource.Token); + }, token.Token); return (completionSource.Task, mutex, cancellationSource); } diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 3d8ab4b4c7..5c6a0d34e0 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -182,7 +182,7 @@ namespace osu.Game.Online.Multiplayer return joinOrLeaveTaskChain.Add(async () => { await scheduledReset; - await LeaveRoomInternal().CatchUnobservedExceptions(); + await LeaveRoomInternal(); }); } diff --git a/osu.Game/Utils/TaskChain.cs b/osu.Game/Utils/TaskChain.cs index 2bc2c00e28..30aea7578f 100644 --- a/osu.Game/Utils/TaskChain.cs +++ b/osu.Game/Utils/TaskChain.cs @@ -14,8 +14,8 @@ namespace osu.Game.Utils /// public class TaskChain { - private readonly object currentTaskLock = new object(); - private Task? currentTask; + private readonly object finalTaskLock = new object(); + private Task? finalTask; /// /// Adds a new task to the end of this . @@ -23,31 +23,22 @@ namespace osu.Game.Utils /// The action to be executed. /// The for this task. Does not affect further tasks in the chain. /// The awaitable . - public Task Add(Action action, CancellationToken cancellationToken = default) + public async Task Add(Action action, CancellationToken cancellationToken = default) { - lock (currentTaskLock) - { - // Note: Attaching the cancellation token to the continuation could lead to re-ordering of tasks in the chain. - // Therefore, the cancellation token is not used to cancel the continuation but only the run of each task. - if (currentTask == null) - { - currentTask = Task.Run(() => - { - cancellationToken.ThrowIfCancellationRequested(); - action(); - }, CancellationToken.None); - } - else - { - currentTask = currentTask.ContinueWith(_ => - { - cancellationToken.ThrowIfCancellationRequested(); - action(); - }, CancellationToken.None); - } + Task? previousTask; + Task currentTask; - return currentTask; + lock (finalTaskLock) + { + previousTask = finalTask; + finalTask = currentTask = new Task(action, cancellationToken); } + + if (previousTask != null) + await previousTask; + + currentTask.Start(); + await currentTask; } } } From fcfb0d52c2d1d605ac3b8510cdffa01a3714e928 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Jan 2021 19:50:16 +0900 Subject: [PATCH 18/49] Proposal to use extension method instead of TaskChain class --- osu.Game.Tests/NonVisual/TaskChainTest.cs | 19 ++++++++--- osu.Game/Extensions/TaskExtensions.cs | 39 +++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/NonVisual/TaskChainTest.cs b/osu.Game.Tests/NonVisual/TaskChainTest.cs index 0a56468818..bd4f15a6eb 100644 --- a/osu.Game.Tests/NonVisual/TaskChainTest.cs +++ b/osu.Game.Tests/NonVisual/TaskChainTest.cs @@ -1,6 +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.Threading; using System.Threading.Tasks; using NUnit.Framework; @@ -12,15 +13,17 @@ namespace osu.Game.Tests.NonVisual [TestFixture] public class TaskChainTest { - private TaskChain taskChain; + private Task taskChain; + private int currentTask; private CancellationTokenSource globalCancellationToken; [SetUp] public void Setup() { + taskChain = Task.CompletedTask; + globalCancellationToken = new CancellationTokenSource(); - taskChain = new TaskChain(); currentTask = 0; } @@ -79,9 +82,15 @@ namespace osu.Game.Tests.NonVisual { var mutex = new ManualResetEventSlim(false); - var task = taskChain.Add(async () => + var task = taskChain.ContinueWithSequential(async () => { - await Task.Run(() => mutex.Wait(globalCancellationToken.Token)).CatchUnobservedExceptions(); + try + { + await Task.Run(() => mutex.Wait(globalCancellationToken.Token)); + } + catch (OperationCanceledException) + { + } }); // Allow task to potentially complete @@ -103,7 +112,7 @@ namespace osu.Game.Tests.NonVisual var cancellationSource = new CancellationTokenSource(); var token = CancellationTokenSource.CreateLinkedTokenSource(cancellationSource.Token, globalCancellationToken.Token); - taskChain.Add(() => + taskChain = taskChain.ContinueWithSequential(() => { mutex.Wait(globalCancellationToken.Token); completionSource.SetResult(Interlocked.Increment(ref currentTask)); diff --git a/osu.Game/Extensions/TaskExtensions.cs b/osu.Game/Extensions/TaskExtensions.cs index 24f0188cf0..fd0274f39e 100644 --- a/osu.Game/Extensions/TaskExtensions.cs +++ b/osu.Game/Extensions/TaskExtensions.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Threading; using System.Threading.Tasks; using osu.Framework.Extensions.ExceptionExtensions; using osu.Framework.Logging; @@ -32,5 +33,43 @@ namespace osu.Game.Extensions Logger.Log($"Error running task: {exception}", LoggingTarget.Runtime, LogLevel.Debug); }, TaskContinuationOptions.NotOnRanToCompletion); } + + public static Task ContinueWithSequential(this Task task, Action continuationFunction, CancellationToken cancellationToken = default) + { + return task.ContinueWithSequential(() => Task.Run(continuationFunction, cancellationToken), cancellationToken); + } + + public static Task ContinueWithSequential(this Task task, Func continuationFunction, CancellationToken cancellationToken = default) + { + var tcs = new TaskCompletionSource(); + + task.ContinueWith(t => + { + if (cancellationToken.IsCancellationRequested) + { + tcs.SetCanceled(); + } + else + { + continuationFunction().ContinueWith(t2 => + { + if (cancellationToken.IsCancellationRequested || t2.IsCanceled) + { + tcs.TrySetCanceled(); + } + else if (t2.IsFaulted) + { + tcs.TrySetException(t2.Exception); + } + else + { + tcs.TrySetResult(true); + } + }, cancellationToken: default); + } + }, cancellationToken: default); + + return tcs.Task; + } } } From a30aecbafeb7d45f87f1061a2ccc71ee757c91cd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Jan 2021 20:01:21 +0900 Subject: [PATCH 19/49] Comment and add xmldoc --- osu.Game.Tests/NonVisual/TaskChainTest.cs | 1 - osu.Game/Extensions/TaskExtensions.cs | 32 +++++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/NonVisual/TaskChainTest.cs b/osu.Game.Tests/NonVisual/TaskChainTest.cs index bd4f15a6eb..342f137dfd 100644 --- a/osu.Game.Tests/NonVisual/TaskChainTest.cs +++ b/osu.Game.Tests/NonVisual/TaskChainTest.cs @@ -6,7 +6,6 @@ using System.Threading; using System.Threading.Tasks; using NUnit.Framework; using osu.Game.Extensions; -using osu.Game.Utils; namespace osu.Game.Tests.NonVisual { diff --git a/osu.Game/Extensions/TaskExtensions.cs b/osu.Game/Extensions/TaskExtensions.cs index fd0274f39e..62b249b869 100644 --- a/osu.Game/Extensions/TaskExtensions.cs +++ b/osu.Game/Extensions/TaskExtensions.cs @@ -34,32 +34,46 @@ namespace osu.Game.Extensions }, TaskContinuationOptions.NotOnRanToCompletion); } - public static Task ContinueWithSequential(this Task task, Action continuationFunction, CancellationToken cancellationToken = default) - { - return task.ContinueWithSequential(() => Task.Run(continuationFunction, cancellationToken), cancellationToken); - } + /// + /// Add a continuation to be performed only after the attached task has completed. + /// + /// The previous task to be awaited on. + /// The action to run. + /// An optional cancellation token. Will only cancel the provided action, not the sequence. + /// A task representing the provided action. + public static Task ContinueWithSequential(this Task task, Action action, CancellationToken cancellationToken = default) => + task.ContinueWithSequential(() => Task.Run(action, cancellationToken), cancellationToken); + /// + /// Add a continuation to be performed only after the attached task has completed. + /// + /// The previous task to be awaited on. + /// The continuation to run. Generally should be an async function. + /// An optional cancellation token. Will only cancel the provided action, not the sequence. + /// A task representing the provided action. public static Task ContinueWithSequential(this Task task, Func continuationFunction, CancellationToken cancellationToken = default) { var tcs = new TaskCompletionSource(); task.ContinueWith(t => { + // the previous task has finished execution or been cancelled, so we can run the provided continuation. + if (cancellationToken.IsCancellationRequested) { tcs.SetCanceled(); } else { - continuationFunction().ContinueWith(t2 => + continuationFunction().ContinueWith(continuationTask => { - if (cancellationToken.IsCancellationRequested || t2.IsCanceled) + if (cancellationToken.IsCancellationRequested || continuationTask.IsCanceled) { tcs.TrySetCanceled(); } - else if (t2.IsFaulted) + else if (continuationTask.IsFaulted) { - tcs.TrySetException(t2.Exception); + tcs.TrySetException(continuationTask.Exception); } else { @@ -69,6 +83,8 @@ namespace osu.Game.Extensions } }, cancellationToken: default); + // importantly, we are not returning the continuation itself but rather a task which represents its status in sequential execution order. + // this will not be cancelled or completed until the previous task has also. return tcs.Task; } } From 1ec305e10d4ba70ee06a9f21bb3087e0d2e245e9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Jan 2021 16:06:57 +0900 Subject: [PATCH 20/49] Update TaskChain to use ContinueWithSequential internally It turns out we may still want to use TaskChain for its locking behaviour, so I've made it internally use the refactored version I implemented, while keeping the general structure. --- osu.Game/Utils/TaskChain.cs | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/osu.Game/Utils/TaskChain.cs b/osu.Game/Utils/TaskChain.cs index 30aea7578f..df28faf9fb 100644 --- a/osu.Game/Utils/TaskChain.cs +++ b/osu.Game/Utils/TaskChain.cs @@ -6,6 +6,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using osu.Game.Extensions; namespace osu.Game.Utils { @@ -14,8 +15,9 @@ namespace osu.Game.Utils /// public class TaskChain { - private readonly object finalTaskLock = new object(); - private Task? finalTask; + private readonly object taskLock = new object(); + + private Task lastTaskInChain = Task.CompletedTask; /// /// Adds a new task to the end of this . @@ -23,22 +25,22 @@ namespace osu.Game.Utils /// The action to be executed. /// The for this task. Does not affect further tasks in the chain. /// The awaitable . - public async Task Add(Action action, CancellationToken cancellationToken = default) + public Task Add(Action action, CancellationToken cancellationToken = default) { - Task? previousTask; - Task currentTask; + lock (taskLock) + return lastTaskInChain = lastTaskInChain.ContinueWithSequential(action, cancellationToken); + } - lock (finalTaskLock) - { - previousTask = finalTask; - finalTask = currentTask = new Task(action, cancellationToken); - } - - if (previousTask != null) - await previousTask; - - currentTask.Start(); - await currentTask; + /// + /// Adds a new task to the end of this . + /// + /// The task to be executed. + /// The for this task. Does not affect further tasks in the chain. + /// The awaitable . + public Task Add(Func task, CancellationToken cancellationToken = default) + { + lock (taskLock) + return lastTaskInChain = lastTaskInChain.ContinueWithSequential(task, cancellationToken); } } } From c3aec3bfe43da7c71c243f2188b39bf24c8233cc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Jan 2021 16:20:25 +0900 Subject: [PATCH 21/49] Revert test changes to test original class/scope Importantly, this removes the call to CatchUnobservedExceptions(), which was outright incorrect (awaiting on the wrong task as a result) in the original test code. --- osu.Game.Tests/NonVisual/TaskChainTest.cs | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/NonVisual/TaskChainTest.cs b/osu.Game.Tests/NonVisual/TaskChainTest.cs index 342f137dfd..d83eaafe20 100644 --- a/osu.Game.Tests/NonVisual/TaskChainTest.cs +++ b/osu.Game.Tests/NonVisual/TaskChainTest.cs @@ -1,28 +1,25 @@ // 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.Threading; using System.Threading.Tasks; using NUnit.Framework; -using osu.Game.Extensions; +using osu.Game.Utils; namespace osu.Game.Tests.NonVisual { [TestFixture] public class TaskChainTest { - private Task taskChain; - + private TaskChain taskChain; private int currentTask; private CancellationTokenSource globalCancellationToken; [SetUp] public void Setup() { - taskChain = Task.CompletedTask; - globalCancellationToken = new CancellationTokenSource(); + taskChain = new TaskChain(); currentTask = 0; } @@ -81,16 +78,7 @@ namespace osu.Game.Tests.NonVisual { var mutex = new ManualResetEventSlim(false); - var task = taskChain.ContinueWithSequential(async () => - { - try - { - await Task.Run(() => mutex.Wait(globalCancellationToken.Token)); - } - catch (OperationCanceledException) - { - } - }); + var task = taskChain.Add(async () => await Task.Run(() => mutex.Wait(globalCancellationToken.Token))); // Allow task to potentially complete Thread.Sleep(1000); @@ -111,7 +99,7 @@ namespace osu.Game.Tests.NonVisual var cancellationSource = new CancellationTokenSource(); var token = CancellationTokenSource.CreateLinkedTokenSource(cancellationSource.Token, globalCancellationToken.Token); - taskChain = taskChain.ContinueWithSequential(() => + taskChain.Add(() => { mutex.Wait(globalCancellationToken.Token); completionSource.SetResult(Interlocked.Increment(ref currentTask)); From c3ba92f057349d56c79906bd96ab0de8b67e0fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 30 Jan 2021 16:13:50 +0100 Subject: [PATCH 22/49] Set canceled result in scheduleAsync Was holding up the task completion source, and in consequence, potentially the entire task chain. --- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 4bec327884..de51a4b117 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -551,7 +551,10 @@ namespace osu.Game.Online.Multiplayer Scheduler.Add(() => { if (cancellationToken.IsCancellationRequested) + { + tcs.SetCanceled(); return; + } try { From 90ba8ae234bbec60aab5b42f9cd625eb8e2d2f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 30 Jan 2021 23:39:01 +0100 Subject: [PATCH 23/49] Don't part room if join task was cancelled --- .../Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs index 65d112a032..1e57847f04 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs @@ -87,7 +87,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { if (t.IsCompletedSuccessfully) Schedule(() => onSuccess?.Invoke(room)); - else + else if (t.IsFaulted) { const string message = "Failed to join multiplayer room."; From 65d45ec74cefcc4db80ef0d83d9bc598bc2e864b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 3 Feb 2021 20:50:22 +0900 Subject: [PATCH 24/49] Unschedule cancellation --- .../Multiplayer/StatefulMultiplayerClient.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index de51a4b117..7d729a3c11 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -117,13 +117,7 @@ namespace osu.Game.Online.Multiplayer /// The API . public async Task JoinRoom(Room room) { - var cancellationSource = new CancellationTokenSource(); - - await scheduleAsync(() => - { - joinCancellationSource?.Cancel(); - joinCancellationSource = cancellationSource; - }, CancellationToken.None); + var cancellationSource = joinCancellationSource = new CancellationTokenSource(); await joinOrLeaveTaskChain.Add(async () => { @@ -162,15 +156,15 @@ namespace osu.Game.Online.Multiplayer public Task LeaveRoom() { + // The join may have not completed yet, so certain tasks that either update the room or reference the room should be cancelled. + // This includes the setting of Room itself along with the initial update of the room settings on join. + joinCancellationSource?.Cancel(); + // Leaving rooms is expected to occur instantaneously whilst the operation is finalised in the background. // However a few members need to be reset immediately to prevent other components from entering invalid states whilst the operation hasn't yet completed. // For example, if a room was left and the user immediately pressed the "create room" button, then the user could be taken into the lobby if the value of Room is not reset in time. var scheduledReset = scheduleAsync(() => { - // The join may have not completed yet, so certain tasks that either update the room or reference the room should be cancelled. - // This includes the setting of Room itself along with the initial update of the room settings on join. - joinCancellationSource?.Cancel(); - apiRoom = null; Room = null; CurrentMatchPlayingUserIds.Clear(); From 18e50815232534bc456429b064ae3e71bc320a01 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 00:42:38 +0900 Subject: [PATCH 25/49] Fix test failures --- .../Tests/Visual/Multiplayer/MultiplayerTestScene.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs index a87b22affe..d76f354774 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs +++ b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs @@ -6,6 +6,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Testing; using osu.Game.Online.Multiplayer; using osu.Game.Screens.OnlinePlay; using osu.Game.Screens.OnlinePlay.Lounge.Components; @@ -50,5 +51,13 @@ namespace osu.Game.Tests.Visual.Multiplayer if (joinRoom) RoomManager.Schedule(() => RoomManager.CreateRoom(Room)); }); + + public override void SetUpSteps() + { + base.SetUpSteps(); + + if (joinRoom) + AddUntilStep("wait for room join", () => Client.Room != null); + } } } From dbea6d4ceed7504ff104683efcdf316d3c712d47 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 00:57:23 +0900 Subject: [PATCH 26/49] Remove unused using --- osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs index d76f354774..2e8c834c65 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs +++ b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs @@ -6,7 +6,6 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Testing; using osu.Game.Online.Multiplayer; using osu.Game.Screens.OnlinePlay; using osu.Game.Screens.OnlinePlay.Lounge.Components; From 0528469b440d33308cdc61c7d2a136027e4731e4 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:04:04 +0900 Subject: [PATCH 27/49] Rename OrderedHitPolicy -> StartTimeOrderedHitPolicy --- osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs | 4 ++-- .../UI/{OrderedHitPolicy.cs => StartTimeOrderedHitPolicy.cs} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename osu.Game.Rulesets.Osu/UI/{OrderedHitPolicy.cs => StartTimeOrderedHitPolicy.cs} (97%) diff --git a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs index 975b444699..5c77112df7 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs @@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Osu.UI private readonly ProxyContainer spinnerProxies; private readonly JudgementContainer judgementLayer; private readonly FollowPointRenderer followPoints; - private readonly OrderedHitPolicy hitPolicy; + private readonly StartTimeOrderedHitPolicy hitPolicy; public static readonly Vector2 BASE_SIZE = new Vector2(512, 384); @@ -54,7 +54,7 @@ namespace osu.Game.Rulesets.Osu.UI approachCircles = new ProxyContainer { RelativeSizeAxes = Axes.Both }, }; - hitPolicy = new OrderedHitPolicy(HitObjectContainer); + hitPolicy = new StartTimeOrderedHitPolicy(HitObjectContainer); var hitWindows = new OsuHitWindows(); diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs similarity index 97% rename from osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs rename to osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs index 8e4f81347d..1d9a5010ed 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs @@ -18,11 +18,11 @@ namespace osu.Game.Rulesets.Osu.UI /// The hit causes all previous s to missed otherwise. /// /// - public class OrderedHitPolicy + public class StartTimeOrderedHitPolicy { private readonly HitObjectContainer hitObjectContainer; - public OrderedHitPolicy(HitObjectContainer hitObjectContainer) + public StartTimeOrderedHitPolicy(HitObjectContainer hitObjectContainer) { this.hitObjectContainer = hitObjectContainer; } From df1df8184771f096a58acc095efc576485c7301c Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:04:21 +0900 Subject: [PATCH 28/49] Better indicate ordering --- osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs index 1d9a5010ed..2b13a36e47 100644 --- a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs @@ -11,7 +11,7 @@ using osu.Game.Rulesets.UI; namespace osu.Game.Rulesets.Osu.UI { /// - /// Ensures that s are hit in-order. Affectionately known as "note lock". + /// Ensures that s are hit in-order of their start times. Affectionately known as "note lock". /// If a is hit out of order: /// /// The hit is blocked if it occurred earlier than the previous 's start time. From 08aae011c10880a24204717cfea5eb5c2629571b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:04:32 +0900 Subject: [PATCH 29/49] Add IHitPolicy interface --- osu.Game.Rulesets.Osu/UI/IHitPolicy.cs | 25 +++++++++++++++++++ .../UI/StartTimeOrderedHitPolicy.cs | 13 ++-------- 2 files changed, 27 insertions(+), 11 deletions(-) create mode 100644 osu.Game.Rulesets.Osu/UI/IHitPolicy.cs diff --git a/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs new file mode 100644 index 0000000000..fcc18b5f6f --- /dev/null +++ b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs @@ -0,0 +1,25 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; + +namespace osu.Game.Rulesets.Osu.UI +{ + public interface IHitPolicy + { + /// + /// Determines whether a can be hit at a point in time. + /// + /// The to check. + /// The time to check. + /// Whether can be hit at the given . + bool IsHittable(DrawableHitObject hitObject, double time); + + /// + /// Handles a being hit. + /// + /// The that was hit. + void HandleHit(DrawableHitObject hitObject); + } +} diff --git a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs index 2b13a36e47..2b5a440f67 100644 --- a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs @@ -18,7 +18,7 @@ namespace osu.Game.Rulesets.Osu.UI /// The hit causes all previous s to missed otherwise. /// /// - public class StartTimeOrderedHitPolicy + public class StartTimeOrderedHitPolicy : IHitPolicy { private readonly HitObjectContainer hitObjectContainer; @@ -27,12 +27,6 @@ namespace osu.Game.Rulesets.Osu.UI this.hitObjectContainer = hitObjectContainer; } - /// - /// Determines whether a can be hit at a point in time. - /// - /// The to check. - /// The time to check. - /// Whether can be hit at the given . public bool IsHittable(DrawableHitObject hitObject, double time) { DrawableHitObject blockingObject = null; @@ -54,10 +48,6 @@ namespace osu.Game.Rulesets.Osu.UI return blockingObject.Judged || time >= blockingObject.HitObject.StartTime; } - /// - /// Handles a being hit to potentially miss all earlier s. - /// - /// The that was hit. public void HandleHit(DrawableHitObject hitObject) { // Hitobjects which themselves don't block future hitobjects don't cause misses (e.g. slider ticks, spinners). @@ -67,6 +57,7 @@ namespace osu.Game.Rulesets.Osu.UI if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset)) throw new InvalidOperationException($"A {hitObject} was hit before it became hittable!"); + // Miss all hitobjects prior to the hit one. foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime)) { if (obj.Judged) From 8adf37d958abc4bb3b42e600149a4b0fdbbbaa16 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:09:25 +0900 Subject: [PATCH 30/49] Add SetHitObjects() to IHitPolicy instead of using ctor --- osu.Game.Rulesets.Osu/UI/IHitPolicy.cs | 7 +++++++ osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs | 3 ++- osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs | 10 +++------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs index fcc18b5f6f..72c3d781bb 100644 --- a/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs @@ -1,6 +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.Collections.Generic; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; @@ -8,6 +9,12 @@ namespace osu.Game.Rulesets.Osu.UI { public interface IHitPolicy { + /// + /// Sets the s which this controls. + /// + /// An enumeration of the s. + void SetHitObjects(IEnumerable hitObjects); + /// /// Determines whether a can be hit at a point in time. /// diff --git a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs index 5c77112df7..c7900558a0 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs @@ -54,7 +54,8 @@ namespace osu.Game.Rulesets.Osu.UI approachCircles = new ProxyContainer { RelativeSizeAxes = Axes.Both }, }; - hitPolicy = new StartTimeOrderedHitPolicy(HitObjectContainer); + hitPolicy = new StartTimeOrderedHitPolicy(); + hitPolicy.SetHitObjects(HitObjectContainer.AliveObjects); var hitWindows = new OsuHitWindows(); diff --git a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs index 2b5a440f67..38ba5fc490 100644 --- a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Osu.Objects.Drawables; -using osu.Game.Rulesets.UI; namespace osu.Game.Rulesets.Osu.UI { @@ -20,12 +19,9 @@ namespace osu.Game.Rulesets.Osu.UI /// public class StartTimeOrderedHitPolicy : IHitPolicy { - private readonly HitObjectContainer hitObjectContainer; + private IEnumerable hitObjects; - public StartTimeOrderedHitPolicy(HitObjectContainer hitObjectContainer) - { - this.hitObjectContainer = hitObjectContainer; - } + public void SetHitObjects(IEnumerable hitObjects) => this.hitObjects = hitObjects; public bool IsHittable(DrawableHitObject hitObject, double time) { @@ -77,7 +73,7 @@ namespace osu.Game.Rulesets.Osu.UI private IEnumerable enumerateHitObjectsUpTo(double targetTime) { - foreach (var obj in hitObjectContainer.AliveObjects) + foreach (var obj in hitObjects) { if (obj.HitObject.StartTime >= targetTime) yield break; From c9481ebbafa4c28ba21730448dcdfbf2b71d59e6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:49:33 +0900 Subject: [PATCH 31/49] Rename test scene --- ...eOutOfOrderHits.cs => TestSceneStartTimeOrderedHitPolicy.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osu.Game.Rulesets.Osu.Tests/{TestSceneOutOfOrderHits.cs => TestSceneStartTimeOrderedHitPolicy.cs} (99%) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs similarity index 99% rename from osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs rename to osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs index 296b421a11..b8c1217a73 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs @@ -25,7 +25,7 @@ using osuTK; namespace osu.Game.Rulesets.Osu.Tests { - public class TestSceneOutOfOrderHits : RateAdjustedBeatmapTestScene + public class TestSceneStartTimeOrderedHitPolicy : RateAdjustedBeatmapTestScene { private const double early_miss_window = 1000; // time after -1000 to -500 is considered a miss private const double late_miss_window = 500; // time after +500 is considered a miss From 64a2c7825e89811dec78f803507fdd4c8696cc52 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 5 Feb 2021 14:53:47 +0900 Subject: [PATCH 32/49] Fix incorrect assert --- .../TestSceneStartTimeOrderedHitPolicy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs index b8c1217a73..177a4f50a1 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs @@ -169,7 +169,7 @@ namespace osu.Game.Rulesets.Osu.Tests addJudgementAssert(hitObjects[0], HitResult.Great); addJudgementAssert(hitObjects[1], HitResult.Great); addJudgementOffsetAssert(hitObjects[0], -200); // time_first_circle - 200 - addJudgementOffsetAssert(hitObjects[0], -200); // time_second_circle - first_circle_time - 100 + addJudgementOffsetAssert(hitObjects[1], -200); // time_second_circle - first_circle_time - 100 } /// From 6b26a18a23162e784fad3c941ace78fa557bf7d4 Mon Sep 17 00:00:00 2001 From: Joehu Date: Mon, 8 Feb 2021 01:34:32 -0800 Subject: [PATCH 33/49] Fix attributes header not being aligned with content in editor timing mode --- osu.Game/Screens/Edit/Timing/ControlPointTable.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index e4b9150df1..81b006e6c8 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -74,7 +74,8 @@ namespace osu.Game.Screens.Edit.Timing { new TableColumn(string.Empty, Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), new TableColumn("Time", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), - new TableColumn("Attributes", Anchor.Centre), + new TableColumn(), + new TableColumn("Attributes", Anchor.CentreLeft), }; return columns.ToArray(); @@ -93,6 +94,7 @@ namespace osu.Game.Screens.Edit.Timing Text = group.Time.ToEditorFormattedString(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold) }, + null, new ControlGroupAttributes(group), }; @@ -108,7 +110,6 @@ namespace osu.Game.Screens.Edit.Timing { RelativeSizeAxes = Axes.Both, Direction = FillDirection.Horizontal, - Padding = new MarginPadding(10), Spacing = new Vector2(2) }; From 5e7823b289a607731f253ec342c24fa9fcde7143 Mon Sep 17 00:00:00 2001 From: Joehu Date: Mon, 8 Feb 2021 01:37:34 -0800 Subject: [PATCH 34/49] Fix attributes content being zero size and disappearing after being half off-screen --- osu.Game/Screens/Edit/Timing/ControlPointTable.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index 81b006e6c8..8980c2019a 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -106,6 +106,7 @@ namespace osu.Game.Screens.Edit.Timing public ControlGroupAttributes(ControlPointGroup group) { + RelativeSizeAxes = Axes.Both; InternalChild = fill = new FillFlowContainer { RelativeSizeAxes = Axes.Both, From b40b159acb276d35bc553a597bc58980f3d3c1dd Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 18:52:50 +0900 Subject: [PATCH 35/49] Round beatlength --- osu.Game/Beatmaps/Beatmap.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 51fdbce96d..434bff14b5 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -1,6 +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 osu.Game.Beatmaps.Timing; using osu.Game.Rulesets.Objects; using System.Collections.Generic; @@ -65,7 +66,7 @@ namespace osu.Game.Beatmaps return (beatLength: t.BeatLength, duration: nextTime - t.Time); }) // Aggregate durations into a set of (beatLength, duration) tuples for each beat length - .GroupBy(t => t.beatLength) + .GroupBy(t => Math.Round(t.beatLength * 1000) / 1000) .Select(g => (beatLength: g.Key, duration: g.Sum(t => t.duration))) // And if there are no timing points, use a default. .DefaultIfEmpty((TimingControlPoint.DEFAULT_BEAT_LENGTH, 0)); From 18e3f8c233da2ed3eb8b859944994d39f9f54512 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 19:03:19 +0900 Subject: [PATCH 36/49] Sort beat lengths rather than linear search --- osu.Game/Beatmaps/Beatmap.cs | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 434bff14b5..e5b6a4bc44 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -55,7 +55,7 @@ namespace osu.Game.Beatmaps // Note: This is more accurate and may present different results because osu-stable didn't have the ability to calculate slider durations in this context. double lastTime = HitObjects.LastOrDefault()?.GetEndTime() ?? ControlPointInfo.TimingPoints.LastOrDefault()?.Time ?? 0; - var beatLengthsAndDurations = + var mostCommon = // Construct a set of (beatLength, duration) tuples for each individual timing point. ControlPointInfo.TimingPoints.Select((t, i) => { @@ -68,23 +68,10 @@ namespace osu.Game.Beatmaps // Aggregate durations into a set of (beatLength, duration) tuples for each beat length .GroupBy(t => Math.Round(t.beatLength * 1000) / 1000) .Select(g => (beatLength: g.Key, duration: g.Sum(t => t.duration))) - // And if there are no timing points, use a default. - .DefaultIfEmpty((TimingControlPoint.DEFAULT_BEAT_LENGTH, 0)); + // Get the most common one, or 0 as a suitable default + .OrderByDescending(i => i.duration).FirstOrDefault(); - // Find the single beat length with the maximum aggregate duration. - double maxDurationBeatLength = double.NegativeInfinity; - double maxDuration = double.NegativeInfinity; - - foreach (var (beatLength, duration) in beatLengthsAndDurations) - { - if (duration > maxDuration) - { - maxDuration = duration; - maxDurationBeatLength = beatLength; - } - } - - return maxDurationBeatLength; + return mostCommon.beatLength; } IBeatmap IBeatmap.Clone() => Clone(); From d8c53e34ae5cba88b491d5379dec5d7ecd15e9f8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 19:42:17 +0900 Subject: [PATCH 37/49] Fix missing using --- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 032493a5c6..f454fe619b 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Threading; From 19368f87fb8c37eec5bf06f71b2d15959722cf08 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 19:59:07 +0900 Subject: [PATCH 38/49] Fix failing test --- osu.Game/Screens/Play/Player.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 669fa93298..7924e1390b 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -174,7 +174,7 @@ namespace osu.Game.Screens.Play } [BackgroundDependencyLoader(true)] - private void load(AudioManager audio, OsuConfigManager config, OsuGame game) + private void load(AudioManager audio, OsuConfigManager config, OsuGameBase game) { Mods.Value = base.Mods.Value.Select(m => m.CreateCopy()).ToArray(); @@ -191,10 +191,9 @@ namespace osu.Game.Screens.Play mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); if (game != null) - { - LocalUserPlaying.BindTo(game.LocalUserPlaying); gameActive.BindTo(game.IsActive); - } + if (game is OsuGame osuGame) + LocalUserPlaying.BindTo(osuGame.LocalUserPlaying); DrawableRuleset = ruleset.CreateDrawableRulesetWith(playableBeatmap, Mods.Value); From 156f5bd5df715323e6dc227c7fb5be7e439ff72e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Feb 2021 20:05:16 +0900 Subject: [PATCH 39/49] Add newline between statements --- osu.Game/Screens/Play/Player.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 7924e1390b..5d06ac5b3a 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -192,6 +192,7 @@ namespace osu.Game.Screens.Play if (game != null) gameActive.BindTo(game.IsActive); + if (game is OsuGame osuGame) LocalUserPlaying.BindTo(osuGame.LocalUserPlaying); From f4a31287bfca31bf088f077e014f7af4d68b6232 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 20:11:06 +0900 Subject: [PATCH 40/49] Add/use IHitObjectContainer interface instead of IEnumerables --- osu.Game.Rulesets.Osu/UI/IHitPolicy.cs | 7 +++--- osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs | 3 +-- .../UI/StartTimeOrderedHitPolicy.cs | 7 +++--- osu.Game/Rulesets/UI/HitObjectContainer.cs | 11 +-------- osu.Game/Rulesets/UI/IHitObjectContainer.cs | 24 +++++++++++++++++++ 5 files changed, 32 insertions(+), 20 deletions(-) create mode 100644 osu.Game/Rulesets/UI/IHitObjectContainer.cs diff --git a/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs index 72c3d781bb..5d8ea035a7 100644 --- a/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/IHitPolicy.cs @@ -1,19 +1,18 @@ // 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.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.UI; namespace osu.Game.Rulesets.Osu.UI { public interface IHitPolicy { /// - /// Sets the s which this controls. + /// The containing the s which this applies to. /// - /// An enumeration of the s. - void SetHitObjects(IEnumerable hitObjects); + IHitObjectContainer HitObjectContainer { set; } /// /// Determines whether a can be hit at a point in time. diff --git a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs index c7900558a0..e085714265 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs @@ -54,8 +54,7 @@ namespace osu.Game.Rulesets.Osu.UI approachCircles = new ProxyContainer { RelativeSizeAxes = Axes.Both }, }; - hitPolicy = new StartTimeOrderedHitPolicy(); - hitPolicy.SetHitObjects(HitObjectContainer.AliveObjects); + hitPolicy = new StartTimeOrderedHitPolicy { HitObjectContainer = HitObjectContainer }; var hitWindows = new OsuHitWindows(); diff --git a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs index 38ba5fc490..0173156246 100644 --- a/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/StartTimeOrderedHitPolicy.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Osu.Objects.Drawables; +using osu.Game.Rulesets.UI; namespace osu.Game.Rulesets.Osu.UI { @@ -19,9 +20,7 @@ namespace osu.Game.Rulesets.Osu.UI /// public class StartTimeOrderedHitPolicy : IHitPolicy { - private IEnumerable hitObjects; - - public void SetHitObjects(IEnumerable hitObjects) => this.hitObjects = hitObjects; + public IHitObjectContainer HitObjectContainer { get; set; } public bool IsHittable(DrawableHitObject hitObject, double time) { @@ -73,7 +72,7 @@ namespace osu.Game.Rulesets.Osu.UI private IEnumerable enumerateHitObjectsUpTo(double targetTime) { - foreach (var obj in hitObjects) + foreach (var obj in HitObjectContainer.AliveObjects) { if (obj.HitObject.StartTime >= targetTime) yield break; diff --git a/osu.Game/Rulesets/UI/HitObjectContainer.cs b/osu.Game/Rulesets/UI/HitObjectContainer.cs index 1972043ccb..11312a46df 100644 --- a/osu.Game/Rulesets/UI/HitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/HitObjectContainer.cs @@ -17,19 +17,10 @@ using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.UI { - public class HitObjectContainer : LifetimeManagementContainer + public class HitObjectContainer : LifetimeManagementContainer, IHitObjectContainer { - /// - /// All currently in-use s. - /// public IEnumerable Objects => InternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); - /// - /// All currently in-use s that are alive. - /// - /// - /// If this uses pooled objects, this is equivalent to . - /// public IEnumerable AliveObjects => AliveInternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); /// diff --git a/osu.Game/Rulesets/UI/IHitObjectContainer.cs b/osu.Game/Rulesets/UI/IHitObjectContainer.cs new file mode 100644 index 0000000000..4c784132e8 --- /dev/null +++ b/osu.Game/Rulesets/UI/IHitObjectContainer.cs @@ -0,0 +1,24 @@ +// 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.Game.Rulesets.Objects.Drawables; + +namespace osu.Game.Rulesets.UI +{ + public interface IHitObjectContainer + { + /// + /// All currently in-use s. + /// + IEnumerable Objects { get; } + + /// + /// All currently in-use s that are alive. + /// + /// + /// If this uses pooled objects, this is equivalent to . + /// + IEnumerable AliveObjects { get; } + } +} From 414e05affdf867f3bda1eb02a9639c965368b3d7 Mon Sep 17 00:00:00 2001 From: Joehu Date: Mon, 8 Feb 2021 02:41:07 -0800 Subject: [PATCH 41/49] Fix editor effect attribute tooltip having unnecessary whitespace when only one is enabled --- osu.Game/Screens/Edit/Timing/ControlPointTable.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index 8980c2019a..cae7d5a021 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -151,7 +151,11 @@ namespace osu.Game.Screens.Edit.Timing return new RowAttribute("difficulty", () => $"{difficulty.SpeedMultiplier:n2}x", colour); case EffectControlPoint effect: - return new RowAttribute("effect", () => $"{(effect.KiaiMode ? "Kiai " : "")}{(effect.OmitFirstBarLine ? "NoBarLine " : "")}", colour); + return new RowAttribute("effect", () => string.Join(" ", new[] + { + effect.KiaiMode ? "Kiai" : string.Empty, + effect.OmitFirstBarLine ? "NoBarLine" : string.Empty + }.Where(s => !string.IsNullOrEmpty(s))), colour); case SampleControlPoint sample: return new RowAttribute("sample", () => $"{sample.SampleBank} {sample.SampleVolume}%", colour); From 3ce605b5e5174632bc78ec5792c75ea0e92be009 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 9 Feb 2021 12:00:03 +0900 Subject: [PATCH 42/49] Small refactoring to use .Trim() instead --- osu.Game/Screens/Edit/Timing/ControlPointTable.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index cae7d5a021..a17b431fcc 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -151,11 +151,10 @@ namespace osu.Game.Screens.Edit.Timing return new RowAttribute("difficulty", () => $"{difficulty.SpeedMultiplier:n2}x", colour); case EffectControlPoint effect: - return new RowAttribute("effect", () => string.Join(" ", new[] - { + return new RowAttribute("effect", () => string.Join(" ", effect.KiaiMode ? "Kiai" : string.Empty, effect.OmitFirstBarLine ? "NoBarLine" : string.Empty - }.Where(s => !string.IsNullOrEmpty(s))), colour); + ).Trim(), colour); case SampleControlPoint sample: return new RowAttribute("sample", () => $"{sample.SampleBank} {sample.SampleVolume}%", colour); From 695e46a358ba1dd75da161ee70e09bd19d462334 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 15:31:55 +0900 Subject: [PATCH 43/49] Fix AutoPilot mod failing to block touch input --- osu.Game.Rulesets.Osu/OsuInputManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/OsuInputManager.cs b/osu.Game.Rulesets.Osu/OsuInputManager.cs index c8fe4f41ca..7314021a14 100644 --- a/osu.Game.Rulesets.Osu/OsuInputManager.cs +++ b/osu.Game.Rulesets.Osu/OsuInputManager.cs @@ -34,7 +34,7 @@ namespace osu.Game.Rulesets.Osu protected override bool Handle(UIEvent e) { - if (e is MouseMoveEvent && !AllowUserCursorMovement) return false; + if ((e is MouseMoveEvent || e is TouchMoveEvent) && !AllowUserCursorMovement) return false; return base.Handle(e); } From b5fa9508006c76decac0752a6bdaf47598196d4d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 18:30:05 +0900 Subject: [PATCH 44/49] Remove unnecessary depth specification --- osu.Game/Overlays/OnlineOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/OnlineOverlay.cs b/osu.Game/Overlays/OnlineOverlay.cs index 4a7318d065..b07f91b9ed 100644 --- a/osu.Game/Overlays/OnlineOverlay.cs +++ b/osu.Game/Overlays/OnlineOverlay.cs @@ -32,7 +32,7 @@ namespace osu.Game.Overlays Direction = FillDirection.Vertical, Children = new Drawable[] { - Header.With(h => h.Depth = -float.MaxValue), + Header, content = new Container { RelativeSizeAxes = Axes.X, From 178d88bcf197393fab133d3bd760d33c35b8e3c4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 18:32:44 +0900 Subject: [PATCH 45/49] Change BackgroundColour into a property --- osu.Game/Overlays/BeatmapListingOverlay.cs | 2 +- osu.Game/Overlays/BeatmapSetOverlay.cs | 2 +- osu.Game/Overlays/ChangelogOverlay.cs | 2 +- osu.Game/Overlays/FullscreenOverlay.cs | 6 +++--- osu.Game/Overlays/UserProfileOverlay.cs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Overlays/BeatmapListingOverlay.cs b/osu.Game/Overlays/BeatmapListingOverlay.cs index cfa0ff00bc..5df7a4650e 100644 --- a/osu.Game/Overlays/BeatmapListingOverlay.cs +++ b/osu.Game/Overlays/BeatmapListingOverlay.cs @@ -85,7 +85,7 @@ namespace osu.Game.Overlays protected override BeatmapListingHeader CreateHeader() => new BeatmapListingHeader(); - protected override Color4 GetBackgroundColour() => ColourProvider.Background6; + protected override Color4 BackgroundColour => ColourProvider.Background6; private void onTypingStarted() { diff --git a/osu.Game/Overlays/BeatmapSetOverlay.cs b/osu.Game/Overlays/BeatmapSetOverlay.cs index 723b61bbc5..bdb3715e73 100644 --- a/osu.Game/Overlays/BeatmapSetOverlay.cs +++ b/osu.Game/Overlays/BeatmapSetOverlay.cs @@ -68,7 +68,7 @@ namespace osu.Game.Overlays protected override BeatmapSetHeader CreateHeader() => new BeatmapSetHeader(); - protected override Color4 GetBackgroundColour() => ColourProvider.Background6; + protected override Color4 BackgroundColour => ColourProvider.Background6; protected override void PopOutComplete() { diff --git a/osu.Game/Overlays/ChangelogOverlay.cs b/osu.Game/Overlays/ChangelogOverlay.cs index 5200b567ff..05bad30107 100644 --- a/osu.Game/Overlays/ChangelogOverlay.cs +++ b/osu.Game/Overlays/ChangelogOverlay.cs @@ -55,7 +55,7 @@ namespace osu.Game.Overlays ListingSelected = ShowListing, }; - protected override Color4 GetBackgroundColour() => ColourProvider.Background4; + protected override Color4 BackgroundColour => ColourProvider.Background4; public void ShowListing() { diff --git a/osu.Game/Overlays/FullscreenOverlay.cs b/osu.Game/Overlays/FullscreenOverlay.cs index d0a0c994aa..735f0bcbd4 100644 --- a/osu.Game/Overlays/FullscreenOverlay.cs +++ b/osu.Game/Overlays/FullscreenOverlay.cs @@ -23,6 +23,8 @@ namespace osu.Game.Overlays public T Header { get; } + protected virtual Color4 BackgroundColour => ColourProvider.Background5; + [Resolved] protected IAPIProvider API { get; private set; } @@ -59,7 +61,7 @@ namespace osu.Game.Overlays new Box { RelativeSizeAxes = Axes.Both, - Colour = GetBackgroundColour() + Colour = BackgroundColour }, content = new Container { @@ -80,8 +82,6 @@ namespace osu.Game.Overlays [NotNull] protected abstract T CreateHeader(); - protected virtual Color4 GetBackgroundColour() => ColourProvider.Background5; - public override void Show() { if (State.Value == Visibility.Visible) diff --git a/osu.Game/Overlays/UserProfileOverlay.cs b/osu.Game/Overlays/UserProfileOverlay.cs index ccd9c291c4..299a14b250 100644 --- a/osu.Game/Overlays/UserProfileOverlay.cs +++ b/osu.Game/Overlays/UserProfileOverlay.cs @@ -36,7 +36,7 @@ namespace osu.Game.Overlays protected override ProfileHeader CreateHeader() => new ProfileHeader(); - protected override Color4 GetBackgroundColour() => ColourProvider.Background6; + protected override Color4 BackgroundColour => ColourProvider.Background6; public void ShowUser(int userId) => ShowUser(new User { Id = userId }); From 167076663304d009769df70fae7feccdf4128f0b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 18:42:15 +0900 Subject: [PATCH 46/49] Avoid unbinding external events --- osu.Game/Overlays/RankingsOverlay.cs | 44 +++++++++------------- osu.Game/Overlays/TabbableOnlineOverlay.cs | 2 +- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/osu.Game/Overlays/RankingsOverlay.cs b/osu.Game/Overlays/RankingsOverlay.cs index 6cd72d6e2c..a093969115 100644 --- a/osu.Game/Overlays/RankingsOverlay.cs +++ b/osu.Game/Overlays/RankingsOverlay.cs @@ -17,8 +17,6 @@ namespace osu.Game.Overlays { protected Bindable Country => Header.Country; - protected Bindable Scope => Header.Current; - private APIRequest lastRequest; [Resolved] @@ -42,31 +40,31 @@ namespace osu.Game.Overlays { // if a country is requested, force performance scope. if (Country.Value != null) - Scope.Value = RankingsScope.Performance; + Header.Current.Value = RankingsScope.Performance; - Scheduler.AddOnce(loadNewContent); - }); - - // Unbind events from scope so base class event will not be called - Scope.UnbindEvents(); - Scope.BindValueChanged(_ => - { - // country filtering is only valid for performance scope. - if (Scope.Value != RankingsScope.Performance) - Country.Value = null; - - Scheduler.AddOnce(loadNewContent); + Scheduler.AddOnce(triggerTabChanged); }); ruleset.BindValueChanged(_ => { - if (Scope.Value == RankingsScope.Spotlights) + if (Header.Current.Value == RankingsScope.Spotlights) return; - Scheduler.AddOnce(loadNewContent); + Scheduler.AddOnce(triggerTabChanged); }); } + protected override void OnTabChanged(RankingsScope tab) + { + // country filtering is only valid for performance scope. + if (Header.Current.Value != RankingsScope.Performance) + Country.Value = null; + + Scheduler.AddOnce(triggerTabChanged); + } + + private void triggerTabChanged() => base.OnTabChanged(Header.Current.Value); + protected override RankingsOverlayHeader CreateHeader() => new RankingsOverlayHeader(); public void ShowCountry(Country requested) @@ -79,17 +77,11 @@ namespace osu.Game.Overlays Country.Value = requested; } - public void ShowSpotlights() - { - Scope.Value = RankingsScope.Spotlights; - Show(); - } - protected override void CreateDisplayToLoad(RankingsScope tab) { lastRequest?.Cancel(); - if (Scope.Value == RankingsScope.Spotlights) + if (Header.Current.Value == RankingsScope.Spotlights) { LoadDisplay(new SpotlightsLayout { @@ -115,7 +107,7 @@ namespace osu.Game.Overlays private APIRequest createScopedRequest() { - switch (Scope.Value) + switch (Header.Current.Value) { case RankingsScope.Performance: return new GetUserRankingsRequest(ruleset.Value, country: Country.Value?.FlagName); @@ -153,8 +145,6 @@ namespace osu.Game.Overlays return null; } - private void loadNewContent() => OnTabChanged(Scope.Value); - protected override void Dispose(bool isDisposing) { lastRequest?.Cancel(); diff --git a/osu.Game/Overlays/TabbableOnlineOverlay.cs b/osu.Game/Overlays/TabbableOnlineOverlay.cs index cbcf3cd96e..8172e99c1b 100644 --- a/osu.Game/Overlays/TabbableOnlineOverlay.cs +++ b/osu.Game/Overlays/TabbableOnlineOverlay.cs @@ -68,7 +68,7 @@ namespace osu.Game.Overlays }, (cancellationToken = new CancellationTokenSource()).Token); } - protected void OnTabChanged(TEnum tab) + protected virtual void OnTabChanged(TEnum tab) { cancellationToken?.Cancel(); Loading.Show(); From 17475e60b0a733344a60a619a3c4fc16f2d9b95d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 18:48:50 +0900 Subject: [PATCH 47/49] Fix missed test scene update --- osu.Game.Tests/Visual/Online/TestSceneRankingsOverlay.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneRankingsOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneRankingsOverlay.cs index 626f545b91..aff510dd95 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneRankingsOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneRankingsOverlay.cs @@ -25,7 +25,7 @@ namespace osu.Game.Tests.Visual.Online Add(rankingsOverlay = new TestRankingsOverlay { Country = { BindTarget = countryBindable }, - Scope = { BindTarget = scope }, + Header = { Current = { BindTarget = scope } }, }); } @@ -65,8 +65,6 @@ namespace osu.Game.Tests.Visual.Online private class TestRankingsOverlay : RankingsOverlay { public new Bindable Country => base.Country; - - public new Bindable Scope => base.Scope; } } } From 0a96f4d403cf3be814b10de8e2d7cc3e4e2c335e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 18:56:27 +0900 Subject: [PATCH 48/49] Avoid assigning null to a non-nullable property --- .../Visual/Online/TestSceneFullscreenOverlay.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs index f8b059e471..dc468bb62d 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs @@ -53,7 +53,16 @@ namespace osu.Game.Tests.Visual.Online }; } - protected override OverlayHeader CreateHeader() => null; + protected override OverlayHeader CreateHeader() => new TestHeader(); + + internal class TestHeader : OverlayHeader + { + protected override OverlayTitle CreateTitle() => new TestTitle(); + + internal class TestTitle : OverlayTitle + { + } + } } } } From d8d830db6e39a3d590dc3963f10e60a5919727fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 9 Feb 2021 19:46:57 +0900 Subject: [PATCH 49/49] Defer playlist load to improve load time of the now playing overlay --- osu.Game/Overlays/NowPlayingOverlay.cs | 37 +++++++++++++++++++------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index 9beb859f28..f94b41155a 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -84,11 +84,6 @@ namespace osu.Game.Overlays AutoSizeAxes = Axes.Y, Children = new Drawable[] { - playlist = new PlaylistOverlay - { - RelativeSizeAxes = Axes.X, - Y = player_height + 10, - }, playerContainer = new Container { RelativeSizeAxes = Axes.X, @@ -171,7 +166,7 @@ namespace osu.Game.Overlays Anchor = Anchor.CentreRight, Position = new Vector2(-bottom_black_area_height / 2, 0), Icon = FontAwesome.Solid.Bars, - Action = () => playlist.ToggleVisibility(), + Action = togglePlaylist }, } }, @@ -191,13 +186,35 @@ namespace osu.Game.Overlays }; } + private void togglePlaylist() + { + if (playlist == null) + { + LoadComponentAsync(playlist = new PlaylistOverlay + { + RelativeSizeAxes = Axes.X, + Y = player_height + 10, + }, _ => + { + dragContainer.Add(playlist); + + playlist.BeatmapSets.BindTo(musicController.BeatmapSets); + playlist.State.BindValueChanged(s => playlistButton.FadeColour(s.NewValue == Visibility.Visible ? colours.Yellow : Color4.White, 200, Easing.OutQuint), true); + + togglePlaylist(); + }); + + return; + } + + if (!beatmap.Disabled) + playlist.ToggleVisibility(); + } + protected override void LoadComplete() { base.LoadComplete(); - playlist.BeatmapSets.BindTo(musicController.BeatmapSets); - playlist.State.BindValueChanged(s => playlistButton.FadeColour(s.NewValue == Visibility.Visible ? colours.Yellow : Color4.White, 200, Easing.OutQuint), true); - beatmap.BindDisabledChanged(beatmapDisabledChanged, true); musicController.TrackChanged += trackChanged; @@ -306,7 +323,7 @@ namespace osu.Game.Overlays private void beatmapDisabledChanged(bool disabled) { if (disabled) - playlist.Hide(); + playlist?.Hide(); prevButton.Enabled.Value = !disabled; nextButton.Enabled.Value = !disabled;