Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix osu!taiko slider velocity being written incorrectly to .osu file on export #25689

Merged
merged 11 commits into from
Dec 7, 2023
52 changes: 46 additions & 6 deletions osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,61 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Globalization;
using System.IO;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Extensions;
using osu.Framework.Utils;
using osu.Game.Rulesets.Taiko.Beatmaps;
using osu.Game.Beatmaps;
using osu.Game.Tests.Visual;
using SharpCompress.Archives.Zip;

namespace osu.Game.Rulesets.Taiko.Tests.Editor
{
public partial class TestSceneTaikoEditorSaving : EditorSavingTestScene
{
protected override Ruleset CreateRuleset() => new TaikoRuleset();

[TestCase(null)]
[TestCase(1f)]
[TestCase(2f)]
[TestCase(2.4f)]
public void TestTaikoSliderMultiplierInExport(float? multiplier)
{
if (multiplier.HasValue)
AddStep("Set slider multiplier", () => EditorBeatmap.Difficulty.SliderMultiplier = multiplier.Value);

SaveEditor();
AddStep("export beatmap", () => Game.BeatmapManager.Export(EditorBeatmap.BeatmapInfo.BeatmapSet!).WaitSafely());

AddAssert("check slider multiplier correct in file", () =>
{
string export = LocalStorage.GetFiles("exports").First();

using (var stream = LocalStorage.GetStream(export))
using (var zip = ZipArchive.Open(stream))
{
using (var osuStream = zip.Entries.First().OpenEntryStream())
using (var reader = new StreamReader(osuStream))
{
string? line;

while ((line = reader.ReadLine()) != null)
{
if (line.StartsWith("SliderMultiplier", StringComparison.Ordinal))
{
return float.Parse(line.Split(':', StringSplitOptions.TrimEntries).Last(), provider: CultureInfo.InvariantCulture);
}
}
}
}

return 0;
}, () => Is.EqualTo(multiplier ?? new BeatmapDifficulty().SliderMultiplier).Within(Precision.FLOAT_EPSILON));
}

[Test]
public void TestTaikoSliderMultiplier()
{
Expand All @@ -27,11 +71,7 @@ public void TestTaikoSliderMultiplier()

bool assertTaikoSliderMulitplier()
{
// we can only assert value correctness on TaikoMultiplierAppliedDifficulty, because that is the final difficulty converted taiko beatmaps use.
// therefore, ensure that we have that difficulty type by calling .CopyFrom(), which is a no-op if the type is already correct.
var taikoDifficulty = new TaikoBeatmapConverter.TaikoMultiplierAppliedDifficulty();
taikoDifficulty.CopyFrom(EditorBeatmap.Difficulty);
return Precision.AlmostEquals(taikoDifficulty.SliderMultiplier, 2);
return Precision.AlmostEquals(EditorBeatmap.Difficulty.SliderMultiplier, 2);
}
}
}
Expand Down
59 changes: 13 additions & 46 deletions osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,25 @@
using System.Linq;
using osu.Framework.Utils;
using System.Threading;
using JetBrains.Annotations;
using osu.Game.Audio;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Beatmaps.Formats;
using osu.Game.Rulesets.Objects.Legacy;

namespace osu.Game.Rulesets.Taiko.Beatmaps
{
internal class TaikoBeatmapConverter : BeatmapConverter<TaikoHitObject>
{
/// <summary>
/// A speed multiplier applied globally to osu!taiko.
/// </summary>
/// <remarks>
/// osu! is generally slower than taiko, so a factor was historically added to increase speed for converts.
/// This must be used everywhere slider length or beat length is used in taiko.
///
/// Of note, this has never been exposed to the end user, and is considered a hidden internal multiplier.
/// </remarks>
public const float VELOCITY_MULTIPLIER = 1.4f;

/// <summary>
/// Because swells are easier in taiko than spinners are in osu!,
/// legacy taiko multiplies a factor when converting the number of required hits.
Expand All @@ -43,12 +52,6 @@ public TaikoBeatmapConverter(IBeatmap beatmap, Ruleset ruleset)

protected override Beatmap<TaikoHitObject> ConvertBeatmap(IBeatmap original, CancellationToken cancellationToken)
{
if (!(original.Difficulty is TaikoMultiplierAppliedDifficulty))
{
// Rewrite the beatmap info to add the slider velocity multiplier
original.Difficulty = new TaikoMultiplierAppliedDifficulty(original.Difficulty);
}

Beatmap<TaikoHitObject> converted = base.ConvertBeatmap(original, cancellationToken);

if (original.BeatmapInfo.Ruleset.OnlineID == 0)
Expand Down Expand Up @@ -180,7 +183,7 @@ private bool shouldConvertSliderToHits(HitObject obj, IBeatmap beatmap, IHasPath
double distance = pathData.Path.ExpectedDistance.Value ?? 0;

// Do not combine the following two lines!
distance *= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
distance *= VELOCITY_MULTIPLIER;
distance *= spans;

TimingControlPoint timingPoint = beatmap.ControlPointInfo.TimingPointAt(obj.StartTime);
Expand All @@ -192,7 +195,7 @@ private bool shouldConvertSliderToHits(HitObject obj, IBeatmap beatmap, IHasPath
else
beatLength = timingPoint.BeatLength;

double sliderScoringPointDistance = osu_base_scoring_distance * beatmap.Difficulty.SliderMultiplier / beatmap.Difficulty.SliderTickRate;
double sliderScoringPointDistance = osu_base_scoring_distance * (beatmap.Difficulty.SliderMultiplier * TaikoBeatmapConverter.VELOCITY_MULTIPLIER) / beatmap.Difficulty.SliderTickRate;
peppy marked this conversation as resolved.
Show resolved Hide resolved

// The velocity and duration of the taiko hit object - calculated as the velocity of a drum roll.
double taikoVelocity = sliderScoringPointDistance * beatmap.Difficulty.SliderTickRate;
Expand All @@ -218,41 +221,5 @@ private bool shouldConvertSliderToHits(HitObject obj, IBeatmap beatmap, IHasPath
}

protected override Beatmap<TaikoHitObject> CreateBeatmap() => new TaikoBeatmap();

// Important to note that this is subclassing a realm object.
// Realm doesn't allow this, but for now this can work since we aren't (in theory?) persisting this to the database.
// It is only used during beatmap conversion and processing.
internal class TaikoMultiplierAppliedDifficulty : BeatmapDifficulty
{
public TaikoMultiplierAppliedDifficulty(IBeatmapDifficultyInfo difficulty)
{
CopyFrom(difficulty);
}

[UsedImplicitly]
public TaikoMultiplierAppliedDifficulty()
{
}

#region Overrides of BeatmapDifficulty

public override BeatmapDifficulty Clone() => new TaikoMultiplierAppliedDifficulty(this);

public override void CopyTo(BeatmapDifficulty other)
{
base.CopyTo(other);
if (!(other is TaikoMultiplierAppliedDifficulty))
other.SliderMultiplier /= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
}

public override void CopyFrom(IBeatmapDifficultyInfo other)
{
base.CopyFrom(other);
if (!(other is TaikoMultiplierAppliedDifficulty))
SliderMultiplier *= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
}

#endregion
}
}
}
8 changes: 4 additions & 4 deletions osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Game.Rulesets.Objects.Types;
using System.Threading;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Beatmaps.Formats;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Scoring;
using osu.Game.Rulesets.Taiko.Beatmaps;
using osuTK;

namespace osu.Game.Rulesets.Taiko.Objects
Expand Down Expand Up @@ -51,7 +51,7 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I
TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime);
EffectControlPoint effectPoint = controlPointInfo.EffectPointAt(StartTime);

double scoringDistance = base_distance * difficulty.SliderMultiplier * effectPoint.ScrollSpeed;
double scoringDistance = base_distance * (difficulty.SliderMultiplier * TaikoBeatmapConverter.VELOCITY_MULTIPLIER) * effectPoint.ScrollSpeed;
Velocity = scoringDistance / timingPoint.BeatLength;

TickRate = difficulty.SliderTickRate == 3 ? 3 : 4;
Expand Down Expand Up @@ -116,7 +116,7 @@ public StrongNestedHit(TaikoHitObject parent)
double IHasDistance.Distance => Duration * Velocity;

SliderPath IHasPath.Path
=> new SliderPath(PathType.LINEAR, new[] { Vector2.Zero, new Vector2(1) }, ((IHasDistance)this).Distance / LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER);
=> new SliderPath(PathType.LINEAR, new[] { Vector2.Zero, new Vector2(1) }, ((IHasDistance)this).Distance / TaikoBeatmapConverter.VELOCITY_MULTIPLIER);

#endregion
}
Expand Down
3 changes: 2 additions & 1 deletion osu.Game.Rulesets.Taiko/UI/DrawableTaikoRuleset.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.Taiko.Beatmaps;
using osu.Game.Rulesets.Taiko.Objects;
using osu.Game.Rulesets.Taiko.Replays;
using osu.Game.Rulesets.Timing;
Expand Down Expand Up @@ -70,7 +71,7 @@ protected override void Update()
protected virtual double ComputeTimeRange()
{
// Taiko scrolls at a constant 100px per 1000ms. More notes become visible as the playfield is lengthened.
const float scroll_rate = 10;
const float scroll_rate = 10 / TaikoBeatmapConverter.VELOCITY_MULTIPLIER;
bdach marked this conversation as resolved.
Show resolved Hide resolved

// Since the time range will depend on a positional value, it is referenced to the x480 pixel space.
// Width is used because it defines how many notes fit on the playfield.
Expand Down
12 changes: 1 addition & 11 deletions osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ public class LegacyBeatmapEncoder
{
public const int FIRST_LAZER_VERSION = 128;

/// <summary>
/// osu! is generally slower than taiko, so a factor is added to increase
/// speed. This must be used everywhere slider length or beat length is used.
/// </summary>
public const float LEGACY_TAIKO_VELOCITY_MULTIPLIER = 1.4f;

private readonly IBeatmap beatmap;

private readonly ISkin? skin;
Expand Down Expand Up @@ -149,11 +143,7 @@ private void handleDifficulty(TextWriter writer)
writer.WriteLine(FormattableString.Invariant($"OverallDifficulty: {beatmap.Difficulty.OverallDifficulty}"));
writer.WriteLine(FormattableString.Invariant($"ApproachRate: {beatmap.Difficulty.ApproachRate}"));

// Taiko adjusts the slider multiplier (see: LEGACY_TAIKO_VELOCITY_MULTIPLIER)
writer.WriteLine(onlineRulesetID == 1
? FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier / LEGACY_TAIKO_VELOCITY_MULTIPLIER}")
: FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier}"));

writer.WriteLine(FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier}"));
writer.WriteLine(FormattableString.Invariant($"SliderTickRate: {beatmap.Difficulty.SliderTickRate}"));
}

Expand Down