From 502e6af008757204e53f04eb5cbfe7f586558989 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 26 Jan 2022 00:21:54 +0900 Subject: [PATCH 01/42] Remove PlayingUsers list from SpectatorClient --- .../Gameplay/TestSceneSpectatorPlayback.cs | 38 ------------------- ...TestSceneMultiplayerGameplayLeaderboard.cs | 2 +- osu.Game/Online/Spectator/SpectatorClient.cs | 10 ----- .../Dashboard/CurrentlyPlayingDisplay.cs | 30 +++++++-------- .../Visual/Spectator/TestSpectatorClient.cs | 7 ++-- 5 files changed, 19 insertions(+), 68 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs index 90abdf2ba3..75369661a0 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs @@ -3,12 +3,9 @@ using System; using System.Collections.Generic; -using System.Collections.Specialized; -using System.Diagnostics; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; @@ -20,7 +17,6 @@ using osu.Framework.Testing; using osu.Framework.Timing; using osu.Game.Beatmaps; using osu.Game.Graphics.Sprites; -using osu.Game.Online.API; using osu.Game.Online.Spectator; using osu.Game.Replays; using osu.Game.Replays.Legacy; @@ -47,8 +43,6 @@ namespace osu.Game.Tests.Visual.Gameplay private Replay replay; - private readonly IBindableList users = new BindableList(); - private TestReplayRecorder recorder; private ManualClock manualClock; @@ -57,9 +51,6 @@ namespace osu.Game.Tests.Visual.Gameplay private TestFramedReplayInputHandler replayHandler; - [Resolved] - private IAPIProvider api { get; set; } - [Resolved] private SpectatorClient spectatorClient { get; set; } @@ -78,35 +69,6 @@ namespace osu.Game.Tests.Visual.Gameplay spectatorClient.OnNewFrames += onNewFrames; - users.BindTo(spectatorClient.PlayingUsers); - users.BindCollectionChanged((obj, args) => - { - switch (args.Action) - { - case NotifyCollectionChangedAction.Add: - Debug.Assert(args.NewItems != null); - - foreach (int user in args.NewItems) - { - if (user == api.LocalUser.Value.Id) - spectatorClient.WatchUser(user); - } - - break; - - case NotifyCollectionChangedAction.Remove: - Debug.Assert(args.OldItems != null); - - foreach (int user in args.OldItems) - { - if (user == api.LocalUser.Value.Id) - spectatorClient.StopWatchingUser(user); - } - - break; - } - }, true); - Children = new Drawable[] { new GridContainer diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs index 9b8e67b07a..8b71dfac21 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs @@ -115,7 +115,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public void RandomlyUpdateState() { - foreach (int userId in PlayingUsers) + foreach ((int userId, _) in PlayingUserStates) { if (RNG.NextBool()) continue; diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index fddb94fad7..c3e70edcd3 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -37,9 +37,6 @@ namespace osu.Game.Online.Spectator private readonly List watchingUsers = new List(); - public IBindableList PlayingUsers => playingUsers; - private readonly BindableList playingUsers = new BindableList(); - public IBindableDictionary PlayingUserStates => playingUserStates; private readonly BindableDictionary playingUserStates = new BindableDictionary(); @@ -88,10 +85,7 @@ namespace osu.Game.Online.Spectator BeginPlayingInternal(currentState); } else - { - playingUsers.Clear(); playingUserStates.Clear(); - } }), true); } @@ -99,9 +93,6 @@ namespace osu.Game.Online.Spectator { Schedule(() => { - if (!playingUsers.Contains(userId)) - playingUsers.Add(userId); - // UserBeganPlaying() is called by the server regardless of whether the local user is watching the remote user, and is called a further time when the remote user is watched. // This may be a temporary thing (see: https://github.com/ppy/osu-server-spectator/blob/2273778e02cfdb4a9c6a934f2a46a8459cb5d29c/osu.Server.Spectator/Hubs/SpectatorHub.cs#L28-L29). // We don't want the user states to update unless the player is being watched, otherwise calling BindUserBeganPlaying() can lead to double invocations. @@ -118,7 +109,6 @@ namespace osu.Game.Online.Spectator { Schedule(() => { - playingUsers.Remove(userId); playingUserStates.Remove(userId); OnUserFinishedPlaying?.Invoke(userId, state); diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index fde20575fc..afbfb9f7ef 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Collections.Specialized; +using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -22,7 +22,7 @@ namespace osu.Game.Overlays.Dashboard { internal class CurrentlyPlayingDisplay : CompositeDrawable { - private readonly IBindableList playingUsers = new BindableList(); + private readonly IBindableDictionary playingUserStates = new BindableDictionary(); private FillFlowContainer userFlow; @@ -51,18 +51,20 @@ namespace osu.Game.Overlays.Dashboard { base.LoadComplete(); - playingUsers.BindTo(spectatorClient.PlayingUsers); - playingUsers.BindCollectionChanged(onUsersChanged, true); + playingUserStates.BindTo(spectatorClient.PlayingUserStates); + playingUserStates.BindCollectionChanged(onUsersChanged, true); } - private void onUsersChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() => + private void onUsersChanged(object sender, NotifyDictionaryChangedEventArgs e) => Schedule(() => { switch (e.Action) { - case NotifyCollectionChangedAction.Add: - foreach (int id in e.NewItems.OfType().ToArray()) + case NotifyDictionaryChangedAction.Add: + Debug.Assert(e.NewItems != null); + + foreach ((int userId, _) in e.NewItems) { - users.GetUserAsync(id).ContinueWith(task => + users.GetUserAsync(userId).ContinueWith(task => { var user = task.GetResultSafely(); @@ -71,7 +73,7 @@ namespace osu.Game.Overlays.Dashboard Schedule(() => { // user may no longer be playing. - if (!playingUsers.Contains(user.Id)) + if (!playingUserStates.ContainsKey(user.Id)) return; userFlow.Add(createUserPanel(user)); @@ -81,13 +83,11 @@ namespace osu.Game.Overlays.Dashboard break; - case NotifyCollectionChangedAction.Remove: - foreach (int u in e.OldItems.OfType()) - userFlow.FirstOrDefault(card => card.User.Id == u)?.Expire(); - break; + case NotifyDictionaryChangedAction.Remove: + Debug.Assert(e.OldItems != null); - case NotifyCollectionChangedAction.Reset: - userFlow.Clear(); + foreach ((int userId, _) in e.OldItems) + userFlow.FirstOrDefault(card => card.User.Id == userId)?.Expire(); break; } }); diff --git a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs index 1a1d493249..7848a825f4 100644 --- a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs +++ b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs @@ -6,7 +6,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -42,7 +41,7 @@ namespace osu.Game.Tests.Visual.Spectator { OnNewFrames += (i, bundle) => { - if (PlayingUsers.Contains(i)) + if (PlayingUserStates.ContainsKey(i)) lastReceivedUserFrames[i] = bundle.Frames[^1]; }; } @@ -65,7 +64,7 @@ namespace osu.Game.Tests.Visual.Spectator /// The user to end play for. public void EndPlay(int userId) { - if (!PlayingUsers.Contains(userId)) + if (!PlayingUserStates.ContainsKey(userId)) return; ((ISpectatorClient)this).UserFinishedPlaying(userId, new SpectatorState @@ -131,7 +130,7 @@ namespace osu.Game.Tests.Visual.Spectator protected override Task WatchUserInternal(int userId) { // When newly watching a user, the server sends the playing state immediately. - if (PlayingUsers.Contains(userId)) + if (PlayingUserStates.ContainsKey(userId)) sendPlayingState(userId); return Task.CompletedTask; From 781cb9f18d55e21b153967ee389c1a45ca934e0a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 26 Jan 2022 01:45:11 +0900 Subject: [PATCH 02/42] Move HasPassed/HasFailed into GameplayState --- .../TestSceneTaikoSuddenDeath.cs | 2 +- .../Visual/Gameplay/TestSceneFailAnimation.cs | 2 +- .../Visual/Gameplay/TestSceneFailJudgement.cs | 2 +- .../Visual/Gameplay/TestScenePause.cs | 12 ++++++------ .../TestScenePlayerScoreSubmission.cs | 4 ++-- .../Gameplay/TestSceneStoryboardWithOutro.cs | 2 +- .../Navigation/TestSceneScreenNavigation.cs | 2 +- .../Multiplayer/MultiplayerPlayerLoader.cs | 2 +- osu.Game/Screens/Play/GameplayState.cs | 10 ++++++++++ osu.Game/Screens/Play/Player.cs | 19 ++++++------------- 10 files changed, 30 insertions(+), 27 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoSuddenDeath.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoSuddenDeath.cs index 0be005e1c4..eec88d7bf8 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoSuddenDeath.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoSuddenDeath.cs @@ -45,7 +45,7 @@ namespace osu.Game.Rulesets.Taiko.Tests Player.ScoreProcessor.NewJudgement += b => judged = true; }); AddUntilStep("swell judged", () => judged); - AddAssert("failed", () => Player.HasFailed); + AddAssert("failed", () => Player.GameplayState.HasFailed); } } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs index 7167d3120a..744227c55e 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs @@ -29,7 +29,7 @@ namespace osu.Game.Tests.Visual.Gameplay protected override void AddCheckSteps() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddUntilStep("wait for fail overlay", () => ((FailPlayer)Player).FailOverlay.State.Value == Visibility.Visible); // The pause screen and fail animation both ramp frequency. diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneFailJudgement.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFailJudgement.cs index fa27e1abdd..6430c29dfa 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFailJudgement.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFailJudgement.cs @@ -21,7 +21,7 @@ namespace osu.Game.Tests.Visual.Gameplay protected override void AddCheckSteps() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddUntilStep("wait for multiple judgements", () => ((FailPlayer)Player).ScoreProcessor.JudgedHits > 1); AddAssert("total number of results == 1", () => { diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 04676f656f..ea0255ab76 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -185,7 +185,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestPauseAfterFail() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddUntilStep("fail overlay shown", () => Player.FailOverlayVisible); confirmClockRunning(false); @@ -201,7 +201,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestExitFromFailedGameplayAfterFailAnimation() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddUntilStep("wait for fail overlay shown", () => Player.FailOverlayVisible); confirmClockRunning(false); @@ -213,7 +213,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestExitFromFailedGameplayDuringFailAnimation() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); // will finish the fail animation and show the fail/pause screen. AddStep("attempt exit via pause key", () => Player.ExitViaPause()); @@ -227,7 +227,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestQuickRetryFromFailedGameplay() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddStep("quick retry", () => Player.GameplayClockContainer.ChildrenOfType().First().Action?.Invoke()); confirmExited(); @@ -236,7 +236,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestQuickExitFromFailedGameplay() { - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddStep("quick exit", () => Player.GameplayClockContainer.ChildrenOfType().First().Action?.Invoke()); confirmExited(); @@ -341,7 +341,7 @@ namespace osu.Game.Tests.Visual.Gameplay { confirmClockRunning(false); confirmNotExited(); - AddAssert("player not failed", () => !Player.HasFailed); + AddAssert("player not failed", () => !Player.GameplayState.HasFailed); AddAssert("pause overlay shown", () => Player.PauseOverlayVisible); } diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs index a9675a2ee2..58b5df2612 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs @@ -159,7 +159,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddUntilStep("wait for token request", () => Player.TokenCreationRequested); - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddStep("exit", () => Player.Exit()); AddAssert("ensure no submission", () => Player.SubmittedScore == null); @@ -176,7 +176,7 @@ namespace osu.Game.Tests.Visual.Gameplay addFakeHit(); - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddStep("exit", () => Player.Exit()); AddAssert("ensure failing submission", () => Player.SubmittedScore?.ScoreInfo.Passed == false); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs index 69798dcb82..b87183cbc7 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs @@ -96,7 +96,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("set storyboard duration to 0.6s", () => currentStoryboardDuration = 600); }); - AddUntilStep("wait for fail", () => Player.HasFailed); + AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed); AddUntilStep("storyboard ends", () => Player.GameplayClockContainer.GameplayClock.CurrentTime >= currentStoryboardDuration); AddUntilStep("wait for fail overlay", () => Player.FailOverlay.State.Value == Visibility.Visible); } diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index ed484e03f6..63984a1bed 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -209,7 +209,7 @@ namespace osu.Game.Tests.Visual.Navigation return (player = Game.ScreenStack.CurrentScreen as Player) != null; }); - AddUntilStep("wait for fail", () => player.HasFailed); + AddUntilStep("wait for fail", () => player.GameplayState.HasFailed); AddUntilStep("wait for track stop", () => !Game.MusicController.IsPlaying); AddAssert("Ensure time before preview point", () => Game.MusicController.CurrentTrack.CurrentTime < beatmap().BeatmapInfo.Metadata.PreviewTime); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs index 470ba59a76..772651727e 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs @@ -9,7 +9,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { public class MultiplayerPlayerLoader : PlayerLoader { - public bool GameplayPassed => player?.GameplayPassed == true; + public bool GameplayPassed => player?.GameplayState.HasPassed == true; private Player player; diff --git a/osu.Game/Screens/Play/GameplayState.cs b/osu.Game/Screens/Play/GameplayState.cs index 83881f739d..60fbfa6644 100644 --- a/osu.Game/Screens/Play/GameplayState.cs +++ b/osu.Game/Screens/Play/GameplayState.cs @@ -39,6 +39,16 @@ namespace osu.Game.Screens.Play /// public readonly Score Score; + /// + /// Whether gameplay completed without the user failing. + /// + public bool HasPassed { get; set; } + + /// + /// Whether the user failed during gameplay. + /// + public bool HasFailed { get; set; } + /// /// A bindable tracking the last judgement result applied to any hit object. /// diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 0312789b12..814aacd2fa 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -72,15 +72,8 @@ namespace osu.Game.Screens.Play /// protected virtual bool PauseOnFocusLost => true; - /// - /// Whether gameplay has completed without the user having failed. - /// - public bool GameplayPassed { get; private set; } - public Action RestartRequested; - public bool HasFailed { get; private set; } - private Bindable mouseWheelDisabled; private readonly Bindable storyboardReplacesBackground = new Bindable(); @@ -559,7 +552,7 @@ namespace osu.Game.Screens.Play if (showDialogFirst && !pauseOrFailDialogVisible) { // if the fail animation is currently in progress, accelerate it (it will show the pause dialog on completion). - if (ValidForResume && HasFailed) + if (ValidForResume && GameplayState.HasFailed) { failAnimationLayer.FinishTransforms(true); return; @@ -678,7 +671,7 @@ namespace osu.Game.Screens.Play resultsDisplayDelegate?.Cancel(); resultsDisplayDelegate = null; - GameplayPassed = false; + GameplayState.HasPassed = false; ValidForResume = true; skipOutroOverlay.Hide(); return; @@ -688,7 +681,7 @@ namespace osu.Game.Screens.Play if (HealthProcessor.HasFailed) return; - GameplayPassed = true; + GameplayState.HasPassed = true; // Setting this early in the process means that even if something were to go wrong in the order of events following, there // is no chance that a user could return to the (already completed) Player instance from a child screen. @@ -804,7 +797,7 @@ namespace osu.Game.Screens.Play if (!CheckModsAllowFailure()) return false; - HasFailed = true; + GameplayState.HasFailed = true; Score.ScoreInfo.Passed = false; // There is a chance that we could be in a paused state as the ruleset's internal clock (see FrameStabilityContainer) @@ -859,13 +852,13 @@ namespace osu.Game.Screens.Play // replays cannot be paused and exit immediately && !DrawableRuleset.HasReplayLoaded.Value // cannot pause if we are already in a fail state - && !HasFailed; + && !GameplayState.HasFailed; private bool canResume => // cannot resume from a non-paused state GameplayClockContainer.IsPaused.Value // cannot resume if we are already in a fail state - && !HasFailed + && !GameplayState.HasFailed // already resuming && !IsResuming; From 38e075c522f72371742e4d02a948f7a53a0aac5d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 26 Jan 2022 02:02:31 +0900 Subject: [PATCH 03/42] Add HasQuit gameplay state --- osu.Game/Screens/Play/GameplayState.cs | 5 +++++ osu.Game/Screens/Play/Player.cs | 3 +++ 2 files changed, 8 insertions(+) diff --git a/osu.Game/Screens/Play/GameplayState.cs b/osu.Game/Screens/Play/GameplayState.cs index 60fbfa6644..64e873b3bb 100644 --- a/osu.Game/Screens/Play/GameplayState.cs +++ b/osu.Game/Screens/Play/GameplayState.cs @@ -49,6 +49,11 @@ namespace osu.Game.Screens.Play /// public bool HasFailed { get; set; } + /// + /// Whether the user quit gameplay without either having either passed or failed. + /// + public bool HasQuit { get; set; } + /// /// A bindable tracking the last judgement result applied to any hit object. /// diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 814aacd2fa..f6a2310826 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -983,6 +983,9 @@ namespace osu.Game.Screens.Play public override bool OnExiting(IScreen next) { + if (!GameplayState.HasPassed && !GameplayState.HasFailed) + GameplayState.HasQuit = true; + screenSuspension?.RemoveAndDisposeImmediately(); failAnimationLayer?.RemoveFilters(); From 41007169f722febe2217ff805cf95cdb6d81c3dd Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 1 Feb 2022 15:51:41 +0900 Subject: [PATCH 04/42] Give SpectatorState a user state --- .../Visual/Gameplay/TestSceneSpectator.cs | 2 +- .../Online/Spectator/SpectatingUserState.cs | 33 +++++++++++++++++++ osu.Game/Online/Spectator/SpectatorClient.cs | 10 +++++- osu.Game/Online/Spectator/SpectatorState.cs | 7 ++-- osu.Game/Rulesets/UI/ReplayRecorder.cs | 4 ++- osu.Game/Screens/Play/Player.cs | 2 +- 6 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 osu.Game/Online/Spectator/SpectatingUserState.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 917b3c89a8..86d6de6975 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -211,7 +211,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("send frames and finish play", () => { spectatorClient.HandleFrame(new OsuReplayFrame(1000, Vector2.Zero)); - spectatorClient.EndPlaying(); + spectatorClient.EndPlaying(new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()) { HasPassed = true }); }); // We can't access API because we're an "online" test. diff --git a/osu.Game/Online/Spectator/SpectatingUserState.cs b/osu.Game/Online/Spectator/SpectatingUserState.cs new file mode 100644 index 0000000000..c7ba4ba248 --- /dev/null +++ b/osu.Game/Online/Spectator/SpectatingUserState.cs @@ -0,0 +1,33 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Online.Spectator +{ + public enum SpectatingUserState + { + /// + /// The spectated user has not yet played. + /// + Idle, + + /// + /// The spectated user is currently playing. + /// + Playing, + + /// + /// The spectated user has successfully completed gameplay. + /// + Completed, + + /// + /// The spectator user has failed during gameplay. + /// + Failed, + + /// + /// The spectated user has quit during gameplay. + /// + Quit + } +} diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index c3e70edcd3..f4161e1db9 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -138,6 +138,7 @@ namespace osu.Game.Online.Spectator currentState.BeatmapID = score.ScoreInfo.BeatmapInfo.OnlineID; currentState.RulesetID = score.ScoreInfo.RulesetID; currentState.Mods = score.ScoreInfo.Mods.Select(m => new APIMod(m)).ToArray(); + currentState.State = SpectatingUserState.Playing; currentBeatmap = state.Beatmap; currentScore = score; @@ -148,7 +149,7 @@ namespace osu.Game.Online.Spectator public void SendFrames(FrameDataBundle data) => lastSend = SendFramesInternal(data); - public void EndPlaying() + public void EndPlaying(GameplayState state) { // This method is most commonly called via Dispose(), which is can be asynchronous (via the AsyncDisposalQueue). // We probably need to find a better way to handle this... @@ -163,6 +164,13 @@ namespace osu.Game.Online.Spectator IsPlaying = false; currentBeatmap = null; + if (state.HasPassed) + currentState.State = SpectatingUserState.Completed; + else if (state.HasFailed) + currentState.State = SpectatingUserState.Failed; + else + currentState.State = SpectatingUserState.Quit; + EndPlayingInternal(currentState); }); } diff --git a/osu.Game/Online/Spectator/SpectatorState.cs b/osu.Game/Online/Spectator/SpectatorState.cs index ebb91e4dd2..fc62f16bba 100644 --- a/osu.Game/Online/Spectator/SpectatorState.cs +++ b/osu.Game/Online/Spectator/SpectatorState.cs @@ -24,14 +24,17 @@ namespace osu.Game.Online.Spectator [Key(2)] public IEnumerable Mods { get; set; } = Enumerable.Empty(); + [Key(3)] + public SpectatingUserState State { get; set; } + public bool Equals(SpectatorState other) { if (ReferenceEquals(null, other)) return false; if (ReferenceEquals(this, other)) return true; - return BeatmapID == other.BeatmapID && Mods.SequenceEqual(other.Mods) && RulesetID == other.RulesetID; + return BeatmapID == other.BeatmapID && Mods.SequenceEqual(other.Mods) && RulesetID == other.RulesetID && State == other.State; } - public override string ToString() => $"Beatmap:{BeatmapID} Mods:{string.Join(',', Mods)} Ruleset:{RulesetID}"; + public override string ToString() => $"Beatmap:{BeatmapID} Mods:{string.Join(',', Mods)} Ruleset:{RulesetID} State:{State}"; } } diff --git a/osu.Game/Rulesets/UI/ReplayRecorder.cs b/osu.Game/Rulesets/UI/ReplayRecorder.cs index 976f95cef8..277040b2a6 100644 --- a/osu.Game/Rulesets/UI/ReplayRecorder.cs +++ b/osu.Game/Rulesets/UI/ReplayRecorder.cs @@ -55,7 +55,9 @@ namespace osu.Game.Rulesets.UI protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - spectatorClient?.EndPlaying(); + + if (spectatorClient != null && gameplayState != null) + spectatorClient.EndPlaying(gameplayState); } protected override void Update() diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index f6a2310826..1aa6cded48 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1000,7 +1000,7 @@ namespace osu.Game.Screens.Play // EndPlaying() is typically called from ReplayRecorder.Dispose(). Disposal is currently asynchronous. // To resolve test failures, forcefully end playing synchronously when this screen exits. // Todo: Replace this with a more permanent solution once osu-framework has a synchronous cleanup method. - spectatorClient.EndPlaying(); + spectatorClient.EndPlaying(GameplayState); // GameplayClockContainer performs seeks / start / stop operations on the beatmap's track. // as we are no longer the current screen, we cannot guarantee the track is still usable. From f4210f7a302a3bad2130181800106cdcefeb2020 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 26 Jan 2022 22:21:07 +0900 Subject: [PATCH 05/42] Rework spectator components to use new user state --- osu.Game/Online/Spectator/SpectatorClient.cs | 8 +--- .../Dashboard/CurrentlyPlayingDisplay.cs | 42 ++++++++++++------- osu.Game/Screens/Spectate/SpectatorScreen.cs | 33 ++++++++------- 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index f4161e1db9..fb7526347b 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -93,12 +93,8 @@ namespace osu.Game.Online.Spectator { Schedule(() => { - // UserBeganPlaying() is called by the server regardless of whether the local user is watching the remote user, and is called a further time when the remote user is watched. - // This may be a temporary thing (see: https://github.com/ppy/osu-server-spectator/blob/2273778e02cfdb4a9c6a934f2a46a8459cb5d29c/osu.Server.Spectator/Hubs/SpectatorHub.cs#L28-L29). - // We don't want the user states to update unless the player is being watched, otherwise calling BindUserBeganPlaying() can lead to double invocations. if (watchingUsers.Contains(userId)) playingUserStates[userId] = state; - OnUserBeganPlaying?.Invoke(userId, state); }); @@ -109,8 +105,8 @@ namespace osu.Game.Online.Spectator { Schedule(() => { - playingUserStates.Remove(userId); - + if (watchingUsers.Contains(userId)) + playingUserStates[userId] = state; OnUserFinishedPlaying?.Invoke(userId, state); }); diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index afbfb9f7ef..383f17d8d2 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -22,7 +22,7 @@ namespace osu.Game.Overlays.Dashboard { internal class CurrentlyPlayingDisplay : CompositeDrawable { - private readonly IBindableDictionary playingUserStates = new BindableDictionary(); + private readonly IBindableDictionary userStates = new BindableDictionary(); private FillFlowContainer userFlow; @@ -51,33 +51,32 @@ namespace osu.Game.Overlays.Dashboard { base.LoadComplete(); - playingUserStates.BindTo(spectatorClient.PlayingUserStates); - playingUserStates.BindCollectionChanged(onUsersChanged, true); + userStates.BindTo(spectatorClient.PlayingUserStates); + userStates.BindCollectionChanged(onUserStatesChanged, true); } - private void onUsersChanged(object sender, NotifyDictionaryChangedEventArgs e) => Schedule(() => + private void onUserStatesChanged(object sender, NotifyDictionaryChangedEventArgs e) => Schedule(() => { switch (e.Action) { case NotifyDictionaryChangedAction.Add: + case NotifyDictionaryChangedAction.Replace: Debug.Assert(e.NewItems != null); - foreach ((int userId, _) in e.NewItems) + foreach ((int userId, SpectatorState state) in e.NewItems) { + if (state.State != SpectatingUserState.Playing) + { + removePlayingUser(userId); + continue; + } + users.GetUserAsync(userId).ContinueWith(task => { var user = task.GetResultSafely(); - if (user == null) return; - - Schedule(() => - { - // user may no longer be playing. - if (!playingUserStates.ContainsKey(user.Id)) - return; - - userFlow.Add(createUserPanel(user)); - }); + if (user != null) + Schedule(() => addPlayingUser(user)); }); } @@ -87,9 +86,20 @@ namespace osu.Game.Overlays.Dashboard Debug.Assert(e.OldItems != null); foreach ((int userId, _) in e.OldItems) - userFlow.FirstOrDefault(card => card.User.Id == userId)?.Expire(); + removePlayingUser(userId); break; } + + void addPlayingUser(APIUser user) + { + // user may no longer be playing. + if (!userStates.TryGetValue(user.Id, out var state2) || state2.State != SpectatingUserState.Playing) + return; + + userFlow.Add(createUserPanel(user)); + } + + void removePlayingUser(int userId) => userFlow.FirstOrDefault(card => card.User.Id == userId)?.Expire(); }); private PlayingUserPanel createUserPanel(APIUser user) => diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 3cf9f79611..783ba494eb 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -42,7 +42,7 @@ namespace osu.Game.Screens.Spectate [Resolved] private UserLookupCache userLookupCache { get; set; } - private readonly IBindableDictionary playingUserStates = new BindableDictionary(); + private readonly IBindableDictionary userStates = new BindableDictionary(); private readonly Dictionary userMap = new Dictionary(); private readonly Dictionary gameplayStates = new Dictionary(); @@ -77,8 +77,8 @@ namespace osu.Game.Screens.Spectate userMap[u.Id] = u; } - playingUserStates.BindTo(spectatorClient.PlayingUserStates); - playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true); + userStates.BindTo(spectatorClient.PlayingUserStates); + userStates.BindCollectionChanged(onUserStatesChanged, true); realmSubscription = realm.RegisterForNotifications( realm => realm.All().Where(s => !s.DeletePending), beatmapsChanged); @@ -99,7 +99,7 @@ namespace osu.Game.Screens.Spectate { foreach ((int userId, _) in userMap) { - if (!playingUserStates.TryGetValue(userId, out var userState)) + if (!userStates.TryGetValue(userId, out var userState)) continue; if (beatmapSet.Beatmaps.Any(b => b.OnlineID == userState.BeatmapID)) @@ -107,40 +107,41 @@ namespace osu.Game.Screens.Spectate } } - private void onPlayingUserStatesChanged(object sender, NotifyDictionaryChangedEventArgs e) + private void onUserStatesChanged(object sender, NotifyDictionaryChangedEventArgs e) { switch (e.Action) { case NotifyDictionaryChangedAction.Add: foreach ((int userId, var state) in e.NewItems.AsNonNull()) - onUserStateAdded(userId, state); + onUserStateChanged(userId, state); break; case NotifyDictionaryChangedAction.Remove: - foreach ((int userId, var _) in e.OldItems.AsNonNull()) + foreach ((int userId, _) in e.OldItems.AsNonNull()) onUserStateRemoved(userId); break; case NotifyDictionaryChangedAction.Replace: - foreach ((int userId, var _) in e.OldItems.AsNonNull()) - onUserStateRemoved(userId); - foreach ((int userId, var state) in e.NewItems.AsNonNull()) - onUserStateAdded(userId, state); + onUserStateChanged(userId, state); break; } } - private void onUserStateAdded(int userId, SpectatorState state) + private void onUserStateChanged(int userId, SpectatorState newState) { - if (state.RulesetID == null || state.BeatmapID == null) + if (newState.RulesetID == null || newState.BeatmapID == null) return; if (!userMap.ContainsKey(userId)) return; - Schedule(() => OnUserStateChanged(userId, state)); - updateGameplayState(userId); + // Do nothing for failed/completed states. + if (newState.State == SpectatingUserState.Playing) + { + Schedule(() => OnUserStateChanged(userId, newState)); + updateGameplayState(userId); + } } private void onUserStateRemoved(int userId) @@ -162,7 +163,7 @@ namespace osu.Game.Screens.Spectate Debug.Assert(userMap.ContainsKey(userId)); var user = userMap[userId]; - var spectatorState = playingUserStates[userId]; + var spectatorState = userStates[userId]; var resolvedRuleset = rulesets.AvailableRulesets.FirstOrDefault(r => r.OnlineID == spectatorState.RulesetID)?.CreateInstance(); if (resolvedRuleset == null) From 9d1d13c7152592d31a212fc04b04aac1fac171b1 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 2 Feb 2022 23:00:43 +0900 Subject: [PATCH 06/42] Fix up TestSpectatorClient implementation Rather than using a list which is supposed to be updated "client"-side, now uses the "server"-side list. --- .../Visual/Spectator/TestSpectatorClient.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs index 7848a825f4..6862cda88c 100644 --- a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs +++ b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs @@ -39,11 +39,7 @@ namespace osu.Game.Tests.Visual.Spectator public TestSpectatorClient() { - OnNewFrames += (i, bundle) => - { - if (PlayingUserStates.ContainsKey(i)) - lastReceivedUserFrames[i] = bundle.Frames[^1]; - }; + OnNewFrames += (i, bundle) => lastReceivedUserFrames[i] = bundle.Frames[^1]; } /// @@ -62,16 +58,20 @@ namespace osu.Game.Tests.Visual.Spectator /// Ends play for an arbitrary user. /// /// The user to end play for. - public void EndPlay(int userId) + /// The spectator state to end play with. + public void EndPlay(int userId, SpectatingUserState state = SpectatingUserState.Quit) { - if (!PlayingUserStates.ContainsKey(userId)) + if (!userBeatmapDictionary.ContainsKey(userId)) return; ((ISpectatorClient)this).UserFinishedPlaying(userId, new SpectatorState { BeatmapID = userBeatmapDictionary[userId], RulesetID = 0, + State = state }); + + userBeatmapDictionary.Remove(userId); } public new void Schedule(Action action) => base.Schedule(action); @@ -130,7 +130,7 @@ namespace osu.Game.Tests.Visual.Spectator protected override Task WatchUserInternal(int userId) { // When newly watching a user, the server sends the playing state immediately. - if (PlayingUserStates.ContainsKey(userId)) + if (userBeatmapDictionary.ContainsKey(userId)) sendPlayingState(userId); return Task.CompletedTask; @@ -144,6 +144,7 @@ namespace osu.Game.Tests.Visual.Spectator { BeatmapID = userBeatmapDictionary[userId], RulesetID = 0, + State = SpectatingUserState.Playing }); } } From 589f5e7a31f53275f7cd001adbe4deedb1e18473 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 2 Feb 2022 23:09:38 +0900 Subject: [PATCH 07/42] Update test which has now been resolved --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index d28e5203e3..9f8470446c 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -155,11 +155,12 @@ namespace osu.Game.Tests.Visual.Gameplay waitForPlayer(); checkPaused(true); + sendFrames(); - finish(); + finish(SpectatingUserState.Failed); - checkPaused(false); - // TODO: should replay until running out of frames then fail + checkPaused(false); // Should continue playing until out of frames + checkPaused(true); } [Test] @@ -246,7 +247,7 @@ namespace osu.Game.Tests.Visual.Gameplay private void start(int? beatmapId = null) => AddStep("start play", () => spectatorClient.StartPlay(streamingUser.Id, beatmapId ?? importedBeatmapId)); - private void finish() => AddStep("end play", () => spectatorClient.EndPlay(streamingUser.Id)); + private void finish(SpectatingUserState state = SpectatingUserState.Quit) => AddStep("end play", () => spectatorClient.EndPlay(streamingUser.Id, state)); private void checkPaused(bool state) => AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); From fcbba3d9481ca396acd9885593065ea782362475 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 2 Feb 2022 23:11:29 +0900 Subject: [PATCH 08/42] Rename PlayingUserStates -> WatchingUserStates --- .../Visual/Gameplay/TestSceneSpectatorHost.cs | 4 ++-- .../TestSceneMultiplayerGameplayLeaderboard.cs | 2 +- osu.Game/Online/Spectator/SpectatorClient.cs | 12 ++++++------ .../Overlays/Dashboard/CurrentlyPlayingDisplay.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs index 409cec4cf6..6d6b0bf89e 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs @@ -37,8 +37,8 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestClientSendsCorrectRuleset() { - AddUntilStep("spectator client sending frames", () => spectatorClient.PlayingUserStates.ContainsKey(dummy_user_id)); - AddAssert("spectator client sent correct ruleset", () => spectatorClient.PlayingUserStates[dummy_user_id].RulesetID == Ruleset.Value.OnlineID); + AddUntilStep("spectator client sending frames", () => spectatorClient.WatchingUserStates.ContainsKey(dummy_user_id)); + AddAssert("spectator client sent correct ruleset", () => spectatorClient.WatchingUserStates[dummy_user_id].RulesetID == Ruleset.Value.OnlineID); } public override void TearDownSteps() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs index 8b71dfac21..55450b36e2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs @@ -115,7 +115,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public void RandomlyUpdateState() { - foreach ((int userId, _) in PlayingUserStates) + foreach ((int userId, _) in WatchingUserStates) { if (RNG.NextBool()) continue; diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index 156d544960..3646c51d94 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -37,8 +37,8 @@ namespace osu.Game.Online.Spectator private readonly List watchingUsers = new List(); - public IBindableDictionary PlayingUserStates => playingUserStates; - private readonly BindableDictionary playingUserStates = new BindableDictionary(); + public IBindableDictionary WatchingUserStates => watchingUserStates; + private readonly BindableDictionary watchingUserStates = new BindableDictionary(); private IBeatmap? currentBeatmap; private Score? currentScore; @@ -85,7 +85,7 @@ namespace osu.Game.Online.Spectator BeginPlayingInternal(currentState); } else - playingUserStates.Clear(); + watchingUserStates.Clear(); }), true); } @@ -94,7 +94,7 @@ namespace osu.Game.Online.Spectator Schedule(() => { if (watchingUsers.Contains(userId)) - playingUserStates[userId] = state; + watchingUserStates[userId] = state; OnUserBeganPlaying?.Invoke(userId, state); }); @@ -106,7 +106,7 @@ namespace osu.Game.Online.Spectator Schedule(() => { if (watchingUsers.Contains(userId)) - playingUserStates[userId] = state; + watchingUserStates[userId] = state; OnUserFinishedPlaying?.Invoke(userId, state); }); @@ -193,7 +193,7 @@ namespace osu.Game.Online.Spectator Schedule(() => { watchingUsers.Remove(userId); - playingUserStates.Remove(userId); + watchingUserStates.Remove(userId); StopWatchingUserInternal(userId); }); } diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 383f17d8d2..975402a443 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -51,7 +51,7 @@ namespace osu.Game.Overlays.Dashboard { base.LoadComplete(); - userStates.BindTo(spectatorClient.PlayingUserStates); + userStates.BindTo(spectatorClient.WatchingUserStates); userStates.BindCollectionChanged(onUserStatesChanged, true); } diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 783ba494eb..0b2a7cecf6 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -77,7 +77,7 @@ namespace osu.Game.Screens.Spectate userMap[u.Id] = u; } - userStates.BindTo(spectatorClient.PlayingUserStates); + userStates.BindTo(spectatorClient.WatchingUserStates); userStates.BindCollectionChanged(onUserStatesChanged, true); realmSubscription = realm.RegisterForNotifications( From 81a22dbd29543b3ff5387669bd6ce1fc048a4811 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 2 Feb 2022 23:19:43 +0900 Subject: [PATCH 09/42] Add back playing users list --- osu.Game/Online/Spectator/SpectatorClient.cs | 28 +++++++++-- .../Dashboard/CurrentlyPlayingDisplay.cs | 48 ++++++++----------- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index 3646c51d94..9e168411b0 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -35,16 +35,28 @@ namespace osu.Game.Online.Spectator /// public abstract IBindable IsConnected { get; } + /// + /// The states of all users currently being watched. + /// + public IBindableDictionary WatchingUserStates => watchingUserStates; + + /// + /// A global list of all players currently playing. + /// + public IBindableList PlayingUsers => playingUsers; + + /// + /// All users currently being watched. + /// private readonly List watchingUsers = new List(); - public IBindableDictionary WatchingUserStates => watchingUserStates; private readonly BindableDictionary watchingUserStates = new BindableDictionary(); + private readonly BindableList playingUsers = new BindableList(); + private readonly SpectatorState currentState = new SpectatorState(); private IBeatmap? currentBeatmap; private Score? currentScore; - private readonly SpectatorState currentState = new SpectatorState(); - /// /// Whether the local user is playing. /// @@ -85,7 +97,10 @@ namespace osu.Game.Online.Spectator BeginPlayingInternal(currentState); } else + { watchingUserStates.Clear(); + playingUsers.Clear(); + } }), true); } @@ -93,8 +108,12 @@ namespace osu.Game.Online.Spectator { Schedule(() => { + if (!playingUsers.Contains(userId)) + playingUsers.Add(userId); + if (watchingUsers.Contains(userId)) watchingUserStates[userId] = state; + OnUserBeganPlaying?.Invoke(userId, state); }); @@ -105,8 +124,11 @@ namespace osu.Game.Online.Spectator { Schedule(() => { + playingUsers.Remove(userId); + if (watchingUsers.Contains(userId)) watchingUserStates[userId] = state; + OnUserFinishedPlaying?.Invoke(userId, state); }); diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 975402a443..02ef28f825 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; @@ -22,7 +23,7 @@ namespace osu.Game.Overlays.Dashboard { internal class CurrentlyPlayingDisplay : CompositeDrawable { - private readonly IBindableDictionary userStates = new BindableDictionary(); + private readonly IBindableList playingUsers = new BindableList(); private FillFlowContainer userFlow; @@ -51,55 +52,46 @@ namespace osu.Game.Overlays.Dashboard { base.LoadComplete(); - userStates.BindTo(spectatorClient.WatchingUserStates); - userStates.BindCollectionChanged(onUserStatesChanged, true); + playingUsers.BindTo(spectatorClient.PlayingUsers); + playingUsers.BindCollectionChanged(onPlayingUsersChanged, true); } - private void onUserStatesChanged(object sender, NotifyDictionaryChangedEventArgs e) => Schedule(() => + private void onPlayingUsersChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() => { switch (e.Action) { - case NotifyDictionaryChangedAction.Add: - case NotifyDictionaryChangedAction.Replace: + case NotifyCollectionChangedAction.Add: Debug.Assert(e.NewItems != null); - foreach ((int userId, SpectatorState state) in e.NewItems) + foreach (int userId in e.NewItems) { - if (state.State != SpectatingUserState.Playing) - { - removePlayingUser(userId); - continue; - } - users.GetUserAsync(userId).ContinueWith(task => { var user = task.GetResultSafely(); if (user != null) - Schedule(() => addPlayingUser(user)); + { + Schedule(() => + { + // user may no longer be playing. + if (!playingUsers.Contains(user.Id)) + return; + + userFlow.Add(createUserPanel(user)); + }); + } }); } break; - case NotifyDictionaryChangedAction.Remove: + case NotifyCollectionChangedAction.Remove: Debug.Assert(e.OldItems != null); - foreach ((int userId, _) in e.OldItems) - removePlayingUser(userId); + foreach (int userId in e.OldItems) + userFlow.FirstOrDefault(card => card.User.Id == userId)?.Expire(); break; } - - void addPlayingUser(APIUser user) - { - // user may no longer be playing. - if (!userStates.TryGetValue(user.Id, out var state2) || state2.State != SpectatingUserState.Playing) - return; - - userFlow.Add(createUserPanel(user)); - } - - void removePlayingUser(int userId) => userFlow.FirstOrDefault(card => card.User.Id == userId)?.Expire(); }); private PlayingUserPanel createUserPanel(APIUser user) => From 62537eb4aa5f458fd959846a8bf318b16e2acef0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 3 Feb 2022 12:44:33 +0900 Subject: [PATCH 10/42] Fix spectator not completing --- osu.Game/Screens/Spectate/SpectatorScreen.cs | 21 +++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 0b2a7cecf6..460352ee07 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -112,6 +112,7 @@ namespace osu.Game.Screens.Spectate switch (e.Action) { case NotifyDictionaryChangedAction.Add: + case NotifyDictionaryChangedAction.Replace: foreach ((int userId, var state) in e.NewItems.AsNonNull()) onUserStateChanged(userId, state); break; @@ -120,11 +121,6 @@ namespace osu.Game.Screens.Spectate foreach ((int userId, _) in e.OldItems.AsNonNull()) onUserStateRemoved(userId); break; - - case NotifyDictionaryChangedAction.Replace: - foreach ((int userId, var state) in e.NewItems.AsNonNull()) - onUserStateChanged(userId, state); - break; } } @@ -136,11 +132,18 @@ namespace osu.Game.Screens.Spectate if (!userMap.ContainsKey(userId)) return; - // Do nothing for failed/completed states. - if (newState.State == SpectatingUserState.Playing) + switch (newState.State) { - Schedule(() => OnUserStateChanged(userId, newState)); - updateGameplayState(userId); + case SpectatingUserState.Completed: + // Make sure that gameplay completes to the end. + if (gameplayStates.TryGetValue(userId, out var gameplayState)) + gameplayState.Score.Replay.HasReceivedAllFrames = true; + break; + + case SpectatingUserState.Playing: + Schedule(() => OnUserStateChanged(userId, newState)); + updateGameplayState(userId); + break; } } From f2850601486ec6a8b4b4a88d76b13c75fddd0320 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 3 Feb 2022 21:50:15 +0900 Subject: [PATCH 11/42] Fix MultiSpectatorScreen not continuing to results --- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 5 ++++- osu.Game/Screens/Play/SoloSpectator.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 16 ++++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 4646f42d63..bbdd7a3d56 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -215,8 +215,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate protected override void StartGameplay(int userId, SpectatorGameplayState spectatorGameplayState) => instances.Single(i => i.UserId == userId).LoadScore(spectatorGameplayState.Score); - protected override void EndGameplay(int userId) + protected override void EndGameplay(int userId, SpectatorState state) { + if (state.State == SpectatingUserState.Completed || state.State == SpectatingUserState.Failed) + return; + RemoveUser(userId); var instance = instances.Single(i => i.UserId == userId); diff --git a/osu.Game/Screens/Play/SoloSpectator.cs b/osu.Game/Screens/Play/SoloSpectator.cs index b530965269..a710db6d24 100644 --- a/osu.Game/Screens/Play/SoloSpectator.cs +++ b/osu.Game/Screens/Play/SoloSpectator.cs @@ -180,7 +180,7 @@ namespace osu.Game.Screens.Play scheduleStart(spectatorGameplayState); } - protected override void EndGameplay(int userId) + protected override void EndGameplay(int userId, SpectatorState state) { scheduledStart?.Cancel(); immediateSpectatorGameplayState = null; diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 460352ee07..7b58f669a0 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -118,8 +118,8 @@ namespace osu.Game.Screens.Spectate break; case NotifyDictionaryChangedAction.Remove: - foreach ((int userId, _) in e.OldItems.AsNonNull()) - onUserStateRemoved(userId); + foreach ((int userId, SpectatorState state) in e.OldItems.AsNonNull()) + onUserStateRemoved(userId, state); break; } } @@ -147,7 +147,7 @@ namespace osu.Game.Screens.Spectate } } - private void onUserStateRemoved(int userId) + private void onUserStateRemoved(int userId, SpectatorState state) { if (!userMap.ContainsKey(userId)) return; @@ -158,7 +158,7 @@ namespace osu.Game.Screens.Spectate gameplayState.Score.Replay.HasReceivedAllFrames = true; gameplayStates.Remove(userId); - Schedule(() => EndGameplay(userId)); + Schedule(() => EndGameplay(userId, state)); } private void updateGameplayState(int userId) @@ -212,7 +212,8 @@ namespace osu.Game.Screens.Spectate /// Ends gameplay for a user. /// /// The user to end gameplay for. - protected abstract void EndGameplay(int userId); + /// The final user state. + protected abstract void EndGameplay(int userId, SpectatorState state); /// /// Stops spectating a user. @@ -220,7 +221,10 @@ namespace osu.Game.Screens.Spectate /// The user to stop spectating. protected void RemoveUser(int userId) { - onUserStateRemoved(userId); + if (!userStates.TryGetValue(userId, out var state)) + return; + + onUserStateRemoved(userId, state); users.Remove(userId); userMap.Remove(userId); From 0d99017178af1b373212f4174242e7bf8ee224a3 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Feb 2022 20:27:08 +0900 Subject: [PATCH 12/42] Add state tests --- .../Visual/Gameplay/TestSceneSpectator.cs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 9f8470446c..65583919d6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -235,6 +235,71 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("last frame has header", () => lastBundle.Frames[^1].Header != null); } + [Test] + public void TestPlayingState() + { + loadSpectatingScreen(); + + start(); + sendFrames(); + waitForPlayer(); + AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + } + + [Test] + public void TestCompletionState() + { + loadSpectatingScreen(); + + start(); + sendFrames(); + waitForPlayer(); + + AddStep("send completion", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Completed)); + AddUntilStep("state is completed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Completed); + + start(); + sendFrames(); + waitForPlayer(); + AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + } + + [Test] + public void TestQuitState() + { + loadSpectatingScreen(); + + start(); + sendFrames(); + waitForPlayer(); + + AddStep("send quit", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Quit)); + AddUntilStep("state is quit", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Quit); + + start(); + sendFrames(); + waitForPlayer(); + AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + } + + [Test] + public void TestFailedState() + { + loadSpectatingScreen(); + + start(); + sendFrames(); + waitForPlayer(); + + AddStep("send failed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Failed)); + AddUntilStep("state is failed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Failed); + + start(); + sendFrames(); + waitForPlayer(); + AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + } + private OsuFramedReplayInputHandler replayHandler => (OsuFramedReplayInputHandler)Stack.ChildrenOfType().First().ReplayInputHandler; From 4c76027178933d9bcade478495f0588746220a18 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Feb 2022 20:29:49 +0900 Subject: [PATCH 13/42] Rename completed state to passed --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 6 +++--- osu.Game/Online/Spectator/SpectatingUserState.cs | 10 +++++----- osu.Game/Online/Spectator/SpectatorClient.cs | 2 +- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 65583919d6..a263bb1e6f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -247,7 +247,7 @@ namespace osu.Game.Tests.Visual.Gameplay } [Test] - public void TestCompletionState() + public void TestPassedState() { loadSpectatingScreen(); @@ -255,8 +255,8 @@ namespace osu.Game.Tests.Visual.Gameplay sendFrames(); waitForPlayer(); - AddStep("send completion", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Completed)); - AddUntilStep("state is completed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Completed); + AddStep("send passed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Passed)); + AddUntilStep("state is passed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Passed); start(); sendFrames(); diff --git a/osu.Game/Online/Spectator/SpectatingUserState.cs b/osu.Game/Online/Spectator/SpectatingUserState.cs index c7ba4ba248..30be57b739 100644 --- a/osu.Game/Online/Spectator/SpectatingUserState.cs +++ b/osu.Game/Online/Spectator/SpectatingUserState.cs @@ -6,7 +6,7 @@ namespace osu.Game.Online.Spectator public enum SpectatingUserState { /// - /// The spectated user has not yet played. + /// The spectated user is not yet playing. /// Idle, @@ -16,17 +16,17 @@ namespace osu.Game.Online.Spectator Playing, /// - /// The spectated user has successfully completed gameplay. + /// The spectated user has passed gameplay. /// - Completed, + Passed, /// - /// The spectator user has failed during gameplay. + /// The spectator user has failed gameplay. /// Failed, /// - /// The spectated user has quit during gameplay. + /// The spectated user has quit gameplay. /// Quit } diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index 9e168411b0..ee5fdc917a 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -186,7 +186,7 @@ namespace osu.Game.Online.Spectator currentBeatmap = null; if (state.HasPassed) - currentState.State = SpectatingUserState.Completed; + currentState.State = SpectatingUserState.Passed; else if (state.HasFailed) currentState.State = SpectatingUserState.Failed; else diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 125e4b261c..60bf87ff61 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -216,7 +216,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate protected override void EndGameplay(int userId, SpectatorState state) { - if (state.State == SpectatingUserState.Completed || state.State == SpectatingUserState.Failed) + if (state.State == SpectatingUserState.Passed || state.State == SpectatingUserState.Failed) return; RemoveUser(userId); diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 7b58f669a0..394afb3a4c 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -134,7 +134,7 @@ namespace osu.Game.Screens.Spectate switch (newState.State) { - case SpectatingUserState.Completed: + case SpectatingUserState.Passed: // Make sure that gameplay completes to the end. if (gameplayStates.TryGetValue(userId, out var gameplayState)) gameplayState.Score.Replay.HasReceivedAllFrames = true; From c1766d8a411f1626255c1d74fbf0b634e7bc4610 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Feb 2022 20:29:53 +0900 Subject: [PATCH 14/42] Add paused state --- osu.Game/Online/Spectator/SpectatingUserState.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Online/Spectator/SpectatingUserState.cs b/osu.Game/Online/Spectator/SpectatingUserState.cs index 30be57b739..e00c2e1947 100644 --- a/osu.Game/Online/Spectator/SpectatingUserState.cs +++ b/osu.Game/Online/Spectator/SpectatingUserState.cs @@ -15,6 +15,11 @@ namespace osu.Game.Online.Spectator /// Playing, + /// + /// The spectated user is currently paused. Unused for the time being. + /// + Paused, + /// /// The spectated user has passed gameplay. /// From 886d1d2df637dcbac1e3b7147b3062ba576ce178 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Feb 2022 21:20:33 +0900 Subject: [PATCH 15/42] Refactorings --- .../Visual/Gameplay/TestSceneSpectator.cs | 3 ++- osu.Game/Online/Spectator/SpectatorClient.cs | 2 +- .../Dashboard/CurrentlyPlayingDisplay.cs | 20 +++++++++---------- .../Spectate/MultiSpectatorScreen.cs | 2 ++ osu.Game/Screens/Play/GameplayState.cs | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index a263bb1e6f..e0946df2dc 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -160,7 +160,8 @@ namespace osu.Game.Tests.Visual.Gameplay finish(SpectatingUserState.Failed); checkPaused(false); // Should continue playing until out of frames - checkPaused(true); + checkPaused(true); // And eventually stop after running out of frames and fail. + // Todo: Should check for + display a failed message. } [Test] diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index ee5fdc917a..ed6e355ddf 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -98,8 +98,8 @@ namespace osu.Game.Online.Spectator } else { - watchingUserStates.Clear(); playingUsers.Clear(); + watchingUserStates.Clear(); } }), true); } diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 02ef28f825..117de88166 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -69,17 +69,17 @@ namespace osu.Game.Overlays.Dashboard { var user = task.GetResultSafely(); - if (user != null) - { - Schedule(() => - { - // user may no longer be playing. - if (!playingUsers.Contains(user.Id)) - return; + if (user == null) + return; - userFlow.Add(createUserPanel(user)); - }); - } + Schedule(() => + { + // user may no longer be playing. + if (!playingUsers.Contains(user.Id)) + return; + + userFlow.Add(createUserPanel(user)); + }); }); } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 60bf87ff61..55df8301da 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -216,6 +216,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate protected override void EndGameplay(int userId, SpectatorState state) { + // Allowed passed/failed users to complete their remaining replay frames. + // The failed state isn't really possible in multiplayer (yet?) but is added here just for safety in case it starts being used. if (state.State == SpectatingUserState.Passed || state.State == SpectatingUserState.Failed) return; diff --git a/osu.Game/Screens/Play/GameplayState.cs b/osu.Game/Screens/Play/GameplayState.cs index 64e873b3bb..c6a072da74 100644 --- a/osu.Game/Screens/Play/GameplayState.cs +++ b/osu.Game/Screens/Play/GameplayState.cs @@ -50,7 +50,7 @@ namespace osu.Game.Screens.Play public bool HasFailed { get; set; } /// - /// Whether the user quit gameplay without either having either passed or failed. + /// Whether the user quit gameplay without having either passed or failed. /// public bool HasQuit { get; set; } From 4966c4e974c9a3e7b98910adeee5b658d6889e22 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 9 Feb 2022 11:51:47 +0900 Subject: [PATCH 16/42] Remove redundant parameter --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index e0946df2dc..7235bce789 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -274,7 +274,7 @@ namespace osu.Game.Tests.Visual.Gameplay sendFrames(); waitForPlayer(); - AddStep("send quit", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Quit)); + AddStep("send quit", () => spectatorClient.EndPlay(streamingUser.Id)); AddUntilStep("state is quit", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Quit); start(); From ffc4c64f7e7fb0951c9c3f3109497b37f50196b3 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 9 Feb 2022 12:09:04 +0900 Subject: [PATCH 17/42] Unify namings across the board --- .../Visual/Gameplay/TestSceneSpectator.cs | 22 ++++++------ .../Visual/Gameplay/TestSceneSpectatorHost.cs | 4 +-- ...TestSceneMultiplayerGameplayLeaderboard.cs | 2 +- ...tingUserState.cs => SpectatedUserState.cs} | 4 +-- osu.Game/Online/Spectator/SpectatorClient.cs | 36 +++++++++---------- osu.Game/Online/Spectator/SpectatorState.cs | 2 +- .../Spectate/MultiSpectatorScreen.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 6 ++-- .../Visual/Spectator/TestSpectatorClient.cs | 4 +-- 9 files changed, 41 insertions(+), 41 deletions(-) rename osu.Game/Online/Spectator/{SpectatingUserState.cs => SpectatedUserState.cs} (90%) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 7235bce789..157c248d69 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -157,7 +157,7 @@ namespace osu.Game.Tests.Visual.Gameplay checkPaused(true); sendFrames(); - finish(SpectatingUserState.Failed); + finish(SpectatedUserState.Failed); checkPaused(false); // Should continue playing until out of frames checkPaused(true); // And eventually stop after running out of frames and fail. @@ -244,7 +244,7 @@ namespace osu.Game.Tests.Visual.Gameplay start(); sendFrames(); waitForPlayer(); - AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing); } [Test] @@ -256,13 +256,13 @@ namespace osu.Game.Tests.Visual.Gameplay sendFrames(); waitForPlayer(); - AddStep("send passed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Passed)); - AddUntilStep("state is passed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Passed); + AddStep("send passed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatedUserState.Passed)); + AddUntilStep("state is passed", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Passed); start(); sendFrames(); waitForPlayer(); - AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing); } [Test] @@ -275,12 +275,12 @@ namespace osu.Game.Tests.Visual.Gameplay waitForPlayer(); AddStep("send quit", () => spectatorClient.EndPlay(streamingUser.Id)); - AddUntilStep("state is quit", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Quit); + AddUntilStep("state is quit", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Quit); start(); sendFrames(); waitForPlayer(); - AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing); } [Test] @@ -292,13 +292,13 @@ namespace osu.Game.Tests.Visual.Gameplay sendFrames(); waitForPlayer(); - AddStep("send failed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatingUserState.Failed)); - AddUntilStep("state is failed", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Failed); + AddStep("send failed", () => spectatorClient.EndPlay(streamingUser.Id, SpectatedUserState.Failed)); + AddUntilStep("state is failed", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Failed); start(); sendFrames(); waitForPlayer(); - AddUntilStep("state is playing", () => spectatorClient.WatchingUserStates[streamingUser.Id].State == SpectatingUserState.Playing); + AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing); } private OsuFramedReplayInputHandler replayHandler => @@ -313,7 +313,7 @@ namespace osu.Game.Tests.Visual.Gameplay private void start(int? beatmapId = null) => AddStep("start play", () => spectatorClient.StartPlay(streamingUser.Id, beatmapId ?? importedBeatmapId)); - private void finish(SpectatingUserState state = SpectatingUserState.Quit) => AddStep("end play", () => spectatorClient.EndPlay(streamingUser.Id, state)); + private void finish(SpectatedUserState state = SpectatedUserState.Quit) => AddStep("end play", () => spectatorClient.EndPlay(streamingUser.Id, state)); private void checkPaused(bool state) => AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs index 6d6b0bf89e..034519fbf8 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorHost.cs @@ -37,8 +37,8 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestClientSendsCorrectRuleset() { - AddUntilStep("spectator client sending frames", () => spectatorClient.WatchingUserStates.ContainsKey(dummy_user_id)); - AddAssert("spectator client sent correct ruleset", () => spectatorClient.WatchingUserStates[dummy_user_id].RulesetID == Ruleset.Value.OnlineID); + AddUntilStep("spectator client sending frames", () => spectatorClient.WatchedUserStates.ContainsKey(dummy_user_id)); + AddAssert("spectator client sent correct ruleset", () => spectatorClient.WatchedUserStates[dummy_user_id].RulesetID == Ruleset.Value.OnlineID); } public override void TearDownSteps() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs index 55450b36e2..1322fbc96e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs @@ -115,7 +115,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public void RandomlyUpdateState() { - foreach ((int userId, _) in WatchingUserStates) + foreach ((int userId, _) in WatchedUserStates) { if (RNG.NextBool()) continue; diff --git a/osu.Game/Online/Spectator/SpectatingUserState.cs b/osu.Game/Online/Spectator/SpectatedUserState.cs similarity index 90% rename from osu.Game/Online/Spectator/SpectatingUserState.cs rename to osu.Game/Online/Spectator/SpectatedUserState.cs index e00c2e1947..0f0a3068b8 100644 --- a/osu.Game/Online/Spectator/SpectatingUserState.cs +++ b/osu.Game/Online/Spectator/SpectatedUserState.cs @@ -3,7 +3,7 @@ namespace osu.Game.Online.Spectator { - public enum SpectatingUserState + public enum SpectatedUserState { /// /// The spectated user is not yet playing. @@ -26,7 +26,7 @@ namespace osu.Game.Online.Spectator Passed, /// - /// The spectator user has failed gameplay. + /// The spectated user has failed gameplay. /// Failed, diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index ed6e355ddf..a54ea0d9ee 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -38,7 +38,7 @@ namespace osu.Game.Online.Spectator /// /// The states of all users currently being watched. /// - public IBindableDictionary WatchingUserStates => watchingUserStates; + public IBindableDictionary WatchedUserStates => watchedUserStates; /// /// A global list of all players currently playing. @@ -48,9 +48,9 @@ namespace osu.Game.Online.Spectator /// /// All users currently being watched. /// - private readonly List watchingUsers = new List(); + private readonly List watchedUsers = new List(); - private readonly BindableDictionary watchingUserStates = new BindableDictionary(); + private readonly BindableDictionary watchedUserStates = new BindableDictionary(); private readonly BindableList playingUsers = new BindableList(); private readonly SpectatorState currentState = new SpectatorState(); @@ -85,8 +85,8 @@ namespace osu.Game.Online.Spectator if (connected.NewValue) { // get all the users that were previously being watched - int[] users = watchingUsers.ToArray(); - watchingUsers.Clear(); + int[] users = watchedUsers.ToArray(); + watchedUsers.Clear(); // resubscribe to watched users. foreach (int userId in users) @@ -99,7 +99,7 @@ namespace osu.Game.Online.Spectator else { playingUsers.Clear(); - watchingUserStates.Clear(); + watchedUserStates.Clear(); } }), true); } @@ -111,8 +111,8 @@ namespace osu.Game.Online.Spectator if (!playingUsers.Contains(userId)) playingUsers.Add(userId); - if (watchingUsers.Contains(userId)) - watchingUserStates[userId] = state; + if (watchedUsers.Contains(userId)) + watchedUserStates[userId] = state; OnUserBeganPlaying?.Invoke(userId, state); }); @@ -126,8 +126,8 @@ namespace osu.Game.Online.Spectator { playingUsers.Remove(userId); - if (watchingUsers.Contains(userId)) - watchingUserStates[userId] = state; + if (watchedUsers.Contains(userId)) + watchedUserStates[userId] = state; OnUserFinishedPlaying?.Invoke(userId, state); }); @@ -159,7 +159,7 @@ namespace osu.Game.Online.Spectator currentState.BeatmapID = score.ScoreInfo.BeatmapInfo.OnlineID; currentState.RulesetID = score.ScoreInfo.RulesetID; currentState.Mods = score.ScoreInfo.Mods.Select(m => new APIMod(m)).ToArray(); - currentState.State = SpectatingUserState.Playing; + currentState.State = SpectatedUserState.Playing; currentBeatmap = state.Beatmap; currentScore = score; @@ -186,11 +186,11 @@ namespace osu.Game.Online.Spectator currentBeatmap = null; if (state.HasPassed) - currentState.State = SpectatingUserState.Passed; + currentState.State = SpectatedUserState.Passed; else if (state.HasFailed) - currentState.State = SpectatingUserState.Failed; + currentState.State = SpectatedUserState.Failed; else - currentState.State = SpectatingUserState.Quit; + currentState.State = SpectatedUserState.Quit; EndPlayingInternal(currentState); }); @@ -200,10 +200,10 @@ namespace osu.Game.Online.Spectator { Debug.Assert(ThreadSafety.IsUpdateThread); - if (watchingUsers.Contains(userId)) + if (watchedUsers.Contains(userId)) return; - watchingUsers.Add(userId); + watchedUsers.Add(userId); WatchUserInternal(userId); } @@ -214,8 +214,8 @@ namespace osu.Game.Online.Spectator // Todo: This should not be a thing, but requires framework changes. Schedule(() => { - watchingUsers.Remove(userId); - watchingUserStates.Remove(userId); + watchedUsers.Remove(userId); + watchedUserStates.Remove(userId); StopWatchingUserInternal(userId); }); } diff --git a/osu.Game/Online/Spectator/SpectatorState.cs b/osu.Game/Online/Spectator/SpectatorState.cs index fc62f16bba..77686d12da 100644 --- a/osu.Game/Online/Spectator/SpectatorState.cs +++ b/osu.Game/Online/Spectator/SpectatorState.cs @@ -25,7 +25,7 @@ namespace osu.Game.Online.Spectator public IEnumerable Mods { get; set; } = Enumerable.Empty(); [Key(3)] - public SpectatingUserState State { get; set; } + public SpectatedUserState State { get; set; } public bool Equals(SpectatorState other) { diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 55df8301da..571b6b4324 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -218,7 +218,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate { // Allowed passed/failed users to complete their remaining replay frames. // The failed state isn't really possible in multiplayer (yet?) but is added here just for safety in case it starts being used. - if (state.State == SpectatingUserState.Passed || state.State == SpectatingUserState.Failed) + if (state.State == SpectatedUserState.Passed || state.State == SpectatedUserState.Failed) return; RemoveUser(userId); diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 394afb3a4c..9d0f8abddc 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -77,7 +77,7 @@ namespace osu.Game.Screens.Spectate userMap[u.Id] = u; } - userStates.BindTo(spectatorClient.WatchingUserStates); + userStates.BindTo(spectatorClient.WatchedUserStates); userStates.BindCollectionChanged(onUserStatesChanged, true); realmSubscription = realm.RegisterForNotifications( @@ -134,13 +134,13 @@ namespace osu.Game.Screens.Spectate switch (newState.State) { - case SpectatingUserState.Passed: + case SpectatedUserState.Passed: // Make sure that gameplay completes to the end. if (gameplayStates.TryGetValue(userId, out var gameplayState)) gameplayState.Score.Replay.HasReceivedAllFrames = true; break; - case SpectatingUserState.Playing: + case SpectatedUserState.Playing: Schedule(() => OnUserStateChanged(userId, newState)); updateGameplayState(userId); break; diff --git a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs index 6862cda88c..1322a99ea7 100644 --- a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs +++ b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs @@ -59,7 +59,7 @@ namespace osu.Game.Tests.Visual.Spectator /// /// The user to end play for. /// The spectator state to end play with. - public void EndPlay(int userId, SpectatingUserState state = SpectatingUserState.Quit) + public void EndPlay(int userId, SpectatedUserState state = SpectatedUserState.Quit) { if (!userBeatmapDictionary.ContainsKey(userId)) return; @@ -144,7 +144,7 @@ namespace osu.Game.Tests.Visual.Spectator { BeatmapID = userBeatmapDictionary[userId], RulesetID = 0, - State = SpectatingUserState.Playing + State = SpectatedUserState.Playing }); } } From 18251c9285a559ac3f4070cdc8538211175e18ab Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 9 Feb 2022 12:20:07 +0900 Subject: [PATCH 18/42] Clean up SpectatorScreen based on suggestions --- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 2 +- osu.Game/Screens/Play/SoloSpectator.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 571b6b4324..e5eeeb3448 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -207,7 +207,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } } - protected override void OnUserStateChanged(int userId, SpectatorState spectatorState) + protected override void OnNewPlayingUserState(int userId, SpectatorState spectatorState) { } diff --git a/osu.Game/Screens/Play/SoloSpectator.cs b/osu.Game/Screens/Play/SoloSpectator.cs index a710db6d24..a0b07fcbd9 100644 --- a/osu.Game/Screens/Play/SoloSpectator.cs +++ b/osu.Game/Screens/Play/SoloSpectator.cs @@ -166,7 +166,7 @@ namespace osu.Game.Screens.Play automaticDownload.Current.BindValueChanged(_ => checkForAutomaticDownload()); } - protected override void OnUserStateChanged(int userId, SpectatorState spectatorState) + protected override void OnNewPlayingUserState(int userId, SpectatorState spectatorState) { clearDisplay(); showBeatmapPanel(spectatorState); diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 9d0f8abddc..9eb374f0f7 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -103,7 +103,7 @@ namespace osu.Game.Screens.Spectate continue; if (beatmapSet.Beatmaps.Any(b => b.OnlineID == userState.BeatmapID)) - updateGameplayState(userId); + startGameplay(userId); } } @@ -141,8 +141,8 @@ namespace osu.Game.Screens.Spectate break; case SpectatedUserState.Playing: - Schedule(() => OnUserStateChanged(userId, newState)); - updateGameplayState(userId); + Schedule(() => OnNewPlayingUserState(userId, newState)); + startGameplay(userId); break; } } @@ -161,7 +161,7 @@ namespace osu.Game.Screens.Spectate Schedule(() => EndGameplay(userId, state)); } - private void updateGameplayState(int userId) + private void startGameplay(int userId) { Debug.Assert(userMap.ContainsKey(userId)); @@ -195,11 +195,11 @@ namespace osu.Game.Screens.Spectate } /// - /// Invoked when a spectated user's state has changed. + /// Invoked when a spectated user's state has changed to a new state indicating the player is currently playing. /// /// The user whose state has changed. /// The new state. - protected abstract void OnUserStateChanged(int userId, [NotNull] SpectatorState spectatorState); + protected abstract void OnNewPlayingUserState(int userId, [NotNull] SpectatorState spectatorState); /// /// Starts gameplay for a user. From a3896a8ebdc87b95c9d97a1f5112c65bafe761b9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 10 Feb 2022 14:18:29 +0900 Subject: [PATCH 19/42] Remove allowance of null dependency --- osu.Game/Rulesets/UI/ReplayRecorder.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/UI/ReplayRecorder.cs b/osu.Game/Rulesets/UI/ReplayRecorder.cs index 277040b2a6..0a94b55221 100644 --- a/osu.Game/Rulesets/UI/ReplayRecorder.cs +++ b/osu.Game/Rulesets/UI/ReplayRecorder.cs @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.UI public int RecordFrameRate = 60; - [Resolved(canBeNull: true)] + [Resolved] private SpectatorClient spectatorClient { get; set; } [Resolved] @@ -48,8 +48,7 @@ namespace osu.Game.Rulesets.UI base.LoadComplete(); inputManager = GetContainingInputManager(); - - spectatorClient?.BeginPlaying(gameplayState, target); + spectatorClient.BeginPlaying(gameplayState, target); } protected override void Dispose(bool isDisposing) From f7fb7825cc322c3ac08a0c310928591ed96e3142 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 10 Feb 2022 14:21:33 +0900 Subject: [PATCH 20/42] Simplify disposal --- osu.Game/Rulesets/UI/ReplayRecorder.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/UI/ReplayRecorder.cs b/osu.Game/Rulesets/UI/ReplayRecorder.cs index 0a94b55221..6843beef7b 100644 --- a/osu.Game/Rulesets/UI/ReplayRecorder.cs +++ b/osu.Game/Rulesets/UI/ReplayRecorder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Graphics; @@ -54,9 +55,7 @@ namespace osu.Game.Rulesets.UI protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - - if (spectatorClient != null && gameplayState != null) - spectatorClient.EndPlaying(gameplayState); + spectatorClient?.EndPlaying(gameplayState); } protected override void Update() From ebd105422fd6c2903bced01cb820d7e0d542789a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 10 Feb 2022 14:22:08 +0900 Subject: [PATCH 21/42] Remove unused using --- osu.Game/Rulesets/UI/ReplayRecorder.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/ReplayRecorder.cs b/osu.Game/Rulesets/UI/ReplayRecorder.cs index 6843beef7b..dcd8f12028 100644 --- a/osu.Game/Rulesets/UI/ReplayRecorder.cs +++ b/osu.Game/Rulesets/UI/ReplayRecorder.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Graphics; From 88bb9d4237c62d37678d8e0e110438ea247b7ae5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 10 Feb 2022 15:50:03 +0900 Subject: [PATCH 22/42] Fix migration errors not outputting the call stack to logs --- .../Settings/Sections/Maintenance/MigrationRunScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs index b0b61554eb..adb347e7b8 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs @@ -88,7 +88,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance .ContinueWith(t => { if (t.IsFaulted) - Logger.Log($"Error during migration: {t.Exception?.Message}", level: LogLevel.Error); + Logger.Error(t.Exception, $"Error during migration: {t.Exception?.Message}"); Schedule(this.Exit); }); From 44f2d8a4481d686aea94d6d480cfc0e17ec7351b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 10 Feb 2022 18:48:37 +0900 Subject: [PATCH 23/42] Allow game folder migration to fail gracefully when cleanup cannot completely succeed --- osu.Game.Tournament/IO/TournamentStorage.cs | 4 ++- osu.Game/IO/MigratableStorage.cs | 30 +++++++++++----- osu.Game/IO/OsuStorage.cs | 7 ++-- osu.Game/OsuGameBase.cs | 9 ++--- .../Maintenance/MigrationRunScreen.cs | 36 ++++++++++++++++--- 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/osu.Game.Tournament/IO/TournamentStorage.cs b/osu.Game.Tournament/IO/TournamentStorage.cs index 347d368a04..b4859d0c91 100644 --- a/osu.Game.Tournament/IO/TournamentStorage.cs +++ b/osu.Game.Tournament/IO/TournamentStorage.cs @@ -70,7 +70,7 @@ namespace osu.Game.Tournament.IO public IEnumerable ListTournaments() => AllTournaments.GetDirectories(string.Empty); - public override void Migrate(Storage newStorage) + public override bool Migrate(Storage newStorage) { // this migration only happens once on moving to the per-tournament storage system. // listed files are those known at that point in time. @@ -94,6 +94,8 @@ namespace osu.Game.Tournament.IO ChangeTargetStorage(newStorage); storageConfig.SetValue(StorageConfig.CurrentTournament, default_tournament); storageConfig.Save(); + + return true; } private void moveFileIfExists(string file, DirectoryInfo destination) diff --git a/osu.Game/IO/MigratableStorage.cs b/osu.Game/IO/MigratableStorage.cs index 1b76725b04..e478144294 100644 --- a/osu.Game/IO/MigratableStorage.cs +++ b/osu.Game/IO/MigratableStorage.cs @@ -33,7 +33,8 @@ namespace osu.Game.IO /// A general purpose migration method to move the storage to a different location. /// The target storage of the migration. /// - public virtual void Migrate(Storage newStorage) + /// Whether cleanup could complete. + public virtual bool Migrate(Storage newStorage) { var source = new DirectoryInfo(GetFullPath(".")); var destination = new DirectoryInfo(newStorage.GetFullPath(".")); @@ -57,17 +58,20 @@ namespace osu.Game.IO CopyRecursive(source, destination); ChangeTargetStorage(newStorage); - DeleteRecursive(source); + + return DeleteRecursive(source); } - protected void DeleteRecursive(DirectoryInfo target, bool topLevelExcludes = true) + protected bool DeleteRecursive(DirectoryInfo target, bool topLevelExcludes = true) { + bool allFilesDeleted = true; + foreach (System.IO.FileInfo fi in target.GetFiles()) { if (topLevelExcludes && IgnoreFiles.Contains(fi.Name)) continue; - AttemptOperation(() => fi.Delete()); + allFilesDeleted &= AttemptOperation(() => fi.Delete(), throwOnFailure: false); } foreach (DirectoryInfo dir in target.GetDirectories()) @@ -75,11 +79,13 @@ namespace osu.Game.IO if (topLevelExcludes && IgnoreDirectories.Contains(dir.Name)) continue; - AttemptOperation(() => dir.Delete(true)); + allFilesDeleted &= AttemptOperation(() => dir.Delete(true), throwOnFailure: false); } if (target.GetFiles().Length == 0 && target.GetDirectories().Length == 0) - AttemptOperation(target.Delete); + allFilesDeleted &= AttemptOperation(target.Delete, throwOnFailure: false); + + return allFilesDeleted; } protected void CopyRecursive(DirectoryInfo source, DirectoryInfo destination, bool topLevelExcludes = true) @@ -110,19 +116,25 @@ namespace osu.Game.IO /// /// The action to perform. /// The number of attempts (250ms wait between each). - protected static void AttemptOperation(Action action, int attempts = 10) + /// Whether to throw an exception on failure. If false, will silently fail. + protected static bool AttemptOperation(Action action, int attempts = 10, bool throwOnFailure = true) { while (true) { try { action(); - return; + return true; } catch (Exception) { if (attempts-- == 0) - throw; + { + if (throwOnFailure) + throw; + + return false; + } } Thread.Sleep(250); diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 802c71e363..6e7cb545e3 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -113,11 +113,14 @@ namespace osu.Game.IO } } - public override void Migrate(Storage newStorage) + public override bool Migrate(Storage newStorage) { - base.Migrate(newStorage); + bool cleanupSucceeded = base.Migrate(newStorage); + storageConfig.SetValue(StorageConfig.FullPath, newStorage.GetFullPath(".")); storageConfig.Save(); + + return cleanupSucceeded; } } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 5b2eb5607a..0b2644d5ba 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -413,7 +413,7 @@ namespace osu.Game Scheduler.AddDelayed(GracefullyExit, 2000); } - public void Migrate(string path) + public bool Migrate(string path) { Logger.Log($@"Migrating osu! data from ""{Storage.GetFullPath(string.Empty)}"" to ""{path}""..."); @@ -432,14 +432,15 @@ namespace osu.Game readyToRun.Wait(); - (Storage as OsuStorage)?.Migrate(Host.GetStorage(path)); + bool? cleanupSucceded = (Storage as OsuStorage)?.Migrate(Host.GetStorage(path)); + + Logger.Log(@"Migration complete!"); + return cleanupSucceded != false; } finally { realmBlocker?.Dispose(); } - - Logger.Log(@"Migration complete!"); } protected override UserInputManager CreateUserInputManager() => new OsuUserInputManager(); diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs index adb347e7b8..fb7ff0dbd1 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/MigrationRunScreen.cs @@ -4,13 +4,16 @@ using System.IO; using System.Threading.Tasks; using osu.Framework.Allocation; +using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; +using osu.Framework.Platform; using osu.Framework.Screens; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; +using osu.Game.Overlays.Notifications; using osu.Game.Screens; using osuTK; @@ -23,6 +26,15 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance [Resolved(canBeNull: true)] private OsuGame game { get; set; } + [Resolved] + private NotificationOverlay notifications { get; set; } + + [Resolved] + private Storage storage { get; set; } + + [Resolved] + private GameHost host { get; set; } + public override bool AllowBackButton => false; public override bool AllowExternalScreenChange => false; @@ -84,17 +96,33 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance Beatmap.Value = Beatmap.Default; + var originalStorage = new NativeStorage(storage.GetFullPath(string.Empty), host); + migrationTask = Task.Run(PerformMigration) - .ContinueWith(t => + .ContinueWith(task => { - if (t.IsFaulted) - Logger.Error(t.Exception, $"Error during migration: {t.Exception?.Message}"); + if (task.IsFaulted) + { + Logger.Error(task.Exception, $"Error during migration: {task.Exception?.Message}"); + } + else if (!task.GetResultSafely()) + { + notifications.Post(new SimpleNotification + { + Text = "Some files couldn't be cleaned up during migration. Clicking this notification will open the folder so you can manually clean things up.", + Activated = () => + { + originalStorage.PresentExternally(); + return true; + } + }); + } Schedule(this.Exit); }); } - protected virtual void PerformMigration() => game?.Migrate(destination.FullName); + protected virtual bool PerformMigration() => game?.Migrate(destination.FullName) != false; public override void OnEntering(IScreen last) { From 19cb8cb03a100c9baa80c23b7b307ff300130df2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 10 Feb 2022 19:21:17 +0900 Subject: [PATCH 24/42] Update tests --- .../Settings/TestSceneMigrationScreens.cs | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneMigrationScreens.cs b/osu.Game.Tests/Visual/Settings/TestSceneMigrationScreens.cs index 2883e54385..a68090504d 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneMigrationScreens.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneMigrationScreens.cs @@ -3,32 +3,69 @@ using System.IO; using System.Threading; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Framework.Screens; +using osu.Game.Overlays; using osu.Game.Overlays.Settings.Sections.Maintenance; namespace osu.Game.Tests.Visual.Settings { public class TestSceneMigrationScreens : ScreenTestScene { + [Cached] + private readonly NotificationOverlay notifications; + public TestSceneMigrationScreens() { - AddStep("Push screen", () => Stack.Push(new TestMigrationSelectScreen())); + Children = new Drawable[] + { + notifications = new NotificationOverlay + { + Anchor = Anchor.TopRight, + Origin = Anchor.TopRight, + } + }; + } + + [Test] + public void TestDeleteSuccess() + { + AddStep("Push screen", () => Stack.Push(new TestMigrationSelectScreen(true))); + } + + [Test] + public void TestDeleteFails() + { + AddStep("Push screen", () => Stack.Push(new TestMigrationSelectScreen(false))); } private class TestMigrationSelectScreen : MigrationSelectScreen { - protected override void BeginMigration(DirectoryInfo target) => this.Push(new TestMigrationRunScreen()); + private readonly bool deleteSuccess; + + public TestMigrationSelectScreen(bool deleteSuccess) + { + this.deleteSuccess = deleteSuccess; + } + + protected override void BeginMigration(DirectoryInfo target) => this.Push(new TestMigrationRunScreen(deleteSuccess)); private class TestMigrationRunScreen : MigrationRunScreen { - protected override void PerformMigration() - { - Thread.Sleep(3000); - } + private readonly bool success; - public TestMigrationRunScreen() + public TestMigrationRunScreen(bool success) : base(null) { + this.success = success; + } + + protected override bool PerformMigration() + { + Thread.Sleep(3000); + return success; } } } From 2939bc4644b2b5079a5af5ffa578288676cf0ca2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 11 Feb 2022 01:49:52 +0900 Subject: [PATCH 25/42] Update resources --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 71525a7acb..147f576c55 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -51,7 +51,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index c8f634284b..dd10807ec2 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -37,7 +37,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 5978f6d685..6fbc468586 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -61,7 +61,7 @@ - + From 9cd88ec2b84d8460af461bd91deec5fc355ddced Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 10 Feb 2022 21:23:38 +0300 Subject: [PATCH 26/42] Update API models with score pinning changes --- osu.Game/Online/API/Requests/GetUserScoresRequest.cs | 3 ++- osu.Game/Online/API/Requests/Responses/APIUser.cs | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Online/API/Requests/GetUserScoresRequest.cs b/osu.Game/Online/API/Requests/GetUserScoresRequest.cs index 653abf7427..5d39799f6b 100644 --- a/osu.Game/Online/API/Requests/GetUserScoresRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserScoresRequest.cs @@ -39,6 +39,7 @@ namespace osu.Game.Online.API.Requests { Best, Firsts, - Recent + Recent, + Pinned } } diff --git a/osu.Game/Online/API/Requests/Responses/APIUser.cs b/osu.Game/Online/API/Requests/Responses/APIUser.cs index e4a432b074..2b64e5de06 100644 --- a/osu.Game/Online/API/Requests/Responses/APIUser.cs +++ b/osu.Game/Online/API/Requests/Responses/APIUser.cs @@ -151,6 +151,9 @@ namespace osu.Game.Online.API.Requests.Responses [JsonProperty(@"scores_recent_count")] public int ScoresRecentCount; + [JsonProperty(@"scores_pinned_count")] + public int ScoresPinnedCount; + [JsonProperty(@"beatmap_playcounts_count")] public int BeatmapPlayCountsCount; From 4f7003928acb9a14a12adc02c343cb9329181ec5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 10 Feb 2022 21:32:07 +0300 Subject: [PATCH 27/42] Add score container for pinned scores in ranks section --- .../Overlays/Profile/Sections/Ranks/PaginatedScoreContainer.cs | 3 +++ osu.Game/Overlays/Profile/Sections/RanksSection.cs | 2 ++ 2 files changed, 5 insertions(+) diff --git a/osu.Game/Overlays/Profile/Sections/Ranks/PaginatedScoreContainer.cs b/osu.Game/Overlays/Profile/Sections/Ranks/PaginatedScoreContainer.cs index 5532e35cc5..5c67da1911 100644 --- a/osu.Game/Overlays/Profile/Sections/Ranks/PaginatedScoreContainer.cs +++ b/osu.Game/Overlays/Profile/Sections/Ranks/PaginatedScoreContainer.cs @@ -46,6 +46,9 @@ namespace osu.Game.Overlays.Profile.Sections.Ranks case ScoreType.Recent: return user.ScoresRecentCount; + case ScoreType.Pinned: + return user.ScoresPinnedCount; + default: return 0; } diff --git a/osu.Game/Overlays/Profile/Sections/RanksSection.cs b/osu.Game/Overlays/Profile/Sections/RanksSection.cs index 00a68d5bf9..02d8bd8c52 100644 --- a/osu.Game/Overlays/Profile/Sections/RanksSection.cs +++ b/osu.Game/Overlays/Profile/Sections/RanksSection.cs @@ -18,6 +18,8 @@ namespace osu.Game.Overlays.Profile.Sections { Children = new[] { + // todo: update to use UsersStrings.ShowExtraTopRanksPinnedTitle once that exists. + new PaginatedScoreContainer(ScoreType.Pinned, User, "Pinned Scores"), new PaginatedScoreContainer(ScoreType.Best, User, UsersStrings.ShowExtraTopRanksBestTitle), new PaginatedScoreContainer(ScoreType.Firsts, User, UsersStrings.ShowExtraTopRanksFirstTitle) }; From 9574bc13820665f5e6721915b3c05367ac9dccb7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 04:00:40 +0300 Subject: [PATCH 28/42] Allow `IRulesetInfo`s of same type to be comparable At first I was planning on making `CompareTo` implemented at `IRulesetInfo` itself and shared across classes, but turns out it only implements it explicitly and not allow direct `IRulesetInfo.Equals` calls. It messed with my head enough that I decided to just let each class have its own implementation and only allow same type. --- osu.Game/Rulesets/IRulesetInfo.cs | 2 +- osu.Game/Rulesets/RulesetInfo.cs | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/IRulesetInfo.cs b/osu.Game/Rulesets/IRulesetInfo.cs index 44731a2495..60a02212fc 100644 --- a/osu.Game/Rulesets/IRulesetInfo.cs +++ b/osu.Game/Rulesets/IRulesetInfo.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets /// /// A representation of a ruleset's metadata. /// - public interface IRulesetInfo : IHasOnlineID, IEquatable, IComparable + public interface IRulesetInfo : IHasOnlineID, IEquatable, IComparable { /// /// The user-exposed name of this ruleset. diff --git a/osu.Game/Rulesets/RulesetInfo.cs b/osu.Game/Rulesets/RulesetInfo.cs index 0a0941d1ff..88e3988431 100644 --- a/osu.Game/Rulesets/RulesetInfo.cs +++ b/osu.Game/Rulesets/RulesetInfo.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets { [ExcludeFromDynamicCompile] [MapTo("Ruleset")] - public class RulesetInfo : RealmObject, IEquatable, IRulesetInfo + public class RulesetInfo : RealmObject, IEquatable, IComparable, IRulesetInfo { [PrimaryKey] public string ShortName { get; set; } = string.Empty; @@ -47,7 +47,7 @@ namespace osu.Game.Rulesets return ShortName == other.ShortName; } - public bool Equals(IRulesetInfo? other) => other is RulesetInfo b && Equals(b); + public bool Equals(IRulesetInfo? other) => other is RulesetInfo r && Equals(r); public int CompareTo(RulesetInfo other) { @@ -63,6 +63,14 @@ namespace osu.Game.Rulesets return string.Compare(ShortName, other.ShortName, StringComparison.Ordinal); } + public int CompareTo(IRulesetInfo other) + { + if (!(other is RulesetInfo ruleset)) + throw new ArgumentException($@"Object is not of type {nameof(RulesetInfo)}.", nameof(other)); + + return CompareTo(ruleset); + } + public override int GetHashCode() { // Importantly, ignore the underlying realm hash code, as it will usually not match. From 1b729e891d53e8134efcd5ef8f09d5655c5bf20f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 04:01:10 +0300 Subject: [PATCH 29/42] Update pointless `CompareTo` implementation once again --- osu.Game/Rulesets/EFRulesetInfo.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/EFRulesetInfo.cs b/osu.Game/Rulesets/EFRulesetInfo.cs index ba56adac49..4174aa773c 100644 --- a/osu.Game/Rulesets/EFRulesetInfo.cs +++ b/osu.Game/Rulesets/EFRulesetInfo.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets { [ExcludeFromDynamicCompile] [Table(@"RulesetInfo")] - public sealed class EFRulesetInfo : IEquatable, IRulesetInfo + public sealed class EFRulesetInfo : IEquatable, IComparable, IRulesetInfo { public int? ID { get; set; } @@ -42,7 +42,15 @@ namespace osu.Game.Rulesets public bool Equals(EFRulesetInfo other) => other != null && ID == other.ID && Available == other.Available && Name == other.Name && InstantiationInfo == other.InstantiationInfo; - public int CompareTo(RulesetInfo other) => OnlineID.CompareTo(other.OnlineID); + public int CompareTo(EFRulesetInfo other) => OnlineID.CompareTo(other.OnlineID); + + public int CompareTo(IRulesetInfo other) + { + if (!(other is EFRulesetInfo ruleset)) + throw new ArgumentException($@"Object is not of type {nameof(EFRulesetInfo)}.", nameof(other)); + + return CompareTo(ruleset); + } public override bool Equals(object obj) => obj is EFRulesetInfo rulesetInfo && Equals(rulesetInfo); From 26839f6ad8f3c49c3d1c63016b3af47e9afd2602 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 04:17:24 +0300 Subject: [PATCH 30/42] Consider `OnlineID`s during ruleset equality if available Required for `APIBeatmap`s, which provide `Ruleset` instances with `OnlineID` available only. Also consistent with the comparer implementation. --- osu.Game/Rulesets/RulesetInfo.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Rulesets/RulesetInfo.cs b/osu.Game/Rulesets/RulesetInfo.cs index 88e3988431..ba7c8d191d 100644 --- a/osu.Game/Rulesets/RulesetInfo.cs +++ b/osu.Game/Rulesets/RulesetInfo.cs @@ -44,6 +44,9 @@ namespace osu.Game.Rulesets if (ReferenceEquals(this, other)) return true; if (other == null) return false; + if (OnlineID >= 0 && other.OnlineID >= 0) + return OnlineID == other.OnlineID; + return ShortName == other.ShortName; } From 6f0e32826c6d90c92c1130cd7aa07894e3b7f38f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 04:06:28 +0300 Subject: [PATCH 31/42] Standardise ordering/grouping of `IRulesetInfo`/`RulesetInfo`s --- .../Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs | 2 +- osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs | 6 ++---- osu.Game/Screens/Edit/Editor.cs | 2 +- osu.Game/Screens/Select/Carousel/SetPanelContent.cs | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs b/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs index 7753d8480a..eeb86f4702 100644 --- a/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs +++ b/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs @@ -33,7 +33,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards bool firstGroup = true; - foreach (var group in beatmapSetInfo.Beatmaps.GroupBy(beatmap => beatmap.Ruleset.OnlineID).OrderBy(group => group.Key)) + foreach (var group in beatmapSetInfo.Beatmaps.GroupBy(beatmap => beatmap.Ruleset).OrderBy(group => group.Key)) { if (!firstGroup) { diff --git a/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs b/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs index 5b211084ab..5b467d67e2 100644 --- a/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs +++ b/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs @@ -62,10 +62,8 @@ namespace osu.Game.Beatmaps.Drawables // matching web: https://github.com/ppy/osu-web/blob/d06d8c5e735eb1f48799b1654b528e9a7afb0a35/resources/assets/lib/beatmapset-panel.tsx#L127 bool collapsed = beatmapSet.Beatmaps.Count() > 12; - foreach (var rulesetGrouping in beatmapSet.Beatmaps.GroupBy(beatmap => beatmap.Ruleset.OnlineID).OrderBy(group => group.Key)) - { - flow.Add(new RulesetDifficultyGroup(rulesetGrouping.Key, rulesetGrouping, collapsed)); - } + foreach (var rulesetGrouping in beatmapSet.Beatmaps.GroupBy(beatmap => beatmap.Ruleset).OrderBy(group => group.Key)) + flow.Add(new RulesetDifficultyGroup(rulesetGrouping.Key.OnlineID, rulesetGrouping, collapsed)); } protected override void LoadComplete() diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 5503a62ba2..2aec63fa65 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -851,7 +851,7 @@ namespace osu.Game.Screens.Edit var difficultyItems = new List(); - foreach (var rulesetBeatmaps in beatmapSet.Beatmaps.GroupBy(b => b.Ruleset.ShortName).OrderBy(group => group.Key)) + foreach (var rulesetBeatmaps in beatmapSet.Beatmaps.GroupBy(b => b.Ruleset).OrderBy(group => group.Key)) { if (difficultyItems.Count > 0) difficultyItems.Add(new EditorMenuItemSpacer()); diff --git a/osu.Game/Screens/Select/Carousel/SetPanelContent.cs b/osu.Game/Screens/Select/Carousel/SetPanelContent.cs index 82523c9d9d..760915b528 100644 --- a/osu.Game/Screens/Select/Carousel/SetPanelContent.cs +++ b/osu.Game/Screens/Select/Carousel/SetPanelContent.cs @@ -87,7 +87,7 @@ namespace osu.Game.Screens.Select.Carousel var beatmaps = carouselSet.Beatmaps.ToList(); return beatmaps.Count > maximum_difficulty_icons - ? (IEnumerable)beatmaps.GroupBy(b => b.BeatmapInfo.Ruleset.ShortName) + ? (IEnumerable)beatmaps.GroupBy(b => b.BeatmapInfo.Ruleset) .Select(group => new FilterableGroupedDifficultyIcon(group.ToList(), group.Last().BeatmapInfo.Ruleset)) : beatmaps.Select(b => new FilterableDifficultyIcon(b)); } From c29cc78853f9cfb5efc4e923c7d9fca6fda8fbf7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 04:44:54 +0300 Subject: [PATCH 32/42] Fix failing test case --- .../Visual/UserInterface/TestSceneModSelectOverlay.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs index b429619044..9083415a78 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs @@ -457,10 +457,12 @@ namespace osu.Game.Tests.Visual.UserInterface public override ModType Type => ModType.Conversion; } - private class TestUnimplementedModOsuRuleset : OsuRuleset + private class TestUnimplementedModOsuRuleset : OsuRuleset, ILegacyRuleset { public override string ShortName => "unimplemented"; + int ILegacyRuleset.LegacyID => -1; + public override IEnumerable GetModsFor(ModType type) { if (type == ModType.Conversion) return base.GetModsFor(type).Concat(new[] { new TestUnimplementedMod() }); From 92e22c57a77a2860e2ddc78ee85da46f2e5f43d4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 08:02:51 +0300 Subject: [PATCH 33/42] Introduce private `APIRuleset` for online ID equality comparison --- .../API/Requests/Responses/APIBeatmap.cs | 26 ++++++++++++++++++- osu.Game/Rulesets/RulesetInfo.cs | 3 --- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs b/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs index ebbac0dcab..dca60e54cb 100644 --- a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs +++ b/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs @@ -98,7 +98,7 @@ namespace osu.Game.Online.API.Requests.Responses public string MD5Hash => Checksum; - public IRulesetInfo Ruleset => new RulesetInfo { OnlineID = RulesetID }; + public IRulesetInfo Ruleset => new APIRuleset { OnlineID = RulesetID }; [JsonIgnore] public string Hash => throw new NotImplementedException(); @@ -106,5 +106,29 @@ namespace osu.Game.Online.API.Requests.Responses #endregion public bool Equals(IBeatmapInfo? other) => other is APIBeatmap b && this.MatchesOnlineID(b); + + private class APIRuleset : IRulesetInfo + { + public int OnlineID { get; set; } = -1; + + public string Name => $@"{nameof(APIRuleset)} (ID: {OnlineID})"; + public string ShortName => nameof(APIRuleset); + public string InstantiationInfo => string.Empty; + + public Ruleset CreateInstance() => throw new NotImplementedException(); + + public bool Equals(IRulesetInfo? other) => other is APIRuleset r && this.MatchesOnlineID(r); + + public int CompareTo(IRulesetInfo other) + { + if (!(other is APIRuleset ruleset)) + throw new ArgumentException($@"Object is not of type {nameof(APIRuleset)}.", nameof(other)); + + return OnlineID.CompareTo(ruleset.OnlineID); + } + + // ReSharper disable once NonReadonlyMemberInGetHashCode + public override int GetHashCode() => OnlineID; + } } } diff --git a/osu.Game/Rulesets/RulesetInfo.cs b/osu.Game/Rulesets/RulesetInfo.cs index ba7c8d191d..88e3988431 100644 --- a/osu.Game/Rulesets/RulesetInfo.cs +++ b/osu.Game/Rulesets/RulesetInfo.cs @@ -44,9 +44,6 @@ namespace osu.Game.Rulesets if (ReferenceEquals(this, other)) return true; if (other == null) return false; - if (OnlineID >= 0 && other.OnlineID >= 0) - return OnlineID == other.OnlineID; - return ShortName == other.ShortName; } From b06caf2bf7b3a8705d26101f5e1fba464542a701 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 09:12:02 +0300 Subject: [PATCH 34/42] Update resources --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 147f576c55..1a2859c851 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -51,7 +51,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index dd10807ec2..a9c0226951 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -37,7 +37,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 6fbc468586..5e0b264834 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -61,7 +61,7 @@ - + From f049f175d5d2d3a971bce6825d8cdbd2fedea42d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 11 Feb 2022 15:19:55 +0900 Subject: [PATCH 35/42] Revert "Fix failing test case" This reverts commit c29cc78853f9cfb5efc4e923c7d9fca6fda8fbf7. --- .../Visual/UserInterface/TestSceneModSelectOverlay.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs index 9083415a78..b429619044 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs @@ -457,12 +457,10 @@ namespace osu.Game.Tests.Visual.UserInterface public override ModType Type => ModType.Conversion; } - private class TestUnimplementedModOsuRuleset : OsuRuleset, ILegacyRuleset + private class TestUnimplementedModOsuRuleset : OsuRuleset { public override string ShortName => "unimplemented"; - int ILegacyRuleset.LegacyID => -1; - public override IEnumerable GetModsFor(ModType type) { if (type == ModType.Conversion) return base.GetModsFor(type).Concat(new[] { new TestUnimplementedMod() }); From f012f64fd1f441c96c589035afd63010b41d0b1e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Feb 2022 16:45:57 +0900 Subject: [PATCH 36/42] Add test coverage checking carousel panel visual state after ruleset filter change --- .../SongSelect/TestSceneBeatmapCarousel.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 4e46901e08..540b820250 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -41,6 +41,68 @@ namespace osu.Game.Tests.Visual.SongSelect this.rulesets = rulesets; } + [Test] + public void TestExternalRulesetChange() + { + createCarousel(new List()); + + AddStep("filter to ruleset 0", () => carousel.Filter(new FilterCriteria + { + Ruleset = rulesets.AvailableRulesets.ElementAt(0), + AllowConvertedBeatmaps = true, + }, false)); + + AddStep("add mixed ruleset beatmapset", () => + { + var testMixed = TestResources.CreateTestBeatmapSetInfo(3); + + for (int i = 0; i <= 2; i++) + { + testMixed.Beatmaps[i].Ruleset = rulesets.AvailableRulesets.ElementAt(i); + } + + carousel.UpdateBeatmapSet(testMixed); + }); + + AddUntilStep("wait for filtered difficulties", () => + { + var visibleBeatmapPanels = carousel.Items.OfType().Where(p => p.IsPresent).ToArray(); + + return visibleBeatmapPanels.Length == 1 + && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item).BeatmapInfo.Ruleset.OnlineID == 0) == 1; + }); + + AddStep("filter to ruleset 1", () => carousel.Filter(new FilterCriteria + { + Ruleset = rulesets.AvailableRulesets.ElementAt(1), + AllowConvertedBeatmaps = true, + }, false)); + + AddUntilStep("wait for filtered difficulties", () => + { + var visibleBeatmapPanels = carousel.Items.OfType().Where(p => p.IsPresent).ToArray(); + + return visibleBeatmapPanels.Length == 2 + && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item).BeatmapInfo.Ruleset.OnlineID == 0) == 1 + && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item).BeatmapInfo.Ruleset.OnlineID == 1) == 1; + }); + + AddStep("filter to ruleset 2", () => carousel.Filter(new FilterCriteria + { + Ruleset = rulesets.AvailableRulesets.ElementAt(2), + AllowConvertedBeatmaps = true, + }, false)); + + AddUntilStep("wait for filtered difficulties", () => + { + var visibleBeatmapPanels = carousel.Items.OfType().Where(p => p.IsPresent).ToArray(); + + return visibleBeatmapPanels.Length == 2 + && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item).BeatmapInfo.Ruleset.OnlineID == 0) == 1 + && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item).BeatmapInfo.Ruleset.OnlineID == 2) == 1; + }); + } + [Test] public void TestScrollPositionMaintainedOnAdd() { From ccd664896185a0aff852fb8eaf6c47cb1b7bf125 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Feb 2022 09:22:09 +0300 Subject: [PATCH 37/42] Update pinned score container header to use localised title --- osu.Game/Overlays/Profile/Sections/RanksSection.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Profile/Sections/RanksSection.cs b/osu.Game/Overlays/Profile/Sections/RanksSection.cs index 02d8bd8c52..f48e33dc12 100644 --- a/osu.Game/Overlays/Profile/Sections/RanksSection.cs +++ b/osu.Game/Overlays/Profile/Sections/RanksSection.cs @@ -18,8 +18,7 @@ namespace osu.Game.Overlays.Profile.Sections { Children = new[] { - // todo: update to use UsersStrings.ShowExtraTopRanksPinnedTitle once that exists. - new PaginatedScoreContainer(ScoreType.Pinned, User, "Pinned Scores"), + new PaginatedScoreContainer(ScoreType.Pinned, User, UsersStrings.ShowExtraTopRanksPinnedTitle), new PaginatedScoreContainer(ScoreType.Best, User, UsersStrings.ShowExtraTopRanksBestTitle), new PaginatedScoreContainer(ScoreType.Firsts, User, UsersStrings.ShowExtraTopRanksFirstTitle) }; From beb3731c0b079c6b2bdaf1d051bd51b5350899f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 11 Feb 2022 15:53:45 +0900 Subject: [PATCH 38/42] Standardise and combine base implementation of score submission requests These share too much yet have very different constructor signatures and property exposure. Just a clean-up pass as I begin to look at replay submission. --- .../Online/Rooms/SubmitRoomScoreRequest.cs | 32 ++------------- osu.Game/Online/Rooms/SubmitScoreRequest.cs | 41 +++++++++++++++++++ .../Online/Solo/SubmitSoloScoreRequest.cs | 33 ++------------- osu.Game/Screens/Play/RoomSubmittingPlayer.cs | 2 +- osu.Game/Screens/Play/SoloPlayer.cs | 2 +- 5 files changed, 51 insertions(+), 59 deletions(-) create mode 100644 osu.Game/Online/Rooms/SubmitScoreRequest.cs diff --git a/osu.Game/Online/Rooms/SubmitRoomScoreRequest.cs b/osu.Game/Online/Rooms/SubmitRoomScoreRequest.cs index e24d113822..39193be1af 100644 --- a/osu.Game/Online/Rooms/SubmitRoomScoreRequest.cs +++ b/osu.Game/Online/Rooms/SubmitRoomScoreRequest.cs @@ -1,46 +1,22 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Net.Http; -using Newtonsoft.Json; -using osu.Framework.IO.Network; -using osu.Game.Online.API; -using osu.Game.Online.Solo; using osu.Game.Scoring; namespace osu.Game.Online.Rooms { - public class SubmitRoomScoreRequest : APIRequest + public class SubmitRoomScoreRequest : SubmitScoreRequest { - private readonly long scoreId; private readonly long roomId; private readonly long playlistItemId; - private readonly SubmittableScore score; - public SubmitRoomScoreRequest(long scoreId, long roomId, long playlistItemId, ScoreInfo scoreInfo) + public SubmitRoomScoreRequest(ScoreInfo scoreInfo, long scoreId, long roomId, long playlistItemId) + : base(scoreInfo, scoreId) { - this.scoreId = scoreId; this.roomId = roomId; this.playlistItemId = playlistItemId; - score = new SubmittableScore(scoreInfo); } - protected override WebRequest CreateWebRequest() - { - var req = base.CreateWebRequest(); - - req.ContentType = "application/json"; - req.Method = HttpMethod.Put; - req.Timeout = 30000; - - req.AddRaw(JsonConvert.SerializeObject(score, new JsonSerializerSettings - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore - })); - - return req; - } - - protected override string Target => $@"rooms/{roomId}/playlist/{playlistItemId}/scores/{scoreId}"; + protected override string Target => $@"rooms/{roomId}/playlist/{playlistItemId}/scores/{ScoreId}"; } } diff --git a/osu.Game/Online/Rooms/SubmitScoreRequest.cs b/osu.Game/Online/Rooms/SubmitScoreRequest.cs new file mode 100644 index 0000000000..14f858f007 --- /dev/null +++ b/osu.Game/Online/Rooms/SubmitScoreRequest.cs @@ -0,0 +1,41 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Net.Http; +using Newtonsoft.Json; +using osu.Framework.IO.Network; +using osu.Game.Online.API; +using osu.Game.Online.Solo; +using osu.Game.Scoring; + +namespace osu.Game.Online.Rooms +{ + public abstract class SubmitScoreRequest : APIRequest + { + public readonly SubmittableScore Score; + + protected readonly long ScoreId; + + protected SubmitScoreRequest(ScoreInfo scoreInfo, long scoreId) + { + Score = new SubmittableScore(scoreInfo); + this.ScoreId = scoreId; + } + + protected override WebRequest CreateWebRequest() + { + var req = base.CreateWebRequest(); + + req.ContentType = "application/json"; + req.Method = HttpMethod.Put; + req.Timeout = 30000; + + req.AddRaw(JsonConvert.SerializeObject(Score, new JsonSerializerSettings + { + ReferenceLoopHandling = ReferenceLoopHandling.Ignore + })); + + return req; + } + } +} diff --git a/osu.Game/Online/Solo/SubmitSoloScoreRequest.cs b/osu.Game/Online/Solo/SubmitSoloScoreRequest.cs index 78ebddb2e6..77fd7b813b 100644 --- a/osu.Game/Online/Solo/SubmitSoloScoreRequest.cs +++ b/osu.Game/Online/Solo/SubmitSoloScoreRequest.cs @@ -1,46 +1,21 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Net.Http; -using Newtonsoft.Json; -using osu.Framework.IO.Network; -using osu.Game.Online.API; using osu.Game.Online.Rooms; using osu.Game.Scoring; namespace osu.Game.Online.Solo { - public class SubmitSoloScoreRequest : APIRequest + public class SubmitSoloScoreRequest : SubmitScoreRequest { - public readonly SubmittableScore Score; - - private readonly long scoreId; - private readonly int beatmapId; - public SubmitSoloScoreRequest(int beatmapId, long scoreId, ScoreInfo scoreInfo) + public SubmitSoloScoreRequest(ScoreInfo scoreInfo, long scoreId, int beatmapId) + : base(scoreInfo, scoreId) { this.beatmapId = beatmapId; - this.scoreId = scoreId; - Score = new SubmittableScore(scoreInfo); } - protected override WebRequest CreateWebRequest() - { - var req = base.CreateWebRequest(); - - req.ContentType = "application/json"; - req.Method = HttpMethod.Put; - req.Timeout = 30000; - - req.AddRaw(JsonConvert.SerializeObject(Score, new JsonSerializerSettings - { - ReferenceLoopHandling = ReferenceLoopHandling.Ignore - })); - - return req; - } - - protected override string Target => $@"beatmaps/{beatmapId}/solo/scores/{scoreId}"; + protected override string Target => $@"beatmaps/{beatmapId}/solo/scores/{ScoreId}"; } } diff --git a/osu.Game/Screens/Play/RoomSubmittingPlayer.cs b/osu.Game/Screens/Play/RoomSubmittingPlayer.cs index 1002e7607f..fc96dfa965 100644 --- a/osu.Game/Screens/Play/RoomSubmittingPlayer.cs +++ b/osu.Game/Screens/Play/RoomSubmittingPlayer.cs @@ -34,7 +34,7 @@ namespace osu.Game.Screens.Play protected override APIRequest CreateSubmissionRequest(Score score, long token) { Debug.Assert(Room.RoomID.Value != null); - return new SubmitRoomScoreRequest(token, Room.RoomID.Value.Value, PlaylistItem.ID, score.ScoreInfo); + return new SubmitRoomScoreRequest(score.ScoreInfo, token, Room.RoomID.Value.Value, PlaylistItem.ID); } } } diff --git a/osu.Game/Screens/Play/SoloPlayer.cs b/osu.Game/Screens/Play/SoloPlayer.cs index eced2d142b..824c0072e3 100644 --- a/osu.Game/Screens/Play/SoloPlayer.cs +++ b/osu.Game/Screens/Play/SoloPlayer.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.Play Debug.Assert(beatmap.OnlineID > 0); - return new SubmitSoloScoreRequest(beatmap.OnlineID, token, score.ScoreInfo); + return new SubmitSoloScoreRequest(score.ScoreInfo, token, beatmap.OnlineID); } } } From 28bbf34b1495908b1e145f8575f6ffaaf0979bd4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 11 Feb 2022 16:21:49 +0900 Subject: [PATCH 39/42] Remove unnecessary `this.` prefix --- osu.Game/Online/Rooms/SubmitScoreRequest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/SubmitScoreRequest.cs b/osu.Game/Online/Rooms/SubmitScoreRequest.cs index 14f858f007..b263262d2b 100644 --- a/osu.Game/Online/Rooms/SubmitScoreRequest.cs +++ b/osu.Game/Online/Rooms/SubmitScoreRequest.cs @@ -19,7 +19,7 @@ namespace osu.Game.Online.Rooms protected SubmitScoreRequest(ScoreInfo scoreInfo, long scoreId) { Score = new SubmittableScore(scoreInfo); - this.ScoreId = scoreId; + ScoreId = scoreId; } protected override WebRequest CreateWebRequest() From 2ed3d5853144436f76260feb38f0d4d1c3a56e52 Mon Sep 17 00:00:00 2001 From: PercyDan54 <50285552+PercyDan54@users.noreply.github.com> Date: Sat, 12 Feb 2022 08:51:09 +0800 Subject: [PATCH 40/42] Ignore short spinners for relax mod --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 5d191119b9..905d55c64e 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -87,8 +87,9 @@ namespace osu.Game.Rulesets.Osu.Mods requiresHold |= slider.Ball.IsHovered || h.IsHovered; break; - case DrawableSpinner _: - requiresHold = true; + case DrawableSpinner spinner: + if (spinner.HitObject.SpinsRequired > 0) + requiresHold = true; break; } } From 053f41d755999ec120c1674c958cfadadb7898f7 Mon Sep 17 00:00:00 2001 From: PercyDan54 <50285552+PercyDan54@users.noreply.github.com> Date: Sat, 12 Feb 2022 10:06:43 +0800 Subject: [PATCH 41/42] Simplify code --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 905d55c64e..10abd24e80 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -88,8 +88,7 @@ namespace osu.Game.Rulesets.Osu.Mods break; case DrawableSpinner spinner: - if (spinner.HitObject.SpinsRequired > 0) - requiresHold = true; + requiresHold = spinner.HitObject.SpinsRequired > 0; break; } } From 639d813d06fe2275b6220fc41b6133e0390de5f5 Mon Sep 17 00:00:00 2001 From: PercyDan <50285552+PercyDan54@users.noreply.github.com> Date: Sat, 12 Feb 2022 11:15:03 +0800 Subject: [PATCH 42/42] Don't override previous value Co-authored-by: Salman Ahmed --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 10abd24e80..1bf63ef6d4 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -88,7 +88,7 @@ namespace osu.Game.Rulesets.Osu.Mods break; case DrawableSpinner spinner: - requiresHold = spinner.HitObject.SpinsRequired > 0; + requiresHold |= spinner.HitObject.SpinsRequired > 0; break; } }