Merge pull request #2159 from andy840119/clean-up-code-2

Clean up code.

osu.Game.Rulesets.Karaoke.Tests/Beatmaps/TestElementId.cs

@@ -35,11 +35,17 @@ public void TestCreateElementId(string id, bool created)
     {
         if (created)
         {
-            Assert.DoesNotThrow(() => new ElementId(id));
+            Assert.DoesNotThrow(() =>
+            {
+                var _ = new ElementId(id);
+            });
         }
         else
         {
-            Assert.Throws<ArgumentException>(() => new ElementId(id));
+            Assert.Throws<ArgumentException>(() =>
+            {
+                var _ = new ElementId(id);
+            });
         }
     }
 

osu.Game.Rulesets.Karaoke.Tests/Editor/TestSceneSetupScreen.cs

@@ -18,22 +18,17 @@ public partial class TestSceneSetupScreen : EditorClockTestScene
 {
     [Cached(typeof(EditorBeatmap))]
     [Cached(typeof(IBeatSnapProvider))]
-    private readonly EditorBeatmap editorBeatmap;
+    private readonly EditorBeatmap editorBeatmap = new(new KaraokeBeatmap
+    {
+        BeatmapInfo =
+        {
+            Ruleset = new KaraokeRuleset().RulesetInfo,
+        },
+    });
 
     [Cached]
     private readonly OverlayColourProvider colourProvider = new(OverlayColourScheme.Blue);
 
-    public TestSceneSetupScreen()
-    {
-        editorBeatmap = new EditorBeatmap(new KaraokeBeatmap
-        {
-            BeatmapInfo =
-            {
-                Ruleset = new KaraokeRuleset().RulesetInfo,
-            },
-        });
-    }
-
     [Test]
     public void TestKaraoke() => runForRuleset(new KaraokeRuleset().RulesetInfo);
 

osu.Game.Rulesets.Karaoke.Tests/Replays/TestSceneAutoGenerationBySinger.cs

@@ -56,10 +56,13 @@ private static IReadOnlyList<TestKaraokeReplayFrame> getCompareResultFromName(st
 
     private struct TestKaraokeReplayFrame
     {
+        [JsonProperty]
         public double Time { get; set; }
 
+        [JsonProperty]
         public float Pitch { get; set; }
 
+        [JsonProperty]
         public bool Sound { get; set; }
     }
 }

osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Beatmap/Lyrics/CaretPosition/Algorithms/BaseCaretPositionAlgorithmTest.cs

@@ -106,15 +106,17 @@ protected Lyric[] GetLyricsByMethodName(string methodName)
 
     private static PropertyInfo? getMethod(Type type, string methodName)
     {
-        var theMethod = type.GetProperty(methodName, BindingFlags.NonPublic | BindingFlags.Static);
+        Type? targetType = type;
 
-        if (theMethod != null)
-            return theMethod;
+        while (targetType != null)
+        {
+            var theMethod = targetType.GetProperty(methodName, BindingFlags.NonPublic | BindingFlags.Static);
+            if (theMethod != null)
+                return theMethod;
 
-        var baseType = type.BaseType;
-        if (baseType == null)
-            return null;
+            targetType = targetType.BaseType;
+        }
 
-        return getMethod(baseType, methodName);
+        return null;
     }
 }

osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Beatmap/Lyrics/Components/TestScenePreviewKaraokeSpriteText.cs

@@ -27,18 +27,13 @@ public partial class TestScenePreviewKaraokeSpriteText : OsuTestScene
 
     private Action? updateAction;
 
