From 36137e06197c1b083873cadb3cfec7e64dd6d858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 13 Feb 2022 14:37:37 +0100 Subject: [PATCH 1/9] Add simple carousel divisor type selector --- .../Compose/Components/BeatDivisorControl.cs | 101 +++++++++++++++--- .../Compose/Components/BeatDivisorType.cs | 25 +++++ 2 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index de63b265d2..f1779c0d18 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -2,7 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; using System.Linq; +using Humanizer; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -14,6 +16,7 @@ using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Game.Graphics; +using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osuTK; using osuTK.Graphics; @@ -24,6 +27,7 @@ namespace osu.Game.Screens.Edit.Compose.Components public class BeatDivisorControl : CompositeDrawable { private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); + private readonly Bindable divisorType = new Bindable(); public BeatDivisorControl(BindableBeatDivisor beatDivisor) { @@ -84,7 +88,6 @@ namespace osu.Game.Screens.Edit.Compose.Components new Container { RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Horizontal = 5 }, Child = new GridContainer { RelativeSizeAxes = Axes.Both, @@ -92,13 +95,13 @@ namespace osu.Game.Screens.Edit.Compose.Components { new Drawable[] { - new DivisorButton + new ChevronButton { Icon = FontAwesome.Solid.ChevronLeft, Action = beatDivisor.Previous }, - new DivisorText(beatDivisor), - new DivisorButton + new DivisorText { BeatDivisor = { BindTarget = beatDivisor } }, + new ChevronButton { Icon = FontAwesome.Solid.ChevronRight, Action = beatDivisor.Next @@ -121,29 +124,80 @@ namespace osu.Game.Screens.Edit.Compose.Components new TextFlowContainer(s => s.Font = s.Font.With(size: 14)) { Padding = new MarginPadding { Horizontal = 15 }, - Text = "beat snap divisor", + Text = "beat snap", RelativeSizeAxes = Axes.X, TextAnchor = Anchor.TopCentre }, - } + }, + new Drawable[] + { + new Container + { + RelativeSizeAxes = Axes.Both, + Children = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = colours.Gray4 + }, + new Container + { + RelativeSizeAxes = Axes.Both, + Child = new GridContainer + { + RelativeSizeAxes = Axes.Both, + Content = new[] + { + new Drawable[] + { + new ChevronButton + { + Icon = FontAwesome.Solid.ChevronLeft, + Action = () => cycleDivisorType(-1) + }, + new DivisorTypeText { BeatDivisorType = { BindTarget = divisorType } }, + new ChevronButton + { + Icon = FontAwesome.Solid.ChevronRight, + Action = () => cycleDivisorType(1) + } + }, + }, + ColumnDimensions = new[] + { + new Dimension(GridSizeMode.Absolute, 20), + new Dimension(), + new Dimension(GridSizeMode.Absolute, 20) + } + } + } + } + } + }, }, RowDimensions = new[] { new Dimension(GridSizeMode.Absolute, 30), - new Dimension(GridSizeMode.Absolute, 25), + new Dimension(GridSizeMode.Absolute, 20), + new Dimension(GridSizeMode.Absolute, 15) } } }; } + private void cycleDivisorType(int direction) + { + Debug.Assert(Math.Abs(direction) == 1); + divisorType.Value = (BeatDivisorType)(((int)divisorType.Value + direction) % (int)(BeatDivisorType.Last + 1)); + } + private class DivisorText : SpriteText { - private readonly Bindable beatDivisor = new Bindable(); + public Bindable BeatDivisor { get; } = new Bindable(); - public DivisorText(BindableBeatDivisor beatDivisor) + public DivisorText() { - this.beatDivisor.BindTo(beatDivisor); - Anchor = Anchor.Centre; Origin = Anchor.Centre; } @@ -157,13 +211,32 @@ namespace osu.Game.Screens.Edit.Compose.Components protected override void LoadComplete() { base.LoadComplete(); - beatDivisor.BindValueChanged(val => Text = $"1/{val.NewValue}", true); + BeatDivisor.BindValueChanged(val => Text = $"1/{val.NewValue}", true); } } - private class DivisorButton : IconButton + private class DivisorTypeText : OsuSpriteText { - public DivisorButton() + public Bindable BeatDivisorType { get; } = new Bindable(); + + public DivisorTypeText() + { + Anchor = Anchor.Centre; + Origin = Anchor.Centre; + + Font = OsuFont.Default.With(size: 14); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + BeatDivisorType.BindValueChanged(val => Text = val.NewValue.Humanize(LetterCasing.LowerCase), true); + } + } + + private class ChevronButton : IconButton + { + public ChevronButton() { Anchor = Anchor.Centre; Origin = Anchor.Centre; diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs new file mode 100644 index 0000000000..2a7774118e --- /dev/null +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs @@ -0,0 +1,25 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Screens.Edit.Compose.Components +{ + public enum BeatDivisorType + { + /// + /// Most common divisors, all with denominators being powers of two. + /// + Common, + + /// + /// Divisors with denominators divisible by 3. + /// + Triplets, + + /// + /// Fully arbitrary/custom beat divisors. + /// + Custom, + + Last = Custom + } +} From d0c01afc2e7610e7797b929be4acc97510e45043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 13 Feb 2022 15:50:40 +0100 Subject: [PATCH 2/9] Add flow for changing set of valid divisors between presets --- .../Editing/TestSceneBeatDivisorControl.cs | 7 +- .../ControlPoints/ControlPointInfo.cs | 2 +- osu.Game/Screens/Edit/BindableBeatDivisor.cs | 57 ++++++++++------ .../Compose/Components/BeatDivisorControl.cs | 66 ++++++++++++------- .../Components/BeatDivisorPresetCollection.cs | 41 ++++++++++++ .../Compose/Components/BeatDivisorType.cs | 1 + .../Timeline/TimelineTickDisplay.cs | 2 +- 7 files changed, 127 insertions(+), 49 deletions(-) create mode 100644 osu.Game/Screens/Edit/Compose/Components/BeatDivisorPresetCollection.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs index ed7bb9e301..131bfde86e 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs @@ -20,8 +20,8 @@ namespace osu.Game.Tests.Visual.Editing private BeatDivisorControl beatDivisorControl; private BindableBeatDivisor bindableBeatDivisor; - private SliderBar tickSliderBar; - private EquilateralTriangle tickMarkerHead; + private SliderBar tickSliderBar => beatDivisorControl.ChildrenOfType>().Single(); + private EquilateralTriangle tickMarkerHead => tickSliderBar.ChildrenOfType().Single(); [SetUp] public void SetUp() => Schedule(() => @@ -32,9 +32,6 @@ namespace osu.Game.Tests.Visual.Editing Origin = Anchor.Centre, Size = new Vector2(90, 90) }; - - tickSliderBar = beatDivisorControl.ChildrenOfType>().Single(); - tickMarkerHead = tickSliderBar.ChildrenOfType().Single(); }); [Test] diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 246d1f8af5..af03d639be 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -164,7 +164,7 @@ namespace osu.Game.Beatmaps.ControlPoints int closestDivisor = 0; double closestTime = double.MaxValue; - foreach (int divisor in BindableBeatDivisor.VALID_DIVISORS) + foreach (int divisor in BindableBeatDivisor.PREDEFINED_DIVISORS) { double distanceFromSnap = Math.Abs(time - getClosestSnappedTime(timingPoint, time, divisor)); diff --git a/osu.Game/Screens/Edit/BindableBeatDivisor.cs b/osu.Game/Screens/Edit/BindableBeatDivisor.cs index 1a350d7261..9077898ec8 100644 --- a/osu.Game/Screens/Edit/BindableBeatDivisor.cs +++ b/osu.Game/Screens/Edit/BindableBeatDivisor.cs @@ -1,46 +1,63 @@ // 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.Linq; using osu.Framework.Bindables; using osu.Game.Graphics; +using osu.Game.Screens.Edit.Compose.Components; using osuTK.Graphics; namespace osu.Game.Screens.Edit { public class BindableBeatDivisor : BindableInt { - public static readonly int[] VALID_DIVISORS = { 1, 2, 3, 4, 6, 8, 12, 16 }; + public static readonly int[] PREDEFINED_DIVISORS = { 1, 2, 3, 4, 6, 8, 12, 16 }; + + public Bindable ValidDivisors { get; } = new Bindable(BeatDivisorPresetCollection.COMMON); public BindableBeatDivisor(int value = 1) : base(value) { + ValidDivisors.BindValueChanged(_ => updateBindableProperties(), true); + BindValueChanged(_ => ensureValidDivisor()); } - public void Next() => Value = VALID_DIVISORS[Math.Min(VALID_DIVISORS.Length - 1, Array.IndexOf(VALID_DIVISORS, Value) + 1)]; - - public void Previous() => Value = VALID_DIVISORS[Math.Max(0, Array.IndexOf(VALID_DIVISORS, Value) - 1)]; - - public override int Value + private void updateBindableProperties() { - get => base.Value; - set - { - if (!VALID_DIVISORS.Contains(value)) - { - // If it doesn't match, value will be 0, but will be clamped to the valid range via DefaultMinValue - value = Array.FindLast(VALID_DIVISORS, d => d < value); - } + ensureValidDivisor(); - base.Value = value; - } + MinValue = ValidDivisors.Value.Presets.Min(); + MaxValue = ValidDivisors.Value.Presets.Max(); + } + + private void ensureValidDivisor() + { + if (!ValidDivisors.Value.Presets.Contains(Value)) + Value = 1; + } + + public void Next() + { + var presets = ValidDivisors.Value.Presets; + Value = presets.Cast().SkipWhile(preset => preset != Value).ElementAtOrDefault(1) ?? presets[0]; + } + + public void Previous() + { + var presets = ValidDivisors.Value.Presets; + Value = presets.Cast().TakeWhile(preset => preset != Value).LastOrDefault() ?? presets[^1]; } - protected override int DefaultMinValue => VALID_DIVISORS.First(); - protected override int DefaultMaxValue => VALID_DIVISORS.Last(); protected override int DefaultPrecision => 1; + public override void BindTo(Bindable them) + { + base.BindTo(them); + + if (them is BindableBeatDivisor otherBeatDivisor) + ValidDivisors.BindTo(otherBeatDivisor.ValidDivisors); + } + protected override Bindable CreateInstance() => new BindableBeatDivisor(); /// @@ -92,7 +109,7 @@ namespace osu.Game.Screens.Edit { int beat = index % beatDivisor; - foreach (int divisor in BindableBeatDivisor.VALID_DIVISORS) + foreach (int divisor in PREDEFINED_DIVISORS) { if ((beat * divisor) % beatDivisor == 0) return divisor; diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index f1779c0d18..a9b1213738 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -27,7 +27,6 @@ namespace osu.Game.Screens.Edit.Compose.Components public class BeatDivisorControl : CompositeDrawable { private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); - private readonly Bindable divisorType = new Bindable(); public BeatDivisorControl(BindableBeatDivisor beatDivisor) { @@ -66,7 +65,7 @@ namespace osu.Game.Screens.Edit.Compose.Components RelativeSizeAxes = Axes.Both, Colour = Color4.Black }, - new TickSliderBar(beatDivisor, BindableBeatDivisor.VALID_DIVISORS) + new TickSliderBar(beatDivisor) { RelativeSizeAxes = Axes.Both, } @@ -156,7 +155,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Icon = FontAwesome.Solid.ChevronLeft, Action = () => cycleDivisorType(-1) }, - new DivisorTypeText { BeatDivisorType = { BindTarget = divisorType } }, + new DivisorTypeText { BeatDivisor = { BindTarget = beatDivisor } }, new ChevronButton { Icon = FontAwesome.Solid.ChevronRight, @@ -189,7 +188,26 @@ namespace osu.Game.Screens.Edit.Compose.Components private void cycleDivisorType(int direction) { Debug.Assert(Math.Abs(direction) == 1); - divisorType.Value = (BeatDivisorType)(((int)divisorType.Value + direction) % (int)(BeatDivisorType.Last + 1)); + int nextDivisorType = (int)beatDivisor.ValidDivisors.Value.Type + direction; + if (nextDivisorType > (int)BeatDivisorType.Last) + nextDivisorType = (int)BeatDivisorType.First; + else if (nextDivisorType < (int)BeatDivisorType.First) + nextDivisorType = (int)BeatDivisorType.Last; + + switch ((BeatDivisorType)nextDivisorType) + { + case BeatDivisorType.Common: + beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.COMMON; + break; + + case BeatDivisorType.Triplets: + beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.TRIPLETS; + break; + + case BeatDivisorType.Custom: + beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.Custom(18); // todo + break; + } } private class DivisorText : SpriteText @@ -217,7 +235,7 @@ namespace osu.Game.Screens.Edit.Compose.Components private class DivisorTypeText : OsuSpriteText { - public Bindable BeatDivisorType { get; } = new Bindable(); + public BindableBeatDivisor BeatDivisor { get; } = new BindableBeatDivisor(); public DivisorTypeText() { @@ -230,7 +248,7 @@ namespace osu.Game.Screens.Edit.Compose.Components protected override void LoadComplete() { base.LoadComplete(); - BeatDivisorType.BindValueChanged(val => Text = val.NewValue.Humanize(LetterCasing.LowerCase), true); + BeatDivisor.ValidDivisors.BindValueChanged(val => Text = val.NewValue.Type.Humanize(LetterCasing.LowerCase), true); } } @@ -265,20 +283,27 @@ namespace osu.Game.Screens.Edit.Compose.Components private OsuColour colours { get; set; } private readonly BindableBeatDivisor beatDivisor; - private readonly int[] availableDivisors; - public TickSliderBar(BindableBeatDivisor beatDivisor, params int[] divisors) + public TickSliderBar(BindableBeatDivisor beatDivisor) { CurrentNumber.BindTo(this.beatDivisor = beatDivisor); - availableDivisors = divisors; Padding = new MarginPadding { Horizontal = 5 }; } - [BackgroundDependencyLoader] - private void load() + protected override void LoadComplete() { - foreach (int t in availableDivisors) + base.LoadComplete(); + + beatDivisor.ValidDivisors.BindValueChanged(_ => updateDivisors(), true); + } + + private void updateDivisors() + { + ClearInternal(); + CurrentNumber.ValueChanged -= moveMarker; + + foreach (int t in beatDivisor.ValidDivisors.Value.Presets) { AddInternal(new Tick { @@ -291,17 +316,14 @@ namespace osu.Game.Screens.Edit.Compose.Components } AddInternal(marker = new Marker()); + CurrentNumber.ValueChanged += moveMarker; + CurrentNumber.TriggerChange(); } - protected override void LoadComplete() + private void moveMarker(ValueChangedEvent divisor) { - base.LoadComplete(); - - CurrentNumber.BindValueChanged(div => - { - marker.MoveToX(getMappedPosition(div.NewValue), 100, Easing.OutQuint); - marker.Flash(); - }, true); + marker.MoveToX(getMappedPosition(divisor.NewValue), 100, Easing.OutQuint); + marker.Flash(); } protected override void UpdateValue(float value) @@ -362,11 +384,11 @@ namespace osu.Game.Screens.Edit.Compose.Components // copied from SliderBar so we can do custom spacing logic. float xPosition = (ToLocalSpace(screenSpaceMousePosition).X - RangePadding) / UsableWidth; - CurrentNumber.Value = availableDivisors.OrderBy(d => Math.Abs(getMappedPosition(d) - xPosition)).First(); + CurrentNumber.Value = beatDivisor.ValidDivisors.Value.Presets.OrderBy(d => Math.Abs(getMappedPosition(d) - xPosition)).First(); OnUserChange(Current.Value); } - private float getMappedPosition(float divisor) => MathF.Pow((divisor - 1) / (availableDivisors.Last() - 1), 0.90f); + private float getMappedPosition(float divisor) => MathF.Pow((divisor - 1) / (beatDivisor.ValidDivisors.Value.Presets.Last() - 1), 0.90f); private class Tick : CompositeDrawable { diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorPresetCollection.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorPresetCollection.cs new file mode 100644 index 0000000000..4616669c6d --- /dev/null +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorPresetCollection.cs @@ -0,0 +1,41 @@ +// 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; + +namespace osu.Game.Screens.Edit.Compose.Components +{ + public class BeatDivisorPresetCollection + { + public BeatDivisorType Type { get; } + public IReadOnlyList Presets { get; } + + private BeatDivisorPresetCollection(BeatDivisorType type, IEnumerable presets) + { + Type = type; + Presets = presets.ToArray(); + } + + public static readonly BeatDivisorPresetCollection COMMON = new BeatDivisorPresetCollection(BeatDivisorType.Common, new[] { 1, 2, 4, 8, 16 }); + + public static readonly BeatDivisorPresetCollection TRIPLETS = new BeatDivisorPresetCollection(BeatDivisorType.Triplets, new[] { 1, 3, 6, 12 }); + + public static BeatDivisorPresetCollection Custom(int maxDivisor) + { + var presets = new List(); + + for (int candidate = 1; candidate <= Math.Sqrt(maxDivisor); ++candidate) + { + if (maxDivisor % candidate != 0) + continue; + + presets.Add(candidate); + presets.Add(maxDivisor / candidate); + } + + return new BeatDivisorPresetCollection(BeatDivisorType.Custom, presets.Distinct().OrderBy(d => d)); + } + } +} diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs index 2a7774118e..15a8c504c5 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs @@ -20,6 +20,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// Custom, + First = Common, Last = Custom } } diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs index cc4041394d..3a32dc18e5 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs @@ -31,7 +31,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline [Resolved] private OsuColour colours { get; set; } - private static readonly int highest_divisor = BindableBeatDivisor.VALID_DIVISORS.Last(); + private static readonly int highest_divisor = BindableBeatDivisor.PREDEFINED_DIVISORS.Last(); public TimelineTickDisplay() { From 423838a649fe8400829a2cbee6de186135799936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 13 Feb 2022 16:27:12 +0100 Subject: [PATCH 3/9] Add flow for specifying entirely custom snaps --- .../Editing/TestSceneBeatDivisorControl.cs | 13 +- osu.Game/Screens/Edit/BindableBeatDivisor.cs | 5 +- .../Compose/Components/BeatDivisorControl.cs | 117 ++++++++++++++++-- .../Compose/Components/BeatDivisorType.cs | 3 - 4 files changed, 118 insertions(+), 20 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs index 131bfde86e..e37019e5d3 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; +using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.UserInterface; using osu.Framework.Testing; @@ -26,11 +27,15 @@ namespace osu.Game.Tests.Visual.Editing [SetUp] public void SetUp() => Schedule(() => { - Child = beatDivisorControl = new BeatDivisorControl(bindableBeatDivisor = new BindableBeatDivisor(16)) + Child = new PopoverContainer { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Size = new Vector2(90, 90) + RelativeSizeAxes = Axes.Both, + Child = beatDivisorControl = new BeatDivisorControl(bindableBeatDivisor = new BindableBeatDivisor(16)) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Size = new Vector2(90, 90) + } }; }); diff --git a/osu.Game/Screens/Edit/BindableBeatDivisor.cs b/osu.Game/Screens/Edit/BindableBeatDivisor.cs index 9077898ec8..af958e3448 100644 --- a/osu.Game/Screens/Edit/BindableBeatDivisor.cs +++ b/osu.Game/Screens/Edit/BindableBeatDivisor.cs @@ -52,10 +52,11 @@ namespace osu.Game.Screens.Edit public override void BindTo(Bindable them) { - base.BindTo(them); - + // bind to valid divisors first (if applicable) to ensure correct transfer of the actual divisor. if (them is BindableBeatDivisor otherBeatDivisor) ValidDivisors.BindTo(otherBeatDivisor.ValidDivisors); + + base.BindTo(them); } protected override Bindable CreateInstance() => new BindableBeatDivisor(); diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index a9b1213738..04e9ac842c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -7,17 +7,21 @@ using System.Linq; using Humanizer; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Game.Graphics; +using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; +using osu.Game.Graphics.UserInterfaceV2; using osuTK; using osuTK.Graphics; using osuTK.Input; @@ -99,7 +103,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Icon = FontAwesome.Solid.ChevronLeft, Action = beatDivisor.Previous }, - new DivisorText { BeatDivisor = { BindTarget = beatDivisor } }, + new DivisorDisplay { BeatDivisor = { BindTarget = beatDivisor } }, new ChevronButton { Icon = FontAwesome.Solid.ChevronRight, @@ -189,10 +193,10 @@ namespace osu.Game.Screens.Edit.Compose.Components { Debug.Assert(Math.Abs(direction) == 1); int nextDivisorType = (int)beatDivisor.ValidDivisors.Value.Type + direction; - if (nextDivisorType > (int)BeatDivisorType.Last) - nextDivisorType = (int)BeatDivisorType.First; - else if (nextDivisorType < (int)BeatDivisorType.First) - nextDivisorType = (int)BeatDivisorType.Last; + if (nextDivisorType > (int)BeatDivisorType.Triplets) + nextDivisorType = (int)BeatDivisorType.Common; + else if (nextDivisorType < (int)BeatDivisorType.Common) + nextDivisorType = (int)BeatDivisorType.Triplets; switch ((BeatDivisorType)nextDivisorType) { @@ -205,31 +209,122 @@ namespace osu.Game.Screens.Edit.Compose.Components break; case BeatDivisorType.Custom: - beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.Custom(18); // todo + beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.Custom(beatDivisor.ValidDivisors.Value.Presets.Max()); break; } } - private class DivisorText : SpriteText + private class DivisorDisplay : OsuAnimatedButton, IHasPopover { - public Bindable BeatDivisor { get; } = new Bindable(); + public BindableBeatDivisor BeatDivisor { get; } = new BindableBeatDivisor(); - public DivisorText() + private readonly OsuSpriteText divisorText; + + public DivisorDisplay() { Anchor = Anchor.Centre; Origin = Anchor.Centre; + + AutoSizeAxes = Axes.Both; + + Add(divisorText = new OsuSpriteText + { + Font = OsuFont.Default.With(size: 20), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Margin = new MarginPadding + { + Horizontal = 5 + } + }); + + Action = this.ShowPopover; } [BackgroundDependencyLoader] private void load(OsuColour colours) { - Colour = colours.BlueLighter; + divisorText.Colour = colours.BlueLighter; } protected override void LoadComplete() { base.LoadComplete(); - BeatDivisor.BindValueChanged(val => Text = $"1/{val.NewValue}", true); + updateState(); + } + + private void updateState() + { + BeatDivisor.BindValueChanged(val => divisorText.Text = $"1/{val.NewValue}", true); + } + + public Popover GetPopover() => new CustomDivisorPopover + { + BeatDivisor = { BindTarget = BeatDivisor } + }; + } + + private class CustomDivisorPopover : OsuPopover + { + public BindableBeatDivisor BeatDivisor { get; } = new BindableBeatDivisor(); + + private readonly OsuNumberBox divisorTextBox; + + public CustomDivisorPopover() + { + Child = new FillFlowContainer + { + Width = 150, + AutoSizeAxes = Axes.Y, + Spacing = new Vector2(10), + Children = new Drawable[] + { + divisorTextBox = new OsuNumberBox + { + RelativeSizeAxes = Axes.X, + PlaceholderText = "Beat divisor" + }, + new OsuTextFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Text = "All other applicable smaller divisors will be automatically added to the list of presets." + } + } + }; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + BeatDivisor.BindValueChanged(_ => updateState(), true); + divisorTextBox.OnCommit += (_, __) => setPresets(); + } + + private void setPresets() + { + if (!int.TryParse(divisorTextBox.Text, out int divisor) || divisor < 1 || divisor > 64) + { + updateState(); + return; + } + + if (!BeatDivisor.ValidDivisors.Value.Presets.Contains(divisor)) + { + if (BeatDivisorPresetCollection.COMMON.Presets.Contains(divisor)) + BeatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.COMMON; + else if (BeatDivisorPresetCollection.TRIPLETS.Presets.Contains(divisor)) + BeatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.TRIPLETS; + else + BeatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.Custom(divisor); + } + + BeatDivisor.Value = divisor; + } + + private void updateState() + { + divisorTextBox.Text = BeatDivisor.Value.ToString(); } } diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs index 15a8c504c5..4a25144881 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorType.cs @@ -19,8 +19,5 @@ namespace osu.Game.Screens.Edit.Compose.Components /// Fully arbitrary/custom beat divisors. /// Custom, - - First = Common, - Last = Custom } } From 7de5dad4f0ae616c7b3803f3a4b1d2d260b33bf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 27 Feb 2022 19:23:02 +0100 Subject: [PATCH 4/9] Add test coverage for divisor behaviour --- .../Editing/TestSceneBeatDivisorControl.cs | 115 ++++++++++++++++++ .../Compose/Components/BeatDivisorControl.cs | 6 +- 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs index e37019e5d3..4503b1a5f6 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; @@ -81,5 +82,119 @@ namespace osu.Game.Tests.Visual.Editing sliderDrawQuad.Centre.Y ); } + + [Test] + public void TestBeatChevronNavigation() + { + switchBeatSnap(1); + assertBeatSnap(1); + + switchBeatSnap(3); + assertBeatSnap(8); + + switchBeatSnap(-1); + assertBeatSnap(4); + + switchBeatSnap(-3); + assertBeatSnap(16); + } + + [Test] + public void TestBeatPresetNavigation() + { + assertPreset(BeatDivisorType.Common); + + switchPresets(1); + assertPreset(BeatDivisorType.Triplets); + + switchPresets(1); + assertPreset(BeatDivisorType.Common); + + switchPresets(-1); + assertPreset(BeatDivisorType.Triplets); + + switchPresets(-1); + assertPreset(BeatDivisorType.Common); + + setDivisorViaInput(3); + assertPreset(BeatDivisorType.Triplets); + + setDivisorViaInput(8); + assertPreset(BeatDivisorType.Common); + + setDivisorViaInput(15); + assertPreset(BeatDivisorType.Custom, 15); + + switchBeatSnap(-1); + assertBeatSnap(5); + + switchBeatSnap(-1); + assertBeatSnap(3); + + setDivisorViaInput(5); + assertPreset(BeatDivisorType.Custom, 15); + + switchPresets(1); + assertPreset(BeatDivisorType.Common); + + switchPresets(-1); + assertPreset(BeatDivisorType.Triplets); + } + + private void switchBeatSnap(int direction) => AddRepeatStep($"move snap {(direction > 0 ? "forward" : "backward")}", () => + { + int chevronIndex = direction > 0 ? 1 : 0; + var chevronButton = beatDivisorControl.ChildrenOfType().ElementAt(chevronIndex); + InputManager.MoveMouseTo(chevronButton); + InputManager.Click(MouseButton.Left); + }, Math.Abs(direction)); + + private void assertBeatSnap(int expected) => AddAssert($"beat snap is {expected}", + () => bindableBeatDivisor.Value == expected); + + private void switchPresets(int direction) => AddRepeatStep($"move presets {(direction > 0 ? "forward" : "backward")}", () => + { + int chevronIndex = direction > 0 ? 3 : 2; + var chevronButton = beatDivisorControl.ChildrenOfType().ElementAt(chevronIndex); + InputManager.MoveMouseTo(chevronButton); + InputManager.Click(MouseButton.Left); + }, Math.Abs(direction)); + + private void assertPreset(BeatDivisorType type, int? maxDivisor = null) + { + AddAssert($"preset is {type}", () => bindableBeatDivisor.ValidDivisors.Value.Type == type); + + if (type == BeatDivisorType.Custom) + { + Debug.Assert(maxDivisor != null); + AddAssert($"max divisor is {maxDivisor}", () => bindableBeatDivisor.ValidDivisors.Value.Presets.Max() == maxDivisor.Value); + } + } + + private void setDivisorViaInput(int divisor) + { + AddStep("open divisor input popover", () => + { + var button = beatDivisorControl.ChildrenOfType().Single(); + InputManager.MoveMouseTo(button); + InputManager.Click(MouseButton.Left); + }); + + BeatDivisorControl.CustomDivisorPopover popover = null; + AddUntilStep("wait for popover", () => (popover = this.ChildrenOfType().SingleOrDefault()) != null && popover.IsLoaded); + AddStep($"set divisor to {divisor}", () => + { + var textBox = popover.ChildrenOfType().Single(); + InputManager.MoveMouseTo(textBox); + InputManager.Click(MouseButton.Left); + textBox.Text = divisor.ToString(); + InputManager.Key(Key.Enter); + }); + AddStep("dismiss popover", () => + { + InputManager.Key(Key.Escape); + }); + AddUntilStep("wait for dismiss", () => !this.ChildrenOfType().Any()); + } } } diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index 04e9ac842c..d5b0ab19aa 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -214,7 +214,7 @@ namespace osu.Game.Screens.Edit.Compose.Components } } - private class DivisorDisplay : OsuAnimatedButton, IHasPopover + internal class DivisorDisplay : OsuAnimatedButton, IHasPopover { public BindableBeatDivisor BeatDivisor { get; } = new BindableBeatDivisor(); @@ -264,7 +264,7 @@ namespace osu.Game.Screens.Edit.Compose.Components }; } - private class CustomDivisorPopover : OsuPopover + internal class CustomDivisorPopover : OsuPopover { public BindableBeatDivisor BeatDivisor { get; } = new BindableBeatDivisor(); @@ -347,7 +347,7 @@ namespace osu.Game.Screens.Edit.Compose.Components } } - private class ChevronButton : IconButton + internal class ChevronButton : IconButton { public ChevronButton() { From a5600516f0200a8e596fa2011ba488106259bc1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 27 Feb 2022 20:13:44 +0100 Subject: [PATCH 5/9] Fix test failures --- .../Visual/Editing/TestSceneBeatDivisorControl.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs index 4503b1a5f6..94c0822235 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs @@ -43,10 +43,10 @@ namespace osu.Game.Tests.Visual.Editing [Test] public void TestBindableBeatDivisor() { - AddRepeatStep("move previous", () => bindableBeatDivisor.Previous(), 4); + AddRepeatStep("move previous", () => bindableBeatDivisor.Previous(), 2); AddAssert("divisor is 4", () => bindableBeatDivisor.Value == 4); - AddRepeatStep("move next", () => bindableBeatDivisor.Next(), 3); - AddAssert("divisor is 12", () => bindableBeatDivisor.Value == 12); + AddRepeatStep("move next", () => bindableBeatDivisor.Next(), 1); + AddAssert("divisor is 12", () => bindableBeatDivisor.Value == 8); } [Test] From a1786f62d71999fc9b8991d3c43161932a0eeeb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 27 Feb 2022 23:10:22 +0100 Subject: [PATCH 6/9] Fix test failure due to attempting to set non-present divisor With the latest changes permitting fully custom beat snapping, the 1/3 snap divisor isn't immediately available in editor, requiring a switch to "triplets" mode first. --- .../Editor/TestSceneSliderStreamConversion.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderStreamConversion.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderStreamConversion.cs index 559d612037..70a9c03e65 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderStreamConversion.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderStreamConversion.cs @@ -7,6 +7,7 @@ using osu.Framework.Utils; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Screens.Edit; +using osu.Game.Screens.Edit.Compose.Components; using osuTK; using osuTK.Input; @@ -72,7 +73,11 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor EditorClock.Seek(slider.StartTime); EditorBeatmap.SelectedHitObjects.Add(slider); }); - AddStep("change beat divisor", () => beatDivisor.Value = 3); + AddStep("change beat divisor", () => + { + beatDivisor.ValidDivisors.Value = BeatDivisorPresetCollection.TRIPLETS; + beatDivisor.Value = 3; + }); convertToStream(); From 3634e12e66ff499831a441d5ff27f7ceab1e8627 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 28 Feb 2022 15:21:01 +0900 Subject: [PATCH 7/9] Automatically focus divisor textbox and hide popover after successful change --- osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs | 4 ---- .../Screens/Edit/Compose/Components/BeatDivisorControl.cs | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs index 94c0822235..6a0950c6dd 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneBeatDivisorControl.cs @@ -190,10 +190,6 @@ namespace osu.Game.Tests.Visual.Editing textBox.Text = divisor.ToString(); InputManager.Key(Key.Enter); }); - AddStep("dismiss popover", () => - { - InputManager.Key(Key.Escape); - }); AddUntilStep("wait for dismiss", () => !this.ChildrenOfType().Any()); } } diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index d5b0ab19aa..c65423ef9f 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -16,6 +16,7 @@ using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; +using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Game.Graphics; using osu.Game.Graphics.Containers; @@ -299,6 +300,8 @@ namespace osu.Game.Screens.Edit.Compose.Components base.LoadComplete(); BeatDivisor.BindValueChanged(_ => updateState(), true); divisorTextBox.OnCommit += (_, __) => setPresets(); + + Schedule(() => GetContainingInputManager().ChangeFocus(divisorTextBox)); } private void setPresets() @@ -320,6 +323,8 @@ namespace osu.Game.Screens.Edit.Compose.Components } BeatDivisor.Value = divisor; + + this.HidePopover(); } private void updateState() From 368eadd8d1f02ac7263b093835cb4d98d69ec27b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 28 Feb 2022 15:24:02 +0900 Subject: [PATCH 8/9] Remove unused using statement --- osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index c65423ef9f..bea72b7447 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -16,7 +16,6 @@ using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; -using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Game.Graphics; using osu.Game.Graphics.Containers; From 2be40f36f77c81501d5530ff136908111ce3ddb8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 28 Feb 2022 15:26:09 +0900 Subject: [PATCH 9/9] Reword popup text to read better (or more vaguely) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed some words but also don't mention "smaller" because it's... musically incorrect and also functionally incorrect – entering 1/[8] will result in 1/16 also being populated for instance. --- osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs index bea72b7447..370c9016c7 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatDivisorControl.cs @@ -288,7 +288,7 @@ namespace osu.Game.Screens.Edit.Compose.Components { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, - Text = "All other applicable smaller divisors will be automatically added to the list of presets." + Text = "Related divisors will be added to the list of presets." } } };