From a60e53c3828c6077e0803b93d0223ccda39e21ad Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Thu, 23 Nov 2017 12:31:18 +0100 Subject: [PATCH 1/9] Changed the text updates into a scheduled (async) operation, and implemented this in the updateStatistics and clearStats methods --- osu.Game/Screens/Select/BeatmapDetails.cs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index d7c509d979..6eaed67534 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -1,6 +1,7 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE +using System; using OpenTK; using OpenTK.Graphics; using osu.Framework.Allocation; @@ -187,9 +188,7 @@ namespace osu.Game.Screens.Select ratingsContainer.FadeIn(transition_duration); advanced.Beatmap = Beatmap; - description.Text = Beatmap.Version; - source.Text = Beatmap.Metadata.Source; - tags.Text = Beatmap.Metadata.Tags; + loadDetailsAsync(Beatmap); var requestedBeatmap = Beatmap; if (requestedBeatmap.Metrics == null) @@ -213,6 +212,19 @@ namespace osu.Game.Screens.Select displayMetrics(requestedBeatmap.Metrics, false); } + private void loadDetailsAsync(BeatmapInfo beatmap) + { + if (description == null || source == null || tags == null) + throw new InvalidOperationException($@"Requires all {nameof(MetadataSection)} elements to be non-null."); + + Schedule(() => + { + description.Text = Beatmap?.Version; + source.Text = Beatmap?.Metadata?.Source; + tags.Text = Beatmap?.Metadata?.Tags; + }); + } + private void displayMetrics(BeatmapMetrics metrics, bool failOnMissing = true) { var hasRatings = metrics?.Ratings?.Any() ?? false; @@ -258,9 +270,7 @@ namespace osu.Game.Screens.Select private void clearStats() { - description.Text = null; - source.Text = null; - tags.Text = null; + loadDetailsAsync(null); advanced.Beatmap = new BeatmapInfo { StarDifficulty = 0, From b34e724b8d17a1fda502683659e2d5b3e4c44d7b Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:18:09 +0100 Subject: [PATCH 2/9] Changed MetadataSection so that the Text setter loads the new text in async before displaying it. --- osu.Game/Screens/Select/BeatmapDetails.cs | 24 ++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 6eaed67534..1b2108a6c8 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -316,7 +316,7 @@ namespace osu.Game.Screens.Select private class MetadataSection : Container { - private readonly TextFlowContainer textFlow; + private TextFlowContainer textFlow; public string Text { @@ -329,11 +329,29 @@ namespace osu.Game.Screens.Select } this.FadeIn(transition_duration); - textFlow.Clear(); - textFlow.AddText(value, s => s.TextSize = 14); + addTextAsync(value); } } + private void addTextAsync(string text) + { + var newTextFlow = new TextFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Colour = textFlow.Colour, + }; + + newTextFlow.AddText(text, s => s.TextSize = 14); + + LoadComponentAsync(newTextFlow, d => + { + var textContainer = (InternalChild as FillFlowContainer); + textContainer.Remove(textFlow); + textContainer.Add(textFlow = d); + }); + } + public Color4 TextColour { get { return textFlow.Colour; } From d49ee295d99e0438fd016c7e00d2bd13255f0d41 Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:47:36 +0100 Subject: [PATCH 3/9] Removed unnecessary schedule and null checks --- osu.Game/Screens/Select/BeatmapDetails.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 1b2108a6c8..f203f5cdc4 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -217,12 +217,9 @@ namespace osu.Game.Screens.Select if (description == null || source == null || tags == null) throw new InvalidOperationException($@"Requires all {nameof(MetadataSection)} elements to be non-null."); - Schedule(() => - { - description.Text = Beatmap?.Version; - source.Text = Beatmap?.Metadata?.Source; - tags.Text = Beatmap?.Metadata?.Tags; - }); + description.Text = beatmap.Version; + source.Text = beatmap.Metadata?.Source; + tags.Text = beatmap.Metadata?.Tags; } private void displayMetrics(BeatmapMetrics metrics, bool failOnMissing = true) From ac0942df8681714a92cddbfb1a146637c02d8b11 Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:48:12 +0100 Subject: [PATCH 4/9] Removed unnecessary private method to update the metadata --- osu.Game/Screens/Select/BeatmapDetails.cs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index f203f5cdc4..892156cc22 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -188,7 +188,9 @@ namespace osu.Game.Screens.Select ratingsContainer.FadeIn(transition_duration); advanced.Beatmap = Beatmap; - loadDetailsAsync(Beatmap); + description.Text = Beatmap.Version; + source.Text = Beatmap.Metadata.Source; + tags.Text = Beatmap.Metadata.Tags; var requestedBeatmap = Beatmap; if (requestedBeatmap.Metrics == null) @@ -212,16 +214,6 @@ namespace osu.Game.Screens.Select displayMetrics(requestedBeatmap.Metrics, false); } - private void loadDetailsAsync(BeatmapInfo beatmap) - { - if (description == null || source == null || tags == null) - throw new InvalidOperationException($@"Requires all {nameof(MetadataSection)} elements to be non-null."); - - description.Text = beatmap.Version; - source.Text = beatmap.Metadata?.Source; - tags.Text = beatmap.Metadata?.Tags; - } - private void displayMetrics(BeatmapMetrics metrics, bool failOnMissing = true) { var hasRatings = metrics?.Ratings?.Any() ?? false; @@ -267,7 +259,10 @@ namespace osu.Game.Screens.Select private void clearStats() { - loadDetailsAsync(null); + description.Text = null; + source.Text = null; + tags.Text = null; + advanced.Beatmap = new BeatmapInfo { StarDifficulty = 0, From 507da0dfb74cf97ae08199e9238a2b078508dd44 Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:48:47 +0100 Subject: [PATCH 5/9] Renamed addTextAsync to setTextAsync --- osu.Game/Screens/Select/BeatmapDetails.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 892156cc22..8bb7b45d7d 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -321,11 +321,11 @@ namespace osu.Game.Screens.Select } this.FadeIn(transition_duration); - addTextAsync(value); + setTextAsync(value); } } - private void addTextAsync(string text) + private void setTextAsync(string text) { var newTextFlow = new TextFlowContainer { From b4513497d68ec54e447e18c1ebe4b8ec6c264a3c Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:52:26 +0100 Subject: [PATCH 6/9] Added a textContainer reference to MetadataSection so casting is no longer required for setting new text --- osu.Game/Screens/Select/BeatmapDetails.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 8bb7b45d7d..2c188319e5 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -308,6 +308,7 @@ namespace osu.Game.Screens.Select private class MetadataSection : Container { + private readonly FillFlowContainer textContainer; private TextFlowContainer textFlow; public string Text @@ -338,7 +339,6 @@ namespace osu.Game.Screens.Select LoadComponentAsync(newTextFlow, d => { - var textContainer = (InternalChild as FillFlowContainer); textContainer.Remove(textFlow); textContainer.Add(textFlow = d); }); @@ -355,7 +355,7 @@ namespace osu.Game.Screens.Select RelativeSizeAxes = Axes.X; AutoSizeAxes = Axes.Y; - InternalChild = new FillFlowContainer + InternalChild = textContainer = new FillFlowContainer { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, From 82a9b57277e72fa83a354de26c493af4d44c6b6d Mon Sep 17 00:00:00 2001 From: FreezyLemon Date: Mon, 27 Nov 2017 08:58:09 +0100 Subject: [PATCH 7/9] Removed unnecessary "using" statement --- osu.Game/Screens/Select/BeatmapDetails.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 2c188319e5..3cd6c3b107 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -1,7 +1,6 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE -using System; using OpenTK; using OpenTK.Graphics; using osu.Framework.Allocation; From 2b7bf285e4bc860074058bf163aac32e9aaf78a1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 29 Nov 2017 20:07:00 +0900 Subject: [PATCH 8/9] Improve code quality --- osu.Game/Screens/Select/BeatmapDetails.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index 3cd6c3b107..eb313319bd 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -320,26 +320,25 @@ namespace osu.Game.Screens.Select return; } - this.FadeIn(transition_duration); setTextAsync(value); } } private void setTextAsync(string text) { - var newTextFlow = new TextFlowContainer + LoadComponentAsync(new TextFlowContainer(s => s.TextSize = 14) { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, Colour = textFlow.Colour, - }; - - newTextFlow.AddText(text, s => s.TextSize = 14); - - LoadComponentAsync(newTextFlow, d => + Text = text + }, loaded => { - textContainer.Remove(textFlow); - textContainer.Add(textFlow = d); + textFlow?.Expire(); + textContainer.Add(textFlow = loaded); + + // fade in if we haven't yet. + this.FadeIn(transition_duration); }); } From b2fc50247c053b1563f69a744664b4b407501e70 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 29 Nov 2017 20:13:00 +0900 Subject: [PATCH 9/9] Hide metadata by default to avoid initial jump when no data present --- osu.Game/Screens/Select/BeatmapDetails.cs | 65 ++++++++++++----------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index eb313319bd..a9a778fe17 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -310,6 +310,39 @@ namespace osu.Game.Screens.Select private readonly FillFlowContainer textContainer; private TextFlowContainer textFlow; + public MetadataSection(string title) + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + Alpha = 0; + + InternalChild = textContainer = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Spacing = new Vector2(spacing / 2), + Children = new Drawable[] + { + new Container + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Child = new OsuSpriteText + { + Text = title, + Font = @"Exo2.0-Bold", + TextSize = 14, + }, + }, + textFlow = new TextFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + }, + }, + }; + } + public string Text { set @@ -347,38 +380,6 @@ namespace osu.Game.Screens.Select get { return textFlow.Colour; } set { textFlow.Colour = value; } } - - public MetadataSection(string title) - { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - - InternalChild = textContainer = new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Spacing = new Vector2(spacing / 2), - Children = new Drawable[] - { - new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Child = new OsuSpriteText - { - Text = title, - Font = @"Exo2.0-Bold", - TextSize = 14, - }, - }, - textFlow = new TextFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - }, - }, - }; - } } private class DimmedLoadingAnimation : VisibilityContainer