From c937c45360228a8f8d11808d7d3903fa7cf2227f Mon Sep 17 00:00:00 2001 From: ekrctb Date: Thu, 8 Jul 2021 18:49:32 +0900 Subject: [PATCH 1/9] Don't move selected objects outside the playfield in catch editor --- .../Edit/CatchSelectionHandler.cs | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index d35d74d93d..9fcfa22cac 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -1,9 +1,12 @@ // Copyright (c) ppy Pty Ltd . 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; @@ -26,13 +29,19 @@ namespace osu.Game.Rulesets.Catch.Edit Vector2 targetPosition = HitObjectContainer.ToLocalSpace(blueprint.ScreenSpaceSelectionPoint + moveEvent.ScreenSpaceDelta); float deltaX = targetPosition.X - originalPosition.X; + foreach (float x in EditorBeatmap.SelectedHitObjects.SelectMany(getOriginalPositions)) + deltaX = Math.Clamp(deltaX, -x, CatchPlayfield.WIDTH - x); + + if (deltaX == 0) + { + // Returns true: even 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 +51,37 @@ namespace osu.Game.Rulesets.Catch.Edit return true; } + + /// + /// Enumerate X positions that should be contained in-bounds after move offset is applied. + /// + private IEnumerable getOriginalPositions(HitObject hitObject) + { + switch (hitObject) + { + case Fruit fruit: + yield return fruit.OriginalX; + + break; + + case JuiceStream juiceStream: + foreach (var nested in juiceStream.NestedHitObjects.OfType()) + { + // Exclude tiny droplets: even if `OriginalX` is outside the playfield, it 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; + } + } } } From 7e146796066588763ab367a6915e32c7a1f5d902 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Fri, 9 Jul 2021 12:58:08 +0900 Subject: [PATCH 2/9] Expand the selection movement limiting code with detailed comments --- .../Edit/CatchSelectionHandler.cs | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index 9fcfa22cac..d3e79e1778 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -27,10 +27,9 @@ 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; - foreach (float x in EditorBeatmap.SelectedHitObjects.SelectMany(getOriginalPositions)) - deltaX = Math.Clamp(deltaX, -x, CatchPlayfield.WIDTH - x); + float deltaX = targetPosition.X - originalPosition.X; + deltaX = limitMovement(deltaX, EditorBeatmap.SelectedHitObjects); if (deltaX == 0) { @@ -52,6 +51,36 @@ namespace osu.Game.Rulesets.Catch.Edit return true; } + /// + /// Limit positional movement of the objects by the constraint that moved objects should stay in bounds. + /// + /// The positional movement. + /// The objects to be moved. + /// The positional movement with the restriction applied. + private float limitMovement(float deltaX, IEnumerable 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); + } + /// /// Enumerate X positions that should be contained in-bounds after move offset is applied. /// From f3b68a4fbf5d8e810f8451299ac4b57ad138228d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 9 Jul 2021 13:17:25 +0900 Subject: [PATCH 3/9] Fix storage wrapping logic setting logger too early in startup sequence --- osu.Game/IO/OsuStorage.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 75130b0f9b..802c71e363 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/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) From 6a5f0e823718cb5cd419eab59ebcff55c159aa37 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 9 Jul 2021 14:28:57 +0900 Subject: [PATCH 4/9] Move handling of replay seek operations out of progress bar This is in order to avoid using the now obsoleted property `SliderBar.AllowKeyboardInputWhenNotHovered` (see https://github.com/ppy/osu-framework/pull/4579). --- .../Input/Bindings/GlobalActionContainer.cs | 8 +++++ osu.Game/Screens/Play/ReplayPlayer.cs | 32 +++++++++++++++++++ osu.Game/Screens/Play/SongProgressBar.cs | 2 -- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index 2482be90ee..d3cc90ef99 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/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, } } diff --git a/osu.Game/Screens/Play/ReplayPlayer.cs b/osu.Game/Screens/Play/ReplayPlayer.cs index f70c05c2ff..adbb5a53f6 100644 --- a/osu.Game/Screens/Play/ReplayPlayer.cs +++ b/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; + } } } } diff --git a/osu.Game/Screens/Play/SongProgressBar.cs b/osu.Game/Screens/Play/SongProgressBar.cs index 939b5fad1f..5052b32335 100644 --- a/osu.Game/Screens/Play/SongProgressBar.cs +++ b/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; From 9083b28114039b18078b0b0cc1402d15de88ed87 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 9 Jul 2021 14:47:11 +0900 Subject: [PATCH 5/9] Add test coverage of seeking and pausing --- .../Visual/Gameplay/TestSceneReplayPlayer.cs | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneReplayPlayer.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayPlayer.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayPlayer.cs new file mode 100644 index 0000000000..fcd65eaff3 --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayPlayer.cs @@ -0,0 +1,87 @@ +// Copyright (c) ppy Pty Ltd . 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); + } + } +} From 995ef953c689746c546457b9ec477368a89af366 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Fri, 9 Jul 2021 15:13:54 +0900 Subject: [PATCH 6/9] Modify comment --- osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index d3e79e1778..02dc6f61c8 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -33,7 +33,7 @@ namespace osu.Game.Rulesets.Catch.Edit if (deltaX == 0) { - // Returns true: even there is no positional change, there may be a time change. + // Even there is no positional change, there may be a time change. return true; } @@ -96,7 +96,7 @@ namespace osu.Game.Rulesets.Catch.Edit case JuiceStream juiceStream: foreach (var nested in juiceStream.NestedHitObjects.OfType()) { - // Exclude tiny droplets: even if `OriginalX` is outside the playfield, it can be moved inside the playfield after the random offset application. + // 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; } From 6ae631b03a1ac6048f94a29e97f370fa26454a58 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 9 Jul 2021 16:50:54 +0900 Subject: [PATCH 7/9] Remove previous seek testing logic from common test scene --- osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs index bc7cf8eee2..fdc3916c47 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs +++ b/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); From 494089e402dcd358dc6d1e20eadf8736fa858407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 10 Jul 2021 11:22:54 +0200 Subject: [PATCH 8/9] Fix up English in comment --- osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index 02dc6f61c8..51ccdb7410 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -33,7 +33,7 @@ namespace osu.Game.Rulesets.Catch.Edit if (deltaX == 0) { - // Even there is no positional change, there may be a time change. + // Even if there is no positional change, there may be a time change. return true; } From c5011865fca0f93672703d183b3268d5071cbd86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 10 Jul 2021 11:23:38 +0200 Subject: [PATCH 9/9] Invert strangely negated condition --- osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index 51ccdb7410..7eebf04ca2 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -75,7 +75,7 @@ namespace osu.Game.Rulesets.Catch.Edit 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)) + if (lowerBound > upperBound) return 0; return Math.Clamp(deltaX, lowerBound, upperBound);