From 005ec4b3733cdb3a8197efa9e45dc9f4129c9a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Dec 2019 20:17:51 +0100 Subject: [PATCH 1/4] Demonstrate bug in scrolling container scene Modify TestSceneScrollingHitObjects to showcase the effect of origin choice on object lifetime for all four scrolling directions. --- .../Visual/Gameplay/TestSceneScrollingHitObjects.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs index aa80819694..7ab53b2d65 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs @@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.Gameplay RelativeSizeAxes = Axes.Both, Child = playfields[0] = new TestPlayfield() }, - scrollContainers[1] = new ScrollingTestContainer(ScrollingDirection.Up) + scrollContainers[1] = new ScrollingTestContainer(ScrollingDirection.Down) { RelativeSizeAxes = Axes.Both, Child = playfields[1] = new TestPlayfield() @@ -53,12 +53,12 @@ namespace osu.Game.Tests.Visual.Gameplay }, new Drawable[] { - scrollContainers[2] = new ScrollingTestContainer(ScrollingDirection.Up) + scrollContainers[2] = new ScrollingTestContainer(ScrollingDirection.Left) { RelativeSizeAxes = Axes.Both, Child = playfields[2] = new TestPlayfield() }, - scrollContainers[3] = new ScrollingTestContainer(ScrollingDirection.Up) + scrollContainers[3] = new ScrollingTestContainer(ScrollingDirection.Right) { RelativeSizeAxes = Axes.Both, Child = playfields[3] = new TestPlayfield() @@ -207,7 +207,9 @@ namespace osu.Game.Tests.Visual.Gameplay public TestDrawableHitObject(double time) : base(new HitObject { StartTime = time }) { - Origin = Anchor.Centre; + Origin = Anchor.Custom; + OriginPosition = new Vector2(75 / 4.0f); + AutoSizeAxes = Axes.Both; AddInternal(new Box { Size = new Vector2(75) }); From 193e41f8789c5f1da4acc038569fca9f1a858327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Dec 2019 20:23:16 +0100 Subject: [PATCH 2/4] Add origin adjustment for hitobject lifetime Visual inspection of taiko gameplay has shown that hitobjects appeared on screen only when the origin of the hitobject came into the bounds of the screen, instead of appearing when any visible part of the hitobject came into the screen bounds. This behaviour was due to lifetime calculation being based on the origin of the hitobject and not taking into account the actual object dimensions. Adjust the lifetime start of the hitobject by subtracting the time needed to show the part of the hitobject that should already be visible on screen when the origin comes into frame. --- .../Scrolling/ScrollingHitObjectContainer.cs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs index 857929ff9e..04b4374fc4 100644 --- a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs @@ -100,7 +100,7 @@ namespace osu.Game.Rulesets.UI.Scrolling private void computeLifetimeStartRecursive(DrawableHitObject hitObject) { - hitObject.LifetimeStart = scrollingInfo.Algorithm.GetDisplayStartTime(hitObject.HitObject.StartTime, timeRange.Value); + hitObject.LifetimeStart = computeOriginAdjustedLifetimeStart(hitObject); foreach (var obj in hitObject.NestedHitObjects) computeLifetimeStartRecursive(obj); @@ -108,6 +108,35 @@ namespace osu.Game.Rulesets.UI.Scrolling private readonly Dictionary hitObjectInitialStateCache = new Dictionary(); + private double computeOriginAdjustedLifetimeStart(DrawableHitObject hitObject) + { + float originAdjustment = 0.0f; + + // calculate the dimension of the part of the hitobject that should already be visible + // when the hitobject origin first appears inside the scrolling container + switch (direction.Value) + { + case ScrollingDirection.Up: + originAdjustment = hitObject.OriginPosition.Y; + break; + + case ScrollingDirection.Down: + originAdjustment = hitObject.DrawHeight - hitObject.OriginPosition.Y; + break; + + case ScrollingDirection.Left: + originAdjustment = hitObject.OriginPosition.X; + break; + + case ScrollingDirection.Right: + originAdjustment = hitObject.DrawWidth - hitObject.OriginPosition.X; + break; + } + + var adjustedStartTime = scrollingInfo.Algorithm.TimeAt(-originAdjustment, hitObject.HitObject.StartTime, timeRange.Value, scrollLength); + return scrollingInfo.Algorithm.GetDisplayStartTime(adjustedStartTime, timeRange.Value); + } + // Cant use AddOnce() since the delegate is re-constructed every invocation private void computeInitialStateRecursive(DrawableHitObject hitObject) => hitObject.Schedule(() => { From e2a55b79cac9c4bc9c5a95dbf2122baef3fd3f3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Dec 2019 16:47:46 +0100 Subject: [PATCH 3/4] Refactor scrolling hit object scene To better demonstrate the desired effect of the fix introduced in 193e41f, refactor TestSceneScrollingHitObjects to contain two tests, one of which contains the pre-existing controls to test scroll algorithms, and the other aims to showcase the fix by setting scroll parameters appropriately. --- .../Gameplay/TestSceneScrollingHitObjects.cs | 60 +++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs index 7ab53b2d65..922001f3dd 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs @@ -3,12 +3,14 @@ using System; using System.Collections.Generic; +using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; +using osu.Framework.Threading; using osu.Game.Configuration; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; @@ -28,12 +30,16 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(IReadOnlyList))] private IReadOnlyList mods { get; set; } = Array.Empty(); + private const int spawn_interval = 5000; + private readonly ScrollingTestContainer[] scrollContainers = new ScrollingTestContainer[4]; private readonly TestPlayfield[] playfields = new TestPlayfield[4]; + private ScheduledDelegate hitObjectSpawnDelegate; - public TestSceneScrollingHitObjects() + [SetUp] + public void Setup() { - Add(new GridContainer + Child = new GridContainer { RelativeSizeAxes = Axes.Both, Content = new[] @@ -43,12 +49,14 @@ namespace osu.Game.Tests.Visual.Gameplay scrollContainers[0] = new ScrollingTestContainer(ScrollingDirection.Up) { RelativeSizeAxes = Axes.Both, - Child = playfields[0] = new TestPlayfield() + Child = playfields[0] = new TestPlayfield(), + TimeRange = spawn_interval }, scrollContainers[1] = new ScrollingTestContainer(ScrollingDirection.Down) { RelativeSizeAxes = Axes.Both, - Child = playfields[1] = new TestPlayfield() + Child = playfields[1] = new TestPlayfield(), + TimeRange = spawn_interval }, }, new Drawable[] @@ -56,35 +64,51 @@ namespace osu.Game.Tests.Visual.Gameplay scrollContainers[2] = new ScrollingTestContainer(ScrollingDirection.Left) { RelativeSizeAxes = Axes.Both, - Child = playfields[2] = new TestPlayfield() + Child = playfields[2] = new TestPlayfield(), + TimeRange = spawn_interval }, scrollContainers[3] = new ScrollingTestContainer(ScrollingDirection.Right) { RelativeSizeAxes = Axes.Both, - Child = playfields[3] = new TestPlayfield() + Child = playfields[3] = new TestPlayfield(), + TimeRange = spawn_interval } } } - }); + }; + setUpHitObjects(); + } + + private void setUpHitObjects() + { + scrollContainers.ForEach(c => c.ControlPoints.Add(new MultiplierControlPoint(0))); + + for (int i = 0; i <= spawn_interval; i += 1000) + addHitObject(Time.Current + i); + + hitObjectSpawnDelegate?.Cancel(); + hitObjectSpawnDelegate = Scheduler.AddDelayed(() => addHitObject(Time.Current + spawn_interval), 1000, true); + } + + [Test] + public void TestScrollAlgorithms() + { AddStep("Constant scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Constant)); AddStep("Overlapping scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Overlapping)); AddStep("Sequential scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Sequential)); - AddSliderStep("Time range", 100, 10000, 5000, v => scrollContainers.ForEach(c => c.TimeRange = v)); - AddStep("Add control point", () => addControlPoint(Time.Current + 5000)); + AddSliderStep("Time range", 100, 10000, spawn_interval, v => scrollContainers.Where(c => c != null).ForEach(c => c.TimeRange = v)); + AddStep("Add control point", () => addControlPoint(Time.Current + spawn_interval)); } - protected override void LoadComplete() + [Test] + public void TestScrollLifetime() { - base.LoadComplete(); - - scrollContainers.ForEach(c => c.ControlPoints.Add(new MultiplierControlPoint(0))); - - for (int i = 0; i <= 5000; i += 1000) - addHitObject(Time.Current + i); - - Scheduler.AddDelayed(() => addHitObject(Time.Current + 5000), 1000, true); + AddStep("Set constant scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Constant)); + // scroll container time range must be less than the rate of spawning hitobjects + // otherwise the hitobjects will spawn already partly visible on screen and look wrong + AddStep("Set time range", () => scrollContainers.ForEach(c => c.TimeRange = spawn_interval / 2.0)); } private void addHitObject(double time) From d828b31ae4b1582ef901f06499a83bd2eec3d3b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Dec 2019 17:16:43 +0100 Subject: [PATCH 4/4] Schedule child mutation in test setup --- .../Visual/Gameplay/TestSceneScrollingHitObjects.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs index 922001f3dd..8629522dc2 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs @@ -37,7 +37,7 @@ namespace osu.Game.Tests.Visual.Gameplay private ScheduledDelegate hitObjectSpawnDelegate; [SetUp] - public void Setup() + public void Setup() => Schedule(() => { Child = new GridContainer { @@ -78,7 +78,7 @@ namespace osu.Game.Tests.Visual.Gameplay }; setUpHitObjects(); - } + }); private void setUpHitObjects() {