From f4e155bfa6a6f8158a578540e9e01621e5b46553 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Sat, 30 Nov 2024 15:19:35 +0100 Subject: [PATCH 1/4] Account for rate changing mods when disabling the "Ready" button --- .../Playlists/PlaylistsReadyButton.cs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs index a460779ea6..3c7808356c 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.ComponentModel; using System.Linq; using osu.Framework.Allocation; @@ -10,7 +11,9 @@ using osu.Framework.Localisation; using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Online.Rooms; +using osu.Game.Rulesets.Mods; using osu.Game.Screens.OnlinePlay.Components; +using osu.Game.Utils; namespace osu.Game.Screens.OnlinePlay.Playlists { @@ -19,6 +22,9 @@ namespace osu.Game.Screens.OnlinePlay.Playlists [Resolved] private IBindable gameBeatmap { get; set; } = null!; + [Resolved] + private IBindable> mods { get; set; } = null!; + private readonly Room room; public PlaylistsReadyButton(Room room) @@ -63,14 +69,14 @@ namespace osu.Game.Screens.OnlinePlay.Playlists { base.Update(); - Enabled.Value = hasRemainingAttempts && enoughTimeLeft; + Enabled.Value = hasRemainingAttempts && enoughTimeLeft(); } public override LocalisableString TooltipText { get { - if (!enoughTimeLeft) + if (!enoughTimeLeft()) return "No time left!"; if (!hasRemainingAttempts) @@ -80,9 +86,16 @@ namespace osu.Game.Screens.OnlinePlay.Playlists } } - private bool enoughTimeLeft => + private bool enoughTimeLeft() + { + // this doesn't consider mods which apply variable rates, yet. + double rate = ModUtils.CalculateRateWithMods(mods.Value); + + double hitLength = Math.Round(gameBeatmap.Value.Track.Length / rate); + // This should probably consider the length of the currently selected item, rather than a constant 30 seconds. - room.EndDate != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(gameBeatmap.Value.Track.Length) < room.EndDate; + return room.EndDate != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(hitLength) < room.EndDate; + } protected override void Dispose(bool isDisposing) { From 164b809c8911262c5f8f775b5c8c4c3ce843afc7 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Sat, 30 Nov 2024 23:02:22 +0100 Subject: [PATCH 2/4] Document ready button enable state with some comments --- .../OnlinePlay/Playlists/PlaylistsReadyButton.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs index 3c7808356c..0a4b504749 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs @@ -88,13 +88,15 @@ namespace osu.Game.Screens.OnlinePlay.Playlists private bool enoughTimeLeft() { - // this doesn't consider mods which apply variable rates, yet. + // TODO: This doesn't consider mods which apply variable rates, yet. double rate = ModUtils.CalculateRateWithMods(mods.Value); - double hitLength = Math.Round(gameBeatmap.Value.Track.Length / rate); + // We want to avoid users not being able to submit scores if they chose to not skip, + // so track length is chosen over playable length. + double trackLength = Math.Round(gameBeatmap.Value.Track.Length / rate); - // This should probably consider the length of the currently selected item, rather than a constant 30 seconds. - return room.EndDate != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(hitLength) < room.EndDate; + // Additional 30 second delay added to account for load and/or submit time. + return room.EndDate != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(trackLength) < room.EndDate; } protected override void Dispose(bool isDisposing) From e920cfa1872d233f90df27f4db76ffd0e75da6a8 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Mon, 2 Dec 2024 23:49:26 +0100 Subject: [PATCH 3/4] Move rate-changing TODO to a common place in CalculateRateWithMods --- osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs | 1 - osu.Game/Screens/Select/BeatmapInfoWedge.cs | 1 - osu.Game/Utils/ModUtils.cs | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs index 0a4b504749..e72f8be50a 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs @@ -88,7 +88,6 @@ namespace osu.Game.Screens.OnlinePlay.Playlists private bool enoughTimeLeft() { - // TODO: This doesn't consider mods which apply variable rates, yet. double rate = ModUtils.CalculateRateWithMods(mods.Value); // We want to avoid users not being able to submit scores if they chose to not skip, diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index 3b0fdc3e47..fd1c944689 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -401,7 +401,6 @@ namespace osu.Game.Screens.Select if (beatmap == null || bpmLabelContainer == null) return; - // this doesn't consider mods which apply variable rates, yet. double rate = ModUtils.CalculateRateWithMods(mods.Value); int bpmMax = FormatUtils.RoundBPM(beatmap.ControlPointInfo.BPMMaximum, rate); diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index f901f15388..15fc34b468 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -286,6 +286,7 @@ namespace osu.Game.Utils { double rate = 1; + // TODO: This doesn't consider mods which apply variable rates, yet. foreach (var mod in mods.OfType()) rate = mod.ApplyToRate(0, rate); From 6ff1dec7b2b9fa2eebcd96620c316ddfc7a67c6e Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 3 Dec 2024 15:45:58 +0900 Subject: [PATCH 4/4] Add tests --- .../TestScenePlaylistsRoomSubScreen.cs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs index 5f9e06fda5..de84ca680d 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs @@ -5,25 +5,64 @@ using System; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Extensions; +using osu.Framework.Platform; using osu.Framework.Screens; using osu.Framework.Testing; +using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Rooms; using osu.Game.Online.Rooms.RoomStatuses; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Scoring; using osu.Game.Screens.OnlinePlay.Playlists; +using osu.Game.Tests.Resources; using osu.Game.Tests.Visual.OnlinePlay; namespace osu.Game.Tests.Visual.Playlists { public partial class TestScenePlaylistsRoomSubScreen : OnlinePlayTestScene { + private const double track_length = 10000; + [Resolved] private IAPIProvider api { get; set; } = null!; protected new TestRoomManager RoomManager => (TestRoomManager)base.RoomManager; + private BeatmapManager beatmaps = null!; + private RulesetStore rulesets = null!; + private BeatmapSetInfo? importedSet; + + [BackgroundDependencyLoader] + private void load(GameHost host, AudioManager audio) + { + Dependencies.Cache(rulesets = new RealmRulesetStore(Realm)); + Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default)); + Dependencies.Cache(new ScoreManager(rulesets, () => beatmaps, LocalStorage, Realm, API)); + Dependencies.Cache(Realm); + + beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely(); + + Realm.Write(r => + { + foreach (var set in r.All()) + { + foreach (var b in set.Beatmaps) + { + // These will all have a virtual track length of 1000, see WorkingBeatmap.GetVirtualTrack(). + b.Length = track_length - 1000; + } + } + }); + + importedSet = beatmaps.GetAllUsableBeatmapSets().First(); + } + [Test] public void TestStatusUpdateOnEnter() { @@ -69,5 +108,42 @@ namespace osu.Game.Tests.Visual.Playlists AddAssert("close button present", () => roomScreen.ChildrenOfType().Any()); AddUntilStep("wait for close button to disappear", () => !roomScreen.ChildrenOfType().Any()); } + + [TestCase(120_000, true)] // Definitely enough time. + [TestCase(45_000, true)] // Enough time. + [TestCase(35_000, false)] // Not enough time to complete beatmap after lenience. + [TestCase(20_000, false)] // Not enough time. + [TestCase(5_000, false)] // Not enough time to complete beatmap before lenience. + [TestCase(37_500, true, 2)] // Enough time to complete beatmap after mods are applied. + public void TestReadyButtonEnablementPeriod(int offsetMs, bool enabled, double rate = 1) + { + Room room = null!; + PlaylistsRoomSubScreen roomScreen = null!; + + AddStep("create room", () => + { + RoomManager.AddRoom(room = new Room + { + Name = @"Test Room", + Host = api.LocalUser.Value, + Category = RoomCategory.Normal, + StartDate = DateTimeOffset.Now, + EndDate = DateTimeOffset.Now.AddMilliseconds(offsetMs), + Playlist = + [ + new PlaylistItem(importedSet!.Beatmaps[0]) + { + RequiredMods = rate == 1 + ? [] + : [new APIMod(new OsuModDoubleTime { SpeedChange = { Value = rate } })] + } + ] + }); + }); + + AddStep("push screen", () => LoadScreen(roomScreen = new PlaylistsRoomSubScreen(room))); + AddUntilStep("wait for screen load", () => roomScreen.IsCurrentScreen()); + AddUntilStep("ready button enabled", () => roomScreen.ChildrenOfType().SingleOrDefault()?.Enabled.Value, () => Is.EqualTo(enabled)); + } } }