Skip to content

Limit delay before score screen at end of song#993

Open
wyrdough wants to merge 4 commits intoYARC-Official:devfrom
wyrdough:end-delay
Open

Limit delay before score screen at end of song#993
wyrdough wants to merge 4 commits intoYARC-Official:devfrom
wyrdough:end-delay

Conversation

@wyrdough
Copy link
Copy Markdown
Contributor

@wyrdough wyrdough commented Mar 12, 2025

There is currently an excessive delay before showing the score screen at the end of many songs. This is because there is a fixed two second delay after the end of the greater of the chart end time, the audio end time, or the end event time. In many cases this can lead to 5+ seconds of sitting in silence awaiting the score screen.

This PR changes the delay to be adaptive, with a minimum of two seconds wait after the last charted note for any instrument before showing the score screen, thus eliminating the excessive delay.

SongLength is set depending on whether an end event is charted (endTime is the time of the end event):

  • With an end event SongLength = Max(lastNoteTime, endTime)
  • With no end event SongLength = Max(lastNoteTime, audioLength)

The song ending again depends on whether an end event is charted:

  • With an end event the song ends at the greater of lastNoteTime + SONG_END_DELAY and endTime
  • With no end event the song ends at the greater of lastNoteTime + SONG_END_DELAY and audioLength

The only exception is if the end_events tag is set in song.ini and an end event exists. In that case, the end event is respected precisely with no delay, regardless of any other considerations.

Additionally, silence detection is run on the StemMixer output once lastNoteTime + SONG_END_DELAY has elapsed when there is no end event and SongLength is being controlled by audioLength. This is to avoid long waits for audio tracks with excessive trailing silence.

Requires YARG.Core PR #242

Comment thread Assets/Script/Gameplay/GameManager.Loading.cs Outdated
Comment thread Assets/Script/Gameplay/GameManager.cs Outdated
Comment thread Assets/Script/Gameplay/GameManager.Loading.cs Outdated
@wyrdough wyrdough changed the title Modify FinalizeChart to avoid excessive delay at the end of a song. Limit delay before score screen at end of song Mar 12, 2025
@wyrdough
Copy link
Copy Markdown
Contributor Author

wyrdough commented Mar 12, 2025

During testing I ran into a bunch of charts with no end event and several seconds of trailing silence (in one case over 10 seconds!). This greatly annoyed me, so I implemented some rudimentary silence detection in GameManager.Audio that gets triggered beginning two seconds after the last note that will end the song before SongLength if a third of a second of continuous silence is detected.

I'm not wedded to the silence threshold value being used or the third of a second duration, so suggestions are more than welcome.

Edited to add: Also, the rationale for not doing the silence detection on song loading is that my tests with the BASS example code take around a second to process a 4ish minute song, and that's with a combined stem. Obviously, that is an unacceptable level of delay during the song loading process.

@wyrdough wyrdough requested a review from TheNathannator March 12, 2025 21:49
@Dansla116
Copy link
Copy Markdown
Contributor

I've been doing a fair bit of testing across a wide range of charts (mostly legacy rips), and I wanted to share some results that highlight how this PR behaves in the wild. Out of everything I tested, these four RBN songs stood out as good examples:

Buckner and Garcia – "The Defender"
Ends with ~2 seconds of silence remaining. From what I can tell, this change kicks in right around the natural end anyway, so nothing feels different.

Cetan Clawson and the Soul Side – "White Heat"
Has a short silence after the last note, and it transitions cleanly to the score screen at ~2:15/2:16. This one feels like a perfect example of the intended behavior.

Fallen Angel – "The One Who Walks Alone" and Askari Nari - "Mercury Blooming"
The former has a few seconds of wind ambiance after the last note that gets cut short. It's noticeable, but not necessarily a problem. The latter has ~10 seconds of beach/wave ambiance that also gets cut off, but it feels more abrupt and jarring.

All that in mind, I can definitely see this being a subjective issue. Some folks in the community - especially the more competitive "play and FC as many songs as possible" crowd - will probably appreciate the faster transitions and reduced downtime. But others - particularly more casual or immersion-focused players - might miss the extra ambiance and breathing room at the end of a track.

Even so, I think this PR is a great solution for the vast number of charts without end events. Since there's no practical way to manually "fix" all of them, silence detection makes sense as a fallback and generally improves the flow of most songs.

If there's ever a way to make this behavior configurable (maybe a menu toggle), that might help satisfy both camps. But that's likely beyond the scope of what this PR set out to do. As it stands, this is a solid QoL improvement.

@wyrdough
Copy link
Copy Markdown
Contributor Author

Thanks for coming up with specific examples of songs on which the silence detection is triggering when it shouldn't. That's very helpful for getting this tuned correctly. As far as making the silence detection optional goes, my thinking is that if it is tuned well it shouldn't ever be noticeable that it's even happening, aside from the song timer not quite making it all the way to the end.

@wyrdough
Copy link
Copy Markdown
Contributor Author

Apparently I had a total braino when I was implementing silence detection and confused two different variants of the getlevel call, so it wasn't actually doing what it was supposed to. Fixing that seems to make Mercury Blooming work better, though I haven't yet tested it on anything else.

@Dansla116
Copy link
Copy Markdown
Contributor

I suggested making the silence detection optional because I misunderstood and thought the abrupt cutoffs (e.g. Mercury Blooming) were intentional. But now that we're on the same page and you've got that sorted out - I just retested all four songs to check the fix and make sure there were no regressions - I can confirm they all work exactly as expected, and I agree that a toggle isn't necessary.

Going forward, I'm testing other stuff right now, so I'll be keeping a passive eye out for more songs with long or awkward ending silences as I go. They're a bit tricky to find. And I'll report back if anything seems off. For now, this is working great! Thanks for the fix!

wyrdough added a commit to wyrdough/YARG that referenced this pull request Jul 31, 2025
RileyTheFox pushed a commit that referenced this pull request Aug 3, 2025
* add GetLevel_Internal to BassStemMixer so YARG engine-fixes will build against Core engine-fixes

- Extracted from YARG PR #993

* Add support for verbose replays command line arg

- When set, we capture, save and read frame times from replays (assuming the replays have them). Otherwise, frame time data is not captured, and we tell core not to bother us with frame time data in replays that have that data.

* Pass ReplayReadOptions struct to ReplayIO instead of bare bool
@Ape
Copy link
Copy Markdown
Contributor

Ape commented Jan 12, 2026

Idea: Move to the score screen soon after the last note, but keep playing the song until the end in that scree so to music is not cut out ever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants