From 098d643955fc7fd05a35f1106451b1fb12290ea4 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 23 Jan 2020 14:39:56 +0900
Subject: [PATCH 1/2] Move beat snapping to its own interface

---
 ...tSceneHitObjectComposerDistanceSnapping.cs |  9 +++++-
 .../Visual/Editor/TestSceneComposeScreen.cs   |  2 ++
 .../Editor/TestSceneHitObjectComposer.cs      |  1 +
 osu.Game/Rulesets/Edit/HitObjectComposer.cs   | 26 ++++------------
 osu.Game/Rulesets/Edit/IBeatSnapProvider.cs   | 30 +++++++++++++++++++
 osu.Game/Screens/Edit/Editor.cs               | 15 ++++++++--
 osu.Game/Screens/Edit/EditorBeatmap.cs        | 20 +++++++++++--
 7 files changed, 78 insertions(+), 25 deletions(-)
 create mode 100644 osu.Game/Rulesets/Edit/IBeatSnapProvider.cs

diff --git a/osu.Game.Tests/Editor/TestSceneHitObjectComposerDistanceSnapping.cs b/osu.Game.Tests/Editor/TestSceneHitObjectComposerDistanceSnapping.cs
index 2d336bd19c..e825df5a3f 100644
--- a/osu.Game.Tests/Editor/TestSceneHitObjectComposerDistanceSnapping.cs
+++ b/osu.Game.Tests/Editor/TestSceneHitObjectComposerDistanceSnapping.cs
@@ -5,6 +5,7 @@ using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Beatmaps;
 using osu.Game.Rulesets.Osu.Edit;
@@ -19,7 +20,13 @@ namespace osu.Game.Tests.Editor
         private TestHitObjectComposer composer;
 
         [Cached(typeof(EditorBeatmap))]
-        private readonly EditorBeatmap editorBeatmap = new EditorBeatmap(new OsuBeatmap());
+        [Cached(typeof(IBeatSnapProvider))]
+        private readonly EditorBeatmap editorBeatmap;
+
+        public TestSceneHitObjectComposerDistanceSnapping()
+        {
+            editorBeatmap = new EditorBeatmap(new OsuBeatmap(), BeatDivisor);
+        }
 
         [SetUp]
         public void Setup() => Schedule(() =>
diff --git a/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs b/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs
index 3562689482..a8830824c0 100644
--- a/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs
+++ b/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs
@@ -3,6 +3,7 @@
 
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Beatmaps;
 using osu.Game.Screens.Edit;
@@ -14,6 +15,7 @@ namespace osu.Game.Tests.Visual.Editor
     public class TestSceneComposeScreen : EditorClockTestScene
     {
         [Cached(typeof(EditorBeatmap))]
+        [Cached(typeof(IBeatSnapProvider))]
         private readonly EditorBeatmap editorBeatmap =
             new EditorBeatmap(new OsuBeatmap
             {
diff --git a/osu.Game.Tests/Visual/Editor/TestSceneHitObjectComposer.cs b/osu.Game.Tests/Visual/Editor/TestSceneHitObjectComposer.cs
index c001c83877..e41c2427fb 100644
--- a/osu.Game.Tests/Visual/Editor/TestSceneHitObjectComposer.cs
+++ b/osu.Game.Tests/Visual/Editor/TestSceneHitObjectComposer.cs
@@ -66,6 +66,7 @@ namespace osu.Game.Tests.Visual.Editor
             Dependencies.CacheAs<IAdjustableClock>(clock);
             Dependencies.CacheAs<IFrameBasedClock>(clock);
             Dependencies.CacheAs(editorBeatmap);
+            Dependencies.CacheAs<IBeatSnapProvider>(editorBeatmap);
 
             Child = new OsuHitObjectComposer(new OsuRuleset());
         }
diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs
index bfaa7e8872..e1ce294581 100644
--- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs
+++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs
@@ -46,7 +46,7 @@ namespace osu.Game.Rulesets.Edit
         private IAdjustableClock adjustableClock { get; set; }
 
         [Resolved]
-        private BindableBeatDivisor beatDivisor { get; set; }
+        private IBeatSnapProvider beatSnapProvider { get; set; }
 
         private IBeatmapProcessor beatmapProcessor;
 
@@ -257,40 +257,26 @@ namespace osu.Game.Rulesets.Edit
         public override float GetBeatSnapDistanceAt(double referenceTime)
         {
             DifficultyControlPoint difficultyPoint = EditorBeatmap.ControlPointInfo.DifficultyPointAt(referenceTime);
-            return (float)(100 * EditorBeatmap.BeatmapInfo.BaseDifficulty.SliderMultiplier * difficultyPoint.SpeedMultiplier / beatDivisor.Value);
+            return (float)(100 * EditorBeatmap.BeatmapInfo.BaseDifficulty.SliderMultiplier * difficultyPoint.SpeedMultiplier / beatSnapProvider.BeatDivisor);
         }
 
         public override float DurationToDistance(double referenceTime, double duration)
         {
-            double beatLength = EditorBeatmap.ControlPointInfo.TimingPointAt(referenceTime).BeatLength / beatDivisor.Value;
+            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime, beatSnapProvider.BeatDivisor);
             return (float)(duration / beatLength * GetBeatSnapDistanceAt(referenceTime));
         }
 
         public override double DistanceToDuration(double referenceTime, float distance)
         {
-            double beatLength = EditorBeatmap.ControlPointInfo.TimingPointAt(referenceTime).BeatLength / beatDivisor.Value;
+            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime, beatSnapProvider.BeatDivisor);
             return distance / GetBeatSnapDistanceAt(referenceTime) * beatLength;
         }
 
         public override double GetSnappedDurationFromDistance(double referenceTime, float distance)
-            => beatSnap(referenceTime, DistanceToDuration(referenceTime, distance));
+            => beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance), beatSnapProvider.BeatDivisor);
 
         public override float GetSnappedDistanceFromDistance(double referenceTime, float distance)
