Merge branch 'master' into fix-run-from-screen-test-failures

osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs

@@ -1,9 +1,12 @@
 // 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.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Game.Rulesets.Catch.Objects;
+using osu.Game.Rulesets.Catch.UI;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.UI;
 using osu.Game.Rulesets.UI.Scrolling;
@@ -24,15 +27,20 @@ namespace osu.Game.Rulesets.Catch.Edit
             var blueprint = moveEvent.Blueprint;
             Vector2 originalPosition = HitObjectContainer.ToLocalSpace(blueprint.ScreenSpaceSelectionPoint);
             Vector2 targetPosition = HitObjectContainer.ToLocalSpace(blueprint.ScreenSpaceSelectionPoint + moveEvent.ScreenSpaceDelta);
+
             float deltaX = targetPosition.X - originalPosition.X;
+            deltaX = limitMovement(deltaX, EditorBeatmap.SelectedHitObjects);
+
+            if (deltaX == 0)
+            {
+                // Even if there is no positional change, there may be a time change.
+                return true;
+            }
 
             EditorBeatmap.PerformOnSelection(h =>
             {
                 if (!(h is CatchHitObject hitObject)) return;
 
-                if (hitObject is BananaShower) return;
-
-                // TODO: confine in bounds
                 hitObject.OriginalX += deltaX;
 
                 // Move the nested hit objects to give an instant result before nested objects are recreated.
@@ -42,5 +50,67 @@ namespace osu.Game.Rulesets.Catch.Edit
 
             return true;
         }
+
+        /// <summary>
+        /// Limit positional movement of the objects by the constraint that moved objects should stay in bounds.
+        /// </summary>
+        /// <param name="deltaX">The positional movement.</param>
+        /// <param name="movingObjects">The objects to be moved.</param>
+        /// <returns>The positional movement with the restriction applied.</returns>
+        private float limitMovement(float deltaX, IEnumerable<HitObject> movingObjects)
+        {
+            float minX = float.PositiveInfinity;
+            float maxX = float.NegativeInfinity;
+
+            foreach (float x in movingObjects.SelectMany(getOriginalPositions))
+            {
+                minX = Math.Min(minX, x);
+                maxX = Math.Max(maxX, x);
+            }
+
+            // To make an object with position `x` stay in bounds after `deltaX` movement, `0 <= x + deltaX <= WIDTH` should be satisfied.
+            // Subtracting `x`, we get `-x <= deltaX <= WIDTH - x`.
+            // We only need to apply the inequality to extreme values of `x`.
+            float lowerBound = -minX;
+            float upperBound = CatchPlayfield.WIDTH - maxX;
+            // The inequality may be unsatisfiable if the objects were already out of bounds.
+            // In that case, don't move objects at all.
+            if (lowerBound > upperBound)
+                return 0;
+
+            return Math.Clamp(deltaX, lowerBound, upperBound);
+        }
+
+        /// <summary>
+        /// Enumerate X positions that should be contained in-bounds after move offset is applied.
+        /// </summary>
+        private IEnumerable<float> getOriginalPositions(HitObject hitObject)
+        {
+            switch (hitObject)
+            {
+                case Fruit fruit:
+                    yield return fruit.OriginalX;
+
+                    break;
+
+                case JuiceStream juiceStream:
+                    foreach (var nested in juiceStream.NestedHitObjects.OfType<CatchHitObject>())
+                    {
+                        // Even if `OriginalX` is outside the playfield, tiny droplets can be moved inside the playfield after the random offset application.
+                        if (!(nested is TinyDroplet))
+                            yield return nested.OriginalX;
+                    }
+
+                    break;
+
+                case BananaShower _:
+                    // A banana shower occupies the whole screen width.
+                    // If the selection contains a banana shower, the selection cannot be moved horizontally.
+                    yield return 0;
+                    yield return CatchPlayfield.WIDTH;
+
+                    break;
+            }
+        }
     }
 }

osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs

@@ -11,7 +11,6 @@ using osu.Game.Rulesets.Scoring;
 using osu.Game.Screens.Play;
 using osu.Game.Screens.Play.Break;
 using osu.Game.Screens.Ranking;