-    private readonly Lyric lyric;
-
-    public TestScenePreviewKaraokeSpriteText()
+    private readonly Lyric lyric = new()
     {
-        lyric = new Lyric
-        {
-            Text = "カラオケ",
-            TimeTags = TestCaseTagHelper.ParseTimeTags(new[] { "[0,start]:1000", "[1,start]:2000", "[2,start]:3000", "[3,start]:4000", "[3,end]:5000" }),
-            RubyTags = TestCaseTagHelper.ParseRubyTags(new[] { "[0]:か", "[1]:ら", "[2]:お", "[3]:け" }),
-            RomajiTags = TestCaseTagHelper.ParseRomajiTags(new[] { "[0]:ka", "[1]:ra", "[2]:o", "[3]:ke" }),
-        };
-    }
+        Text = "カラオケ",
+        TimeTags = TestCaseTagHelper.ParseTimeTags(new[] { "[0,start]:1000", "[1,start]:2000", "[2,start]:3000", "[3,start]:4000", "[3,end]:5000" }),
+        RubyTags = TestCaseTagHelper.ParseRubyTags(new[] { "[0]:か", "[1]:ら", "[2]:お", "[3]:け" }),
+        RomajiTags = TestCaseTagHelper.ParseRomajiTags(new[] { "[0]:ka", "[1]:ra", "[2]:o", "[3]:ke" }),
+    };
 
     protected override void Update()
     {

osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Import/TestSceneLyricImporter.cs

@@ -25,7 +25,7 @@ public partial class TestSceneLyricImporter : ScreenTestScene<TestSceneLyricImpo
     private readonly OverlayColourProvider overlayColourProvider = new(OverlayColourScheme.Blue);
 
     [Cached]
-    private readonly KaraokeRulesetLyricEditorConfigManager lyricEditorConfigManager;
+    private readonly KaraokeRulesetLyricEditorConfigManager lyricEditorConfigManager = new();
 
     protected override Container<Drawable> Content { get; } = new Container { RelativeSizeAxes = Axes.Both };
 
@@ -37,11 +37,6 @@ protected override TestLyricImporter CreateScreen()
 
     private DialogOverlay dialogOverlay = null!;
 
-    public TestSceneLyricImporter()
-    {
-        lyricEditorConfigManager = new KaraokeRulesetLyricEditorConfigManager();
-    }
-
     [BackgroundDependencyLoader]
     private void load()
     {

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

@@ -50,10 +50,13 @@ public void TestCompareByProperty(string leftObjectProperty, string rightObjectP
 
     private class TestObject
     {
-        public int A { get; set; } = 0;
+        [JsonProperty]
+        public int A { get; set; }
 
-        public double B { get; set; } = 0;
+        [JsonProperty]
+        public double B { get; set; }
 
+        [JsonProperty]
         public string C { get; set; } = string.Empty;
     }
 }

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

@@ -48,12 +48,9 @@ protected override IList<JsonProperty> CreateProperties(Type type, MemberSeriali
         {
             var props = base.CreateProperties(type, memberSerialization);
 
-            if (type == typeof(BeatmapInfo))
-            {
-                return props.Where(p => p.PropertyName != "ruleset_id").ToList();
-            }
-
-            return props;
+            return type == typeof(BeatmapInfo)
+                ? props.Where(p => p.PropertyName != "ruleset_id").ToList()
+                : props;
         }
     }
 }

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

@@ -52,11 +52,9 @@ private IEnumerable<string> encodeNote(Beatmap output)
         // Convert single note
         static string convertNote(Note note)
         {
-            if (!note.Display)
-                return "-";
-
-            // TODO : Fill if customize ruby and percentage
-            return convertTone(note.Tone);
+            return !note.Display
+                ? "-"
+                : convertTone(note.Tone);
 
             // Convert tone to string
             static string convertTone(Tone tone) => tone.Scale + (tone.Half ? "#" : string.Empty);

osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmapExtension.cs

@@ -12,14 +12,9 @@ public static class KaraokeBeatmapExtension
 {
     public static bool IsScorable(this IBeatmap beatmap)
     {
-        if (beatmap is not KaraokeBeatmap karaokeBeatmap)
-        {
-            // we should throw invalidate exception here but it will cause test case failed.
-            // because beatmap in the working beatmap in test case not always be karaoke beatmap class.
-            return false;
-        }
-
-        return karaokeBeatmap.Scorable;
+        // we should throw invalidate exception here but it will cause test case failed.
+        // because beatmap in the working beatmap in test case not always be karaoke beatmap class.
+        return beatmap is KaraokeBeatmap karaokeBeatmap && karaokeBeatmap.Scorable;
     }
 
     public static IList<CultureInfo> AvailableTranslates(this IBeatmap beatmap) => (beatmap as KaraokeBeatmap)?.AvailableTranslates ?? new List<CultureInfo>();

osu.Game.Rulesets.Karaoke/Difficulty/KaraokeDifficultyCalculator.cs

@@ -79,16 +79,11 @@ protected override IEnumerable<DifficultyHitObject> CreateDifficultyHitObjects(I
 
     private int getHitWindow300(Mod[] mods)
     {
-        if (isForCurrentRuleset)
-        {
-            double od = Math.Min(10.0, Math.Max(0, 10.0 - originalOverallDifficulty));
-            return applyModAdjustments(34 + 3 * od, mods);
-        }
-
-        if (Math.Round(originalOverallDifficulty) > 4)
-            return applyModAdjustments(34, mods);
+        if (!isForCurrentRuleset)
+            return applyModAdjustments(Math.Round(originalOverallDifficulty) > 4 ? 34 : 47, mods);
 
-        return applyModAdjustments(47, mods);
+        double od = Math.Min(10.0, Math.Max(0, 10.0 - originalOverallDifficulty));
+        return applyModAdjustments(34 + 3 * od, mods);
 
         static int applyModAdjustments(double value, Mod[] mods)
         {

osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapClassicStageInfo.cs

@@ -143,10 +143,6 @@ public IssueTemplateInvalidRowHeight(ICheck check)
         public Issue Create() => new(this);
     }
 
-    #region stage definition
-
-    #endregion
-
     #region timing info
 
     public class IssueTemplateLessThanTwoTimingPoints : IssueTemplate

osu.Game.Rulesets.Karaoke/Edit/KaraokeHitObjectComposer.cs

@@ -126,7 +126,7 @@ public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePositio
             : result;
     }
 
-    protected override DrawableRuleset<KaraokeHitObject> CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList<Mod>? mods = null)
+    protected override DrawableRuleset<KaraokeHitObject> CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList<Mod> mods)
     {
         drawableRuleset = new DrawableKaraokeEditorRuleset(ruleset, beatmap, mods);
 

osu.Game.Rulesets.Karaoke/Edit/Setup/Components/SingerDisplay.cs

@@ -8,7 +8,6 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.UserInterface;
-using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
@@ -79,9 +78,6 @@ private partial class SingerCircle : OsuClickableContainer, IHasContextMenu, IHa
 
         private readonly DrawableSingerAvatar singerAvatar;
 
-        [Resolved]
-        private OsuColour colours { get; set; } = null!;
-
         public SingerCircle()
         {
             RelativeSizeAxes = Axes.X;

osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/CaretPosition/Algorithms/TimeTagCaretPositionAlgorithm.cs

@@ -81,11 +81,8 @@ protected override bool PositionMovable(TimeTagCaretPosition position)
     protected override TimeTagCaretPosition? MoveToFirstLyric()
     {
         var firstLyric = Lyrics.FirstOrDefault(x => x.TimeTags.Any(timeTagMovable));
-        if (firstLyric == null)
-            return null;
-
-        var firstTimeTag = firstLyric.TimeTags.FirstOrDefault(timeTagMovable);
-        if (firstTimeTag == null)
+        var firstTimeTag = firstLyric?.TimeTags.FirstOrDefault(timeTagMovable);
+        if (firstLyric == null || firstTimeTag == null)
             return null;
 
         return CreateCaretPosition(firstLyric, firstTimeTag);
@@ -94,11 +91,8 @@ protected override bool PositionMovable(TimeTagCaretPosition position)
     protected override TimeTagCaretPosition? MoveToLastLyric()
     {
         var lastLyric = Lyrics.LastOrDefault(x => x.TimeTags.Any(timeTagMovable));
-        if (lastLyric == null)
-            return null;
-
-        var lastTimeTag = lastLyric.TimeTags.LastOrDefault(timeTagMovable);
-        if (lastTimeTag == null)
+        var lastTimeTag = lastLyric?.TimeTags.LastOrDefault(timeTagMovable);
+        if (lastLyric == null || lastTimeTag == null)
             return null;
 
         return CreateCaretPosition(lastLyric, lastTimeTag);

osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Components/Lyrics/PreviewKaraokeSpriteText.cs

@@ -26,7 +26,7 @@ public partial class PreviewKaraokeSpriteText : DrawableKaraokeSpriteText<Previe
 
     public Lyric HitObject;
 
-    public Action? SizeChanged = null;
+    public Action? SizeChanged;
 
     private readonly EditorLyricSpriteText spriteText;
 
@@ -136,31 +136,35 @@ public RectangleF GetRectByCharIndicator(int charIndex)
 
     public TimeTag? GetTimeTagByPosition(float position)
     {
+        var hoverIndex = getHoverIndex();
+        if (hoverIndex == null)
+            return null;
+
         // todo: will use better way to get the time-tag
-        var textIndex = getHoverIndex();
-        return HitObject.TimeTags.FirstOrDefault(x => x.Index == textIndex);
+        return HitObject.TimeTags.FirstOrDefault(x => x.Index == hoverIndex);
 
-        TextIndex getHoverIndex()
+        TextIndex? getHoverIndex()
         {
             for (int i = 0; i < Text.Length; i++)
             {
-                if (getTriggerPositionByTimeIndex(new TextIndex(i)) > position)
-                    return new TextIndex(i);
-
-                if (getTriggerPositionByTimeIndex(new TextIndex(i, TextIndex.IndexState.End)) > position)
-                    return new TextIndex(i, TextIndex.IndexState.End);
+                foreach (var indexState in Enum.GetValues<TextIndex.IndexState>())
+                {
+                    var textIndex = new TextIndex(i, indexState);
+                    var triggerRange = getTriggerRange(textIndex);
+                    if (position >= triggerRange.Item1 && position <= triggerRange.Item2)
+                        return textIndex;
+                }
             }
 
-            return new TextIndex(Text.Length - 1, TextIndex.IndexState.End);
+            // hover the last time-tag if exceed the range.
+            return null;
 
-            // todo : might have a better way to call spriteText.GetTimeTagPosition just once.
-            float getTriggerPositionByTimeIndex(TextIndex textIndex)
+            Tuple<float, float> getTriggerRange(TextIndex textIndex)
             {
-                int charIndex = textIndex.Index;
-                float startPosition = spriteText.GetTimeTagPosition(new TextIndex(charIndex)).X;
-                float endPosition = spriteText.GetTimeTagPosition(new TextIndex(charIndex, TextIndex.IndexState.End)).X;
-
-                return TextIndexUtils.GetValueByState(textIndex, () => startPosition + (endPosition - startPosition) / 2, () => endPosition);
+                var rect = spriteText.GetCharacterDrawRectangle(textIndex.Index);
+                return TextIndexUtils.GetValueByState(textIndex,
+                    () => new Tuple<float, float>(rect.Left, rect.Centre.X),
+                    () => new Tuple<float, float>(rect.Centre.X, rect.Right));
             }
         }
     }

osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/Compose/Toolbar/ToolbarButton.cs

@@ -5,7 +5,6 @@
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Graphics.Textures;
-using osu.Framework.Input;
 using osu.Game.Graphics.Containers;
 using osuTK;
 
@@ -26,9 +25,6 @@ public void SetIcon(Drawable icon)
     [Resolved]
     private TextureStore textures { get; set; } = null!;
 
-    [Resolved]
-    private ReadableKeyCombinationProvider keyCombinationProvider { get; set; } = null!;
-
     public void SetIcon(string texture) =>
         SetIcon(new Sprite
         {

osu.Game.Rulesets.Karaoke/Screens/Edit/Beatmaps/Lyrics/LyricEditorClipboard.cs

@@ -44,9 +44,6 @@ public partial class LyricEditorClipboard : Component, ILyricEditorClipboard
     [Resolved]
     private ITimeTagModeState timeTagModeState { get; set; } = null!;
 
-    [Resolved]
-    private IEditRomajiModeState editRomajiModeState { get; set; } = null!;
-
     [Resolved]
     private ILyricsChangeHandler? lyricsChangeHandler { get; set; }