Merge pull request #2314 from andy840119/migrate-lyric-style-into-stage

Use command pattern to update the lyric style.
karaoke-dev · Dec 11, 2024 · c41d65b · c41d65b
2 parents 1100ff7 + 3c68ed3
commit c41d65b
Show file tree

Hide file tree

Showing 34 changed files with 186 additions and 246 deletions.
diff --git a/...esets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs b/...esets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs
@@ -225,7 +225,11 @@ public override IPlayfieldCommandProvider CreatePlayfieldCommandProvider(bool di
         #endregion
     }
 
-    private class TestCategory : StageElementCategory<TestStageElement, Lyric>;
+    private class TestCategory : StageElementCategory<TestStageElement, Lyric>
+    {
+        protected override TestStageElement CreateDefaultElement()
+            => new();
+    }
 
     public class TestStageElement : StageElement, IComparable<TestStageElement>
     {

diff --git a/...ame.Rulesets.Karaoke.Tests/IO/Serialization/Converters/KaraokeSkinElementConverterTest.cs b/...ame.Rulesets.Karaoke.Tests/IO/Serialization/Converters/KaraokeSkinElementConverterTest.cs
@@ -43,42 +43,20 @@ public void TestLyricConfigDeserialize()
         ObjectAssert.ArePropertyEqual(expected, actual);
     }
 
-    [Test]
-    public void TestLyricStyleSerializer()
-    {
-        var lyricStyle = LyricStyle.CreateDefault();
-
-        const string expected =
-            "{\"$type\":1,\"left_lyric_text_shaders\":[{\"$type\":\"StepShader\",\"name\":\"Step shader\",\"draw\":true,\"step_shaders\":[{\"$type\":\"OutlineShader\",\"outline_colour\":\"#CCA532\",\"radius\":3.0},{\"$type\":\"ShadowShader\",\"shadow_colour\":\"#6B5B2D\",\"shadow_offset\":{\"x\":3.0,\"y\":3.0}}]}],\"right_lyric_text_shaders\":[{\"$type\":\"StepShader\",\"name\":\"Step shader\",\"draw\":true,\"step_shaders\":[{\"$type\":\"OutlineShader\",\"outline_colour\":\"#5932CC\",\"radius\":3.0},{\"$type\":\"ShadowShader\",\"shadow_colour\":\"#3D2D6B\",\"shadow_offset\":{\"x\":3.0,\"y\":3.0}}]}],\"name\":\"Default\"}";
-        string actual = JsonConvert.SerializeObject(lyricStyle, CreateSettings());
-        Assert.AreEqual(expected, actual);
-    }
-
-    [Test]
-    public void TestLyricStyleDeserializer()
-    {
-        const string json =
-            "{\"$type\":1,\"left_lyric_text_shaders\":[{\"$type\":\"StepShader\",\"name\":\"Step shader\",\"draw\":true,\"step_shaders\":[{\"$type\":\"OutlineShader\",\"outline_colour\":\"#CCA532\",\"radius\":3.0},{\"$type\":\"ShadowShader\",\"shadow_colour\":\"#6B5B2D\",\"shadow_offset\":{\"x\":3.0,\"y\":3.0}}]}],\"right_lyric_text_shaders\":[{\"$type\":\"StepShader\",\"name\":\"Step shader\",\"draw\":true,\"step_shaders\":[{\"$type\":\"OutlineShader\",\"outline_colour\":\"#5932CC\",\"radius\":3.0},{\"$type\":\"ShadowShader\",\"shadow_colour\":\"#3D2D6B\",\"shadow_offset\":{\"x\":3.0,\"y\":3.0}}]}],\"name\":\"Default\"}";
-
-        var expected = LyricStyle.CreateDefault();
-        var actual = (LyricStyle)JsonConvert.DeserializeObject<IKaraokeSkinElement>(json, CreateSettings())!;
-        ObjectAssert.ArePropertyEqual(expected, actual);
-    }
-
     [Test]
     public void TestNoteStyleSerializer()
     {
         var lyricConfig = NoteStyle.CreateDefault();
 
-        const string expected = "{\"$type\":2,\"name\":\"Default\",\"note_color\":\"#44AADD\",\"blink_color\":\"#FF66AA\",\"text_color\":\"#FFFFFF\",\"bold_text\":true}";
+        const string expected = "{\"$type\":1,\"name\":\"Default\",\"note_color\":\"#44AADD\",\"blink_color\":\"#FF66AA\",\"text_color\":\"#FFFFFF\",\"bold_text\":true}";
         string actual = JsonConvert.SerializeObject(lyricConfig, CreateSettings());
         Assert.AreEqual(expected, actual);
     }
 
     [Test]
     public void TestNoteStyleDeserializer()
     {
-        const string json = "{\"$type\":2,\"name\":\"Default\",\"note_color\":\"#44AADD\",\"blink_color\":\"#FF66AA\",\"text_color\":\"#FFFFFF\",\"bold_text\":true}";
+        const string json = "{\"$type\":1,\"name\":\"Default\",\"note_color\":\"#44AADD\",\"blink_color\":\"#FF66AA\",\"text_color\":\"#FFFFFF\",\"bold_text\":true}";
 
         var expected = NoteStyle.CreateDefault();
         var actual = (NoteStyle)JsonConvert.DeserializeObject<IKaraokeSkinElement>(json, CreateSettings())!;

diff --git a/osu.Game.Rulesets.Karaoke.Tests/Resources/special-skin/lyric-styles.json b/osu.Game.Rulesets.Karaoke.Tests/Resources/special-skin/lyric-styles.json
diff --git a/osu.Game.Rulesets.Karaoke.Tests/Screens/Skin/TestSceneKaraokeSkinEditor.cs b/osu.Game.Rulesets.Karaoke.Tests/Screens/Skin/TestSceneKaraokeSkinEditor.cs
@@ -67,7 +67,6 @@ public TestingSkin(SkinInfo skin, IStorageResourceProvider? resources)
             : base(skin, resources)
         {
             DefaultElement[ElementType.LyricFontInfo] = LyricFontInfo.CreateDefault();
-            DefaultElement[ElementType.LyricStyle] = LyricStyle.CreateDefault();
             DefaultElement[ElementType.NoteStyle] = NoteStyle.CreateDefault();
         }
     }

