From 133a1bc1fada7b2ca93cbcd9c4e9e7385a7907c8 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Mar 2025 21:26:14 +0900 Subject: [PATCH 1/7] Add test for changing item in list Brought forward from https://github.com/ppy/osu/pull/32250. --- .../TestSceneMultiplayerQueueList.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerQueueList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerQueueList.cs index 7283e3a1fe..d7659351bb 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerQueueList.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerQueueList.cs @@ -49,13 +49,15 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("create playlist", () => { - Child = playlist = new MultiplayerQueueList(room) + Child = playlist = new MultiplayerQueueList { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Size = new Vector2(500, 300), + Size = new Vector2(500, 300) }; + playlist.Items.ReplaceRange(0, playlist.Items.Count, MultiplayerClient.ClientAPIRoom!.Playlist); + MultiplayerClient.ClientAPIRoom!.PropertyChanged += (_, e) => { if (e.PropertyName == nameof(Room.Playlist)) @@ -132,6 +134,18 @@ namespace osu.Game.Tests.Visual.Multiplayer assertDeleteButtonVisibility(1, false); } + [Test] + public void TestChangeExistingItem() + { + AddStep("change beatmap", () => MultiplayerClient.EditPlaylistItem(new MultiplayerPlaylistItem + { + ID = playlist.Items[0].ID, + BeatmapID = 1337 + }).WaitSafely()); + + AddUntilStep("first playlist item has new beatmap", () => playlist.Items[0].Beatmap.OnlineID, () => Is.EqualTo(1337)); + } + private void addPlaylistItem(Func userId) { long itemId = -1; From 67eb691c8310620b629211626ec5425de9b2538f Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Mar 2025 21:49:32 +0900 Subject: [PATCH 2/7] Fix room ids not being incremented in tests Tests will [preserve](https://github.com/ppy/osu/blob/f4c96ecb54ff7650e617597faa583f1f40ce323b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs#L270) the incoming `RoomId` value if non-null. But since now everything goes through `MultiplayerClient.CreateRoom(MultiplayerRoom)` which internally creates a representative `Room` to return in requests, this would previously return a `Room` with `RoomId == 0` and not the expected `null` value. The only consumer of this outside of tests is [server-spectator](https://github.com/ppy/osu-server-spectator/blob/c1f33672cc2245614bdcfeb3109801b20a89c709/osu.Server.Spectator/Services/SharedInterop.cs#L147), but only in the context of creating rooms at which point RoomId is irrelevant. --- osu.Game/Online/Rooms/Room.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/Room.cs b/osu.Game/Online/Rooms/Room.cs index e965f9c187..b93917eff6 100644 --- a/osu.Game/Online/Rooms/Room.cs +++ b/osu.Game/Online/Rooms/Room.cs @@ -348,7 +348,7 @@ namespace osu.Game.Online.Rooms public Room(MultiplayerRoom room) { - RoomID = room.RoomID; + RoomID = room.RoomID > 0 ? room.RoomID : null; Name = room.Settings.Name; Password = room.Settings.Password; Type = room.Settings.MatchType; From b93e21c6667acfbf51e88ac2a0f62da58daf20a3 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Mar 2025 21:51:30 +0900 Subject: [PATCH 3/7] Refactor `MultiplayerPlaylist` to not have a selected item bindable --- .../TestSceneMultiplayerPlaylist.cs | 6 +- .../Match/Playlist/MultiplayerPlaylist.cs | 61 ++++++++----------- .../Match/Playlist/MultiplayerQueueList.cs | 35 +---------- .../Multiplayer/MultiplayerMatchSubScreen.cs | 5 +- 4 files changed, 31 insertions(+), 76 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs index 1affa08813..1bde02762e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs @@ -6,7 +6,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; -using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Platform; @@ -53,13 +52,12 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("create list", () => { - Child = list = new MultiplayerPlaylist(room) + Child = list = new MultiplayerPlaylist { Anchor = Anchor.Centre, Origin = Anchor.Centre, RelativeSizeAxes = Axes.Both, - Size = new Vector2(0.4f, 0.8f), - SelectedItem = new Bindable() + Size = new Vector2(0.4f, 0.8f) }; }); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs index 9feee0ae41..8f59db467d 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs @@ -7,6 +7,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; @@ -19,12 +20,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist { public readonly Bindable DisplayMode = new Bindable(); - public required Bindable SelectedItem - { - get => selectedItem; - set => selectedItem.Current = value; - } - /// /// Invoked when an item requests to be edited. /// @@ -33,18 +28,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist [Resolved] private MultiplayerClient client { get; set; } = null!; - private readonly Room room; - private readonly BindableWithCurrent selectedItem = new BindableWithCurrent(); private MultiplayerPlaylistTabControl playlistTabControl = null!; private MultiplayerQueueList queueList = null!; private MultiplayerHistoryList historyList = null!; private bool firstPopulation = true; - public MultiplayerPlaylist(Room room) - { - this.room = room; - } - [BackgroundDependencyLoader] private void load() { @@ -65,17 +53,15 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist Masking = true, Children = new Drawable[] { - queueList = new MultiplayerQueueList(room) + queueList = new MultiplayerQueueList { RelativeSizeAxes = Axes.Both, - SelectedItem = { BindTarget = selectedItem }, RequestEdit = item => RequestEdit?.Invoke(item) }, historyList = new MultiplayerHistoryList { RelativeSizeAxes = Axes.Both, Alpha = 0, - SelectedItem = { BindTarget = selectedItem } } } } @@ -89,10 +75,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist base.LoadComplete(); DisplayMode.BindValueChanged(onDisplayModeChanged, true); + client.ItemAdded += playlistItemAdded; client.ItemRemoved += playlistItemRemoved; client.ItemChanged += playlistItemChanged; client.RoomUpdated += onRoomUpdated; + updateState(); } @@ -121,28 +109,29 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist firstPopulation = false; } + + // As a small optimisation, only the ID is required to match the selected item. + PlaylistItem? selectedItem = client.Room == null ? null : new PlaylistItem(new APIBeatmap()) { ID = client.Room.Settings.PlaylistItemId }; + queueList.SelectedItem.Value = selectedItem; + historyList.SelectedItem.Value = selectedItem; } - private void playlistItemAdded(MultiplayerPlaylistItem item) => Schedule(() => addItemToLists(item)); + private void playlistItemAdded(MultiplayerPlaylistItem item) => Scheduler.Add(() => addItemToLists(item)); - private void playlistItemRemoved(long item) => Schedule(() => removeItemFromLists(item)); + private void playlistItemRemoved(long item) => Scheduler.Add(() => removeItemFromLists(item)); - private void playlistItemChanged(MultiplayerPlaylistItem item) => Schedule(() => + private void playlistItemChanged(MultiplayerPlaylistItem item) => Scheduler.Add(() => { if (client.Room == null) return; - var newApiItem = new PlaylistItem(item); - var existingApiItemInQueue = queueList.Items.SingleOrDefault(i => i.ID == item.ID); + var existingItem = queueList.Items.SingleOrDefault(i => i.ID == item.ID); // Test if the only change between the two playlist items is the order. - if (existingApiItemInQueue != null && existingApiItemInQueue.With(playlistOrder: newApiItem.PlaylistOrder).Equals(newApiItem)) + if (existingItem != null && existingItem.With(playlistOrder: item.PlaylistOrder).Equals(new PlaylistItem(item))) { - // Set the new playlist order directly without refreshing the DrawablePlaylistItem. - existingApiItemInQueue.PlaylistOrder = newApiItem.PlaylistOrder; - - // The following isn't really required, but is here for safety and explicitness. - // MultiplayerQueueList internally binds to changes in Playlist to invalidate its own layout, which is mutated on every playlist operation. + // Set the new order directly and refresh the flow layout as an optimisation to avoid refreshing the items' visual state. + existingItem.PlaylistOrder = item.PlaylistOrder; queueList.Invalidate(); } else @@ -154,22 +143,22 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist private void addItemToLists(MultiplayerPlaylistItem item) { - var apiItem = client.Room?.Playlist.SingleOrDefault(i => i.ID == item.ID); - - // Item could have been removed from the playlist while the local player was in gameplay. - if (apiItem == null) + if (client.Room == null) return; if (item.Expired) - historyList.Items.Add(new PlaylistItem(apiItem)); + historyList.Items.Add(new PlaylistItem(item)); else - queueList.Items.Add(new PlaylistItem(apiItem)); + queueList.Items.Add(new PlaylistItem(item)); } - private void removeItemFromLists(long item) + private void removeItemFromLists(long itemId) { - queueList.Items.RemoveAll(i => i.ID == item); - historyList.Items.RemoveAll(i => i.ID == item); + if (client.Room == null) + return; + + queueList.Items.RemoveAll(i => i.ID == itemId); + historyList.Items.RemoveAll(i => i.ID == itemId); } } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerQueueList.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerQueueList.cs index 04bb9b69e6..dc6a713908 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerQueueList.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerQueueList.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.ComponentModel; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Extensions.ObjectExtensions; @@ -20,50 +19,20 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist /// public partial class MultiplayerQueueList : DrawableRoomPlaylist { - private readonly Room room; - - private QueueFillFlowContainer flow = null!; - - public MultiplayerQueueList(Room room) + public MultiplayerQueueList() { - this.room = room; ShowItemOwners = true; } - protected override void LoadComplete() - { - base.LoadComplete(); - - room.PropertyChanged += onRoomPropertyChanged; - updateRoomPlaylist(); - } - - private void onRoomPropertyChanged(object? sender, PropertyChangedEventArgs e) - { - if (e.PropertyName == nameof(Room.Playlist)) - updateRoomPlaylist(); - } - - private void updateRoomPlaylist() - => flow.InvalidateLayout(); - - protected override FillFlowContainer> CreateListFillFlowContainer() => flow = new QueueFillFlowContainer + protected override FillFlowContainer> CreateListFillFlowContainer() => new QueueFillFlowContainer { Spacing = new Vector2(0, 2) }; protected override DrawableRoomPlaylistItem CreateDrawablePlaylistItem(PlaylistItem item) => new QueuePlaylistItem(item); - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - room.PropertyChanged -= onRoomPropertyChanged; - } - private partial class QueueFillFlowContainer : FillFlowContainer> { - public new void InvalidateLayout() => base.InvalidateLayout(); - public override IEnumerable FlowingChildren => base.FlowingChildren.OfType>().OrderBy(item => item.Model.PlaylistOrder); } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 6c6932f479..08a469fa03 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -145,11 +145,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer null, new Drawable[] { - new MultiplayerPlaylist(Room) + new MultiplayerPlaylist { RelativeSizeAxes = Axes.Both, - RequestEdit = OpenSongSelection, - SelectedItem = SelectedItem + RequestEdit = OpenSongSelection } }, new[] From 1a0432ce3a17aaf8da1bf84e0526a0aca8fa7145 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Mar 2025 22:01:23 +0900 Subject: [PATCH 4/7] Re-enable ignored playlist test --- .../TestSceneMultiplayerPlaylist.cs | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs index 1bde02762e..c6a203c77a 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs @@ -31,7 +31,6 @@ namespace osu.Game.Tests.Visual.Multiplayer private BeatmapManager beatmaps = null!; private BeatmapSetInfo importedSet = null!; private BeatmapInfo importedBeatmap = null!; - private Room room = null!; [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) @@ -46,8 +45,7 @@ namespace osu.Game.Tests.Visual.Multiplayer { base.SetUpSteps(); - AddStep("create room", () => room = CreateDefaultRoom()); - AddStep("join room", () => JoinRoom(room)); + AddStep("join room", () => JoinRoom(CreateDefaultRoom())); WaitForJoined(); AddStep("create list", () => @@ -156,37 +154,36 @@ namespace osu.Game.Tests.Visual.Multiplayer assertQueueTabCount(0); } - [Ignore("Expired items are initially removed from the room.")] [Test] public void TestJoinRoomWithMixedItemsAddedInCorrectLists() { AddStep("leave room", () => MultiplayerClient.LeaveRoom()); AddUntilStep("wait for room part", () => !RoomJoined); - AddStep("join room with items", () => + AddStep("join room with expired items", () => { - API.Queue(new CreateRoomRequest(new Room - { - Name = "test name", - Playlist = - [ - new PlaylistItem(new TestBeatmap(Ruleset.Value).BeatmapInfo) - { - RulesetID = Ruleset.Value.OnlineID - }, - new PlaylistItem(new TestBeatmap(Ruleset.Value).BeatmapInfo) - { - RulesetID = Ruleset.Value.OnlineID, - Expired = true - } - ] - })); + Room room = CreateDefaultRoom(); + room.Playlist = + [ + new PlaylistItem(new TestBeatmap(Ruleset.Value).BeatmapInfo) + { + RulesetID = Ruleset.Value.OnlineID + }, + new PlaylistItem(new TestBeatmap(Ruleset.Value).BeatmapInfo) + { + RulesetID = Ruleset.Value.OnlineID, + Expired = true + } + ]; + + JoinRoom(room); }); - AddUntilStep("wait for room join", () => RoomJoined); + WaitForJoined(); - assertItemInQueueListStep(1, 0); - assertItemInHistoryListStep(2, 0); + // IDs are offset by 1 because we've joined two rooms in this test. + assertItemInQueueListStep(2, 0); + assertItemInHistoryListStep(3, 0); } [Test] From 36e1077f99a6545b1540e306b2c4e3279a8202a5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Mar 2025 22:07:59 +0900 Subject: [PATCH 5/7] Add missing disposal unbinds This is an issue on `master` and have likely been causing some leaks. --- .../Match/Playlist/MultiplayerPlaylist.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs index 8f59db467d..d44cb1dde8 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Online.API.Requests.Responses; @@ -160,5 +161,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist queueList.Items.RemoveAll(i => i.ID == itemId); historyList.Items.RemoveAll(i => i.ID == itemId); } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (client.IsNotNull()) + { + client.ItemAdded -= playlistItemAdded; + client.ItemRemoved -= playlistItemRemoved; + client.ItemChanged -= playlistItemChanged; + client.RoomUpdated -= onRoomUpdated; + } + } } } From 117e91bfabc343f467b98fea80ea9d9cff017ff2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 31 Mar 2025 16:21:49 +0900 Subject: [PATCH 6/7] Remove local optimisation, use `CurrentPlaylistItem` --- .../Multiplayer/Match/Playlist/MultiplayerPlaylist.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs index d44cb1dde8..fba3acc32a 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs @@ -8,7 +8,6 @@ using osu.Framework.Bindables; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; @@ -111,10 +110,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist firstPopulation = false; } - // As a small optimisation, only the ID is required to match the selected item. - PlaylistItem? selectedItem = client.Room == null ? null : new PlaylistItem(new APIBeatmap()) { ID = client.Room.Settings.PlaylistItemId }; - queueList.SelectedItem.Value = selectedItem; - historyList.SelectedItem.Value = selectedItem; + PlaylistItem? currentItem = client.Room == null ? null : new PlaylistItem(client.Room.CurrentPlaylistItem); + queueList.SelectedItem.Value = currentItem; + historyList.SelectedItem.Value = currentItem; } private void playlistItemAdded(MultiplayerPlaylistItem item) => Scheduler.Add(() => addItemToLists(item)); From 74d234a7debd6be49a8d6eba8045368ab65d1b07 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 31 Mar 2025 16:32:32 +0900 Subject: [PATCH 7/7] Replace RoomID change with test-local fixes --- .../Visual/DailyChallenge/TestSceneDailyChallenge.cs | 8 ++------ .../DailyChallenge/TestSceneDailyChallengeIntro.cs | 11 +++++------ osu.Game/Online/Rooms/Room.cs | 2 +- .../Visual/OnlinePlay/TestRoomRequestsHandler.cs | 2 +- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallenge.cs b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallenge.cs index c974a852f3..185ebc1d39 100644 --- a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallenge.cs +++ b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallenge.cs @@ -43,7 +43,6 @@ namespace osu.Game.Tests.Visual.DailyChallenge { var room = new Room { - RoomID = 1234, Name = "Daily Challenge: June 4, 2024", Playlist = [ @@ -66,7 +65,6 @@ namespace osu.Game.Tests.Visual.DailyChallenge { var room = new Room { - RoomID = 1234, Name = "Daily Challenge: June 4, 2024", Playlist = [ @@ -99,7 +97,6 @@ namespace osu.Game.Tests.Visual.DailyChallenge { var room = new Room { - RoomID = 1234, Name = "Daily Challenge: June 4, 2024", Playlist = [ @@ -114,7 +111,7 @@ namespace osu.Game.Tests.Visual.DailyChallenge }; AddStep("add room", () => API.Perform(new CreateRoomRequest(room))); - AddStep("set daily challenge info", () => metadataClient.DailyChallengeInfo.Value = new DailyChallengeInfo { RoomID = 1234 }); + AddStep("set daily challenge info", () => metadataClient.DailyChallengeInfo.Value = new DailyChallengeInfo { RoomID = room.RoomID!.Value }); Screens.OnlinePlay.DailyChallenge.DailyChallenge screen = null!; AddStep("push screen", () => LoadScreen(screen = new Screens.OnlinePlay.DailyChallenge.DailyChallenge(room))); @@ -128,7 +125,6 @@ namespace osu.Game.Tests.Visual.DailyChallenge { var room = new Room { - RoomID = 1234, Name = "Daily Challenge: June 4, 2024", Playlist = [ @@ -143,7 +139,7 @@ namespace osu.Game.Tests.Visual.DailyChallenge }; AddStep("add room", () => API.Perform(new CreateRoomRequest(room))); - AddStep("set daily challenge info", () => metadataClient.DailyChallengeInfo.Value = new DailyChallengeInfo { RoomID = 1234 }); + AddStep("set daily challenge info", () => metadataClient.DailyChallengeInfo.Value = new DailyChallengeInfo { RoomID = room.RoomID!.Value }); Screens.OnlinePlay.DailyChallenge.DailyChallenge screen = null!; AddStep("push screen", () => LoadScreen(screen = new Screens.OnlinePlay.DailyChallenge.DailyChallenge(room))); diff --git a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs index d6665e24a4..97b957df43 100644 --- a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs +++ b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs @@ -44,17 +44,17 @@ namespace osu.Game.Tests.Visual.DailyChallenge [Test] public void TestDailyChallenge() { - startChallenge(1234); + startChallenge(); AddStep("push screen", () => LoadScreen(new DailyChallengeIntro(room))); } [Test] public void TestPlayIntroOnceFlag() { - startChallenge(1234); + startChallenge(); AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); - startChallenge(1235); + startChallenge(); AddAssert("intro played flag reset", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.False); @@ -62,13 +62,12 @@ namespace osu.Game.Tests.Visual.DailyChallenge AddUntilStep("intro played flag set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.True); } - private void startChallenge(int roomId) + private void startChallenge() { AddStep("add room", () => { API.Perform(new CreateRoomRequest(room = new Room { - RoomID = roomId, Name = "Daily Challenge: June 4, 2024", Playlist = [ @@ -83,7 +82,7 @@ namespace osu.Game.Tests.Visual.DailyChallenge Category = RoomCategory.DailyChallenge })); }); - AddStep("signal client", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = roomId })); + AddStep("signal client", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = room.RoomID!.Value })); } } } diff --git a/osu.Game/Online/Rooms/Room.cs b/osu.Game/Online/Rooms/Room.cs index b93917eff6..e965f9c187 100644 --- a/osu.Game/Online/Rooms/Room.cs +++ b/osu.Game/Online/Rooms/Room.cs @@ -348,7 +348,7 @@ namespace osu.Game.Online.Rooms public Room(MultiplayerRoom room) { - RoomID = room.RoomID > 0 ? room.RoomID : null; + RoomID = room.RoomID; Name = room.Settings.Name; Password = room.Settings.Password; Type = room.Settings.MatchType; diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs index 46c1251d42..08f61f3ddc 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs @@ -267,7 +267,7 @@ namespace osu.Game.Tests.Visual.OnlinePlay /// The room host. public void AddServerSideRoom(Room room, APIUser host) { - room.RoomID ??= currentRoomId++; + room.RoomID = currentRoomId++; room.Host = host; for (int i = 0; i < room.Playlist.Count; i++)