General cleanup

osu.Game.Tests/Mods/ModValidationTest.cs

@@ -15,7 +15,7 @@ namespace osu.Game.Tests.Mods
         public void TestModIsCompatibleByItself()
         {
             var mod = new Mock<Mod>();
-            Assert.That(ModValidation.CheckCompatible(new[] { mod.Object }));
+            Assert.That(ModUtils.CheckCompatibleSet(new[] { mod.Object }));
         }
 
         [Test]
@@ -27,8 +27,8 @@ namespace osu.Game.Tests.Mods
             mod1.Setup(m => m.IncompatibleMods).Returns(new[] { mod2.Object.GetType() });
 
             // Test both orderings.
-            Assert.That(ModValidation.CheckCompatible(new[] { mod1.Object, mod2.Object }), Is.False);
+            Assert.That(ModUtils.CheckCompatibleSet(new[] { mod1.Object, mod2.Object }), Is.False);
-            Assert.That(ModValidation.CheckCompatible(new[] { mod2.Object, mod1.Object }), Is.False);
+            Assert.That(ModUtils.CheckCompatibleSet(new[] { mod2.Object, mod1.Object }), Is.False);
         }
 
         [Test]
@@ -43,22 +43,22 @@ namespace osu.Game.Tests.Mods
             var multiMod = new MultiMod(new MultiMod(mod2.Object));
 
             // Test both orderings.
-            Assert.That(ModValidation.CheckCompatible(new[] { multiMod, mod1.Object }), Is.False);
+            Assert.That(ModUtils.CheckCompatibleSet(new[] { multiMod, mod1.Object }), Is.False);
-            Assert.That(ModValidation.CheckCompatible(new[] { mod1.Object, multiMod }), Is.False);
+            Assert.That(ModUtils.CheckCompatibleSet(new[] { mod1.Object, multiMod }), Is.False);
         }
 
         [Test]
         public void TestAllowedThroughMostDerivedType()
         {
             var mod = new Mock<Mod>();
-            Assert.That(ModValidation.CheckAllowed(new[] { mod.Object }, new[] { mod.Object.GetType() }));
+            Assert.That(ModUtils.CheckAllowed(new[] { mod.Object }, new[] { mod.Object.GetType() }));
         }
 
         [Test]
         public void TestNotAllowedThroughBaseType()
         {
             var mod = new Mock<Mod>();
-            Assert.That(ModValidation.CheckAllowed(new[] { mod.Object }, new[] { typeof(Mod) }), Is.False);
+            Assert.That(ModUtils.CheckAllowed(new[] { mod.Object }, new[] { typeof(Mod) }), Is.False);
         }
     }
 }

osu.Game.Tests/Visual/Multiplayer/TestSceneFreeModSelectOverlay.cs

@@ -3,19 +3,16 @@
 
 using NUnit.Framework;
 using osu.Framework.Graphics.Containers;
-using osu.Game.Overlays.Mods;
 using osu.Game.Screens.OnlinePlay.Match;
 
 namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneFreeModSelectOverlay : MultiplayerTestScene
     {
-        private ModSelectOverlay overlay;
-
         [SetUp]
         public new void Setup() => Schedule(() =>
         {
-            Child = overlay = new FreeModSelectOverlay
+            Child = new FreeModSelectOverlay
             {
                 State = { Value = Visibility.Visible }
             };

osu.Game/Online/Multiplayer/IMultiplayerClient.cs

@@ -57,6 +57,11 @@ namespace osu.Game.Online.Multiplayer
         /// <param name="beatmapAvailability">The new beatmap availability state of the user.</param>
         Task UserBeatmapAvailabilityChanged(int userId, BeatmapAvailability beatmapAvailability);
 
+        /// <summary>
+        /// Signals that a user in this room changed their local mods.
+        /// </summary>
+        /// <param name="userId">The ID of the user whose mods have changed.</param>
+        /// <param name="mods">The user's new local mods.</param>
         Task UserModsChanged(int userId, IEnumerable<APIMod> mods);
 
         /// <summary>

osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs

@@ -49,6 +49,10 @@ namespace osu.Game.Online.Multiplayer
         /// <param name="newBeatmapAvailability">The proposed new beatmap availability state.</param>
         Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability);
 
+        /// <summary>
+        /// Change the local user's mods in the currently joined room.
+        /// </summary>
+        /// <param name="newMods">The proposed new mods, excluding any required by the room itself.</param>
         Task ChangeUserMods(IEnumerable<APIMod> newMods);
 
         /// <summary>

osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs

@@ -26,6 +26,9 @@ namespace osu.Game.Online.Multiplayer
         /// </summary>
         public BeatmapAvailability BeatmapAvailability { get; set; } = BeatmapAvailability.LocallyAvailable();
 
+        /// <summary>
+        /// Any mods applicable only to the local user.
+        /// </summary>
         [NotNull]
         public IEnumerable<APIMod> UserMods { get; set; } = Enumerable.Empty<APIMod>();
 

osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs

@@ -232,6 +232,10 @@ namespace osu.Game.Online.Multiplayer
 
         public abstract Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability);
 
+        /// <summary>
+        /// Change the local user's mods in the currently joined room.
+        /// </summary>
+        /// <param name="newMods">The proposed new mods, excluding any required by the room itself.</param>
         public Task ChangeUserMods(IEnumerable<Mod> newMods) => ChangeUserMods(newMods.Select(m => new APIMod(m)).ToList());
 
         public abstract Task ChangeUserMods(IEnumerable<APIMod> newMods);
@@ -393,7 +397,7 @@ namespace osu.Game.Online.Multiplayer
             {
                 var user = Room?.Users.SingleOrDefault(u => u.UserID == userId);
 
-                // errors here are not critical - user mods is mostly for display.
+                // errors here are not critical - user mods are mostly for display.
                 if (user == null)
                     return;
 

osu.Game/Overlays/Mods/ModButton.cs

@@ -174,7 +174,7 @@ namespace osu.Game.Overlays.Mods
                 switch (e.Button)
                 {
                     case MouseButton.Right:
-                        OnRightClick(e);
+                        SelectNext(-1);
                         break;
                 }
             }
@@ -183,12 +183,8 @@ namespace osu.Game.Overlays.Mods
         protected override bool OnClick(ClickEvent e)
         {
             SelectNext(1);
-            return true;
-        }
 
-        protected virtual void OnRightClick(MouseUpEvent e)
+            return true;
-        {
-            SelectNext(-1);
         }
 
         /// <summary>

osu.Game/Overlays/Mods/ModSelectOverlay.cs

@@ -61,7 +61,7 @@ namespace osu.Game.Overlays.Mods
         private Func<Mod, bool> isValidMod = m => true;
 
         /// <summary>
-        /// A function that checks whether a given mod is valid.
+        /// A function that checks whether a given mod is selectable.
         /// </summary>
         [NotNull]
         public Func<Mod, bool> IsValidMod
@@ -442,12 +442,20 @@ namespace osu.Game.Overlays.Mods
                 IEnumerable<Mod> modEnumeration = availableMods.Value[section.ModType];
 
                 if (!Stacked)
-                    modEnumeration = ModValidation.FlattenMods(modEnumeration);
+                    modEnumeration = ModUtils.FlattenMods(modEnumeration);
 
                 section.Mods = modEnumeration.Select(validModOrNull).Where(m => m != null);
             }
         }
 
