Merge pull request #18502 from peppy/editor-timing-follow-current-time

Add automatic control point tracking to the timing screen

osu.Game.Tests/Visual/Editing/TestSceneTimingScreen.cs

@@ -1,14 +1,18 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Testing;
 using osu.Game.Overlays;
 using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Screens.Edit;
 using osu.Game.Screens.Edit.Timing;
+using osu.Game.Screens.Edit.Timing.RowAttributes;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Editing
 {
@@ -22,6 +26,8 @@ namespace osu.Game.Tests.Visual.Editing
         [Cached]
         private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Blue);
 
+        private TimingScreen timingScreen;
+
         protected override bool ScrollUsingMouseWheel => false;
 
         public TestSceneTimingScreen()
@@ -36,12 +42,54 @@ namespace osu.Game.Tests.Visual.Editing
             Beatmap.Value = CreateWorkingBeatmap(editorBeatmap.PlayableBeatmap);
             Beatmap.Disabled = true;
 
-            Child = new TimingScreen
+            Child = timingScreen = new TimingScreen
             {
                 State = { Value = Visibility.Visible },
             };
         }
 
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("Stop clock", () => Clock.Stop());
+
+            AddUntilStep("wait for rows to load", () => Child.ChildrenOfType<EffectRowAttribute>().Any());
+        }
+
+        [Test]
+        public void TestTrackingCurrentTimeWhileRunning()
+        {
+            AddStep("Select first effect point", () =>
+            {
+                InputManager.MoveMouseTo(Child.ChildrenOfType<EffectRowAttribute>().First());
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value.Time == 54670);
+            AddUntilStep("Ensure seeked to correct time", () => Clock.CurrentTimeAccurate == 54670);
+
+            AddStep("Seek to just before next point", () => Clock.Seek(69000));
+            AddStep("Start clock", () => Clock.Start());
+
+            AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value.Time == 69670);
+        }
+
+        [Test]
+        public void TestTrackingCurrentTimeWhilePaused()
+        {
+            AddStep("Select first effect point", () =>
+            {
+                InputManager.MoveMouseTo(Child.ChildrenOfType<EffectRowAttribute>().First());
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value.Time == 54670);
+            AddUntilStep("Ensure seeked to correct time", () => Clock.CurrentTimeAccurate == 54670);
+
+            AddStep("Seek to later", () => Clock.Seek(80000));
+            AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value.Time == 69670);
+        }
+
         protected override void Dispose(bool isDisposing)
         {
             Beatmap.Disabled = false;

osu.Game/Screens/Edit/Timing/ControlPointTable.cs

@@ -61,6 +61,7 @@ namespace osu.Game.Screens.Edit.Timing
 
             selectedGroup.BindValueChanged(group =>
             {
+                // TODO: This should scroll the selected row into view.
                 foreach (var b in BackgroundFlow) b.Selected = b.Item == group.NewValue;
             }, true);
         }

osu.Game/Screens/Edit/Timing/EffectSection.cs

@@ -31,18 +31,33 @@ namespace osu.Game.Screens.Edit.Timing
             });
         }
 
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            kiai.Current.BindValueChanged(_ => saveChanges());
+            omitBarLine.Current.BindValueChanged(_ => saveChanges());
+            scrollSpeedSlider.Current.BindValueChanged(_ => saveChanges());
+
+            void saveChanges()
+            {
+                if (!isRebinding) ChangeHandler?.SaveState();
+            }
+        }
+
+        private bool isRebinding;
+
         protected override void OnControlPointChanged(ValueChangedEvent<EffectControlPoint> point)
         {
             if (point.NewValue != null)
             {
+                isRebinding = true;
+
                 kiai.Current = point.NewValue.KiaiModeBindable;
-                kiai.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
-
                 omitBarLine.Current = point.NewValue.OmitFirstBarLineBindable;
-                omitBarLine.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
-
                 scrollSpeedSlider.Current = point.NewValue.ScrollSpeedBindable;
-                scrollSpeedSlider.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
+
+                isRebinding = false;
             }
         }
 

osu.Game/Screens/Edit/Timing/TimingScreen.cs

@@ -19,7 +19,7 @@ namespace osu.Game.Screens.Edit.Timing
     public class TimingScreen : EditorScreenWithTimeline
     {
         [Cached]
-        private Bindable<ControlPointGroup> selectedGroup = new Bindable<ControlPointGroup>();
+        public readonly Bindable<ControlPointGroup> SelectedGroup = new Bindable<ControlPointGroup>();
 
         public TimingScreen()
             : base(EditorScreenMode.Timing)
@@ -132,6 +132,40 @@ namespace osu.Game.Screens.Edit.Timing
                 }, true);
             }
 
+            protected override void Update()
+            {
+                base.Update();
+
+                trackActivePoint();
+            }
+
+            /// <summary>
+            /// Given the user has selected a control point group, we want to track any group which is
+            /// active at the current point in time which matches the type the user has selected.
+            ///
+            /// So if the user is currently looking at a timing point and seeks into the future, a
+            /// future timing point would be automatically selected if it is now the new "current" point.
+            /// </summary>
+            private void trackActivePoint()
+            {
+                // For simplicity only match on the first type of the active control point.
+                var selectedPointType = selectedGroup.Value?.ControlPoints.FirstOrDefault()?.GetType();
+
+                if (selectedPointType != null)
+                {
+                    // We don't have an efficient way of looking up groups currently, only individual point types.
+                    // To improve the efficiency of this in the future, we should reconsider the overall structure of ControlPointInfo.
+
+                    // Find the next group which has the same type as the selected one.
+                    var found = Beatmap.ControlPointInfo.Groups
+                                       .Where(g => g.ControlPoints.Any(cp => cp.GetType() == selectedPointType))
+                                       .LastOrDefault(g => g.Time <= clock.CurrentTimeAccurate);
+
+                    if (found != null)
+                        selectedGroup.Value = found;
+                }
+            }
+
             private void delete()
             {
                 if (selectedGroup.Value == null)

osu.Game/Screens/Edit/Timing/TimingSection.cs

@@ -28,15 +28,31 @@ namespace osu.Game.Screens.Edit.Timing
             });
         }
 
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            bpmTextEntry.Current.BindValueChanged(_ => saveChanges());
+            timeSignature.Current.BindValueChanged(_ => saveChanges());
+
+            void saveChanges()
+            {
+                if (!isRebinding) ChangeHandler?.SaveState();
+            }
+        }
+
+        private bool isRebinding;
+
         protected override void OnControlPointChanged(ValueChangedEvent<TimingControlPoint> point)
         {
             if (point.NewValue != null)
             {
-                bpmTextEntry.Bindable = point.NewValue.BeatLengthBindable;
+                isRebinding = true;
-                bpmTextEntry.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
 
+                bpmTextEntry.Bindable = point.NewValue.BeatLengthBindable;
                 timeSignature.Current = point.NewValue.TimeSignatureBindable;
-                timeSignature.Current.BindValueChanged(_ => ChangeHandler?.SaveState());
+
+                isRebinding = false;
             }
         }