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

Populate beatmap ruleset in SoloScoreInfo.ToScoreInfo() #20984

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Oct 28, 2022

If we're to do "is converted beatmap" checks as per #20558, which I don't think is necessarily a bad idea, then the beatmaps' ruleset needs to be populated in the conversion from SoloScoreInfo to ScoreInfo.

This will also require an osu-queue-score-statistics change to pass in the APIBeatmap, that I'll do as a followup if this PR is accepted/when a new nuget package is released.

I've done the bare minimum here by populating just the ruleset, but there are other members in IBeatmapInfo that aren't passed across like BPM, Length, etc... Unfortunately, as far as I know, we don't have a function to convert IBeatmapInfo/APIBeatmapInfo to BeatmapInfo, so populating all members will require substantial effort. Please give me your opinions on this.

@smoogipoo smoogipoo requested review from peppy and bdach October 28, 2022 05:46
@smoogipoo smoogipoo changed the title Populate beatmap ruleset in SoloScoreInfo.ToScoreInfo() Populate beatmap ruleset in SoloScoreInfo.ToScoreInfo() Oct 28, 2022
Copy link
Sponsor Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Seems harmless enough for now.

@peppy peppy merged commit a826f77 into ppy:master Oct 28, 2022
@smoogipoo smoogipoo deleted the fix-missing-beatmap-ruleset branch September 11, 2023 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants