Merge 9f67629f1b into ce8e4120b7

Do not deselect objects when control-clicking without hitting anything

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs

@@ -231,6 +231,36 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
             AddAssert("slider still has 2 anchors", () => secondSlider.Path.ControlPoints.Count, () => Is.EqualTo(2));
         }
 
+        [Test]
+        public void TestControlClickDoesNotDiscardExistingSelectionEvenIfNothingHit()
+        {
+            var firstSlider = new Slider
+            {
+                StartTime = 0,
+                Position = new Vector2(0, 0),
+                Path = new SliderPath
+                {
+                    ControlPoints =
+                    {
+                        new PathControlPoint(),
+                        new PathControlPoint(new Vector2(100))
+                    }
+                }
+            };
+
+            AddStep("add object", () => EditorBeatmap.AddRange([firstSlider]));
+            AddStep("select first slider", () => EditorBeatmap.SelectedHitObjects.AddRange([firstSlider]));
+
+            AddStep("move mouse to middle of playfield", () => InputManager.MoveMouseTo(blueprintContainer.ScreenSpaceDrawQuad.Centre));
+            AddStep("control-click left mouse", () =>
+            {
+                InputManager.PressKey(Key.ControlLeft);
+                InputManager.Click(MouseButton.Left);
+                InputManager.ReleaseKey(Key.ControlLeft);
+            });
+            AddAssert("selection preserved", () => EditorBeatmap.SelectedHitObjects.Count, () => Is.EqualTo(1));
+        }
+
         private ComposeBlueprintContainer blueprintContainer
             => Editor.ChildrenOfType<ComposeBlueprintContainer>().First();
 

osu.Game.Rulesets.Osu.Tests/TestSceneOsuAnalysisContainer.cs

@@ -21,7 +21,7 @@ namespace osu.Game.Rulesets.Osu.Tests
     public partial class TestSceneOsuAnalysisContainer : OsuTestScene
     {
         private TestReplayAnalysisOverlay analysisContainer = null!;
-        private ReplayAnalysisSettings settings = null!;
+        private OsuReplayAnalysisSettings settings = null!;
 
         [Cached]
         private OsuRulesetConfigManager config = new OsuRulesetConfigManager(null, new OsuRuleset().RulesetInfo);
@@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                     {
                         Child = analysisContainer = new TestReplayAnalysisOverlay(fabricateReplay()),
                     },
-                    settings = new ReplayAnalysisSettings(config),
+                    settings = new TestOsuReplayAnalysisSettings(Ruleset.Value.CreateInstance(), config),
                 };
 
                 settings.ShowClickMarkers.Value = false;
@@ -129,5 +129,26 @@ namespace osu.Game.Rulesets.Osu.Tests
             public bool AimMarkersVisible => FrameMarkers?.Alpha > 0 && FrameMarkers.Entries.Any();
             public bool AimLinesVisible => CursorPath?.Alpha > 0 && CursorPath.Vertices.Count > 1;
         }
+
+        private partial class TestOsuReplayAnalysisSettings : OsuReplayAnalysisSettings
+        {
+            private readonly OsuRulesetConfigManager config;
+
+            public TestOsuReplayAnalysisSettings(Ruleset ruleset, OsuRulesetConfigManager config)
+                : base(ruleset)
+            {
+                this.config = config;
+            }
+
+            [BackgroundDependencyLoader]
+            private void load()
+            {
+                config.BindWith(OsuRulesetSetting.ReplayClickMarkersEnabled, ShowClickMarkers);
+                config.BindWith(OsuRulesetSetting.ReplayFrameMarkersEnabled, ShowAimMarkers);
+                config.BindWith(OsuRulesetSetting.ReplayCursorPathEnabled, ShowCursorPath);
+                config.BindWith(OsuRulesetSetting.ReplayCursorHideEnabled, HideSkinCursor);
+                config.BindWith(OsuRulesetSetting.ReplayAnalysisDisplayLength, DisplayLength);
+            }
+        }
     }
 }

osu.Game.Rulesets.Osu/OsuRuleset.cs

@@ -382,5 +382,12 @@ namespace osu.Game.Rulesets.Osu
         }
 
         public override bool EditorShowScrollSpeed => false;
+
+        public override ReplayAnalysisSettings CreateReplayAnalysisSettings()
+        {
+            var settings = new OsuReplayAnalysisSettings(this);
+            settings.Expanded.Value = false;
+            return settings;
+        }
     }
 }

osu.Game.Rulesets.Osu/UI/DrawableOsuRuleset.cs

@@ -40,14 +40,13 @@ namespace osu.Game.Rulesets.Osu.UI
         }
 
         [BackgroundDependencyLoader]
