Merge pull request #32649 from smoogipoo/refactor-mp-mod-select

Refactor multiplayer mod select to remove selected item bindable
2026-05-21 10:50:20 +08:00 · 2025-04-02 14:52:28 +02:00
parent 596b7038ba f120684b14
commit 18f5555865
4 changed files with 183 additions and 59 deletions
@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.

+using System;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
@@ -336,6 +337,61 @@ namespace osu.Game.Tests.Visual.Multiplayer
            AddAssert("button hidden", () => this.ChildrenOfType<MultiplayerRoomPanel>().Single().ChangeSettingsButton.Alpha, () => Is.EqualTo(0));
        }

+        [Test]
+        public void TestUserModSelectUpdatesWhenNotVisible()
+        {
+            AddStep("add playlist item", () =>
+            {
+                room.Playlist =
+                [
+                    new PlaylistItem(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo)
+                    {
+                        RulesetID = new OsuRuleset().RulesetInfo.OnlineID,
+                        AllowedMods = [new APIMod(new OsuModFlashlight())]
+                    }
+                ];
+            });
+
+            ClickButtonWhenEnabled<MultiplayerMatchSettingsOverlay.CreateOrUpdateButton>();
+            AddUntilStep("wait for join", () => RoomJoined);
+
+            // 1. Open the mod select overlay and enable flashlight
+
+            ClickButtonWhenEnabled<UserModSelectButton>();
+            AddUntilStep("mod select contents loaded", () => this.ChildrenOfType<ModColumn>().Any() && this.ChildrenOfType<ModColumn>().All(col => col.IsLoaded && col.ItemsLoaded));
+            AddStep("click flashlight panel", () =>
+            {
+                ModPanel panel = this.ChildrenOfType<ModPanel>().Single(p => p.Mod is OsuModFlashlight);
+                InputManager.MoveMouseTo(panel);
+                InputManager.Click(MouseButton.Left);
+            });
+            AddUntilStep("flashlight mod enabled", () => MultiplayerClient.ClientRoom!.Users[0].Mods.Any());
+
+            // 2. Close the mod select overlay, edit the playlist to disable allowed mods, and then edit it again to re-enable allowed mods.
+
+            AddStep("close mod select overlay", () => this.ChildrenOfType<MultiplayerUserModSelectOverlay>().Single().Hide());
+            AddUntilStep("mod select overlay not present", () => !this.ChildrenOfType<MultiplayerUserModSelectOverlay>().Single().IsPresent);
+            AddStep("disable allowed mods", () => MultiplayerClient.EditPlaylistItem(new MultiplayerPlaylistItem(new PlaylistItem(MultiplayerClient.ServerRoom!.Playlist[0])
+            {
+                AllowedMods = []
+            })));
+            // This would normally be done as part of the above operation with an actual server.
+            AddStep("disable user mods", () => MultiplayerClient.ChangeUserMods(API.LocalUser.Value.OnlineID, Array.Empty<APIMod>()));
+            AddUntilStep("flashlight mod disabled", () => !MultiplayerClient.ClientRoom!.Users[0].Mods.Any());
+            AddStep("re-enable allowed mods", () => MultiplayerClient.EditPlaylistItem(new MultiplayerPlaylistItem(new PlaylistItem(MultiplayerClient.ServerRoom!.Playlist[0])
+            {
+                AllowedMods = [new APIMod(new OsuModFlashlight())]
+            })));
+            AddAssert("flashlight mod still disabled", () => !MultiplayerClient.ClientRoom!.Users[0].Mods.Any());
+
+            // 3. Open the mod select overlay, check that the flashlight mod panel is deactivated.
+
+            ClickButtonWhenEnabled<UserModSelectButton>();
+            AddUntilStep("mod select contents loaded", () => this.ChildrenOfType<ModColumn>().Any() && this.ChildrenOfType<ModColumn>().All(col => col.IsLoaded && col.ItemsLoaded));
+            AddAssert("flashlight mod still disabled", () => !MultiplayerClient.ClientRoom!.Users[0].Mods.Any());
+            AddAssert("flashlight mod panel not activated", () => !this.ChildrenOfType<ModPanel>().Single(p => p.Mod is OsuModFlashlight).Active.Value);
+        }
+
        private partial class TestMultiplayerMatchSubScreen : MultiplayerMatchSubScreen
        {
            [Resolved(canBeNull: true)]
@@ -2,7 +2,6 @@
 // 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;
@@ -27,6 +26,7 @@ using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.Menu;
 using osu.Game.Screens.OnlinePlay.Match.Components;
 using osu.Game.Screens.OnlinePlay.Multiplayer;
+using osu.Game.Screens.OnlinePlay.Multiplayer.Match;
 using osu.Game.Utils;
 using Container = osu.Framework.Graphics.Containers.Container;

@@ -62,11 +62,6 @@ namespace osu.Game.Screens.OnlinePlay.Match

        private Sample? sampleStart;

-        /// <summary>
-        /// Any mods applied by/to the local user.
-        /// </summary>
-        protected readonly Bindable<IReadOnlyList<Mod>> UserMods = new Bindable<IReadOnlyList<Mod>>(Array.Empty<Mod>());
-
        [Resolved(CanBeNull = true)]
        private IOverlayManager? overlayManager { get; set; }

@@ -245,12 +240,7 @@ namespace osu.Game.Screens.OnlinePlay.Match
                }
            };