diff --git a/osu.Game.Rulesets.Karaoke.Tests/Skinning/KaraokeBeatmapSkinDecodingTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Skinning/KaraokeBeatmapSkinDecodingTest.cs
@@ -27,17 +27,14 @@ public void TestKaraokeBeatmapSkinDefaultValue()
 
         // try to get default value from the skin.
         var defaultLyricFontInfo = skin.GetConfig<Lyric, LyricFontInfo>(referencedLyric)!.Value;
-        var defaultLyricStyle = skin.GetConfig<Lyric, LyricStyle>(referencedLyric)!.Value;
         var defaultNoteStyle = skin.GetConfig<Note, NoteStyle>(testingNote)!.Value;
 
         // should be able to get the default value.
         Assert.IsNotNull(defaultLyricFontInfo);
-        Assert.IsNotNull(defaultLyricStyle);
         Assert.IsNotNull(defaultNoteStyle);
 
         // Check the content
         Assert.IsNotNull(defaultLyricFontInfo.Name, "Default lyric config");
-        Assert.IsNotNull(defaultLyricStyle.Name, "Default lyric style");
         Assert.IsNotNull(defaultNoteStyle.Name, "Default note style");
     }
 }
diff --git a/osu.Game.Rulesets.Karaoke.Tests/Skinning/KaraokeSkinDecodingTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Skinning/KaraokeSkinDecodingTest.cs
@@ -27,17 +27,14 @@ public void TestKaraokeSkinDefaultValue()
 
         // try to get default value from the skin.
         var defaultLyricFontInfo = skin.GetConfig<Lyric, LyricFontInfo>(referencedLyric)!.Value;
