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 multiplayer scores being submitted as pass even if failed #26384

Merged
merged 5 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 8 additions & 0 deletions osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.Osu.Mods;
using osu.Game.Scoring;
using osu.Game.Screens.OnlinePlay.Components;
using osu.Game.Screens.OnlinePlay.Lounge;
using osu.Game.Screens.OnlinePlay.Lounge.Components;
Expand Down Expand Up @@ -690,6 +691,13 @@ public void TestGameplayFlow()
}

AddUntilStep("wait for results", () => multiplayerComponents.CurrentScreen is ResultsScreen);

AddAssert("check is fail", () =>
{
var scoreInfo = ((ResultsScreen)multiplayerComponents.CurrentScreen).Score;

return !scoreInfo.Passed && scoreInfo.Rank == ScoreRank.F;
});
}

[Test]
Expand Down
3 changes: 0 additions & 3 deletions osu.Game/Rulesets/Mods/ModFlashlight.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ public abstract partial class ModFlashlight<T> : ModFlashlight, IApplicableToDra
public void ApplyToScoreProcessor(ScoreProcessor scoreProcessor)
{
Combo.BindTo(scoreProcessor.Combo);

// Default value of ScoreProcessor's Rank in Flashlight Mod should be SS+
scoreProcessor.Rank.Value = ScoreRank.XH;
}

public ScoreRank AdjustRank(ScoreRank rank, double accuracy)
Expand Down
2 changes: 0 additions & 2 deletions osu.Game/Rulesets/Mods/ModHidden.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ public abstract class ModHidden : ModWithVisibilityAdjustment, IApplicableToScor

public void ApplyToScoreProcessor(ScoreProcessor scoreProcessor)
{
// Default value of ScoreProcessor's Rank in Hidden Mod should be SS+
scoreProcessor.Rank.Value = ScoreRank.XH;
}

public ScoreRank AdjustRank(ScoreRank rank, double accuracy)
Expand Down
17 changes: 11 additions & 6 deletions osu.Game/Rulesets/Scoring/ScoreProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public partial class ScoreProcessor : JudgementProcessor
/// <summary>
/// The current rank.
/// </summary>
public readonly Bindable<ScoreRank> Rank = new Bindable<ScoreRank>(ScoreRank.X);
public IBindable<ScoreRank> Rank => rank;

private readonly Bindable<ScoreRank> rank = new Bindable<ScoreRank>(ScoreRank.X);

/// <summary>
/// The highest combo achieved by this score.
Expand Down Expand Up @@ -186,9 +188,13 @@ public ScoreProcessor(Ruleset ruleset)
Combo.ValueChanged += combo => HighestCombo.Value = Math.Max(HighestCombo.Value, combo.NewValue);
Accuracy.ValueChanged += accuracy =>
{
Rank.Value = RankFromAccuracy(accuracy.NewValue);
// Once failed, we shouldn't update the rank anymore.
if (rank.Value == ScoreRank.F)
return;

rank.Value = RankFromAccuracy(accuracy.NewValue);
foreach (var mod in Mods.Value.OfType<IApplicableToScoreProcessor>())
Rank.Value = mod.AdjustRank(Rank.Value, accuracy.NewValue);
rank.Value = mod.AdjustRank(Rank.Value, accuracy.NewValue);
};

Mods.ValueChanged += mods =>
Expand Down Expand Up @@ -411,8 +417,7 @@ protected override void Reset(bool storeResults)
TotalScore.Value = 0;
Accuracy.Value = 1;
Combo.Value = 0;
Rank.Disabled = false;
Rank.Value = ScoreRank.X;
rank.Value = ScoreRank.X;
HighestCombo.Value = 0;
}

Expand Down Expand Up @@ -448,7 +453,7 @@ public void FailScore(ScoreInfo score)
return;

score.Passed = false;
Rank.Value = ScoreRank.F;
rank.Value = ScoreRank.F;

