feat(gameplay): sync video rewind and add background fades#1409
feat(gameplay): sync video rewind and add background fades#1409Nort1346 wants to merge 3 commits into
Conversation
fbabb83 to
e4f3942
Compare
| Resume(); | ||
|
|
||
|
|
||
| var waitCanceled = await UniTask.WaitUntil(() => SongTime > targetResumeTime, cancellationToken: token) |
There was a problem hiding this comment.
My recollection is that this was necessary to deal with pausing again during the rewind/resume cycle...
(The exact details escape me at the moment, sorry)
There was a problem hiding this comment.
The reason I removed it is that with the new logic, the video now seeks to the target time simultaneously with the audio.
Previously the video stay paused for that 1 second rewind duration, so we had to 'wait' for the notes to catch up to the video's frame. Now that they stay in sync during the jump. There's no longer a gap to wait for
There was a problem hiding this comment.
If you look at the await chain, you'll note that GameManager.Resume() waits on GameManager.RewindAndResume() so that ResumeCore() doesn't get called until after the WaitUntil has completed and we abort the resume there if the player pauses again before we reach the point in the song where the game was paused.
This is necessary because we only want the visual stuff getting updated until we've reached the time of the original pause.
I suspect what you want to do here is to have the call to BackgroundManager.SetPaused() that currently happens in ResumeCore() happen earlier in the resume process. The issue is making sure that only happens for video backgrounds. I'd imagine one possibility would be to have BackgroundManager.SetPaused() take a 3 state enum or something along those lines so it can do the right thing depending on the phase of the resume process and the type of background being used.
There was a problem hiding this comment.
You're right that WaitUntil was important for aborting resume if player pauses again before reaching target time.
However in the new implementation:
- Video stays paused during the entire rewind phase (0.5s animation)
- Video seeks to audio position after rewind completes via
SetTime() - If player pauses again during rewind,
_rewindSource.Cancel()aborts the rewind andResumeCore()is never called - The WaitUntil was specifically to handle the 1 second gap where video was frozen while audio rewound. That gap no longer exists since video now rewinds simultaneously
There was a problem hiding this comment.
The delay in calling ResumeCore also prevents the engines from being updated and inputs queued inappropriately until we reach the time of the original pause, ensuring that as far as the engine is concerned there was never any discontinuity.
There was a problem hiding this comment.
Is this a theoretical concern or has there been a specific bug in the past? I have tested thoroughly and can't find any issues. What should I look for?
Previously, when rewinding, the video would just sit there on a single frame and wait for the audio and gems to catch up. Now, the video actually rewinds along with everything else. I've also added fade in/out animations to the background so the transitions aren't so abrupt
Additionally I removed the blocking WaitUntil in SongRunner.RewindAndResume to keep things moving smoothly
Note
It's probably worth looking at YARG.Core (BaseEngine.Generic.cs line 355) to tweak the logic there otherwise it might spam some errors in the logs during the rewind