Merge pull request #2062 from andy840119/change-timing-point-id-type

Change timing point id type
karaoke-dev · Jul 9, 2023 · 5eca2d0 · 5eca2d0
2 parents f597a21 + 9c755b3
commit 5eca2d0
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 35 deletions.
diff --git a/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Stages/Classic/ClassicLyricTimingInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Stages/Classic/ClassicLyricTimingInfoTest.cs
@@ -25,7 +25,7 @@ public void TestAddTimingPoint()
         });
 
         Assert.AreEqual(1, timingInfo.Timings.Count);
-        Assert.AreEqual(1, timingInfo.Timings[0].ID);
+        Assert.IsNotEmpty(timingInfo.Timings[0].ID.ToString());
         Assert.AreEqual(1000, timingInfo.Timings[0].Time);
     }
 

diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs
@@ -130,7 +130,7 @@ public void TestCheckTimingInfoMappingHasNoTiming()
             timingInfos.AddTimingPoint(x => x.Time = MIN_TIMING_INTERVAL + 1);
 
             // should have error because mapping value is empty.
-            timingInfos.Mappings.Add(lyrics.First().ID, Array.Empty<int>());
+            timingInfos.Mappings.Add(lyrics.First().ID, Array.Empty<ElementId>());
         });
         AssertNotOk<IssueTemplateTimingInfoMappingHasNoTiming>(getContext(beatmap));
     }
@@ -150,7 +150,7 @@ public void TestCheckTimingInfoTimingNotExist()
             timingInfos.AddTimingPoint(x => x.Time = MIN_TIMING_INTERVAL + 1);
 
             // should have error because mapping value is not exist.
-            timingInfos.Mappings.Add(lyrics.First().ID, new[] { 100 });
+            timingInfos.Mappings.Add(lyrics.First().ID, new[] { ElementId.NewElementId() });
         });
         AssertNotOk<IssueTemplateTimingInfoTimingNotExist>(getContext(beatmap));
     }

diff --git a/...oke.Tests/Editor/Generator/Beatmaps/Stages/Classic/ClassicLyricTimingInfoGeneratorTest.cs b/...oke.Tests/Editor/Generator/Beatmaps/Stages/Classic/ClassicLyricTimingInfoGeneratorTest.cs
@@ -128,6 +128,8 @@ public void TestGenerateWithThreeLyrics()
     protected override void AssertEqual(ClassicLyricTimingInfo expected, ClassicLyricTimingInfo actual)
     {
         Assert.AreEqual(expected.Timings.Select(x => x.Time), actual.Timings.Select(x => x.Time));
-        Assert.AreEqual(expected.Mappings, actual.Mappings);
+
+        // because we cannot check the id in the mapping value, so just check the key.
+        Assert.AreEqual(expected.Mappings.Keys, actual.Mappings.Keys);
     }
 }
diff --git a/osu.Game.Rulesets.Karaoke/Beatmaps/Stages/Classic/ClassicLyricTimingInfo.cs b/osu.Game.Rulesets.Karaoke/Beatmaps/Stages/Classic/ClassicLyricTimingInfo.cs
@@ -30,8 +30,12 @@ public class ClassicLyricTimingInfo
 
     private readonly Bindable<int> mappingVersion = new();
 
