From 1d2db85866906bf17e56f93a488d80df49ed5adf Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 7 May 2019 17:23:44 +0900 Subject: [PATCH 1/2] Improve background sprite testcase --- ...stCaseUpdateableBeatmapBackgroundSprite.cs | 67 ++++++++++++++----- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs b/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs index 74114b2e53..dc0aff420a 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs @@ -2,8 +2,8 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Drawables; @@ -11,12 +11,15 @@ using osu.Game.Online.API; using osu.Game.Online.API.Requests; using osu.Game.Rulesets; using osu.Game.Tests.Beatmaps.IO; +using osuTK; namespace osu.Game.Tests.Visual.UserInterface { public class TestCaseUpdateableBeatmapBackgroundSprite : OsuTestCase { - private TestUpdateableBeatmapBackgroundSprite backgroundSprite; + private BeatmapSetInfo testBeatmap; + private IAPIProvider api; + private RulesetStore rulesets; [Resolved] private BeatmapManager beatmaps { get; set; } @@ -24,40 +27,68 @@ namespace osu.Game.Tests.Visual.UserInterface [BackgroundDependencyLoader] private void load(OsuGameBase osu, IAPIProvider api, RulesetStore rulesets) { - Bindable beatmapBindable = new Bindable(); + this.api = api; + this.rulesets = rulesets; - var imported = ImportBeatmapTest.LoadOszIntoOsu(osu); + testBeatmap = ImportBeatmapTest.LoadOszIntoOsu(osu); + } - Child = backgroundSprite = new TestUpdateableBeatmapBackgroundSprite { RelativeSizeAxes = Axes.Both }; + [Test] + public void TestNullBeatmap() + { + TestUpdateableBeatmapBackgroundSprite background = null; - backgroundSprite.Beatmap.BindTo(beatmapBindable); + AddStep("load null beatmap", () => Child = background = new TestUpdateableBeatmapBackgroundSprite { RelativeSizeAxes = Axes.Both }); + AddUntilStep("wait for load", () => background.ContentLoaded); + } - var req = new GetBeatmapSetRequest(1); - api.Queue(req); + [Test] + public void TestLocalBeatmap() + { + TestUpdateableBeatmapBackgroundSprite background = null; - AddStep("load null beatmap", () => beatmapBindable.Value = null); - AddUntilStep("wait for cleanup...", () => backgroundSprite.ChildCount == 1); - AddStep("load imported beatmap", () => beatmapBindable.Value = imported.Beatmaps.First()); - AddUntilStep("wait for cleanup...", () => backgroundSprite.ChildCount == 1); + AddStep("load local beatmap", () => + { + Child = background = new TestUpdateableBeatmapBackgroundSprite + { + RelativeSizeAxes = Axes.Both, + Beatmap = { Value = testBeatmap.Beatmaps.First() } + }; + }); + AddUntilStep("wait for load", () => background.ContentLoaded); + } + + [Test] + public void TestOnlineBeatmap() + { if (api.IsLoggedIn) { + var req = new GetBeatmapSetRequest(1); + api.Queue(req); + AddUntilStep("wait for api response", () => req.Result != null); - AddStep("load online beatmap", () => beatmapBindable.Value = new BeatmapInfo + + TestUpdateableBeatmapBackgroundSprite background = null; + + AddStep("load online beatmap", () => { - BeatmapSet = req.Result?.ToBeatmapSet(rulesets) + Child = background = new TestUpdateableBeatmapBackgroundSprite + { + RelativeSizeAxes = Axes.Both, + Beatmap = { Value = new BeatmapInfo { BeatmapSet = req.Result?.ToBeatmapSet(rulesets) } } + }; }); - AddUntilStep("wait for cleanup...", () => backgroundSprite.ChildCount == 1); + + AddUntilStep("wait for load", () => background.ContentLoaded); } else - { AddStep("online (login first)", () => { }); - } } private class TestUpdateableBeatmapBackgroundSprite : UpdateableBeatmapBackgroundSprite { - public int ChildCount => InternalChildren.Count; + public bool ContentLoaded => ((DelayedLoadUnloadWrapper)InternalChildren.LastOrDefault())?.Content?.IsLoaded ?? false; } } } From a00e2b18a9034e5f1736d1b1c99489d8c1c47463 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 7 May 2019 17:24:05 +0900 Subject: [PATCH 2/2] Fix background unloading/reloading sometimes crashing --- ...stCaseUpdateableBeatmapBackgroundSprite.cs | 55 +++++++++++++++++++ .../UpdateableBeatmapBackgroundSprite.cs | 9 ++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs b/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs index dc0aff420a..a4dd7b83e2 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestCaseUpdateableBeatmapBackgroundSprite.cs @@ -1,10 +1,12 @@ // 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 System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Drawables; using osu.Game.Online.API; @@ -86,8 +88,61 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("online (login first)", () => { }); } + [Test] + public void TestUnloadAndReload() + { + var backgrounds = new List(); + ScrollContainer scrollContainer = null; + + AddStep("create backgrounds hierarchy", () => + { + FillFlowContainer backgroundFlow; + + Child = scrollContainer = new ScrollContainer + { + Size = new Vector2(500), + Child = backgroundFlow = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Spacing = new Vector2(10), + Padding = new MarginPadding { Bottom = 250 } + } + }; + + for (int i = 0; i < 25; i++) + { + var background = new TestUpdateableBeatmapBackgroundSprite { RelativeSizeAxes = Axes.Both }; + + if (i % 2 == 0) + background.Beatmap.Value = testBeatmap.Beatmaps.First(); + + backgroundFlow.Add(new Container + { + RelativeSizeAxes = Axes.X, + Height = 100, + Masking = true, + Child = background + }); + + backgrounds.Add(background); + } + }); + + var loadedBackgrounds = backgrounds.Where(b => b.ContentLoaded); + + int initialLoadCount = 0; + + AddUntilStep("some loaded", () => (initialLoadCount = loadedBackgrounds.Count()) > 0); + AddStep("scroll to bottom", () => scrollContainer.ScrollToEnd()); + AddUntilStep("some unloaded", () => loadedBackgrounds.Count() < initialLoadCount); + } + private class TestUpdateableBeatmapBackgroundSprite : UpdateableBeatmapBackgroundSprite { + protected override double UnloadDelay => 2000; + public bool ContentLoaded => ((DelayedLoadUnloadWrapper)InternalChildren.LastOrDefault())?.Content?.IsLoaded ?? false; } } diff --git a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs index ce7811fc0d..bd80919851 100644 --- a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs @@ -26,6 +26,11 @@ namespace osu.Game.Beatmaps.Drawables this.beatmapSetCoverType = beatmapSetCoverType; } + /// + /// Delay before the background is unloaded while off-screen. + /// + protected virtual double UnloadDelay => 10000; + private BeatmapInfo lastModel; protected override DelayedLoadWrapper CreateDelayedLoadWrapper(Drawable content, double timeBeforeLoad) @@ -34,13 +39,13 @@ namespace osu.Game.Beatmaps.Drawables { // If DelayedLoadUnloadWrapper is attempting to RELOAD the same content (Beatmap), that means that it was // previously UNLOADED and thus its children have been disposed of, so we need to recreate them here. - if (lastModel == Beatmap.Value && Beatmap.Value != null) + if (lastModel == Beatmap.Value) return CreateDrawable(Beatmap.Value); // If the model has changed since the previous unload (or if there was no load), then we can safely use the given content lastModel = Beatmap.Value; return content; - }, timeBeforeLoad, 10000); + }, timeBeforeLoad, UnloadDelay); } protected override Drawable CreateDrawable(BeatmapInfo model)