-        private void load(ReplayPlayer? replayPlayer)
+        private void load(Player? player)
         {
-            if (replayPlayer != null)
+            if (player is ReplayPlayer || player is SpectatorPlayer)
             {
                 ReplayAnalysisOverlay analysisOverlay;
-                PlayfieldAdjustmentContainer.Add(analysisOverlay = new ReplayAnalysisOverlay(replayPlayer.Score.Replay));
+                PlayfieldAdjustmentContainer.Add(analysisOverlay = new ReplayAnalysisOverlay(player.Score.Replay));
                 Overlays.Add(analysisOverlay.CreateProxy().With(p => p.Depth = float.NegativeInfinity));
-                replayPlayer.AddSettings(new ReplayAnalysisSettings(Config));
 
                 cursorHideEnabled = Config.GetBindable<bool>(OsuRulesetSetting.ReplayCursorHideEnabled);
 

osu.Game.Rulesets.Osu/UI/OsuReplayAnalysisSettings.cs

@@ -5,13 +5,14 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Game.Configuration;
 using osu.Game.Rulesets.Osu.Configuration;
+using osu.Game.Rulesets.UI;
 using osu.Game.Screens.Play.PlayerSettings;
 
 namespace osu.Game.Rulesets.Osu.UI
 {
-    public partial class ReplayAnalysisSettings : PlayerSettingsGroup
+    public partial class OsuReplayAnalysisSettings : ReplayAnalysisSettings
     {
-        private readonly OsuRulesetConfigManager config;
+        protected new OsuRulesetConfigManager Config => (OsuRulesetConfigManager)base.Config;
 
         [SettingSource("Show click markers", SettingControlType = typeof(PlayerCheckbox))]
         public BindableBool ShowClickMarkers { get; } = new BindableBool();
@@ -34,22 +35,19 @@ namespace osu.Game.Rulesets.Osu.UI
             Precision = 200,
         };
 
-        public ReplayAnalysisSettings(OsuRulesetConfigManager config)
-            : base("Analysis Settings")
+        public OsuReplayAnalysisSettings(Ruleset ruleset)
+            : base(ruleset)
         {
-            this.config = config;
         }
 
         [BackgroundDependencyLoader]
         private void load()
         {
-            AddRange(this.CreateSettingsControls());
-
-            config.BindWith(OsuRulesetSetting.ReplayClickMarkersEnabled, ShowClickMarkers);
-            config.BindWith(OsuRulesetSetting.ReplayFrameMarkersEnabled, ShowAimMarkers);
-            config.BindWith(OsuRulesetSetting.ReplayCursorPathEnabled, ShowCursorPath);
-            config.BindWith(OsuRulesetSetting.ReplayCursorHideEnabled, HideSkinCursor);
-            config.BindWith(OsuRulesetSetting.ReplayAnalysisDisplayLength, DisplayLength);
+            Config.BindWith(OsuRulesetSetting.ReplayClickMarkersEnabled, ShowClickMarkers);
+            Config.BindWith(OsuRulesetSetting.ReplayFrameMarkersEnabled, ShowAimMarkers);
+            Config.BindWith(OsuRulesetSetting.ReplayCursorPathEnabled, ShowCursorPath);
+            Config.BindWith(OsuRulesetSetting.ReplayCursorHideEnabled, HideSkinCursor);
+            Config.BindWith(OsuRulesetSetting.ReplayAnalysisDisplayLength, DisplayLength);
         }
     }
 }

osu.Game.Rulesets.Osu/UI/ReplayAnalysisOverlay.cs

@@ -27,6 +27,8 @@ namespace osu.Game.Rulesets.Osu.UI
 
         private readonly Replay replay;
 
+        private int replayFrameIndex;
+
         public ReplayAnalysisOverlay(Replay replay)
         {
             RelativeSizeAxes = Axes.Both;
@@ -52,20 +54,39 @@ namespace osu.Game.Rulesets.Osu.UI
             displayLength.BindValueChanged(_ =>
             {
                 // Need to fully reload to make this work.
-                loaded.Invalidate();
+                invalidateLoaded();
             }, true);
         }
 
-        private readonly Cached loaded = new Cached();
+        /// <summary>
+        /// Invalidated when containers are not loaded nor loading, false if loading, and true if loaded
+        /// </summary>
+        /// <remarks>
+        /// Knowing the loading/loaded state is for avoiding an enumeration error when adding
+        /// new entries and not starting a new load while loading
+        /// </remarks>
+        private readonly Cached<bool> loadState = new Cached<bool>();
 
         private CancellationTokenSource? generationCancellationSource;
 
+        private void invalidateLoaded()
+        {
+            loadState.Invalidate();
+            replayFrameIndex = 0;
+        }
+
         protected override void Update()
         {
             base.Update();
 
             if (requireDisplay)
+            {
                 initialise();
+                // adding entries while the component is asynchronously loading
+                // can collide with enumeration operations and cause an error
+                if (loadState.IsValid && loadState.Value)
+                    addEntries();
+            }
 
             if (ClickMarkers != null) ClickMarkers.Alpha = showClickMarkers.Value ? 1 : 0;
             if (FrameMarkers != null) FrameMarkers.Alpha = showFrameMarkers.Value ? 1 : 0;
@@ -74,10 +95,10 @@ namespace osu.Game.Rulesets.Osu.UI
 
         private void initialise()
         {
-            if (loaded.IsValid)
+            if (loadState.IsValid)
                 return;
 
-            loaded.Validate();
+            loadState.Value = false;
 
             generationCancellationSource?.Cancel();
             generationCancellationSource = new CancellationTokenSource();
@@ -90,14 +111,23 @@ namespace osu.Game.Rulesets.Osu.UI
                 FrameMarkers = new FrameMarkerContainer(),
             };
 
+            LoadComponentsAsync(newDrawables, drawables =>
+            {
+                InternalChildrenEnumerable = drawables;
+                loadState.Value = true;
+            }, generationCancellationSource.Token);
+        }
+
+        private void addEntries()
+        {
             bool leftHeld = false;
             bool rightHeld = false;
 
             // This should probably be async as well, but it's a bit of a pain to debounce and everything.
             // Let's address concerns when they are raised.
-            foreach (var frame in replay.Frames)
+            while (replayFrameIndex < replay.Frames.Count)
             {
-                var osuFrame = (OsuReplayFrame)frame;
+                var osuFrame = (OsuReplayFrame)replay.Frames[replayFrameIndex];
 
                 bool leftButton = osuFrame.Actions.Contains(OsuAction.LeftButton);
                 bool rightButton = osuFrame.Actions.Contains(OsuAction.RightButton);
@@ -107,7 +137,7 @@ namespace osu.Game.Rulesets.Osu.UI
                 else if (!leftHeld && leftButton)
                 {
                     leftHeld = true;
-                    ClickMarkers.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, OsuAction.LeftButton));
+                    ClickMarkers!.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, OsuAction.LeftButton));
                 }
 
                 if (rightHeld && !rightButton)