-            => DurationToDistance(referenceTime, beatSnap(referenceTime, DistanceToDuration(referenceTime, distance)));
-
-        /// <summary>
-        /// Snaps a duration to the closest beat of a timing point applicable at the reference time.
-        /// </summary>
-        /// <param name="referenceTime">The time of the timing point which <paramref name="duration"/> resides in.</param>
-        /// <param name="duration">The duration to snap.</param>
-        /// <returns>A value that represents <paramref name="duration"/> snapped to the closest beat of the timing point.</returns>
-        private double beatSnap(double referenceTime, double duration)
-        {
-            double beatLength = EditorBeatmap.ControlPointInfo.TimingPointAt(referenceTime).BeatLength / beatDivisor.Value;
-
-            // A 1ms offset prevents rounding errors due to minute variations in duration
-            return (int)((duration + 1) / beatLength) * beatLength;
-        }
+            => DurationToDistance(referenceTime, beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance), beatSnapProvider.BeatDivisor));
 
         protected override void Dispose(bool isDisposing)
         {
diff --git a/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs b/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs
new file mode 100644
index 0000000000..ed6e08d054
--- /dev/null
+++ b/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs
@@ -0,0 +1,30 @@
+// 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.
+
+namespace osu.Game.Rulesets.Edit
+{
+    public interface IBeatSnapProvider
+    {
+        /// <summary>
+        /// Snaps a duration to the closest beat of a timing point applicable at the reference time.
+        /// </summary>
+        /// <param name="referenceTime">The time of the timing point which <paramref name="duration"/> resides in.</param>
+        /// <param name="duration">The duration to snap.</param>
+        /// <param name="beatDivisor">The divisor to use for snapping purposes.</param>
+        /// <returns>A value that represents <paramref name="duration"/> snapped to the closest beat of the timing point.</returns>
+        double SnapTime(double referenceTime, double duration, int beatDivisor);
+
+        /// <summary>
+        /// Get the most appropriate beat length at a given time.
+        /// </summary>
+        /// <param name="referenceTime">A reference time used for lookup.</param>
+        /// <param name="beatDivisor">The divisor to use for snapping purposes.</param>
+        /// <returns>The most appropriate beat length.</returns>
+        double GetBeatLengthAtTime(double referenceTime, int beatDivisor);
+
+        /// <summary>
+        /// Returns the current beat divisor.
+        /// </summary>
+        int BeatDivisor { get; }
+    }
+}
diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs
index e5cb319fb9..820915e8e3 100644
--- a/osu.Game/Screens/Edit/Editor.cs
+++ b/osu.Game/Screens/Edit/Editor.cs
@@ -26,6 +26,7 @@ using osu.Framework.Input.Bindings;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.Cursor;
 using osu.Game.Input.Bindings;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Screens.Edit.Compose;
 using osu.Game.Screens.Edit.Setup;
 using osu.Game.Screens.Edit.Timing;
@@ -34,7 +35,8 @@ using osu.Game.Users;
 
 namespace osu.Game.Screens.Edit
 {
-    public class Editor : ScreenWithBeatmapBackground, IKeyBindingHandler<GlobalAction>
+    [Cached(typeof(IBeatSnapProvider))]
+    public class Editor : ScreenWithBeatmapBackground, IKeyBindingHandler<GlobalAction>, IBeatSnapProvider
     {
         public override float BackgroundParallaxAmount => 0.1f;
 
@@ -77,11 +79,14 @@ namespace osu.Game.Screens.Edit
             clock.ChangeSource(sourceClock);
 
             playableBeatmap = Beatmap.Value.GetPlayableBeatmap(Beatmap.Value.BeatmapInfo.Ruleset);
-            editorBeatmap = new EditorBeatmap(playableBeatmap);
+            editorBeatmap = new EditorBeatmap(playableBeatmap, beatDivisor);
 
             dependencies.CacheAs<IFrameBasedClock>(clock);
             dependencies.CacheAs<IAdjustableClock>(clock);
+
+            // todo: remove caching of this and consume via editorBeatmap?
             dependencies.Cache(beatDivisor);
+
             dependencies.CacheAs(editorBeatmap);
 
             EditorMenuBar menuBar;
@@ -347,5 +352,11 @@ namespace osu.Game.Screens.Edit
             saveBeatmap();
             beatmapManager.Export(Beatmap.Value.BeatmapSetInfo);
         }
+
+        public double SnapTime(double referenceTime, double duration, int beatDivisor) => editorBeatmap.SnapTime(referenceTime, duration, beatDivisor);
+
+        public double GetBeatLengthAtTime(double referenceTime, int beatDivisor) => editorBeatmap.GetBeatLengthAtTime(referenceTime, beatDivisor);
+
+        public int BeatDivisor => beatDivisor.Value;
     }
 }
diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs
index 6ed74dfdb0..7c037c435f 100644
--- a/osu.Game/Screens/Edit/EditorBeatmap.cs
+++ b/osu.Game/Screens/Edit/EditorBeatmap.cs
@@ -8,11 +8,12 @@ using osu.Framework.Bindables;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects;
 
 namespace osu.Game.Screens.Edit
 {
-    public class EditorBeatmap : IBeatmap
+    public class EditorBeatmap : IBeatmap, IBeatSnapProvider
     {
         /// <summary>
         /// Invoked when a <see cref="HitObject"/> is added to this <see cref="EditorBeatmap"/>.
@@ -31,11 +32,14 @@ namespace osu.Game.Screens.Edit
 
         public readonly IBeatmap PlayableBeatmap;
 
+        private readonly BindableBeatDivisor beatDivisor;
+
         private readonly Dictionary<HitObject, Bindable<double>> startTimeBindables = new Dictionary<HitObject, Bindable<double>>();
 
-        public EditorBeatmap(IBeatmap playableBeatmap)
+        public EditorBeatmap(IBeatmap playableBeatmap, BindableBeatDivisor beatDivisor = null)
         {
             PlayableBeatmap = playableBeatmap;
+            this.beatDivisor = beatDivisor;
 
             foreach (var obj in HitObjects)
                 trackStartTime(obj);
@@ -121,5 +125,17 @@ namespace osu.Game.Screens.Edit
 
             return list.Count - 1;
         }
+
+        public double SnapTime(double referenceTime, double duration, int beatDivisor)
+        {
+            double beatLength = GetBeatLengthAtTime(referenceTime, beatDivisor);
+
+            // A 1ms offset prevents rounding errors due to minute variations in duration
+            return (int)((duration + 1) / beatLength) * beatLength;
+        }
+
+        public double GetBeatLengthAtTime(double referenceTime, int beatDivisor) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor;
+
+        public int BeatDivisor => beatDivisor?.Value ?? 1;
     }
 }

From ccf911884be0e8a822b4ed4215413cd103752758 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 23 Jan 2020 15:31:56 +0900
Subject: [PATCH 2/2] Remove passed in BaetDivisor