-            LoadComponent(UserModsSelectOverlay = new RoomModSelectOverlay
-            {
-                SelectedItem = { BindTarget = SelectedItem },
-                SelectedMods = { BindTarget = UserMods },
-                IsValidMod = _ => false
-            });
+            LoadComponent(UserModsSelectOverlay = new MultiplayerUserModSelectOverlay());
        }

        protected override void LoadComplete()
@@ -258,7 +248,6 @@ namespace osu.Game.Screens.OnlinePlay.Match
            base.LoadComplete();

            SelectedItem.BindValueChanged(_ => updateSpecifics());
-            UserMods.BindValueChanged(_ => updateSpecifics());

            beatmapAvailabilityTracker.Availability.BindValueChanged(_ => updateSpecifics());

@@ -441,11 +430,6 @@ namespace osu.Game.Screens.OnlinePlay.Match
                ? rulesetInstance.AllMods.OfType<Mod>().Where(m => ModUtils.IsValidFreeModForMatchType(m, Room.Type)).ToArray()
                : item.AllowedMods.Select(m => m.ToMod(rulesetInstance)).ToArray();

-            // Remove any user mods that are no longer allowed.
-            Mod[] newUserMods = UserMods.Value.Where(m => allowedMods.Any(a => m.GetType() == a.GetType())).ToArray();
-            if (!newUserMods.SequenceEqual(UserMods.Value))
-                UserMods.Value = newUserMods;
-
            // Retrieve the corresponding local beatmap, since we can't directly use the playlist's beatmap info
            int beatmapId = GetGameplayBeatmap().OnlineID;
            var localBeatmap = beatmapManager.QueryBeatmap($@"{nameof(BeatmapInfo.OnlineID)} == $0 AND {nameof(BeatmapInfo.MD5Hash)} == {nameof(BeatmapInfo.OnlineMD5Hash)}", beatmapId);
@@ -456,15 +440,11 @@ namespace osu.Game.Screens.OnlinePlay.Match
            Ruleset.Value = GetGameplayRuleset();

            if (allowedMods.Length > 0)
-            {
                UserModsSection.Show();
-                UserModsSelectOverlay.IsValidMod = m => allowedMods.Any(a => a.GetType() == m.GetType());
-            }
            else
            {
                UserModsSection.Hide();
                UserModsSelectOverlay.Hide();
-                UserModsSelectOverlay.IsValidMod = _ => false;
            }

            if (item.Freestyle)
@@ -488,7 +468,7 @@ namespace osu.Game.Screens.OnlinePlay.Match
                UserStyleSection.Hide();
        }

-        protected virtual APIMod[] GetGameplayMods() => UserMods.Value.Select(m => new APIMod(m)).Concat(SelectedItem.Value!.RequiredMods).ToArray();
+        protected virtual APIMod[] GetGameplayMods() => SelectedItem.Value!.RequiredMods;

        protected virtual RulesetInfo GetGameplayRuleset() => Rulesets.GetRuleset(SelectedItem.Value!.RulesetID)!;