PopulateScore(score);
}
Expand Down
4 changes: 1 addition & 3 deletions osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ public partial class MultiplayerPlayer : RoomSubmittingPlayer
{
protected override bool PauseOnFocusLost => false;

// Disallow fails in multiplayer for now.
protected override bool CheckModsAllowFailure() => false;

protected override UserActivity InitialActivity => new UserActivity.InMultiplayerGame(Beatmap.Value.BeatmapInfo, Ruleset.Value);

[Resolved]
Expand All @@ -55,6 +52,7 @@ public MultiplayerPlayer(Room room, PlaylistItem playlistItem, MultiplayerRoomUs
{
AllowPause = false,
AllowRestart = false,
AllowFailAnimation = false,
AllowSkipping = room.AutoSkip.Value,
AutomaticallySkipIntro = room.AutoSkip.Value,
AlwaysShowLeaderboard = true,
Expand Down
4 changes: 3 additions & 1 deletion osu.Game/Screens/Play/BreakOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
#nullable disable

using System.Collections.Generic;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Graphics.Shapes;
using osu.Framework.Graphics.UserInterface;
using osu.Game.Beatmaps.Timing;
using osu.Game.Rulesets.Scoring;
using osu.Game.Scoring;
using osu.Game.Screens.Play.Break;

namespace osu.Game.Screens.Play
Expand Down Expand Up @@ -113,7 +115,7 @@ protected override void LoadComplete()
if (scoreProcessor != null)
{
info.AccuracyDisplay.Current.BindTo(scoreProcessor.Accuracy);
info.GradeDisplay.Current.BindTo(scoreProcessor.Rank);
((IBindable<ScoreRank>)info.GradeDisplay.Current).BindTo(scoreProcessor.Rank);
}
}

Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Screens/Play/GameplayState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class GameplayState
public bool HasPassed { get; set; }

/// <summary>
/// Whether the user failed during gameplay.
/// Whether the user failed during gameplay. This is only set when the gameplay session has completed due to the fail.
/// </summary>
public bool HasFailed { get; set; }

Expand Down
61 changes: 33 additions & 28 deletions osu.Game/Screens/Play/Player.cs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ private void checkScoreCompleted()
}

// Only show the completion screen if the player hasn't failed
if (HealthProcessor.HasFailed)
if (GameplayState.HasFailed)
return;

GameplayState.HasPassed = true;
Expand Down Expand Up @@ -801,8 +801,6 @@ private void progressToResults(bool withDelay)
// 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);

Expand Down Expand Up @@ -924,37 +922,44 @@ private bool onFail()
if (!CheckModsAllowFailure())
return false;

Debug.Assert(!GameplayState.HasFailed);
Debug.Assert(!GameplayState.HasPassed);
Debug.Assert(!GameplayState.HasQuit);

GameplayState.HasFailed = true;
if (Configuration.AllowFailAnimation)
{
Debug.Assert(!GameplayState.HasFailed);
Debug.Assert(!GameplayState.HasPassed);
Debug.Assert(!GameplayState.HasQuit);

updateGameplayState();
GameplayState.HasFailed = true;

// There is a chance that we could be in a paused state as the ruleset's internal clock (see FrameStabilityContainer)
// could process an extra frame after the GameplayClock is stopped.
// In such cases we want the fail state to precede a user triggered pause.
if (PauseOverlay.State.Value == Visibility.Visible)
PauseOverlay.Hide();
updateGameplayState();

failAnimationContainer.Start();
// There is a chance that we could be in a paused state as the ruleset's internal clock (see FrameStabilityContainer)
// could process an extra frame after the GameplayClock is stopped.
// In such cases we want the fail state to precede a user triggered pause.
if (PauseOverlay.State.Value == Visibility.Visible)
PauseOverlay.Hide();

failAnimationContainer.Start();

// Failures can be triggered either by a judgement, or by a mod.
//
// For the case of a judgement, due to ordering considerations, ScoreProcessor will not have received
// the final judgement which triggered the failure yet (see DrawableRuleset.NewResult handling above).
//
// A schedule here ensures that any lingering judgements from the current frame are applied before we
// finalise the score as "failed".
Schedule(() =>
{
ScoreProcessor.FailScore(Score.ScoreInfo);
OnFail();

// Failures can be triggered either by a judgement, or by a mod.
//
// For the case of a judgement, due to ordering considerations, ScoreProcessor will not have received
// the final judgement which triggered the failure yet (see DrawableRuleset.NewResult handling above).
//
// A schedule here ensures that any lingering judgements from the current frame are applied before we
// finalise the score as "failed".
Schedule(() =>
if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
Restart(true);
});
}
else
{
ScoreProcessor.FailScore(Score.ScoreInfo);
OnFail();

if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
Restart(true);
});
}

return true;
}
Expand Down
6 changes: 6 additions & 0 deletions osu.Game/Screens/Play/PlayerConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ public class PlayerConfiguration
/// </summary>
public bool ShowResults { get; set; } = true;

/// <summary>
/// Whether the fail animation / screen should be triggered on failing.
/// If false, the score will still be marked as failed but gameplay will continue.
/// </summary>
public bool AllowFailAnimation { get; set; } = true;

/// <summary>
/// Whether the player should be allowed to trigger a restart.
/// </summary>
Expand Down
Loading