+        /// <summary>
+        /// Returns a valid form of a given <see cref="Mod"/> if possible, or null otherwise.
+        /// </summary>
+        /// <remarks>
+        /// This is a recursive process during which any invalid mods are culled while preserving <see cref="MultiMod"/> structures where possible.
+        /// </remarks>
+        /// <param name="mod">The <see cref="Mod"/> to check.</param>
+        /// <returns>A valid form of <paramref name="mod"/> if exists, or null otherwise.</returns>
         [CanBeNull]
         private Mod validModOrNull([NotNull] Mod mod)
         {

osu.Game/Screens/OnlinePlay/Match/FreeModSelectOverlay.cs

@@ -5,13 +5,15 @@ using System;
 using System.Linq;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Input.Events;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Overlays.Mods;
 using osu.Game.Rulesets.Mods;
 
 namespace osu.Game.Screens.OnlinePlay.Match
 {
+    /// <summary>
+    /// A <see cref="ModSelectOverlay"/> used for free-mod selection in online play.
+    /// </summary>
     public class FreeModSelectOverlay : ModSelectOverlay
     {
         protected override bool AllowCustomisation => false;
@@ -29,8 +31,6 @@ namespace osu.Game.Screens.OnlinePlay.Match
             {
             }
 
-            protected override ModButton CreateModButton(Mod mod) => new FreeModButton(mod);
-
             protected override Drawable CreateHeader(string text) => new Container
             {
                 AutoSizeAxes = Axes.Y,
@@ -47,7 +47,6 @@ namespace osu.Game.Screens.OnlinePlay.Match
                 foreach (var button in ButtonsContainer.OfType<ModButton>())
                 {
                     if (value)
-                        // Note: Buttons where only part of the group has an implementation are not fully supported.
                         button.SelectAt(0);
                     else
                         button.Deselect();
@@ -77,29 +76,5 @@ namespace osu.Game.Screens.OnlinePlay.Match
                 Changed?.Invoke(value);
             }
         }
-
-        private class FreeModButton : ModButton
-        {
-            public FreeModButton(Mod mod)
-                : base(mod)
-            {
-            }
-
-            protected override bool OnClick(ClickEvent e)
-            {
-                onClick();
-                return true;
-            }
-
-            protected override void OnRightClick(MouseUpEvent e) => onClick();
-
-            private void onClick()
-            {
-                if (Selected)
-                    Deselect();
-                else
-                    SelectNext(1);
-            }
-        }
     }
 }

osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs

@@ -107,7 +107,7 @@ namespace osu.Game.Screens.OnlinePlay.Match
             if (SelectedItem.Value == null)
                 return;
 
-            // Remove any extra mods that are no longer allowed.
+            // Remove any user mods that are no longer allowed.
             UserMods.Value = UserMods.Value
                                      .Where(m => SelectedItem.Value.AllowedMods.Any(a => m.GetType() == a.GetType()))
                                      .ToList();

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSongSelect.cs

@@ -34,7 +34,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             Mods.Value = Playlist.FirstOrDefault()?.RequiredMods.Select(m => m.CreateCopy()).ToArray() ?? Array.Empty<Mod>();
         }
 
-        protected override void OnSetItem(PlaylistItem item)
+        protected override void SelectItem(PlaylistItem item)
         {
             // If the client is already in a room, update via the client.
             // Otherwise, update the playlist directly in preparation for it to be submitted to the API on match creation.

osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs

@@ -76,11 +76,15 @@ namespace osu.Game.Screens.OnlinePlay
             item.AllowedMods.Clear();
             item.AllowedMods.AddRange(freeMods.Value.Select(m => m.CreateCopy()));
 
-            OnSetItem(item);
+            SelectItem(item);
             return true;
         }
 
-        protected abstract void OnSetItem(PlaylistItem item);
+        /// <summary>
+        /// Invoked when the user has requested a selection of a beatmap.
+        /// </summary>
+        /// <param name="item">The resultant <see cref="PlaylistItem"/>. This item has not yet been added to the <see cref="Room"/>'s.</param>
+        protected abstract void SelectItem(PlaylistItem item);
 
         public override bool OnBackButton()
         {
@@ -117,8 +121,18 @@ namespace osu.Game.Screens.OnlinePlay
             return buttons;
         }
 
+        /// <summary>
+        /// Checks whether a given <see cref="Mod"/> is valid for global selection.
+        /// </summary>
+        /// <param name="mod">The <see cref="Mod"/> to check.</param>
+        /// <returns>Whether <paramref name="mod"/> is a valid mod for online play.</returns>
         protected virtual bool IsValidMod(Mod mod) => !(mod is ModAutoplay) && (mod as MultiMod)?.Mods.Any(mm => mm is ModAutoplay) != true;
 
+        /// <summary>
+        /// Checks whether a given <see cref="Mod"/> is valid for per-player free-mod selection.
+        /// </summary>
+        /// <param name="mod">The <see cref="Mod"/> to check.</param>
+        /// <returns>Whether <paramref name="mod"/> is a selectable free-mod.</returns>
         protected virtual bool IsValidFreeMod(Mod mod) => IsValidMod(mod);
     }
 }

osu.Game/Screens/Play/HUD/ModDisplay.cs

@@ -26,6 +26,9 @@ namespace osu.Game.Screens.Play.HUD
 
         public ExpansionMode ExpansionMode = ExpansionMode.ExpandOnHover;
 