-        var defaultLyricStyle = skin.GetConfig<Lyric, LyricStyle>(referencedLyric)!.Value;
         var defaultNoteStyle = skin.GetConfig<Note, NoteStyle>(testingNote)!.Value;
 
         // should be able to get the default value.
         Assert.IsNotNull(defaultLyricFontInfo);
-        Assert.IsNotNull(defaultLyricStyle);
         Assert.IsNotNull(defaultNoteStyle);
 
         // Check the content
         Assert.IsNotNull(defaultLyricFontInfo.Name, "Default lyric config");
-        Assert.IsNotNull(defaultLyricStyle.Name, "Default lyric style");
         Assert.IsNotNull(defaultNoteStyle.Name, "Default note style");
     }
 }
diff --git a/osu.Game.Rulesets.Karaoke.Tests/Stages/Infos/StageElementCategoryTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Stages/Infos/StageElementCategoryTest.cs
@@ -238,5 +238,9 @@ public int CompareTo(TestStageElement? other)
         }
     }
 
-    private class TestStageElementCategory : StageElementCategory<TestStageElement, Lyric>;
+    private class TestStageElementCategory : StageElementCategory<TestStageElement, Lyric>
+    {
+        protected override TestStageElement CreateDefaultElement()
+            => new();
+    }
 }
diff --git a/osu.Game.Rulesets.Karaoke/Graphics/Sprites/DrawableKaraokeSpriteText.cs b/osu.Game.Rulesets.Karaoke/Graphics/Sprites/DrawableKaraokeSpriteText.cs
@@ -1,8 +1,11 @@
 // Copyright (c) andy840119 <[email protected]>. Licensed under the GPL Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using osu.Framework.Allocation;
+using osu.Framework.Graphics.Shaders;
 using osu.Framework.Graphics.Sprites;
 using osu.Game.Rulesets.Karaoke.Objects;
+using osu.Game.Rulesets.Karaoke.Skinning.Tools;
 
 namespace osu.Game.Rulesets.Karaoke.Graphics.Sprites;
 
@@ -18,6 +21,9 @@ public DrawableKaraokeSpriteText(Lyric lyric)
 {
     private readonly DisplayLyricProcessor processor;
 
+    [Resolved]
+    private ShaderManager? shaderManager { get; set; }
+
     protected DrawableKaraokeSpriteText(Lyric lyric)
     {
         processor = new DisplayLyricProcessor(lyric)
@@ -68,4 +74,14 @@ protected override void Dispose(bool isDisposing)
         base.Dispose(isDisposing);
         processor.Dispose();
     }
+
+    public void UpdateStyle(LyricStyle style)
+    {
+        // for prevent issue Collection was modified; enumeration operation may not execute.
+        Schedule(() =>
+        {
+            LeftLyricTextShaders = SkinConverterTool.ConvertLeftSideShader(shaderManager, style);
+            RightLyricTextShaders = SkinConverterTool.ConvertRightSideShader(shaderManager, style);
+        });
+    }
 }
diff --git a/...s.Karaoke/Skinning/Elements/LyricStyle.cs → ...ts.Karaoke/Graphics/Sprites/LyricStyle.cs b/...s.Karaoke/Skinning/Elements/LyricStyle.cs → ...ts.Karaoke/Graphics/Sprites/LyricStyle.cs
@@ -1,23 +1,18 @@
-// Copyright (c) andy840119 <[email protected]>. Licensed under the GPL Licence.
+// 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 System.Collections.Generic;
-using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
-using osu.Framework.Graphics;
 using osu.Framework.Graphics.Shaders;
-using osu.Game.Rulesets.Karaoke.Objects.Drawables;
-using osu.Game.Rulesets.Karaoke.Skinning.Tools;
 using osuTK;
 
