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.Tests/Visual/Gameplay/TestSceneSkinEditor.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs index 91188f5bac..97889eea4d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs @@ -15,6 +15,7 @@ using osu.Framework.Graphics.UserInterface; using osu.Framework.Input; using osu.Framework.Testing; using osu.Game.Database; +using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osu.Game.Overlays.Settings; using osu.Game.Overlays.SkinEditor; @@ -458,6 +459,62 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType().Count() == 1); } + [Test] + public void TestAnchorRadioButtonBehavior() + { + ISerialisableDrawable? selectedComponent = null; + + AddStep("Select first component", () => + { + var blueprint = skinEditor.ChildrenOfType().First(); + skinEditor.SelectedComponents.Clear(); + skinEditor.SelectedComponents.Add(blueprint.Item); + selectedComponent = blueprint.Item; + }); + + AddStep("Right-click to open context menu", () => + { + if (selectedComponent != null) + InputManager.MoveMouseTo(((Drawable)selectedComponent).ScreenSpaceDrawQuad.Centre); + InputManager.Click(MouseButton.Right); + }); + + AddStep("Click on Anchor menu", () => + { + InputManager.MoveMouseTo(getMenuItemByText("Anchor")); + InputManager.Click(MouseButton.Left); + }); + + AddStep("Right-click TopLeft anchor", () => + { + InputManager.MoveMouseTo(getMenuItemByText("TopLeft")); + InputManager.Click(MouseButton.Right); + }); + + AddAssert("TopLeft item checked", () => (getMenuItemByText("TopLeft").Item as TernaryStateRadioMenuItem)?.State.Value == TernaryState.True); + + AddStep("Right-click Centre anchor", () => + { + InputManager.MoveMouseTo(getMenuItemByText("Centre")); + InputManager.Click(MouseButton.Right); + }); + + AddAssert("Centre item checked", () => (getMenuItemByText("Centre").Item as TernaryStateRadioMenuItem)?.State.Value == TernaryState.True); + AddAssert("TopLeft item unchecked", () => (getMenuItemByText("TopLeft").Item as TernaryStateRadioMenuItem)?.State.Value == TernaryState.False); + + AddStep("Right-click Closest anchor", () => + { + InputManager.MoveMouseTo(getMenuItemByText("Closest")); + InputManager.Click(MouseButton.Right); + }); + + AddAssert("Closest item checked", () => (getMenuItemByText("Closest").Item as TernaryStateRadioMenuItem)?.State.Value == TernaryState.True); + AddAssert("Centre item unchecked", () => (getMenuItemByText("Centre").Item as TernaryStateRadioMenuItem)?.State.Value == TernaryState.False); + + Menu.DrawableMenuItem getMenuItemByText(string text) + => this.ChildrenOfType().First(m => m.Item.Text.ToString() == text); + } + private Skin importSkinFromArchives(string filename) { var imported = skins.Import(new ImportTask(TestResources.OpenResource($@"Archives/{filename}"), filename)).GetResultSafely(); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs index 1affa08813..c6a203c77a 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; @@ -32,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) @@ -47,19 +45,17 @@ 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", () => { - 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) }; }); @@ -158,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] 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; diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneUserTagControl.cs b/osu.Game.Tests/Visual/Ranking/TestSceneUserTagControl.cs index cfedd89b12..958eacfd56 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneUserTagControl.cs +++ b/osu.Game.Tests/Visual/Ranking/TestSceneUserTagControl.cs @@ -8,6 +8,8 @@ using osu.Game.Beatmaps; using osu.Game.Online.API; using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Taiko; using osu.Game.Screens.Ranking; namespace osu.Game.Tests.Visual.Ranking @@ -19,10 +21,6 @@ namespace osu.Game.Tests.Visual.Ranking [SetUpSteps] public void SetUpSteps() { - AddStep("set up working beatmap", () => - { - Beatmap.Value.BeatmapInfo.OnlineID = 42; - }); AddStep("set up network requests", () => { dummyAPI.HandleRequest = request => @@ -40,6 +38,7 @@ namespace osu.Game.Tests.Visual.Ranking new APITag { Id = 2, Name = "style/clean", Description = "Visually uncluttered and organised patterns, often involving few overlaps and equal visual spacing between objects.", }, new APITag { Id = 3, Name = "aim/aim control", Description = "Patterns with velocity or direction changes which strongly go against a player's natural movement pattern.", }, new APITag { Id = 4, Name = "tap/bursts", Description = "Patterns requiring continuous movement and alternating, typically 9 notes or less.", }, + new APITag { Id = 5, Name = "style/mono-heavy", Description = "Features monos used in large amounts.", RulesetId = 1, }, ] }), 500); return true; @@ -67,15 +66,30 @@ namespace osu.Game.Tests.Visual.Ranking return false; }; }); - AddStep("create control", () => + AddStep("show for osu! beatmap", () => { - Child = new UserTagControl(Beatmap.Value.BeatmapInfo) - { - Width = 700, - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - }; + var working = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo); + working.BeatmapInfo.OnlineID = 42; + Beatmap.Value = working; + recreateControl(); }); + AddStep("show for taiko beatmap", () => + { + var working = CreateWorkingBeatmap(new TaikoRuleset().RulesetInfo); + working.BeatmapInfo.OnlineID = 44; + Beatmap.Value = working; + recreateControl(); + }); + } + + private void recreateControl() + { + Child = new UserTagControl(Beatmap.Value.BeatmapInfo) + { + Width = 700, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }; } } } diff --git a/osu.Game/Online/API/Requests/Responses/APITag.cs b/osu.Game/Online/API/Requests/Responses/APITag.cs index 4dd18663af..b0454fdb1d 100644 --- a/osu.Game/Online/API/Requests/Responses/APITag.cs +++ b/osu.Game/Online/API/Requests/Responses/APITag.cs @@ -15,5 +15,8 @@ namespace osu.Game.Online.API.Requests.Responses [JsonProperty("description")] public string Description { get; set; } = string.Empty; + + [JsonProperty("ruleset_id")] + public int? RulesetId { get; set; } } } diff --git a/osu.Game/Overlays/SkinEditor/SkinEditor.cs b/osu.Game/Overlays/SkinEditor/SkinEditor.cs index 534abd1ab3..c1c64cac1f 100644 --- a/osu.Game/Overlays/SkinEditor/SkinEditor.cs +++ b/osu.Game/Overlays/SkinEditor/SkinEditor.cs @@ -760,11 +760,7 @@ namespace osu.Game.Overlays.SkinEditor #region Delegation of IEditorChangeHandler - public event Action? OnStateChange - { - add => throw new NotImplementedException(); - remove => throw new NotImplementedException(); - } + public event Action? OnStateChange; private IEditorChangeHandler? beginChangeHandler; @@ -773,6 +769,9 @@ namespace osu.Game.Overlays.SkinEditor // Change handler may change between begin and end, which can cause unbalanced operations. // Let's track the one that was used when beginning the change so we can call EndChange on it specifically. (beginChangeHandler = changeHandler)?.BeginChange(); + + if (beginChangeHandler != null) + beginChangeHandler.OnStateChange += OnStateChange; } public void EndChange() => beginChangeHandler?.EndChange(); diff --git a/osu.Game/Overlays/SkinEditor/SkinSelectionHandler.cs b/osu.Game/Overlays/SkinEditor/SkinSelectionHandler.cs index bc878b9214..838b5ff2f0 100644 --- a/osu.Game/Overlays/SkinEditor/SkinSelectionHandler.cs +++ b/osu.Game/Overlays/SkinEditor/SkinSelectionHandler.cs @@ -22,7 +22,10 @@ namespace osu.Game.Overlays.SkinEditor { public partial class SkinSelectionHandler : SelectionHandler { - private OsuMenuItem originMenu = null!; + private OsuMenuItem? originMenu; + + private TernaryStateRadioMenuItem? closestAnchor; + private AnchorMenuItem[]? fixedAnchors; [Resolved] private SkinEditor skinEditor { get; set; } = null!; @@ -44,6 +47,38 @@ namespace osu.Game.Overlays.SkinEditor return scaleHandler; } + protected override void LoadComplete() + { + base.LoadComplete(); + + if (ChangeHandler != null) + ChangeHandler.OnStateChange += updateTernaryStates; + SelectedItems.BindCollectionChanged((_, _) => updateTernaryStates()); + } + + private void updateTernaryStates() + { + var usingClosestAnchor = GetStateFromSelection(SelectedBlueprints, c => !c.Item.UsesFixedAnchor); + + if (closestAnchor != null) + closestAnchor.State.Value = usingClosestAnchor; + + if (fixedAnchors != null) + { + foreach (var fixedAnchor in fixedAnchors) + fixedAnchor.State.Value = GetStateFromSelection(SelectedBlueprints, c => c.Item.UsesFixedAnchor && ((Drawable)c.Item).Anchor == fixedAnchor.Anchor); + } + + if (originMenu != null) + { + foreach (var origin in originMenu.Items.OfType()) + { + origin.State.Value = GetStateFromSelection(SelectedBlueprints, c => ((Drawable)c.Item).Origin == origin.Anchor); + origin.Action.Disabled = usingClosestAnchor == TernaryState.True; + } + } + } + public override bool HandleFlip(Direction direction, bool flipOverOrigin) { var selectionQuad = getSelectionQuad(); @@ -102,27 +137,17 @@ namespace osu.Game.Overlays.SkinEditor protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { - var closestItem = new TernaryStateRadioMenuItem(SkinEditorStrings.Closest, MenuItemType.Standard, _ => applyClosestAnchors()) - { - State = { Value = GetStateFromSelection(selection, c => !c.Item.UsesFixedAnchor) } - }; + closestAnchor = new TernaryStateRadioMenuItem(SkinEditorStrings.Closest, MenuItemType.Standard, _ => applyClosestAnchors()); + fixedAnchors = createAnchorItems(applyFixedAnchors).ToArray(); yield return new OsuMenuItem(SkinEditorStrings.Anchor) { - Items = createAnchorItems((d, a) => d.UsesFixedAnchor && ((Drawable)d).Anchor == a, applyFixedAnchors) - .Prepend(closestItem) - .ToArray() + Items = fixedAnchors.Prepend(closestAnchor).ToArray() }; yield return originMenu = new OsuMenuItem(SkinEditorStrings.Origin); - closestItem.State.BindValueChanged(s => - { - // For UX simplicity, origin should only be user-editable when "closest" anchor mode is disabled. - originMenu.Items = s.NewValue == TernaryState.True - ? Array.Empty() - : createAnchorItems((d, o) => ((Drawable)d).Origin == o, applyOrigins).ToArray(); - }, true); + originMenu.Items = createAnchorItems(applyOrigins).ToArray(); yield return new OsuMenuItemSpacer(); @@ -163,27 +188,37 @@ namespace osu.Game.Overlays.SkinEditor foreach (var item in base.GetContextMenuItemsForSelection(selection)) yield return item; - IEnumerable createAnchorItems(Func checkFunction, Action applyFunction) + updateTernaryStates(); + } + + private IEnumerable createAnchorItems(Action applyFunction) + { + var displayableAnchors = new[] { - var displayableAnchors = new[] - { - Anchor.TopLeft, - Anchor.TopCentre, - Anchor.TopRight, - Anchor.CentreLeft, - Anchor.Centre, - Anchor.CentreRight, - Anchor.BottomLeft, - Anchor.BottomCentre, - Anchor.BottomRight, - }; - return displayableAnchors.Select(a => - { - return new TernaryStateRadioMenuItem(a.ToString(), MenuItemType.Standard, _ => applyFunction(a)) - { - State = { Value = GetStateFromSelection(selection, c => checkFunction(c.Item, a)) } - }; - }); + Anchor.TopLeft, + Anchor.TopCentre, + Anchor.TopRight, + Anchor.CentreLeft, + Anchor.Centre, + Anchor.CentreRight, + Anchor.BottomLeft, + Anchor.BottomCentre, + Anchor.BottomRight, + }; + return displayableAnchors.Select(a => + { + return new AnchorMenuItem(a, _ => applyFunction(a)); + }); + } + + private partial class AnchorMenuItem : TernaryStateRadioMenuItem + { + public readonly Anchor Anchor; + + public AnchorMenuItem(Anchor anchor, Action applyFunction) + : base(anchor.ToString(), MenuItemType.Standard, _ => applyFunction(anchor)) + { + Anchor = anchor; } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerPlaylist.cs index 9feee0ae41..fba3acc32a 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.Multiplayer; @@ -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,28 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist firstPopulation = false; } + + PlaylistItem? currentItem = client.Room == null ? null : new PlaylistItem(client.Room.CurrentPlaylistItem); + queueList.SelectedItem.Value = currentItem; + historyList.SelectedItem.Value = currentItem; } - 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 +142,35 @@ 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); + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (client.IsNotNull()) + { + client.ItemAdded -= playlistItemAdded; + client.ItemRemoved -= playlistItemRemoved; + client.ItemChanged -= playlistItemChanged; + client.RoomUpdated -= onRoomUpdated; + } } } } 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[] diff --git a/osu.Game/Screens/Ranking/UserTagControl.cs b/osu.Game/Screens/Ranking/UserTagControl.cs index 289b7b3ecd..7b36077bb3 100644 --- a/osu.Game/Screens/Ranking/UserTagControl.cs +++ b/osu.Game/Screens/Ranking/UserTagControl.cs @@ -49,7 +49,7 @@ namespace osu.Game.Screens.Ranking private BindableList displayedTags { get; } = new BindableList(); private Bindable apiTags = null!; - private BindableDictionary allTagsById { get; } = new BindableDictionary(); + private BindableDictionary relevantTagsById { get; } = new BindableDictionary(); private readonly Bindable apiBeatmap = new Bindable(); @@ -106,7 +106,7 @@ namespace osu.Game.Screens.Ranking }, new TagList { - AvailableTags = { BindTarget = allTagsById }, + AvailableTags = { BindTarget = relevantTagsById }, OnSelected = toggleVote, } } @@ -149,12 +149,14 @@ namespace osu.Game.Screens.Ranking if (apiTags.Value == null || apiBeatmap.Value == null) return; - allTagsById.Clear(); - allTagsById.AddRange(apiTags.Value.Select(t => new KeyValuePair(t.Id, new UserTag(t)))); + relevantTagsById.Clear(); + relevantTagsById.AddRange(apiTags.Value + .Where(t => t.RulesetId == null || t.RulesetId == beatmapInfo.Ruleset.OnlineID) + .Select(t => new KeyValuePair(t.Id, new UserTag(t)))); foreach (var topTag in apiBeatmap.Value.TopTags ?? []) { - if (allTagsById.TryGetValue(topTag.TagId, out var tag)) + if (relevantTagsById.TryGetValue(topTag.TagId, out var tag)) { tag.VoteCount.Value = topTag.VoteCount; displayedTags.Add(tag); @@ -163,7 +165,7 @@ namespace osu.Game.Screens.Ranking foreach (long ownTagId in apiBeatmap.Value.OwnTagIds ?? []) { - if (allTagsById.TryGetValue(ownTagId, out var tag)) + if (relevantTagsById.TryGetValue(ownTagId, out var tag)) tag.Voted.Value = true; } 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++)