From 65ebdd8f7a48c57f31e46acd21ac3369f8521be9 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 10:08:08 +0200 Subject: [PATCH] Move check origin from `IssueTemplate` to `Issue` As a result we can also make check an interface, and need to provide the check itself when constructing an issue. --- .../Edit/Checks/CheckOffscreenObjects.cs | 14 +++++------ .../Edit/OsuBeatmapVerifier.cs | 2 +- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 2 +- .../Rulesets/Edit/Checks/CheckBackground.cs | 12 +++++----- .../Rulesets/Edit/Checks/Components/Check.cs | 14 ++++------- .../Rulesets/Edit/Checks/Components/Issue.cs | 23 +++++++++++-------- .../Edit/Checks/Components/IssueTemplate.cs | 5 ---- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 2 +- 8 files changed, 33 insertions(+), 41 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index 252f65ab5a..b34c9966a4 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -9,7 +9,7 @@ using osuTK; namespace osu.Game.Rulesets.Osu.Edit.Checks { - public class CheckOffscreenObjects : Check + public class CheckOffscreenObjects : ICheck { // These are close approximates to the edges of the screen // in gameplay on a 4:3 aspect ratio for osu!stable. @@ -22,13 +22,13 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks // (higher = more performant, but higher false-negative chance). private const int path_step_size = 5; - public override CheckMetadata Metadata { get; } = new CheckMetadata + public CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Compose, description: "Offscreen hitobjects." ); - public override IEnumerable PossibleTemplates => new[] + public IEnumerable PossibleTemplates => new[] { templateOffscreen, templateOffscreenSliderPath @@ -46,7 +46,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks unformattedMessage: "This slider goes offscreen here on a 4:3 aspect ratio." ); - public override IEnumerable Run(IBeatmap beatmap) + public IEnumerable Run(IBeatmap beatmap) { foreach (var hitobject in beatmap.HitObjects) { @@ -63,7 +63,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks case HitCircle circle: { if (isOffscreen(circle.StackedPosition, circle.Radius)) - yield return new Issue(circle, templateOffscreen); + yield return new Issue(this, circle, templateOffscreen); break; } @@ -89,7 +89,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks // `SpanDuration` ensures we don't include reverses. double time = slider.StartTime + progress * slider.SpanDuration; - yield return new Issue(slider, templateOffscreenSliderPath) { Time = time }; + yield return new Issue(this, slider, templateOffscreenSliderPath) { Time = time }; yield break; } @@ -98,7 +98,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks if (!isOffscreen(slider.StackedEndPosition, slider.Radius)) yield break; - yield return new Issue(slider, templateOffscreenSliderPath) { Time = slider.EndTime }; + yield return new Issue(this, slider, templateOffscreenSliderPath) { Time = slider.EndTime }; } private bool isOffscreen(Vector2 position, double radius) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 272612dbaf..2faa239720 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Osu.Edit { public class OsuBeatmapVerifier : BeatmapVerifier { - private readonly List checks = new List + private readonly List checks = new List { new CheckOffscreenObjects() }; diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index f67a068525..1d0508705a 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -15,7 +15,7 @@ namespace osu.Game.Rulesets.Edit /// Checks which are performed regardless of ruleset. /// These handle things like beatmap metadata, timing, and other ruleset agnostic elements. /// - private readonly IReadOnlyList generalChecks = new List + private readonly IReadOnlyList generalChecks = new List { new CheckBackground() }; diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 22f98b6fd7..c922aa03c0 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -8,15 +8,15 @@ using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit.Checks { - public class CheckBackground : Check + public class CheckBackground : ICheck { - public override CheckMetadata Metadata { get; } = new CheckMetadata + public CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Resources, description: "Missing background." ); - public override IEnumerable PossibleTemplates => new[] + public IEnumerable PossibleTemplates => new[] { templateNoneSet, templateDoesNotExist @@ -34,11 +34,11 @@ namespace osu.Game.Rulesets.Edit.Checks unformattedMessage: "The background file \"{0}\" is does not exist." ); - public override IEnumerable Run(IBeatmap beatmap) + public IEnumerable Run(IBeatmap beatmap) { if (beatmap.Metadata.BackgroundFile == null) { - yield return new Issue(templateNoneSet); + yield return new Issue(this, templateNoneSet); yield break; } @@ -51,7 +51,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (file != null) yield break; - yield return new Issue(templateDoesNotExist, beatmap.Metadata.BackgroundFile); + yield return new Issue(this, templateDoesNotExist, beatmap.Metadata.BackgroundFile); } } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 7c039d1572..f355ae734e 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -6,28 +6,22 @@ using osu.Game.Beatmaps; namespace osu.Game.Rulesets.Edit.Checks.Components { - public abstract class Check + public interface ICheck { /// /// The metadata for this check. /// - public abstract CheckMetadata Metadata { get; } + public CheckMetadata Metadata { get; } /// /// All possible templates for issues that this check may return. /// - public abstract IEnumerable PossibleTemplates { get; } + public IEnumerable PossibleTemplates { get; } /// /// Runs this check and returns any issues detected for the provided beatmap. /// /// The beatmap to run the check on. - public abstract IEnumerable Run(IBeatmap beatmap); - - protected Check() - { - foreach (var template in PossibleTemplates) - template.Origin = this; - } + public IEnumerable Run(IBeatmap beatmap); } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index 7241fabf5b..d0f7df857b 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -26,38 +26,41 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// public IssueTemplate Template; + /// + /// The check that this issue originates from. + /// + public ICheck Check; + /// /// The arguments that give this issue its context, based on the . These are then substituted into the . /// This could for instance include timestamps, which diff is being compared to, what some volume is, etc. /// public object[] Arguments; - public Issue(IssueTemplate template, params object[] args) + public Issue(ICheck check, IssueTemplate template, params object[] args) { + Check = check; Time = null; HitObjects = Array.Empty(); Template = template; Arguments = args; - - if (template.Origin == null) - throw new ArgumentException("A template had no origin. Make sure the `Templates()` method contains all templates used."); } - public Issue(double? time, IssueTemplate template, params object[] args) - : this(template, args) + public Issue(ICheck check, double? time, IssueTemplate template, params object[] args) + : this(check, template, args) { Time = time; } - public Issue(HitObject hitObject, IssueTemplate template, params object[] args) - : this(template, args) + public Issue(ICheck check, HitObject hitObject, IssueTemplate template, params object[] args) + : this(check, template, args) { Time = hitObject.StartTime; HitObjects = new[] { hitObject }; } - public Issue(IEnumerable hitObjects, IssueTemplate template, params object[] args) - : this(template, args) + public Issue(ICheck check, IEnumerable hitObjects, IssueTemplate template, params object[] args) + : this(check, template, args) { var hitObjectList = hitObjects.ToList(); diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index a1156c7c72..4a5f98ca5f 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -14,11 +14,6 @@ namespace osu.Game.Rulesets.Edit.Checks.Components private static readonly Color4 negligible_green = new Colour4(0.33f, 0.8f, 0.5f, 1.0f); private static readonly Color4 error_gray = new Colour4(0.5f, 0.5f, 0.5f, 1.0f); - /// - /// The check that this template originates from. - /// - public Check Origin; - /// /// The type of the issue. /// diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index 0c4aa04ff3..c0c4a040f6 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -130,7 +130,7 @@ namespace osu.Game.Screens.Edit.Verify { table.Issues = beatmapVerifier.Run(Beatmap) .OrderByDescending(issue => issue.Template.Type) - .ThenByDescending(issue => issue.Template.Origin.Metadata.Category); + .ThenByDescending(issue => issue.Check.Metadata.Category); } } }