From 6c40cf08cce811707cadc5c8e68c6e7d4fe3bc60 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Nov 2017 21:29:07 +0900 Subject: [PATCH 1/7] Optimise leaderboard display Adds async loading support and cleans up the code quite a bit in the process. --- .../Graphics/Containers/OsuScrollContainer.cs | 2 +- .../Select/Leaderboards/Leaderboard.cs | 62 +++++---- .../Select/Leaderboards/LeaderboardScore.cs | 130 +++++++----------- 3 files changed, 87 insertions(+), 107 deletions(-) diff --git a/osu.Game/Graphics/Containers/OsuScrollContainer.cs b/osu.Game/Graphics/Containers/OsuScrollContainer.cs index e395f1b7bd..3fc9f439fa 100644 --- a/osu.Game/Graphics/Containers/OsuScrollContainer.cs +++ b/osu.Game/Graphics/Containers/OsuScrollContainer.cs @@ -7,7 +7,7 @@ using OpenTK.Input; namespace osu.Game.Graphics.Containers { - internal class OsuScrollContainer : ScrollContainer + public class OsuScrollContainer : ScrollContainer { /// /// Allows controlling the scroll bar from any position in the container using the right mouse button. diff --git a/osu.Game/Screens/Select/Leaderboards/Leaderboard.cs b/osu.Game/Screens/Select/Leaderboards/Leaderboard.cs index 7d65b8b648..4b1070f236 100644 --- a/osu.Game/Screens/Select/Leaderboards/Leaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/Leaderboard.cs @@ -17,19 +17,21 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Scoring; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using System.Linq; namespace osu.Game.Screens.Select.Leaderboards { public class Leaderboard : Container { private readonly ScrollContainer scrollContainer; - private readonly FillFlowContainer scrollFlow; + private FillFlowContainer scrollFlow; public Action ScoreSelected; private readonly LoadingAnimation loading; private IEnumerable scores; + public IEnumerable Scores { get { return scores; } @@ -41,33 +43,43 @@ namespace osu.Game.Screens.Select.Leaderboards int i = 150; if (scores == null) { - foreach (var c in scrollFlow.Children) - c.FadeOut(i += 10); + if (scrollFlow != null) + { + foreach (var c in scrollFlow.Children) + c.FadeOut(i += 10); - foreach (var c in scrollFlow.Children) - c.LifetimeEnd = Time.Current + i; + foreach (var c in scrollFlow.Children) + c.LifetimeEnd = Time.Current + i; + } return; } - scrollFlow.Clear(); - - i = 0; - foreach (var s in scores) + // schedule because we may not be loaded yet (LoadComponentAsync complains). + Schedule(() => { - var ls = new LeaderboardScore(s, 1 + i) + LoadComponentAsync(new FillFlowContainer { - AlwaysPresent = true, - Action = () => ScoreSelected?.Invoke(s), - State = Visibility.Hidden, - }; - scrollFlow.Add(ls); + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Spacing = new Vector2(0f, 5f), + Padding = new MarginPadding { Top = 10, Bottom = 5 }, + ChildrenEnumerable = scores.Select(s => new LeaderboardScore(s, 1 + i) { Action = () => ScoreSelected?.Invoke(s) }) + }, f => + { + scrollFlow?.Expire(); + scrollContainer.Add(scrollFlow = f); - using (BeginDelayedSequence(i++ * 50, true)) - ls.Show(); - } + i = 0; + foreach (var s in f.Children) + { + using (s.BeginDelayedSequence(i++ * 50, true)) + s.Show(); + } - scrollContainer.ScrollTo(0f, false); + scrollContainer.ScrollTo(0f, false); + }); + }); } } @@ -79,16 +91,6 @@ namespace osu.Game.Screens.Select.Leaderboards { RelativeSizeAxes = Axes.Both, ScrollbarVisible = false, - Children = new Drawable[] - { - scrollFlow = new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Spacing = new Vector2(0f, 5f), - Padding = new MarginPadding { Top = 10, Bottom = 5 }, - }, - }, }, loading = new LoadingAnimation() }; @@ -152,6 +154,8 @@ namespace osu.Game.Screens.Select.Leaderboards if (!scrollContainer.IsScrolledToEnd()) fadeStart -= LeaderboardScore.HEIGHT; + if (scrollFlow == null) return; + foreach (var c in scrollFlow.Children) { var topY = c.ToSpaceOfOtherDrawable(Vector2.Zero, scrollFlow).Y; diff --git a/osu.Game/Screens/Select/Leaderboards/LeaderboardScore.cs b/osu.Game/Screens/Select/Leaderboards/LeaderboardScore.cs index 9044938a75..e9de3fb672 100644 --- a/osu.Game/Screens/Select/Leaderboards/LeaderboardScore.cs +++ b/osu.Game/Screens/Select/Leaderboards/LeaderboardScore.cs @@ -2,9 +2,10 @@ // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE using System; +using System.Linq; using OpenTK; using OpenTK.Graphics; -using osu.Framework; +using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -13,14 +14,13 @@ using osu.Framework.Input; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; -using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.UI; using osu.Game.Users; namespace osu.Game.Screens.Select.Leaderboards { - public class LeaderboardScore : OsuClickableContainer, IStateful + public class LeaderboardScore : OsuClickableContainer { public static readonly float HEIGHT = 60; @@ -34,72 +34,16 @@ namespace osu.Game.Screens.Select.Leaderboards private const float background_alpha = 0.25f; private const float rank_width = 30; - private readonly Box background; - private readonly Container content; - private readonly Container avatar; - private readonly DrawableRank scoreRank; - private readonly OsuSpriteText nameLabel; - private readonly GlowingSpriteText scoreLabel; - private readonly ScoreComponentLabel maxCombo; - private readonly ScoreComponentLabel accuracy; - private readonly Container flagBadgeContainer; - private readonly FillFlowContainer modsContainer; - - private Visibility state; - - public Visibility State - { - get { return state; } - set - { - if (state == value) - return; - state = value; - - switch (state) - { - case Visibility.Hidden: - foreach (var d in new Drawable[] { avatar, nameLabel, scoreLabel, scoreRank, flagBadgeContainer, maxCombo, accuracy, modsContainer }) - d.FadeOut(); - - Alpha = 0; - - content.MoveToY(75); - avatar.MoveToX(75); - nameLabel.MoveToX(150); - break; - case Visibility.Visible: - this.FadeIn(200); - content.MoveToY(0, 800, Easing.OutQuint); - - using (BeginDelayedSequence(100, true)) - { - avatar.FadeIn(300, Easing.OutQuint); - nameLabel.FadeIn(350, Easing.OutQuint); - - avatar.MoveToX(0, 300, Easing.OutQuint); - nameLabel.MoveToX(0, 350, Easing.OutQuint); - - using (BeginDelayedSequence(250, true)) - { - scoreLabel.FadeIn(200); - scoreRank.FadeIn(200); - - using (BeginDelayedSequence(50, true)) - { - var drawables = new Drawable[] { flagBadgeContainer, maxCombo, accuracy, modsContainer, }; - for (int i = 0; i < drawables.Length; i++) - drawables[i].FadeIn(100 + i * 50); - } - } - } - - break; - } - - StateChanged?.Invoke(State); - } - } + private Box background; + private Container content; + private Container avatar; + private DrawableRank scoreRank; + private OsuSpriteText nameLabel; + private GlowingSpriteText scoreLabel; + private ScoreComponentLabel maxCombo; + private ScoreComponentLabel accuracy; + private Container flagBadgeContainer; + private FillFlowContainer modsContainer; public LeaderboardScore(Score score, int rank) { @@ -108,7 +52,11 @@ namespace osu.Game.Screens.Select.Leaderboards RelativeSizeAxes = Axes.X; Height = HEIGHT; + } + [BackgroundDependencyLoader] + private void load() + { Children = new Drawable[] { new Container @@ -255,23 +203,51 @@ namespace osu.Game.Screens.Select.Leaderboards Origin = Anchor.BottomRight, AutoSizeAxes = Axes.Both, Direction = FillDirection.Horizontal, + ChildrenEnumerable = Score.Mods.Select(mod => new ModIcon(mod) { Scale = new Vector2(0.375f) }) }, }, }, }, }, }; - - foreach (Mod mod in Score.Mods) - { - modsContainer.Add(new ModIcon(mod) { Scale = new Vector2(0.375f) }); - } } - public void ToggleVisibility() => State = State == Visibility.Visible ? Visibility.Hidden : Visibility.Visible; + public override void Show() + { + foreach (var d in new Drawable[] { avatar, nameLabel, scoreLabel, scoreRank, flagBadgeContainer, maxCombo, accuracy, modsContainer }) + d.FadeOut(); - public override void Hide() => State = Visibility.Hidden; - public override void Show() => State = Visibility.Visible; + Alpha = 0; + + content.MoveToY(75); + avatar.MoveToX(75); + nameLabel.MoveToX(150); + + this.FadeIn(200); + content.MoveToY(0, 800, Easing.OutQuint); + + using (BeginDelayedSequence(100, true)) + { + avatar.FadeIn(300, Easing.OutQuint); + nameLabel.FadeIn(350, Easing.OutQuint); + + avatar.MoveToX(0, 300, Easing.OutQuint); + nameLabel.MoveToX(0, 350, Easing.OutQuint); + + using (BeginDelayedSequence(250, true)) + { + scoreLabel.FadeIn(200); + scoreRank.FadeIn(200); + + using (BeginDelayedSequence(50, true)) + { + var drawables = new Drawable[] { flagBadgeContainer, maxCombo, accuracy, modsContainer, }; + for (int i = 0; i < drawables.Length; i++) + drawables[i].FadeIn(100 + i * 50); + } + } + } + } protected override bool OnHover(InputState state) { From d75e3d8e818e68f8b6e2f6024fcc41a4f37dca32 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 17 Nov 2017 18:35:54 +0900 Subject: [PATCH 2/7] Use lazy for WorkingBeatmap component loading --- osu.Game/Beatmaps/WorkingBeatmap.cs | 129 +++++++++++----------------- 1 file changed, 50 insertions(+), 79 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 959e71d48d..9ae87bc0a7 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -28,16 +28,10 @@ namespace osu.Game.Beatmaps Metadata = beatmapInfo.Metadata ?? BeatmapSetInfo?.Metadata ?? new BeatmapMetadata(); Mods.ValueChanged += mods => applyRateAdjustments(); - } - private void applyRateAdjustments() - { - var t = track; - if (t == null) return; - - t.ResetSpeedAdjustments(); - foreach (var mod in Mods.Value.OfType()) - mod.ApplyToClock(t); + beatmap = new Lazy(populateBeatmap); + background = new Lazy(populateBackground); + track = new Lazy(populateTrack); } protected abstract Beatmap GetBeatmap(); @@ -45,98 +39,75 @@ namespace osu.Game.Beatmaps protected abstract Track GetTrack(); protected virtual Waveform GetWaveform() => new Waveform(); - private Beatmap beatmap; - private readonly object beatmapLock = new object(); - public Beatmap Beatmap + public bool BeatmapLoaded => beatmap.IsValueCreated; + public Beatmap Beatmap => beatmap.Value; + private readonly Lazy beatmap; + + private Beatmap populateBeatmap() { - get - { - lock (beatmapLock) - { - if (beatmap != null) return beatmap; + var b = GetBeatmap() ?? new Beatmap(); - beatmap = GetBeatmap() ?? new Beatmap(); + // use the database-backed info. + b.BeatmapInfo = BeatmapInfo; - // use the database-backed info. - beatmap.BeatmapInfo = BeatmapInfo; - - return beatmap; - } - } + return b; } - private readonly object backgroundLock = new object(); - private Texture background; - public Texture Background + public bool BackgroundLoaded => background.IsValueCreated; + public Texture Background => background.Value; + private Lazy background; + + private Texture populateBackground() => GetBackground(); + + public bool TrackLoaded => track.IsValueCreated; + public Track Track => track.Value; + private Lazy track; + + private Track populateTrack() { - get - { - lock (backgroundLock) - { - return background ?? (background = GetBackground()); - } - } + // we want to ensure that we always have a track, even if it's a fake one. + var t = GetTrack() ?? new TrackVirtual(); + applyRateAdjustments(t); + return t; } - private Track track; - private readonly object trackLock = new object(); - public Track Track - { - get - { - lock (trackLock) - { - if (track != null) return track; + public bool WaveformLoaded => waveform.IsValueCreated; + public Waveform Waveform => waveform.Value; + private Lazy waveform; - // we want to ensure that we always have a track, even if it's a fake one. - track = GetTrack() ?? new TrackVirtual(); - - applyRateAdjustments(); - return track; - } - } - } - - private Waveform waveform; - private readonly object waveformLock = new object(); - public Waveform Waveform - { - get - { - lock (waveformLock) - return waveform ?? (waveform = GetWaveform()); - } - } - - public bool TrackLoaded => track != null; + private Waveform populateWaveform() => GetWaveform(); public void TransferTo(WorkingBeatmap other) { - lock (trackLock) - { - if (track != null && BeatmapInfo.AudioEquals(other.BeatmapInfo)) - other.track = track; - } + if (track.IsValueCreated && track.Value != null && BeatmapInfo.AudioEquals(other.BeatmapInfo)) + other.track = track; - if (background != null && BeatmapInfo.BackgroundEquals(other.BeatmapInfo)) + if (background.IsValueCreated && background.Value != null && BeatmapInfo.BackgroundEquals(other.BeatmapInfo)) other.background = background; } public virtual void Dispose() { - background?.Dispose(); - background = null; - - waveform?.Dispose(); + if (BackgroundLoaded) Background.Dispose(); + if (WaveformLoaded) Waveform.Dispose(); } public void DisposeTrack() { - lock (trackLock) - { - track?.Dispose(); - track = null; - } + if (!track.IsValueCreated) return; + + track.Value?.Dispose(); + track = null; + } + + private void applyRateAdjustments(Track t = null) + { + if (t == null && track.IsValueCreated) t = track.Value; + if (t == null) return; + + t.ResetSpeedAdjustments(); + foreach (var mod in Mods.Value.OfType()) + mod.ApplyToClock(t); } } } From b3aae2340b7dcd4816efe7c860b7ac7725c5f054 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 17 Nov 2017 18:36:24 +0900 Subject: [PATCH 3/7] Avoid accessing beatmaps from BeatSyncedContainer until they are loaded --- osu.Game/Graphics/Containers/BeatSyncedContainer.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Containers/BeatSyncedContainer.cs b/osu.Game/Graphics/Containers/BeatSyncedContainer.cs index 123ef0662d..fb85af12cb 100644 --- a/osu.Game/Graphics/Containers/BeatSyncedContainer.cs +++ b/osu.Game/Graphics/Containers/BeatSyncedContainer.cs @@ -35,9 +35,12 @@ namespace osu.Game.Graphics.Containers protected override void Update() { - var track = Beatmap.Value.Track; + if (!Beatmap.Value.TrackLoaded || !Beatmap.Value.BeatmapLoaded) return; - if (track == null) + var track = Beatmap.Value.Track; + var beatmap = Beatmap.Value.Beatmap; + + if (track == null || beatmap == null) return; double currentTrackTime = track.Length > 0 ? track.CurrentTime + EarlyActivationMilliseconds : Clock.CurrentTime; From 6b591ac77f4d4f2c4c173fcf41248f18ef76c183 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 17 Nov 2017 19:12:30 +0900 Subject: [PATCH 4/7] Add missing initialisation --- osu.Game/Beatmaps/WorkingBeatmap.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 9ae87bc0a7..c9acbefd87 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -32,6 +32,7 @@ namespace osu.Game.Beatmaps beatmap = new Lazy(populateBeatmap); background = new Lazy(populateBackground); track = new Lazy(populateTrack); + waveform = new Lazy(populateWaveform); } protected abstract Beatmap GetBeatmap(); @@ -73,7 +74,7 @@ namespace osu.Game.Beatmaps public bool WaveformLoaded => waveform.IsValueCreated; public Waveform Waveform => waveform.Value; - private Lazy waveform; + private readonly Lazy waveform; private Waveform populateWaveform() => GetWaveform(); From 1a6b0f5b3f235ef02340a072fdc717d5101587bf Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Nov 2017 21:46:43 +0900 Subject: [PATCH 5/7] Update framework --- osu-framework | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu-framework b/osu-framework index c95b9350ed..887db793c7 160000 --- a/osu-framework +++ b/osu-framework @@ -1 +1 @@ -Subproject commit c95b9350edb6305cfefdf08f902f6f73d336736b +Subproject commit 887db793c705b45071aea5e0c1cc931a7887165f From 4ced1b64906507ae871753cc55f63542e4970671 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Nov 2017 22:10:24 +0900 Subject: [PATCH 6/7] Use more of the properties --- osu.Game/Beatmaps/WorkingBeatmap.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index c9acbefd87..cd080b7bbe 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -80,10 +80,10 @@ namespace osu.Game.Beatmaps public void TransferTo(WorkingBeatmap other) { - if (track.IsValueCreated && track.Value != null && BeatmapInfo.AudioEquals(other.BeatmapInfo)) + if (track.IsValueCreated && Track != null && BeatmapInfo.AudioEquals(other.BeatmapInfo)) other.track = track; - if (background.IsValueCreated && background.Value != null && BeatmapInfo.BackgroundEquals(other.BeatmapInfo)) + if (background.IsValueCreated && Background != null && BeatmapInfo.BackgroundEquals(other.BeatmapInfo)) other.background = background; } @@ -95,15 +95,12 @@ namespace osu.Game.Beatmaps public void DisposeTrack() { - if (!track.IsValueCreated) return; - - track.Value?.Dispose(); - track = null; + if (TrackLoaded) Track.Dispose(); } private void applyRateAdjustments(Track t = null) { - if (t == null && track.IsValueCreated) t = track.Value; + if (t == null && track.IsValueCreated) t = Track; if (t == null) return; t.ResetSpeedAdjustments(); From ddf402d9486bc4632bdee6b21216ad9262c6b1dd Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Nov 2017 22:14:28 +0900 Subject: [PATCH 7/7] Add nullchecks in Disposal where population methods return nulls --- osu.Game/Beatmaps/WorkingBeatmap.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index cd080b7bbe..1d4ed75688 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -89,13 +89,13 @@ namespace osu.Game.Beatmaps public virtual void Dispose() { - if (BackgroundLoaded) Background.Dispose(); - if (WaveformLoaded) Waveform.Dispose(); + if (BackgroundLoaded) Background?.Dispose(); + if (WaveformLoaded) Waveform?.Dispose(); } public void DisposeTrack() { - if (TrackLoaded) Track.Dispose(); + if (TrackLoaded) Track?.Dispose(); } private void applyRateAdjustments(Track t = null)