-using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Gameplay
 {
@@ -36,18 +35,6 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddStep("rewind", () => Player.GameplayClockContainer.Seek(-80000));
             AddUntilStep("key counter reset", () => Player.HUDOverlay.KeyCounter.Children.All(kc => kc.CountPresses == 0));
 
-            double? time = null;
-
-            AddStep("store time", () => time = Player.GameplayClockContainer.GameplayClock.CurrentTime);
-
-            // test seek via keyboard
-            AddStep("seek with right arrow key", () => InputManager.Key(Key.Right));
-            AddAssert("time seeked forward", () => Player.GameplayClockContainer.GameplayClock.CurrentTime > time + 2000);
-
-            AddStep("store time", () => time = Player.GameplayClockContainer.GameplayClock.CurrentTime);
-            AddStep("seek with left arrow key", () => InputManager.Key(Key.Left));
-            AddAssert("time seeked backward", () => Player.GameplayClockContainer.GameplayClock.CurrentTime < time);
-
             seekToBreak(0);
             seekToBreak(1);
 

osu.Game.Tests/Visual/Gameplay/TestSceneReplayPlayer.cs

@@ -0,0 +1,87 @@
+// 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 NUnit.Framework;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osuTK.Input;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneReplayPlayer : RateAdjustedBeatmapTestScene
+    {
+        protected TestReplayPlayer Player;
+
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+
+            AddStep("Initialise player", () => Player = CreatePlayer(new OsuRuleset()));
+            AddStep("Load player", () => LoadScreen(Player));
+            AddUntilStep("player loaded", () => Player.IsLoaded);
+        }
+
+        [Test]
+        public void TestPause()
+        {
+            double? lastTime = null;
+
+            AddUntilStep("wait for first hit", () => Player.ScoreProcessor.TotalScore.Value > 0);
+
+            AddStep("Pause playback", () => InputManager.Key(Key.Space));
+
+            AddUntilStep("Time stopped progressing", () =>
+            {
+                double current = Player.GameplayClockContainer.CurrentTime;
+                bool changed = lastTime != current;
+                lastTime = current;
+
+                return !changed;
+            });
+
+            AddWaitStep("wait some", 10);
+
+            AddAssert("Time still stopped", () => lastTime == Player.GameplayClockContainer.CurrentTime);
+        }
+
+        [Test]
+        public void TestSeekBackwards()
+        {
+            double? lastTime = null;
+
+            AddUntilStep("wait for first hit", () => Player.ScoreProcessor.TotalScore.Value > 0);
+
+            AddStep("Seek backwards", () =>
+            {
+                lastTime = Player.GameplayClockContainer.CurrentTime;
+                InputManager.Key(Key.Left);
+            });
+
+            AddAssert("Jumped backwards", () => Player.GameplayClockContainer.CurrentTime - lastTime < 0);
+        }
+
+        [Test]
+        public void TestSeekForwards()
+        {
+            double? lastTime = null;
+
+            AddUntilStep("wait for first hit", () => Player.ScoreProcessor.TotalScore.Value > 0);
+
+            AddStep("Seek forwards", () =>
+            {
+                lastTime = Player.GameplayClockContainer.CurrentTime;
+                InputManager.Key(Key.Right);
+            });
+
+            AddAssert("Jumped forwards", () => Player.GameplayClockContainer.CurrentTime - lastTime > 500);
+        }
+
+        protected TestReplayPlayer CreatePlayer(Ruleset ruleset)
+        {
+            Beatmap.Value = CreateWorkingBeatmap(ruleset.RulesetInfo);
+            SelectedMods.Value = new[] { ruleset.GetAutoplayMod() };
+
+            return new TestReplayPlayer(false);
+        }
+    }
+}

osu.Game/IO/OsuStorage.cs

@@ -102,8 +102,15 @@ namespace osu.Game.IO
 
         protected override void ChangeTargetStorage(Storage newStorage)
         {
+            var lastStorage = UnderlyingStorage;
             base.ChangeTargetStorage(newStorage);
-            Logger.Storage = UnderlyingStorage.GetStorageForDirectory("logs");
+
+            if (lastStorage != null)
+            {
+                // for now we assume that if there was a previous storage, this is a migration operation.
+                // the logger shouldn't be set during initialisation as it can cause cross-talk in tests (due to being static).
+                Logger.Storage = UnderlyingStorage.GetStorageForDirectory("logs");
+            }
         }
 
         public override void Migrate(Storage newStorage)

osu.Game/Input/Bindings/GlobalActionContainer.cs

@@ -87,6 +87,8 @@ namespace osu.Game.Input.Bindings
             new KeyBinding(new[] { InputKey.Shift, InputKey.Tab }, GlobalAction.ToggleInGameInterface),
             new KeyBinding(InputKey.MouseMiddle, GlobalAction.PauseGameplay),
             new KeyBinding(InputKey.Space, GlobalAction.TogglePauseReplay),
+            new KeyBinding(InputKey.Left, GlobalAction.SeekReplayBackward),
+            new KeyBinding(InputKey.Right, GlobalAction.SeekReplayForward),
             new KeyBinding(InputKey.Control, GlobalAction.HoldForHUD),
         };
 
@@ -272,5 +274,11 @@ namespace osu.Game.Input.Bindings
 
         [Description("Next volume meter")]
         NextVolumeMeter,
+
+        [Description("Seek replay forward")]
+        SeekReplayForward,
+
+        [Description("Seek replay backward")]
+        SeekReplayBackward,
     }
 }

osu.Game/Screens/Play/ReplayPlayer.cs

@@ -3,11 +3,15 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using osu.Framework.Input.Bindings;
+using osu.Framework.Threading;
 using osu.Game.Beatmaps;
+using osu.Game.Extensions;
 using osu.Game.Input.Bindings;
 using osu.Game.Rulesets.Mods;
+using osu.Game.Rulesets.Objects;
 using osu.Game.Scoring;
 using osu.Game.Screens.Ranking;
 
@@ -43,10 +47,24 @@ namespace osu.Game.Screens.Play
 
         protected override ResultsScreen CreateResults(ScoreInfo score) => new SoloResultsScreen(score, false);
 
+        private ScheduledDelegate keyboardSeekDelegate;
+
         public bool OnPressed(GlobalAction action)
         {
+            const double keyboard_seek_amount = 5000;
+
             switch (action)
             {
+                case GlobalAction.SeekReplayBackward:
+                    keyboardSeekDelegate?.Cancel();
+                    keyboardSeekDelegate = this.BeginKeyRepeat(Scheduler, () => keyboardSeek(-1));
+                    return true;
+
+                case GlobalAction.SeekReplayForward:
+                    keyboardSeekDelegate?.Cancel();
+                    keyboardSeekDelegate = this.BeginKeyRepeat(Scheduler, () => keyboardSeek(1));
+                    return true;
+
                 case GlobalAction.TogglePauseReplay:
                     if (GameplayClockContainer.IsPaused.Value)
                         GameplayClockContainer.Start();
@@ -56,10 +74,24 @@ namespace osu.Game.Screens.Play
             }
 
             return false;
+
+            void keyboardSeek(int direction)
+            {
+                double target = Math.Clamp(GameplayClockContainer.CurrentTime + direction * keyboard_seek_amount, 0, GameplayBeatmap.HitObjects.Last().GetEndTime());
+
+                Seek(target);
+            }
         }
 
         public void OnReleased(GlobalAction action)
         {
+            switch (action)
+            {
+                case GlobalAction.SeekReplayBackward:
+                case GlobalAction.SeekReplayForward:
+                    keyboardSeekDelegate?.Cancel();
+                    break;
+            }
         }
     }
 }

osu.Game/Screens/Play/SongProgressBar.cs

@@ -57,8 +57,6 @@ namespace osu.Game.Screens.Play
             set => CurrentNumber.Value = value;
         }
 
-        protected override bool AllowKeyboardInputWhenNotHovered => true;
-
         public SongProgressBar(float barHeight, float handleBarHeight, Vector2 handleSize)
         {
             CurrentNumber.MinValue = 0;

osu.Game/Tests/CleanRunHeadlessGameHost.cs

@@ -25,8 +25,11 @@ namespace osu.Game.Tests
 
         protected override void SetupForRun()
         {
-            base.SetupForRun();
             Storage.DeleteDirectory(string.Empty);
+
+            // base call needs to be run *after* storage is emptied, as it updates the (static) logger's storage and may start writing
+            // log entries from another source if a unit test host is shared over multiple tests, causing a file access denied exception.
+            base.SetupForRun();
         }
     }
 }