---
 osu.Game/Rulesets/Edit/HitObjectComposer.cs | 8 ++++----
 osu.Game/Rulesets/Edit/IBeatSnapProvider.cs | 6 ++----
 osu.Game/Screens/Edit/Editor.cs             | 4 ++--
 osu.Game/Screens/Edit/EditorBeatmap.cs      | 6 +++---
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs
index e1ce294581..dbd29a167c 100644
--- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs
+++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs
@@ -262,21 +262,21 @@ namespace osu.Game.Rulesets.Edit
 
         public override float DurationToDistance(double referenceTime, double duration)
         {
-            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime, beatSnapProvider.BeatDivisor);
+            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime);
             return (float)(duration / beatLength * GetBeatSnapDistanceAt(referenceTime));
         }
 
         public override double DistanceToDuration(double referenceTime, float distance)
         {
-            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime, beatSnapProvider.BeatDivisor);
+            double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceTime);
             return distance / GetBeatSnapDistanceAt(referenceTime) * beatLength;
         }
 
         public override double GetSnappedDurationFromDistance(double referenceTime, float distance)
-            => beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance), beatSnapProvider.BeatDivisor);
+            => beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance));
 
         public override float GetSnappedDistanceFromDistance(double referenceTime, float distance)
-            => DurationToDistance(referenceTime, beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance), beatSnapProvider.BeatDivisor));
+            => DurationToDistance(referenceTime, beatSnapProvider.SnapTime(referenceTime, DistanceToDuration(referenceTime, distance)));
 
         protected override void Dispose(bool isDisposing)
         {
diff --git a/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs b/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs
index ed6e08d054..e1daafaebe 100644
--- a/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs
+++ b/osu.Game/Rulesets/Edit/IBeatSnapProvider.cs
@@ -10,17 +10,15 @@ namespace osu.Game.Rulesets.Edit
         /// </summary>
         /// <param name="referenceTime">The time of the timing point which <paramref name="duration"/> resides in.</param>
         /// <param name="duration">The duration to snap.</param>
-        /// <param name="beatDivisor">The divisor to use for snapping purposes.</param>
         /// <returns>A value that represents <paramref name="duration"/> snapped to the closest beat of the timing point.</returns>
-        double SnapTime(double referenceTime, double duration, int beatDivisor);
+        double SnapTime(double referenceTime, double duration);
 
         /// <summary>
         /// Get the most appropriate beat length at a given time.
         /// </summary>
         /// <param name="referenceTime">A reference time used for lookup.</param>
-        /// <param name="beatDivisor">The divisor to use for snapping purposes.</param>
         /// <returns>The most appropriate beat length.</returns>
-        double GetBeatLengthAtTime(double referenceTime, int beatDivisor);
+        double GetBeatLengthAtTime(double referenceTime);
 
         /// <summary>
         /// Returns the current beat divisor.
diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs
index 820915e8e3..8d66cef16e 100644
--- a/osu.Game/Screens/Edit/Editor.cs
+++ b/osu.Game/Screens/Edit/Editor.cs
@@ -353,9 +353,9 @@ namespace osu.Game.Screens.Edit
             beatmapManager.Export(Beatmap.Value.BeatmapSetInfo);
         }
 
-        public double SnapTime(double referenceTime, double duration, int beatDivisor) => editorBeatmap.SnapTime(referenceTime, duration, beatDivisor);
+        public double SnapTime(double referenceTime, double duration) => editorBeatmap.SnapTime(referenceTime, duration);
 
-        public double GetBeatLengthAtTime(double referenceTime, int beatDivisor) => editorBeatmap.GetBeatLengthAtTime(referenceTime, beatDivisor);
+        public double GetBeatLengthAtTime(double referenceTime) => editorBeatmap.GetBeatLengthAtTime(referenceTime);
 
         public int BeatDivisor => beatDivisor.Value;
     }
diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs
index 7c037c435f..a67a0ff0f1 100644
--- a/osu.Game/Screens/Edit/EditorBeatmap.cs
+++ b/osu.Game/Screens/Edit/EditorBeatmap.cs
@@ -126,15 +126,15 @@ namespace osu.Game.Screens.Edit
             return list.Count - 1;
         }
 
-        public double SnapTime(double referenceTime, double duration, int beatDivisor)
+        public double SnapTime(double referenceTime, double duration)
         {
-            double beatLength = GetBeatLengthAtTime(referenceTime, beatDivisor);
+            double beatLength = GetBeatLengthAtTime(referenceTime);
 
             // A 1ms offset prevents rounding errors due to minute variations in duration
             return (int)((duration + 1) / beatLength) * beatLength;
         }
 
-        public double GetBeatLengthAtTime(double referenceTime, int beatDivisor) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor;
+        public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor;
 
         public int BeatDivisor => beatDivisor?.Value ?? 1;
     }