Skip to content

Add mania key count grouping to song select#37243

Open
adminLAla wants to merge 4 commits intoppy:masterfrom
adminLAla:feat/mania-keycount-grouping
Open

Add mania key count grouping to song select#37243
adminLAla wants to merge 4 commits intoppy:masterfrom
adminLAla:feat/mania-keycount-grouping

Conversation

@adminLAla
Copy link
Copy Markdown

Summary

I found the key count grouping in osu!stable's mania song select very useful.
While playing osu!lazer, I noticed that this grouping option was missing, so I implemented it here.

Changes

  • Adds a ManiaKeyCount grouping mode in song select.
  • Makes ManiaKeyCount available only when mania is the active ruleset.
  • Keeps the UI label as Key Count.
  • Keeps beatmap sets grouped together in this mode to avoid expanding all beatmap entries at once.
  • Adds key count result caching for grouping calculations.
  • Adds a cache size cap with auto-clear at 100,000 entries to prevent unbounded growth in very long sessions.

Testing

  • dotnet build osu.Game.Tests -p:GenerateFullPaths=true -m -verbosity:m

    • Succeeded, 0 warnings, 0 errors.
  • dotnet test osu.Game.Tests --filter "FullyQualifiedName~BeatmapCarouselFilterGroupingTest" -v minimal

    • Passed: 18, Failed: 0, Skipped: 0.
  • InspectCode.ps1

    • Exits with code 1 in my local environment because dotnet-nvika is not available as a local tool.
  • Equivalent static analysis steps were run manually:

    • dotnet jb inspectcode "osu.Desktop.slnf" --no-build --format=Xml --output="inspectcodereport.xml" --caches-home="inspectcode" --verbosity=WARN --jobs=1 --no-updates
    • nvika parsereport "inspectcodereport.xml" --treatwarningsaserrors
    • Result: 0 issues.

private Dictionary<GroupedBeatmapSet, HashSet<CarouselItem>> setMap = new Dictionary<GroupedBeatmapSet, HashSet<CarouselItem>>();
private Dictionary<GroupDefinition, HashSet<CarouselItem>> groupMap = new Dictionary<GroupDefinition, HashSet<CarouselItem>>();

private readonly ConcurrentDictionary<(Guid beatmapId, int converterModsSignature), int> maniaKeyCountCache = new ConcurrentDictionary<(Guid beatmapId, int converterModsSignature), int>();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is this? Why does it exist? Why is there any "caching" here at all? What's with the "mods signature", whatever it is?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for calling this out. You are right to question it.

I originally added the cache because I first assumed the lag came from repeatedly computing key counts during regrouping. After further testing, that assumption was wrong. The actual slowdown was caused by all groups being expanded, which increased the number of visible entries and rendering work while dragging the range slider.

Since the expansion behaviour has now been fixed, this cache (and the converter-mod signature key) does not provide meaningful value and only adds complexity. I will remove the caching/signature code and keep the grouping path straightforward.

I’ll push an update shortly.

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.

2 participants