-    // todo: should be private.
-    public BindableDictionary<ElementId, int[]> Mappings = new();
+    /// <summary>
+    /// Mapping between <see cref="Lyric.ID"/> and <see cref="ClassicLyricTimingPoint.ID"/>
+    /// This is the 1st mapping roles.
+    /// todo: should be private.
+    /// </summary>
+    public BindableDictionary<ElementId, ElementId[]> Mappings = new();
 
     public ClassicLyricTimingInfo()
     {
@@ -88,21 +92,12 @@ void onTimingChanged()
 
     public ClassicLyricTimingPoint AddTimingPoint(Action<ClassicLyricTimingPoint>? action = null)
     {
-        int id = getNewTimingPointId();
-        var timingPoint = new ClassicLyricTimingPoint(id);
+        var timingPoint = new ClassicLyricTimingPoint();
 
         action?.Invoke(timingPoint);
         Timings.Add(timingPoint);
 
         return timingPoint;
-
-        int getNewTimingPointId()
-        {
-            if (Timings.Count == 0)
-                return 1;
-
-            return Timings.Max(x => x.ID) + 1;
-        }
     }
 
     public void RemoveTimingPoint(ClassicLyricTimingPoint point)
@@ -115,12 +110,12 @@ public void RemoveTimingPoint(ClassicLyricTimingPoint point)
     public void AddToMapping(ClassicLyricTimingPoint point, Lyric lyric)
     {
         var key = lyric.ID;
-        int value = point.ID;
+        var value = point.ID;
 
         if (!Timings.Contains(point))
             throw new InvalidOperationException($"{nameof(point)} does ont in the {nameof(point)}.");
 
-        if (Mappings.TryGetValue(key, out int[]? timingIds))
+        if (Mappings.TryGetValue(key, out ElementId[]? timingIds))
         {
             Mappings[key] = timingIds.Concat(new[] { value }).ToArray();
         }
@@ -133,12 +128,12 @@ public void AddToMapping(ClassicLyricTimingPoint point, Lyric lyric)
     public void RemoveFromMapping(ClassicLyricTimingPoint point, Lyric lyric)
     {
         var key = lyric.ID;
-        int value = point.ID;
+        var value = point.ID;
 
         if (!Timings.Contains(point))
             throw new InvalidOperationException($"{nameof(point)} does ont in the {nameof(point)}.");
 
-        if (!Mappings.TryGetValue(key, out int[]? values))
+        if (!Mappings.TryGetValue(key, out ElementId[]? values))
             return;
 
         if (values.All(x => x == point.ID))
@@ -153,9 +148,9 @@ public void RemoveFromMapping(ClassicLyricTimingPoint point, Lyric lyric)
 
     public void ClearTimingPointFromMapping(ClassicLyricTimingPoint point)
     {
-        int value = point.ID;
+        var value = point.ID;
 
-        foreach ((var key, int[]? values) in Mappings)
+        foreach ((var key, ElementId[]? values) in Mappings)
         {
             if (values.All(x => x == point.ID))
             {
@@ -185,7 +180,7 @@ public void ClearLyricFromMapping(Lyric lyric)
 
     public IEnumerable<ClassicLyricTimingPoint> GetLyricTimingPoints(Lyric lyric)
     {
-        if (!Mappings.TryGetValue(lyric.ID, out int[]? ids))
+        if (!Mappings.TryGetValue(lyric.ID, out ElementId[]? ids))
             return Array.Empty<ClassicLyricTimingPoint>();
 
         return SortedTimings.Where(x => ids.Contains(x.ID));

diff --git a/osu.Game.Rulesets.Karaoke/Beatmaps/Stages/Classic/ClassicLyricTimingPoint.cs b/osu.Game.Rulesets.Karaoke/Beatmaps/Stages/Classic/ClassicLyricTimingPoint.cs
@@ -2,25 +2,17 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using System.Text.Json.Serialization;
+using Newtonsoft.Json;
 using osu.Framework.Bindables;
 using osu.Game.Rulesets.Karaoke.Utils;
 using osu.Game.Utils;
 
 namespace osu.Game.Rulesets.Karaoke.Beatmaps.Stages.Classic;
 
-public class ClassicLyricTimingPoint : IDeepCloneable<ClassicLyricTimingPoint>, IComparable<ClassicLyricTimingPoint>
+public class ClassicLyricTimingPoint : IDeepCloneable<ClassicLyricTimingPoint>, IComparable<ClassicLyricTimingPoint>, IHasPrimaryKey
 {
-    public ClassicLyricTimingPoint()
-    {
-    }
-
-    public ClassicLyricTimingPoint(int id)
-    {
-        ID = id;
-    }
-
-    public int ID { get; protected set; }
+    [JsonProperty]
+    public ElementId ID { get; private set; } = ElementId.NewElementId();
 
     [JsonIgnore]
     public readonly Bindable<double> TimeBindable = new();