From b4d790c076904a1bd24bd086fae7d159ccd06614 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 12:53:54 +0900 Subject: [PATCH 1/5] Fix taiko sample mapping for strong hits --- .../Beatmaps/TaikoBeatmapConverter.cs | 30 +++++++++++++++++-- .../Objects/Drawables/DrawableHit.cs | 8 +++++ .../Drawables/DrawableTaikoHitObject.cs | 4 +-- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs index caf645d5a2..822931396a 100644 --- a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs +++ b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs @@ -167,13 +167,39 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps default: { - bool isRim = samples.Any(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE); + bool isRimDefinition(HitSampleInfo s) => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE; + + bool isRim = samples.Any(isRimDefinition); + + if (isRim) + { + // consume then remove the rim definition sample types. + var updatedSamples = samples.Where(s => !isRimDefinition(s)).ToList(); + + // strong + rim always maps to whistle. + if (strong) + { + for (var i = 0; i < updatedSamples.Count; i++) + { + var s = samples[i]; + + if (s.Name != HitSampleInfo.HIT_FINISH) + continue; + + var sClone = s.Clone(); + sClone.Name = HitSampleInfo.HIT_WHISTLE; + updatedSamples[i] = sClone; + } + } + + samples = updatedSamples; + } yield return new Hit { StartTime = obj.StartTime, Type = isRim ? HitType.Rim : HitType.Centre, - Samples = obj.Samples, + Samples = samples, IsStrong = strong }; diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index d2671eadda..d4dc3316e7 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -2,9 +2,11 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using osu.Framework.Graphics; +using osu.Game.Audio; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.Taiko.Objects.Drawables.Pieces; @@ -47,6 +49,12 @@ 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() + { + // normal and claps are always handled by the drum (see DrumSampleMapping). + return HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); + } + protected override void CheckForResult(bool userTriggered, double timeOffset) { Debug.Assert(HitObject.HitWindows != null); diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index 1be04f1760..90daf3950c 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -165,8 +165,8 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables return base.CreateNestedHitObject(hitObject); } - // Normal and clap samples are handled by the drum - protected override IEnumerable GetSamples() => HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); + // Most osu!taiko hitsounds are managed by the drum (see DrumSampleMapping). + protected override IEnumerable GetSamples() => Enumerable.Empty(); protected abstract SkinnableDrawable CreateMainPiece(); From 77041bdbb571a738da0c22f10948059642502bb2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 16:19:47 +0900 Subject: [PATCH 2/5] Move implementation to DrawableHit to avoid "breaking" legacy encoding --- .../Beatmaps/TaikoBeatmapConverter.cs | 24 ----------------- .../Objects/Drawables/DrawableHit.cs | 27 ++++++++++++++++++- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs index 822931396a..d324441285 100644 --- a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs +++ b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs @@ -171,30 +171,6 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps bool isRim = samples.Any(isRimDefinition); - if (isRim) - { - // consume then remove the rim definition sample types. - var updatedSamples = samples.Where(s => !isRimDefinition(s)).ToList(); - - // strong + rim always maps to whistle. - if (strong) - { - for (var i = 0; i < updatedSamples.Count; i++) - { - var s = samples[i]; - - if (s.Name != HitSampleInfo.HIT_FINISH) - continue; - - var sClone = s.Clone(); - sClone.Name = HitSampleInfo.HIT_WHISTLE; - updatedSamples[i] = sClone; - } - } - - samples = updatedSamples; - } - yield return new Hit { StartTime = obj.StartTime, diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index d4dc3316e7..d332f90cd4 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -52,7 +52,32 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables protected override IEnumerable GetSamples() { // normal and claps are always handled by the drum (see DrumSampleMapping). - return HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); + var samples = HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); + + if (HitObject.Type == HitType.Rim && HitObject.IsStrong) + { + // strong + rim always maps to whistle. + // TODO: this should really be in the legacy decoder, but can't be because legacy encoding parity would be broken. + // when we add a taiko editor, this is probably not going to play nice. + + var corrected = samples.ToList(); + + for (var i = 0; i < corrected.Count; i++) + { + var s = corrected[i]; + + if (s.Name != HitSampleInfo.HIT_FINISH) + continue; + + var sClone = s.Clone(); + sClone.Name = HitSampleInfo.HIT_WHISTLE; + corrected[i] = sClone; + } + + return corrected; + } + + return samples; } protected override void CheckForResult(bool userTriggered, double timeOffset) From 93440874db8d92e300d0e62508e5cf44e343f4bb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 16:30:54 +0900 Subject: [PATCH 3/5] Refactor beatmap encoder test to be a bit easier to understand --- .../Formats/LegacyBeatmapEncoderTest.cs | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index bcc873b0b7..64efe08929 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -28,14 +28,15 @@ namespace osu.Game.Tests.Beatmaps.Formats private static IEnumerable allBeatmaps => TestResources.GetStore().GetAvailableResources().Where(res => res.EndsWith(".osu")); [TestCaseSource(nameof(allBeatmaps))] - public void TestBeatmap(string name) + public void TestEncodeDecodeStability(string name) { - var decoded = decode(name, out var encoded); + var decoded = decodeFromLegacy(TestResources.GetStore().GetStream(name)); + var decodedAfterEncode = decodeFromLegacy(encodeToLegacy(decoded)); sort(decoded); - sort(encoded); + sort(decodedAfterEncode); - Assert.That(encoded.Serialize(), Is.EqualTo(decoded.Serialize())); + Assert.That(decodedAfterEncode.Serialize(), Is.EqualTo(decoded.Serialize())); } private void sort(IBeatmap beatmap) @@ -48,27 +49,22 @@ namespace osu.Game.Tests.Beatmaps.Formats } } - private IBeatmap decode(string filename, out IBeatmap encoded) + private IBeatmap decodeFromLegacy(Stream stream) { - using (var stream = TestResources.GetStore().GetStream(filename)) - using (var sr = new LineBufferedReader(stream)) - { - var legacyDecoded = convert(new LegacyBeatmapDecoder { ApplyOffsets = false }.Decode(sr)); + using (var reader = new LineBufferedReader(stream)) + return convert(new LegacyBeatmapDecoder { ApplyOffsets = false }.Decode(reader)); + } - using (var ms = new MemoryStream()) - using (var sw = new StreamWriter(ms)) - using (var sr2 = new LineBufferedReader(ms, true)) - { - new LegacyBeatmapEncoder(legacyDecoded).Encode(sw); + private Stream encodeToLegacy(IBeatmap beatmap) + { + var stream = new MemoryStream(); - sw.Flush(); - ms.Position = 0; + using (var writer = new StreamWriter(stream, leaveOpen: true)) + new LegacyBeatmapEncoder(beatmap).Encode(writer); - encoded = convert(new LegacyBeatmapDecoder { ApplyOffsets = false }.Decode(sr2)); + stream.Position = 0; - return legacyDecoded; - } - } + return stream; } private IBeatmap convert(IBeatmap beatmap) From 7f7d5e6617a127de2729e01b53b7c7b34608dfc1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 16:37:08 +0900 Subject: [PATCH 4/5] Fix apparently required argument --- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index 64efe08929..b1782f7f08 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -6,6 +6,7 @@ using System.Collections; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text; using NUnit.Framework; using osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; @@ -59,7 +60,7 @@ namespace osu.Game.Tests.Beatmaps.Formats { var stream = new MemoryStream(); - using (var writer = new StreamWriter(stream, leaveOpen: true)) + using (var writer = new StreamWriter(stream, Encoding.UTF8, leaveOpen: true)) new LegacyBeatmapEncoder(beatmap).Encode(writer); stream.Position = 0; From ca6e6f7496f5468df67644e47c6b1c01bd0b82c6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 11 May 2020 17:26:11 +0900 Subject: [PATCH 5/5] Add required parameter for android build --- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index b1782f7f08..30331e98d2 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -60,7 +60,7 @@ namespace osu.Game.Tests.Beatmaps.Formats { var stream = new MemoryStream(); - using (var writer = new StreamWriter(stream, Encoding.UTF8, leaveOpen: true)) + using (var writer = new StreamWriter(stream, Encoding.UTF8, 1024, true)) new LegacyBeatmapEncoder(beatmap).Encode(writer); stream.Position = 0;