Add verify checks for incorrect title markers

[64ArthurAraujo]

Addresses a check listed at #12091. Adds checks for incorrect formats of tv size, cut ver, nightcore mix, and the others, based on the Mapset Verifier code.

[frenzibyte]

I honestly don't know whether this check is worth spending any time on, but assuming it is, I tried refactoring this into a state that looks to be at least mergable in my eyes, and I got stuck in some Regex weirdness that is making me regret this.

diff

diff: ```diff diff --git a/osu.Game.Tests/Editing/Checks/CheckTitleMarkersTest.cs b/osu.Game.Tests/Editing/Checks/CheckTitleMarkersTest.cs index a8f86a6d45..441dcdce3e 100644 --- a/osu.Game.Tests/Editing/Checks/CheckTitleMarkersTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckTitleMarkersTest.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; @@ -39,192 +40,128 @@ public void Setup() [Test] public void TestNoTitleMarkers() { - var issues = check.Run(getContext(beatmap)).ToList(); - Assert.That(issues, Has.Count.EqualTo(0)); + performTest(string.Empty, string.Empty); } [Test] public void TestTvSizeMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (TV Size)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (TV Size)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedTvSizeMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (tv size)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (tv size)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(TV Size)", "(TV Size)"); + performTest("(Tv size)", "(TV Size)"); + performTest("[TV Size]", "(TV Size)"); + performTest("(TV Ver.)", "(TV Size)"); + performTest("(TV Ver)", "(TV Size)"); } [Test] public void TestGameVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Game Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Game Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedGameVerMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (game ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (game ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Game Ver.)", "(Game Ver.)"); + performTest("(Game ver.)", "(Game Ver.)"); + performTest("[Game Ver.]", "(Game Ver.)"); + performTest("(Game Size)", "(Game Ver.)"); + performTest("(Game Ver)", "(Game Ver.)"); } [Test] public void TestShortVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Short Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Short Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedShortVerMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (short ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (short ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Short Ver.)", "(Short Ver.)"); + performTest("(Short ver.)", "(Short Ver.)"); + performTest("[Short Ver.]", "(Short Ver.)"); + performTest("(Short Size)", "(Short Ver.)"); + performTest("(Short Ver)", "(Short Ver.)"); } [Test] public void TestCutVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Cut Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Cut Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedCutVerMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (cut ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (cut ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Cut Ver.)", "(Cut Ver.)"); + performTest("(Cut ver.)", "(Cut Ver.)"); + performTest("[Cut Ver.]", "(Cut Ver.)"); + performTest("(Cut Size)", "(Cut Ver.)"); + performTest("(Cut Ver)", "(Cut Ver.)"); } [Test] public void TestSpedUpVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Sped Up Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Sped Up Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedSpedUpVerMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (sped up ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (sped up ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Sped Up Ver.)", "(Sped Up Ver.)"); + performTest("(Sped Up ver.)", "(Sped Up Ver.)"); + performTest("[Sped Up Ver.]", "(Sped Up Ver.)"); + performTest("(Speed Up Ver.)", "(Sped Up Ver.)"); + performTest("(Sped Up Ver)", "(Sped Up Ver.)"); } [Test] public void TestNightcoreMixMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Nightcore Mix)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Nightcore Mix)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); - } - - [Test] - public void TestMalformedNightcoreMixMarker() - { - beatmap.BeatmapInfo.Metadata.Title += " (nightcore mix)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (nightcore mix)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Nightcore Mix)", "(Nightcore Mix)"); + performTest("(Nightcore mix)", "(Nightcore Mix)"); + performTest("[Nightcore Mix]", "(Nightcore Mix)"); + performTest("(Nightcore Ver.)", "(Nightcore Mix)"); + performTest("(Nightcore Ver)", "(Nightcore Mix)"); + performTest("(Night Core Mix)", "(Nightcore Mix)"); + performTest("(Night Core Ver.)", "(Nightcore Mix)"); } [Test] public void TestSpedUpCutVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (Sped Up & Cut Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Sped Up & Cut Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); + performTest("(Sped Up & Cut Ver.)", "(Sped Up & Cut Ver.)"); + performTest("(Sped up & cut ver.)", "(Sped Up & Cut Ver.)"); + performTest("[Sped Up & Cut Ver.]", "(Sped Up & Cut Ver.)"); + performTest("(Speed Up & Cut Ver.)", "(Sped Up & Cut Ver.)"); + performTest("(Sped Up & Cut Size)", "(Sped Up & Cut Ver.)"); + performTest("(Sped Up Ver. & Cut Ver.)", "(Sped Up & Cut Ver.)"); + performTest("(Sped Up Ver & Cut Ver.)", "(Sped Up & Cut Ver.)"); + performTest("(SpedUp & Cut Ver.)", "(Sped Up & Cut Ver.)"); + performTest("(Sped Up & Cut Ver)", "(Sped Up & Cut Ver.)"); } [Test] - public void TestMalformedSpedUpCutVerMarker() + public void TestNightcoreCutVerMarker() { - beatmap.BeatmapInfo.Metadata.Title += " (sped up & cut ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (sped up & cut ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + performTest("(Nightcore & Cut Ver.)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore & cut ver.)", "(Nightcore & Cut Ver.)"); + performTest("[Nightcore & Cut Ver.]", "(Nightcore & Cut Ver.)"); + performTest("(Night Core & Cut Ver.)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore Ver. & Cut Ver.)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore Mix & Cut Ver.)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore & Cut Size)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore & Cut Ver)", "(Nightcore & Cut Ver.)"); + performTest("(Nightcore Ver & Cut Ver.)", "(Nightcore & Cut Ver.)"); } - [Test] - public void TestNightcoreCutVerMarker() + private void performTest(string marker, string expected) { - beatmap.BeatmapInfo.Metadata.Title += " (Nightcore & Cut Ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (Nightcore & Cut Ver.)"; - - var issues = check.Run(getContext(beatmap)).ToList(); - - Assert.That(issues, Has.Count.EqualTo(0)); + performTest(marker, expected, false); + performTest(marker, expected, true); } - [Test] - public void TestMalformedNightcoreCutVerMarker() + private void performTest(string marker, string expected, bool hasRomanisedTitle) { - beatmap.BeatmapInfo.Metadata.Title += " (nightcore & cut ver.)"; - beatmap.BeatmapInfo.Metadata.TitleUnicode += " (nightcore & cut ver.)"; + beatmap.BeatmapInfo.Metadata.Title = "Egao no Kanata " + marker; + + if (hasRomanisedTitle) + beatmap.BeatmapInfo.Metadata.TitleUnicode = "エガオノカナタ " + marker; + else + beatmap.BeatmapInfo.Metadata.TitleUnicode = beatmap.BeatmapInfo.Metadata.Title; var issues = check.Run(getContext(beatmap)).ToList(); - Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckTitleMarkers.IssueTemplateIncorrectMarker)); + if (marker == expected) + CollectionAssert.IsEmpty(issues); + else + { + if (hasRomanisedTitle) + { + Console.WriteLine(string.Join('\n', issues)); + Assert.That(issues, Has.Count.EqualTo(2)); + Assert.That(issues[0].Arguments[1], Is.EqualTo(expected)); + Assert.That(issues[0].Arguments[2], Is.EqualTo(marker)); + } + } } private BeatmapVerifierContext getContext(IBeatmap beatmap) @@ -232,4 +169,4 @@ private BeatmapVerifierContext getContext(IBeatmap beatmap) return new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap)); } } -} \ No newline at end of file +} diff --git a/osu.Game/Rulesets/Edit/Checks/CheckTitleMarkers.cs b/osu.Game/Rulesets/Edit/Checks/CheckTitleMarkers.cs index 2471d175ae..6d928bf006 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckTitleMarkers.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckTitleMarkers.cs @@ -3,73 +3,80 @@ using System.Collections.Generic; using System.Text.RegularExpressions; +using JetBrains.Annotations; +using osu.Game.Localisation; using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit.Checks { public class CheckTitleMarkers : ICheck { - public CheckMetadata Metadata => new CheckMetadata(CheckCategory.Metadata, "Checks for incorrect formats of (TV Size) / (Game Ver.) / (Short Ver.) / (Cut Ver.) / (Sped Up Ver.) / etc in title."); + private static readonly IReadOnlyList marker_checks = new[] + { + new MarkerFormatCheck(@"(TV Size)", @"(.tv (size|ver).*)"), + new MarkerFormatCheck(@"(Game Ver.)", @"(.game (size|ver).*)"), + new MarkerFormatCheck(@"(Short Ver.)", @"(.short (size|ver).*)"), + new MarkerFormatCheck(@"(Cut Ver.)", @"(? new CheckMetadata(CheckCategory.Metadata, "Incorrect format of markers such as \"(TV Size)\", \"(Game Ver.)\", etc. in title"); public IEnumerable PossibleTemplates => new IssueTemplate[] { new IssueTemplateIncorrectMarker(this), }; - private IEnumerable markerChecks = [ - new MarkerCheck("(TV Size)", @"(?i)(tv (size|ver))"), - new MarkerCheck("(Game Ver.)", @"(?i)(game (size|ver))"), - new MarkerCheck("(Short Ver.)", @"(?i)(short (size|ver))"), - new MarkerCheck("(Cut Ver.)", @"(?i)(? Run(BeatmapVerifierContext context) { - string romanisedTitle = context.Beatmap.Metadata.Title; - string unicodeTitle = context.Beatmap.Metadata.TitleUnicode; + string title = context.Beatmap.Metadata.Title; + string titleUnicode = context.Beatmap.Metadata.TitleUnicode; + bool hasRomanisedTitle = titleUnicode != title; - foreach (var check in markerChecks) + foreach (var check in marker_checks) { - bool hasRomanisedTitle = unicodeTitle != romanisedTitle; - - if (check.AnyRegex.IsMatch(romanisedTitle) && !check.ExactRegex.IsMatch(romanisedTitle)) + if (hasRomanisedTitle) { - yield return new IssueTemplateIncorrectMarker(this).Create(hasRomanisedTitle ? "Romanised title" : "Title", check.CorrectMarkerFormat); - } + var romanisedMatch = check.Regex.Match(title); + if (romanisedMatch.Length > 0 && romanisedMatch.Value != check.Expectation) + yield return new IssueTemplateIncorrectMarker(this).Create(check.Expectation, romanisedMatch.Value, true); - if (hasRomanisedTitle && check.AnyRegex.IsMatch(unicodeTitle) && !check.ExactRegex.IsMatch(unicodeTitle)) + var unicodeMatch = check.Regex.Match(titleUnicode); + if (unicodeMatch.Length > 0 && unicodeMatch.Value != check.Expectation) + yield return new IssueTemplateIncorrectMarker(this).Create(check.Expectation, unicodeMatch.Value, false); + } + else { - yield return new IssueTemplateIncorrectMarker(this).Create("Title", check.CorrectMarkerFormat); + var match = check.Regex.Match(title); + if (match.Length > 0 && match.Value != check.Expectation) + yield return new IssueTemplateIncorrectMarker(this).Create(check.Expectation, match.Value, false); } } } - private class MarkerCheck + public class IssueTemplateIncorrectMarker : IssueTemplate { - public string CorrectMarkerFormat; - public Regex ExactRegex; - public Regex AnyRegex; - - public MarkerCheck(string exact, string anyRegex) + public IssueTemplateIncorrectMarker(ICheck check) + : base(check, IssueType.Problem, "Expected marker in {0} to be in the format \"{1}\", but is \"{2}\"") { - CorrectMarkerFormat = exact; - ExactRegex = new Regex(Regex.Escape(exact)); - AnyRegex = new Regex(anyRegex); } + + public Issue Create(string expected, string actual, bool romanised) => new Issue(this, romanised ? EditorSetupStrings.RomanisedTitle.ToString() : EditorSetupStrings.Title.ToString(), expected, actual); } - public class IssueTemplateIncorrectMarker : IssueTemplate + private class MarkerFormatCheck { - public IssueTemplateIncorrectMarker(ICheck check) - : base(check, IssueType.Problem, "{0} field has a incorrect format of marker {1}") + public readonly Regex Regex; + public readonly string Expectation; + + public MarkerFormatCheck(string expectation, [RegexPattern] string pattern) { + Expectation = expectation; + Regex = new Regex(pattern, RegexOptions.IgnoreCase); } - - public Issue Create(string titleField, string correctMarkerFormat) => new Issue(this, titleField, correctMarkerFormat); } } -} \ No newline at end of file +} ```

Failing cases:

I'll just bail out from this PR and leave it to someone else because I spent too much time here.

[bdach]

Upon closer inspection I find the bulk of @frenzibyte's review invalid. This is completely fine, and requesting any changes is excessive.

I've done minor touch-ups to the logic and resolved code quality and that's it.