@@ -115,14 +145,14 @@ namespace osu.Game.Rulesets.Osu.UI
                 else if (!rightHeld && rightButton)
                 {
                     rightHeld = true;
-                    ClickMarkers.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, OsuAction.RightButton));
+                    ClickMarkers!.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, OsuAction.RightButton));
                 }
 
-                FrameMarkers.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, osuFrame.Actions.ToArray()));
-                CursorPath.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position));
-            }
+                FrameMarkers!.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position, osuFrame.Actions.ToArray()));
+                CursorPath!.Add(new AnalysisFrameEntry(osuFrame.Time, displayLength.Value, osuFrame.Position));
 
-            LoadComponentsAsync(newDrawables, drawables => InternalChildrenEnumerable = drawables, generationCancellationSource.Token);
+                replayFrameIndex++;
+            }
         }
     }
 }

osu.Game/Rulesets/Ruleset.cs

@@ -426,5 +426,11 @@ namespace osu.Game.Rulesets
         /// Can be overridden to avoid showing scroll speed changes in the editor.
         /// </summary>
         public virtual bool EditorShowScrollSpeed => true;
+
+        /// <summary>
+        /// Creates a ruleset-specific replay analysis settings drawable
+        /// </summary>
+        /// <returns>The replay analysis settings drawable</returns>
+        public virtual ReplayAnalysisSettings? CreateReplayAnalysisSettings() => null;
     }
 }

osu.Game/Rulesets/UI/ReplayAnalysisSettings.cs

@@ -0,0 +1,42 @@
+// 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.
+
+#nullable disable
+
+using osu.Framework.Allocation;
+using osu.Game.Configuration;
+using osu.Game.Rulesets.Configuration;
+using osu.Game.Screens.Play.PlayerSettings;
+
+namespace osu.Game.Rulesets.UI
+{
+    public partial class ReplayAnalysisSettings : PlayerSettingsGroup
+    {
+        private readonly Ruleset ruleset;
+
+        protected IRulesetConfigManager Config;
+
+        public ReplayAnalysisSettings(Ruleset ruleset)
+            : base("Analysis Settings")
+        {
+            this.ruleset = ruleset;
+        }
+
+        [BackgroundDependencyLoader]
+        private void load()
+        {
+            AddRange(this.CreateSettingsControls());
+        }
+
+        protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
+        {
+            var dependencies = new DependencyContainer(base.CreateChildDependencies(parent));
+
+            Config = dependencies.Get<IRulesetConfigCache>().GetConfigFor(ruleset);
+            if (Config is not null)
+                dependencies.Cache(Config);
+
+            return dependencies;
+        }
+    }
+}

osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs

@@ -433,7 +433,10 @@ namespace osu.Game.Screens.Edit.Compose.Components
         /// Finishes the current blueprint selection.
         /// </summary>
         /// <param name="e">The mouse event which triggered end of selection.</param>
-        /// <returns>Whether a click selection was active.</returns>
+        /// <returns>
+        /// Whether the mouse event is considered to be fully handled.
+        /// If the return value is <see langword="false"/>, the standard click / mouse up action will follow.
+        /// </returns>
         private bool endClickSelection(MouseButtonEvent e)
         {
             // If already handled a selection, double-click, or drag, we don't want to perform a mouse up / click action.
@@ -443,14 +446,16 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
             if (e.ControlPressed)
             {
-                // if a selection didn't occur, we may want to trigger a deselection.
-
                 // Iterate from the top of the input stack (blueprints closest to the front of the screen first).
                 // Priority is given to already-selected blueprints.
                 foreach (SelectionBlueprint<T> blueprint in SelectionBlueprints.AliveChildren.Where(b => b.IsHovered).OrderByDescending(b => b.IsSelected))
                     return clickSelectionHandled = SelectionHandler.MouseUpSelectionRequested(blueprint, e);
 
-                return false;
+                // can only be reached if there are no hovered blueprints.
+                // in that case, we still want to suppress mouse up / click handling, because when control is pressed,
+                // it is presumed we want to add to existing selection, not remove from it
+                // (unless explicitly control-clicking a selected object, which is handled above).
+                return true;
             }
 
             if (selectedBlueprintAlreadySelectedOnMouseDown && SelectedItems.Count == 1)

osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs

@@ -54,6 +54,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         private SpectatorSyncManager syncManager = null!;
         private PlayerGrid grid = null!;
         private MultiSpectatorLeaderboard leaderboard = null!;
+        private FillFlowContainer leaderboardFlow = null!;
         private PlayerArea? currentAudioSource;
 
         private readonly Room room;
@@ -76,7 +77,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         [BackgroundDependencyLoader]
         private void load()
         {
-            FillFlowContainer leaderboardFlow;
             Container scoreDisplayContainer;
 
             InternalChildren = new Drawable[]
@@ -157,6 +157,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             {
                 Expanded = { Value = true },
             }, chat => leaderboardFlow.Insert(1, chat));
+
+            var replayAnalysisSettings = Ruleset.Value.CreateInstance().CreateReplayAnalysisSettings();
+            if (replayAnalysisSettings is not null)
+                leaderboardFlow.Insert(2, replayAnalysisSettings);
         }
 
         protected override void LoadComplete()

osu.Game/Screens/Play/Player.cs

@@ -1296,6 +1296,16 @@ namespace osu.Game.Screens.Play
             }
         }
 
+        /// <summary>
+        /// Create and add <see cref="ReplayAnalysisSettings"/> to settings overlay.
+        /// </summary>
+        protected void AddReplayAnalysisSettings()
+        {
+            var replayAnalysisSettings = DrawableRuleset.Ruleset.CreateReplayAnalysisSettings();
+            if (replayAnalysisSettings is not null)
+                HUDOverlay.PlayerSettingsOverlay.Add(replayAnalysisSettings);
+        }
+
         #endregion
 
         IBindable<bool> ISamplePlaybackDisabler.SamplePlaybackDisabled => samplePlaybackDisabled;

osu.Game/Screens/Play/ReplayPlayer.cs

@@ -55,16 +55,6 @@ namespace osu.Game.Screens.Play
             this.createScore = createScore;
         }
 
-        /// <summary>
-        /// Add a settings group to the HUD overlay. Intended to be used by rulesets to add replay-specific settings.
-        /// </summary>
-        /// <param name="settings">The settings group to be shown.</param>
-        public void AddSettings(PlayerSettingsGroup settings) => Schedule(() =>
-        {
-            settings.Expanded.Value = false;
-            HUDOverlay.PlayerSettingsOverlay.Add(settings);
-        });
-
         [BackgroundDependencyLoader]
         private void load(OsuConfigManager config)
         {
@@ -81,6 +71,8 @@ namespace osu.Game.Screens.Play
                 playbackSettings.UserPlaybackRate.BindTo(master.UserPlaybackRate);
 
             HUDOverlay.PlayerSettingsOverlay.AddAtStart(playbackSettings);
+
+            AddReplayAnalysisSettings();
         }
 
         protected override void PrepareReplay()

osu.Game/Screens/Play/SpectatorPlayer.cs

@@ -50,6 +50,8 @@ namespace osu.Game.Screens.Play
                 Anchor = Anchor.TopCentre,
                 Origin = Anchor.TopCentre,
             });
+
+            AddReplayAnalysisSettings();
         }
 
         protected override void LoadComplete()