Only play nightcore hat sounds when tick rate is a multiple of 2#36459
Only play nightcore hat sounds when tick rate is a multiple of 2#36459peppy merged 2 commits intoppy:masterfrom
Conversation
| public NightcoreBeatContainer(double sliderTickRate) | ||
| { | ||
| Divisor = 2; | ||
| playHats = Precision.AlmostEquals(sliderTickRate % 2, 0); |
There was a problem hiding this comment.
almost equals with a modulo is not going to work how you may want it to.
There was a problem hiding this comment.
Were you intending to resolve this in your latest push? It's still kind of an issue.
sliderTickRate is hopefully never negative so that isn't a fail case, but you could still get something like 1.9999998 which would fail this check as written.
There was a problem hiding this comment.
All I intended to do here was match stable on a code level.
Do you think we should be doing better? The PR's original AlmostEquals would be closest to what I'd consider changing this to, if you prefer that.
There was a problem hiding this comment.
I guess if the thought here is to mostly follow stable then it's probably fine.
|
@bdach before I even begin look into this one (turns out this is resolving an issue which was in the project), you recently touched nightcore and may have thoughts on this. |
Not many other than what I mentioned in the OP of #36702:
Stable nightcore just did not really appear to attempt to accommodate waltz time in any way. It seemed to assume everything is always in 4/4. |
|
Right, I think what you said there makes some sense, but based on what's being said here this may be a scenario we want to match stable to restore sanity (since people are using very specific "workarounds" to make things sound as they want for already ranked maps). Could you get behind that? If so I'll check this change matches stable and get it in. |
|
Bringing this back is a mostly subjective call so I'm neither firmly here or there really. You could decide to bring it back and I wouldn't really have any meaningful reasons to oppose it. Just note that lazer's special treatment of 3/4 means that some maps will not match stable anyway. And I don't even mean something in graveyard mapped 10 years ago. One example is given in the aforementioned PR and it's https://osu.ppy.sh/beatmapsets/534385#osu/1131956: 
You could say it's a pretty pathological example in how it uses 3/4 and 4/4 timing points next to each other in an attempt to compensate for very irregular timing, but it also is in ranked... |
This is fucked and I wish I never saw this. |
peppy
force-pushed
the
fix-nightcore-hat-sounds
branch
from
4a8b055 to
0f60ab9
Compare
|
@bdach can you link me to something explaining "lazer's special treatment of 3/4"? I guess you mean this? osu/osu.Game/Rulesets/Mods/ModNightcore.cs Lines 154 to 155 in 0f60ab9 Also, the example you give has a tick rate of 1 which means with this PR's change it will stop playing hats (for the best i think?) |
Uh... well, stable nightcore just does not check time signature in the same way and assumes 4/4. Support for waltz time nightcore in lazer blames back to 72404bf. I don't have anything else to link, it's just all down to the basic fact that lazer tries to do anything different in 3/4.
Yeah maybe that specific example was fine, I don't recall at this time. |
bdach
left a comment
bdach
left a comment
There was a problem hiding this comment.
honestly not gonna pretend I've crosschecked this against stable, let's just get in and deal with issues as people report
github-project-automation
bot
moved this from Pending Review
to Done
in osu! team task tracker
3 participants
Closes #13513
Matches stable behavior where hat only plays when the slider tick rate is a multiple of 2.