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

Fix GameplaySampleTriggerSource not considering nested objects when determining the best sample to play #22659

Merged
merged 10 commits into from Feb 22, 2023

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Feb 16, 2023

While we correctly support this at a decoding level, the special case of taiko / mania playing back samples in a non-standard method meant that things would fall apart there.

This manifests in beatmaps which adjust sample sets or volumes mid-object (think osu!taiko drum roll or osu!mania hold notes), where the actual playback would not respect these adjustments.

See https://github.com/ppy/osu-wiki/pull/8791/files#r1106839986.
Also mentioned on #17247.

Can test using: Kenji Ninuma - DISCOPRINCE (peppy).osz.zip

@peppy peppy force-pushed the fix-taiko-drum-nested-sample-detection branch from fe6c13a to affa950 Compare February 16, 2023 09:21
As pointed out in review, if the current time is after the end of the
slider, the correct hit object to use for sample retrieval is the object
itself, not any nested object.
bdach
bdach previously approved these changes Feb 21, 2023
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

seems correct

@bdach bdach enabled auto-merge February 21, 2023 19:53
@bdach bdach disabled auto-merge February 21, 2023 20:27
@bdach
Copy link
Collaborator

bdach commented Feb 21, 2023

Curious windows-only test failures here that I initially failed to spot... Something related to clock offsets?

@bdach bdach dismissed their stale review February 21, 2023 20:28

maybe not quite yet

@bdach
Copy link
Collaborator

bdach commented Feb 21, 2023

Yeah this is most definitely the windows-specific extra 15ms of offset:

TearDown : System.TimeoutException : "wait for frame stable clock to catch up" timed out: Expected: 9915.0d +/- 9.9999999999999995E-08d
  But was:  9900.0d
  Off by:   15.0d

I guess all seeks should be using GameplayClockContainer rather than the track? I'm not 100% confident on it though. That would imply that choice of offset influences the correct hit sample to use, which I guess makes sense?

@bdach bdach merged commit d679703 into ppy:master Feb 22, 2023
@peppy peppy deleted the fix-taiko-drum-nested-sample-detection branch February 23, 2023 04:19
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