From 1063d800b54dcaa28997031c802ad37104a12eaf Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 25 Sep 2024 22:59:24 +0800 Subject: [PATCH 1/3] Separate the singer and singer state into 2 different list. --- .../Beatmaps/Metadatas/SingerInfo.cs | 27 ++++++++++++++----- .../Beatmaps/BeatmapSingersChangeHandler.cs | 4 +-- .../Beatmaps/IBeatmapSingersChangeHandler.cs | 2 +- .../Settings/Singers/SingerEditSection.cs | 2 +- .../Singers/SingerRearrangeableList.cs | 5 ++-- .../Singers/SingerRearrangeableListItem.cs | 9 +++---- 6 files changed, 30 insertions(+), 19 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Beatmaps/Metadatas/SingerInfo.cs b/osu.Game.Rulesets.Karaoke/Beatmaps/Metadatas/SingerInfo.cs index dd8ea2098..92d772e80 100644 --- a/osu.Game.Rulesets.Karaoke/Beatmaps/Metadatas/SingerInfo.cs +++ b/osu.Game.Rulesets.Karaoke/Beatmaps/Metadatas/SingerInfo.cs @@ -14,13 +14,16 @@ public class SingerInfo public bool SupportSingerState { get; set; } // todo: should make the property as readonly. - public BindableList Singers { get; set; } = new(); + public BindableList Singers { get; set; } = new(); + + // todo: should make the property as readonly. + public BindableList SingerState { get; set; } = new(); public IEnumerable GetAllSingers() => - Singers.OfType().OrderBy(x => x.Order); + Singers.OrderBy(x => x.Order); public IEnumerable GetAllAvailableSingerStates(Singer singer) => - Singers.OfType().Where(x => x.MainSingerId == singer.ID).OrderBy(x => x.Order); + SingerState.Where(x => x.MainSingerId == singer.ID).OrderBy(x => x.Order); public IDictionary GetSingerByIds(ElementId[] singerIds) { @@ -58,7 +61,7 @@ public SingerState AddSingerState(Singer singer, Action? action = n var singerState = new SingerState(mainSingerId); action?.Invoke(singerState); - Singers.Add(singerState); + SingerState.Add(singerState); return singerState; } @@ -76,14 +79,24 @@ public bool RemoveSinger(ISinger singer) RemoveSinger(singerState); } - return Singers.Remove(singer); + return Singers.Remove(mainSinger); } - case SingerState: - return Singers.Remove(singer); + case SingerState singerState: + return SingerState.Remove(singerState); default: throw new InvalidCastException(); } } + + public bool HasSinger(ISinger singer) + { + return singer switch + { + Singer mainSinger => Singers.Contains(mainSinger), + SingerState singerState => SingerState.Contains(singerState), + _ => throw new InvalidCastException(), + }; + } } diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/BeatmapSingersChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/BeatmapSingersChangeHandler.cs index 691279460..67a5e239c 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/BeatmapSingersChangeHandler.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/BeatmapSingersChangeHandler.cs @@ -24,7 +24,7 @@ public partial class BeatmapSingersChangeHandler : BeatmapPropertyChangeHandler, private SingerInfo singerInfo => KaraokeBeatmap.SingerInfo; - public BindableList Singers => singerInfo.Singers; + public BindableList Singers => singerInfo.Singers; public void ChangeOrder(ISinger singer, int newIndex) { @@ -111,7 +111,7 @@ private void performSingerChanged(TSinger singer, Action actio { performSingerInfoChanged(singerInfo => { - if (!singerInfo.Singers.Contains(singer)) + if (!singerInfo.HasSinger(singer)) throw new InvalidOperationException("Singer should be in the beatmap"); action(singer); diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/IBeatmapSingersChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/IBeatmapSingersChangeHandler.cs index 9f313238f..299bf5fd0 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/IBeatmapSingersChangeHandler.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/IBeatmapSingersChangeHandler.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Beatmaps; public interface IBeatmapSingersChangeHandler { // todo: should use IBindableList eventually, but cannot do that because it's bind to selection item. - BindableList Singers { get; } + BindableList Singers { get; } void ChangeOrder(ISinger singer, int newIndex); diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Settings/Singers/SingerEditSection.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Settings/Singers/SingerEditSection.cs index 75755c993..cf103cb0d 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Settings/Singers/SingerEditSection.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Settings/Singers/SingerEditSection.cs @@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Karaoke.Screens.Edit.Beatmaps.Lyrics.Settings.Singer public partial class SingerEditSection : LyricPropertySection { - private readonly IBindableList bindableSingers = new BindableList(); + private readonly IBindableList bindableSingers = new BindableList(); private readonly IBindableList singerIndexes = new BindableList(); protected override LocalisableString Title => "Singer"; diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableList.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableList.cs index f009c7d86..b311b1a32 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableList.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableList.cs @@ -4,6 +4,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Graphics.Containers; +using osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas; using osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas.Types; using osu.Game.Rulesets.Karaoke.Graphics.Containers; using osu.Game.Rulesets.Karaoke.Screens.Edit.Beatmaps.Singers.Rows; @@ -11,7 +12,7 @@ namespace osu.Game.Rulesets.Karaoke.Screens.Edit.Beatmaps.Singers; -public partial class SingerRearrangeableList : OrderRearrangeableListContainer +public partial class SingerRearrangeableList : OrderRearrangeableListContainer { protected override Vector2 Spacing => new(0, 5); @@ -24,7 +25,7 @@ public SingerRearrangeableList() }; } - protected override OsuRearrangeableListItem CreateOsuDrawable(ISinger item) + protected override OsuRearrangeableListItem CreateOsuDrawable(Singer item) => new SingerRearrangeableListItem(item); protected override Drawable CreateBottomDrawable() diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableListItem.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableListItem.cs index 37ea496df..80e4ae762 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableListItem.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Singers/SingerRearrangeableListItem.cs @@ -15,20 +15,17 @@ namespace osu.Game.Rulesets.Karaoke.Screens.Edit.Beatmaps.Singers; -public partial class SingerRearrangeableListItem : OsuRearrangeableListItem +public partial class SingerRearrangeableListItem : OsuRearrangeableListItem { private Box dragAlert = null!; - public SingerRearrangeableListItem(ISinger item) + public SingerRearrangeableListItem(Singer item) : base(item) { } protected override Drawable CreateContent() { - if (Model is not Singer singer) - throw new InvalidCastException($"Currently we are only able to edit the {nameof(Singer)}."); - return new Container { Masking = true, @@ -42,7 +39,7 @@ protected override Drawable CreateContent() RelativeSizeAxes = Axes.Both, Alpha = 0, }, - new SingerLyricPlacementColumn(singer) + new SingerLyricPlacementColumn(Model) { RelativeSizeAxes = Axes.Both, }, From a142f8764f4dc277950f93e492436ba47d8a8ccd Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 25 Sep 2024 22:59:58 +0800 Subject: [PATCH 2/3] Fix the test broken. --- .../Beatmaps/Metadatas/SingerInfoTest.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Metadatas/SingerInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Metadatas/SingerInfoTest.cs index 2b83fb3e8..575ea8616 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Metadatas/SingerInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Beatmaps/Metadatas/SingerInfoTest.cs @@ -17,10 +17,10 @@ public void TestSingers() var singer = singerInfo.AddSinger(); var singerState = singerInfo.AddSingerState(singer); - var allSingers = singerInfo.Singers; - Assert.AreEqual(2, allSingers.Count); - Assert.AreEqual(singer, allSingers[0]); - Assert.AreEqual(singerState, allSingers[1]); + Assert.AreEqual(1, singerInfo.Singers.Count); + Assert.AreEqual(1, singerInfo.SingerState.Count); + Assert.AreEqual(singer, singerInfo.Singers[0]); + Assert.AreEqual(singerState, singerInfo.SingerState[0]); } [Test] @@ -102,7 +102,8 @@ public void TestAddSingerState() var singer = singerInfo.AddSinger(); var singerState = singerInfo.AddSingerState(singer); - Assert.AreEqual(2, singerInfo.Singers.Count); + Assert.AreEqual(1, singerInfo.Singers.Count); + Assert.AreEqual(1, singerInfo.SingerState.Count); Assert.IsNotEmpty(singerState.ID.ToString()); Assert.IsNotEmpty(singerState.MainSingerId.ToString()); } From dec89770ee79df14347c9fadccb982031b260389 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 25 Sep 2024 23:02:03 +0800 Subject: [PATCH 3/3] Remove the converter for the singer interface because the singer and singer state are stored into the different list, so there's no need to distinguish type. --- .../Converters/SingerConverterTest.cs | 94 ------------------- .../Converters/SingerConverter.cs | 20 ---- .../KaraokeJsonSerializableExtensions.cs | 3 - 3 files changed, 117 deletions(-) delete mode 100644 osu.Game.Rulesets.Karaoke.Tests/IO/Serialization/Converters/SingerConverterTest.cs delete mode 100644 osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/SingerConverter.cs diff --git a/osu.Game.Rulesets.Karaoke.Tests/IO/Serialization/Converters/SingerConverterTest.cs b/osu.Game.Rulesets.Karaoke.Tests/IO/Serialization/Converters/SingerConverterTest.cs deleted file mode 100644 index dbb49bdd8..000000000 --- a/osu.Game.Rulesets.Karaoke.Tests/IO/Serialization/Converters/SingerConverterTest.cs +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright (c) andy840119 . Licensed under the GPL Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using Newtonsoft.Json; -using NUnit.Framework; -using osu.Game.Rulesets.Karaoke.Beatmaps; -using osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas; -using osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas.Types; -using osu.Game.Rulesets.Karaoke.IO.Serialization.Converters; -using osu.Game.Rulesets.Karaoke.Tests.Helper; - -namespace osu.Game.Rulesets.Karaoke.Tests.IO.Serialization.Converters; - -public class SingerConverterTest : BaseSingleConverterTest -{ - private static readonly ElementId main_singer_id = TestCaseElementIdHelper.CreateElementIdByNumber(2); - - protected override IEnumerable CreateExtraConverts() - { - yield return new ElementIdConverter(); - } - - [Test] - public void TestMainSingerSerializer() - { - var singer = new Singer(); - - string expected = $"{{\"$type\":\"Singer\",\"id\":\"{singer.ID}\"}}"; - string actual = JsonConvert.SerializeObject(singer, CreateSettings()); - Assert.AreEqual(expected, actual); - } - - [Test] - public void TestMainSingerDeserializer() - { - var expected = new Singer(); - - string json = $"{{\"$type\":\"Singer\",\"id\":\"{expected.ID}\"}}"; - var actual = (Singer)JsonConvert.DeserializeObject(json, CreateSettings())!; - - Assert.AreEqual(expected.ID, actual.ID); - } - - [Test] - public void TestSingerStateSerializer() - { - var singer = new SingerState(main_singer_id); - - string expected = $"{{\"$type\":\"SingerState\",\"id\":\"{singer.ID}\",\"main_singer_id\":\"{singer.MainSingerId}\"}}"; - string actual = JsonConvert.SerializeObject(singer, CreateSettings()); - Assert.AreEqual(expected, actual); - } - - [Test] - public void TestSingerStateDeserializer() - { - var expected = new SingerState(main_singer_id); - - string json = $"{{\"$type\":\"SingerState\",\"id\":\"{expected.ID}\",\"main_singer_id\":\"{expected.MainSingerId}\"}}"; - var actual = (SingerState)JsonConvert.DeserializeObject(json, CreateSettings())!; - - Assert.AreEqual(expected.ID, actual.ID); - Assert.AreEqual(expected.MainSingerId, actual.MainSingerId); - } - - [Test] - public void TestSingerInfoSerializer() - { - var singerInfo = new SingerInfo(); - var singer = singerInfo.AddSinger(); - var singerState = singerInfo.AddSingerState(singer); - - string expected = - $"{{\"singers\":[{{\"$type\":\"Singer\",\"id\":\"{singer.ID}\"}},{{\"$type\":\"SingerState\",\"id\":\"{singerState.ID}\",\"main_singer_id\":\"{singerState.MainSingerId}\"}}]}}"; - string actual = JsonConvert.SerializeObject(singerInfo, CreateSettings()); - Assert.AreEqual(expected, actual); - } - - [Test] - public void TestSingerInfoDeserializer() - { - var singerInfo = new SingerInfo(); - var singer = singerInfo.AddSinger(); - var singerState = singerInfo.AddSingerState(singer); - - string json = $"{{\"singers\":[{{\"$type\":\"Singer\",\"id\":\"{singer.ID}\"}},{{\"$type\":\"SingerState\",\"id\":\"{singerState.ID}\",\"main_singer_id\":\"{singerState.MainSingerId}\"}}]}}"; - var actual = JsonConvert.DeserializeObject(json, CreateSettings())!; - - Assert.AreEqual(singerInfo.Singers.Count, actual.Singers.Count); - Assert.AreEqual(singerInfo.Singers[0].ID, singerInfo.Singers[0].ID); // test singer - Assert.AreEqual(singerInfo.Singers[1].ID, singerInfo.Singers[1].ID); // test sub-singer. - } -} diff --git a/osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/SingerConverter.cs b/osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/SingerConverter.cs deleted file mode 100644 index e48240d17..000000000 --- a/osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/SingerConverter.cs +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) andy840119 . Licensed under the GPL Licence. -// See the LICENCE file in the repository root for full licence text. - -using System; -using System.Diagnostics; -using System.Reflection; -using osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas.Types; - -namespace osu.Game.Rulesets.Karaoke.IO.Serialization.Converters; - -public class SingerConverter : GenericTypeConverter -{ - protected override Type GetTypeByName(string name) - { - var assembly = Assembly.GetExecutingAssembly(); - var type = assembly.GetType($"osu.Game.Rulesets.Karaoke.Beatmaps.Metadatas.{name}"); - Debug.Assert(type != null); - return type; - } -} diff --git a/osu.Game.Rulesets.Karaoke/IO/Serialization/KaraokeJsonSerializableExtensions.cs b/osu.Game.Rulesets.Karaoke/IO/Serialization/KaraokeJsonSerializableExtensions.cs index ab3692f21..2d2efa5df 100644 --- a/osu.Game.Rulesets.Karaoke/IO/Serialization/KaraokeJsonSerializableExtensions.cs +++ b/osu.Game.Rulesets.Karaoke/IO/Serialization/KaraokeJsonSerializableExtensions.cs @@ -13,9 +13,6 @@ public static JsonSerializerSettings CreateGlobalSettings() { var globalSetting = JsonSerializableExtensions.CreateGlobalSettings(); - // karaoke beatmap. - globalSetting.Converters.Add(new SingerConverter()); - // hit-object globalSetting.Converters.Add(new CultureInfoConverter()); globalSetting.Converters.Add(new ElementIdConverter());