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

Save grid rotation to beatmap #28973

Closed
wants to merge 1 commit into from
Closed

Conversation

kstefanowicz
Copy link
Contributor

Adds the GridLinesRotation int from OsuGridToolboxGroup.cs to the [Editor] portion of the beatmap .osu file. Allows grid rotation to be saved instead of reverting to default every time the map is loaded.

I'd imagine something similar can be done for StartPosition and GridType, but I am not familiar enough with C# typing to implement.

Osu-Save-Rotation.webm

@frenzibyte
Copy link
Member

See #28473, #23418, #28810, #20883. The grid rotation should be part of the .osu file, not in the databased model.

@frenzibyte frenzibyte closed this Jul 20, 2024
@kstefanowicz
Copy link
Contributor Author

This implementation does add the grid rotation to the .osu file via LegacyBeatmapDecoder.cs and reads it via LegacyBeatmapEncoder.cs

@frenzibyte
Copy link
Member

Yes but it's also persisting it in the database by adding it in BeatmapInfo, which is frowned upon. It should be added in IBeatmap (which is what I mean by .osu file). I've closed this PR until the concern in #20883 is resolved first.

@peppy
Copy link
Member

peppy commented Jul 22, 2024

@OliBomby you were tackling this last i heard, including more than just rotation. do you have anything to show yet?

@OliBomby
Copy link
Contributor

@OliBomby you were tackling this last i heard, including more than just rotation. do you have anything to show yet?

I had created a working implementation by adding the properties to BeatmapInfo but didnt create a PR because we dont want it it in BeatmapInfo. I'm waiting on the PR that moves stuff out of BeatmapInfo #28473

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.

4 participants