From 89d8ed8e20b2850cb96af75fd0d5b69adf575c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 20:26:16 +0200 Subject: [PATCH 01/29] Port existing test coverage --- .../UserInterface/TestSceneModSelectScreen.cs | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs index ec6e962c6a..bd63023efb 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs @@ -2,17 +2,21 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Graphics.UserInterface; using osu.Game.Overlays.Mods; +using osu.Game.Overlays.Settings; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; using osuTK.Input; @@ -169,6 +173,206 @@ namespace osu.Game.Tests.Visual.UserInterface assertCustomisationToggleState(disabled: true, active: false); // config was dismissed without explicit user action. } + /// + /// Ensure that two mod overlays are not cross polluting via central settings instances. + /// + [Test] + public void TestSettingsNotCrossPolluting() + { + Bindable> selectedMods2 = null; + ModSelectScreen modSelectScreen2 = null; + + createScreen(); + AddStep("select diff adjust", () => SelectedMods.Value = new Mod[] { new OsuModDifficultyAdjust() }); + + AddStep("set setting", () => modSelectScreen.ChildrenOfType>().First().Current.Value = 8); + + AddAssert("ensure setting is propagated", () => SelectedMods.Value.OfType().Single().CircleSize.Value == 8); + + AddStep("create second bindable", () => selectedMods2 = new Bindable>(new Mod[] { new OsuModDifficultyAdjust() })); + + AddStep("create second overlay", () => + { + Add(modSelectScreen2 = new UserModSelectScreen().With(d => + { + d.Origin = Anchor.TopCentre; + d.Anchor = Anchor.TopCentre; + d.SelectedMods.BindTarget = selectedMods2; + })); + }); + + AddStep("show", () => modSelectScreen2.Show()); + + AddAssert("ensure first is unchanged", () => SelectedMods.Value.OfType().Single().CircleSize.Value == 8); + AddAssert("ensure second is default", () => selectedMods2.Value.OfType().Single().CircleSize.Value == null); + } + + [Test] + public void TestSettingsResetOnDeselection() + { + var osuModDoubleTime = new OsuModDoubleTime { SpeedChange = { Value = 1.2 } }; + + createScreen(); + changeRuleset(0); + + AddStep("set dt mod with custom rate", () => { SelectedMods.Value = new[] { osuModDoubleTime }; }); + + AddAssert("selected mod matches", () => (SelectedMods.Value.Single() as OsuModDoubleTime)?.SpeedChange.Value == 1.2); + + AddStep("deselect", () => getPanelForMod(typeof(OsuModDoubleTime)).TriggerClick()); + AddAssert("selected mods empty", () => SelectedMods.Value.Count == 0); + + AddStep("reselect", () => getPanelForMod(typeof(OsuModDoubleTime)).TriggerClick()); + AddAssert("selected mod has default value", () => (SelectedMods.Value.Single() as OsuModDoubleTime)?.SpeedChange.IsDefault == true); + } + + [Test] + public void TestAnimationFlushOnClose() + { + createScreen(); + changeRuleset(0); + + AddStep("Select all fun mods", () => + { + modSelectScreen.ChildrenOfType() + .Single(c => c.ModType == ModType.DifficultyIncrease) + .SelectAll(); + }); + + AddUntilStep("many mods selected", () => SelectedMods.Value.Count >= 5); + + AddStep("trigger deselect and close overlay", () => + { + modSelectScreen.ChildrenOfType() + .Single(c => c.ModType == ModType.DifficultyIncrease) + .DeselectAll(); + + modSelectScreen.Hide(); + }); + + AddAssert("all mods deselected", () => SelectedMods.Value.Count == 0); + } + + [Test] + public void TestRulesetChanges() + { + createScreen(); + changeRuleset(0); + + var noFailMod = new OsuRuleset().GetModsFor(ModType.DifficultyReduction).FirstOrDefault(m => m is OsuModNoFail); + + AddStep("set mods externally", () => { SelectedMods.Value = new[] { noFailMod }; }); + + changeRuleset(0); + + AddAssert("ensure mods still selected", () => SelectedMods.Value.SingleOrDefault(m => m is OsuModNoFail) != null); + + changeRuleset(3); + + AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0); + + changeRuleset(0); + + AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0); + } + + [Test] + public void TestExternallySetCustomizedMod() + { + createScreen(); + changeRuleset(0); + + AddStep("set customized mod externally", () => SelectedMods.Value = new[] { new OsuModDoubleTime { SpeedChange = { Value = 1.01 } } }); + + AddAssert("ensure button is selected and customized accordingly", () => + { + var button = getPanelForMod(SelectedMods.Value.Single().GetType()); + return ((OsuModDoubleTime)button.Mod).SpeedChange.Value == 1.01; + }); + } + + [Test] + public void TestSettingsAreRetainedOnReload() + { + createScreen(); + changeRuleset(0); + + AddStep("set customized mod externally", () => SelectedMods.Value = new[] { new OsuModDoubleTime { SpeedChange = { Value = 1.01 } } }); + AddAssert("setting remains", () => (SelectedMods.Value.SingleOrDefault() as OsuModDoubleTime)?.SpeedChange.Value == 1.01); + + createScreen(); + AddAssert("setting remains", () => (SelectedMods.Value.SingleOrDefault() as OsuModDoubleTime)?.SpeedChange.Value == 1.01); + } + + [Test] + public void TestExternallySetModIsReplacedByOverlayInstance() + { + Mod external = new OsuModDoubleTime(); + Mod overlayButtonMod = null; + + createScreen(); + changeRuleset(0); + + AddStep("set mod externally", () => { SelectedMods.Value = new[] { external }; }); + + AddAssert("ensure button is selected", () => + { + var button = getPanelForMod(SelectedMods.Value.Single().GetType()); + overlayButtonMod = button.Mod; + return button.Active.Value; + }); + + // Right now, when an external change occurs, the ModSelectOverlay will replace the global instance with its own + AddAssert("mod instance doesn't match", () => external != overlayButtonMod); + + AddAssert("one mod present in global selected", () => SelectedMods.Value.Count == 1); + AddAssert("globally selected matches button's mod instance", () => SelectedMods.Value.Contains(overlayButtonMod)); + AddAssert("globally selected doesn't contain original external change", () => !SelectedMods.Value.Contains(external)); + } + + [Test] + public void TestChangeIsValidChangesButtonVisibility() + { + createScreen(); + changeRuleset(0); + + AddAssert("double time visible", () => modSelectScreen.ChildrenOfType().Where(panel => panel.Mod is OsuModDoubleTime).Any(panel => !panel.Filtered.Value)); + + AddStep("make double time invalid", () => modSelectScreen.IsValidMod = m => !(m is OsuModDoubleTime)); + AddUntilStep("double time not visible", () => modSelectScreen.ChildrenOfType().Where(panel => panel.Mod is OsuModDoubleTime).All(panel => panel.Filtered.Value)); + AddAssert("nightcore still visible", () => modSelectScreen.ChildrenOfType().Where(panel => panel.Mod is OsuModNightcore).Any(panel => !panel.Filtered.Value)); + + AddStep("make double time valid again", () => modSelectScreen.IsValidMod = m => true); + AddUntilStep("double time visible", () => modSelectScreen.ChildrenOfType().Where(panel => panel.Mod is OsuModDoubleTime).Any(panel => !panel.Filtered.Value)); + AddAssert("nightcore still visible", () => modSelectScreen.ChildrenOfType().Where(b => b.Mod is OsuModNightcore).Any(panel => !panel.Filtered.Value)); + } + + [Test] + public void TestChangeIsValidPreservesSelection() + { + createScreen(); + changeRuleset(0); + + AddStep("select DT + HD", () => SelectedMods.Value = new Mod[] { new OsuModDoubleTime(), new OsuModHidden() }); + AddAssert("DT + HD selected", () => modSelectScreen.ChildrenOfType().Count(panel => panel.Active.Value) == 2); + + AddStep("make NF invalid", () => modSelectScreen.IsValidMod = m => !(m is ModNoFail)); + AddAssert("DT + HD still selected", () => modSelectScreen.ChildrenOfType().Count(panel => panel.Active.Value) == 2); + } + + [Test] + public void TestUnimplementedModIsUnselectable() + { + var testRuleset = new TestUnimplementedModOsuRuleset(); + + createScreen(); + + AddStep("set ruleset", () => Ruleset.Value = testRuleset.RulesetInfo); + waitForColumnLoad(); + + AddAssert("unimplemented mod panel is filtered", () => getPanelForMod(typeof(TestUnimplementedMod)).Filtered.Value); + } + private void waitForColumnLoad() => AddUntilStep("all column content loaded", () => modSelectScreen.ChildrenOfType().Any() && modSelectScreen.ChildrenOfType().All(column => column.IsLoaded && column.ItemsLoaded)); @@ -188,5 +392,26 @@ namespace osu.Game.Tests.Visual.UserInterface private ModPanel getPanelForMod(Type modType) => modSelectScreen.ChildrenOfType().Single(panel => panel.Mod.GetType() == modType); + + private class TestUnimplementedMod : Mod + { + public override string Name => "Unimplemented mod"; + public override string Acronym => "UM"; + public override string Description => "A mod that is not implemented."; + public override double ScoreMultiplier => 1; + public override ModType Type => ModType.Conversion; + } + + private class TestUnimplementedModOsuRuleset : OsuRuleset + { + public override string ShortName => "unimplemented"; + + public override IEnumerable GetModsFor(ModType type) + { + if (type == ModType.Conversion) return base.GetModsFor(type).Concat(new[] { new TestUnimplementedMod() }); + + return base.GetModsFor(type); + } + } } } From 746a4a740316f6bd1fbdd17f8bcc1d63c3474e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 21:23:54 +0200 Subject: [PATCH 02/29] Fix mod column using wrong equality type --- osu.Game/Overlays/Mods/ModColumn.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index 018922c074..9e32602a85 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -296,7 +296,7 @@ namespace osu.Game.Overlays.Mods private void updateActiveState() { foreach (var panel in panelFlow) - panel.Active.Value = SelectedMods.Value.Contains(panel.Mod, EqualityComparer.Default); + panel.Active.Value = SelectedMods.Value.Any(selected => selected.GetType() == panel.Mod.GetType()); } #region Bulk select / deselect From fe59f4ae58ae848af1f6992d9c4b63f9f3d3a0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 20:32:45 +0200 Subject: [PATCH 03/29] Fix multiselection operation not flushing on close --- osu.Game/Overlays/Mods/ModColumn.cs | 9 +++++++++ osu.Game/Overlays/Mods/ModSelectScreen.cs | 10 ++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index 9e32602a85..c839305746 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -364,6 +364,15 @@ namespace osu.Game.Overlays.Mods pendingSelectionOperations.Enqueue(() => button.Active.Value = false); } + /// + /// Play out all remaining animations immediately to leave mods in a good (final) state. + /// + public void FlushAnimation() + { + while (pendingSelectionOperations.TryDequeue(out var dequeuedAction)) + dequeuedAction(); + } + private class ToggleAllCheckbox : OsuCheckbox { private Color4 accentColour; diff --git a/osu.Game/Overlays/Mods/ModSelectScreen.cs b/osu.Game/Overlays/Mods/ModSelectScreen.cs index ffd6e9a52c..20c5b5a321 100644 --- a/osu.Game/Overlays/Mods/ModSelectScreen.cs +++ b/osu.Game/Overlays/Mods/ModSelectScreen.cs @@ -313,10 +313,12 @@ namespace osu.Game.Overlays.Mods { const float distance = 700; - columnFlow[i].Column - .TopLevelContent - .MoveToY(i % 2 == 0 ? -distance : distance, fade_out_duration, Easing.OutQuint) - .FadeOut(fade_out_duration, Easing.OutQuint); + var column = columnFlow[i].Column; + + column.FlushAnimation(); + column.TopLevelContent + .MoveToY(i % 2 == 0 ? -distance : distance, fade_out_duration, Easing.OutQuint) + .FadeOut(fade_out_duration, Easing.OutQuint); } } From 7c04bf5c53442b3fe2d0adf05928c674a378923c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 22:12:06 +0200 Subject: [PATCH 04/29] Refactor mod reference management to meet test expectations --- osu.Game/Overlays/Mods/ModColumn.cs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index c839305746..c2273abf53 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Collections.Generic; using System.Linq; @@ -10,6 +12,7 @@ using Humanizer; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; @@ -25,8 +28,6 @@ using osuTK; using osuTK.Graphics; using osuTK.Input; -#nullable enable - namespace osu.Game.Overlays.Mods { public class ModColumn : CompositeDrawable @@ -256,7 +257,9 @@ namespace osu.Game.Overlays.Mods private void updateMods() { - var newMods = ModUtils.FlattenMods(availableMods.Value.GetValueOrDefault(ModType) ?? Array.Empty()).ToList(); + var newMods = ModUtils.FlattenMods(availableMods.Value.GetValueOrDefault(ModType) ?? Array.Empty()) + .Select(m => m.DeepClone()) + .ToList(); if (newMods.SequenceEqual(panelFlow.Children.Select(p => p.Mod))) return; @@ -280,9 +283,16 @@ namespace osu.Game.Overlays.Mods panel.Active.BindValueChanged(_ => { updateToggleAllState(); - SelectedMods.Value = panel.Active.Value - ? SelectedMods.Value.Append(panel.Mod).ToArray() - : SelectedMods.Value.Except(new[] { panel.Mod }).ToArray(); + + var newSelectedMods = SelectedMods.Value; + + var matchingModInstance = SelectedMods.Value.SingleOrDefault(selected => selected.GetType() == panel.Mod.GetType()); + if (matchingModInstance != null && (matchingModInstance != panel.Mod || !panel.Active.Value)) + newSelectedMods = newSelectedMods.Except(matchingModInstance.Yield()).ToArray(); + if (panel.Active.Value) + newSelectedMods = newSelectedMods.Append(panel.Mod).ToArray(); + + SelectedMods.Value = newSelectedMods.ToArray(); }); } }, (cancellationTokenSource = new CancellationTokenSource()).Token); @@ -296,7 +306,12 @@ namespace osu.Game.Overlays.Mods private void updateActiveState() { foreach (var panel in panelFlow) - panel.Active.Value = SelectedMods.Value.Any(selected => selected.GetType() == panel.Mod.GetType()); + { + var matchingSelectedMod = SelectedMods.Value.SingleOrDefault(selected => selected.GetType() == panel.Mod.GetType()); + panel.Active.Value = matchingSelectedMod != null; + if (matchingSelectedMod != null) + panel.Mod.CopyFrom(matchingSelectedMod); + } } #region Bulk select / deselect From f91ee4b0422bd8b91585b4f0f9a689719a8b0f06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 22:14:28 +0200 Subject: [PATCH 05/29] Reset panel mod instance settings to defaults on deselect --- osu.Game/Overlays/Mods/ModColumn.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index c2273abf53..d480382fcb 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -309,8 +309,10 @@ namespace osu.Game.Overlays.Mods { var matchingSelectedMod = SelectedMods.Value.SingleOrDefault(selected => selected.GetType() == panel.Mod.GetType()); panel.Active.Value = matchingSelectedMod != null; - if (matchingSelectedMod != null) + if (panel.Active.Value) panel.Mod.CopyFrom(matchingSelectedMod); + else + panel.Mod.ResetSettingsToDefaults(); } } From e3641213e1832c627def0b48aae6213f455e4b17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 17 Apr 2022 22:16:59 +0200 Subject: [PATCH 06/29] Always hide unimplemented mods on mod select screen --- osu.Game/Overlays/Mods/ModSelectScreen.cs | 2 +- osu.Game/Screens/OnlinePlay/FreeModSelectScreen.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModSelectScreen.cs b/osu.Game/Overlays/Mods/ModSelectScreen.cs index 20c5b5a321..2f6eae0afb 100644 --- a/osu.Game/Overlays/Mods/ModSelectScreen.cs +++ b/osu.Game/Overlays/Mods/ModSelectScreen.cs @@ -203,7 +203,7 @@ namespace osu.Game.Overlays.Mods private void updateAvailableMods() { foreach (var column in columnFlow.Columns) - column.Filter = isValidMod; + column.Filter = m => m.HasImplementation && isValidMod.Invoke(m); } private void updateCustomisation(ValueChangedEvent> valueChangedEvent) diff --git a/osu.Game/Screens/OnlinePlay/FreeModSelectScreen.cs b/osu.Game/Screens/OnlinePlay/FreeModSelectScreen.cs index c85a4fc38b..5a7a60b479 100644 --- a/osu.Game/Screens/OnlinePlay/FreeModSelectScreen.cs +++ b/osu.Game/Screens/OnlinePlay/FreeModSelectScreen.cs @@ -16,7 +16,7 @@ namespace osu.Game.Screens.OnlinePlay public new Func IsValidMod { get => base.IsValidMod; - set => base.IsValidMod = m => m.HasImplementation && m.UserPlayable && value.Invoke(m); + set => base.IsValidMod = m => m.UserPlayable && value.Invoke(m); } public FreeModSelectScreen() From 970361676b374995f5c768805e9d9097e23ec855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 2 May 2022 23:05:35 +0200 Subject: [PATCH 07/29] Fix setting items not creating if mods initially not empty --- .../Visual/UserInterface/TestSceneModSelectScreen.cs | 2 ++ osu.Game/Overlays/Mods/ModSettingsArea.cs | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs index bd63023efb..7690029ba2 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs @@ -68,6 +68,7 @@ namespace osu.Game.Tests.Visual.UserInterface return Precision.AlmostEquals(multiplier, modSelectScreen.ChildrenOfType().Single().Current.Value); }); assertCustomisationToggleState(disabled: false, active: false); + AddAssert("setting items created", () => modSelectScreen.ChildrenOfType().Any()); } [Test] @@ -82,6 +83,7 @@ namespace osu.Game.Tests.Visual.UserInterface return Precision.AlmostEquals(multiplier, modSelectScreen.ChildrenOfType().Single().Current.Value); }); assertCustomisationToggleState(disabled: false, active: false); + AddAssert("setting items created", () => modSelectScreen.ChildrenOfType().Any()); } [Test] diff --git a/osu.Game/Overlays/Mods/ModSettingsArea.cs b/osu.Game/Overlays/Mods/ModSettingsArea.cs index be72c1e3e3..9c5f3b7f11 100644 --- a/osu.Game/Overlays/Mods/ModSettingsArea.cs +++ b/osu.Game/Overlays/Mods/ModSettingsArea.cs @@ -1,6 +1,7 @@ // 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; @@ -21,7 +22,7 @@ namespace osu.Game.Overlays.Mods { public class ModSettingsArea : CompositeDrawable { - public Bindable> SelectedMods { get; } = new Bindable>(); + public Bindable> SelectedMods { get; } = new Bindable>(Array.Empty()); public const float HEIGHT = 250; @@ -77,7 +78,7 @@ namespace osu.Game.Overlays.Mods protected override void LoadComplete() { base.LoadComplete(); - SelectedMods.BindValueChanged(_ => updateMods()); + SelectedMods.BindValueChanged(_ => updateMods(), true); } private void updateMods() From 216dfb7e9121b5e0f8b662d3f9039aeb7860fbcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 2 May 2022 23:29:30 +0200 Subject: [PATCH 08/29] Expand incompatibility test to cover logic more thoroughly --- .../Visual/UserInterface/TestSceneModSelectScreen.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs index 7690029ba2..0bd138275b 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs @@ -104,17 +104,25 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("activate DT", () => getPanelForMod(typeof(OsuModDoubleTime)).TriggerClick()); AddAssert("DT active", () => SelectedMods.Value.Single().GetType() == typeof(OsuModDoubleTime)); + AddAssert("DT panel active", () => getPanelForMod(typeof(OsuModDoubleTime)).Active.Value); AddStep("activate NC", () => getPanelForMod(typeof(OsuModNightcore)).TriggerClick()); AddAssert("only NC active", () => SelectedMods.Value.Single().GetType() == typeof(OsuModNightcore)); + AddAssert("DT panel not active", () => !getPanelForMod(typeof(OsuModDoubleTime)).Active.Value); + AddAssert("NC panel active", () => getPanelForMod(typeof(OsuModNightcore)).Active.Value); AddStep("activate HR", () => getPanelForMod(typeof(OsuModHardRock)).TriggerClick()); AddAssert("NC+HR active", () => SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModNightcore)) && SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModHardRock))); + AddAssert("NC panel active", () => getPanelForMod(typeof(OsuModNightcore)).Active.Value); + AddAssert("HR panel active", () => getPanelForMod(typeof(OsuModHardRock)).Active.Value); AddStep("activate MR", () => getPanelForMod(typeof(OsuModMirror)).TriggerClick()); AddAssert("NC+MR active", () => SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModNightcore)) && SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModMirror))); + AddAssert("NC panel active", () => getPanelForMod(typeof(OsuModNightcore)).Active.Value); + AddAssert("HR panel not active", () => !getPanelForMod(typeof(OsuModHardRock)).Active.Value); + AddAssert("MR panel active", () => getPanelForMod(typeof(OsuModMirror)).Active.Value); } [Test] From f5fa41356e33a528039742f9909d45dff71e451e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 3 May 2022 21:44:44 +0200 Subject: [PATCH 09/29] Rewrite mod instance management again to pass tests --- .../UserInterface/TestSceneModSelectScreen.cs | 4 +- osu.Game/Overlays/Mods/ModColumn.cs | 129 ++++++++++++------ osu.Game/Overlays/Mods/ModSelectScreen.cs | 33 ++--- osu.Game/Overlays/Mods/UserModSelectScreen.cs | 7 +- 4 files changed, 111 insertions(+), 62 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs index 0bd138275b..bdb423a43c 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectScreen.cs @@ -336,8 +336,8 @@ namespace osu.Game.Tests.Visual.UserInterface AddAssert("mod instance doesn't match", () => external != overlayButtonMod); AddAssert("one mod present in global selected", () => SelectedMods.Value.Count == 1); - AddAssert("globally selected matches button's mod instance", () => SelectedMods.Value.Contains(overlayButtonMod)); - AddAssert("globally selected doesn't contain original external change", () => !SelectedMods.Value.Contains(external)); + AddAssert("globally selected matches button's mod instance", () => SelectedMods.Value.Any(mod => ReferenceEquals(mod, overlayButtonMod))); + AddAssert("globally selected doesn't contain original external change", () => !SelectedMods.Value.Any(mod => ReferenceEquals(mod, external))); } [Test] diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index d480382fcb..f9332dd28b 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -53,9 +53,22 @@ namespace osu.Game.Overlays.Mods } } - public Bindable> SelectedMods = new Bindable>(Array.Empty()); public Bindable Active = new BindableBool(true); + /// + /// List of mods marked as selected in this column. + /// + /// + /// Note that the mod instances returned by this property are owned solely by this column + /// (as in, they are locally-managed clones, to ensure proper isolation from any other external instances). + /// + public IReadOnlyList SelectedMods { get; private set; } = Array.Empty(); + + /// + /// Invoked when a mod panel has been selected interactively by the user. + /// + public event Action? SelectionChangedByUser; + protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => base.ReceivePositionalInputAtSubTree(screenSpacePos) && Active.Value; protected virtual ModPanel CreateModPanel(Mod mod) => new ModPanel(mod); @@ -64,6 +77,15 @@ namespace osu.Game.Overlays.Mods private readonly Bindable>> availableMods = new Bindable>>(); + /// + /// All mods that are available for the current ruleset in this particular column. + /// + /// + /// Note that the mod instances in this list are owned solely by this column + /// (as in, they are locally-managed clones, to ensure proper isolation from any other external instances). + /// + private IReadOnlyList localAvailableMods = Array.Empty(); + private readonly TextFlowContainer headerText; private readonly Box headerBackground; private readonly Container contentContainer; @@ -227,6 +249,9 @@ namespace osu.Game.Overlays.Mods private void load(OsuGameBase game, OverlayColourProvider colourProvider, OsuColour colours) { availableMods.BindTo(game.AvailableMods); + // this `BindValueChanged` callback is intentionally here, to ensure that local available mods are constructed as early as possible. + // this is needed to make sure no external changes to mods are dropped while mod panels are asynchronously loading. + availableMods.BindValueChanged(_ => updateLocalAvailableMods(), true); headerBackground.Colour = accentColour = colours.ForModType(ModType); @@ -240,33 +265,26 @@ namespace osu.Game.Overlays.Mods contentBackground.Colour = colourProvider.Background4; } - protected override void LoadComplete() - { - base.LoadComplete(); - availableMods.BindValueChanged(_ => Scheduler.AddOnce(updateMods)); - SelectedMods.BindValueChanged(_ => - { - // if a load is in progress, don't try to update the selection - the load flow will do so. - if (latestLoadTask == null) - updateActiveState(); - }); - updateMods(); - } - - private CancellationTokenSource? cancellationTokenSource; - - private void updateMods() + private void updateLocalAvailableMods() { var newMods = ModUtils.FlattenMods(availableMods.Value.GetValueOrDefault(ModType) ?? Array.Empty()) .Select(m => m.DeepClone()) .ToList(); - if (newMods.SequenceEqual(panelFlow.Children.Select(p => p.Mod))) + if (newMods.SequenceEqual(localAvailableMods)) return; + localAvailableMods = newMods; + Scheduler.AddOnce(loadPanels); + } + + private CancellationTokenSource? cancellationTokenSource; + + private void loadPanels() + { cancellationTokenSource?.Cancel(); - var panels = newMods.Select(mod => CreateModPanel(mod).With(panel => panel.Shear = new Vector2(-ShearedOverlayContainer.SHEAR, 0))); + var panels = localAvailableMods.Select(mod => CreateModPanel(mod).With(panel => panel.Shear = new Vector2(-ShearedOverlayContainer.SHEAR, 0))); Task? loadTask; @@ -280,20 +298,7 @@ namespace osu.Game.Overlays.Mods foreach (var panel in panelFlow) { - panel.Active.BindValueChanged(_ => - { - updateToggleAllState(); - - var newSelectedMods = SelectedMods.Value; - - var matchingModInstance = SelectedMods.Value.SingleOrDefault(selected => selected.GetType() == panel.Mod.GetType()); - if (matchingModInstance != null && (matchingModInstance != panel.Mod || !panel.Active.Value)) - newSelectedMods = newSelectedMods.Except(matchingModInstance.Yield()).ToArray(); - if (panel.Active.Value) - newSelectedMods = newSelectedMods.Append(panel.Mod).ToArray(); - - SelectedMods.Value = newSelectedMods.ToArray(); - }); + panel.Active.BindValueChanged(_ => panelStateChanged(panel)); } }, (cancellationTokenSource = new CancellationTokenSource()).Token); loadTask.ContinueWith(_ => @@ -306,14 +311,62 @@ namespace osu.Game.Overlays.Mods private void updateActiveState() { foreach (var panel in panelFlow) + panel.Active.Value = SelectedMods.Contains(panel.Mod); + } + + /// + /// This flag helps to determine the source of changes to . + /// If the value is false, then are changing due to a user selection on the UI. + /// If the value is true, then are changing due to an external call. + /// + private bool externalSelectionUpdateInProgress; + + private void panelStateChanged(ModPanel panel) + { + updateToggleAllState(); + + var newSelectedMods = panel.Active.Value + ? SelectedMods.Append(panel.Mod) + : SelectedMods.Except(panel.Mod.Yield()); + + SelectedMods = newSelectedMods.ToArray(); + if (!externalSelectionUpdateInProgress) + SelectionChangedByUser?.Invoke(); + } + + /// + /// Adjusts the set of selected mods in this column to match the passed in . + /// + /// + /// This method exists to be able to receive mod instances that come from potentially-external sources and to copy the changes across to this column's state. + /// uses this to substitute any external mod references in + /// to references that are owned by this column. + /// + internal void SetSelection(IReadOnlyList mods) + { + externalSelectionUpdateInProgress = true; + + var newSelection = new List(); + + foreach (var mod in localAvailableMods) { - var matchingSelectedMod = SelectedMods.Value.SingleOrDefault(selected => selected.GetType() == panel.Mod.GetType()); - panel.Active.Value = matchingSelectedMod != null; - if (panel.Active.Value) - panel.Mod.CopyFrom(matchingSelectedMod); + var matchingSelectedMod = mods.SingleOrDefault(selected => selected.GetType() == mod.GetType()); + + if (matchingSelectedMod != null) + { + mod.CopyFrom(matchingSelectedMod); + newSelection.Add(mod); + } else - panel.Mod.ResetSettingsToDefaults(); + { + mod.ResetSettingsToDefaults(); + } } + + SelectedMods = newSelection; + updateActiveState(); + + externalSelectionUpdateInProgress = false; } #region Bulk select / deselect diff --git a/osu.Game/Overlays/Mods/ModSelectScreen.cs b/osu.Game/Overlays/Mods/ModSelectScreen.cs index 2f6eae0afb..bb12143d7c 100644 --- a/osu.Game/Overlays/Mods/ModSelectScreen.cs +++ b/osu.Game/Overlays/Mods/ModSelectScreen.cs @@ -179,7 +179,7 @@ namespace osu.Game.Overlays.Mods foreach (var column in columnFlow.Columns) { - column.SelectedMods.BindValueChanged(updateBindableFromSelection); + column.SelectionChangedByUser += updateBindableFromSelection; } customisationVisible.BindValueChanged(_ => updateCustomisationVisualState(), true); @@ -250,33 +250,26 @@ namespace osu.Game.Overlays.Mods private void updateSelectionFromBindable() { - // note that selectionBindableSyncInProgress is purposefully not checked here. - // this is because in the case of mod selection in solo gameplay, a user selection of a mod can actually lead to deselection of other incompatible mods. - // to synchronise state correctly, updateBindableFromSelection() computes the final mods (including incompatibility rules) and updates SelectedMods, - // and this method then runs unconditionally again to make sure the new visual selection accurately reflects the final set of selected mods. - // selectionBindableSyncInProgress ensures that mutual infinite recursion does not happen after that unconditional call. + // `SelectedMods` may contain mod references that come from external sources. + // to ensure isolation, first pull in the potentially-external change into the mod columns... foreach (var column in columnFlow.Columns) - column.SelectedMods.Value = SelectedMods.Value.Where(mod => mod.Type == column.ModType).ToArray(); + column.SetSelection(SelectedMods.Value); + + // and then, when done, replace the potentially-external mod references in `SelectedMods` with ones we own. + updateBindableFromSelection(); } - private bool selectionBindableSyncInProgress; - - private void updateBindableFromSelection(ValueChangedEvent> modSelectionChange) + private void updateBindableFromSelection() { - if (selectionBindableSyncInProgress) + var candidateSelection = columnFlow.Columns.SelectMany(column => column.SelectedMods).ToArray(); + + if (candidateSelection.SequenceEqual(SelectedMods.Value)) return; - selectionBindableSyncInProgress = true; - - SelectedMods.Value = ComputeNewModsFromSelection( - modSelectionChange.NewValue.Except(modSelectionChange.OldValue), - modSelectionChange.OldValue.Except(modSelectionChange.NewValue)); - - selectionBindableSyncInProgress = false; + SelectedMods.Value = ComputeNewModsFromSelection(SelectedMods.Value, candidateSelection); } - protected virtual IReadOnlyList ComputeNewModsFromSelection(IEnumerable addedMods, IEnumerable removedMods) - => columnFlow.Columns.SelectMany(column => column.SelectedMods.Value).ToArray(); + protected virtual IReadOnlyList ComputeNewModsFromSelection(IReadOnlyList oldSelection, IReadOnlyList newSelection) => newSelection; protected override void PopIn() { diff --git a/osu.Game/Overlays/Mods/UserModSelectScreen.cs b/osu.Game/Overlays/Mods/UserModSelectScreen.cs index ed0a07521b..ca33d35605 100644 --- a/osu.Game/Overlays/Mods/UserModSelectScreen.cs +++ b/osu.Game/Overlays/Mods/UserModSelectScreen.cs @@ -14,9 +14,12 @@ namespace osu.Game.Overlays.Mods { protected override ModColumn CreateModColumn(ModType modType, Key[] toggleKeys = null) => new UserModColumn(modType, false, toggleKeys); - protected override IReadOnlyList ComputeNewModsFromSelection(IEnumerable addedMods, IEnumerable removedMods) + protected override IReadOnlyList ComputeNewModsFromSelection(IReadOnlyList oldSelection, IReadOnlyList newSelection) { - IEnumerable modsAfterRemoval = SelectedMods.Value.Except(removedMods).ToList(); + var addedMods = newSelection.Except(oldSelection); + var removedMods = oldSelection.Except(newSelection); + + IEnumerable modsAfterRemoval = newSelection.Except(removedMods).ToList(); // the preference is that all new mods should override potential incompatible old mods. // in general that's a bit difficult to compute if more than one mod is added at a time, From d52a1a5d2324a5d0041fe7d7e6e548e56f907aa2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 4 May 2022 03:52:10 +0300 Subject: [PATCH 10/29] Add key binding for beatmap selection in song select --- .../Input/Bindings/GlobalActionContainer.cs | 10 +++++++- .../GlobalActionKeyBindingStrings.cs | 10 ++++++++ osu.Game/Screens/Select/BeatmapCarousel.cs | 24 ++++--------------- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index 47cb7be2cf..ec368a6fda 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -100,10 +100,12 @@ namespace osu.Game.Input.Bindings public IEnumerable SongSelectKeyBindings => new[] { + new KeyBinding(InputKey.Left, GlobalAction.SelectPreviousBeatmap), + new KeyBinding(InputKey.Right, GlobalAction.SelectNextBeatmap), new KeyBinding(InputKey.F1, GlobalAction.ToggleModSelection), new KeyBinding(InputKey.F2, GlobalAction.SelectNextRandom), new KeyBinding(new[] { InputKey.Shift, InputKey.F2 }, GlobalAction.SelectPreviousRandom), - new KeyBinding(InputKey.F3, GlobalAction.ToggleBeatmapOptions) + new KeyBinding(InputKey.F3, GlobalAction.ToggleBeatmapOptions), }; public IEnumerable AudioControlKeyBindings => new[] @@ -301,5 +303,11 @@ namespace osu.Game.Input.Bindings [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.EditorFlipVertically))] EditorFlipVertically, + + [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectPreviousBeatmap))] + SelectPreviousBeatmap, + + [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectNextBeatmap))] + SelectNextBeatmap, } } diff --git a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs index 777e97d1e3..08e13e24ae 100644 --- a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs +++ b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs @@ -194,6 +194,16 @@ namespace osu.Game.Localisation /// public static LocalisableString ToggleInGameInterface => new TranslatableString(getKey(@"toggle_in_game_interface"), @"Toggle in-game interface"); + /// + /// "Previous beatmap selection" + /// + public static LocalisableString SelectPreviousBeatmap => new TranslatableString(getKey(@"select_previous_beatmap"), @"Previous beatmap selection"); + + /// + /// "Next beatmap selection" + /// + public static LocalisableString SelectNextBeatmap => new TranslatableString(getKey(@"select_next_beatmap"), @"Next beatmap selection"); + /// /// "Toggle Mod Select" /// diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index c3d340ac61..85b5af049e 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -604,34 +604,20 @@ namespace osu.Game.Screens.Select public void ScrollToSelected(bool immediate = false) => pendingScrollOperation = immediate ? PendingScrollOperation.Immediate : PendingScrollOperation.Standard; - #region Key / button selection logic - - protected override bool OnKeyDown(KeyDownEvent e) - { - switch (e.Key) - { - case Key.Left: - SelectNext(-1); - return true; - - case Key.Right: - SelectNext(); - return true; - } - - return false; - } + #region Button selection logic public bool OnPressed(KeyBindingPressEvent e) { switch (e.Action) { case GlobalAction.SelectNext: - SelectNext(1, false); + case GlobalAction.SelectNextBeatmap: + SelectNext(1, e.Action == GlobalAction.SelectNextBeatmap); return true; case GlobalAction.SelectPrevious: - SelectNext(-1, false); + case GlobalAction.SelectPreviousBeatmap: + SelectNext(-1, e.Action == GlobalAction.SelectPreviousBeatmap); return true; } From a66743266f6460dc7c50ad873e2b231a813a8d0b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 May 2022 17:20:58 +0900 Subject: [PATCH 11/29] Remove unused `ScrollingToolboxGroup` class --- .../Rulesets/Edit/ScrollingToolboxGroup.cs | 34 ------------------- 1 file changed, 34 deletions(-) delete mode 100644 osu.Game/Rulesets/Edit/ScrollingToolboxGroup.cs diff --git a/osu.Game/Rulesets/Edit/ScrollingToolboxGroup.cs b/osu.Game/Rulesets/Edit/ScrollingToolboxGroup.cs deleted file mode 100644 index 98e026c49a..0000000000 --- a/osu.Game/Rulesets/Edit/ScrollingToolboxGroup.cs +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Game.Graphics.Containers; - -namespace osu.Game.Rulesets.Edit -{ - public class ScrollingToolboxGroup : EditorToolboxGroup - { - protected readonly OsuScrollContainer Scroll; - - protected readonly FillFlowContainer FillFlow; - - protected override Container Content { get; } - - public ScrollingToolboxGroup(string title, float scrollAreaHeight) - : base(title) - { - base.Content.Add(Scroll = new OsuScrollContainer - { - RelativeSizeAxes = Axes.X, - Height = scrollAreaHeight, - Child = Content = FillFlow = new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - }, - }); - } - } -} From b325f0ee0b3ebc286f13ba82d95e590707ece455 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 May 2022 17:41:29 +0900 Subject: [PATCH 12/29] Combine editor toolbox container implementation and fix input blocking Until now, toolbox scroll areas would block input from arriving behind them, even when no visible element was clicked. In addition, clicking on a button inside a toolbox would still send a `MouseDown` event to things behind it. Specifically, the editor's `HitObjectComposer` would receive these events and also place objects when the user does not expect them to be placed. This fixes another regression that occurred due to `ScrollContainer`s no longer blocking input theirselves. --- .../Edit/DistancedHitObjectComposer.cs | 19 ++--------- .../Edit/ExpandingToolboxContainer.cs | 34 +++++++++++++++++++ osu.Game/Rulesets/Edit/HitObjectComposer.cs | 16 ++------- 3 files changed, 38 insertions(+), 31 deletions(-) create mode 100644 osu.Game/Rulesets/Edit/ExpandingToolboxContainer.cs diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs index 66a78640c6..370e8ccbda 100644 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs @@ -6,12 +6,10 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; -using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Input.Bindings; using osu.Game.Overlays.Settings.Sections; using osu.Game.Rulesets.Objects; -using osuTK; namespace osu.Game.Rulesets.Edit { @@ -44,8 +42,9 @@ namespace osu.Game.Rulesets.Edit [BackgroundDependencyLoader] private void load() { - AddInternal(RightSideToolboxContainer = new ExpandingToolboxContainer + AddInternal(RightSideToolboxContainer = new ExpandingToolboxContainer(130, 250) { + Padding = new MarginPadding { Right = 10 }, Alpha = DistanceSpacingMultiplier.Disabled ? 0 : 1, Anchor = Anchor.TopRight, Origin = Anchor.TopRight, @@ -154,19 +153,5 @@ namespace osu.Game.Rulesets.Edit return DurationToDistance(referenceObject, snappedEndTime - startTime); } - - protected class ExpandingToolboxContainer : ExpandingContainer - { - protected override double HoverExpansionDelay => 250; - - public ExpandingToolboxContainer() - : base(130, 250) - { - RelativeSizeAxes = Axes.Y; - Padding = new MarginPadding { Left = 10 }; - - FillFlow.Spacing = new Vector2(10); - } - } } } diff --git a/osu.Game/Rulesets/Edit/ExpandingToolboxContainer.cs b/osu.Game/Rulesets/Edit/ExpandingToolboxContainer.cs new file mode 100644 index 0000000000..e807dbd482 --- /dev/null +++ b/osu.Game/Rulesets/Edit/ExpandingToolboxContainer.cs @@ -0,0 +1,34 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using osu.Framework.Graphics; +using osu.Framework.Input.Events; +using osu.Game.Graphics.Containers; +using osuTK; + +namespace osu.Game.Rulesets.Edit +{ + public class ExpandingToolboxContainer : ExpandingContainer + { + protected override double HoverExpansionDelay => 250; + + public ExpandingToolboxContainer(float contractedWidth, float expandedWidth) + : base(contractedWidth, expandedWidth) + { + RelativeSizeAxes = Axes.Y; + + FillFlow.Spacing = new Vector2(10); + } + + protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => base.ReceivePositionalInputAtSubTree(screenSpacePos) && anyToolboxHovered(screenSpacePos); + + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) && anyToolboxHovered(screenSpacePos); + + private bool anyToolboxHovered(Vector2 screenSpacePos) => FillFlow.Children.Any(d => d.ScreenSpaceDrawQuad.Contains(screenSpacePos)); + + protected override bool OnMouseDown(MouseDownEvent e) => true; + + protected override bool OnClick(ClickEvent e) => true; + } +} diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index a235a5bc60..1c388fe68f 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -13,7 +13,6 @@ using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Framework.Logging; using osu.Game.Beatmaps; -using osu.Game.Graphics.Containers; using osu.Game.Rulesets.Configuration; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mods; @@ -115,8 +114,9 @@ namespace osu.Game.Rulesets.Edit .WithChild(BlueprintContainer = CreateBlueprintContainer()) } }, - new LeftToolboxFlow + new ExpandingToolboxContainer(80, 200) { + Padding = new MarginPadding { Left = 10 }, Children = new Drawable[] { new EditorToolboxGroup("toolbox (1-9)") @@ -382,18 +382,6 @@ namespace osu.Game.Rulesets.Edit } #endregion - - private class LeftToolboxFlow : ExpandingButtonContainer - { - public LeftToolboxFlow() - : base(80, 200) - { - RelativeSizeAxes = Axes.Y; - Padding = new MarginPadding { Right = 10 }; - - FillFlow.Spacing = new Vector2(10); - } - } } /// From f899c3e68fa5ad4eb53196de2dd509fd3d9d46b8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 May 2022 17:56:38 +0900 Subject: [PATCH 13/29] Add test coverage of circle placement around editor toolboxes --- .../Editing/TestSceneHitObjectComposer.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs index ae1b691767..231a1f408f 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs @@ -19,6 +19,7 @@ using osu.Game.Rulesets.Osu.Edit; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Screens.Edit; using osu.Game.Screens.Edit.Components.RadioButtons; +using osu.Game.Screens.Edit.Components.TernaryButtons; using osu.Game.Screens.Edit.Compose.Components; using osuTK; using osuTK.Input; @@ -87,6 +88,65 @@ namespace osu.Game.Tests.Visual.Editing AddAssert("Tool changed", () => hitObjectComposer.ChildrenOfType().First().CurrentTool is HitCircleCompositionTool); } + [Test] + public void TestPlacementFailsWhenClickingButton() + { + AddStep("clear all control points and hitobjects", () => + { + editorBeatmap.ControlPointInfo.Clear(); + editorBeatmap.Clear(); + }); + + AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); + + AddStep("Change to hitcircle", () => hitObjectComposer.ChildrenOfType().First(d => d.Button.Label == "HitCircle").TriggerClick()); + + AddStep("move mouse to overlapping toggle button", () => + { + var playfield = hitObjectComposer.Playfield.ScreenSpaceDrawQuad; + var button = hitObjectComposer + .ChildrenOfType().First() + .ChildrenOfType().First(b => playfield.Contains(b.ScreenSpaceDrawQuad.Centre)); + + InputManager.MoveMouseTo(button); + }); + + AddAssert("no circles placed", () => editorBeatmap.HitObjects.Count == 0); + + AddStep("attempt place circle", () => InputManager.Click(MouseButton.Left)); + + AddAssert("no circles placed", () => editorBeatmap.HitObjects.Count == 0); + } + + [Test] + public void TestPlacementWithinToolboxScrollArea() + { + AddStep("clear all control points and hitobjects", () => + { + editorBeatmap.ControlPointInfo.Clear(); + editorBeatmap.Clear(); + }); + + AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); + + AddStep("Change to hitcircle", () => hitObjectComposer.ChildrenOfType().First(d => d.Button.Label == "HitCircle").TriggerClick()); + + AddStep("move mouse to scroll area", () => + { + // Specifically wanting to test the area of overlap between the left expanding toolbox container + // and the playfield/composer. + var scrollArea = hitObjectComposer.ChildrenOfType().First().ScreenSpaceDrawQuad; + var playfield = hitObjectComposer.Playfield.ScreenSpaceDrawQuad; + InputManager.MoveMouseTo(new Vector2(scrollArea.TopLeft.X, playfield.Centre.Y)); + }); + + AddAssert("no circles placed", () => editorBeatmap.HitObjects.Count == 0); + + AddStep("place circle", () => InputManager.Click(MouseButton.Left)); + + AddAssert("circle placed", () => editorBeatmap.HitObjects.Count == 1); + } + [Test] public void TestDistanceSpacingHotkeys() { From a759e641ebf17f71cbd825491fb34fc95045f382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 4 May 2022 11:56:29 +0200 Subject: [PATCH 14/29] Enforce composer aspect ratio to avoid depending on ambient window size --- osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs index 231a1f408f..fb179a448c 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs @@ -71,6 +71,11 @@ namespace osu.Game.Tests.Visual.Editing Child = editorBeatmapContainer = new EditorBeatmapContainer(Beatmap.Value) { Child = hitObjectComposer = new OsuHitObjectComposer(new OsuRuleset()) + { + // force the composer to fully overlap the playfield area by setting a 4:3 aspect ratio. + FillMode = FillMode.Fit, + FillAspectRatio = 4 / 3f + } }; }); } From 6380216263c0cff2c46417322362da11631aa63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 4 May 2022 11:56:44 +0200 Subject: [PATCH 15/29] Nudge click location to avoid placement failures --- osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs index fb179a448c..c9d44fdab7 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs @@ -142,7 +142,7 @@ namespace osu.Game.Tests.Visual.Editing // and the playfield/composer. var scrollArea = hitObjectComposer.ChildrenOfType().First().ScreenSpaceDrawQuad; var playfield = hitObjectComposer.Playfield.ScreenSpaceDrawQuad; - InputManager.MoveMouseTo(new Vector2(scrollArea.TopLeft.X, playfield.Centre.Y)); + InputManager.MoveMouseTo(new Vector2(scrollArea.TopLeft.X + 1, playfield.Centre.Y)); }); AddAssert("no circles placed", () => editorBeatmap.HitObjects.Count == 0); From 3eead5a6a39c3add1966990f280d655c2d71f3c2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 May 2022 19:40:08 +0900 Subject: [PATCH 16/29] Rename `FlushAnimation` to `FlushPendingSelections` to better match purpose --- osu.Game/Overlays/Mods/ModColumn.cs | 4 ++-- osu.Game/Overlays/Mods/ModSection.cs | 4 ++-- osu.Game/Overlays/Mods/ModSelectOverlay.cs | 2 +- osu.Game/Overlays/Mods/ModSelectScreen.cs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModColumn.cs b/osu.Game/Overlays/Mods/ModColumn.cs index f9332dd28b..a792c0a81e 100644 --- a/osu.Game/Overlays/Mods/ModColumn.cs +++ b/osu.Game/Overlays/Mods/ModColumn.cs @@ -435,9 +435,9 @@ namespace osu.Game.Overlays.Mods } /// - /// Play out all remaining animations immediately to leave mods in a good (final) state. + /// Run any delayed selections (due to animation) immediately to leave mods in a good (final) state. /// - public void FlushAnimation() + public void FlushPendingSelections() { while (pendingSelectionOperations.TryDequeue(out var dequeuedAction)) dequeuedAction(); diff --git a/osu.Game/Overlays/Mods/ModSection.cs b/osu.Game/Overlays/Mods/ModSection.cs index 5bf8cddd0c..a70191a864 100644 --- a/osu.Game/Overlays/Mods/ModSection.cs +++ b/osu.Game/Overlays/Mods/ModSection.cs @@ -250,9 +250,9 @@ namespace osu.Game.Overlays.Mods protected virtual ModButton CreateModButton(Mod mod) => new ModButton(mod); /// - /// Play out all remaining animations immediately to leave mods in a good (final) state. + /// Run any delayed selections (due to animation) immediately to leave mods in a good (final) state. /// - public void FlushAnimation() + public void FlushPendingSelections() { while (pendingSelectionOperations.TryDequeue(out var dequeuedAction)) dequeuedAction(); diff --git a/osu.Game/Overlays/Mods/ModSelectOverlay.cs b/osu.Game/Overlays/Mods/ModSelectOverlay.cs index 9ce79c25f7..cf57322594 100644 --- a/osu.Game/Overlays/Mods/ModSelectOverlay.cs +++ b/osu.Game/Overlays/Mods/ModSelectOverlay.cs @@ -369,7 +369,7 @@ namespace osu.Game.Overlays.Mods foreach (var section in ModSectionsContainer) { - section.FlushAnimation(); + section.FlushPendingSelections(); } FooterContainer.MoveToX(content_width, WaveContainer.DISAPPEAR_DURATION, Easing.InSine); diff --git a/osu.Game/Overlays/Mods/ModSelectScreen.cs b/osu.Game/Overlays/Mods/ModSelectScreen.cs index bb12143d7c..8060bca65f 100644 --- a/osu.Game/Overlays/Mods/ModSelectScreen.cs +++ b/osu.Game/Overlays/Mods/ModSelectScreen.cs @@ -308,7 +308,7 @@ namespace osu.Game.Overlays.Mods var column = columnFlow[i].Column; - column.FlushAnimation(); + column.FlushPendingSelections(); column.TopLevelContent .MoveToY(i % 2 == 0 ? -distance : distance, fade_out_duration, Easing.OutQuint) .FadeOut(fade_out_duration, Easing.OutQuint); From 9416346c942ad20baa7b8000ebcb0a028a8c796e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 4 May 2022 16:46:23 +0300 Subject: [PATCH 17/29] Globalise beatmap selection key bindings as "group" selection --- .../Input/Bindings/GlobalActionContainer.cs | 13 ++++++------ .../GlobalActionKeyBindingStrings.cs | 20 +++++++++---------- osu.Game/Overlays/Volume/VolumeMeter.cs | 4 ++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 8 ++++---- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index ec368a6fda..9cb8f2dfcc 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -59,6 +59,9 @@ namespace osu.Game.Input.Bindings new KeyBinding(InputKey.Up, GlobalAction.SelectPrevious), new KeyBinding(InputKey.Down, GlobalAction.SelectNext), + new KeyBinding(InputKey.Left, GlobalAction.SelectPreviousGroup), + new KeyBinding(InputKey.Right, GlobalAction.SelectNextGroup), + new KeyBinding(InputKey.Space, GlobalAction.Select), new KeyBinding(InputKey.Enter, GlobalAction.Select), new KeyBinding(InputKey.KeypadEnter, GlobalAction.Select), @@ -100,8 +103,6 @@ namespace osu.Game.Input.Bindings public IEnumerable SongSelectKeyBindings => new[] { - new KeyBinding(InputKey.Left, GlobalAction.SelectPreviousBeatmap), - new KeyBinding(InputKey.Right, GlobalAction.SelectNextBeatmap), new KeyBinding(InputKey.F1, GlobalAction.ToggleModSelection), new KeyBinding(InputKey.F2, GlobalAction.SelectNextRandom), new KeyBinding(new[] { InputKey.Shift, InputKey.F2 }, GlobalAction.SelectPreviousRandom), @@ -304,10 +305,10 @@ namespace osu.Game.Input.Bindings [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.EditorFlipVertically))] EditorFlipVertically, - [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectPreviousBeatmap))] - SelectPreviousBeatmap, + [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectPreviousGroup))] + SelectPreviousGroup, - [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectNextBeatmap))] - SelectNextBeatmap, + [LocalisableDescription(typeof(GlobalActionKeyBindingStrings), nameof(GlobalActionKeyBindingStrings.SelectNextGroup))] + SelectNextGroup, } } diff --git a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs index 08e13e24ae..129706261b 100644 --- a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs +++ b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs @@ -129,6 +129,16 @@ namespace osu.Game.Localisation /// public static LocalisableString SelectNext => new TranslatableString(getKey(@"select_next"), @"Next selection"); + /// + /// "Previous group selection" + /// + public static LocalisableString SelectPreviousGroup => new TranslatableString(getKey(@"select_previous_group"), @"Previous group selection"); + + /// + /// "Next group selection" + /// + public static LocalisableString SelectNextGroup => new TranslatableString(getKey(@"select_next_group"), @"Next group selection"); + /// /// "Home" /// @@ -194,16 +204,6 @@ namespace osu.Game.Localisation /// public static LocalisableString ToggleInGameInterface => new TranslatableString(getKey(@"toggle_in_game_interface"), @"Toggle in-game interface"); - /// - /// "Previous beatmap selection" - /// - public static LocalisableString SelectPreviousBeatmap => new TranslatableString(getKey(@"select_previous_beatmap"), @"Previous beatmap selection"); - - /// - /// "Next beatmap selection" - /// - public static LocalisableString SelectNextBeatmap => new TranslatableString(getKey(@"select_next_beatmap"), @"Next beatmap selection"); - /// /// "Toggle Mod Select" /// diff --git a/osu.Game/Overlays/Volume/VolumeMeter.cs b/osu.Game/Overlays/Volume/VolumeMeter.cs index e2afd46c18..46b8b35da2 100644 --- a/osu.Game/Overlays/Volume/VolumeMeter.cs +++ b/osu.Game/Overlays/Volume/VolumeMeter.cs @@ -372,12 +372,12 @@ namespace osu.Game.Overlays.Volume switch (e.Action) { - case GlobalAction.SelectPrevious: + case GlobalAction.SelectPreviousGroup: State = SelectionState.Selected; adjust(1, false); return true; - case GlobalAction.SelectNext: + case GlobalAction.SelectNextGroup: State = SelectionState.Selected; adjust(-1, false); return true; diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 85b5af049e..a59f14647d 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -611,13 +611,13 @@ namespace osu.Game.Screens.Select switch (e.Action) { case GlobalAction.SelectNext: - case GlobalAction.SelectNextBeatmap: - SelectNext(1, e.Action == GlobalAction.SelectNextBeatmap); + case GlobalAction.SelectNextGroup: + SelectNext(1, e.Action == GlobalAction.SelectNextGroup); return true; case GlobalAction.SelectPrevious: - case GlobalAction.SelectPreviousBeatmap: - SelectNext(-1, e.Action == GlobalAction.SelectPreviousBeatmap); + case GlobalAction.SelectPreviousGroup: + SelectNext(-1, e.Action == GlobalAction.SelectPreviousGroup); return true; } From 81b462262096d38d864be7e1b1bf22cc13e75eb7 Mon Sep 17 00:00:00 2001 From: Supersonicboss1 Date: Wed, 4 May 2022 22:25:34 +0100 Subject: [PATCH 18/29] fixed autoplay not showing compat, + relax compat --- osu.Game.Rulesets.Osu/Mods/OsuModAlternate.cs | 2 +- osu.Game.Rulesets.Osu/Mods/OsuModAutoplay.cs | 2 +- osu.Game.Rulesets.Osu/Mods/OsuModCinema.cs | 2 +- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModAlternate.cs b/osu.Game.Rulesets.Osu/Mods/OsuModAlternate.cs index dfa28a537a..0832cfb545 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModAlternate.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModAlternate.cs @@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Osu.Mods public override string Acronym => @"AL"; public override string Description => @"Don't use the same key twice in a row!"; public override double ScoreMultiplier => 1.0; - public override Type[] IncompatibleMods => new[] { typeof(ModAutoplay) }; + public override Type[] IncompatibleMods => new[] { typeof(ModAutoplay), typeof(ModRelax) }; public override ModType Type => ModType.Conversion; public override IconUsage? Icon => FontAwesome.Solid.Keyboard; diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModAutoplay.cs b/osu.Game.Rulesets.Osu/Mods/OsuModAutoplay.cs index b31ef5d2fd..507b3588bd 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModAutoplay.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModAutoplay.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Osu.Mods { public class OsuModAutoplay : ModAutoplay { - public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModMagnetised), typeof(OsuModAutopilot), typeof(OsuModSpunOut) }).ToArray(); + public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModMagnetised), typeof(OsuModAutopilot), typeof(OsuModSpunOut), typeof(OsuModAlternate) }).ToArray(); public override ModReplayData CreateReplayData(IBeatmap beatmap, IReadOnlyList mods) => new ModReplayData(new OsuAutoGenerator(beatmap, mods).Generate(), new ModCreatedUser { Username = "Autoplay" }); diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModCinema.cs b/osu.Game.Rulesets.Osu/Mods/OsuModCinema.cs index 5b42772358..99d7535957 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModCinema.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModCinema.cs @@ -13,7 +13,7 @@ namespace osu.Game.Rulesets.Osu.Mods { public class OsuModCinema : ModCinema { - public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModMagnetised), typeof(OsuModAutopilot), typeof(OsuModSpunOut) }).ToArray(); + public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModMagnetised), typeof(OsuModAutopilot), typeof(OsuModSpunOut), typeof(OsuModAlternate) }).ToArray(); public override ModReplayData CreateReplayData(IBeatmap beatmap, IReadOnlyList mods) => new ModReplayData(new OsuAutoGenerator(beatmap, mods).Generate(), new ModCreatedUser { Username = "Autoplay" }); diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 6b81efdca6..5f37c6a0ae 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.Osu.Mods public class OsuModRelax : ModRelax, IUpdatableByPlayfield, IApplicableToDrawableRuleset, IApplicableToPlayer { public override string Description => @"You don't need to click. Give your clicking/tapping fingers a break from the heat of things."; - public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModAutopilot), typeof(OsuModMagnetised) }).ToArray(); + public override Type[] IncompatibleMods => base.IncompatibleMods.Concat(new[] { typeof(OsuModAutopilot), typeof(OsuModMagnetised), typeof(OsuModAlternate) }).ToArray(); /// /// How early before a hitobject's start time to trigger a hit. From cbc58c67bfd32b0631089afd0b7ee89c0ca01ea3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 15:16:01 +0900 Subject: [PATCH 19/29] Remove weird strict tracking icon and reword description to explain what the mod does --- osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs index ab45e5192d..22ace52c2a 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs @@ -4,7 +4,6 @@ using System; using System.Linq; using System.Threading; -using osu.Framework.Graphics.Sprites; using osu.Game.Beatmaps; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Mods; @@ -23,9 +22,8 @@ namespace osu.Game.Rulesets.Osu.Mods { public override string Name => @"Strict Tracking"; public override string Acronym => @"ST"; - public override IconUsage? Icon => FontAwesome.Solid.PenFancy; public override ModType Type => ModType.DifficultyIncrease; - public override string Description => @"Follow circles just got serious..."; + public override string Description => @"Once you start a slider, follow precisely or get a miss."; public override double ScoreMultiplier => 1.0; public override Type[] IncompatibleMods => new[] { typeof(ModClassic), typeof(OsuModTarget) }; From f6fc926f1ade6fb5d1366edd4dd2f6632ec83913 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 15:58:21 +0900 Subject: [PATCH 20/29] Add xmldoc and rename methods in `IPositionSnapProvider` for legibility --- osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs | 4 ++-- .../Editor/TestSceneManiaBeatSnapGrid.cs | 4 ++-- osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs | 6 +++--- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 4 ++-- .../Sliders/Components/PathControlPointVisualiser.cs | 2 +- osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs | 8 ++++---- .../Visual/Editing/TestSceneDistanceSnapGrid.cs | 4 ++-- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 6 +++--- osu.Game/Rulesets/Edit/IPositionSnapProvider.cs | 10 +++++++--- .../Edit/Compose/Components/BlueprintContainer.cs | 4 ++-- .../Compose/Components/ComposeBlueprintContainer.cs | 2 +- .../Edit/Compose/Components/Timeline/Timeline.cs | 4 ++-- .../Components/Timeline/TimelineHitObjectBlueprint.cs | 2 +- 13 files changed, 32 insertions(+), 28 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index 9ff6d10a49..630a2cf645 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -89,9 +89,9 @@ namespace osu.Game.Rulesets.Catch.Edit new TernaryButton(distanceSnapToggle, "Distance Snap", () => new SpriteIcon { Icon = FontAwesome.Solid.Ruler }) }); - public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) { - var result = base.SnapScreenSpacePositionToValidTime(screenSpacePosition); + var result = base.FindSnappedPositionAndTime(screenSpacePosition); result.ScreenSpacePosition.X = screenSpacePosition.X; if (distanceSnapGrid.IsPresent && distanceSnapGrid.GetSnappedPosition(result.ScreenSpacePosition) is SnapResult snapResult && diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaBeatSnapGrid.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaBeatSnapGrid.cs index 2e55f86bb6..4bb049b1a4 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaBeatSnapGrid.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaBeatSnapGrid.cs @@ -97,12 +97,12 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor set => InternalChild = value; } - public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) { throw new System.NotImplementedException(); } - public override SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPosition(Vector2 screenSpacePosition) { throw new System.NotImplementedException(); } diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs index 2baec95c94..fef315e2ef 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs @@ -56,9 +56,9 @@ namespace osu.Game.Rulesets.Mania.Edit protected override Playfield PlayfieldAtScreenSpacePosition(Vector2 screenSpacePosition) => Playfield.GetColumnByPosition(screenSpacePosition); - public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) { - var result = base.SnapScreenSpacePositionToValidTime(screenSpacePosition); + var result = base.FindSnappedPositionAndTime(screenSpacePosition); switch (ScrollingInfo.Direction.Value) { @@ -112,7 +112,7 @@ namespace osu.Game.Rulesets.Mania.Edit } else { - var result = SnapScreenSpacePositionToValidTime(inputManager.CurrentState.Mouse.Position); + var result = FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); if (result.Time is double time) beatSnapGrid.SelectionTimeRange = (time, time); else diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 2ba30c5f74..27381d9d55 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -182,10 +182,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor private class SnapProvider : IDistanceSnapProvider { - public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) => + public SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, null); - public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); + public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); public IBindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 5de614722f..e71bde7357 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -255,7 +255,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { // Special handling for selections containing head control point - the position of the slider changes which means the snapped position and time have to be taken into account Vector2 newHeadPosition = Parent.ToScreenSpace(e.MousePosition + (dragStartPositions[0] - dragStartPositions[draggedControlPointIndex])); - var result = snapProvider?.SnapScreenSpacePositionToValidTime(newHeadPosition); + var result = snapProvider?.FindSnappedPositionAndTime(newHeadPosition); Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? newHeadPosition) - slider.Position; diff --git a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs index ba7c6e9d33..02beb0f2a4 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs @@ -123,7 +123,7 @@ namespace osu.Game.Rulesets.Osu.Edit } } - public override SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPosition(Vector2 screenSpacePosition) { if (snapToVisibleBlueprints(screenSpacePosition, out var snapResult)) return snapResult; @@ -131,9 +131,9 @@ namespace osu.Game.Rulesets.Osu.Edit return new SnapResult(screenSpacePosition, null); } - public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) { - var positionSnap = SnapScreenSpacePositionToValidPosition(screenSpacePosition); + var positionSnap = FindSnappedPosition(screenSpacePosition); if (positionSnap.ScreenSpacePosition != screenSpacePosition) return positionSnap; @@ -149,7 +149,7 @@ namespace osu.Game.Rulesets.Osu.Edit return new SnapResult(rectangularPositionSnapGrid.ToScreenSpace(pos), null, PlayfieldAtScreenSpacePosition(screenSpacePosition)); } - return base.SnapScreenSpacePositionToValidTime(screenSpacePosition); + return base.FindSnappedPositionAndTime(screenSpacePosition); } private bool snapToVisibleBlueprints(Vector2 screenSpacePosition, out SnapResult snapResult) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index b9cfa84a5d..1be4ac2eab 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -162,10 +162,10 @@ namespace osu.Game.Tests.Visual.Editing private class SnapProvider : IDistanceSnapProvider { - public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) => + public SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, null); - public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); + public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); public IBindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 1c388fe68f..e0cfd5c74a 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -362,7 +362,7 @@ namespace osu.Game.Rulesets.Edit /// The most relevant . protected virtual Playfield PlayfieldAtScreenSpacePosition(Vector2 screenSpacePosition) => drawableRulesetWrapper.Playfield; - public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) + public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) { var playfield = PlayfieldAtScreenSpacePosition(screenSpacePosition); double? targetTime = null; @@ -416,9 +416,9 @@ namespace osu.Game.Rulesets.Edit #region IPositionSnapProvider - public abstract SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition); + public abstract SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition); - public virtual SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) => + public virtual SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, null); #endregion diff --git a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs index 8a179ed424..8b6dbb6af3 100644 --- a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs @@ -5,23 +5,27 @@ using osuTK; namespace osu.Game.Rulesets.Edit { + /// + /// A snap provider which given the position of a hit object, offers a more correct position and time value inferred from the context of the beatmap. + /// Provided values are done so in an isolated context, without consideration of other nearby hit objects. + /// public interface IPositionSnapProvider { /// /// Given a position, find a valid time and position snap. /// /// - /// This call should be equivalent to running with any additional logic that can be performed without the time immutability restriction. + /// This call should be equivalent to running with any additional logic that can be performed without the time immutability restriction. /// /// The screen-space position to be snapped. /// The time and position post-snapping. - SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition); + SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition); /// /// Given a position, find a value position snap, restricting time to its input value. /// /// The screen-space position to be snapped. /// The position post-snapping. Time will always be null. - SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition); + SnapResult FindSnappedPosition(Vector2 screenSpacePosition); } } diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 6dc6f20cfe..186c66e0af 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -486,7 +486,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Vector2 originalPosition = movementBlueprintOriginalPositions[i]; var testPosition = originalPosition + distanceTravelled; - var positionalResult = snapProvider.SnapScreenSpacePositionToValidPosition(testPosition); + var positionalResult = snapProvider.FindSnappedPosition(testPosition); if (positionalResult.ScreenSpacePosition == testPosition) continue; @@ -505,7 +505,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Vector2 movePosition = movementBlueprintOriginalPositions.First() + distanceTravelled; // Retrieve a snapped position. - var result = snapProvider?.SnapScreenSpacePositionToValidTime(movePosition); + var result = snapProvider?.FindSnappedPositionAndTime(movePosition); if (result == null) { diff --git a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs index 79b38861ee..68be20720d 100644 --- a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs @@ -214,7 +214,7 @@ namespace osu.Game.Screens.Edit.Compose.Components private void updatePlacementPosition() { - var snapResult = Composer.SnapScreenSpacePositionToValidTime(inputManager.CurrentState.Mouse.Position); + var snapResult = Composer.FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); // if no time was found from positional snapping, we should still quantize to the beat. snapResult.Time ??= Beatmap.SnapTime(EditorClock.CurrentTime, null); diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs index c2b2bdb861..56ff972801 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs @@ -307,10 +307,10 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline /// public double VisibleRange => track.Length / Zoom; - public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) => + public SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, null); - public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) => + public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, beatSnapProvider.SnapTime(getTimeFromPosition(Content.ToLocalSpace(screenSpacePosition)))); private double getTimeFromPosition(Vector2 localPosition) => diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs index 6426d33e99..33ea137d51 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs @@ -382,7 +382,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline { OnDragHandled?.Invoke(e); - if (timeline.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition).Time is double time) + if (timeline.FindSnappedPositionAndTime(e.ScreenSpaceMousePosition).Time is double time) { switch (hitObject) { From 1fce0da33189907d8c14588af5b35dd8b5efb13a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:04:34 +0900 Subject: [PATCH 21/29] Reword slightly, to allow better conformity with `IDistanceSnapProvider` --- osu.Game/Rulesets/Edit/IPositionSnapProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs index 8b6dbb6af3..c6a365938c 100644 --- a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs @@ -6,8 +6,8 @@ using osuTK; namespace osu.Game.Rulesets.Edit { /// - /// A snap provider which given the position of a hit object, offers a more correct position and time value inferred from the context of the beatmap. - /// Provided values are done so in an isolated context, without consideration of other nearby hit objects. + /// A snap provider which given a proposed position for a hit object, potentially offers a more correct position and time value inferred from the context of the beatmap. + /// Provided values are inferred in an isolated context, without consideration of other nearby hit objects. /// public interface IPositionSnapProvider { From 6227e3f876bfd9d6fceebb5e2f7a8a276fdcccf6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:17:37 +0900 Subject: [PATCH 22/29] Add comprehensive documentation of `BeatmapInfo.DistanceSpacing` --- osu.Game/Beatmaps/BeatmapInfo.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 1a9703f478..abc9020dc6 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -1,21 +1,23 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Linq; using JetBrains.Annotations; using Newtonsoft.Json; using osu.Framework.Testing; +using osu.Game.Beatmaps.ControlPoints; using osu.Game.Database; using osu.Game.Models; using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays.BeatmapSet.Scores; using osu.Game.Rulesets; +using osu.Game.Rulesets.Edit; using osu.Game.Scoring; using Realms; -#nullable enable - namespace osu.Game.Beatmaps { /// @@ -109,6 +111,16 @@ namespace osu.Game.Beatmaps public bool SamplesMatchPlaybackRate { get; set; } = true; + /// + /// The ratio of distance travelled per time unit. + /// Generally used to decouple the spacing between hit objects from the enforced "velocity" of the beatmap (see ). + /// + /// + /// The most common method of understanding is that at a default value of 1.0, the time-to-distance ratio will match the slider velocity of the beatmap + /// at the current point in time. Increasing this value will make hit objects more spaced apart when compared to the cursor movement required to track a slider. + /// + /// This is only a hint property, used by the editor in implementations. It does not directly affect the beatmap or gameplay. + /// public double DistanceSpacing { get; set; } = 1.0; public int BeatDivisor { get; set; } From 977e6d8a8081fff93503156670ce034ec54387f7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:25:05 +0900 Subject: [PATCH 23/29] Add xmldoc for `IDistanceSnapProvider` and related properties --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 4 ++-- .../Sliders/SliderPlacementBlueprint.cs | 2 +- ...tSceneHitObjectComposerDistanceSnapping.cs | 4 ++-- .../Editing/TestSceneDistanceSnapGrid.cs | 4 ++-- .../Edit/DistancedHitObjectComposer.cs | 4 ++-- .../Rulesets/Edit/IDistanceSnapProvider.cs | 23 +++++++++++-------- .../Rulesets/Objects/SliderPathExtensions.cs | 2 +- .../Components/CircularDistanceSnapGrid.cs | 2 +- 8 files changed, 25 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 2ba30c5f74..2ea39e47f4 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -195,9 +195,9 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public double DistanceToDuration(HitObject referenceObject, float distance) => distance; - public double GetSnappedDurationFromDistance(HitObject referenceObject, float distance) => 0; + public double FindSnappedDuration(HitObject referenceObject, float distance) => 0; - public float GetSnappedDistanceFromDistance(HitObject referenceObject, float distance) => 0; + public float FindSnappedDistance(HitObject referenceObject, float distance) => 0; } } } diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs index 73a4ea5434..501589987d 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs @@ -220,7 +220,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders private void updateSlider() { - HitObject.Path.ExpectedDistance.Value = snapProvider?.GetSnappedDistanceFromDistance(HitObject, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; + HitObject.Path.ExpectedDistance.Value = snapProvider?.FindSnappedDistance(HitObject, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; bodyPiece.UpdateFrom(HitObject); headCirclePiece.UpdateFrom(HitObject.HeadCircle); diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs b/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs index a2ee97210a..82fc7a208b 100644 --- a/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs +++ b/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs @@ -213,10 +213,10 @@ namespace osu.Game.Tests.Editing => AddAssert($"distance = {distance} -> duration = {expectedDuration}", () => composer.DistanceToDuration(new HitObject(), distance) == expectedDuration); private void assertSnappedDuration(float distance, double expectedDuration) - => AddAssert($"distance = {distance} -> duration = {expectedDuration} (snapped)", () => composer.GetSnappedDurationFromDistance(new HitObject(), distance) == expectedDuration); + => AddAssert($"distance = {distance} -> duration = {expectedDuration} (snapped)", () => composer.FindSnappedDuration(new HitObject(), distance) == expectedDuration); private void assertSnappedDistance(float distance, float expectedDistance) - => AddAssert($"distance = {distance} -> distance = {expectedDistance} (snapped)", () => composer.GetSnappedDistanceFromDistance(new HitObject(), distance) == expectedDistance); + => AddAssert($"distance = {distance} -> distance = {expectedDistance} (snapped)", () => composer.FindSnappedDistance(new HitObject(), distance) == expectedDistance); private class TestHitObjectComposer : OsuHitObjectComposer { diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index b9cfa84a5d..7cb1e8f36a 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -175,9 +175,9 @@ namespace osu.Game.Tests.Visual.Editing public double DistanceToDuration(HitObject referenceObject, float distance) => distance; - public double GetSnappedDurationFromDistance(HitObject referenceObject, float distance) => 0; + public double FindSnappedDuration(HitObject referenceObject, float distance) => 0; - public float GetSnappedDistanceFromDistance(HitObject referenceObject, float distance) => 0; + public float FindSnappedDistance(HitObject referenceObject, float distance) => 0; } } } diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs index 680fc9aaa8..2280211421 100644 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs @@ -146,10 +146,10 @@ namespace osu.Game.Rulesets.Edit return distance / GetBeatSnapDistanceAt(referenceObject) * beatLength; } - public virtual double GetSnappedDurationFromDistance(HitObject referenceObject, float distance) + public virtual double FindSnappedDuration(HitObject referenceObject, float distance) => BeatSnapProvider.SnapTime(referenceObject.StartTime + DistanceToDuration(referenceObject, distance), referenceObject.StartTime) - referenceObject.StartTime; - public virtual float GetSnappedDistanceFromDistance(HitObject referenceObject, float distance) + public virtual float FindSnappedDistance(HitObject referenceObject, float distance) { double startTime = referenceObject.StartTime; diff --git a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs index c6e866561e..8e4e9afb1c 100644 --- a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs @@ -7,10 +7,13 @@ using osu.Game.Rulesets.Objects; namespace osu.Game.Rulesets.Edit { + /// + /// A snap provider which given a reference hit object and proposed distance from it, offers a more correct duration or distance value. + /// public interface IDistanceSnapProvider : IPositionSnapProvider { /// - /// The spacing multiplier applied to beat snap distances. + /// A multiplier which changes the ratio of distance travelled per time unit. /// /// IBindable DistanceSpacingMultiplier { get; } @@ -23,7 +26,7 @@ namespace osu.Game.Rulesets.Edit float GetBeatSnapDistanceAt(HitObject referenceObject); /// - /// Converts a duration to a distance. + /// Converts a duration to a distance without applying any snapping. /// /// An object to be used as a reference point for this operation. /// The duration to convert. @@ -31,7 +34,7 @@ namespace osu.Game.Rulesets.Edit float DurationToDistance(HitObject referenceObject, double duration); /// - /// Converts a distance to a duration. + /// Converts a distance to a duration without applying any snapping. /// /// An object to be used as a reference point for this operation. /// The distance to convert. @@ -39,20 +42,22 @@ namespace osu.Game.Rulesets.Edit double DistanceToDuration(HitObject referenceObject, float distance); /// - /// Converts a distance to a snapped duration. + /// Given a distance from the provided hit object, find the valid snapped duration. /// /// An object to be used as a reference point for this operation. /// The distance to convert. /// A value that represents as a duration snapped to the closest beat of the timing point. - double GetSnappedDurationFromDistance(HitObject referenceObject, float distance); + double FindSnappedDuration(HitObject referenceObject, float distance); /// - /// Converts an unsnapped distance to a snapped distance. - /// The returned distance will always be floored (as to never exceed the provided . + /// Given a distance from the provided hit object, find the valid snapped distance. /// /// An object to be used as a reference point for this operation. /// The distance to convert. - /// A value that represents snapped to the closest beat of the timing point. - float GetSnappedDistanceFromDistance(HitObject referenceObject, float distance); + /// + /// A value that represents snapped to the closest beat of the timing point. + /// The distance will always be less than or equal to the provided . + /// + float FindSnappedDistance(HitObject referenceObject, float distance); } } diff --git a/osu.Game/Rulesets/Objects/SliderPathExtensions.cs b/osu.Game/Rulesets/Objects/SliderPathExtensions.cs index 3100d26a55..dd418a1b7b 100644 --- a/osu.Game/Rulesets/Objects/SliderPathExtensions.cs +++ b/osu.Game/Rulesets/Objects/SliderPathExtensions.cs @@ -18,7 +18,7 @@ namespace osu.Game.Rulesets.Objects public static void SnapTo(this THitObject hitObject, IDistanceSnapProvider? snapProvider) where THitObject : HitObject, IHasPath { - hitObject.Path.ExpectedDistance.Value = snapProvider?.GetSnappedDistanceFromDistance(hitObject, (float)hitObject.Path.CalculatedDistance) ?? hitObject.Path.CalculatedDistance; + hitObject.Path.ExpectedDistance.Value = snapProvider?.FindSnappedDistance(hitObject, (float)hitObject.Path.CalculatedDistance) ?? hitObject.Path.CalculatedDistance; } /// diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 6b32ff96c4..50d5f0389a 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -80,7 +80,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Vector2 normalisedDirection = direction * new Vector2(1f / distance); Vector2 snappedPosition = StartPosition + normalisedDirection * radialCount * radius; - return (snappedPosition, StartTime + SnapProvider.GetSnappedDurationFromDistance(ReferenceObject, (snappedPosition - StartPosition).Length)); + return (snappedPosition, StartTime + SnapProvider.FindSnappedDuration(ReferenceObject, (snappedPosition - StartPosition).Length)); } } } From b411b59006da5cfbb5d39cd0e0754db943b3f298 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:32:14 +0900 Subject: [PATCH 24/29] Move `IPlacementHandler` caching to interface --- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 1 - osu.Game/Screens/Edit/Compose/IPlacementHandler.cs | 2 ++ osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 1c388fe68f..fdc12bf549 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -35,7 +35,6 @@ namespace osu.Game.Rulesets.Edit /// Responsible for providing snapping and generally gluing components together. /// /// The base type of supported objects. - [Cached(Type = typeof(IPlacementHandler))] public abstract class HitObjectComposer : HitObjectComposer, IPlacementHandler where TObject : HitObject { diff --git a/osu.Game/Screens/Edit/Compose/IPlacementHandler.cs b/osu.Game/Screens/Edit/Compose/IPlacementHandler.cs index aefcbc6542..59eb13cae5 100644 --- a/osu.Game/Screens/Edit/Compose/IPlacementHandler.cs +++ b/osu.Game/Screens/Edit/Compose/IPlacementHandler.cs @@ -1,10 +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 osu.Framework.Allocation; using osu.Game.Rulesets.Objects; namespace osu.Game.Screens.Edit.Compose { + [Cached] public interface IPlacementHandler { /// diff --git a/osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs b/osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs index b2f5b1754f..b981a31bd1 100644 --- a/osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs +++ b/osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs @@ -14,7 +14,6 @@ using osu.Game.Screens.Edit.Compose; namespace osu.Game.Tests.Visual { - [Cached(Type = typeof(IPlacementHandler))] public abstract class PlacementBlueprintTestScene : OsuManualInputManagerTestScene, IPlacementHandler { protected readonly Container HitObjectContainer; From 1c6a233cc0daba2519a3ba54609ebabea6c7c637 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:35:52 +0900 Subject: [PATCH 25/29] Move snap provider caching to interfaces --- osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs | 1 - osu.Game/Rulesets/Edit/HitObjectComposer.cs | 1 - osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs | 2 ++ osu.Game/Rulesets/Edit/IPositionSnapProvider.cs | 2 ++ osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs | 1 - 5 files changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs index 680fc9aaa8..7a997bb423 100644 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs @@ -23,7 +23,6 @@ namespace osu.Game.Rulesets.Edit /// Represents a for rulesets with the concept of distances between objects. /// /// The base type of supported objects. - [Cached(typeof(IDistanceSnapProvider))] public abstract class DistancedHitObjectComposer : HitObjectComposer, IDistanceSnapProvider, IScrollBindingHandler where TObject : HitObject { diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index fdc12bf549..c384eb4ea9 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -388,7 +388,6 @@ namespace osu.Game.Rulesets.Edit /// Generally used to access certain methods without requiring a generic type for . /// [Cached(typeof(HitObjectComposer))] - [Cached(typeof(IPositionSnapProvider))] public abstract class HitObjectComposer : CompositeDrawable, IPositionSnapProvider { protected HitObjectComposer() diff --git a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs index c6e866561e..7850edf3f1 100644 --- a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs @@ -1,12 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects; namespace osu.Game.Rulesets.Edit { + [Cached] public interface IDistanceSnapProvider : IPositionSnapProvider { /// diff --git a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs index 8a179ed424..d7379b26ce 100644 --- a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs @@ -1,10 +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 osu.Framework.Allocation; using osuTK; namespace osu.Game.Rulesets.Edit { + [Cached] public interface IPositionSnapProvider { /// diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs index c2b2bdb861..f91b321835 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs @@ -19,7 +19,6 @@ using osuTK; namespace osu.Game.Screens.Edit.Compose.Components.Timeline { - [Cached(typeof(IPositionSnapProvider))] [Cached] public class Timeline : ZoomableScrollContainer, IPositionSnapProvider { From 5a1ac71d90fdb698d6e8e2a68ef7779faf651acd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 16:42:57 +0900 Subject: [PATCH 26/29] Remove unnecessary type specification in `HitObjectComposer`'s caching --- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index c384eb4ea9..b97690c9e0 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -387,7 +387,7 @@ namespace osu.Game.Rulesets.Edit /// A non-generic definition of a HitObject composer class. /// Generally used to access certain methods without requiring a generic type for . /// - [Cached(typeof(HitObjectComposer))] + [Cached] public abstract class HitObjectComposer : CompositeDrawable, IPositionSnapProvider { protected HitObjectComposer() From c3d2648f85bd87289cb58a5622038e2f4d44e7f4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:07:05 +0900 Subject: [PATCH 27/29] Reword weird xmldoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Rulesets/Edit/IPositionSnapProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs index c6a365938c..200005cb11 100644 --- a/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IPositionSnapProvider.cs @@ -22,7 +22,7 @@ namespace osu.Game.Rulesets.Edit SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition); /// - /// Given a position, find a value position snap, restricting time to its input value. + /// Given a position, find a valid position snap, without changing the time value. /// /// The screen-space position to be snapped. /// The position post-snapping. Time will always be null. From 67341db0e742e302802a006d21e9c8baa2bba68a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 5 May 2022 12:40:02 +0300 Subject: [PATCH 28/29] Wrap `BeatmapOnlineLookupQueue` cache request in a task --- osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs b/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs index a24b6b315a..46f5b418bd 100644 --- a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs +++ b/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs @@ -153,7 +153,7 @@ namespace osu.Game.Beatmaps } }; - cacheDownloadRequest.PerformAsync(); + Task.Run(() => cacheDownloadRequest.PerformAsync()); } private bool checkLocalCache(BeatmapSetInfo set, BeatmapInfo beatmapInfo) From 684db27bb8d962c6ada325dcb903b71c95106dbb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 19:40:44 +0900 Subject: [PATCH 29/29] Reword binding text to read better --- osu.Game/Localisation/GlobalActionKeyBindingStrings.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs index cfe0fd55ce..b2f25de7f2 100644 --- a/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs +++ b/osu.Game/Localisation/GlobalActionKeyBindingStrings.cs @@ -130,14 +130,14 @@ namespace osu.Game.Localisation public static LocalisableString SelectNext => new TranslatableString(getKey(@"select_next"), @"Next selection"); /// - /// "Previous group selection" + /// "Select previous group" /// - public static LocalisableString SelectPreviousGroup => new TranslatableString(getKey(@"select_previous_group"), @"Previous group selection"); + public static LocalisableString SelectPreviousGroup => new TranslatableString(getKey(@"select_previous_group"), @"Select previous group"); /// - /// "Next group selection" + /// "Select next group" /// - public static LocalisableString SelectNextGroup => new TranslatableString(getKey(@"select_next_group"), @"Next group selection"); + public static LocalisableString SelectNextGroup => new TranslatableString(getKey(@"select_next_group"), @"Select next group"); /// /// "Home"