Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions Assets/Script/Gameplay/BackgroundManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using Cinemachine;
using Cysharp.Threading.Tasks;
using DG.Tweening;
using UniHumanoid;
using UnityEngine;
using UnityEngine.Animations;
Expand Down Expand Up @@ -438,8 +439,7 @@ public void SetTime(double songTime)
enabled = false; // Temp disable
_videoPlayer.enabled = true;

// Hack to ensure the video stays synced to the audio
_videoSeeking = true; // Signaling flag; must come first
_videoSeeking = true;
if (SettingsManager.Settings.WaitForSongVideo.Value)
GameManager.OverridePause();

Expand All @@ -461,6 +461,45 @@ private void OnVideoSeeked(VideoPlayer player)
_videoSeeking = false;
}

public async UniTask<bool> FadeOut()
{
_backgroundDimmer.DOKill();
_backgroundDimmer.DOFade(1f, 0.25f);

return true;
}

public async UniTask SeekAndFadeIn(double songTime)
{
_backgroundDimmer.DOKill();
_backgroundDimmer.color = new Color(0, 0, 0, 1f);

var seekTask = PrepareVideoSeekTask();

SetTime(songTime);

await seekTask;

await _backgroundDimmer.DOFade(0f, 0.25f).AsyncWaitForCompletion().AsUniTask();
}

private async UniTask PrepareVideoSeekTask()
{
if (_type != BackgroundType.Video || _videoPlayer == null) return;

var completionSource = new UniTaskCompletionSource();

void OnSeekCompleted(VideoPlayer vp)
{
_videoPlayer.seekCompleted -= OnSeekCompleted;
completionSource.TrySetResult();
}

_videoPlayer.seekCompleted += OnSeekCompleted;

await completionSource.Task.Timeout(TimeSpan.FromSeconds(1)).SuppressCancellationThrow();
}

public void SetSpeed(float speed)
{
switch (_type)
Expand Down
10 changes: 8 additions & 2 deletions Assets/Script/Gameplay/GameManager.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using Cysharp.Threading.Tasks;
Expand Down Expand Up @@ -970,13 +970,19 @@ private async UniTask<bool> RewindAndResume(double seconds)
targetTime = PauseInfo[^1].PauseTime;
}

var canceled = await _songRunner.RewindAndResume(seconds, targetTime);
var (canceled, _) = await UniTask.WhenAll(
_songRunner.RewindAndResume(seconds, targetTime),
BackgroundManager.FadeOut()
);

if (canceled)
{
_ = BackgroundManager.SeekAndFadeIn(VisualTime);
return true;
}

_ = BackgroundManager.SeekAndFadeIn(_songRunner.SongTime + Song.SongOffsetSeconds);

foreach (var player in _players)
{
player.PostRewind(VisualTime - seconds);
Expand Down
9 changes: 2 additions & 7 deletions Assets/Script/Playback/SongRunner.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using DG.Tweening;
Expand Down Expand Up @@ -728,7 +728,6 @@ public async UniTask<bool> RewindAndResume(double seconds, double? overrideTarge

var targetRewindTime = SongTime - seconds;
var targetVisualTime = targetRewindTime + (VideoCalibration - AudioCalibration) * SongSpeed;
var targetResumeTime = overrideTargetTime ?? SongTime;

_rewindTween = DOTween.To(() => VisualTime, x => VisualTime = x, targetVisualTime, 0.5f);

Expand All @@ -748,11 +747,7 @@ public async UniTask<bool> RewindAndResume(double seconds, double? overrideTarge
SetSongTime(targetRewindTime - (AudioCalibration * SongSpeed), 0);
Resume();


var waitCanceled = await UniTask.WaitUntil(() => SongTime > targetResumeTime, cancellationToken: token)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right that WaitUntil was important for aborting resume if player pauses again before reaching target time.

However in the new implementation:

  1. Video stays paused during the entire rewind phase (0.5s animation)
  2. Video seeks to audio position after rewind completes via SetTime()
  3. If player pauses again during rewind, _rewindSource.Cancel() aborts the rewind and ResumeCore() is never called
  4. 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

.SuppressCancellationThrow();

if (waitCanceled || token.IsCancellationRequested)
if (token.IsCancellationRequested)
{
return true;
}
Expand Down