Skip to content

Added "LN Ratio" display in Mania in the place of useless "key count"#37581

Open
pacowoc wants to merge 16 commits intoppy:masterfrom
pacowoc:mania_ln_ratio
Open

Added "LN Ratio" display in Mania in the place of useless "key count"#37581
pacowoc wants to merge 16 commits intoppy:masterfrom
pacowoc:mania_ln_ratio

Conversation

@pacowoc
Copy link
Copy Markdown

@pacowoc pacowoc commented Apr 29, 2026

The "Key Count" metric in mania is very useless since you are already expected to play maps with a specific Key Count when you are queueing.
This PR inserts the proportion of LNs (Long Notes) in the place of that metric since it is one of the ways players can gudge their skillsets (This idea comes from reddit)
Also improved the test suite for other skillsets by making the architecture more minor ruleset friendly

Addresses #37568.

Comment thread osu.Game.Tests/Visual/RankedPlay/RankedPlayTestScene.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/RankedPlayTestScene.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestScenePickScreen.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestScenePickScreen.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestScenePickScreen.cs Fixed
@IceDynamix
Copy link
Copy Markdown

IceDynamix commented Apr 29, 2026

i think this should be solved by changing the implementation of ManiaRuleset.GetBeatmapAttributesForDisplay instead? this feels very hacky. there is also no consideration for localization because the attribute labels currently use SongSelectStrings. also, the number should also be formatted as a percentage if anything.

@pacowoc
Copy link
Copy Markdown
Author

pacowoc commented Apr 29, 2026

i think this should be solved by changing the implementation of ManiaRuleset.GetBeatmapAttributesForDisplay instead? this feels very hacky. there is also no consideration for localization because the attribute labels currently use SongSelectStrings. also, the number should also be formatted as a percentage if anything.

There are absolutely zero consideration of localization in 99% of ranked UI anyways

Comment thread osu.Game.Rulesets.Mania/ManiaRuleset.cs Fixed
Comment thread osu.Game/Rulesets/Ruleset.cs Fixed
Comment thread osu.Game/Rulesets/Ruleset.cs Fixed
@pacowoc
Copy link
Copy Markdown
Author

pacowoc commented Apr 29, 2026

Also the Tests here are bugging so much that it is insane, the pickscreen test stop midway on first run but runs normally on second run lmao, idk why

Comment thread osu.Game.Tests/Visual/RankedPlay/RankedPlayTestScene.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/RankedPlayTestScene.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestSceneDiscardScreen.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestSceneOpponentPickScreen.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestScenePickScreen.cs Fixed
Comment thread osu.Game.Tests/Visual/RankedPlay/TestScenePickScreen.cs Fixed
Comment thread osu.Game.Rulesets.Mania/ManiaRuleset.cs Fixed
Comment thread osu.Game/Rulesets/Ruleset.cs Fixed
@pacowoc
Copy link
Copy Markdown
Author

pacowoc commented May 1, 2026

i think this should be solved by changing the implementation of ManiaRuleset.GetBeatmapAttributesForDisplay instead? this feels very hacky. there is also no consideration for localization because the attribute labels currently use SongSelectStrings. also, the number should also be formatted as a percentage if anything.

Re-implemented with your suggestion

@peppy peppy requested a review from smoogipoo May 7, 2026 06:19
Copy link
Copy Markdown
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

LGTM. Not 10000% sure about the extra Ruleset method but it's probably the easiest place to do it for now.

@smoogipoo smoogipoo moved this from Inbox to Pending Review in osu! team task tracker May 7, 2026
@smoogipoo smoogipoo requested a review from peppy May 7, 2026 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

5 participants