From 00071f49ea998a34141108566d8fd701d135aea0 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 24 Nov 2024 15:34:23 +0800 Subject: [PATCH 1/7] Missing part in the #2304. Should create the layer in after loaded to make sure that component will not be removed. --- .../Stages/Drawables/DrawableStage.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Stages/Drawables/DrawableStage.cs b/osu.Game.Rulesets.Karaoke/Stages/Drawables/DrawableStage.cs index 65d964479..cd2133e2e 100644 --- a/osu.Game.Rulesets.Karaoke/Stages/Drawables/DrawableStage.cs +++ b/osu.Game.Rulesets.Karaoke/Stages/Drawables/DrawableStage.cs @@ -33,21 +33,17 @@ public partial class DrawableStage : Container private readonly StageElementRunner stageElementRunner = new(); - private readonly Container stageLayer; - - public DrawableStage() + [BackgroundDependencyLoader] + private void load(IReadOnlyList mods, IBeatmap beatmap) { - AddInternal(stageLayer = new Container + Container stageLayer = new Container { RelativeSizeAxes = Axes.Both, - }); + }; + AddInternal(stageLayer); stageElementRunner.UpdateStageElements(stageLayer); - } - [BackgroundDependencyLoader] - private void load(IReadOnlyList mods, IBeatmap beatmap) - { if (beatmap is not KaraokeBeatmap karaokeBeatmap) throw new InvalidOperationException(); From 12c5c7af197b63a7903d53680b8c383fcb6d6234 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 24 Nov 2024 15:44:52 +0800 Subject: [PATCH 2/7] Rename the check component because they are not rely on the beatmap anymore. --- ...assicStageInfoTest.cs => CheckClassicStageInfoTest.cs} | 4 ++-- ...CheckBeatmapStageInfoTest.cs => CheckStageInfoTest.cs} | 8 ++++---- ...eatmapClassicStageInfo.cs => CheckClassicStageInfo.cs} | 4 ++-- .../{CheckBeatmapStageInfo.cs => CheckStageInfo.cs} | 2 +- osu.Game.Rulesets.Karaoke/Edit/KaraokeBeatmapVerifier.cs | 2 +- .../Screens/Edit/Components/Issues/IssueIcon.cs | 2 +- .../Edit/Stages/Classic/Stage/StageEditorVerifier.cs | 8 ++++---- 7 files changed, 15 insertions(+), 15 deletions(-) rename osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/{CheckBeatmapClassicStageInfoTest.cs => CheckClassicStageInfoTest.cs} (97%) rename osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/{CheckBeatmapStageInfoTest.cs => CheckStageInfoTest.cs} (90%) rename osu.Game.Rulesets.Karaoke/Edit/Checks/{CheckBeatmapClassicStageInfo.cs => CheckClassicStageInfo.cs} (98%) rename osu.Game.Rulesets.Karaoke/Edit/Checks/{CheckBeatmapStageInfo.cs => CheckStageInfo.cs} (97%) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs similarity index 97% rename from osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs rename to osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs index ef592b68d..6037794aa 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapClassicStageInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs @@ -15,11 +15,11 @@ using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; using osu.Game.Screens.Edit; using osu.Game.Tests.Beatmaps; -using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckBeatmapClassicStageInfo; +using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckClassicStageInfo; namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; -public class CheckBeatmapClassicStageInfoTest : BeatmapPropertyCheckTest +public class CheckClassicStageInfoTest : BeatmapPropertyCheckTest { #region stage definition diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs similarity index 90% rename from osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs rename to osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs index 4b10c02ad..607a6b4d9 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs @@ -15,11 +15,11 @@ using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; using osu.Game.Screens.Edit; using osu.Game.Tests.Beatmaps; -using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckBeatmapStageInfo; +using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckStageInfo; namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; -public class CheckBeatmapStageInfoTest : BeatmapPropertyCheckTest +public class CheckStageInfoTest : BeatmapPropertyCheckTest { [Test] public void TestCheckNoElement() @@ -63,11 +63,11 @@ public void TestCheckMappingItemNotExist() AssertNotOk(getContext(beatmap)); } - public class CheckBeatmapStageInfo : CheckBeatmapStageInfo + public class CheckStageInfo : CheckStageInfo { protected override string Description => "Checks for testing the shared logic"; - public CheckBeatmapStageInfo() + public CheckStageInfo() { // Note that we only test the lyric layout category. RegisterCategory(x => x.StyleCategory, 0); diff --git a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapClassicStageInfo.cs b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs similarity index 98% rename from osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapClassicStageInfo.cs rename to osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs index 8518b2382..fe13fb9f8 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapClassicStageInfo.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Karaoke.Edit.Checks; -public class CheckBeatmapClassicStageInfo : CheckBeatmapStageInfo +public class CheckClassicStageInfo : CheckStageInfo { public const float MIN_ROW_HEIGHT = 30; public const float MAX_ROW_HEIGHT = 200; @@ -37,7 +37,7 @@ public class CheckBeatmapClassicStageInfo : CheckBeatmapStageInfo x.StyleCategory, 0); RegisterCategory(x => x.LyricLayoutCategory, 2); diff --git a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapStageInfo.cs b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs similarity index 97% rename from osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapStageInfo.cs rename to osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs index 6890d4ae8..37512ed92 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckBeatmapStageInfo.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Karaoke.Edit.Checks; -public abstract class CheckBeatmapStageInfo : CheckBeatmapProperty +public abstract class CheckStageInfo : CheckBeatmapProperty where TStageInfo : StageInfo { public sealed override IEnumerable PossibleTemplates => checkTemplates.Concat(StageTemplates); diff --git a/osu.Game.Rulesets.Karaoke/Edit/KaraokeBeatmapVerifier.cs b/osu.Game.Rulesets.Karaoke/Edit/KaraokeBeatmapVerifier.cs index 98771525e..70a670913 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/KaraokeBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/KaraokeBeatmapVerifier.cs @@ -14,7 +14,7 @@ public class KaraokeBeatmapVerifier : IBeatmapVerifier private readonly List checks = new() { new CheckBeatmapAvailableTranslations(), - new CheckBeatmapClassicStageInfo(), + new CheckClassicStageInfo(), new CheckBeatmapNoteInfo(), new CheckBeatmapPageInfo(), new CheckLyricLanguage(), diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Components/Issues/IssueIcon.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Components/Issues/IssueIcon.cs index 8c390e596..0c62e2b19 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Components/Issues/IssueIcon.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Components/Issues/IssueIcon.cs @@ -66,7 +66,7 @@ private static IconUsage getIconUsageByCheck(ICheck check) => check switch { CheckBeatmapAvailableTranslations => FontAwesome.Solid.Language, - CheckBeatmapClassicStageInfo => FontAwesome.Solid.AlignLeft, + CheckClassicStageInfo => FontAwesome.Solid.AlignLeft, CheckBeatmapNoteInfo => FontAwesome.Solid.Microphone, CheckBeatmapPageInfo => FontAwesome.Solid.Pager, CheckLyricLanguage => FontAwesome.Solid.Globe, diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageEditorVerifier.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageEditorVerifier.cs index ef4cbc0ed..c85a67db1 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageEditorVerifier.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageEditorVerifier.cs @@ -6,7 +6,7 @@ using System.Linq; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Karaoke.Edit.Checks; -using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckBeatmapClassicStageInfo; +using static osu.Game.Rulesets.Karaoke.Edit.Checks.CheckClassicStageInfo; namespace osu.Game.Rulesets.Karaoke.Screens.Edit.Stages.Classic.Stage; @@ -15,9 +15,9 @@ public partial class StageEditorVerifier : EditorVerifier CreateChecks(StageEditorEditCategory type) => type switch { - StageEditorEditCategory.Layout => new ICheck[] { new CheckBeatmapClassicStageInfo() }, - StageEditorEditCategory.Timing => new ICheck[] { new CheckBeatmapClassicStageInfo() }, - StageEditorEditCategory.Style => new ICheck[] { new CheckBeatmapClassicStageInfo() }, + StageEditorEditCategory.Layout => new ICheck[] { new CheckClassicStageInfo() }, + StageEditorEditCategory.Timing => new ICheck[] { new CheckClassicStageInfo() }, + StageEditorEditCategory.Style => new ICheck[] { new CheckClassicStageInfo() }, _ => throw new ArgumentOutOfRangeException(nameof(type), type, null), }; From 52e3f40e71d0f75968b5c046fcc2bf875fa4e523 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 24 Nov 2024 16:44:49 +0800 Subject: [PATCH 3/7] Let the stage info check not inherit beatmap check. Also, disable the test case because: 1. It's not possible to get the stage info from the resource file 2. it's not able to edit the stage info right now. --- .../Checks/CheckClassicStageInfoTest.cs | 108 ++++++++++-------- .../Editor/Checks/CheckStageInfoTest.cs | 39 ++++--- .../Edit/Checks/CheckClassicStageInfo.cs | 4 +- .../Edit/Checks/CheckStageInfo.cs | 57 +++++---- 4 files changed, 120 insertions(+), 88 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs index 6037794aa..d0f1692af 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckClassicStageInfoTest.cs @@ -19,24 +19,27 @@ namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; -public class CheckClassicStageInfoTest : BeatmapPropertyCheckTest +[Ignore("Disable this test until able to get the stage info from the resource file.")] +public class CheckClassicStageInfoTest : BaseCheckTest { #region stage definition [Test] public void TestCheckInvalidRowHeight() { - var beatmap = createTestingBeatmap(Array.Empty(), stage => + var beatmap = createTestingBeatmap(Array.Empty()); + var stageInfo = createTestingStageInfo(stage => { stage.StageDefinition.LineHeight = MIN_ROW_HEIGHT - 1; }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); - var beatmap2 = createTestingBeatmap(Array.Empty(), stage => + var beatmap2 = createTestingBeatmap(Array.Empty()); + var stageInfo2 = createTestingStageInfo(stage => { stage.StageDefinition.LineHeight = MAX_ROW_HEIGHT + 1; }); - AssertNotOk(getContext(beatmap2)); + AssertNotOk(getContext(beatmap2, stageInfo2)); } #endregion @@ -52,44 +55,50 @@ public void TestCheckLessThanTwoTimingPoints() }; // test with 0 lyric and 0 timing points. - var beatmap = createTestingBeatmap(null, timingInfos => timingInfos.Timings.Clear()); - AssertNotOk(getContext(beatmap)); + var beatmap = createTestingBeatmap(null); + var stageInfo = createTestingStageInfo(info => info.LyricTimingInfo.Timings.Clear()); + AssertNotOk(getContext(beatmap, stageInfo)); // test with 0 lyric and 1 timing points. - var beatmap2 = createTestingBeatmap(null, timingInfos => timingInfos.AddTimingPoint()); - AssertNotOk(getContext(beatmap2)); + var beatmap2 = createTestingBeatmap(null); + var stageInfo2 = createTestingStageInfo(timingInfos => timingInfos.AddTimingPoint()); + AssertNotOk(getContext(beatmap2, stageInfo2)); // test with 1 lyric and 0 timing points. - var beatmap3 = createTestingBeatmap(lyrics, timingInfos => timingInfos.Timings.Clear()); - AssertNotOk(getContext(beatmap3)); + var beatmap3 = createTestingBeatmap(lyrics); + var stageInfo3 = createTestingStageInfo(timingInfos => timingInfos.Timings.Clear()); + AssertNotOk(getContext(beatmap3, stageInfo3)); // test with 1 lyric and 1 timing points. - var beatmap4 = createTestingBeatmap(lyrics, timingInfos => timingInfos.AddTimingPoint()); - AssertNotOk(getContext(beatmap4)); + var beatmap4 = createTestingBeatmap(lyrics); + var stageInfo4 = createTestingStageInfo(timingInfos => timingInfos.AddTimingPoint()); + AssertNotOk(getContext(beatmap4, stageInfo4)); } [Test] public void TestCheckTimingIntervalTooShort() { - var beatmap = createTestingBeatmap(null, timingInfos => + var beatmap = createTestingBeatmap(null); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); timingInfos.AddTimingPoint(x => x.Time = 0); timingInfos.AddTimingPoint(x => x.Time = MIN_TIMING_INTERVAL - 1); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] public void TestCheckTimingIntervalTooLong() { - var beatmap = createTestingBeatmap(null, timingInfos => + var beatmap = createTestingBeatmap(null); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); timingInfos.AddTimingPoint(x => x.Time = 0); timingInfos.AddTimingPoint(x => x.Time = MAX_TIMING_INTERVAL + 1); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [TestCase(true)] @@ -101,7 +110,8 @@ public void TestCheckTimingInfoHitObjectNotExist(bool hasHitObjectsInBeatmap) new(), }; - var beatmap = createTestingBeatmap(hasHitObjectsInBeatmap ? lyrics : null, timingInfos => + var beatmap = createTestingBeatmap(hasHitObjectsInBeatmap ? lyrics : null); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); var timingPoint = timingInfos.AddTimingPoint(x => x.Time = 0); @@ -112,7 +122,7 @@ public void TestCheckTimingInfoHitObjectNotExist(bool hasHitObjectsInBeatmap) // should have error because lyric is not in the beatmap. timingInfos.AddToMapping(timingPoint, lyric); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] @@ -123,7 +133,8 @@ public void TestCheckTimingInfoMappingHasNoTiming() new(), }; - var beatmap = createTestingBeatmap(lyrics, timingInfos => + var beatmap = createTestingBeatmap(lyrics); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); timingInfos.AddTimingPoint(x => x.Time = 0); @@ -132,7 +143,7 @@ public void TestCheckTimingInfoMappingHasNoTiming() // should have error because mapping value is empty. timingInfos.Mappings.Add(lyrics.First().ID, Array.Empty()); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] @@ -143,7 +154,8 @@ public void TestCheckTimingInfoTimingNotExist() new(), }; - var beatmap = createTestingBeatmap(lyrics, timingInfos => + var beatmap = createTestingBeatmap(lyrics); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); timingInfos.AddTimingPoint(x => x.Time = 0); @@ -152,7 +164,7 @@ public void TestCheckTimingInfoTimingNotExist() // should have error because mapping value is not exist. timingInfos.Mappings.Add(lyrics.First().ID, new[] { ElementId.NewElementId() }); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] @@ -163,7 +175,8 @@ public void TestCheckTimingInfoLyricNotHaveTwoTiming() new(), }; - var beatmap = createTestingBeatmap(lyrics, timingInfos => + var beatmap = createTestingBeatmap(lyrics); + var stageInfo = createTestingStageInfo(timingInfos => { timingInfos.Timings.Clear(); timingInfos.AddTimingPoint(x => x.Time = 0); @@ -173,7 +186,7 @@ public void TestCheckTimingInfoLyricNotHaveTwoTiming() // should have error because mapping value is not exactly 2. timingInfos.Mappings.Add(lyrics.First().ID, timingInfos.Timings.Select(x => x.ID).ToArray()); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } #endregion @@ -183,26 +196,41 @@ public void TestCheckTimingInfoLyricNotHaveTwoTiming() [Test] public void TestCheckLyricLayoutInvalidLineNumber() { - var beatmap = createTestingBeatmap(Array.Empty(), stage => + var beatmap = createTestingBeatmap(Array.Empty()); + var stageInfo = createTestingStageInfo(stage => { var layoutElement = stage.LyricLayoutCategory.AvailableElements.First(); layoutElement.Line = MIN_LINE_SIZE - 1; }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); - var beatmap2 = createTestingBeatmap(Array.Empty(), stage => + var beatmap2 = createTestingBeatmap(Array.Empty()); + var stageInfo2 = createTestingStageInfo(stage => { var layoutElement = stage.LyricLayoutCategory.AvailableElements.First(); layoutElement.Line = MAX_LINE_SIZE + 1; }); - AssertNotOk(getContext(beatmap2)); + AssertNotOk(getContext(beatmap2, stageInfo2)); } #endregion - private static IBeatmap createTestingBeatmap(IEnumerable? lyrics, Action? editStageAction = null) + private static IBeatmap createTestingBeatmap(IEnumerable? lyrics) { - return createTestingBeatmap(lyrics, info => + var karaokeBeatmap = new KaraokeBeatmap + { + BeatmapInfo = + { + Ruleset = new KaraokeRuleset().RulesetInfo, + }, + HitObjects = lyrics?.OfType().ToList() ?? new List(), + }; + return new EditorBeatmap(karaokeBeatmap); + } + + private static StageInfo createTestingStageInfo(Action? editStageAction = null) + { + return createTestingStageInfo(info => { // clear the timing info created in the base method. info.LyricTimingInfo.Timings.Clear(); @@ -210,7 +238,7 @@ private static IBeatmap createTestingBeatmap(IEnumerable? lyrics, Action< }); } - private static IBeatmap createTestingBeatmap(IEnumerable? lyrics, Action? editStageAction = null) + private static StageInfo createTestingStageInfo(Action? editStageAction = null) { var stageInfo = new ClassicStageInfo(); @@ -225,21 +253,9 @@ private static IBeatmap createTestingBeatmap(IEnumerable? lyrics, Action< editStageAction?.Invoke(stageInfo); - var karaokeBeatmap = new KaraokeBeatmap - { - BeatmapInfo = - { - Ruleset = new KaraokeRuleset().RulesetInfo, - }, - StageInfos = new List - { - stageInfo, - }, - HitObjects = lyrics?.OfType().ToList() ?? new List(), - }; - return new EditorBeatmap(karaokeBeatmap); + return stageInfo; } - private static BeatmapVerifierContext getContext(IBeatmap beatmap) + private static BeatmapVerifierContext getContext(IBeatmap beatmap, StageInfo stageInfo) => new(beatmap, new TestWorkingBeatmap(beatmap)); } diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs index 607a6b4d9..c34b4ae3f 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckStageInfoTest.cs @@ -19,13 +19,15 @@ namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; -public class CheckStageInfoTest : BeatmapPropertyCheckTest +[Ignore("Disable this test until able to get the stage info from the resource file.")] +public class CheckStageInfoTest : BaseCheckTest { [Test] public void TestCheckNoElement() { var beatmap = createTestingBeatmap(Array.Empty()); - AssertNotOk(getContext(beatmap)); + var stageInfo = createTestingStageInfo(); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] @@ -34,7 +36,8 @@ public void TestCheckMappingHitObjectNotExist() var lyric = new Lyric(); // note that this lyric does not added in to the beatmap. - var beatmap = createTestingBeatmap(Array.Empty(), category => + var beatmap = createTestingBeatmap(Array.Empty()); + var stageInfo = createTestingStageInfo(category => { // add two elements to prevent no element error. category.AddElement(); @@ -43,7 +46,7 @@ public void TestCheckMappingHitObjectNotExist() var firstElement = category.AvailableElements.First(); category.AddToMapping(firstElement, lyric); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } [Test] @@ -51,7 +54,8 @@ public void TestCheckMappingItemNotExist() { var lyric = new Lyric(); - var beatmap = createTestingBeatmap(new[] { lyric }, category => + var beatmap = createTestingBeatmap(new[] { lyric }); + var stageInfo = createTestingStageInfo(category => { // add two elements to prevent no element error. category.AddElement(); @@ -60,7 +64,7 @@ public void TestCheckMappingItemNotExist() // write value to the mapping directly to reproduce the behavior like loading value from the beatmap. category.Mappings.Add(lyric.ID, ElementId.NewElementId()); }); - AssertNotOk(getContext(beatmap)); + AssertNotOk(getContext(beatmap, stageInfo)); } public class CheckStageInfo : CheckStageInfo @@ -74,9 +78,9 @@ public CheckStageInfo() RegisterCategory(x => x.LyricLayoutCategory, 2); } - public override IEnumerable StageTemplates => Array.Empty(); + public override IEnumerable CustomTemplates => Array.Empty(); - public override IEnumerable CheckStageInfo(ClassicStageInfo stageInfo, IReadOnlyList hitObjects) + public override IEnumerable CheckStageInfoWithHitObjects(ClassicStageInfo stageInfo, IReadOnlyList hitObjects) { yield break; } @@ -87,26 +91,27 @@ protected override IEnumerable CheckElement(TStageElement } } - private static IBeatmap createTestingBeatmap(IEnumerable? lyrics, Action? editStageAction = null) + private static IBeatmap createTestingBeatmap(IEnumerable? lyrics) { - var stageInfo = new ClassicStageInfo(); - editStageAction?.Invoke(stageInfo.LyricLayoutCategory); - var karaokeBeatmap = new KaraokeBeatmap { BeatmapInfo = { Ruleset = new KaraokeRuleset().RulesetInfo, }, - StageInfos = new List - { - stageInfo, - }, HitObjects = lyrics?.OfType().ToList() ?? new List(), }; return new EditorBeatmap(karaokeBeatmap); } - private static BeatmapVerifierContext getContext(IBeatmap beatmap) + private static StageInfo createTestingStageInfo( Action? editStageAction = null) + { + var stageInfo = new ClassicStageInfo(); + editStageAction?.Invoke(stageInfo.LyricLayoutCategory); + + return stageInfo; + } + + private static BeatmapVerifierContext getContext(IBeatmap beatmap, StageInfo stageInfo) => new(beatmap, new TestWorkingBeatmap(beatmap)); } diff --git a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs index fe13fb9f8..05e9380d3 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckClassicStageInfo.cs @@ -24,7 +24,7 @@ public class CheckClassicStageInfo : CheckStageInfo protected override string Description => "Check invalid info in the classic stage info."; - public override IEnumerable StageTemplates => new IssueTemplate[] + public override IEnumerable CustomTemplates => new IssueTemplate[] { new IssueTemplateInvalidRowHeight(this), new IssueTemplateLessThanTwoTimingPoints(this), @@ -43,7 +43,7 @@ public CheckClassicStageInfo() RegisterCategory(x => x.LyricLayoutCategory, 2); } - public override IEnumerable CheckStageInfo(ClassicStageInfo stageInfo, IReadOnlyList hitObjects) + public override IEnumerable CheckStageInfoWithHitObjects(ClassicStageInfo stageInfo, IReadOnlyList hitObjects) { var issues = new List(); diff --git a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs index 37512ed92..3123c228a 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/Checks/CheckStageInfo.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Objects; @@ -11,19 +12,23 @@ namespace osu.Game.Rulesets.Karaoke.Edit.Checks; -public abstract class CheckStageInfo : CheckBeatmapProperty +public abstract class CheckStageInfo : ICheck where TStageInfo : StageInfo { - public sealed override IEnumerable PossibleTemplates => checkTemplates.Concat(StageTemplates); + public CheckMetadata Metadata => new(CheckCategory.Files, Description); - private IEnumerable checkTemplates => new IssueTemplate[] + protected abstract string Description { get; } + + public IEnumerable PossibleTemplates => baseTemplates.Concat(CustomTemplates); + + private IEnumerable baseTemplates => new IssueTemplate[] { new IssueTemplateNoElement(this), new IssueTemplateMappingHitObjectNotExist(this), new IssueTemplateMappingItemNotExist(this), }; - public abstract IEnumerable StageTemplates { get; } + public abstract IEnumerable CustomTemplates { get; } private readonly IList, IEnumerable>> stageInfoCategoryActions = new List, IEnumerable>>(); @@ -39,23 +44,6 @@ public void RegisterCategory(Func karaokeBeatmap.StageInfos.OfType().FirstOrDefault(); - - protected sealed override IEnumerable CheckHitObjects(TStageInfo property, IReadOnlyList hitObjects) - { - var issues = CheckStageInfo(property, hitObjects).ToList(); - - foreach (var stageInfoCategoryAction in stageInfoCategoryActions) - { - issues.AddRange(stageInfoCategoryAction(property, hitObjects)); - } - - return issues; - } - - public abstract IEnumerable CheckStageInfo(TStageInfo stageInfo, IReadOnlyList hitObjects); - private IEnumerable checkElementCategory(StageElementCategory category, IReadOnlyList hitObjects, int minimumRequiredElements) where TStageElement : StageElement, IComparable, new() @@ -79,8 +67,6 @@ private IEnumerable checkElementCategory(Stage return issues; } - protected abstract IEnumerable CheckElement(TStageElement element) where TStageElement : StageElement; - private IEnumerable checkMappings(StageElementCategory category, IReadOnlyList hitObjects) where TStageElement : StageElement, IComparable, new() where THitObject : KaraokeHitObject, IHasPrimaryKey @@ -98,6 +84,31 @@ private IEnumerable checkMappings(StageElement } } + public IEnumerable Run(BeatmapVerifierContext context) + { + var property = getStageInfo(context); + if (property == null) + return Array.Empty(); + + var hitObjects = context.Beatmap.HitObjects.OfType().ToList(); + var issues = CheckStageInfoWithHitObjects(property, hitObjects).ToList(); + + foreach (var stageInfoCategoryAction in stageInfoCategoryActions) + { + issues.AddRange(stageInfoCategoryAction(property, hitObjects)); + } + + return issues; + + // todo: get stage info from context. + static TStageInfo? getStageInfo(BeatmapVerifierContext context) + => throw new NotImplementedException(); + } + + public abstract IEnumerable CheckStageInfoWithHitObjects(TStageInfo stageInfo, IReadOnlyList hitObjects); + + protected abstract IEnumerable CheckElement(TStageElement element) where TStageElement : StageElement; + public class IssueTemplateNoElement : IssueTemplate { public IssueTemplateNoElement(ICheck check) From 2b5d04b2909583bd8c31a53098c76707cc9287b5 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 24 Nov 2024 17:29:40 +0800 Subject: [PATCH 4/7] Let stage change handler not inherit the beatmap change handler. --- .../Stages/BaseStageInfoChangeHandlerTest.cs | 22 +++++ .../Stages/ClassicStageChangeHandlerTest.cs | 83 +++++++------------ .../StageElementCategoryChangeHandlerTest.cs | 74 +++++++---------- .../Stages/StagesChangeHandlerTest.cs | 21 ++--- .../Stages/ClassicStageChangeHandler.cs | 17 +--- .../StageElementCategoryChangeHandler.cs | 8 +- .../Stages/StagePropertyChangeHandler.cs | 23 +++++ .../Stages/StagesChangeHandler.cs | 59 ++++++------- 8 files changed, 144 insertions(+), 163 deletions(-) create mode 100644 osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/BaseStageInfoChangeHandlerTest.cs create mode 100644 osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagePropertyChangeHandler.cs diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/BaseStageInfoChangeHandlerTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/BaseStageInfoChangeHandlerTest.cs new file mode 100644 index 000000000..e3aa95f1d --- /dev/null +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/BaseStageInfoChangeHandlerTest.cs @@ -0,0 +1,22 @@ +// Copyright (c) andy840119 . 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.Graphics; +using osu.Game.Rulesets.Karaoke.Stages.Infos; + +namespace osu.Game.Rulesets.Karaoke.Tests.Editor.ChangeHandlers.Stages; + +public abstract partial class BaseStageInfoChangeHandlerTest : BaseChangeHandlerTest + where TChangeHandler : Component +{ + protected virtual void SetUpStageInfo(Action? action = null) + => throw new NotImplementedException(); + + public void AssertStageInfos(Action> assert) + => throw new NotImplementedException(); + + public void AssertStageInfo(Action assert) where TStageInfo: StageInfo + => throw new NotImplementedException(); +} diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/ClassicStageChangeHandlerTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/ClassicStageChangeHandlerTest.cs index 2f40dc52e..2ba882ae6 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/ClassicStageChangeHandlerTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/ClassicStageChangeHandlerTest.cs @@ -1,25 +1,24 @@ // Copyright (c) andy840119 . 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.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; using osu.Game.Rulesets.Karaoke.Objects; -using osu.Game.Rulesets.Karaoke.Stages.Infos; using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; namespace osu.Game.Rulesets.Karaoke.Tests.Editor.ChangeHandlers.Stages; -public partial class ClassicStageChangeHandlerTest : BaseChangeHandlerTest +[Ignore("Ignore all stage-related change handler test until able to edit the stage info.")] +public partial class ClassicStageChangeHandlerTest : BaseStageInfoChangeHandlerTest { #region Layout definition [Test] public void TestEditLayoutDefinition() { + SetUpStageInfo(); + TriggerHandlerChanged(c => { c.EditLayoutDefinition(x => @@ -28,13 +27,12 @@ public void TestEditLayoutDefinition() }); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var classicStageInfo = getClassicStageInfo(karaokeBeatmap); - Assert.IsNotNull(classicStageInfo); + Assert.IsNotNull(stageInfo); // assert definition. - var definition = classicStageInfo.StageDefinition; + var definition = stageInfo.StageDefinition; Assert.AreEqual(12, definition.LineHeight); }); } @@ -46,6 +44,8 @@ public void TestEditLayoutDefinition() [Test] public void TestAddTimingPoint() { + SetUpStageInfo(); + TriggerHandlerChanged(c => { c.AddTimingPoint(x => @@ -54,9 +54,9 @@ public void TestAddTimingPoint() }); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert timing. @@ -71,9 +71,9 @@ public void TestRemoveTimingPoint() { ClassicLyricTimingPoint removedTimingPoint = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; timingInfo.Timings.Add(new ClassicLyricTimingPoint { Time = 1000, @@ -90,9 +90,9 @@ public void TestRemoveTimingPoint() c.RemoveTimingPoint(removedTimingPoint); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert timing. @@ -107,9 +107,9 @@ public void TestRemoveRangeOfTimingPoints() { ClassicLyricTimingPoint removedTimingPoint = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; timingInfo.Timings.Add(new ClassicLyricTimingPoint { Time = 1000, @@ -126,9 +126,9 @@ public void TestRemoveRangeOfTimingPoints() c.RemoveRangeOfTimingPoints(new[] { removedTimingPoint }); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert timing. @@ -144,9 +144,9 @@ public void TestShiftingTimingPoints() ClassicLyricTimingPoint shiftingTimingPoint1 = null!; ClassicLyricTimingPoint shiftingTimingPoint2 = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; timingInfo.Timings.Add(shiftingTimingPoint1 = new ClassicLyricTimingPoint { Time = 1000, @@ -163,9 +163,9 @@ public void TestShiftingTimingPoints() c.ShiftingTimingPoints(new[] { shiftingTimingPoint1, shiftingTimingPoint2 }, 100); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert timing. @@ -181,9 +181,9 @@ public void TestAddLyricIntoTimingPoint() { ClassicLyricTimingPoint timingPoint = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; timingInfo.Timings.Add(timingPoint = new ClassicLyricTimingPoint { Time = 1000, @@ -201,9 +201,9 @@ public void TestAddLyricIntoTimingPoint() c.AddLyricIntoTimingPoint(timingPoint); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert mapping status. @@ -223,9 +223,9 @@ public void TestRemoveLyricFromTimingPoint() PrepareHitObject(() => lyric1 = new Lyric()); PrepareHitObject(() => lyric2 = new Lyric(), false); - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; timingInfo.Timings.Add(timingPoint = new ClassicLyricTimingPoint { Time = 1000, @@ -239,9 +239,9 @@ public void TestRemoveLyricFromTimingPoint() c.RemoveLyricFromTimingPoint(timingPoint); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var timingInfo = getLyricTimingInfo(karaokeBeatmap); + var timingInfo = stageInfo.LyricTimingInfo; Assert.IsNotNull(timingInfo); // assert mapping status. @@ -251,25 +251,4 @@ public void TestRemoveLyricFromTimingPoint() } #endregion - - protected override void SetUpKaraokeBeatmap(Action action) - { - base.SetUpKaraokeBeatmap(karaokeBeatmap => - { - var stageInfo = new ClassicStageInfo(); - karaokeBeatmap.StageInfos = new List - { - stageInfo, - }; - karaokeBeatmap.CurrentStageInfo = stageInfo; - - action(karaokeBeatmap); - }); - } - - private static ClassicStageInfo getClassicStageInfo(KaraokeBeatmap karaokeBeatmap) - => karaokeBeatmap.StageInfos.OfType().First(); - - private static ClassicLyricTimingInfo getLyricTimingInfo(KaraokeBeatmap karaokeBeatmap) - => getClassicStageInfo(karaokeBeatmap).LyricTimingInfo; } diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs index 1614827b4..34dc12deb 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StageElementCategoryChangeHandlerTest.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Game.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; using osu.Game.Rulesets.Karaoke.Objects; using osu.Game.Rulesets.Karaoke.Stages; @@ -16,7 +15,8 @@ namespace osu.Game.Rulesets.Karaoke.Tests.Editor.ChangeHandlers.Stages; -public partial class StageElementCategoryChangeHandlerTest : BaseChangeHandlerTest +[Ignore("Ignore all stage-related change handler test until able to edit the stage info.")] +public partial class StageElementCategoryChangeHandlerTest : BaseStageInfoChangeHandlerTest { protected override TestStageElementCategoryChangeHandler CreateChangeHandler() => new(x => x.OfType().First().Category); @@ -32,9 +32,9 @@ public void TestAddElement() }); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; var firstElement = category.AvailableElements.First(); Assert.AreEqual("Element 1", firstElement.Name); @@ -46,9 +46,9 @@ public void TestEditElement() { TestStageElement element = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; element = category.AddElement(); }); @@ -60,9 +60,9 @@ public void TestEditElement() }); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; var firstElement = category.AvailableElements.First(); Assert.AreEqual("Edit Element 1", firstElement.Name); @@ -74,9 +74,9 @@ public void TestRemoveElement() { TestStageElement element = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; element = category.AddElement(); }); @@ -85,9 +85,9 @@ public void TestRemoveElement() c.RemoveElement(element); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; Assert.IsEmpty(category.AvailableElements); }); @@ -98,9 +98,9 @@ public void TestAddToMapping() { TestStageElement element = null!; - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; element = category.AddElement(); }); @@ -111,9 +111,9 @@ public void TestAddToMapping() c.AddToMapping(element); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; Assert.IsNotEmpty(category.Mappings); }); @@ -125,9 +125,9 @@ public void TestOffsetMapping() Lyric lyric = new Lyric(); Lyric unSelectedLyric = new Lyric(); - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; var element = category.AddElement(x => x.Name = "Element 1"); category.AddElement(x => x.Name = "Element 2"); @@ -144,9 +144,9 @@ public void TestOffsetMapping() c.OffsetMapping(1); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; Assert.AreEqual("Element 2", category.GetElementByItem(lyric).Name); Assert.AreEqual("Element 1", category.GetElementByItem(unSelectedLyric).Name); // should not change the id if lyric is not selected. @@ -167,9 +167,9 @@ public void TestRemoveFromMapping() { Lyric lyric = new Lyric(); - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; var element = category.AddElement(); // Add to Mapping @@ -183,9 +183,9 @@ public void TestRemoveFromMapping() c.RemoveFromMapping(); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; Assert.IsEmpty(category.Mappings); }); @@ -194,9 +194,9 @@ public void TestRemoveFromMapping() [Test] public void TestClearUnusedMapping() { - SetUpKaraokeBeatmap(karaokeBeatmap => + SetUpStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; var element = category.AddElement(); // Add to Mapping @@ -208,32 +208,14 @@ public void TestClearUnusedMapping() c.ClearUnusedMapping(); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfo(stageInfo => { - var category = getStageCategory(karaokeBeatmap); + var category = stageInfo.Category; Assert.IsEmpty(category.Mappings); }); } - protected override void SetUpKaraokeBeatmap(Action action) - { - base.SetUpKaraokeBeatmap(karaokeBeatmap => - { - var stageInfo = new TestStageInfo(); - karaokeBeatmap.StageInfos = new List - { - stageInfo, - }; - karaokeBeatmap.CurrentStageInfo = stageInfo; - - action(karaokeBeatmap); - }); - } - - private static TestCategory getStageCategory(KaraokeBeatmap beatmap) - => beatmap.StageInfos.OfType().First().Category; - public partial class TestStageElementCategoryChangeHandler : StageElementCategoryChangeHandler { public TestStageElementCategoryChangeHandler(Func, StageElementCategory> stageCategoryAction) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StagesChangeHandlerTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StagesChangeHandlerTest.cs index 4c0ece363..b99428d96 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StagesChangeHandlerTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/Stages/StagesChangeHandlerTest.cs @@ -2,16 +2,15 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; using NUnit.Framework; using osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; using osu.Game.Rulesets.Karaoke.Objects; -using osu.Game.Rulesets.Karaoke.Stages.Infos; using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; namespace osu.Game.Rulesets.Karaoke.Tests.Editor.ChangeHandlers.Stages; -public partial class StagesChangeHandlerTest : BaseChangeHandlerTest +[Ignore("Ignore all stage-related change handler test until able to edit the stage info.")] +public partial class StagesChangeHandlerTest : BaseStageInfoChangeHandlerTest { protected override bool IncludeAutoGenerator => true; @@ -26,9 +25,8 @@ public void TestAutoGenerate() c.AutoGenerate(); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfos(stageInfos => { - var stageInfos = karaokeBeatmap.StageInfos; Assert.AreEqual(1, stageInfos.Count); Assert.AreEqual(typeof(ClassicStageInfo), stageInfos[0].GetType()); }); @@ -43,24 +41,15 @@ public void TestAutoGenerate() [Test] public void TestRemove() { - SetUpKaraokeBeatmap(karaokeBeatmap => - { - var stageInfo = new ClassicStageInfo(); - karaokeBeatmap.StageInfos = new List - { - stageInfo, - }; - karaokeBeatmap.CurrentStageInfo = stageInfo; - }); + SetUpStageInfo(); TriggerHandlerChanged(c => { c.Remove(); }); - AssertKaraokeBeatmap(karaokeBeatmap => + AssertStageInfos(stageInfos => { - var stageInfos = karaokeBeatmap.StageInfos; Assert.AreEqual(0, stageInfos.Count); }); diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/ClassicStageChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/ClassicStageChangeHandler.cs index f73e44bb5..502a93034 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/ClassicStageChangeHandler.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/ClassicStageChangeHandler.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; -public partial class ClassicStageChangeHandler : BeatmapPropertyChangeHandler, IClassicStageChangeHandler +public partial class ClassicStageChangeHandler : StagePropertyChangeHandler, IClassicStageChangeHandler { [Resolved] private EditorBeatmap beatmap { get; set; } = null!; @@ -103,22 +103,11 @@ private static bool checkTimingPointExist(ClassicLyricTimingInfo timingInfo, Cla private void performStageInfoChanged(Action action) { - PerformBeatmapChanged(beatmap => - { - if (beatmap.CurrentStageInfo is not ClassicStageInfo classicStageInfo) - throw new NotSupportedException($"Current stage info in the beatmap should be {nameof(ClassicStageInfo)}"); - - action(classicStageInfo); - }); + throw new NotImplementedException(); } private void performTimingInfoChanged(Action action) { - performStageInfoChanged(stageInfo => - { - action(stageInfo.LyricTimingInfo); - - // todo: should trigger the stage wrapper to update the view. - }); + throw new NotImplementedException(); } } diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StageElementCategoryChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StageElementCategoryChangeHandler.cs index a169f3a9f..03d1bbb3a 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StageElementCategoryChangeHandler.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StageElementCategoryChangeHandler.cs @@ -10,7 +10,7 @@ namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; -public partial class StageElementCategoryChangeHandler : BeatmapPropertyChangeHandler, IStageElementCategoryChangeHandler +public partial class StageElementCategoryChangeHandler : StagePropertyChangeHandler, IStageElementCategoryChangeHandler where TStageElement : StageElement, IComparable, new() where THitObject : KaraokeHitObject, IHasPrimaryKey { @@ -102,10 +102,6 @@ public void ClearUnusedMapping() private void performStageInfoChanged(Action> stageAction) { - PerformBeatmapChanged(beatmap => - { - var stageCategory = stageCategoryAction(beatmap.StageInfos); - stageAction(stageCategory); - }); + throw new NotImplementedException(); } } diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagePropertyChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagePropertyChangeHandler.cs new file mode 100644 index 000000000..5e1b9eb53 --- /dev/null +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagePropertyChangeHandler.cs @@ -0,0 +1,23 @@ +// Copyright (c) andy840119 . 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.Graphics; +using osu.Game.Rulesets.Karaoke.Objects; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; + +/// +/// Note: will start to implement this class after the stage info is able to edit. +/// +public partial class StagePropertyChangeHandler : Component +{ + protected IEnumerable Lyrics => throw new NotImplementedException(); + + protected void PerformOnSelection(Action action) where T : HitObject + { + throw new NotImplementedException(); + } +} diff --git a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagesChangeHandler.cs b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagesChangeHandler.cs index 6dba56dbe..23006e0f4 100644 --- a/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagesChangeHandler.cs +++ b/osu.Game.Rulesets.Karaoke/Edit/ChangeHandlers/Stages/StagesChangeHandler.cs @@ -2,18 +2,27 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Framework.Localisation; using osu.Game.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Configuration; using osu.Game.Rulesets.Karaoke.Edit.Generator.Stages; +using osu.Game.Rulesets.Karaoke.Edit.Utils; using osu.Game.Rulesets.Karaoke.Stages.Infos; +using osu.Game.Screens.Edit; namespace osu.Game.Rulesets.Karaoke.Edit.ChangeHandlers.Stages; -public partial class StagesChangeHandler : BeatmapPropertyChangeHandler, IStagesChangeHandler +public partial class StagesChangeHandler : StagePropertyChangeHandler, IStagesChangeHandler { + [Resolved] + private EditorBeatmap beatmap { get; set; } = null!; + + private KaraokeBeatmap karaokeBeatmap => EditorBeatmapUtils.GetPlayableBeatmap(beatmap); + [Resolved] private KaraokeRulesetEditGeneratorConfigManager generatorConfigManager { get; set; } = null!; @@ -27,12 +36,12 @@ public bool CanGenerate() where TStageInfo : StageInfo public LocalisableString? GetGeneratorNotSupportedMessage() where TStageInfo : StageInfo { - var stage = getStageInfo(KaraokeBeatmap); + var stage = getStageInfo(); if (stage != null) return $"{nameof(TStageInfo)} already exist in the beatmap."; var generator = new StageInfoGeneratorSelector(generatorConfigManager); - return generator.GetInvalidMessage(KaraokeBeatmap); + return generator.GetInvalidMessage(karaokeBeatmap); } void IAutoGenerateChangeHandler.AutoGenerate() @@ -40,38 +49,30 @@ void IAutoGenerateChangeHandler.AutoGenerate() public void AutoGenerate() where TStageInfo : StageInfo { - PerformBeatmapChanged(beatmap => - { - var stage = getStageInfo(beatmap); - if (stage != null) - throw new InvalidOperationException($"{nameof(TStageInfo)} already exist in the beatmap."); + var stage = getStageInfo(); + if (stage != null) + throw new InvalidOperationException($"{nameof(TStageInfo)} already exist in the beatmap."); - var generator = new StageInfoGeneratorSelector(generatorConfigManager); - var stageInfo = generator.Generate(beatmap); + var generator = new StageInfoGeneratorSelector(generatorConfigManager); + var stageInfo = generator.Generate(karaokeBeatmap); - beatmap.StageInfos.Add(stageInfo); - }); + getStageInfos().Add(stageInfo); } public void Remove() where TStageInfo : StageInfo { - PerformBeatmapChanged(beatmap => - { - var stage = getStageInfo(beatmap); - if (stage == null) - throw new InvalidOperationException($"There's no {nameof(TStageInfo)} in the beatmap."); - - beatmap.StageInfos.Remove(stage); - - // Should clear the current stage info if stage is removed. - // Beatmap processor will load the suitable stage info. - if (beatmap.CurrentStageInfo == stage) - { - beatmap.CurrentStageInfo = null!; - } - }); + var stage = getStageInfo(); + if (stage == null) + throw new InvalidOperationException($"There's no {nameof(TStageInfo)} in the beatmap."); + + getStageInfos().Remove(stage); + + // todo: maybe should update the current stage. } - private TStageInfo? getStageInfo(KaraokeBeatmap beatmap) where TStageInfo : StageInfo - => beatmap.StageInfos.OfType().FirstOrDefault(); + private IList getStageInfos() + => throw new NotImplementedException(); + + private TStageInfo? getStageInfo() where TStageInfo : StageInfo + => throw new NotImplementedException(); } From ace95e6454d9f903e3c382f13d5fc3905a3d8a0e Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 24 Nov 2024 19:44:21 +0800 Subject: [PATCH 5/7] Remove unnecessary check. --- .../ChangeHandlers/BaseChangeHandlerTest.cs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/BaseChangeHandlerTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/BaseChangeHandlerTest.cs index b4eb29fb4..3895d9eab 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/BaseChangeHandlerTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/ChangeHandlers/BaseChangeHandlerTest.cs @@ -192,10 +192,6 @@ protected void AssertStatus() // also, technically should not call the change handler if there's no possible to change the properties. AssertTransactionOnlyTriggerOnce(); - // We should make sure that the stage info is in the latest state. - // Should trigger the beatmap editor to run the beatmap processor if not the latest. - AssertCalculatedPropertyInStageInfoValid(); - // We should make sure that if the working property is changed by the change handler. // Should trigger the beatmap editor to run the beatmap processor to re-fill the working property. AssertWorkingPropertyInHitObjectValid(); @@ -209,21 +205,6 @@ protected void AssertTransactionOnlyTriggerOnce() }); } - protected void AssertCalculatedPropertyInStageInfoValid() - { - AddWaitStep("Waiting for working property being re-filled in the beatmap processor.", 1); - AddAssert("Check if working property in the hit object is valid", () => - { - var editorBeatmap = Dependencies.Get(); - var karaokeBeatmap = EditorBeatmapUtils.GetPlayableBeatmap(editorBeatmap); - if (karaokeBeatmap.CurrentStageInfo is IHasCalculatedProperty calculatedProperty) - return calculatedProperty.IsUpdated(); - - // ignore check if current stage info no need to calculate the property. - return true; - }); - } - protected void AssertWorkingPropertyInHitObjectValid() { AddWaitStep("Waiting for working property being re-filled in the beatmap processor.", 1); From d1cc4f8a58f2ed98087dedd023a224fadf4a53a8 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sat, 30 Nov 2024 20:21:29 +0800 Subject: [PATCH 6/7] Remove read stage ingo from the beatmap. --- .../Stages/Classic/ClassicStageScreenTestScene.cs | 13 ------------- .../Stages/Classic/TestSceneClassicStageEditor.cs | 11 ----------- .../Edit/Stages/Classic/TestSceneStageScreen.cs | 1 + .../Edit/Stages/Classic/Stage/StageScreen.cs | 12 ++---------- 4 files changed, 3 insertions(+), 34 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/ClassicStageScreenTestScene.cs b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/ClassicStageScreenTestScene.cs index 53e1d663e..602a0b9ba 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/ClassicStageScreenTestScene.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/ClassicStageScreenTestScene.cs @@ -1,24 +1,11 @@ // Copyright (c) andy840119 . Licensed under the GPL Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Game.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Screens.Edit.Stages.Classic; -using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; namespace osu.Game.Rulesets.Karaoke.Tests.Screens.Edit.Stages.Classic; public abstract partial class ClassicStageScreenTestScene : GenericEditorScreenTestScene where T : ClassicStageScreen { - protected override KaraokeBeatmap CreateBeatmap() - { - var karaokeBeatmap = base.CreateBeatmap(); - - // add classic stage info for testing purpose. - var stageInfo = new ClassicStageInfo(); - karaokeBeatmap.StageInfos.Add(stageInfo); - karaokeBeatmap.CurrentStageInfo = stageInfo; - - return karaokeBeatmap; - } } diff --git a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneClassicStageEditor.cs b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneClassicStageEditor.cs index 105ea0927..9e673fc63 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneClassicStageEditor.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneClassicStageEditor.cs @@ -1,21 +1,10 @@ // Copyright (c) andy840119 . Licensed under the GPL Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Game.Rulesets.Karaoke.Beatmaps; using osu.Game.Rulesets.Karaoke.Screens.Edit.Stages.Classic; -using osu.Game.Rulesets.Karaoke.Stages.Infos.Classic; namespace osu.Game.Rulesets.Karaoke.Tests.Screens.Edit.Stages.Classic; public partial class TestSceneClassicStageEditor : GenericEditorTestScene { - protected override KaraokeBeatmap CreateBeatmap() - { - var karaokeBeatmap = base.CreateBeatmap(); - - // add classic stage info for testing purpose. - karaokeBeatmap.StageInfos.Add(new ClassicStageInfo()); - - return karaokeBeatmap; - } } diff --git a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneStageScreen.cs b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneStageScreen.cs index b078537c0..498c55b15 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneStageScreen.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Screens/Edit/Stages/Classic/TestSceneStageScreen.cs @@ -8,6 +8,7 @@ namespace osu.Game.Rulesets.Karaoke.Tests.Screens.Edit.Stages.Classic; +[Ignore("Ingore this test until able to edit the stage.")] public partial class TestSceneStageScreen : ClassicStageScreenTestScene { protected override StageScreen CreateEditorScreen() => new(); diff --git a/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageScreen.cs b/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageScreen.cs index d66551097..505a2b5ae 100644 --- a/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageScreen.cs +++ b/osu.Game.Rulesets.Karaoke/Screens/Edit/Stages/Classic/Stage/StageScreen.cs @@ -28,20 +28,12 @@ public partial class StageScreen : ClassicStageScreen, IStageEditorStateProvider [Cached(typeof(IStageEditorVerifier))] private readonly StageEditorVerifier stageEditorVerifier; - [Resolved] - private EditorBeatmap editorBeatmap { get; set; } = null!; - public ClassicStageInfo StageInfo { get { - // we should make sure that current stage info is classic stage info. - // otherwise, we might not able to see the edit result in the editor. - var currentStageInfo = EditorBeatmapUtils.GetPlayableBeatmap(editorBeatmap).CurrentStageInfo; - if (currentStageInfo is not ClassicStageInfo classicStageInfo) - throw new NotSupportedException(); - - return classicStageInfo; + // todo: should be able to read the stage info from the beatmap. + throw new NotImplementedException(); } } From 2f0203f4a3ec34fc334ae0885d38ca6790d64c1c Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sat, 30 Nov 2024 20:21:45 +0800 Subject: [PATCH 7/7] Seems finally able to remove the stage info from the beatmap. --- osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs b/osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs index fa43064dd..108f7b336 100644 --- a/osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs +++ b/osu.Game.Rulesets.Karaoke/Beatmaps/KaraokeBeatmap.cs @@ -21,14 +21,6 @@ public class KaraokeBeatmap : Beatmap public PageInfo PageInfo { get; set; } = new(); - public IList StageInfos { get; set; } = new List(); - - /// - /// This property will not be null after is called. - /// - [JsonIgnore] - public StageInfo? CurrentStageInfo { get; set; } - public NoteInfo NoteInfo { get; set; } = new(); public bool Scorable { get; set; }