+        /// <summary>
+        /// Whether the mods should initially appear expanded, before potentially contracting into their final expansion state (depending on <see cref="ExpansionMode"/>).
+        /// </summary>
         public bool ExpandOnAppear = true;
 
         private readonly Bindable<IReadOnlyList<Mod>> current = new Bindable<IReadOnlyList<Mod>>();

osu.Game/Screens/Select/MatchSongSelect.cs

@@ -21,7 +21,7 @@ namespace osu.Game.Screens.Select
             CreateNewItem = createNewItem
         };
 
-        protected override void OnSetItem(PlaylistItem item)
+        protected override void SelectItem(PlaylistItem item)
         {
             switch (Playlist.Count)
             {

osu.Game/Screens/Select/SongSelect.cs

@@ -300,6 +300,10 @@ namespace osu.Game.Screens.Select
             }
         }
 
+        /// <summary>
+        /// Creates the buttons to be displayed in the footer.
+        /// </summary>
+        /// <returns>A set of <see cref="FooterButton"/> and an optional <see cref="OverlayContainer"/> which the button opens when pressed.</returns>
         protected virtual IEnumerable<(FooterButton, OverlayContainer)> CreateFooterButtons() => new (FooterButton, OverlayContainer)[]
         {
             (new FooterButtonMods { Current = Mods }, ModSelect),

osu.Game/Utils/ModUtils.cs

@@ -12,9 +12,9 @@ using osu.Game.Rulesets.Mods;
 namespace osu.Game.Utils
 {
     /// <summary>
-    /// A set of utilities to validate <see cref="Mod"/> combinations.
+    /// A set of utilities to handle <see cref="Mod"/> combinations.
     /// </summary>
-    public static class ModValidation
+    public static class ModUtils
     {
         /// <summary>
         /// Checks that all <see cref="Mod"/>s are compatible with each-other, and that all appear within a set of allowed types.
@@ -25,11 +25,11 @@ namespace osu.Game.Utils
         /// <param name="combination">The <see cref="Mod"/>s to check.</param>
         /// <param name="allowedTypes">The set of allowed <see cref="Mod"/> types.</param>
         /// <returns>Whether all <see cref="Mod"/>s are compatible with each-other and appear in the set of allowed types.</returns>
-        public static bool CheckCompatibleAndAllowed(IEnumerable<Mod> combination, IEnumerable<Type> allowedTypes)
+        public static bool CheckCompatibleSetAndAllowed(IEnumerable<Mod> combination, IEnumerable<Type> allowedTypes)
         {
             // Prevent multiple-enumeration.
             var combinationList = combination as ICollection<Mod> ?? combination.ToArray();
-            return CheckCompatible(combinationList) && CheckAllowed(combinationList, allowedTypes);
+            return CheckCompatibleSet(combinationList) && CheckAllowed(combinationList, allowedTypes);
         }
 
         /// <summary>
@@ -37,7 +37,7 @@ namespace osu.Game.Utils
         /// </summary>
         /// <param name="combination">The <see cref="Mod"/> combination to check.</param>
         /// <returns>Whether all <see cref="Mod"/>s in the combination are compatible with each-other.</returns>
-        public static bool CheckCompatible(IEnumerable<Mod> combination)
+        public static bool CheckCompatibleSet(IEnumerable<Mod> combination)
         {
             var incompatibleTypes = new HashSet<Type>();
             var incomingTypes = new HashSet<Type>();
@@ -83,20 +83,6 @@ namespace osu.Game.Utils
                               .All(m => allowedSet.Contains(m.GetType()));
         }
 
-        /// <summary>
-        /// Determines whether a <see cref="Mod"/> is in a set of incompatible types.
-        /// </summary>
-        /// <remarks>
-        /// A <see cref="Mod"/> can be incompatible through its most-declared type or any of its base types.
-        /// </remarks>
-        /// <param name="mod">The <see cref="Mod"/> to test.</param>
-        /// <param name="incompatibleTypes">The set of incompatible <see cref="Mod"/> types.</param>
-        /// <returns>Whether the given <see cref="Mod"/> is incompatible.</returns>
-        private static bool isModIncompatible(Mod mod, ICollection<Type> incompatibleTypes)
-            => FlattenMod(mod)
-               .SelectMany(m => m.GetType().EnumerateBaseTypes())
-               .Any(incompatibleTypes.Contains);
-
         /// <summary>
         /// Flattens a set of <see cref="Mod"/>s, returning a new set with all <see cref="MultiMod"/>s removed.
         /// </summary>