From 32b068fbdc93d3298bf3257c7d6aa9e6b9749119 Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 4 Jul 2021 21:50:58 +0200 Subject: [PATCH 1/5] Fix typo causing nested windows to be ignored --- osu.Game/Rulesets/UI/DrawableRuleset.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index 8dcc1ca164..cccd73f19f 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -496,7 +496,7 @@ namespace osu.Game.Rulesets.UI foreach (var n in h.NestedHitObjects) { - if (h.HitWindows.WindowFor(HitResult.Miss) > 0) + if (n.HitWindows.WindowFor(HitResult.Miss) > 0) return n.HitWindows; } } From 6d2ffe3a94ec9edab261682d16887e6dd519c70e Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 4 Jul 2021 22:51:35 +0200 Subject: [PATCH 2/5] Add basic tests --- .../NonVisual/FirstAvailableHitWindowTest.cs | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs diff --git a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs new file mode 100644 index 0000000000..92766d8334 --- /dev/null +++ b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs @@ -0,0 +1,121 @@ +// 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.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using NUnit.Framework; +using osu.Framework.Graphics.Containers; +using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Scoring; +using osu.Game.Rulesets.UI; +using osu.Game.Scoring; + +namespace osu.Game.Tests.NonVisual +{ + public class FirstAvailableHitWindowsTest + { + private TestDrawableRuleset testDrawableRuleset = new TestDrawableRuleset(); + + [Test] + public void TestResultIfOnlyParentHitWindowIsEmpty() + { + var testObject = new TestHitObject(hitWindows: HitWindows.Empty); + HitObject nested = new TestHitObject(hitWindows: new HitWindows()); + testObject.AddNested(nested); + testDrawableRuleset.HitObjects = new List { testObject }; + + // If the parent window is empty, but its nested object isn't, return the nested object + Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, nested.HitWindows); + } + + [Test] + public void TestResultIfParentHitWindowsIsNotEmpty() + { + var testObject = new TestHitObject(hitWindows: new HitWindows()); + HitObject nested = new TestHitObject(hitWindows: new HitWindows()); + testObject.AddNested(nested); + testDrawableRuleset.HitObjects = new List { testObject }; + + // If the parent window is not empty, return that immediately + Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, testObject.HitWindows); + } + + [Test] + public void TestResultIfParentAndChildHitWindowsAreEmpty() + { + var firstObject = new TestHitObject(hitWindows: HitWindows.Empty); + HitObject nested = new TestHitObject(hitWindows: HitWindows.Empty); + firstObject.AddNested(nested); + + var secondObject = new TestHitObject(hitWindows: new HitWindows()); + testDrawableRuleset.HitObjects = new List { firstObject, secondObject }; + + // If the parent and child windows are empty, return the next object if window isn't empty + Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, secondObject.HitWindows); + } + + [Test] + public void TestResultIfAllHitWindowsAreEmpty() + { + var firstObject = new TestHitObject(hitWindows: HitWindows.Empty); + HitObject nested = new TestHitObject(hitWindows: HitWindows.Empty); + firstObject.AddNested(nested); + + testDrawableRuleset.HitObjects = new List { firstObject }; + + // If all windows are empty, this should return null + Assert.IsNull(testDrawableRuleset.FirstAvailableHitWindows); + } + + [SuppressMessage("ReSharper", "UnassignedGetOnlyAutoProperty")] + private class TestDrawableRuleset : DrawableRuleset + { + public List HitObjects; + public override IEnumerable Objects => HitObjects; + + public override event Action NewResult; + public override event Action RevertResult; + + public override Playfield Playfield { get; } + public override Container Overlays { get; } + public override Container FrameStableComponents { get; } + public override IFrameStableClock FrameStableClock { get; } + internal override bool FrameStablePlayback { get; set; } + public override IReadOnlyList Mods { get; } + + public override double GameplayStartTime { get; } + public override GameplayCursorContainer Cursor { get; } + + public TestDrawableRuleset() + : base(new OsuRuleset()) + { + // won't compile without this. + NewResult?.Invoke(null); + RevertResult?.Invoke(null); + } + + public override void SetReplayScore(Score replayScore) => throw new NotImplementedException(); + + public override void SetRecordTarget(Score score) => throw new NotImplementedException(); + + public override void RequestResume(Action continueResume) => throw new NotImplementedException(); + + public override void CancelResume() => throw new NotImplementedException(); + } + } + + public class TestHitObject : HitObject + { + public TestHitObject(HitWindows hitWindows) + { + HitWindows = hitWindows; + HitWindows.SetDifficulty(0.5f); + } + + public new void AddNested(HitObject nested) => base.AddNested(nested); + } +} From 1facdcf4838c4948f03d58c8b23fcf8dd5cd8759 Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 4 Jul 2021 23:23:24 +0200 Subject: [PATCH 3/5] Apply changes to tests --- .../NonVisual/FirstAvailableHitWindowTest.cs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs index 92766d8334..2f1ad194bb 100644 --- a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs +++ b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs @@ -23,51 +23,47 @@ namespace osu.Game.Tests.NonVisual [Test] public void TestResultIfOnlyParentHitWindowIsEmpty() { - var testObject = new TestHitObject(hitWindows: HitWindows.Empty); - HitObject nested = new TestHitObject(hitWindows: new HitWindows()); + var testObject = new TestHitObject(HitWindows.Empty); + HitObject nested = new TestHitObject(new HitWindows()); testObject.AddNested(nested); testDrawableRuleset.HitObjects = new List { testObject }; - // If the parent window is empty, but its nested object isn't, return the nested object Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, nested.HitWindows); } [Test] public void TestResultIfParentHitWindowsIsNotEmpty() { - var testObject = new TestHitObject(hitWindows: new HitWindows()); - HitObject nested = new TestHitObject(hitWindows: new HitWindows()); + var testObject = new TestHitObject(new HitWindows()); + HitObject nested = new TestHitObject(new HitWindows()); testObject.AddNested(nested); testDrawableRuleset.HitObjects = new List { testObject }; - // If the parent window is not empty, return that immediately Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, testObject.HitWindows); } [Test] public void TestResultIfParentAndChildHitWindowsAreEmpty() { - var firstObject = new TestHitObject(hitWindows: HitWindows.Empty); - HitObject nested = new TestHitObject(hitWindows: HitWindows.Empty); + var firstObject = new TestHitObject(HitWindows.Empty); + HitObject nested = new TestHitObject(HitWindows.Empty); firstObject.AddNested(nested); - var secondObject = new TestHitObject(hitWindows: new HitWindows()); + var secondObject = new TestHitObject(new HitWindows()); testDrawableRuleset.HitObjects = new List { firstObject, secondObject }; - // If the parent and child windows are empty, return the next object if window isn't empty Assert.AreSame(testDrawableRuleset.FirstAvailableHitWindows, secondObject.HitWindows); } [Test] public void TestResultIfAllHitWindowsAreEmpty() { - var firstObject = new TestHitObject(hitWindows: HitWindows.Empty); - HitObject nested = new TestHitObject(hitWindows: HitWindows.Empty); + var firstObject = new TestHitObject(HitWindows.Empty); + HitObject nested = new TestHitObject(HitWindows.Empty); firstObject.AddNested(nested); testDrawableRuleset.HitObjects = new List { firstObject }; - // If all windows are empty, this should return null Assert.IsNull(testDrawableRuleset.FirstAvailableHitWindows); } From 216e52d6d0333d730ef13efdbd09534ee0c59e62 Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 4 Jul 2021 23:24:17 +0200 Subject: [PATCH 4/5] Avoid using single letter variable names --- osu.Game/Rulesets/UI/DrawableRuleset.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index cccd73f19f..daf46dcdcc 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -489,15 +489,15 @@ namespace osu.Game.Rulesets.UI { get { - foreach (var h in Objects) + foreach (var hitObject in Objects) { - if (h.HitWindows.WindowFor(HitResult.Miss) > 0) - return h.HitWindows; + if (hitObject.HitWindows.WindowFor(HitResult.Miss) > 0) + return hitObject.HitWindows; - foreach (var n in h.NestedHitObjects) + foreach (var nested in hitObject.NestedHitObjects) { - if (n.HitWindows.WindowFor(HitResult.Miss) > 0) - return n.HitWindows; + if (nested.HitWindows.WindowFor(HitResult.Miss) > 0) + return nested.HitWindows; } } From cc877f88e2fd97dcd0556e233067a7cc0e2b9d57 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 5 Jul 2021 10:13:01 +0900 Subject: [PATCH 5/5] Fix inspection (create a new ruleset every time) --- osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs index 2f1ad194bb..aed2a5c122 100644 --- a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs +++ b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowTest.cs @@ -18,7 +18,13 @@ namespace osu.Game.Tests.NonVisual { public class FirstAvailableHitWindowsTest { - private TestDrawableRuleset testDrawableRuleset = new TestDrawableRuleset(); + private TestDrawableRuleset testDrawableRuleset; + + [SetUp] + public void Setup() + { + testDrawableRuleset = new TestDrawableRuleset(); + } [Test] public void TestResultIfOnlyParentHitWindowIsEmpty()