Merge pull request #1826 from andy840119/implement-classic-layout-checks

Implement classic layout checks.

osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs

@@ -0,0 +1,85 @@
+// Copyright (c) andy840119 <andy840119@gmail.com>. Licensed under the GPL Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets.Edit;
+using osu.Game.Rulesets.Karaoke.Beatmaps;
+using osu.Game.Rulesets.Karaoke.Beatmaps.Stages;
+using osu.Game.Rulesets.Karaoke.Beatmaps.Stages.Classic;
+using osu.Game.Rulesets.Karaoke.Edit.Checks;
+using osu.Game.Rulesets.Karaoke.Objects;
+using osu.Game.Screens.Edit;
+using osu.Game.Tests.Beatmaps;
+using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckBeatmapClassicStageInfo;
+
+namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks;
+
+public class CheckBeatmapClassicStageInfoTest : BeatmapPropertyCheckTest<CheckBeatmapClassicStageInfo>
+{
+    [Test]
+    public void TestCheckInvalidRowHeight()
+    {
+        var beatmap = createTestingBeatmap(Array.Empty<Lyric>(), stage =>
+        {
+            stage.LyricLayoutDefinition.LineHeight = MIN_ROW_HEIGHT - 1;
+        });
+        AssertNotOk<IssueTemplateInvalidRowHeight>(getContext(beatmap));
+
+        var beatmap2 = createTestingBeatmap(Array.Empty<Lyric>(), stage =>
+        {
+            stage.LyricLayoutDefinition.LineHeight = MAX_ROW_HEIGHT + 1;
+        });
+        AssertNotOk<IssueTemplateInvalidRowHeight>(getContext(beatmap2));
+    }
+
+    [Test]
+    public void TestCheckLyricLayoutInvalidLineNumber()
+    {
+        var beatmap = createTestingBeatmap(Array.Empty<Lyric>(), stage =>
+        {
+            var layoutElement = stage.LyricLayoutCategory.AvailableElements.First();
+            layoutElement.Line = MIN_LINE_SIZE - 1;
+        });
+        AssertNotOk<IssueTemplateLyricLayoutInvalidLineNumber>(getContext(beatmap));
+
+        var beatmap2 = createTestingBeatmap(Array.Empty<Lyric>(), stage =>
+        {
+            var layoutElement = stage.LyricLayoutCategory.AvailableElements.First();
+            layoutElement.Line = MAX_LINE_SIZE + 1;
+        });
+        AssertNotOk<IssueTemplateLyricLayoutInvalidLineNumber>(getContext(beatmap2));
+    }
+
+    private static IBeatmap createTestingBeatmap(IEnumerable<Lyric>? lyrics, Action<ClassicStageInfo>? editStageAction = null)
+    {
+        var stageInfo = new ClassicStageInfo();
+
+        // add two elements to prevent no element error.
+        stageInfo.LyricLayoutCategory.AddElement(x => x.Line = MIN_LINE_SIZE);
+        stageInfo.LyricLayoutCategory.AddElement(x => x.Line = MIN_LINE_SIZE);
+        stageInfo.LyricLayoutDefinition.LineHeight = MIN_ROW_HEIGHT;
+
+        editStageAction?.Invoke(stageInfo);
+
+        var karaokeBeatmap = new KaraokeBeatmap
+        {
+            BeatmapInfo =
+            {
+                Ruleset = new KaraokeRuleset().RulesetInfo,
+            },
+            StageInfos = new List<StageInfo>
+            {
+                stageInfo
+            },
+            HitObjects = lyrics?.OfType<KaraokeHitObject>().ToList() ?? new List<KaraokeHitObject>()
+        };
+        return new EditorBeatmap(karaokeBeatmap);
+    }
+
+    private static BeatmapVerifierContext getContext(IBeatmap beatmap)
+        => new(beatmap, new TestWorkingBeatmap(beatmap));
+}

osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs

@@ -0,0 +1,112 @@
+// Copyright (c) andy840119 <andy840119@gmail.com>. Licensed under the GPL Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets.Edit;
+using osu.Game.Rulesets.Edit.Checks.Components;
+using osu.Game.Rulesets.Karaoke.Beatmaps;
+using osu.Game.Rulesets.Karaoke.Beatmaps.Stages;
+using osu.Game.Rulesets.Karaoke.Beatmaps.Stages.Classic;
+using osu.Game.Rulesets.Karaoke.Edit.Checks;
+using osu.Game.Rulesets.Karaoke.Objects;
+using osu.Game.Screens.Edit;
+using osu.Game.Tests.Beatmaps;
+using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckBeatmapStageInfo<osu.Game.Rulesets.Karaoke.Beatmaps.Stages.Classic.ClassicStageInfo>;
+
+namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks;
+
+public class CheckBeatmapStageInfoTest : BeatmapPropertyCheckTest<CheckBeatmapStageInfoTest.TestCheckBeatmapStageInfo>
+{
+    [Test]
+    public void TestCheckNoElement()
+    {
+        var beatmap = createTestingBeatmap(Array.Empty<Lyric>());
+        AssertNotOk<IssueTemplateNoElement>(getContext(beatmap));
+    }
+
+    [Test]
+    public void TestCheckMappingHitObjectNotExist()
+    {
+        var lyric = new Lyric { ID = 1 };
+
+        // note that this lyric does not added in to the beatmap.
+        var beatmap = createTestingBeatmap(Array.Empty<Lyric>(), category =>
+        {
+            // add two elements to prevent no element error.
+            category.AddElement();
+            category.AddElement();
+
+            var firstElement = category.AvailableElements.First();
+            category.AddToMapping(firstElement, lyric);
+        });
+        AssertNotOk<IssueTemplateMappingHitObjectNotExist>(getContext(beatmap));
+    }
+
+    [Test]
+    public void TestCheckMappingItemNotExist()
+    {
+        var lyric = new Lyric { ID = 1 };
+
+        var beatmap = createTestingBeatmap(new[] { lyric }, category =>
+        {
+            // add two elements to prevent no element error.
+            category.AddElement();
+            category.AddElement();
+
+            // write value to the mapping directly to reproduce the behavior like loading value from the beatmap.
+            category.Mappings.Add(lyric.ID, 3);
+        });
+        AssertNotOk<IssueTemplateMappingItemNotExist>(getContext(beatmap));
+    }
+
+    public class TestCheckBeatmapStageInfo : CheckBeatmapStageInfo<ClassicStageInfo>
+    {
+        protected override string Description => "Checks for testing the shared logic";
+
+        public TestCheckBeatmapStageInfo()
+        {
+            // Note that we only test the lyric layout category.
+            RegisterCategory(x => x.StyleCategory, 0);
+            RegisterCategory(x => x.LyricLayoutCategory, 2);
+        }
+
+        public override IEnumerable<IssueTemplate> StageTemplates => Array.Empty<IssueTemplate>();
+
+        public override IEnumerable<Issue> CheckStageInfo(ClassicStageInfo stageInfo)
+        {
+            yield break;
+        }
+
+        protected override IEnumerable<Issue> CheckElement<TStageElement>(TStageElement element)
+        {
+            yield break;
+        }
+    }
+
+    private static IBeatmap createTestingBeatmap(IEnumerable<Lyric>? lyrics, Action<ClassicLyricLayoutCategory>? editStageAction = null)
+    {
+        var stageInfo = new ClassicStageInfo();
+        editStageAction?.Invoke(stageInfo.LyricLayoutCategory);
+
+        var karaokeBeatmap = new KaraokeBeatmap
+        {
+            BeatmapInfo =
+            {
+                Ruleset = new KaraokeRuleset().RulesetInfo,
+            },
+            StageInfos = new List<StageInfo>
+            {
+                stageInfo
+            },
+            HitObjects = lyrics?.OfType<KaraokeHitObject>().ToList() ?? new List<KaraokeHitObject>()
+        };
+        return new EditorBeatmap(karaokeBeatmap);
+    }
+
+    private static BeatmapVerifierContext getContext(IBeatmap beatmap)
+        => new(beatmap, new TestWorkingBeatmap(beatmap));
+}

osu.Game.Rulesets.Karaoke/Beatmaps/Stages/StageElementCategory.cs

@@ -13,7 +13,7 @@ namespace osu.Game.Rulesets.Karaoke.Beatmaps.Stages;
 /// It's a category to record the list of <typeparamref name="TStageElement"/> and handle the mapping by several rules.
 /// </summary>
 public abstract class StageElementCategory<TStageElement, THitObject>
