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 timing point truncation causing missnaps on compatibility-exported lazer beatmaps #30607

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

OliBomby
Copy link
Contributor

Fixes #30606

namespace osu.Game.Tests.Beatmaps.IO
{
[HeadlessTest]
public partial class LegacyBeatmapExporterTest : OsuTestScene
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if this test would be better suited to the LegacyBeatmapEncoderTest class?

Copy link
Collaborator

@bdach bdach Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd actually prefer them to be here. Encoder shouldn't be caring about backcompat logic, encoder should be encoding, i.e. dumb formatting logic. This same location is where I'd also put legacy export stability tests that you asked for in relation to BSS and which we don't currently have.

@bdach bdach added area:beatmap-parsing .osu file format parsing area:editor labels Nov 14, 2024
@bdach
Copy link
Collaborator

bdach commented Nov 14, 2024

Aside from slight alterations to the assertions in the test (8df7a6f) I see no fault with this.

@peppy anything here you want to intervene on or am I fine to proceed?

@peppy
Copy link
Member

peppy commented Nov 14, 2024

Aside from slight alterations to the assertions in the test (8df7a6f) I see no fault with this.

@peppy anything here you want to intervene on or am I fine to proceed?

at your discretion

@bdach bdach merged commit b94d3d7 into ppy:master Nov 14, 2024
10 checks passed
@OliBomby OliBomby deleted the legacy-export-offset branch November 14, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kiai time offset can sometimes appear to be off by 1ms on compatibility-exported lazer beatmaps
3 participants