-namespace osu.Game.Rulesets.Karaoke.Skinning.Elements;
+namespace osu.Game.Rulesets.Karaoke.Graphics.Sprites;
 
-public class LyricStyle : IKaraokeSkinElement
+public class LyricStyle
 {
     public static LyricStyle CreateDefault() => new()
     {
-        Name = "Default",
         LeftLyricTextShaders = new ICustomizedShader[]
         {
             new StepShader
@@ -60,25 +55,7 @@ public class LyricStyle : IKaraokeSkinElement
         },
     };
 
-    public int ID { get; set; }
-
-    public string Name { get; set; } = string.Empty;
-
     public IReadOnlyList<ICustomizedShader> LeftLyricTextShaders = Array.Empty<ICustomizedShader>();
 
     public IReadOnlyList<ICustomizedShader> RightLyricTextShaders = Array.Empty<ICustomizedShader>();
-
-    public void ApplyTo(Drawable d)
-    {
-        if (d is not DrawableLyric drawableLyric)
-            throw new InvalidDrawableTypeException(nameof(d));
-
-        var shaderManager = drawableLyric.Dependencies.Get<ShaderManager>();
-        drawableLyric.ApplyToLyricPieces(l =>
-        {
-            // Apply shader.
-            l.LeftLyricTextShaders = SkinConverterTool.ConvertLeftSideShader(shaderManager, this);
-            l.RightLyricTextShaders = SkinConverterTool.ConvertRightSideShader(shaderManager, this);
-        });
-    }
 }
diff --git a/osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/KaraokeSkinElementConverter.cs b/osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/KaraokeSkinElementConverter.cs
@@ -19,7 +19,6 @@ public static ElementType GetElementType(MemberInfo elementType) =>
         elementType switch
         {
             _ when elementType == typeof(LyricFontInfo) => ElementType.LyricFontInfo,
-            _ when elementType == typeof(LyricStyle) => ElementType.LyricStyle,
             _ when elementType == typeof(NoteStyle) => ElementType.NoteStyle,
             _ => throw new NotSupportedException(),
         };
@@ -28,7 +27,6 @@ public static Type GetObjectType(ElementType elementType) =>
         elementType switch
         {
             ElementType.LyricFontInfo => typeof(LyricFontInfo),
-            ElementType.LyricStyle => typeof(LyricStyle),
             ElementType.NoteStyle => typeof(NoteStyle),
             _ => throw new NotSupportedException(),
         };

diff --git a/osu.Game.Rulesets.Karaoke/Objects/Drawables/DrawableLyric.cs b/osu.Game.Rulesets.Karaoke/Objects/Drawables/DrawableLyric.cs
@@ -35,7 +35,6 @@ public partial class DrawableLyric : DrawableKaraokeHitObject
     private readonly Bindable<int> romanisationMarginBindable = new();
     private readonly Bindable<FontUsage> translationFontUsageBindable = new();
 
-    private readonly IBindableDictionary<Singer, SingerState[]> singersBindable = new BindableDictionary<Singer, SingerState[]>();
     private readonly BindableDictionary<CultureInfo, string> translationTextBindable = new();
 
     public event Action<DrawableLyric>? OnLyricStart;
@@ -89,7 +88,6 @@ private void load(KaraokeRulesetConfigManager? config)
         translationFontUsageBindable.BindValueChanged(_ => updateLyricFontInfo());
 
         // property in hitobject.
-        singersBindable.BindCollectionChanged((_, _) => { updateFontStyle(); });
         translationTextBindable.BindCollectionChanged((_, _) => { applyTranslation(); });
     }
 
@@ -121,38 +119,23 @@ protected override void OnApply()
         lyricPieces.Add(new DrawableKaraokeSpriteText(HitObject));
         ApplySkin(CurrentSkin, false);
 
