Skip to content

Fix mania hold notes dimming unexpectedly#37008

Merged
bdach merged 8 commits intoppy:masterfrom
smoogipoo:fix-mania-gray-notes
Mar 23, 2026
Merged

Fix mania hold notes dimming unexpectedly#37008
bdach merged 8 commits intoppy:masterfrom
smoogipoo:fix-mania-gray-notes

Conversation

@smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Mar 17, 2026

Fixes #29223

This fixes several issues around hold note dimming.

Notice in the following video:

  • The tail piece of the first note not getting dimmed.
  • The body of the second note not responding to dimming at all.
2026-03-17.19-42-26.mp4

Then, notice in the following video:

  • The body piece of the second note is dimmed from the very beginning.
2026-03-17.19-42-53.mp4

This requires a specific setup whereby the hold note and its components must be reused from the pool. In particular:

  1. The hold note must be long. So long that by the time the tail becomes on screen, the body will already have dimmed.
  2. The hold note must be re-used from the pool. We can induce this by setting the pool sizes to 1 in Column.cs.
  3. The second hold note should be placed far enough in the timeline that the first hold note dies by the time it becomes visible.
  4. Scroll speed should be adjusted to fit the above constraints.

I haven't done a full deep dive into exactly why this is happening, so the fix here is hand-wavy. That said, just by looking at the old code in LegacyBodyPiece you'll get a feeling that something's bound to go wrong;

  • It never resets the missingFadeTime state.
  • It never resets the colours back to Color4.White.
  • It applies transforms onto external components.
  • It jumps through hoops to figure out how to set missingFadeTime.

My hope is that these changes first bring some sanity in the process, and if it breaks again I'll consider doing a more proper root cause (I've had this issue in the back of my mind for about 1 year).

With this PR, they now behave as expected:

2026-03-17.19-43-59.mp4
2026-03-17.19-44-15.mp4

These seem to be unnecessary due to DHO's transform reset.
@smoogipoo smoogipoo added type/behavioural An issue with actual UI or game behaviour. Has a real world impact causing something to not work. area:skinning labels Mar 17, 2026
@bdach bdach self-requested a review March 18, 2026 07:42
Nested hitobjects receive `OnApply()` first, which would cause them to
apply the (now stale) dim state.
@smoogipoo smoogipoo changed the title Fix mania hold notes turning gray unexpectedly Fix mania hold notes dimming unexpectedly Mar 23, 2026
@bdach bdach merged commit d4a4acd into ppy:master Mar 23, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:skinning ruleset/osu!mania size/L type/behavioural An issue with actual UI or game behaviour. Has a real world impact causing something to not work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mania greyed out long notes before being hit (rare)

2 participants