From 1b07b6d16f49fd06572c3366685a08f2a2641669 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 12 Feb 2025 21:48:59 +0900 Subject: [PATCH] Remove selected room leasing, make bindables private I believe once upon a time the `SelectedRoom` bindable used to be bound to `RoomManager.JoinedRoom` or similar. But now it's effectively private to the lounge subscreen and so a lease is unnecessary. --- .../TestScenePlaylistsLoungeSubScreen.cs | 28 +------------ .../OnlinePlay/Lounge/LoungeSubScreen.cs | 39 ++++++------------- 2 files changed, 13 insertions(+), 54 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs index 94a81ecdc7..35bf6dc28a 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs @@ -3,7 +3,6 @@ using System.Linq; using NUnit.Framework; -using osu.Framework.Bindables; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Graphics.Containers; @@ -17,13 +16,13 @@ namespace osu.Game.Tests.Visual.Playlists { public partial class TestScenePlaylistsLoungeSubScreen : OnlinePlayTestScene { - private TestLoungeSubScreen loungeScreen = null!; + private PlaylistsLoungeSubScreen loungeScreen = null!; public override void SetUpSteps() { base.SetUpSteps(); - AddStep("push screen", () => LoadScreen(loungeScreen = new TestLoungeSubScreen())); + AddStep("push screen", () => LoadScreen(loungeScreen = new PlaylistsLoungeSubScreen())); AddUntilStep("wait for present", () => loungeScreen.IsCurrentScreen()); } @@ -62,24 +61,6 @@ namespace osu.Game.Tests.Visual.Playlists AddUntilStep("last room is not masked", () => checkRoomVisible(roomsContainer.DrawableRooms[^1])); } - [Test] - public void TestEnteringRoomTakesLeaseOnSelection() - { - createRooms(GenerateRooms(1)); - - AddAssert("selected room is not disabled", () => !loungeScreen.SelectedRoom.Disabled); - - AddStep("select room", () => roomsContainer.DrawableRooms[0].TriggerClick()); - AddAssert("selected room is non-null", () => loungeScreen.SelectedRoom.Value != null); - - AddStep("enter room", () => roomsContainer.DrawableRooms[0].TriggerClick()); - - AddUntilStep("wait for match load", () => Stack.CurrentScreen is PlaylistsRoomSubScreen); - - AddAssert("selected room is non-null", () => loungeScreen.SelectedRoom.Value != null); - AddAssert("selected room is disabled", () => loungeScreen.SelectedRoom.Disabled); - } - private bool checkRoomVisible(DrawableRoom room) => loungeScreen.ChildrenOfType().First().ScreenSpaceDrawQuad .Contains(room.ScreenSpaceDrawQuad.Centre); @@ -94,10 +75,5 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("refresh lounge", () => loungeScreen.RefreshRooms()); } - - private partial class TestLoungeSubScreen : PlaylistsLoungeSubScreen - { - public new Bindable SelectedRoom => base.SelectedRoom; - } } } diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index 6c383f1bf6..7bb0c67990 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -41,7 +41,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge protected override BackgroundScreen CreateBackground() => new LoungeBackgroundScreen { - SelectedRoom = { BindTarget = SelectedRoom } + SelectedRoom = { BindTarget = selectedRoom } }; protected override UserActivity InitialActivity => new UserActivity.SearchingForLobby(); @@ -53,9 +53,6 @@ namespace osu.Game.Screens.OnlinePlay.Lounge AutoSizeAxes = Axes.Both }; - protected readonly Bindable SelectedRoom = new Bindable(); - protected readonly BindableList Rooms = new BindableList(); - [Resolved] private MusicController music { get; set; } = null!; @@ -75,8 +72,9 @@ namespace osu.Game.Screens.OnlinePlay.Lounge protected OsuConfigManager Config { get; private set; } = null!; private IDisposable? joiningRoomOperation; - private LeasedBindable? selectionLease; + private readonly Bindable selectedRoom = new Bindable(); + private readonly BindableList rooms = new BindableList(); private readonly Bindable filter = new Bindable(); private readonly Bindable hasListingResults = new Bindable(); private readonly IBindable operationInProgress = new Bindable(); @@ -121,8 +119,8 @@ namespace osu.Game.Screens.OnlinePlay.Lounge ScrollbarOverlapsContent = false, Child = roomsContainer = new RoomsContainer { - Rooms = { BindTarget = Rooms }, - SelectedRoom = { BindTarget = SelectedRoom }, + Rooms = { BindTarget = rooms }, + SelectedRoom = { BindTarget = selectedRoom }, Filter = { BindTarget = filter }, } }, @@ -182,7 +180,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge }; // scroll selected room into view on selection. - SelectedRoom.BindValueChanged(val => + selectedRoom.BindValueChanged(val => { var drawable = roomsContainer.DrawableRooms.FirstOrDefault(r => r.Room == val.NewValue); if (drawable != null) @@ -208,7 +206,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge filter.BindValueChanged(_ => { - Rooms.Clear(); + rooms.Clear(); hasListingResults.Value = false; listingPollingComponent.PollImmediately(); }); @@ -218,11 +216,11 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private void onListingReceived(Room[] result) { - Dictionary localRoomsById = Rooms.ToDictionary(r => r.RoomID!.Value); + Dictionary localRoomsById = rooms.ToDictionary(r => r.RoomID!.Value); Dictionary resultRoomsById = result.ToDictionary(r => r.RoomID!.Value); // Remove all local rooms no longer in the result set. - Rooms.RemoveAll(r => !resultRoomsById.ContainsKey(r.RoomID!.Value)); + rooms.RemoveAll(r => !resultRoomsById.ContainsKey(r.RoomID!.Value)); // Add or update local rooms with the result set. foreach (var r in result) @@ -230,7 +228,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge if (localRoomsById.TryGetValue(r.RoomID!.Value, out Room? existingRoom)) existingRoom.CopyFrom(r); else - Rooms.Add(r); + rooms.Add(r); } hasListingResults.Value = true; @@ -286,14 +284,6 @@ namespace osu.Game.Screens.OnlinePlay.Lounge { base.OnResuming(e); - Debug.Assert(selectionLease != null); - - selectionLease.Return(); - selectionLease = null; - - if (SelectedRoom.Value?.RoomID == null) - SelectedRoom.Value = new Room(); - music.EnsurePlayingSomething(); onReturning(); @@ -415,14 +405,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge OpenNewRoom(room ?? CreateNewRoom()); }); - protected virtual void OpenNewRoom(Room room) - { - selectionLease = SelectedRoom.BeginLease(false); - Debug.Assert(selectionLease != null); - selectionLease.Value = room; - - this.Push(CreateRoomSubScreen(room)); - } + protected virtual void OpenNewRoom(Room room) => this.Push(CreateRoomSubScreen(room)); public void RefreshRooms() => listingPollingComponent.PollImmediately();