-    where TStageElement : IStageElement
+    where TStageElement : class, IStageElement
     where THitObject : KaraokeHitObject, IHasPrimaryKey
 {
     /// <summary>
@@ -90,6 +90,12 @@ public void AddToMapping(TStageElement element, THitObject hitObject)
         int key = hitObject.ID;
         int value = element.ID;
 
+        if (!AvailableElements.Contains(element))
+            throw new InvalidOperationException();
+
+        if (element == DefaultElement)
+            throw new InvalidOperationException();
+
         if (Mappings.ContainsKey(key))
         {
             Mappings[key] = value;

osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Beatmaps/BeatmapStageElementCategoryChangeHandler.cs

@@ -11,7 +11,7 @@
 namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Beatmaps;
 
 public partial class BeatmapStageElementCategoryChangeHandler<TStageElement, THitObject> : BeatmapPropertyChangeHandler, IBeatmapStageElementCategoryChangeHandler<TStageElement, THitObject>
-    where TStageElement : IStageElement
+    where TStageElement : class, IStageElement
     where THitObject : KaraokeHitObject, IHasPrimaryKey
 {
     private readonly Func<IEnumerable<StageInfo>, StageElementCategory<TStageElement, THitObject>> action;

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

@@ -0,0 +1,78 @@
+// Copyright (c) andy840119 <andy840119@gmail.com>. 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.Game.Rulesets.Edit.Checks.Components;
+using osu.Game.Rulesets.Karaoke.Beatmaps.Stages.Classic;
+
+namespace osu.Game.Rulesets.Karaoke.Edit.Checks;
+
+public class CheckBeatmapClassicStageInfo : CheckBeatmapStageInfo<ClassicStageInfo>
+{
+    public const double MIN_ROW_HEIGHT = 30;
+    public const double MAX_ROW_HEIGHT = 200;
+
+    public const int MIN_LINE_SIZE = 0;
+    public const int MAX_LINE_SIZE = 4;
+
+    protected override string Description => "Check invalid info in the classic stage info.";
+
+    public override IEnumerable<IssueTemplate> StageTemplates => new IssueTemplate[]
+    {
+        new IssueTemplateInvalidRowHeight(this),
+        new IssueTemplateLyricLayoutInvalidLineNumber(this)
+    };
+
+    public CheckBeatmapClassicStageInfo()
+    {
+        RegisterCategory(x => x.StyleCategory, 0);
+        RegisterCategory(x => x.LyricLayoutCategory, 2);
+    }
+
+    public override IEnumerable<Issue> CheckStageInfo(ClassicStageInfo stageInfo)
+    {
+        var layoutDefinition = stageInfo.LyricLayoutDefinition;
+        if (layoutDefinition.LineHeight is < MIN_ROW_HEIGHT or > MAX_ROW_HEIGHT)
+            yield return new IssueTemplateInvalidRowHeight(this).Create();
+    }
+
+    protected override IEnumerable<Issue> CheckElement<TStageElement>(TStageElement element)
+    {
+        switch (element)
+        {
+            case ClassicLyricLayout classicLyricLayout:
+                if (classicLyricLayout.Line is < MIN_LINE_SIZE or > MAX_LINE_SIZE)
+                    yield return new IssueTemplateLyricLayoutInvalidLineNumber(this).Create();
+
+                break;
+
+            case ClassicStyle:
+                // todo: might need to check if skin resource is exist?
+                break;
+
+            default:
+                throw new InvalidOperationException("Unknown stage element type.");
+        }
+    }
+
+    public class IssueTemplateInvalidRowHeight : IssueTemplate
+    {
+        public IssueTemplateInvalidRowHeight(ICheck check)
+            : base(check, IssueType.Warning, $"Row height should be in the range of {MIN_ROW_HEIGHT} and {MAX_ROW_HEIGHT}.")
+        {
+        }
+
+        public Issue Create() => new(this);
+    }
+
+    public class IssueTemplateLyricLayoutInvalidLineNumber : IssueTemplate
+    {
+        public IssueTemplateLyricLayoutInvalidLineNumber(ICheck check)
+            : base(check, IssueType.Warning, $"Line number should be in the range of {MIN_LINE_SIZE} and {MAX_LINE_SIZE}.")
+        {
+        }
+
+        public Issue Create() => new(this);
+    }
+}

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

@@ -24,6 +24,8 @@ public IEnumerable<Issue> Run(BeatmapVerifierContext context)
     {
         var beatmap = getBeatmap(context);
         var property = GetPropertyFromBeatmap(beatmap);
+        if (property == null)
+            return Array.Empty<Issue>();
 
         var issues = CheckProperty(property);
         var hitObjectIssues = context.Beatmap.HitObjects.OfType<THitObject>().SelectMany(x => CheckHitObject(property, x));
@@ -32,7 +34,7 @@ public IEnumerable<Issue> Run(BeatmapVerifierContext context)
         return issues.Concat(hitObjectIssues).Concat(hitObjectsIssues);
     }
 
-    protected abstract TProperty GetPropertyFromBeatmap(KaraokeBeatmap karaokeBeatmap);
+    protected abstract TProperty? GetPropertyFromBeatmap(KaraokeBeatmap karaokeBeatmap);
 
     protected virtual IEnumerable<Issue> CheckProperty(TProperty property)
     {