Merge pull request #1502 from andy840119/refactor-note-utils

Refactor the Note or Notes utils.

osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Formats/KaraokeLegacyBeatmapDecoderTest.cs

@@ -10,6 +10,7 @@
 using osu.Game.Rulesets.Karaoke.Beatmaps;
 using osu.Game.Rulesets.Karaoke.Beatmaps.Formats;
 using osu.Game.Rulesets.Karaoke.Objects;
+using osu.Game.Rulesets.Karaoke.Tests.Helper;
 using osu.Game.Rulesets.Karaoke.Tests.Resources;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Tests.Beatmaps;
@@ -129,5 +130,36 @@ private static void testNote(string expectedText, int expectedTone, bool expecte
             Assert.AreEqual(expectedTone, actualNote.Tone.Scale);
             Assert.AreEqual(expectedHalf, actualNote.Tone.Half);
         }
+
+        [TestCase(0, 1, new double[] { 1000, 3000 })]
+        [TestCase(0, 0.5, new double[] { 1000, 1500 })]
+        [TestCase(0.5, 0.5, new double[] { 2500, 1500 })]
+        [TestCase(0.3, 1, null)] // start + duration should not exceed 1
+        public void TestSliceNoteTime(double startPercentage, double durationPercentage, double[]? expected)
+        {
+            var lyric = new Lyric
+            {
+                TimeTags = TestCaseTagHelper.ParseTimeTags(new[] { "[0,start]:1000", "[1,start]:4000" }),
+            };
+
+            // start time will be 1000, and duration will be 3000.
+            var note = new Note
+            {
+                ReferenceLyric = lyric,
+                ReferenceTimeTagIndex = 0
+            };
+
+            if (expected != null)
+            {
+                var sliceNote = KaraokeLegacyBeatmapDecoder.SliceNote(note, startPercentage, durationPercentage);
+
+                Assert.AreEqual(expected[0], sliceNote.StartTime);
+                Assert.AreEqual(expected[1], sliceNote.Duration);
+            }
+            else
+            {
+                Assert.IsNull(expected);
+            }
+        }
     }
 }

osu.Game.Rulesets.Karaoke.Tests/Utils/NoteUtilsTest.cs

@@ -3,45 +3,12 @@
 
 using NUnit.Framework;
 using osu.Game.Rulesets.Karaoke.Objects;
-using osu.Game.Rulesets.Karaoke.Tests.Helper;
 using osu.Game.Rulesets.Karaoke.Utils;
 
 namespace osu.Game.Rulesets.Karaoke.Tests.Utils
 {
     public class NoteUtilsTest
     {
-        [TestCase(0, 1, new double[] { 1000, 3000 })]
-        [TestCase(0, 0.5, new double[] { 1000, 1500 })]
-        [TestCase(0.5, 0.5, new double[] { 2500, 1500 })]
-        [TestCase(0.3, 0.4, new double[] { 1900, 1200 })]
-        [TestCase(0.3, 1, null)] // start + duration should not exceed 1
-        public void TestSliceNoteTime(double startPercentage, double durationPercentage, double[]? expected)
-        {
-            var lyric = new Lyric
-            {
-                TimeTags = TestCaseTagHelper.ParseTimeTags(new[] { "[0,start]:1000", "[1,start]:4000" }),
-            };
-
-            // start time will be 1000, and duration will be 3000.
-            var note = new Note
-            {
-                ReferenceLyric = lyric,
-                ReferenceTimeTagIndex = 0
-            };
-
-            if (expected != null)
-            {
-                var sliceNote = NoteUtils.SliceNote(note, startPercentage, durationPercentage);
-
-                Assert.AreEqual(expected[0], sliceNote.StartTime);
-                Assert.AreEqual(expected[1], sliceNote.Duration);
-            }
-            else
-            {
-                Assert.IsNull(expected);
-            }
-        }
-
         [TestCase("karaoke", "", false, "karaoke")]
         [TestCase("karaoke", "ka- ra- o- ke-", false, "karaoke")]
         [TestCase("", "ka- ra- o- ke-", false, "")]

osu.Game.Rulesets.Karaoke/Beatmaps/Formats/KaraokeLegacyBeatmapDecoder.cs

@@ -162,7 +162,7 @@ private void processNotes(Beatmap beatmap, IList<string> lines)
                             }
 
                             // Split note and apply them
-                            var splitDefaultNote = NoteUtils.SliceNote(defaultNote, startPercentage, percentage);
+                            var splitDefaultNote = SliceNote(defaultNote, startPercentage, percentage);
                             startPercentage += percentage;
                             if (!string.IsNullOrEmpty(ruby))
                                 splitDefaultNote.Text = ruby;
@@ -254,5 +254,21 @@ private void processTranslate(Beatmap beatmap, IEnumerable<string> translateLine
 
             beatmap.HitObjects.Add(dictionary);
         }
+
+        internal static Note SliceNote(Note note, double startPercentage, double durationPercentage)
+        {
+            if (startPercentage < 0 || startPercentage + durationPercentage > 1)
+                throw new ArgumentOutOfRangeException($"{nameof(Note)} cannot assign split range of start from {startPercentage} and duration {durationPercentage}");
+
+            double durationFromStartTime = note.Duration * startPercentage;
+            double secondNoteDuration = note.Duration * (1 - startPercentage - durationPercentage);
+
+            // todo: there's no need to create the new note.
+            var newNote = note.DeepClone();
+            newNote.StartTimeOffset = note.StartTimeOffset + durationFromStartTime;
+            newNote.EndTimeOffset = note.EndTimeOffset - secondNoteDuration;
+
+            return newNote;
+        }
     }
 }

osu.Game.Rulesets.Karaoke/Utils/NoteUtils.cs

@@ -1,37 +1,12 @@
 // Copyright (c) andy840119 <[email protected]>. Licensed under the GPL Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System;
 using osu.Game.Rulesets.Karaoke.Objects;
 
 namespace osu.Game.Rulesets.Karaoke.Utils
 {
     public static class NoteUtils
     {
-        public static Note SliceNote(Note note, double startPercentage, double durationPercentage)
-        {
-            if (startPercentage < 0 || startPercentage + durationPercentage > 1)
-                throw new ArgumentOutOfRangeException($"{nameof(Note)} cannot assign split range of start from {startPercentage} and duration {durationPercentage}");
-
-            double startTime = note.StartTime + note.Duration * startPercentage;
-            double duration = note.Duration * durationPercentage;
-
-            return CopyByTime(note, startTime, duration);
-        }
-
-        public static Note CopyByTime(Note originNote, double startTime, double duration)
-        {
-            double fixedStartTime = originNote.StartTime - originNote.StartTimeOffset;
-            double fixedEndTime = originNote.EndTime - originNote.EndTimeOffset;
-            double endTime = startTime + duration;
-
-            var note = originNote.DeepClone();
-            note.StartTimeOffset = startTime - fixedStartTime;
-            note.EndTimeOffset = endTime - fixedEndTime;
-
-            return note;
-        }
-
         /// <summary>
         /// Get the display text while gameplay or in editor.
         /// </summary>

osu.Game.Rulesets.Karaoke/Utils/NotesUtils.cs

@@ -19,14 +19,14 @@ public static Tuple<Note, Note> SplitNote(Note note, double percentage = 0.5)
                     throw new InvalidOperationException($"{nameof(percentage)} cannot be {0} or {1}.");
             }
 
-            double firstNoteStartTime = note.StartTime;
             double firstNoteDuration = note.Duration * percentage;
-
-            double secondNoteStartTime = firstNoteStartTime + firstNoteDuration;
             double secondNoteDuration = note.Duration * (1 - percentage);
 
-            var firstNote = NoteUtils.CopyByTime(note, firstNoteStartTime, firstNoteDuration);
-            var secondNote = NoteUtils.CopyByTime(note, secondNoteStartTime, secondNoteDuration);
+            var firstNote = note.DeepClone();
+            firstNote.EndTimeOffset = note.EndTimeOffset - secondNoteDuration;
+
+            var secondNote = note.DeepClone();
+            secondNote.StartTimeOffset = note.StartTimeOffset + firstNoteDuration;
 
             return new Tuple<Note, Note>(firstNote, secondNote);
         }
@@ -39,10 +39,11 @@ public static Note CombineNote(Note firstLyric, Note secondLyric)
             if (firstLyric.ReferenceTimeTagIndex != secondLyric.ReferenceTimeTagIndex)
                 throw new InvalidOperationException($"{nameof(firstLyric.ReferenceTimeTagIndex)} and {nameof(secondLyric.ReferenceTimeTagIndex)} should be same.");
 
-            double startTime = Math.Min(firstLyric.StartTime, secondLyric.StartTime);
-            double endTime = Math.Max(firstLyric.EndTime, secondLyric.EndTime);
+            var combinedLyric = firstLyric.DeepClone();
+            combinedLyric.StartTimeOffset = Math.Min(firstLyric.StartTimeOffset, secondLyric.StartTimeOffset);
+            combinedLyric.EndTimeOffset = Math.Max(firstLyric.EndTimeOffset, secondLyric.EndTimeOffset);
 
-            return NoteUtils.CopyByTime(firstLyric, startTime, endTime - startTime);
+            return combinedLyric;
         }
     }
 }