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

Round object coordinates to nearest integers on legacy export rather than truncating #31305

Merged
merged 3 commits into from
Jan 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,16 @@ public class EmptyFreeformHitObject : HitObject, IHasPosition

public Vector2 Position { get; set; }

public float X => Position.X;
public float Y => Position.Y;
public float X
{
get => Position.X;
set => Position = new Vector2(value, Y);
}

public float Y
{
get => Position.Y;
set => Position = new Vector2(X, value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,16 @@ public class PippidonHitObject : HitObject, IHasPosition

public Vector2 Position { get; set; }

public float X => Position.X;
public float Y => Position.Y;
public float X
{
get => Position.X;
set => Position = new Vector2(value, Y);
}

public float Y
{
get => Position.Y;
set => Position = new Vector2(X, value);
}
}
}
22 changes: 19 additions & 3 deletions osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,27 @@ public void UpdateComboInformation(IHasComboInformation? lastObj)
/// </summary>
public float LegacyConvertedY { get; set; } = DEFAULT_LEGACY_CONVERT_Y;

float IHasXPosition.X => OriginalX;
float IHasXPosition.X
{
get => OriginalX;
set => OriginalX = value;
}

float IHasYPosition.Y => LegacyConvertedY;
float IHasYPosition.Y
{
get => LegacyConvertedY;
set => LegacyConvertedY = value;
}

Vector2 IHasPosition.Position => new Vector2(OriginalX, LegacyConvertedY);
Vector2 IHasPosition.Position
{
get => new Vector2(OriginalX, LegacyConvertedY);
set
{
((IHasXPosition)this).X = value.X;
((IHasYPosition)this).Y = value.Y;
}
}

#endregion
}
Expand Down
6 changes: 5 additions & 1 deletion osu.Game.Rulesets.Mania/Objects/ManiaHitObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ public virtual int Column

#region LegacyBeatmapEncoder

float IHasXPosition.X => Column;
float IHasXPosition.X
{
get => Column;
set => Column = (int)value;
}

#endregion
}
Expand Down
13 changes: 11 additions & 2 deletions osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,17 @@ public virtual Vector2 Position
set => position.Value = value;
}

public float X => Position.X;
public float Y => Position.Y;
public float X
{
get => Position.X;
set => Position = new Vector2(value, Position.Y);
}

public float Y
{
get => Position.Y;
set => Position = new Vector2(Position.X, value);
}

public Vector2 StackedPosition => Position + StackOffset;

Expand Down
24 changes: 24 additions & 0 deletions osu.Game.Tests/Beatmaps/IO/LegacyBeatmapExporterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using osu.Game.Beatmaps;
using osu.Game.Database;
using osu.Game.IO.Archives;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Tests.Resources;
using osu.Game.Tests.Visual;
using MemoryStream = System.IO.MemoryStream;
Expand Down Expand Up @@ -50,6 +51,29 @@ public void TestObjectsSnappedAfterTruncatingExport()
AddAssert("hit object is snapped", () => beatmap.Beatmap.HitObjects[0].StartTime, () => Is.EqualTo(28519).Within(0.001));
}

[Test]
public void TestFractionalObjectCoordinatesRounded()
{
IWorkingBeatmap beatmap = null!;
MemoryStream outStream = null!;

// Ensure importer encoding is correct
AddStep("import beatmap", () => beatmap = importBeatmapFromArchives(@"fractional-coordinates.olz"));
AddAssert("hit object has fractional position", () => ((IHasYPosition)beatmap.Beatmap.HitObjects[1]).Y, () => Is.EqualTo(383.99997).Within(0.00001));

// Ensure exporter legacy conversion is correct
AddStep("export", () =>
{
outStream = new MemoryStream();

new LegacyBeatmapExporter(LocalStorage)
.ExportToStream((BeatmapSetInfo)beatmap.BeatmapInfo.BeatmapSet!, outStream, null);
});

AddStep("import beatmap again", () => beatmap = importBeatmapFromStream(outStream));
AddAssert("hit object is snapped", () => ((IHasYPosition)beatmap.Beatmap.HitObjects[1]).Y, () => Is.EqualTo(384).Within(0.00001));
}

[Test]
public void TestExportStability()
{
Expand Down
Binary file not shown.
11 changes: 10 additions & 1 deletion osu.Game/Database/LegacyBeatmapExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ public LegacyBeatmapExporter(Storage storage)
return null;

using var contentStreamReader = new LineBufferedReader(contentStream);
var beatmapContent = new LegacyBeatmapDecoder().Decode(contentStreamReader);

// FIRST_LAZER_VERSION is specified here to avoid flooring object coordinates on decode via `(int)` casts.
// we will be making integers out of them lower down, but in a slightly different manner (rounding rather than truncating)
var beatmapContent = new LegacyBeatmapDecoder(LegacyBeatmapEncoder.FIRST_LAZER_VERSION).Decode(contentStreamReader);

var workingBeatmap = new FlatWorkingBeatmap(beatmapContent);
var playableBeatmap = workingBeatmap.GetPlayableBeatmap(beatmapInfo.Ruleset);
Expand Down Expand Up @@ -93,6 +96,12 @@ public LegacyBeatmapExporter(Storage storage)

hitObject.StartTime = Math.Floor(hitObject.StartTime);

if (hitObject is IHasXPosition hasXPosition)
hasXPosition.X = MathF.Round(hasXPosition.X);

if (hitObject is IHasYPosition hasYPosition)
hasYPosition.Y = MathF.Round(hasYPosition.Y);

if (hitObject is not IHasPath hasPath) continue;

// stable's hit object parsing expects the entire slider to use only one type of curve,
Expand Down
14 changes: 11 additions & 3 deletions osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,17 @@ internal abstract class ConvertHitObject : HitObject, IHasCombo, IHasPosition, I

public int ComboOffset { get; set; }

public float X => Position.X;

public float Y => Position.Y;
public float X
{
get => Position.X;
set => Position = new Vector2(value, Position.Y);
}

public float Y
{
get => Position.Y;
set => Position = new Vector2(Position.X, value);
}

public Vector2 Position { get; set; }

Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Rulesets/Objects/Types/IHasPosition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ public interface IHasPosition : IHasXPosition, IHasYPosition
/// <summary>
/// The starting position of the HitObject.
/// </summary>
Vector2 Position { get; }
Vector2 Position { get; set; }
}
}
2 changes: 1 addition & 1 deletion osu.Game/Rulesets/Objects/Types/IHasXPosition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ public interface IHasXPosition
/// <summary>
/// The starting X-position of this HitObject.
/// </summary>
float X { get; }
float X { get; set; }
}
}
2 changes: 1 addition & 1 deletion osu.Game/Rulesets/Objects/Types/IHasYPosition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ public interface IHasYPosition
/// <summary>
/// The starting Y-position of this HitObject.
/// </summary>
float Y { get; }
float Y { get; set; }
}
}
Loading