@@ -0,0 +1,124 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Collections.Generic;
+using System.Linq;
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions.ObjectExtensions;
+using osu.Framework.Threading;
+using osu.Game.Configuration;
+using osu.Game.Online.Multiplayer;
+using osu.Game.Online.Rooms;
+using osu.Game.Overlays;
+using osu.Game.Overlays.Mods;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
+using osu.Game.Utils;
+
+namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match
+{
+    public partial class MultiplayerUserModSelectOverlay : UserModSelectOverlay
+    {
+        [Resolved]
+        private MultiplayerClient client { get; set; } = null!;
+
+        [Resolved]
+        private RulesetStore rulesets { get; set; } = null!;
+
+        private ModSettingChangeTracker? modSettingChangeTracker;
+        private ScheduledDelegate? debouncedModSettingsUpdate;
+
+        public MultiplayerUserModSelectOverlay()
+            : base(OverlayColourScheme.Plum)
+        {
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            client.RoomUpdated += onRoomUpdated;
+            SelectedMods.BindValueChanged(onSelectedModsChanged);
+
+            updateValidMods();
+        }
+
+        private void onRoomUpdated()
+        {
+            // Importantly, this is not scheduled because the client must not skip intermediate server states to validate the allowed mods.
+            updateValidMods();
+        }
+
+        private void onSelectedModsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods)
+        {
+            modSettingChangeTracker?.Dispose();
+
+            if (client.Room == null)
+                return;
+
+            client.ChangeUserMods(mods.NewValue).FireAndForget();
+
+            modSettingChangeTracker = new ModSettingChangeTracker(mods.NewValue);
+            modSettingChangeTracker.SettingChanged += _ =>
+            {
+                // Debounce changes to mod settings so as to not thrash the network.
+                debouncedModSettingsUpdate?.Cancel();
+                debouncedModSettingsUpdate = Scheduler.AddDelayed(() =>
+                {
+                    if (client.Room == null)
+                        return;
+
+                    client.ChangeUserMods(SelectedMods.Value).FireAndForget();
+                }, 500);
+            };
+        }
+
+        private void updateValidMods()
+        {
+            if (client.Room == null || client.LocalUser == null)
+                return;
+
+            MultiplayerPlaylistItem currentItem = client.Room.CurrentPlaylistItem;
+            Ruleset ruleset = rulesets.GetRuleset(client.LocalUser.RulesetId ?? currentItem.RulesetID)!.CreateInstance();
+            Mod[] allowedMods = currentItem.Freestyle
+                ? ruleset.AllMods.OfType<Mod>().Where(m => ModUtils.IsValidFreeModForMatchType(m, client.Room.Settings.MatchType)).ToArray()
+                : currentItem.AllowedMods.Select(m => m.ToMod(ruleset)).ToArray();
+
+            // Update the mod panels to reflect the ones which are valid for selection.
+            IsValidMod = allowedMods.Length > 0
+                ? m => allowedMods.Any(a => a.GetType() == m.GetType())
+                : _ => false;
+
+            // Remove any mods that are no longer allowed.
+            Mod[] newUserMods = SelectedMods.Value.Where(m => allowedMods.Any(a => m.GetType() == a.GetType())).ToArray();
+            if (!newUserMods.SequenceEqual(SelectedMods.Value))
+                SelectedMods.Value = newUserMods;
+
+            // The active mods include the playlist item's required mods which change separately from the selected mods.
+            IReadOnlyList<Mod> newActiveMods = ComputeActiveMods();
+            if (!newActiveMods.SequenceEqual(ActiveMods.Value))
+                ActiveMods.Value = newActiveMods;
+        }
+
+        protected override IReadOnlyList<Mod> ComputeActiveMods()
+        {
+            if (client.Room == null || client.LocalUser == null)
+                return [];
+
+            MultiplayerPlaylistItem currentItem = client.Room.CurrentPlaylistItem;
+            Ruleset ruleset = rulesets.GetRuleset(client.LocalUser.RulesetId ?? currentItem.RulesetID)!.CreateInstance();
+            return currentItem.RequiredMods.Select(m => m.ToMod(ruleset)).Concat(base.ComputeActiveMods()).ToArray();
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (client.IsNotNull())
+                client.RoomUpdated -= onRoomUpdated;
+
+            modSettingChangeTracker?.Dispose();
+        }
+    }
+}
@@ -1,7 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.

-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using osu.Framework.Allocation;
@@ -11,9 +10,7 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Logging;
 using osu.Framework.Screens;
-using osu.Framework.Threading;
 using osu.Game.Beatmaps;
-using osu.Game.Configuration;
 using osu.Game.Graphics.Cursor;
 using osu.Game.Online;
 using osu.Game.Online.API;
@@ -23,7 +20,6 @@ using osu.Game.Online.Rooms;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Dialog;
 using osu.Game.Rulesets;
-using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.OnlinePlay.Components;
 using osu.Game.Screens.OnlinePlay.Match;
 using osu.Game.Screens.OnlinePlay.Match.Components;
@@ -64,7 +60,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
            base.LoadComplete();

            BeatmapAvailability.BindValueChanged(updateBeatmapAvailability, true);
-            UserMods.BindValueChanged(onUserModsChanged);

            client.LoadRequested += onLoadRequested;
            client.RoomUpdated += onRoomUpdated;
@@ -306,35 +301,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer

        protected override void PartRoom() => client.LeaveRoom();

-        private ModSettingChangeTracker? modSettingChangeTracker;
-        private ScheduledDelegate? debouncedModSettingsUpdate;
-
-        private void onUserModsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods)
-        {
-            modSettingChangeTracker?.Dispose();
-
-            if (client.Room == null)
-                return;
-
-            client.ChangeUserMods(mods.NewValue).FireAndForget();
-
-            modSettingChangeTracker = new ModSettingChangeTracker(mods.NewValue);
-            modSettingChangeTracker.SettingChanged += onModSettingsChanged;
-        }
-
-        private void onModSettingsChanged(Mod mod)
-        {
-            // Debounce changes to mod settings so as to not thrash the network.
-            debouncedModSettingsUpdate?.Cancel();
-            debouncedModSettingsUpdate = Scheduler.AddDelayed(() =>
-            {
-                if (client.Room == null)
-                    return;
-
-                client.ChangeUserMods(UserMods.Value).FireAndForget();
-            }, 500);
-        }
-
        private void updateBeatmapAvailability(ValueChangedEvent<BeatmapAvailability> availability)
        {
            if (client.Room == null)
@@ -462,8 +428,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
                client.RoomUpdated -= onRoomUpdated;
                client.LoadRequested -= onLoadRequested;
            }
-
-            modSettingChangeTracker?.Dispose();
        }

        public partial class AddItemButton : PurpleRoundedButton