-        singersBindable.BindTo(HitObject.SingersBindable);
         translationTextBindable.BindTo(HitObject.TranslationsBindable);
     }
 
     protected override void OnFree()
     {
         base.OnFree();
 
-        singersBindable.UnbindFrom(HitObject.SingersBindable);
         translationTextBindable.UnbindFrom(HitObject.TranslationsBindable);
     }
 
     protected override void ApplySkin(ISkinSource skin, bool allowFallback)
     {
         base.ApplySkin(skin, allowFallback);
 
-        updateFontStyle();
         updateLyricFontInfo();
     }
 
-    private void updateFontStyle()
-    {
-        if (CurrentSkin == null)
-            return;
-
-        if (HitObject.IsNull())
-            return;
-
-        var lyricStyle = CurrentSkin.GetConfig<Lyric, LyricStyle>(HitObject)?.Value;
-        lyricStyle?.ApplyTo(this);
-    }
-
     private void updateLyricFontInfo()
     {
         if (CurrentSkin == null)

diff --git a/osu.Game.Rulesets.Karaoke/Resources/Skin/Default/lyric-styles.json b/osu.Game.Rulesets.Karaoke/Resources/Skin/Default/lyric-styles.json
diff --git a/...araoke/Screens/Edit/Beatmaps/Lyrics/Content/Components/Lyrics/PreviewKaraokeSpriteText.cs b/...araoke/Screens/Edit/Beatmaps/Lyrics/Content/Components/Lyrics/PreviewKaraokeSpriteText.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
+using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shaders;
@@ -13,10 +14,10 @@
 using osu.Game.Rulesets.Karaoke.Objects;
 using osu.Game.Rulesets.Karaoke.Objects.Utils;
 using osu.Game.Rulesets.Karaoke.Skinning.Elements;
-using osu.Game.Rulesets.Karaoke.Skinning.Tools;
 using osu.Game.Rulesets.Karaoke.Utils;
 using osu.Game.Skinning;
 using osuTK;
+using LyricStyle = osu.Game.Rulesets.Karaoke.Graphics.Sprites.LyricStyle;
 
 namespace osu.Game.Rulesets.Karaoke.Screens.Edit.Beatmaps.Lyrics.Content.Components.Lyrics;
 
@@ -39,6 +40,28 @@ public PreviewKaraokeSpriteText(Lyric lyric)
         DisplayType = LyricDisplayType.Lyric;
         DisplayProperty = LyricDisplayProperty.Both;
 
+        // give it a default style.
+        UpdateStyle(new LyricStyle
+        {
+            LeftLyricTextShaders = new List<ICustomizedShader>
+            {
+                new OutlineShader
+                {
+                    Radius = 2,
+                    Colour = Color4Extensions.FromHex("#3D2D6B"),
+                    OutlineColour = Color4Extensions.FromHex("#CCA532"),
+                },
+            },
+            RightLyricTextShaders = new List<ICustomizedShader>
+            {
+                new OutlineShader
+                {
+                    Radius = 2,
+                    OutlineColour = Color4Extensions.FromHex("#5932CC"),
+                },
+            },
+        });
+
         spriteText = getSpriteText();
 
         EditorLyricSpriteText getSpriteText()
@@ -217,18 +240,6 @@ static float extraSpacing(IList<TimeTag> timeTagsInLyric, TimeTag timeTag)
     [BackgroundDependencyLoader]
     private void load(ISkinSource skin, ShaderManager? shaderManager)
     {
-        skin.GetConfig<Lyric, LyricStyle>(HitObject)?.BindValueChanged(lyricStyle =>
-        {
-            var newStyle = lyricStyle.NewValue;
-            if (newStyle == null)
-                return;
-
-            LeftLyricTextShaders = SkinConverterTool.ConvertLeftSideShader(shaderManager, newStyle);
-            RightLyricTextShaders = SkinConverterTool.ConvertRightSideShader(shaderManager, newStyle);
-
-            triggerSizeChangedEvent();
-        }, true);
-
         skin.GetConfig<Lyric, LyricFontInfo>(HitObject)?.BindValueChanged(e =>
         {
             var newConfig = e.NewValue;