From 013683c23ba1d02bf348ae6288d56b253adc9385 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 May 2020 00:17:13 +0900 Subject: [PATCH 1/5] Fix taiko rim markers incorrectly playing as whistle samples --- osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index d332f90cd4..1e1f9ae09b 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -52,7 +52,12 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables protected override IEnumerable GetSamples() { // normal and claps are always handled by the drum (see DrumSampleMapping). - var samples = HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); + // in addition, whistles are excluded as they are an alternative rim marker. + + var samples = HitObject.Samples.Where(s => + s.Name != HitSampleInfo.HIT_NORMAL + && s.Name != HitSampleInfo.HIT_CLAP + && s.Name != HitSampleInfo.HIT_WHISTLE); if (HitObject.Type == HitType.Rim && HitObject.IsStrong) { From e9710b6f836578850646db5e78e06f8985066a35 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 May 2020 09:43:05 +0900 Subject: [PATCH 2/5] Add taiko type conversion test coverage --- .../TaikoBeatmapConversionTest.cs | 3 +- ...-type-conversions-expected-conversion.json | 116 ++++++++++++++++++ .../Beatmaps/sample-to-type-conversions.osu | 62 ++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions-expected-conversion.json create mode 100644 osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions.osu diff --git a/osu.Game.Rulesets.Taiko.Tests/TaikoBeatmapConversionTest.cs b/osu.Game.Rulesets.Taiko.Tests/TaikoBeatmapConversionTest.cs index 8c26ca70ac..f7729138ff 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TaikoBeatmapConversionTest.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TaikoBeatmapConversionTest.cs @@ -19,6 +19,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [NonParallelizable] [TestCase("basic")] [TestCase("slider-generating-drumroll")] + [TestCase("sample-to-type-conversions")] public void Test(string name) => base.Test(name); protected override IEnumerable CreateConvertValue(HitObject hitObject) @@ -41,7 +42,7 @@ namespace osu.Game.Rulesets.Taiko.Tests public struct ConvertValue : IEquatable { /// - /// A sane value to account for osu!stable using ints everwhere. + /// A sane value to account for osu!stable using ints everywhere. /// private const float conversion_lenience = 2; diff --git a/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions-expected-conversion.json b/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions-expected-conversion.json new file mode 100644 index 0000000000..47ca6aef68 --- /dev/null +++ b/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions-expected-conversion.json @@ -0,0 +1,116 @@ +{ + "Mappings": [ + { + "StartTime": 110.0, + "Objects": [ + { + "StartTime": 110.0, + "EndTime": 110.0, + "IsRim": false, + "IsCentre": true, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": false + } + ] + }, + { + "StartTime": 538.0, + "Objects": [ + { + "StartTime": 538.0, + "EndTime": 538.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": false + } + ] + }, + { + "StartTime": 967.0, + "Objects": [ + { + "StartTime": 967.0, + "EndTime": 967.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": false + } + ] + }, + { + "StartTime": 1395.0, + "Objects": [ + { + "StartTime": 1395.0, + "EndTime": 1395.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": false + } + ] + }, + { + "StartTime": 1824.0, + "Objects": [ + { + "StartTime": 1824.0, + "EndTime": 1824.0, + "IsRim": false, + "IsCentre": true, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": true + } + ] + }, + { + "StartTime": 2252.0, + "Objects": [ + { + "StartTime": 2252.0, + "EndTime": 2252.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": true + } + ] + }, + { + "StartTime": 2681.0, + "Objects": [ + { + "StartTime": 2681.0, + "EndTime": 2681.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": true + } + ] + }, + { + "StartTime": 3110.0, + "Objects": [ + { + "StartTime": 3110.0, + "EndTime": 3110.0, + "IsRim": true, + "IsCentre": false, + "IsDrumRoll": false, + "IsSwell": false, + "IsStrong": true + } + ] + } + ] +} \ No newline at end of file diff --git a/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions.osu b/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions.osu new file mode 100644 index 0000000000..a3537e7149 --- /dev/null +++ b/osu.Game.Rulesets.Taiko/Resources/Testing/Beatmaps/sample-to-type-conversions.osu @@ -0,0 +1,62 @@ +osu file format v14 + +[General] +AudioFilename: audio.mp3 +AudioLeadIn: 0 +PreviewTime: -1 +Countdown: 0 +SampleSet: Normal +StackLeniency: 0.5 +Mode: 1 +LetterboxInBreaks: 0 +WidescreenStoryboard: 1 + +[Editor] +Bookmarks: 110,13824,54967,82395,109824 +DistanceSpacing: 0.1 +BeatDivisor: 4 +GridSize: 32 +TimelineZoom: 3.099999 + +[Metadata] +Title:test +TitleUnicode:test +Artist:sample conversion +ArtistUnicode:sample conversion +Creator:banchobot +Version:sample test +Source: +Tags: +BeatmapID:0 +BeatmapSetID:-1 + +[Difficulty] +HPDrainRate:6 +CircleSize:2 +OverallDifficulty:6 +ApproachRate:7 +SliderMultiplier:1.4 +SliderTickRate:4 + +[Events] +//Background and Video events +//Break Periods +//Storyboard Layer 0 (Background) +//Storyboard Layer 1 (Fail) +//Storyboard Layer 2 (Pass) +//Storyboard Layer 3 (Foreground) +//Storyboard Layer 4 (Overlay) +//Storyboard Sound Samples + +[TimingPoints] +110,428.571428571429,4,1,0,100,1,0 + +[HitObjects] +256,192,110,5,0,0:0:0:0: +256,192,538,1,8,0:0:0:0: +256,192,967,1,2,0:0:0:0: +256,192,1395,1,10,0:0:0:0: +256,192,1824,1,4,0:0:0:0: +256,192,2252,1,12,0:0:0:0: +256,192,2681,1,6,0:0:0:0: +256,192,3110,1,14,0:0:0:0: From db4e3047ddf02cb09a7603c68a0ab854e1cfe6b2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 May 2020 23:28:13 +0900 Subject: [PATCH 3/5] Add test for final sample output --- .../TestSceneSampleOutput.cs | 47 +++++++++++++++++++ .../Objects/Drawables/DrawableHit.cs | 2 +- .../Drawables/DrawableTaikoHitObject.cs | 2 +- .../Objects/Drawables/DrawableHitObject.cs | 2 +- .../Tests/Beatmaps/BeatmapConversionTest.cs | 11 +++-- 5 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs new file mode 100644 index 0000000000..564ab91291 --- /dev/null +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs @@ -0,0 +1,47 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using osu.Game.Audio; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Taiko.Objects.Drawables; +using osu.Game.Tests.Visual; + +namespace osu.Game.Rulesets.Taiko.Tests +{ + /// + /// Taiko has some interesting rules for legacy mappings. + /// + public class TestSceneSampleOutput : PlayerTestScene + { + public TestSceneSampleOutput() + : base(new TaikoRuleset()) + { + } + + public override void SetUpSteps() + { + base.SetUpSteps(); + AddAssert("has correct samples", () => + { + var names = Player.DrawableRuleset.Playfield.AllHitObjects.OfType().Select(h => string.Join(',', h.GetSamples().Select(s => s.Name))); + + var expected = new[] + { + string.Empty, + string.Empty, + string.Empty, + string.Empty, + HitSampleInfo.HIT_FINISH, + HitSampleInfo.HIT_WHISTLE, + HitSampleInfo.HIT_WHISTLE, + HitSampleInfo.HIT_WHISTLE, + }; + + return names.SequenceEqual(expected); + }); + } + + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TaikoBeatmapConversionTest().GetBeatmap("sample-to-type-conversions"); + } +} diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index 1e1f9ae09b..81b969eaf3 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -49,7 +49,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables ? new SkinnableDrawable(new TaikoSkinComponent(TaikoSkinComponents.CentreHit), _ => new CentreHitCirclePiece(), confineMode: ConfineMode.ScaleToFit) : new SkinnableDrawable(new TaikoSkinComponent(TaikoSkinComponents.RimHit), _ => new RimHitCirclePiece(), confineMode: ConfineMode.ScaleToFit); - protected override IEnumerable GetSamples() + public override IEnumerable GetSamples() { // normal and claps are always handled by the drum (see DrumSampleMapping). // in addition, whistles are excluded as they are an alternative rim marker. diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index 90daf3950c..3ab09d4cbe 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -166,7 +166,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables } // Most osu!taiko hitsounds are managed by the drum (see DrumSampleMapping). - protected override IEnumerable GetSamples() => Enumerable.Empty(); + public override IEnumerable GetSamples() => Enumerable.Empty(); protected abstract SkinnableDrawable CreateMainPiece(); diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index ba6571fe1a..33ea02c22f 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -36,7 +36,7 @@ namespace osu.Game.Rulesets.Objects.Drawables protected SkinnableSound Samples { get; private set; } - protected virtual IEnumerable GetSamples() => HitObject.Samples; + public virtual IEnumerable GetSamples() => HitObject.Samples; private readonly Lazy> nestedHitObjects = new Lazy>(); public IReadOnlyList NestedHitObjects => nestedHitObjects.IsValueCreated ? nestedHitObjects.Value : (IReadOnlyList)Array.Empty(); diff --git a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs index b60add6e3b..06e82394ec 100644 --- a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs +++ b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs @@ -99,7 +99,7 @@ namespace osu.Game.Tests.Beatmaps private ConvertResult convert(string name, Mod[] mods) { - var beatmap = getBeatmap(name); + var beatmap = GetBeatmap(name); var rulesetInstance = CreateRuleset(); beatmap.BeatmapInfo.Ruleset = beatmap.BeatmapInfo.RulesetID == rulesetInstance.RulesetInfo.ID ? rulesetInstance.RulesetInfo : new RulesetInfo(); @@ -143,14 +143,19 @@ namespace osu.Game.Tests.Beatmaps } } - private IBeatmap getBeatmap(string name) + public IBeatmap GetBeatmap(string name) { using (var resStream = openResource($"{resource_namespace}.{name}.osu")) using (var stream = new LineBufferedReader(resStream)) { var decoder = Decoder.GetDecoder(stream); ((LegacyBeatmapDecoder)decoder).ApplyOffsets = false; - return decoder.Decode(stream); + var beatmap = decoder.Decode(stream); + + // not sure but seems to be required. + beatmap.BeatmapInfo.Ruleset = CreateRuleset().RulesetInfo; + + return beatmap; } } From 963806474148d80ddc706e0f47f8bf5d8a22e6a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 May 2020 10:06:23 +0900 Subject: [PATCH 4/5] Tidy up ruleset assignment code --- osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs index 06e82394ec..6ada632850 100644 --- a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs +++ b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs @@ -101,9 +101,6 @@ namespace osu.Game.Tests.Beatmaps { var beatmap = GetBeatmap(name); - var rulesetInstance = CreateRuleset(); - beatmap.BeatmapInfo.Ruleset = beatmap.BeatmapInfo.RulesetID == rulesetInstance.RulesetInfo.ID ? rulesetInstance.RulesetInfo : new RulesetInfo(); - var converterResult = new Dictionary>(); var working = new ConversionWorkingBeatmap(beatmap) @@ -115,7 +112,7 @@ namespace osu.Game.Tests.Beatmaps } }; - working.GetPlayableBeatmap(rulesetInstance.RulesetInfo, mods); + working.GetPlayableBeatmap(CreateRuleset().RulesetInfo, mods); return new ConvertResult { @@ -152,8 +149,8 @@ namespace osu.Game.Tests.Beatmaps ((LegacyBeatmapDecoder)decoder).ApplyOffsets = false; var beatmap = decoder.Decode(stream); - // not sure but seems to be required. - beatmap.BeatmapInfo.Ruleset = CreateRuleset().RulesetInfo; + var rulesetInstance = CreateRuleset(); + beatmap.BeatmapInfo.Ruleset = beatmap.BeatmapInfo.RulesetID == rulesetInstance.RulesetInfo.ID ? rulesetInstance.RulesetInfo : new RulesetInfo(); return beatmap; } From 76080368e900f2b866f46235eebff0f7bda19a6d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 May 2020 10:14:08 +0900 Subject: [PATCH 5/5] Mark test as headless --- osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs index 564ab91291..d541aa8de8 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneSampleOutput.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using osu.Framework.Testing; using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Rulesets.Taiko.Objects.Drawables; @@ -12,6 +13,7 @@ namespace osu.Game.Rulesets.Taiko.Tests /// /// Taiko has some interesting rules for legacy mappings. /// + [HeadlessTest] public class TestSceneSampleOutput : PlayerTestScene { public TestSceneSampleOutput()