1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-07 18:07:28 +08:00

Merge pull request #23048 from peppy/fix-fail-progression-to-results

Fix game incorrectly reaching results screen when saving failed replay on beatmap with video
This commit is contained in:
Dean Herbert 2023-03-31 13:18:40 +09:00 committed by GitHub
commit 8fbb3d50d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 18 deletions

View File

@ -13,6 +13,7 @@ using osu.Framework.Screens;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Configuration;
using osu.Game.Graphics.Containers;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Osu;
@ -106,6 +107,26 @@ namespace osu.Game.Tests.Visual.Gameplay
AddUntilStep("wait for fail overlay", () => Player.FailOverlay.State.Value == Visibility.Visible);
}
[Test]
public void TestSaveFailedReplayWithStoryboardEndedDoesNotProgress()
{
CreateTest(() =>
{
AddStep("fail on first judgement", () => currentFailConditions = (_, _) => true);
AddStep("set storyboard duration to 0s", () => currentStoryboardDuration = 0);
});
AddUntilStep("storyboard ends", () => Player.GameplayClockContainer.CurrentTime >= currentStoryboardDuration);
AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed);
AddUntilStep("wait for fail overlay", () => Player.FailOverlay.State.Value == Visibility.Visible);
AddUntilStep("wait for button clickable", () => Player.ChildrenOfType<SaveFailedScoreButton>().First().ChildrenOfType<OsuClickableContainer>().First().Enabled.Value);
AddStep("click save button", () => Player.ChildrenOfType<SaveFailedScoreButton>().First().ChildrenOfType<OsuClickableContainer>().First().TriggerClick());
// Test a regression where importing the fail replay would cause progression to results screen in a failed state.
AddWaitStep("wait some", 10);
AddAssert("player is still current screen", () => Player.IsCurrentScreen());
}
[Test]
public void TestShowResultsFalse()
{

View File

@ -358,14 +358,10 @@ namespace osu.Game.Screens.Play
ScoreProcessor.RevertResult(r);
};
DimmableStoryboard.HasStoryboardEnded.ValueChanged += storyboardEnded =>
{
if (storyboardEnded.NewValue)
progressToResults(true);
};
DimmableStoryboard.HasStoryboardEnded.ValueChanged += _ => checkScoreCompleted();
// Bind the judgement processors to ourselves
ScoreProcessor.HasCompleted.BindValueChanged(scoreCompletionChanged);
ScoreProcessor.HasCompleted.BindValueChanged(_ => checkScoreCompleted());
HealthProcessor.Failed += onFail;
// Provide judgement processors to mods after they're loaded so that they're on the gameplay clock,
@ -706,19 +702,20 @@ namespace osu.Game.Screens.Play
/// <summary>
/// Handles changes in player state which may progress the completion of gameplay / this screen's lifetime.
/// </summary>
/// <exception cref="InvalidOperationException">Thrown if this method is called more than once without changing state.</exception>
private void scoreCompletionChanged(ValueChangedEvent<bool> completed)
private void checkScoreCompleted()
{
// If this player instance is in the middle of an exit, don't attempt any kind of state update.
if (!this.IsCurrentScreen())
return;
// Special case to handle rewinding post-completion. This is the only way already queued forward progress can be cancelled.
// TODO: Investigate whether this can be moved to a RewindablePlayer subclass or similar.
// Currently, even if this scenario is hit, prepareScoreForDisplay has already been queued (and potentially run).
// In scenarios where rewinding is possible (replay, spectating) this is a non-issue as no submission/import work is done,
// but it still doesn't feel right that this exists here.
if (!completed.NewValue)
// Handle cases of arriving at this method when not in a completed state.
// - When a storyboard completion triggered this call earlier than gameplay finishes.
// - When a replay has been rewound before a queued resultsDisplayDelegate has run.
//
// Currently, even if this scenario is hit, prepareAndImportScoreAsync has already been queued (and potentially run).
// In the scenarios above, this is a non-issue, but it still feels a bit convoluted to have to cancel in this method.
// Maybe this can be improved with further refactoring.
if (!ScoreProcessor.HasCompleted.Value)
{
resultsDisplayDelegate?.Cancel();
resultsDisplayDelegate = null;
@ -742,12 +739,12 @@ namespace osu.Game.Screens.Play
if (!Configuration.ShowResults)
return;
bool storyboardHasOutro = DimmableStoryboard.ContentDisplayed && !DimmableStoryboard.HasStoryboardEnded.Value;
bool storyboardStillRunning = DimmableStoryboard.ContentDisplayed && !DimmableStoryboard.HasStoryboardEnded.Value;
if (storyboardHasOutro)
// If the current beatmap has a storyboard, this method will be called again on storyboard completion.
// Alternatively, the user may press the outro skip button, forcing immediate display of the results screen.
if (storyboardStillRunning)
{
// if the current beatmap has a storyboard, the progression to results will be handled by the storyboard ending
// or the user pressing the skip outro button.
skipOutroOverlay.Show();
return;
}
@ -793,6 +790,8 @@ namespace osu.Game.Screens.Play
// This player instance may already be in the process of exiting.
return;
Debug.Assert(ScoreProcessor.Rank.Value != ScoreRank.F);
this.Push(CreateResults(prepareScoreForDisplayTask.GetResultSafely()));
}, Time.Current + delay, 50);