From d58ef5310bf534f76a8d10170cdd71cd098bace0 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sun, 28 Mar 2021 17:36:22 +0200 Subject: [PATCH 01/55] Add verify tab Currently empty, but works. --- .../Input/Bindings/GlobalActionContainer.cs | 3 + osu.Game/Screens/Edit/Editor.cs | 9 + osu.Game/Screens/Edit/EditorScreenMode.cs | 3 + osu.Game/Screens/Edit/Verify/Issue.cs | 10 + osu.Game/Screens/Edit/Verify/IssueTable.cs | 191 ++++++++++++++++++ osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 68 +++++++ 6 files changed, 284 insertions(+) create mode 100644 osu.Game/Screens/Edit/Verify/Issue.cs create mode 100644 osu.Game/Screens/Edit/Verify/IssueTable.cs create mode 100644 osu.Game/Screens/Edit/Verify/VerifyScreen.cs diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index 8ccdb9249e..6705d3ee61 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -193,6 +193,9 @@ namespace osu.Game.Input.Bindings [Description("Timing mode")] EditorTimingMode, + [Description("Verify mode")] + EditorVerifyMode, + [Description("Hold for HUD")] HoldForHUD, diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 0c24eb6a4d..57499bf219 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -36,6 +36,7 @@ using osu.Game.Screens.Edit.Compose; using osu.Game.Screens.Edit.Design; using osu.Game.Screens.Edit.Setup; using osu.Game.Screens.Edit.Timing; +using osu.Game.Screens.Edit.Verify; using osu.Game.Screens.Play; using osu.Game.Users; using osuTK.Graphics; @@ -445,6 +446,10 @@ namespace osu.Game.Screens.Edit menuBar.Mode.Value = EditorScreenMode.SongSetup; return true; + case GlobalAction.EditorVerifyMode: + menuBar.Mode.Value = EditorScreenMode.Verify; + return true; + default: return false; } @@ -632,6 +637,10 @@ namespace osu.Game.Screens.Edit case EditorScreenMode.Timing: currentScreen = new TimingScreen(); break; + + case EditorScreenMode.Verify: + currentScreen = new VerifyScreen(); + break; } LoadComponentAsync(currentScreen, newScreen => diff --git a/osu.Game/Screens/Edit/EditorScreenMode.cs b/osu.Game/Screens/Edit/EditorScreenMode.cs index 12cfcc605b..ecd39f9b57 100644 --- a/osu.Game/Screens/Edit/EditorScreenMode.cs +++ b/osu.Game/Screens/Edit/EditorScreenMode.cs @@ -18,5 +18,8 @@ namespace osu.Game.Screens.Edit [Description("timing")] Timing, + + [Description("verify")] + Verify, } } diff --git a/osu.Game/Screens/Edit/Verify/Issue.cs b/osu.Game/Screens/Edit/Verify/Issue.cs new file mode 100644 index 0000000000..25e913d819 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Issue.cs @@ -0,0 +1,10 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Screens.Edit.Verify +{ + public class Issue + { + public readonly double Time; + } +} diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs new file mode 100644 index 0000000000..6476cebe48 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -0,0 +1,191 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Extensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Input.Events; +using osu.Game.Extensions; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; +using osu.Game.Graphics.Sprites; +using osuTK.Graphics; + +namespace osu.Game.Screens.Edit.Verify +{ + public class IssueTable : TableContainer + { + private const float horizontal_inset = 20; + private const float row_height = 25; + private const int text_size = 14; + + private readonly FillFlowContainer backgroundFlow; + + public IssueTable() + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + Padding = new MarginPadding { Horizontal = horizontal_inset }; + RowSize = new Dimension(GridSizeMode.Absolute, row_height); + + AddInternal(backgroundFlow = new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Depth = 1f, + Padding = new MarginPadding { Horizontal = -horizontal_inset }, + Margin = new MarginPadding { Top = row_height } + }); + } + + public IEnumerable Issues + { + set + { + Content = null; + backgroundFlow.Clear(); + + if (value?.Any() != true) + return; + + foreach (var issue in value) + { + backgroundFlow.Add(new IssueTable.RowBackground(issue)); + } + + Columns = createHeaders(); + Content = value.Select((g, i) => createContent(i, g)).ToArray().ToRectangular(); + } + } + + private TableColumn[] createHeaders() + { + var columns = new List + { + new TableColumn(string.Empty, Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), + new TableColumn("Time", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), + new TableColumn(), + new TableColumn("Attributes", Anchor.CentreLeft), + }; + + return columns.ToArray(); + } + + private Drawable[] createContent(int index, Issue issue) => new Drawable[] + { + new OsuSpriteText + { + Text = $"#{index + 1}", + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Margin = new MarginPadding(10) + }, + new OsuSpriteText + { + Text = issue.Time.ToEditorFormattedString(), + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold) + }, + null, + null //new ControlGroupAttributes(issue), + }; + + public class RowBackground : OsuClickableContainer + { + private readonly Issue issue; + private const int fade_duration = 100; + + private readonly Box hoveredBackground; + + [Resolved] + private EditorClock clock { get; set; } + + [Resolved] + private Bindable selectedIssue { get; set; } + + public RowBackground(Issue issue) + { + this.issue = issue; + RelativeSizeAxes = Axes.X; + Height = 25; + + AlwaysPresent = true; + + CornerRadius = 3; + Masking = true; + + Children = new Drawable[] + { + hoveredBackground = new Box + { + RelativeSizeAxes = Axes.Both, + Alpha = 0, + }, + }; + + Action = () => + { + selectedIssue.Value = issue; + clock.SeekSmoothlyTo(issue.Time); + }; + } + + private Color4 colourHover; + private Color4 colourSelected; + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + hoveredBackground.Colour = colourHover = colours.BlueDarker; + colourSelected = colours.YellowDarker; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + selectedIssue.BindValueChanged(group => { Selected = issue == group.NewValue; }, true); + } + + private bool selected; + + protected bool Selected + { + get => selected; + set + { + if (value == selected) + return; + + selected = value; + updateState(); + } + } + + protected override bool OnHover(HoverEvent e) + { + updateState(); + return base.OnHover(e); + } + + protected override void OnHoverLost(HoverLostEvent e) + { + updateState(); + base.OnHoverLost(e); + } + + private void updateState() + { + hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); + + if (selected || IsHovered) + hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); + else + hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); + } + } + } +} diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs new file mode 100644 index 0000000000..c15cefae83 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -0,0 +1,68 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; + +namespace osu.Game.Screens.Edit.Verify +{ + public class VerifyScreen : EditorScreenWithTimeline + { + public VerifyScreen() + : base(EditorScreenMode.Verify) + { + } + + protected override Drawable CreateMainContent() => new GridContainer + { + RelativeSizeAxes = Axes.Both, + ColumnDimensions = new[] + { + new Dimension(), + new Dimension(GridSizeMode.Absolute, 200), + }, + Content = new[] + { + new Drawable[] + { + new ControlPointList() + }, + } + }; + + public class ControlPointList : CompositeDrawable + { + private IssueTable table; + + [Resolved] + private EditorClock clock { get; set; } + + [Resolved] + protected EditorBeatmap Beatmap { get; private set; } + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + RelativeSizeAxes = Axes.Both; + + InternalChildren = new Drawable[] + { + new Box + { + Colour = colours.Gray0, + RelativeSizeAxes = Axes.Both, + }, + new OsuScrollContainer + { + RelativeSizeAxes = Axes.Both, + Child = table = new IssueTable(), + } + }; + } + } + } +} From b24ce66a0d4bfcc9f53d84fe998b6d4027ef8a5a Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 14:35:33 +0200 Subject: [PATCH 02/55] Add check/issue classes --- osu.Game/Rulesets/Edit/Checker.cs | 25 ++++++ .../Edit/Verify/Components/BeatmapCheck.cs | 19 +++++ .../Screens/Edit/Verify/Components/Check.cs | 41 +++++++++ .../Edit/Verify/Components/CheckMetadata.cs | 62 ++++++++++++++ .../Screens/Edit/Verify/Components/Issue.cs | 83 ++++++++++++++++++ .../Edit/Verify/Components/IssueTemplate.cs | 84 +++++++++++++++++++ osu.Game/Screens/Edit/Verify/Issue.cs | 10 --- 7 files changed, 314 insertions(+), 10 deletions(-) create mode 100644 osu.Game/Rulesets/Edit/Checker.cs create mode 100644 osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs create mode 100644 osu.Game/Screens/Edit/Verify/Components/Check.cs create mode 100644 osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs create mode 100644 osu.Game/Screens/Edit/Verify/Components/Issue.cs create mode 100644 osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs delete mode 100644 osu.Game/Screens/Edit/Verify/Issue.cs diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/Checker.cs new file mode 100644 index 0000000000..1c267c3435 --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checker.cs @@ -0,0 +1,25 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Checks; +using osu.Game.Screens.Edit.Verify.Components; + +namespace osu.Game.Rulesets.Edit +{ + public abstract class Checker + { + // These are all mode-invariant, hence here instead of in e.g. `OsuChecker`. + private readonly List beatmapChecks = new List + { + new CheckMetadataVowels() + }; + + public virtual IEnumerable Run(IBeatmap beatmap) + { + return beatmapChecks.SelectMany(check => check.Run(beatmap)); + } + } +} diff --git a/osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs b/osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs new file mode 100644 index 0000000000..7297dab60d --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs @@ -0,0 +1,19 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Game.Beatmaps; + +namespace osu.Game.Screens.Edit.Verify.Components +{ + public abstract class BeatmapCheck : Check + { + /// + /// Returns zero, one, or several issues detected by this + /// check on the given beatmap. + /// + /// The beatmap to run the check on. + /// + public abstract override IEnumerable Run(IBeatmap beatmap); + } +} diff --git a/osu.Game/Screens/Edit/Verify/Components/Check.cs b/osu.Game/Screens/Edit/Verify/Components/Check.cs new file mode 100644 index 0000000000..2ae21fd350 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Components/Check.cs @@ -0,0 +1,41 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; + +namespace osu.Game.Screens.Edit.Verify.Components +{ + public abstract class Check + { + /// + /// Returns the for this check. + /// Basically, its information. + /// + /// + public abstract CheckMetadata Metadata(); + + /// + /// The templates for issues that this check may use. + /// Basically, what issues this check can detect. + /// + /// + public abstract IEnumerable Templates(); + + protected Check() + { + foreach (var template in Templates()) + template.Origin = this; + } + } + + public abstract class Check : Check + { + /// + /// Returns zero, one, or several issues detected by + /// this check on the given object. + /// + /// The object to run the check on. + /// + public abstract IEnumerable Run(T obj); + } +} diff --git a/osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs b/osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs new file mode 100644 index 0000000000..1cac99d47d --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs @@ -0,0 +1,62 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Screens.Edit.Verify +{ + public class CheckMetadata + { + /// + /// The category of an issue. + /// + public enum CheckCategory + { + /// Anything to do with control points. + Timing, + + /// Anything to do with artist, title, creator, etc. + Metadata, + + /// Anything to do with non-audio files, e.g. background, skin, sprites, and video. + Resources, + + /// Anything to do with audio files, e.g. song and hitsounds. + Audio, + + /// Anything to do with files that don't fit into the above, e.g. unused, osu, or osb. + Files, + + /// Anything to do with hitobjects unrelated to spread. + Compose, + + /// Anything to do with difficulty levels or their progression. + Spread, + + /// Anything to do with variables like CS, OD, AR, HP, and global SV. + Settings, + + /// Anything to do with hitobject feedback. + Hitsounds, + + /// Anything to do with storyboarding, breaks, video offset, etc. + Events + } + + /// + /// The category this check belongs to. E.g. , + /// , or . + /// + public readonly CheckCategory Category; + + /// + /// Describes the issue(s) that this check looks for. Keep this brief, such that + /// it fits into "No {description}". E.g. "Offscreen objects" / "Too short sliders". + /// + public readonly string Description; + + public CheckMetadata(CheckCategory category, string description) + { + Category = category; + Description = description; + } + } +} diff --git a/osu.Game/Screens/Edit/Verify/Components/Issue.cs b/osu.Game/Screens/Edit/Verify/Components/Issue.cs new file mode 100644 index 0000000000..fe81cb9335 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Components/Issue.cs @@ -0,0 +1,83 @@ +// 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.Collections.Generic; +using System.Linq; +using osu.Game.Extensions; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Screens.Edit.Verify.Components +{ + public class Issue + { + /// + /// The time which this issue is associated with, if any, otherwise null. + /// + public double? Time; + + /// + /// The hitobjects which this issue is associated with. Empty by default. + /// + public IReadOnlyList HitObjects; + + /// + /// The template which this issue is using. This provides properties + /// such as the , and the + /// . + /// + public IssueTemplate Template; + + /// + /// The arguments that give this issue its context, based on the + /// . These are then substituted into the + /// . + /// E.g. timestamps, which diff is being compared to, what some volume is, etc. + /// + public object[] Arguments; + + public Issue(IssueTemplate template, params object[] args) + { + Time = null; + HitObjects = System.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) + { + Time = time; + } + + public Issue(IEnumerable hitObjects, IssueTemplate template, params object[] args) + : this(template, args) + { + Time = hitObjects.FirstOrDefault()?.StartTime; + HitObjects = hitObjects.ToArray(); + } + + public override string ToString() + { + return Template.Message(Arguments); + } + + public string GetEditorTimestamp() + { + // TODO: Editor timestamp formatting is handled in https://github.com/ppy/osu/pull/12030 + // We may be able to use that here too (if we decouple it from the HitObjectComposer class). + + if (Time == null) + return string.Empty; + + return Time.Value.ToEditorFormattedString(); + } + } +} diff --git a/osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs b/osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs new file mode 100644 index 0000000000..b178fa7122 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs @@ -0,0 +1,84 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using Humanizer; +using osu.Framework.Graphics; +using osuTK.Graphics; + +namespace osu.Game.Screens.Edit.Verify.Components +{ + public class IssueTemplate + { + /// + /// The type, or severity, of an issue. This decides its priority. + /// + public enum IssueType + { + /// A must-fix in the vast majority of cases. + Problem = 3, + + /// A possible mistake. Often requires critical thinking. + Warning = 2, + + // TODO: Try/catch all checks run and return error templates if exceptions occur. + /// An error occurred and a complete check could not be made. + Error = 1, + + // TODO: Negligible issues should be hidden by default. + /// A possible mistake so minor/unlikely that it can often be safely ignored. + Negligible = 0, + } + + /// + /// The check that this template originates from. + /// + public Check Origin; + + /// + /// The type of the issue. E.g. , + /// , or . + /// + public readonly IssueType Type; + + /// + /// The unformatted message given when this issue is detected. + /// This gets populated later when an issue is constructed with this template. + /// E.g. "Inconsistent snapping (1/{0}) with [{1}] (1/{2})." + /// + public readonly string UnformattedMessage; + + public IssueTemplate(IssueType type, string unformattedMessage) + { + Type = type; + UnformattedMessage = unformattedMessage; + } + + /// + /// Returns the formatted message given the arguments used to format it. + /// + /// The arguments used to format the message. + /// + public string Message(params object[] args) => UnformattedMessage.FormatWith(args); + + public static readonly Color4 PROBLEM_RED = new Colour4(1.0f, 0.4f, 0.4f, 1.0f); + public static readonly Color4 WARNING_YELLOW = new Colour4(1.0f, 0.8f, 0.2f, 1.0f); + public static readonly Color4 NEGLIGIBLE_GREEN = new Colour4(0.33f, 0.8f, 0.5f, 1.0f); + public static readonly Color4 ERROR_GRAY = new Colour4(0.5f, 0.5f, 0.5f, 1.0f); + + /// + /// Returns the colour corresponding to the type of this issue. + /// + /// + public Colour4 TypeColour() + { + return Type switch + { + IssueType.Problem => PROBLEM_RED, + IssueType.Warning => WARNING_YELLOW, + IssueType.Negligible => NEGLIGIBLE_GREEN, + IssueType.Error => ERROR_GRAY, + _ => Color4.White + }; + } + } +} diff --git a/osu.Game/Screens/Edit/Verify/Issue.cs b/osu.Game/Screens/Edit/Verify/Issue.cs deleted file mode 100644 index 25e913d819..0000000000 --- a/osu.Game/Screens/Edit/Verify/Issue.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -namespace osu.Game.Screens.Edit.Verify -{ - public class Issue - { - public readonly double Time; - } -} From 0343ef7f147b12a8bd71dfced08c6b503b6b8b79 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 14:36:43 +0200 Subject: [PATCH 03/55] Add ruleset-specific checker --- osu.Game.Rulesets.Osu/Edit/OsuChecker.cs | 30 ++++++++++++++++++++++++ osu.Game.Rulesets.Osu/OsuRuleset.cs | 2 ++ osu.Game/Rulesets/Ruleset.cs | 2 ++ 3 files changed, 34 insertions(+) create mode 100644 osu.Game.Rulesets.Osu/Edit/OsuChecker.cs diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs new file mode 100644 index 0000000000..9918b53c85 --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs @@ -0,0 +1,30 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Osu.Edit.Checks; +using osu.Game.Screens.Edit.Verify.Components; + +namespace osu.Game.Rulesets.Osu.Edit +{ + public class OsuChecker : Checker + { + public readonly List beatmapChecks = new List + { + new CheckConsecutiveCircles() + }; + + public override IEnumerable Run(IBeatmap beatmap) + { + // Also run mode-invariant checks. + foreach (var issue in base.Run(beatmap)) + yield return issue; + + foreach (var issue in beatmapChecks.SelectMany(check => check.Run(beatmap))) + yield return issue; + } + } +} diff --git a/osu.Game.Rulesets.Osu/OsuRuleset.cs b/osu.Game.Rulesets.Osu/OsuRuleset.cs index 838d707d64..74679bd578 100644 --- a/osu.Game.Rulesets.Osu/OsuRuleset.cs +++ b/osu.Game.Rulesets.Osu/OsuRuleset.cs @@ -206,6 +206,8 @@ namespace osu.Game.Rulesets.Osu public override HitObjectComposer CreateHitObjectComposer() => new OsuHitObjectComposer(this); + public override Checker CreateChecker() => new OsuChecker(); + public override string Description => "osu!"; public override string ShortName => SHORT_NAME; diff --git a/osu.Game/Rulesets/Ruleset.cs b/osu.Game/Rulesets/Ruleset.cs index 38d30a2e31..71f80c9982 100644 --- a/osu.Game/Rulesets/Ruleset.cs +++ b/osu.Game/Rulesets/Ruleset.cs @@ -202,6 +202,8 @@ namespace osu.Game.Rulesets public virtual HitObjectComposer CreateHitObjectComposer() => null; + public virtual Checker CreateChecker() => null; + public virtual Drawable CreateIcon() => new SpriteIcon { Icon = FontAwesome.Solid.QuestionCircle }; public virtual IResourceStore CreateResourceStore() => new NamespacedResourceStore(new DllResourceStore(GetType().Assembly), @"Resources"); From 9c4604e3c5fdbfa9f5201f39ee41584fa5b47d18 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 14:36:53 +0200 Subject: [PATCH 04/55] Add example checks --- .../Edit/Checks/CheckConsecutiveCircles.cs | 86 +++++++++++++++++++ osu.Game/Checks/CheckMetadataVowels.cs | 65 ++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs create mode 100644 osu.Game/Checks/CheckMetadataVowels.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs new file mode 100644 index 0000000000..c41c0dac2b --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs @@ -0,0 +1,86 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Screens.Edit.Verify; +using osu.Game.Screens.Edit.Verify.Components; + +namespace osu.Game.Rulesets.Osu.Edit.Checks +{ + public class CheckConsecutiveCircles : BeatmapCheck + { + private const double consecutive_threshold = 3; + private const double delta_time_min_expected = 300; + private const double delta_time_min_threshold = 100; + + public override CheckMetadata Metadata() => new CheckMetadata + ( + category: CheckMetadata.CheckCategory.Spread, + description: "Too many or fast consecutive circles." + ); + + private IssueTemplate templateManyInARow = new IssueTemplate + ( + type: IssueTemplate.IssueType.Problem, + unformattedMessage: "There are {0} circles in a row here, expected at most {1}." + ); + + private IssueTemplate templateTooFast = new IssueTemplate + ( + type: IssueTemplate.IssueType.Warning, + unformattedMessage: "These circles are too fast ({0:0} ms), expected at least {1:0} ms." + ); + + private IssueTemplate templateAlmostTooFast = new IssueTemplate + ( + type: IssueTemplate.IssueType.Negligible, + unformattedMessage: "These circles are almost too fast ({0:0} ms), expected at least {1:0} ms." + ); + + public override IEnumerable Templates() => new[] + { + templateManyInARow, + templateTooFast, + templateAlmostTooFast + }; + + public override IEnumerable Run(IBeatmap beatmap) + { + List prevCircles = new List(); + + foreach (HitObject hitobject in beatmap.HitObjects) + { + if (!(hitobject is HitCircle circle) || hitobject == beatmap.HitObjects.Last()) + { + if (prevCircles.Count > consecutive_threshold) + { + yield return new Issue( + prevCircles, + templateManyInARow, + prevCircles.Count, consecutive_threshold + ); + } + + prevCircles.Clear(); + continue; + } + + double? prevDeltaTime = circle.StartTime - prevCircles.LastOrDefault()?.StartTime; + prevCircles.Add(circle); + + if (prevDeltaTime == null || prevDeltaTime >= delta_time_min_expected) + continue; + + yield return new Issue( + prevCircles.TakeLast(2), + prevDeltaTime < delta_time_min_threshold ? templateTooFast : templateAlmostTooFast, + prevDeltaTime, delta_time_min_expected + ); + } + } + } +} diff --git a/osu.Game/Checks/CheckMetadataVowels.cs b/osu.Game/Checks/CheckMetadataVowels.cs new file mode 100644 index 0000000000..8bcfe89c3a --- /dev/null +++ b/osu.Game/Checks/CheckMetadataVowels.cs @@ -0,0 +1,65 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Screens.Edit.Verify; +using osu.Game.Screens.Edit.Verify.Components; + +namespace osu.Game.Checks +{ + public class CheckMetadataVowels : BeatmapCheck + { + private static readonly char[] vowels = { 'a', 'e', 'i', 'o', 'u' }; + + public override CheckMetadata Metadata() => new CheckMetadata + ( + category: CheckMetadata.CheckCategory.Metadata, + description: "Metadata fields contain vowels" + ); + + public override IEnumerable Templates() => new[] + { + templateArtistHasVowels + }; + + private IssueTemplate templateArtistHasVowels = new IssueTemplate + ( + type: IssueTemplate.IssueType.Warning, + unformattedMessage: "The {0} field \"{1}\" contains the vowel(s) {2}." + ); + + public override IEnumerable Run(IBeatmap beatmap) + { + foreach (var issue in GetVowelIssues("artist", beatmap.Metadata.Artist)) + yield return issue; + + foreach (var issue in GetVowelIssues("unicode artist", beatmap.Metadata.ArtistUnicode)) + yield return issue; + + foreach (var issue in GetVowelIssues("title", beatmap.Metadata.Title)) + yield return issue; + + foreach (var issue in GetVowelIssues("unicode title", beatmap.Metadata.TitleUnicode)) + yield return issue; + } + + private IEnumerable GetVowelIssues(string fieldName, string fieldValue) + { + if (fieldValue == null) + // Unicode fields can be null if same as respective romanized fields. + yield break; + + List matches = vowels.Where(c => fieldValue.ToLower().Contains(c)).ToList(); + + if (!matches.Any()) + yield break; + + yield return new Issue( + templateArtistHasVowels, + fieldName, fieldValue, string.Join(", ", matches) + ); + } + } +} From bab36e529a5b9e1ff5b2300e9def2b5bc11687a7 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 14:38:43 +0200 Subject: [PATCH 05/55] Update UI with new components --- osu.Game/Screens/Edit/Verify/IssueSettings.cs | 47 +++++++++ osu.Game/Screens/Edit/Verify/IssueTable.cs | 91 +++++++++-------- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 98 +++++++++++++++---- .../Screens/Edit/Verify/VisibilitySettings.cs | 52 ++++++++++ 4 files changed, 229 insertions(+), 59 deletions(-) create mode 100644 osu.Game/Screens/Edit/Verify/IssueSettings.cs create mode 100644 osu.Game/Screens/Edit/Verify/VisibilitySettings.cs diff --git a/osu.Game/Screens/Edit/Verify/IssueSettings.cs b/osu.Game/Screens/Edit/Verify/IssueSettings.cs new file mode 100644 index 0000000000..608be877de --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/IssueSettings.cs @@ -0,0 +1,47 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; + +namespace osu.Game.Screens.Edit.Verify +{ + public class IssueSettings : CompositeDrawable + { + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + RelativeSizeAxes = Axes.Both; + + InternalChildren = new Drawable[] + { + new Box + { + Colour = colours.Gray3, + RelativeSizeAxes = Axes.Both, + }, + new OsuScrollContainer + { + RelativeSizeAxes = Axes.Both, + Child = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Children = createSections() + }, + } + }; + } + + private IReadOnlyList createSections() => new Drawable[] + { + new VisibilitySettings() + }; + } +} diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 6476cebe48..c70695a849 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -4,16 +4,16 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; -using osu.Game.Extensions; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; +using osu.Game.Input.Bindings; +using osu.Game.Screens.Edit.Verify.Components; using osuTK.Graphics; namespace osu.Game.Screens.Edit.Verify @@ -34,6 +34,9 @@ namespace osu.Game.Screens.Edit.Verify Padding = new MarginPadding { Horizontal = horizontal_inset }; RowSize = new Dimension(GridSizeMode.Absolute, row_height); + Masking = true; + CornerRadius = 6; + AddInternal(backgroundFlow = new FillFlowContainer { RelativeSizeAxes = Axes.Both, @@ -55,7 +58,7 @@ namespace osu.Game.Screens.Edit.Verify foreach (var issue in value) { - backgroundFlow.Add(new IssueTable.RowBackground(issue)); + backgroundFlow.Add(new RowBackground(issue)); } Columns = createHeaders(); @@ -68,9 +71,10 @@ namespace osu.Game.Screens.Edit.Verify var columns = new List { new TableColumn(string.Empty, Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), + new TableColumn("Type", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), new TableColumn("Time", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), - new TableColumn(), - new TableColumn("Attributes", Anchor.CentreLeft), + new TableColumn("Message", Anchor.CentreLeft), + new TableColumn("Category", Anchor.CentreRight, new Dimension(GridSizeMode.AutoSize)), }; return columns.ToArray(); @@ -81,21 +85,36 @@ namespace osu.Game.Screens.Edit.Verify new OsuSpriteText { Text = $"#{index + 1}", + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium) + }, + new OsuSpriteText + { + Text = issue.Template.Type.ToString(), + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Margin = new MarginPadding { Left = 10 }, + Colour = issue.Template.TypeColour() + }, + new OsuSpriteText + { + Text = issue.GetEditorTimestamp(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), Margin = new MarginPadding(10) }, new OsuSpriteText { - Text = issue.Time.ToEditorFormattedString(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold) + Text = issue.ToString(), + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium) }, - null, - null //new ControlGroupAttributes(issue), + new OsuSpriteText + { + Text = issue.Template.Origin.Metadata().Category.ToString(), + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Margin = new MarginPadding(10) + } }; public class RowBackground : OsuClickableContainer { - private readonly Issue issue; private const int fade_duration = 100; private readonly Box hoveredBackground; @@ -104,13 +123,15 @@ namespace osu.Game.Screens.Edit.Verify private EditorClock clock { get; set; } [Resolved] - private Bindable selectedIssue { get; set; } + private Editor editor { get; set; } + + [Resolved] + private EditorBeatmap editorBeatmap { get; set; } public RowBackground(Issue issue) { - this.issue = issue; RelativeSizeAxes = Axes.X; - Height = 25; + Height = row_height; AlwaysPresent = true; @@ -128,41 +149,29 @@ namespace osu.Game.Screens.Edit.Verify Action = () => { - selectedIssue.Value = issue; - clock.SeekSmoothlyTo(issue.Time); + // Supposed to work like clicking timestamps outside of the game. + // TODO: Is there already defined behaviour for this I may be able to call? + + if (issue.Time != null) + { + clock.Seek(issue.Time.Value); + editor.OnPressed(GlobalAction.EditorComposeMode); + } + + if (!issue.HitObjects.Any()) + return; + + editorBeatmap.SelectedHitObjects.Clear(); + editorBeatmap.SelectedHitObjects.AddRange(issue.HitObjects); }; } private Color4 colourHover; - private Color4 colourSelected; [BackgroundDependencyLoader] private void load(OsuColour colours) { hoveredBackground.Colour = colourHover = colours.BlueDarker; - colourSelected = colours.YellowDarker; - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - selectedIssue.BindValueChanged(group => { Selected = issue == group.NewValue; }, true); - } - - private bool selected; - - protected bool Selected - { - get => selected; - set - { - if (value == selected) - return; - - selected = value; - updateState(); - } } protected override bool OnHover(HoverEvent e) @@ -179,9 +188,9 @@ namespace osu.Game.Screens.Edit.Verify private void updateState() { - hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); + hoveredBackground.FadeColour(colourHover, 450, Easing.OutQuint); - if (selected || IsHovered) + if (IsHovered) hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); else hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index c15cefae83..88397fdbff 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -1,40 +1,70 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; +using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.Containers; +using osu.Game.Graphics.UserInterface; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Edit; +using osuTK; namespace osu.Game.Screens.Edit.Verify { - public class VerifyScreen : EditorScreenWithTimeline + public class VerifyScreen : EditorScreen { + private Ruleset ruleset; + private static Checker checker; // TODO: Should not be static, but apparently needs to be? + public VerifyScreen() : base(EditorScreenMode.Verify) { } - protected override Drawable CreateMainContent() => new GridContainer + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) { - RelativeSizeAxes = Axes.Both, - ColumnDimensions = new[] - { - new Dimension(), - new Dimension(GridSizeMode.Absolute, 200), - }, - Content = new[] - { - new Drawable[] - { - new ControlPointList() - }, - } - }; + var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - public class ControlPointList : CompositeDrawable + ruleset = parent.Get>().Value.BeatmapInfo.Ruleset?.CreateInstance(); + checker = ruleset?.CreateChecker(); + + return dependencies; + } + + [BackgroundDependencyLoader] + private void load() + { + Child = new Container + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding(20), + Child = new GridContainer + { + RelativeSizeAxes = Axes.Both, + ColumnDimensions = new[] + { + new Dimension(), + new Dimension(GridSizeMode.Absolute, 200), + }, + Content = new[] + { + new Drawable[] + { + new IssueList(), + new IssueSettings(), + }, + } + } + }; + } + + public class IssueList : CompositeDrawable { private IssueTable table; @@ -60,9 +90,41 @@ namespace osu.Game.Screens.Edit.Verify { RelativeSizeAxes = Axes.Both, Child = table = new IssueTable(), - } + }, + new FillFlowContainer + { + AutoSizeAxes = Axes.Both, + Anchor = Anchor.BottomRight, + Origin = Anchor.BottomRight, + Margin = new MarginPadding(20), + Children = new Drawable[] + { + new TriangleButton + { + Text = "Refresh", + Action = refresh, + Size = new Vector2(120, 40), + Anchor = Anchor.BottomRight, + Origin = Anchor.BottomRight, + }, + } + }, }; } + + protected override void LoadComplete() + { + base.LoadComplete(); + + refresh(); + } + + private void refresh() + { + table.Issues = checker.Run(Beatmap) + .OrderByDescending(issue => issue.Template.Type) + .ThenByDescending(issue => issue.Template.Origin.Metadata().Category); + } } } } diff --git a/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs b/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs new file mode 100644 index 0000000000..6488c616e4 --- /dev/null +++ b/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs @@ -0,0 +1,52 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Graphics.UserInterfaceV2; +using osuTK; + +namespace osu.Game.Screens.Edit.Verify +{ + internal class VisibilitySettings : CompositeDrawable + { + [BackgroundDependencyLoader] + private void load() + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + Padding = new MarginPadding(10); + + InternalChildren = new Drawable[] + { + new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Spacing = new Vector2(10), + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + new LabelledSwitchButton + { + Label = "Show problems", + Current = new Bindable(true) + }, + new LabelledSwitchButton + { + Label = "Show warnings", + Current = new Bindable(true) + }, + new LabelledSwitchButton + { + Label = "Show negligibles" + } + } + }, + }; + } + } +} From 43174b708c007e7bf8156fd3f6a01a6f8313a024 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 12:58:40 +0200 Subject: [PATCH 06/55] Remove visibility settings Can look into this later, not really important for a first iteration. --- osu.Game/Screens/Edit/Verify/IssueSettings.cs | 1 - .../Screens/Edit/Verify/VisibilitySettings.cs | 52 ------------------- 2 files changed, 53 deletions(-) delete mode 100644 osu.Game/Screens/Edit/Verify/VisibilitySettings.cs diff --git a/osu.Game/Screens/Edit/Verify/IssueSettings.cs b/osu.Game/Screens/Edit/Verify/IssueSettings.cs index 608be877de..4519231cd2 100644 --- a/osu.Game/Screens/Edit/Verify/IssueSettings.cs +++ b/osu.Game/Screens/Edit/Verify/IssueSettings.cs @@ -41,7 +41,6 @@ namespace osu.Game.Screens.Edit.Verify private IReadOnlyList createSections() => new Drawable[] { - new VisibilitySettings() }; } } diff --git a/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs b/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs deleted file mode 100644 index 6488c616e4..0000000000 --- a/osu.Game/Screens/Edit/Verify/VisibilitySettings.cs +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Game.Graphics.UserInterfaceV2; -using osuTK; - -namespace osu.Game.Screens.Edit.Verify -{ - internal class VisibilitySettings : CompositeDrawable - { - [BackgroundDependencyLoader] - private void load() - { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - - Padding = new MarginPadding(10); - - InternalChildren = new Drawable[] - { - new FillFlowContainer - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Spacing = new Vector2(10), - Direction = FillDirection.Vertical, - Children = new Drawable[] - { - new LabelledSwitchButton - { - Label = "Show problems", - Current = new Bindable(true) - }, - new LabelledSwitchButton - { - Label = "Show warnings", - Current = new Bindable(true) - }, - new LabelledSwitchButton - { - Label = "Show negligibles" - } - } - }, - }; - } - } -} From d1007ff26aa9b2f9827e666125d0161dba6fd35f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:02:22 +0200 Subject: [PATCH 07/55] Move components to more appropriate spot --- osu.Game.Rulesets.Osu/Edit/OsuChecker.cs | 2 +- osu.Game/Rulesets/Edit/Checker.cs | 4 +-- .../Edit/Checks}/Components/BeatmapCheck.cs | 2 +- .../Edit/Checks}/Components/Check.cs | 2 +- .../Edit/Checks}/Components/CheckMetadata.cs | 2 +- .../Edit/Checks}/Components/Issue.cs | 25 +++++++++++-------- .../Edit/Checks}/Components/IssueTemplate.cs | 2 +- osu.Game/Screens/Edit/Verify/IssueTable.cs | 2 +- 8 files changed, 22 insertions(+), 19 deletions(-) rename osu.Game/{Screens/Edit/Verify => Rulesets/Edit/Checks}/Components/BeatmapCheck.cs (92%) rename osu.Game/{Screens/Edit/Verify => Rulesets/Edit/Checks}/Components/Check.cs (96%) rename osu.Game/{Screens/Edit/Verify => Rulesets/Edit/Checks}/Components/CheckMetadata.cs (97%) rename osu.Game/{Screens/Edit/Verify => Rulesets/Edit/Checks}/Components/Issue.cs (79%) rename osu.Game/{Screens/Edit/Verify => Rulesets/Edit/Checks}/Components/IssueTemplate.cs (98%) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs index 9918b53c85..df3847f2fe 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs @@ -5,8 +5,8 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Osu.Edit.Checks; -using osu.Game.Screens.Edit.Verify.Components; namespace osu.Game.Rulesets.Osu.Edit { diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/Checker.cs index 1c267c3435..9bebfc0668 100644 --- a/osu.Game/Rulesets/Edit/Checker.cs +++ b/osu.Game/Rulesets/Edit/Checker.cs @@ -4,8 +4,8 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Beatmaps; -using osu.Game.Checks; -using osu.Game.Screens.Edit.Verify.Components; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit { diff --git a/osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs b/osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs similarity index 92% rename from osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs rename to osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs index 7297dab60d..d0f93b5eef 100644 --- a/osu.Game/Screens/Edit/Verify/Components/BeatmapCheck.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs @@ -4,7 +4,7 @@ using System.Collections.Generic; using osu.Game.Beatmaps; -namespace osu.Game.Screens.Edit.Verify.Components +namespace osu.Game.Rulesets.Edit.Checks.Components { public abstract class BeatmapCheck : Check { diff --git a/osu.Game/Screens/Edit/Verify/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs similarity index 96% rename from osu.Game/Screens/Edit/Verify/Components/Check.cs rename to osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 2ae21fd350..228c11f0f3 100644 --- a/osu.Game/Screens/Edit/Verify/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -3,7 +3,7 @@ using System.Collections.Generic; -namespace osu.Game.Screens.Edit.Verify.Components +namespace osu.Game.Rulesets.Edit.Checks.Components { public abstract class Check { diff --git a/osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs similarity index 97% rename from osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs rename to osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs index 1cac99d47d..5a226729ad 100644 --- a/osu.Game/Screens/Edit/Verify/Components/CheckMetadata.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -namespace osu.Game.Screens.Edit.Verify +namespace osu.Game.Rulesets.Edit.Checks.Components { public class CheckMetadata { diff --git a/osu.Game/Screens/Edit/Verify/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs similarity index 79% rename from osu.Game/Screens/Edit/Verify/Components/Issue.cs rename to osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index fe81cb9335..0f835202e7 100644 --- a/osu.Game/Screens/Edit/Verify/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -7,7 +7,7 @@ using System.Linq; using osu.Game.Extensions; using osu.Game.Rulesets.Objects; -namespace osu.Game.Screens.Edit.Verify.Components +namespace osu.Game.Rulesets.Edit.Checks.Components { public class Issue { @@ -39,7 +39,7 @@ namespace osu.Game.Screens.Edit.Verify.Components public Issue(IssueTemplate template, params object[] args) { Time = null; - HitObjects = System.Array.Empty(); + HitObjects = Array.Empty(); Template = template; Arguments = args; @@ -57,11 +57,20 @@ namespace osu.Game.Screens.Edit.Verify.Components Time = time; } + public Issue(HitObject hitObject, IssueTemplate template, params object[] args) + : this(template, args) + { + Time = hitObject.StartTime; + HitObjects = new[] { hitObject }; + } + public Issue(IEnumerable hitObjects, IssueTemplate template, params object[] args) : this(template, args) { - Time = hitObjects.FirstOrDefault()?.StartTime; - HitObjects = hitObjects.ToArray(); + var hitObjectList = hitObjects.ToList(); + + Time = hitObjectList.FirstOrDefault()?.StartTime; + HitObjects = hitObjectList; } public override string ToString() @@ -71,13 +80,7 @@ namespace osu.Game.Screens.Edit.Verify.Components public string GetEditorTimestamp() { - // TODO: Editor timestamp formatting is handled in https://github.com/ppy/osu/pull/12030 - // We may be able to use that here too (if we decouple it from the HitObjectComposer class). - - if (Time == null) - return string.Empty; - - return Time.Value.ToEditorFormattedString(); + return Time == null ? string.Empty : Time.Value.ToEditorFormattedString(); } } } diff --git a/osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs similarity index 98% rename from osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs rename to osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index b178fa7122..7eaabdc59b 100644 --- a/osu.Game/Screens/Edit/Verify/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -5,7 +5,7 @@ using Humanizer; using osu.Framework.Graphics; using osuTK.Graphics; -namespace osu.Game.Screens.Edit.Verify.Components +namespace osu.Game.Rulesets.Edit.Checks.Components { public class IssueTemplate { diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index c70695a849..20dceb5333 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -13,7 +13,7 @@ using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; using osu.Game.Input.Bindings; -using osu.Game.Screens.Edit.Verify.Components; +using osu.Game.Rulesets.Edit.Checks.Components; using osuTK.Graphics; namespace osu.Game.Screens.Edit.Verify From b30e41b805ffb7196851c42fc7c8b36c2828df0d Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:02:36 +0200 Subject: [PATCH 08/55] Fix comment; mode -> ruleset --- osu.Game/Rulesets/Edit/Checker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/Checker.cs index 9bebfc0668..6687160b10 100644 --- a/osu.Game/Rulesets/Edit/Checker.cs +++ b/osu.Game/Rulesets/Edit/Checker.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Edit { public abstract class Checker { - // These are all mode-invariant, hence here instead of in e.g. `OsuChecker`. + // These are all ruleset-invariant, hence here instead of in e.g. `OsuChecker`. private readonly List beatmapChecks = new List { new CheckMetadataVowels() From bc4f3351f38bdf5d7e5812db554ceceeaa6ec2cc Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:03:16 +0200 Subject: [PATCH 09/55] Replace checks with realistic ones --- .../Edit/Checks/CheckConsecutiveCircles.cs | 86 -------------- .../Edit/Checks/CheckOffscreenObjects.cs | 110 ++++++++++++++++++ osu.Game.Rulesets.Osu/Edit/OsuChecker.cs | 2 +- osu.Game/Checks/CheckMetadataVowels.cs | 65 ----------- osu.Game/Rulesets/Edit/Checker.cs | 2 +- .../Rulesets/Edit/Checks/CheckBackground.cs | 57 +++++++++ 6 files changed, 169 insertions(+), 153 deletions(-) delete mode 100644 osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs create mode 100644 osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs delete mode 100644 osu.Game/Checks/CheckMetadataVowels.cs create mode 100644 osu.Game/Rulesets/Edit/Checks/CheckBackground.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs deleted file mode 100644 index c41c0dac2b..0000000000 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckConsecutiveCircles.cs +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using System.Linq; -using osu.Game.Beatmaps; -using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Osu.Objects; -using osu.Game.Screens.Edit.Verify; -using osu.Game.Screens.Edit.Verify.Components; - -namespace osu.Game.Rulesets.Osu.Edit.Checks -{ - public class CheckConsecutiveCircles : BeatmapCheck - { - private const double consecutive_threshold = 3; - private const double delta_time_min_expected = 300; - private const double delta_time_min_threshold = 100; - - public override CheckMetadata Metadata() => new CheckMetadata - ( - category: CheckMetadata.CheckCategory.Spread, - description: "Too many or fast consecutive circles." - ); - - private IssueTemplate templateManyInARow = new IssueTemplate - ( - type: IssueTemplate.IssueType.Problem, - unformattedMessage: "There are {0} circles in a row here, expected at most {1}." - ); - - private IssueTemplate templateTooFast = new IssueTemplate - ( - type: IssueTemplate.IssueType.Warning, - unformattedMessage: "These circles are too fast ({0:0} ms), expected at least {1:0} ms." - ); - - private IssueTemplate templateAlmostTooFast = new IssueTemplate - ( - type: IssueTemplate.IssueType.Negligible, - unformattedMessage: "These circles are almost too fast ({0:0} ms), expected at least {1:0} ms." - ); - - public override IEnumerable Templates() => new[] - { - templateManyInARow, - templateTooFast, - templateAlmostTooFast - }; - - public override IEnumerable Run(IBeatmap beatmap) - { - List prevCircles = new List(); - - foreach (HitObject hitobject in beatmap.HitObjects) - { - if (!(hitobject is HitCircle circle) || hitobject == beatmap.HitObjects.Last()) - { - if (prevCircles.Count > consecutive_threshold) - { - yield return new Issue( - prevCircles, - templateManyInARow, - prevCircles.Count, consecutive_threshold - ); - } - - prevCircles.Clear(); - continue; - } - - double? prevDeltaTime = circle.StartTime - prevCircles.LastOrDefault()?.StartTime; - prevCircles.Add(circle); - - if (prevDeltaTime == null || prevDeltaTime >= delta_time_min_expected) - continue; - - yield return new Issue( - prevCircles.TakeLast(2), - prevDeltaTime < delta_time_min_threshold ? templateTooFast : templateAlmostTooFast, - prevDeltaTime, delta_time_min_expected - ); - } - } - } -} diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs new file mode 100644 index 0000000000..c47864855b --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -0,0 +1,110 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Osu.Objects; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Edit.Checks +{ + public class CheckOffscreenObjects : BeatmapCheck + { + // These are close approximates to the edges of the screen + // in gameplay on a 4:3 aspect ratio for osu!stable. + private const int min_x = -67; + private const int min_y = -60; + private const int max_x = 579; + private const int max_y = 428; + + // The amount of milliseconds to step through a slider path at a time + // (higher = more performant, but higher false-negative chance). + private const int path_step_size = 5; + + public override CheckMetadata Metadata() => new CheckMetadata + ( + category: CheckMetadata.CheckCategory.Compose, + description: "Offscreen hitobjects." + ); + + public override IEnumerable Templates() => new[] + { + templateOffscreen, + templateOffscreenSliderPath + }; + + private readonly IssueTemplate templateOffscreen = new IssueTemplate + ( + type: IssueTemplate.IssueType.Problem, + unformattedMessage: "This object goes offscreen on a 4:3 aspect ratio." + ); + + private readonly IssueTemplate templateOffscreenSliderPath = new IssueTemplate + ( + type: IssueTemplate.IssueType.Problem, + unformattedMessage: "This slider goes offscreen here on a 4:3 aspect ratio." + ); + + public override IEnumerable Run(IBeatmap beatmap) + { + foreach (var hitobject in beatmap.HitObjects) + { + switch (hitobject) + { + case Slider slider: + { + foreach (var issue in sliderIssues(slider)) + yield return issue; + + break; + } + + case HitCircle circle: + { + if (isOffscreen(circle.StackedPosition, circle.Radius)) + yield return new Issue(circle, templateOffscreen); + + break; + } + } + } + } + + /// + /// Steps through points on the slider to ensure the entire path is on-screen. + /// Returns at most one issue. + /// + /// The slider whose path to check. + /// + private IEnumerable sliderIssues(Slider slider) + { + for (int i = 0; i < slider.Distance; i += path_step_size) + { + double progress = i / slider.Distance; + Vector2 position = slider.StackedPositionAt(progress); + + if (!isOffscreen(position, slider.Radius)) + continue; + + // `SpanDuration` ensures we don't include reverses. + double time = slider.StartTime + progress * slider.SpanDuration; + yield return new Issue(slider, templateOffscreenSliderPath) { Time = time }; + + yield break; + } + + // Above loop may skip the last position in the slider due to step size. + if (!isOffscreen(slider.StackedEndPosition, slider.Radius)) + yield break; + + yield return new Issue(slider, templateOffscreenSliderPath) { Time = slider.EndTime }; + } + + private bool isOffscreen(Vector2 position, double radius) + { + return position.X - radius < min_x || position.X + radius > max_x || + position.Y - radius < min_y || position.Y + radius > max_y; + } + } +} diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs index df3847f2fe..ee3b230679 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs @@ -14,7 +14,7 @@ namespace osu.Game.Rulesets.Osu.Edit { public readonly List beatmapChecks = new List { - new CheckConsecutiveCircles() + new CheckOffscreenObjects() }; public override IEnumerable Run(IBeatmap beatmap) diff --git a/osu.Game/Checks/CheckMetadataVowels.cs b/osu.Game/Checks/CheckMetadataVowels.cs deleted file mode 100644 index 8bcfe89c3a..0000000000 --- a/osu.Game/Checks/CheckMetadataVowels.cs +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using System.Linq; -using osu.Game.Beatmaps; -using osu.Game.Screens.Edit.Verify; -using osu.Game.Screens.Edit.Verify.Components; - -namespace osu.Game.Checks -{ - public class CheckMetadataVowels : BeatmapCheck - { - private static readonly char[] vowels = { 'a', 'e', 'i', 'o', 'u' }; - - public override CheckMetadata Metadata() => new CheckMetadata - ( - category: CheckMetadata.CheckCategory.Metadata, - description: "Metadata fields contain vowels" - ); - - public override IEnumerable Templates() => new[] - { - templateArtistHasVowels - }; - - private IssueTemplate templateArtistHasVowels = new IssueTemplate - ( - type: IssueTemplate.IssueType.Warning, - unformattedMessage: "The {0} field \"{1}\" contains the vowel(s) {2}." - ); - - public override IEnumerable Run(IBeatmap beatmap) - { - foreach (var issue in GetVowelIssues("artist", beatmap.Metadata.Artist)) - yield return issue; - - foreach (var issue in GetVowelIssues("unicode artist", beatmap.Metadata.ArtistUnicode)) - yield return issue; - - foreach (var issue in GetVowelIssues("title", beatmap.Metadata.Title)) - yield return issue; - - foreach (var issue in GetVowelIssues("unicode title", beatmap.Metadata.TitleUnicode)) - yield return issue; - } - - private IEnumerable GetVowelIssues(string fieldName, string fieldValue) - { - if (fieldValue == null) - // Unicode fields can be null if same as respective romanized fields. - yield break; - - List matches = vowels.Where(c => fieldValue.ToLower().Contains(c)).ToList(); - - if (!matches.Any()) - yield break; - - yield return new Issue( - templateArtistHasVowels, - fieldName, fieldValue, string.Join(", ", matches) - ); - } - } -} diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/Checker.cs index 6687160b10..65d7fc5913 100644 --- a/osu.Game/Rulesets/Edit/Checker.cs +++ b/osu.Game/Rulesets/Edit/Checker.cs @@ -14,7 +14,7 @@ namespace osu.Game.Rulesets.Edit // These are all ruleset-invariant, hence here instead of in e.g. `OsuChecker`. private readonly List beatmapChecks = new List { - new CheckMetadataVowels() + new CheckBackground() }; public virtual IEnumerable Run(IBeatmap beatmap) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs new file mode 100644 index 0000000000..9376f9568a --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -0,0 +1,57 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks.Components; + +namespace osu.Game.Rulesets.Edit.Checks +{ + public class CheckBackground : BeatmapCheck + { + public override CheckMetadata Metadata() => new CheckMetadata + ( + category: CheckMetadata.CheckCategory.Resources, + description: "Missing background." + ); + + public override IEnumerable Templates() => new[] + { + templateNoneSet, + templateDoesNotExist + }; + + private readonly IssueTemplate templateNoneSet = new IssueTemplate + ( + type: IssueTemplate.IssueType.Problem, + unformattedMessage: "No background has been set." + ); + + private readonly IssueTemplate templateDoesNotExist = new IssueTemplate + ( + type: IssueTemplate.IssueType.Problem, + unformattedMessage: "The background file \"{0}\" is does not exist." + ); + + public override IEnumerable Run(IBeatmap beatmap) + { + if (beatmap.Metadata.BackgroundFile == null) + { + yield return new Issue(templateNoneSet); + + yield break; + } + + // If the background is set, also make sure it still exists. + + var set = beatmap.BeatmapInfo.BeatmapSet; + var file = set.Files.FirstOrDefault(f => f.Filename == beatmap.Metadata.BackgroundFile); + + if (file != null) + yield break; + + yield return new Issue(templateDoesNotExist, beatmap.Metadata.BackgroundFile); + } + } +} From 6d3cf78e4a4e50bd40c2b5f667d7e805e5bb93e5 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:04:39 +0200 Subject: [PATCH 10/55] Add issue selection This mainly helps with keeping track of which issue was clicked, since doing so switches tab. --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 42 ++++++++++++++++++-- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 7 ++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 20dceb5333..458b0184b6 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -26,6 +27,9 @@ namespace osu.Game.Screens.Edit.Verify private readonly FillFlowContainer backgroundFlow; + [Resolved] + private Bindable selectedIssue { get; set; } + public IssueTable() { RelativeSizeAxes = Axes.X; @@ -115,6 +119,7 @@ namespace osu.Game.Screens.Edit.Verify public class RowBackground : OsuClickableContainer { + private readonly Issue issue; private const int fade_duration = 100; private readonly Box hoveredBackground; @@ -128,13 +133,16 @@ namespace osu.Game.Screens.Edit.Verify [Resolved] private EditorBeatmap editorBeatmap { get; set; } + [Resolved] + private Bindable selectedIssue { get; set; } + public RowBackground(Issue issue) { + this.issue = issue; + RelativeSizeAxes = Axes.X; Height = row_height; - AlwaysPresent = true; - CornerRadius = 3; Masking = true; @@ -152,6 +160,8 @@ namespace osu.Game.Screens.Edit.Verify // Supposed to work like clicking timestamps outside of the game. // TODO: Is there already defined behaviour for this I may be able to call? + selectedIssue.Value = issue; + if (issue.Time != null) { clock.Seek(issue.Time.Value); @@ -167,11 +177,35 @@ namespace osu.Game.Screens.Edit.Verify } private Color4 colourHover; + private Color4 colourSelected; [BackgroundDependencyLoader] private void load(OsuColour colours) { hoveredBackground.Colour = colourHover = colours.BlueDarker; + colourSelected = colours.YellowDarker; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + selectedIssue.BindValueChanged(change => { Selected = issue == change.NewValue; }, true); + } + + private bool selected; + + protected bool Selected + { + get => selected; + set + { + if (value == selected) + return; + + selected = value; + updateState(); + } } protected override bool OnHover(HoverEvent e) @@ -188,9 +222,9 @@ namespace osu.Game.Screens.Edit.Verify private void updateState() { - hoveredBackground.FadeColour(colourHover, 450, Easing.OutQuint); + hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); - if (IsHovered) + if (selected || IsHovered) hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); else hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index 88397fdbff..ea9f986eb6 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -13,6 +13,7 @@ using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks.Components; using osuTK; namespace osu.Game.Screens.Edit.Verify @@ -22,6 +23,9 @@ namespace osu.Game.Screens.Edit.Verify private Ruleset ruleset; private static Checker checker; // TODO: Should not be static, but apparently needs to be? + [Cached] + private Bindable selectedIssue = new Bindable(); + public VerifyScreen() : base(EditorScreenMode.Verify) { @@ -74,6 +78,9 @@ namespace osu.Game.Screens.Edit.Verify [Resolved] protected EditorBeatmap Beatmap { get; private set; } + [Resolved] + private Bindable selectedIssue { get; set; } + [BackgroundDependencyLoader] private void load(OsuColour colours) { From c995eca029f5f126e734507018b62fb3ea717bb5 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:05:24 +0200 Subject: [PATCH 11/55] Remove todo Doesn't really matter in the end, as only one checker will run at a time in this case. --- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index ea9f986eb6..b9fd93ac19 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -21,7 +21,7 @@ namespace osu.Game.Screens.Edit.Verify public class VerifyScreen : EditorScreen { private Ruleset ruleset; - private static Checker checker; // TODO: Should not be static, but apparently needs to be? + private static Checker checker; [Cached] private Bindable selectedIssue = new Bindable(); From 3a4f2e3d7e393d29d1f0f4ad8dcbf989f4f19073 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:09:16 +0200 Subject: [PATCH 12/55] Show table even if no issues --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 458b0184b6..e4612927fd 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -57,7 +57,7 @@ namespace osu.Game.Screens.Edit.Verify Content = null; backgroundFlow.Clear(); - if (value?.Any() != true) + if (value == null) return; foreach (var issue in value) From 747e0f00dc63e060feb246f69a5ffe6c33249604 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 13:10:05 +0200 Subject: [PATCH 13/55] Improve table formatting --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index e4612927fd..40bd134551 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -74,9 +74,9 @@ namespace osu.Game.Screens.Edit.Verify { var columns = new List { - new TableColumn(string.Empty, Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), - new TableColumn("Type", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), - new TableColumn("Time", Anchor.Centre, new Dimension(GridSizeMode.AutoSize)), + new TableColumn(string.Empty, Anchor.CentreLeft, new Dimension(GridSizeMode.AutoSize)), + new TableColumn("Type", Anchor.CentreLeft, new Dimension(GridSizeMode.AutoSize, minSize: 60)), + new TableColumn("Time", Anchor.CentreLeft, new Dimension(GridSizeMode.AutoSize, minSize: 60)), new TableColumn("Message", Anchor.CentreLeft), new TableColumn("Category", Anchor.CentreRight, new Dimension(GridSizeMode.AutoSize)), }; @@ -89,20 +89,21 @@ namespace osu.Game.Screens.Edit.Verify new OsuSpriteText { Text = $"#{index + 1}", - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium) + Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium), + Margin = new MarginPadding { Right = 10 } }, new OsuSpriteText { Text = issue.Template.Type.ToString(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), - Margin = new MarginPadding { Left = 10 }, + Margin = new MarginPadding { Right = 10 }, Colour = issue.Template.TypeColour() }, new OsuSpriteText { Text = issue.GetEditorTimestamp(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), - Margin = new MarginPadding(10) + Margin = new MarginPadding { Right = 10 }, }, new OsuSpriteText { From 3289bb03791a11f0181d86a4c21ce13d1e214c2b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 14:56:30 +0200 Subject: [PATCH 14/55] Merge `Check` and `BeatmapCheck` We're probably not going to need GeneralChecks or BeatmapsetChecks. The verify tab is only available to a single difficulty at a time, and we already have access to the rest of the set through `IBeatmap`. --- .../Edit/Checks/CheckOffscreenObjects.cs | 2 +- osu.Game.Rulesets.Osu/Edit/OsuChecker.cs | 2 +- osu.Game/Rulesets/Edit/Checker.cs | 4 ++-- .../Rulesets/Edit/Checks/CheckBackground.cs | 2 +- .../Edit/Checks/Components/BeatmapCheck.cs | 19 ------------------ .../Rulesets/Edit/Checks/Components/Check.cs | 20 +++++++++---------- 6 files changed, 14 insertions(+), 35 deletions(-) delete mode 100644 osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index c47864855b..3d411d6e12 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 : BeatmapCheck + public class CheckOffscreenObjects : Check { // These are close approximates to the edges of the screen // in gameplay on a 4:3 aspect ratio for osu!stable. diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs index ee3b230679..d0dbc043d4 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Osu.Edit { public class OsuChecker : Checker { - public readonly List beatmapChecks = new List + public readonly List beatmapChecks = new List { new CheckOffscreenObjects() }; diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/Checker.cs index 65d7fc5913..6ab6ed75e8 100644 --- a/osu.Game/Rulesets/Edit/Checker.cs +++ b/osu.Game/Rulesets/Edit/Checker.cs @@ -12,14 +12,14 @@ namespace osu.Game.Rulesets.Edit public abstract class Checker { // These are all ruleset-invariant, hence here instead of in e.g. `OsuChecker`. - private readonly List beatmapChecks = new List + private readonly IReadOnlyList checks = new List { new CheckBackground() }; public virtual IEnumerable Run(IBeatmap beatmap) { - return beatmapChecks.SelectMany(check => check.Run(beatmap)); + return checks.SelectMany(check => check.Run(beatmap)); } } } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 9376f9568a..aa10fad77b 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -8,7 +8,7 @@ using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit.Checks { - public class CheckBackground : BeatmapCheck + public class CheckBackground : Check { public override CheckMetadata Metadata() => new CheckMetadata ( diff --git a/osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs b/osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs deleted file mode 100644 index d0f93b5eef..0000000000 --- a/osu.Game/Rulesets/Edit/Checks/Components/BeatmapCheck.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using osu.Game.Beatmaps; - -namespace osu.Game.Rulesets.Edit.Checks.Components -{ - public abstract class BeatmapCheck : Check - { - /// - /// Returns zero, one, or several issues detected by this - /// check on the given beatmap. - /// - /// The beatmap to run the check on. - /// - public abstract override IEnumerable Run(IBeatmap beatmap); - } -} diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 228c11f0f3..4e444cacbf 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using osu.Game.Beatmaps; namespace osu.Game.Rulesets.Edit.Checks.Components { @@ -21,21 +22,18 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// public abstract IEnumerable Templates(); + /// + /// Returns zero, one, or several issues detected by this + /// check on the given beatmap. + /// + /// The beatmap to run the check on. + /// + public abstract IEnumerable Run(IBeatmap beatmap); + protected Check() { foreach (var template in Templates()) template.Origin = this; } } - - public abstract class Check : Check - { - /// - /// Returns zero, one, or several issues detected by - /// this check on the given object. - /// - /// The object to run the check on. - /// - public abstract IEnumerable Run(T obj); - } } From 7d40b017223a008e17cf53cf0b05006e8dfe288f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 15:18:15 +0200 Subject: [PATCH 15/55] Remove old todo --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 40bd134551..5f04c7c4e8 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -158,9 +158,6 @@ namespace osu.Game.Screens.Edit.Verify Action = () => { - // Supposed to work like clicking timestamps outside of the game. - // TODO: Is there already defined behaviour for this I may be able to call? - selectedIssue.Value = issue; if (issue.Time != null) From dac733cced62e30089f52fa6147f5210197b3e20 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sat, 10 Apr 2021 15:49:57 +0200 Subject: [PATCH 16/55] Fix field name and accessibility --- osu.Game.Rulesets.Osu/Edit/OsuChecker.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs index d0dbc043d4..10f0ecf0cf 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Osu.Edit { public class OsuChecker : Checker { - public readonly List beatmapChecks = new List + private readonly List checks = new List { new CheckOffscreenObjects() }; @@ -23,7 +23,7 @@ namespace osu.Game.Rulesets.Osu.Edit foreach (var issue in base.Run(beatmap)) yield return issue; - foreach (var issue in beatmapChecks.SelectMany(check => check.Run(beatmap))) + foreach (var issue in checks.SelectMany(check => check.Run(beatmap))) yield return issue; } } From 1c553b5d481d21186eed0498160f90a6e51eedee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:14:53 +0900 Subject: [PATCH 17/55] Checker -> BeatmapVerifier --- .../Edit/{OsuChecker.cs => OsuBeatmapVerifier.cs} | 2 +- osu.Game.Rulesets.Osu/OsuRuleset.cs | 2 +- .../Rulesets/Edit/{Checker.cs => BeatmapVerifier.cs} | 2 +- osu.Game/Rulesets/Ruleset.cs | 2 +- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 10 +++++----- 5 files changed, 9 insertions(+), 9 deletions(-) rename osu.Game.Rulesets.Osu/Edit/{OsuChecker.cs => OsuBeatmapVerifier.cs} (94%) rename osu.Game/Rulesets/Edit/{Checker.cs => BeatmapVerifier.cs} (94%) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs similarity index 94% rename from osu.Game.Rulesets.Osu/Edit/OsuChecker.cs rename to osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 10f0ecf0cf..272612dbaf 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuChecker.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -10,7 +10,7 @@ using osu.Game.Rulesets.Osu.Edit.Checks; namespace osu.Game.Rulesets.Osu.Edit { - public class OsuChecker : Checker + public class OsuBeatmapVerifier : BeatmapVerifier { private readonly List checks = new List { diff --git a/osu.Game.Rulesets.Osu/OsuRuleset.cs b/osu.Game.Rulesets.Osu/OsuRuleset.cs index 74679bd578..1658846e98 100644 --- a/osu.Game.Rulesets.Osu/OsuRuleset.cs +++ b/osu.Game.Rulesets.Osu/OsuRuleset.cs @@ -206,7 +206,7 @@ namespace osu.Game.Rulesets.Osu public override HitObjectComposer CreateHitObjectComposer() => new OsuHitObjectComposer(this); - public override Checker CreateChecker() => new OsuChecker(); + public override BeatmapVerifier CreateChecker() => new OsuBeatmapVerifier(); public override string Description => "osu!"; diff --git a/osu.Game/Rulesets/Edit/Checker.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs similarity index 94% rename from osu.Game/Rulesets/Edit/Checker.cs rename to osu.Game/Rulesets/Edit/BeatmapVerifier.cs index 6ab6ed75e8..230b128cc1 100644 --- a/osu.Game/Rulesets/Edit/Checker.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -9,7 +9,7 @@ using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit { - public abstract class Checker + public abstract class BeatmapVerifier { // These are all ruleset-invariant, hence here instead of in e.g. `OsuChecker`. private readonly IReadOnlyList checks = new List diff --git a/osu.Game/Rulesets/Ruleset.cs b/osu.Game/Rulesets/Ruleset.cs index 71f80c9982..088bcc7712 100644 --- a/osu.Game/Rulesets/Ruleset.cs +++ b/osu.Game/Rulesets/Ruleset.cs @@ -202,7 +202,7 @@ namespace osu.Game.Rulesets public virtual HitObjectComposer CreateHitObjectComposer() => null; - public virtual Checker CreateChecker() => null; + public virtual BeatmapVerifier CreateChecker() => null; public virtual Drawable CreateIcon() => new SpriteIcon { Icon = FontAwesome.Solid.QuestionCircle }; diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index b9fd93ac19..7a520826f5 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -21,7 +21,7 @@ namespace osu.Game.Screens.Edit.Verify public class VerifyScreen : EditorScreen { private Ruleset ruleset; - private static Checker checker; + private static BeatmapVerifier beatmapVerifier; [Cached] private Bindable selectedIssue = new Bindable(); @@ -36,7 +36,7 @@ namespace osu.Game.Screens.Edit.Verify var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); ruleset = parent.Get>().Value.BeatmapInfo.Ruleset?.CreateInstance(); - checker = ruleset?.CreateChecker(); + beatmapVerifier = ruleset?.CreateChecker(); return dependencies; } @@ -128,9 +128,9 @@ namespace osu.Game.Screens.Edit.Verify private void refresh() { - table.Issues = checker.Run(Beatmap) - .OrderByDescending(issue => issue.Template.Type) - .ThenByDescending(issue => issue.Template.Origin.Metadata().Category); + table.Issues = beatmapVerifier.Run(Beatmap) + .OrderByDescending(issue => issue.Template.Type) + .ThenByDescending(issue => issue.Template.Origin.Metadata().Category); } } } From 136627b9ac59697129ccd1ae94a67e50442d3d61 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:27:12 +0900 Subject: [PATCH 18/55] Wrap xmldoc less and make a few fixes --- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 9 ++++++--- osu.Game/Rulesets/Edit/Checks/Components/Check.cs | 12 +++--------- .../Rulesets/Edit/Checks/Components/CheckMetadata.cs | 6 ++---- osu.Game/Rulesets/Edit/Checks/Components/Issue.cs | 10 +++------- .../Rulesets/Edit/Checks/Components/IssueTemplate.cs | 5 +---- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index 230b128cc1..f67a068525 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -11,15 +11,18 @@ namespace osu.Game.Rulesets.Edit { public abstract class BeatmapVerifier { - // These are all ruleset-invariant, hence here instead of in e.g. `OsuChecker`. - private readonly IReadOnlyList checks = new List + /// + /// 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 { new CheckBackground() }; public virtual IEnumerable Run(IBeatmap beatmap) { - return checks.SelectMany(check => check.Run(beatmap)); + return generalChecks.SelectMany(check => check.Run(beatmap)); } } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 4e444cacbf..5a922fde56 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -9,25 +9,19 @@ namespace osu.Game.Rulesets.Edit.Checks.Components public abstract class Check { /// - /// Returns the for this check. - /// Basically, its information. + /// The metadata for this check. /// - /// public abstract CheckMetadata Metadata(); /// - /// The templates for issues that this check may use. - /// Basically, what issues this check can detect. + /// All possible templates for issues that this check may return. /// - /// public abstract IEnumerable Templates(); /// - /// Returns zero, one, or several issues detected by this - /// check on the given beatmap. + /// 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() diff --git a/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs index 5a226729ad..38a9a16325 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs @@ -42,14 +42,12 @@ namespace osu.Game.Rulesets.Edit.Checks.Components } /// - /// The category this check belongs to. E.g. , - /// , or . + /// The category this check belongs to. E.g. , , or . /// public readonly CheckCategory Category; /// - /// Describes the issue(s) that this check looks for. Keep this brief, such that - /// it fits into "No {description}". E.g. "Offscreen objects" / "Too short sliders". + /// Describes the issue(s) that this check looks for. Keep this brief, such that it fits into "No {description}". E.g. "Offscreen objects" / "Too short sliders". /// public readonly string Description; diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index 0f835202e7..6f6ba2a323 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -22,17 +22,13 @@ namespace osu.Game.Rulesets.Edit.Checks.Components public IReadOnlyList HitObjects; /// - /// The template which this issue is using. This provides properties - /// such as the , and the - /// . + /// The template which this issue is using. This provides properties such as the , and the . /// public IssueTemplate Template; /// - /// The arguments that give this issue its context, based on the - /// . These are then substituted into the - /// . - /// E.g. timestamps, which diff is being compared to, what some volume is, etc. + /// 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; diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 7eaabdc59b..69c421140b 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -35,8 +35,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components public Check Origin; /// - /// The type of the issue. E.g. , - /// , or . + /// The type of the issue. /// public readonly IssueType Type; @@ -57,7 +56,6 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// Returns the formatted message given the arguments used to format it. /// /// The arguments used to format the message. - /// public string Message(params object[] args) => UnformattedMessage.FormatWith(args); public static readonly Color4 PROBLEM_RED = new Colour4(1.0f, 0.4f, 0.4f, 1.0f); @@ -68,7 +66,6 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// Returns the colour corresponding to the type of this issue. /// - /// public Colour4 TypeColour() { return Type switch From 257acf9cd88f3826e6da336fe66e71d3a7834c60 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:28:13 +0900 Subject: [PATCH 19/55] Colour constants to private --- .../Edit/Checks/Components/IssueTemplate.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 69c421140b..5381d54600 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -9,6 +9,11 @@ namespace osu.Game.Rulesets.Edit.Checks.Components { public class IssueTemplate { + private static readonly Color4 problem_red = new Colour4(1.0f, 0.4f, 0.4f, 1.0f); + private static readonly Color4 warning_yellow = new Colour4(1.0f, 0.8f, 0.2f, 1.0f); + 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 type, or severity, of an issue. This decides its priority. /// @@ -58,11 +63,6 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// The arguments used to format the message. public string Message(params object[] args) => UnformattedMessage.FormatWith(args); - public static readonly Color4 PROBLEM_RED = new Colour4(1.0f, 0.4f, 0.4f, 1.0f); - public static readonly Color4 WARNING_YELLOW = new Colour4(1.0f, 0.8f, 0.2f, 1.0f); - public static readonly Color4 NEGLIGIBLE_GREEN = new Colour4(0.33f, 0.8f, 0.5f, 1.0f); - public static readonly Color4 ERROR_GRAY = new Colour4(0.5f, 0.5f, 0.5f, 1.0f); - /// /// Returns the colour corresponding to the type of this issue. /// @@ -70,10 +70,10 @@ namespace osu.Game.Rulesets.Edit.Checks.Components { return Type switch { - IssueType.Problem => PROBLEM_RED, - IssueType.Warning => WARNING_YELLOW, - IssueType.Negligible => NEGLIGIBLE_GREEN, - IssueType.Error => ERROR_GRAY, + IssueType.Problem => problem_red, + IssueType.Warning => warning_yellow, + IssueType.Negligible => negligible_green, + IssueType.Error => error_gray, _ => Color4.White }; } From 3551322f1d95738e09c9e5f9c847abf0163d1d3b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:30:33 +0900 Subject: [PATCH 20/55] Fix formatting of colour getter --- .../Edit/Checks/Components/IssueTemplate.cs | 28 +++++++++++++------ osu.Game/Screens/Edit/Verify/IssueTable.cs | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 5381d54600..6fe8b8de87 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -66,16 +66,28 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// Returns the colour corresponding to the type of this issue. /// - public Colour4 TypeColour() + public Colour4 Colour { - return Type switch + get { - IssueType.Problem => problem_red, - IssueType.Warning => warning_yellow, - IssueType.Negligible => negligible_green, - IssueType.Error => error_gray, - _ => Color4.White - }; + switch (Type) + { + case IssueType.Problem: + return problem_red; + + case IssueType.Warning: + return warning_yellow; + + case IssueType.Negligible: + return negligible_green; + + case IssueType.Error: + return error_gray; + + default: + return Color4.White; + } + } } } } diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 5f04c7c4e8..1f275ca537 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -97,7 +97,7 @@ namespace osu.Game.Screens.Edit.Verify Text = issue.Template.Type.ToString(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), Margin = new MarginPadding { Right = 10 }, - Colour = issue.Template.TypeColour() + Colour = issue.Template.Colour }, new OsuSpriteText { From f78239c7f238bd64a6272de1c3b5648511f8c581 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:32:52 +0900 Subject: [PATCH 21/55] Move enums out of nesting --- .../Edit/Checks/CheckOffscreenObjects.cs | 6 +-- .../Rulesets/Edit/Checks/CheckBackground.cs | 6 +-- .../Edit/Checks/Components/CheckCategory.cs | 41 +++++++++++++++++++ .../Edit/Checks/Components/CheckMetadata.cs | 36 ---------------- .../Rulesets/Edit/Checks/Components/Issue.cs | 2 +- .../Edit/Checks/Components/IssueTemplate.cs | 20 --------- .../Edit/Checks/Components/IssueType.cs | 25 +++++++++++ 7 files changed, 73 insertions(+), 63 deletions(-) create mode 100644 osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs create mode 100644 osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index 3d411d6e12..910de76c5f 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -24,7 +24,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks public override CheckMetadata Metadata() => new CheckMetadata ( - category: CheckMetadata.CheckCategory.Compose, + category: CheckCategory.Compose, description: "Offscreen hitobjects." ); @@ -36,13 +36,13 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks private readonly IssueTemplate templateOffscreen = new IssueTemplate ( - type: IssueTemplate.IssueType.Problem, + type: IssueType.Problem, unformattedMessage: "This object goes offscreen on a 4:3 aspect ratio." ); private readonly IssueTemplate templateOffscreenSliderPath = new IssueTemplate ( - type: IssueTemplate.IssueType.Problem, + type: IssueType.Problem, unformattedMessage: "This slider goes offscreen here on a 4:3 aspect ratio." ); diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index aa10fad77b..2de9d9daae 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Edit.Checks { public override CheckMetadata Metadata() => new CheckMetadata ( - category: CheckMetadata.CheckCategory.Resources, + category: CheckCategory.Resources, description: "Missing background." ); @@ -24,13 +24,13 @@ namespace osu.Game.Rulesets.Edit.Checks private readonly IssueTemplate templateNoneSet = new IssueTemplate ( - type: IssueTemplate.IssueType.Problem, + type: IssueType.Problem, unformattedMessage: "No background has been set." ); private readonly IssueTemplate templateDoesNotExist = new IssueTemplate ( - type: IssueTemplate.IssueType.Problem, + type: IssueType.Problem, unformattedMessage: "The background file \"{0}\" is does not exist." ); diff --git a/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs b/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs new file mode 100644 index 0000000000..c37a580dd8 --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs @@ -0,0 +1,41 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Rulesets.Edit.Checks.Components +{ + /// + /// The category of an issue. + /// + public enum CheckCategory + { + /// Anything to do with control points. + Timing, + + /// Anything to do with artist, title, creator, etc. + Metadata, + + /// Anything to do with non-audio files, e.g. background, skin, sprites, and video. + Resources, + + /// Anything to do with audio files, e.g. song and hitsounds. + Audio, + + /// Anything to do with files that don't fit into the above, e.g. unused, osu, or osb. + Files, + + /// Anything to do with hitobjects unrelated to spread. + Compose, + + /// Anything to do with difficulty levels or their progression. + Spread, + + /// Anything to do with variables like CS, OD, AR, HP, and global SV. + Settings, + + /// Anything to do with hitobject feedback. + Hitsounds, + + /// Anything to do with storyboarding, breaks, video offset, etc. + Events + } +} diff --git a/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs index 38a9a16325..cebb2f5455 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/CheckMetadata.cs @@ -5,42 +5,6 @@ namespace osu.Game.Rulesets.Edit.Checks.Components { public class CheckMetadata { - /// - /// The category of an issue. - /// - public enum CheckCategory - { - /// Anything to do with control points. - Timing, - - /// Anything to do with artist, title, creator, etc. - Metadata, - - /// Anything to do with non-audio files, e.g. background, skin, sprites, and video. - Resources, - - /// Anything to do with audio files, e.g. song and hitsounds. - Audio, - - /// Anything to do with files that don't fit into the above, e.g. unused, osu, or osb. - Files, - - /// Anything to do with hitobjects unrelated to spread. - Compose, - - /// Anything to do with difficulty levels or their progression. - Spread, - - /// Anything to do with variables like CS, OD, AR, HP, and global SV. - Settings, - - /// Anything to do with hitobject feedback. - Hitsounds, - - /// Anything to do with storyboarding, breaks, video offset, etc. - Events - } - /// /// The category this check belongs to. E.g. , , or . /// diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index 6f6ba2a323..f392fa8ef4 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -22,7 +22,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components public IReadOnlyList HitObjects; /// - /// The template which this issue is using. This provides properties such as the , and the . + /// The template which this issue is using. This provides properties such as the , and the . /// public IssueTemplate Template; diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 6fe8b8de87..976341e41c 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -14,26 +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 type, or severity, of an issue. This decides its priority. - /// - public enum IssueType - { - /// A must-fix in the vast majority of cases. - Problem = 3, - - /// A possible mistake. Often requires critical thinking. - Warning = 2, - - // TODO: Try/catch all checks run and return error templates if exceptions occur. - /// An error occurred and a complete check could not be made. - Error = 1, - - // TODO: Negligible issues should be hidden by default. - /// A possible mistake so minor/unlikely that it can often be safely ignored. - Negligible = 0, - } - /// /// The check that this template originates from. /// diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs new file mode 100644 index 0000000000..be43060cfc --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs @@ -0,0 +1,25 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Rulesets.Edit.Checks.Components +{ + /// + /// The type, or severity, of an issue. This decides its priority. + /// + public enum IssueType + { + /// A must-fix in the vast majority of cases. + Problem = 3, + + /// A possible mistake. Often requires critical thinking. + Warning = 2, + + // TODO: Try/catch all checks run and return error templates if exceptions occur. + /// An error occurred and a complete check could not be made. + Error = 1, + + // TODO: Negligible issues should be hidden by default. + /// A possible mistake so minor/unlikely that it can often be safely ignored. + Negligible = 0, + } +} From 8c31e96cdfd657c537dce50a722438fdad7b63dc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:37:41 +0900 Subject: [PATCH 22/55] Change some methods to get properties --- osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs | 2 +- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 2 +- osu.Game/Rulesets/Edit/Checks/Components/Check.cs | 4 ++-- osu.Game/Rulesets/Edit/Checks/Components/Issue.cs | 5 +---- osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs | 2 +- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index 910de76c5f..b2a00da12a 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks description: "Offscreen hitobjects." ); - public override IEnumerable Templates() => new[] + public override IEnumerable PossibleTemplates => new[] { templateOffscreen, templateOffscreenSliderPath diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 2de9d9daae..89b2e8293c 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Edit.Checks description: "Missing background." ); - public override IEnumerable Templates() => new[] + public override IEnumerable PossibleTemplates => new[] { templateNoneSet, templateDoesNotExist diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 5a922fde56..550d2ea0a2 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// All possible templates for issues that this check may return. /// - public abstract IEnumerable Templates(); + public abstract IEnumerable PossibleTemplates { get; } /// /// Runs this check and returns any issues detected for the provided beatmap. @@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components protected Check() { - foreach (var template in Templates()) + foreach (var template in PossibleTemplates) template.Origin = this; } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index f392fa8ef4..0d5c5411fd 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -69,10 +69,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components HitObjects = hitObjectList; } - public override string ToString() - { - return Template.Message(Arguments); - } + public override string ToString() => Template.GetMessage(Arguments); public string GetEditorTimestamp() { diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 976341e41c..a1156c7c72 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// Returns the formatted message given the arguments used to format it. /// /// The arguments used to format the message. - public string Message(params object[] args) => UnformattedMessage.FormatWith(args); + public string GetMessage(params object[] args) => UnformattedMessage.FormatWith(args); /// /// Returns the colour corresponding to the type of this issue. From 78bbc8f5c824ffd35a8dab92ca0082c4e7244d7c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:47:32 +0900 Subject: [PATCH 23/55] Tidy some remaining code --- .../Edit/Checks/Components/CheckCategory.cs | 42 ++++++++++++++----- .../Rulesets/Edit/Checks/Components/Issue.cs | 6 +-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs b/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs index c37a580dd8..ae943cfda9 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/CheckCategory.cs @@ -8,34 +8,54 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// public enum CheckCategory { - /// Anything to do with control points. + /// + /// Anything to do with control points. + /// Timing, - /// Anything to do with artist, title, creator, etc. + /// + /// Anything to do with artist, title, creator, etc. + /// Metadata, - /// Anything to do with non-audio files, e.g. background, skin, sprites, and video. + /// + /// Anything to do with non-audio files, e.g. background, skin, sprites, and video. + /// Resources, - /// Anything to do with audio files, e.g. song and hitsounds. + /// + /// Anything to do with audio files, e.g. song and hitsounds. + /// Audio, - /// Anything to do with files that don't fit into the above, e.g. unused, osu, or osb. + /// + /// Anything to do with files that don't fit into the above, e.g. unused, osu, or osb. + /// Files, - /// Anything to do with hitobjects unrelated to spread. + /// + /// Anything to do with hitobjects unrelated to spread. + /// Compose, - /// Anything to do with difficulty levels or their progression. + /// + /// Anything to do with difficulty levels or their progression. + /// Spread, - /// Anything to do with variables like CS, OD, AR, HP, and global SV. + /// + /// Anything to do with variables like CS, OD, AR, HP, and global SV. + /// Settings, - /// Anything to do with hitobject feedback. - Hitsounds, + /// + /// Anything to do with hitobject feedback. + /// + HitObjects, - /// Anything to do with storyboarding, breaks, video offset, etc. + /// + /// Anything to do with storyboarding, breaks, video offset, etc. + /// Events } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index 0d5c5411fd..7241fabf5b 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -40,11 +40,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components Arguments = args; if (template.Origin == null) - { - throw new ArgumentException( - "A template had no origin. Make sure the `Templates()` method contains all templates used." - ); - } + 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) From 8bf85d737c75a71ba9e388842770cfc987f2bed7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 15:52:29 +0900 Subject: [PATCH 24/55] Change Metadata into a get property --- osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs | 2 +- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 2 +- osu.Game/Rulesets/Edit/Checks/Components/Check.cs | 2 +- osu.Game/Screens/Edit/Verify/IssueTable.cs | 2 +- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index b2a00da12a..252f65ab5a 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -22,7 +22,7 @@ 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() => new CheckMetadata + public override CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Compose, description: "Offscreen hitobjects." diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 89b2e8293c..22f98b6fd7 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -10,7 +10,7 @@ namespace osu.Game.Rulesets.Edit.Checks { public class CheckBackground : Check { - public override CheckMetadata Metadata() => new CheckMetadata + public override CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Resources, description: "Missing background." diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs index 550d2ea0a2..7c039d1572 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Check.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// The metadata for this check. /// - public abstract CheckMetadata Metadata(); + public abstract CheckMetadata Metadata { get; } /// /// All possible templates for issues that this check may return. diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 1f275ca537..84dbabf300 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -112,7 +112,7 @@ namespace osu.Game.Screens.Edit.Verify }, new OsuSpriteText { - Text = issue.Template.Origin.Metadata().Category.ToString(), + Text = issue.Template.Origin.Metadata.Category.ToString(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), Margin = new MarginPadding(10) } diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index 7a520826f5..0c4aa04ff3 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.Template.Origin.Metadata.Category); } } } From 42604afcdcf9ed0ae922b07434b4587ad4b4c85d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 16:15:27 +0900 Subject: [PATCH 25/55] Add binding for verify mode (and move enum entry to end) --- osu.Game/Input/Bindings/GlobalActionContainer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index 3ea7f5f5fa..36e465720a 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -70,6 +70,7 @@ namespace osu.Game.Input.Bindings new KeyBinding(new[] { InputKey.F2 }, GlobalAction.EditorDesignMode), new KeyBinding(new[] { InputKey.F3 }, GlobalAction.EditorTimingMode), new KeyBinding(new[] { InputKey.F4 }, GlobalAction.EditorSetupMode), + new KeyBinding(new[] { InputKey.Control, InputKey.Shift, InputKey.A }, GlobalAction.EditorVerifyMode), }; public IEnumerable InGameKeyBindings => new[] @@ -225,9 +226,6 @@ namespace osu.Game.Input.Bindings [Description("Timing mode")] EditorTimingMode, - [Description("Verify mode")] - EditorVerifyMode, - [Description("Hold for HUD")] HoldForHUD, @@ -252,5 +250,8 @@ namespace osu.Game.Input.Bindings [Description("Beatmap Options")] ToggleBeatmapOptions, + + [Description("Verify mode")] + EditorVerifyMode, } } 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 26/55] 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); } } } From a2fc9c398fc731e36c3472bc37a5c153af7d5378 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 10:08:30 +0200 Subject: [PATCH 27/55] Rename `CreateChecker` -> `CreateBeatmapVerifier` --- osu.Game.Rulesets.Osu/OsuRuleset.cs | 2 +- osu.Game/Rulesets/Ruleset.cs | 2 +- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/OsuRuleset.cs b/osu.Game.Rulesets.Osu/OsuRuleset.cs index 1658846e98..63da100a04 100644 --- a/osu.Game.Rulesets.Osu/OsuRuleset.cs +++ b/osu.Game.Rulesets.Osu/OsuRuleset.cs @@ -206,7 +206,7 @@ namespace osu.Game.Rulesets.Osu public override HitObjectComposer CreateHitObjectComposer() => new OsuHitObjectComposer(this); - public override BeatmapVerifier CreateChecker() => new OsuBeatmapVerifier(); + public override BeatmapVerifier CreateBeatmapVerifier() => new OsuBeatmapVerifier(); public override string Description => "osu!"; diff --git a/osu.Game/Rulesets/Ruleset.cs b/osu.Game/Rulesets/Ruleset.cs index 088bcc7712..2a29d88c89 100644 --- a/osu.Game/Rulesets/Ruleset.cs +++ b/osu.Game/Rulesets/Ruleset.cs @@ -202,7 +202,7 @@ namespace osu.Game.Rulesets public virtual HitObjectComposer CreateHitObjectComposer() => null; - public virtual BeatmapVerifier CreateChecker() => null; + public virtual BeatmapVerifier CreateBeatmapVerifier() => null; public virtual Drawable CreateIcon() => new SpriteIcon { Icon = FontAwesome.Solid.QuestionCircle }; diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index c0c4a040f6..806029df4d 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -36,7 +36,7 @@ namespace osu.Game.Screens.Edit.Verify var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); ruleset = parent.Get>().Value.BeatmapInfo.Ruleset?.CreateInstance(); - beatmapVerifier = ruleset?.CreateChecker(); + beatmapVerifier = ruleset?.CreateBeatmapVerifier(); return dependencies; } From 7c4f6d2b62635ef38c95754484f7bb82b2d2122d Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 15:47:26 +0200 Subject: [PATCH 28/55] Rework template usage Includes moving the origin check back to templates, constructing nested template classes in each check, and making parameterized template usage. --- .../Edit/Checks/CheckOffscreenObjects.cs | 57 ++++++++++++------- .../Rulesets/Edit/Checks/CheckBackground.cs | 55 +++++++++++------- .../Rulesets/Edit/Checks/Components/Issue.cs | 17 +++--- .../Edit/Checks/Components/IssueTemplate.cs | 8 ++- 4 files changed, 88 insertions(+), 49 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index b34c9966a4..c4f38e6d09 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -22,29 +22,46 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks // (higher = more performant, but higher false-negative chance). private const int path_step_size = 5; + private readonly IssueTemplateOffscreenCircle templateOffscreenCircle; + private readonly IssueTemplateOffscreenSlider templateOffscreenSlider; + private readonly IssueTemplate[] templates; + + private class IssueTemplateOffscreenCircle : IssueTemplate + { + public IssueTemplateOffscreenCircle(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") + { + } + + public Issue Create(HitCircle circle) => new Issue(circle, this); + } + + private class IssueTemplateOffscreenSlider : IssueTemplate + { + public IssueTemplateOffscreenSlider(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") + { + } + + public Issue Create(Slider slider, double offscreenTime) => new Issue(slider, this) { Time = offscreenTime }; + } + + public CheckOffscreenObjects() + { + templates = new IssueTemplate[] + { + templateOffscreenCircle = new IssueTemplateOffscreenCircle(this), + templateOffscreenSlider = new IssueTemplateOffscreenSlider(this) + }; + } + public CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Compose, description: "Offscreen hitobjects." ); - public IEnumerable PossibleTemplates => new[] - { - templateOffscreen, - templateOffscreenSliderPath - }; - - private readonly IssueTemplate templateOffscreen = new IssueTemplate - ( - type: IssueType.Problem, - unformattedMessage: "This object goes offscreen on a 4:3 aspect ratio." - ); - - private readonly IssueTemplate templateOffscreenSliderPath = new IssueTemplate - ( - type: IssueType.Problem, - unformattedMessage: "This slider goes offscreen here on a 4:3 aspect ratio." - ); + public IEnumerable PossibleTemplates => templates; public IEnumerable Run(IBeatmap beatmap) { @@ -63,7 +80,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks case HitCircle circle: { if (isOffscreen(circle.StackedPosition, circle.Radius)) - yield return new Issue(this, circle, templateOffscreen); + yield return templateOffscreenCircle.Create(circle); break; } @@ -89,7 +106,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(this, slider, templateOffscreenSliderPath) { Time = time }; + yield return templateOffscreenSlider.Create(slider, time); yield break; } @@ -98,7 +115,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks if (!isOffscreen(slider.StackedEndPosition, slider.Radius)) yield break; - yield return new Issue(this, slider, templateOffscreenSliderPath) { Time = slider.EndTime }; + yield return templateOffscreenSlider.Create(slider, slider.EndTime); } private bool isOffscreen(Vector2 position, double radius) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index c922aa03c0..1e45ea261c 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -10,35 +10,52 @@ namespace osu.Game.Rulesets.Edit.Checks { public class CheckBackground : ICheck { + private readonly IssueTemplateNoneSet templateNoneSet; + private readonly IssueTemplateDoesNotExist templateDoesNotExist; + private readonly IssueTemplate[] templates; + + private class IssueTemplateNoneSet : IssueTemplate + { + public IssueTemplateNoneSet(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "No background has been set") + { + } + + public Issue Create() => new Issue(this); + } + + private class IssueTemplateDoesNotExist : IssueTemplate + { + public IssueTemplateDoesNotExist(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "The background file \"{0}\" does not exist.") + { + } + + public Issue Create(string filename) => new Issue(this, filename); + } + + public CheckBackground() + { + templates = new IssueTemplate[] + { + templateNoneSet = new IssueTemplateNoneSet(this), + templateDoesNotExist = new IssueTemplateDoesNotExist(this) + }; + } + public CheckMetadata Metadata { get; } = new CheckMetadata ( category: CheckCategory.Resources, description: "Missing background." ); - public IEnumerable PossibleTemplates => new[] - { - templateNoneSet, - templateDoesNotExist - }; - - private readonly IssueTemplate templateNoneSet = new IssueTemplate - ( - type: IssueType.Problem, - unformattedMessage: "No background has been set." - ); - - private readonly IssueTemplate templateDoesNotExist = new IssueTemplate - ( - type: IssueType.Problem, - unformattedMessage: "The background file \"{0}\" is does not exist." - ); + public IEnumerable PossibleTemplates => templates; public IEnumerable Run(IBeatmap beatmap) { if (beatmap.Metadata.BackgroundFile == null) { - yield return new Issue(this, templateNoneSet); + yield return templateNoneSet.Create(); yield break; } @@ -51,7 +68,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (file != null) yield break; - yield return new Issue(this, templateDoesNotExist, beatmap.Metadata.BackgroundFile); + yield return templateDoesNotExist.Create(beatmap.Metadata.BackgroundFile); } } } diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs index d0f7df857b..2bc9930e8f 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/Issue.cs @@ -29,7 +29,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// The check that this issue originates from. /// - public ICheck Check; + public ICheck Check => Template.Check; /// /// The arguments that give this issue its context, based on the . These are then substituted into the . @@ -37,30 +37,29 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// public object[] Arguments; - public Issue(ICheck check, IssueTemplate template, params object[] args) + public Issue(IssueTemplate template, params object[] args) { - Check = check; Time = null; HitObjects = Array.Empty(); Template = template; Arguments = args; } - public Issue(ICheck check, double? time, IssueTemplate template, params object[] args) - : this(check, template, args) + public Issue(double? time, IssueTemplate template, params object[] args) + : this(template, args) { Time = time; } - public Issue(ICheck check, HitObject hitObject, IssueTemplate template, params object[] args) - : this(check, template, args) + public Issue(HitObject hitObject, IssueTemplate template, params object[] args) + : this(template, args) { Time = hitObject.StartTime; HitObjects = new[] { hitObject }; } - public Issue(ICheck check, IEnumerable hitObjects, IssueTemplate template, params object[] args) - : this(check, template, args) + public Issue(IEnumerable hitObjects, IssueTemplate template, params object[] args) + : this(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 4a5f98ca5f..e746844879 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -14,6 +14,11 @@ 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 ICheck Check; + /// /// The type of the issue. /// @@ -26,8 +31,9 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// public readonly string UnformattedMessage; - public IssueTemplate(IssueType type, string unformattedMessage) + public IssueTemplate(ICheck check, IssueType type, string unformattedMessage) { + Check = check; Type = type; UnformattedMessage = unformattedMessage; } From 1c69829ad488edf10b2aaaa347c5b3be28aeabc0 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 15:47:58 +0200 Subject: [PATCH 29/55] Fix `Template.Origin` -> `Check` --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 84dbabf300..516e1adf44 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -112,7 +112,7 @@ namespace osu.Game.Screens.Edit.Verify }, new OsuSpriteText { - Text = issue.Template.Origin.Metadata.Category.ToString(), + Text = issue.Check.Metadata.Category.ToString(), Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), Margin = new MarginPadding(10) } From 008dbc7dd6f841ea245ec8b732696ebbbb2a73b4 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 15:49:13 +0200 Subject: [PATCH 30/55] Reverse `IssueType` ordering Reversed both in the enum and where it's displayed, so ends up the same in the end. --- osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs | 10 +++++----- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs index be43060cfc..1241e058ad 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueType.cs @@ -4,22 +4,22 @@ namespace osu.Game.Rulesets.Edit.Checks.Components { /// - /// The type, or severity, of an issue. This decides its priority. + /// The type, or severity, of an issue. /// public enum IssueType { /// A must-fix in the vast majority of cases. - Problem = 3, + Problem, /// A possible mistake. Often requires critical thinking. - Warning = 2, + Warning, // TODO: Try/catch all checks run and return error templates if exceptions occur. /// An error occurred and a complete check could not be made. - Error = 1, + Error, // TODO: Negligible issues should be hidden by default. /// A possible mistake so minor/unlikely that it can often be safely ignored. - Negligible = 0, + Negligible, } } diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index 806029df4d..a3d808ce62 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -129,8 +129,8 @@ namespace osu.Game.Screens.Edit.Verify private void refresh() { table.Issues = beatmapVerifier.Run(Beatmap) - .OrderByDescending(issue => issue.Template.Type) - .ThenByDescending(issue => issue.Check.Metadata.Category); + .OrderBy(issue => issue.Template.Type) + .ThenBy(issue => issue.Check.Metadata.Category); } } } From caaaba59505f19f999a593a3c35f460c598c9065 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 16:20:53 +0200 Subject: [PATCH 31/55] Rename `Check` -> `ICheck` --- osu.Game/Rulesets/Edit/Checks/Components/{Check.cs => ICheck.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename osu.Game/Rulesets/Edit/Checks/Components/{Check.cs => ICheck.cs} (100%) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/Check.cs b/osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs similarity index 100% rename from osu.Game/Rulesets/Edit/Checks/Components/Check.cs rename to osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs From 6d50d01186f24c342b0fc406803b2486cebbbf4b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 16:23:05 +0200 Subject: [PATCH 32/55] Make `IssueTemplate.Check` readonly --- osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index e746844879..97df79ecd8 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -17,7 +17,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components /// /// The check that this template originates from. /// - public ICheck Check; + public readonly ICheck Check; /// /// The type of the issue. From 36bd235021d08202dd0c489b1bc664b91a31aca1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 23:36:10 +0900 Subject: [PATCH 33/55] Move nested classes to bottom of file --- .../Edit/Checks/CheckOffscreenObjects.cs | 40 +++++++++---------- .../Rulesets/Edit/Checks/CheckBackground.cs | 40 +++++++++---------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index c4f38e6d09..f2a062b6d7 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -26,26 +26,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks private readonly IssueTemplateOffscreenSlider templateOffscreenSlider; private readonly IssueTemplate[] templates; - private class IssueTemplateOffscreenCircle : IssueTemplate - { - public IssueTemplateOffscreenCircle(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") - { - } - - public Issue Create(HitCircle circle) => new Issue(circle, this); - } - - private class IssueTemplateOffscreenSlider : IssueTemplate - { - public IssueTemplateOffscreenSlider(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") - { - } - - public Issue Create(Slider slider, double offscreenTime) => new Issue(slider, this) { Time = offscreenTime }; - } - public CheckOffscreenObjects() { templates = new IssueTemplate[] @@ -123,5 +103,25 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks return position.X - radius < min_x || position.X + radius > max_x || position.Y - radius < min_y || position.Y + radius > max_y; } + + private class IssueTemplateOffscreenCircle : IssueTemplate + { + public IssueTemplateOffscreenCircle(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") + { + } + + public Issue Create(HitCircle circle) => new Issue(circle, this); + } + + private class IssueTemplateOffscreenSlider : IssueTemplate + { + public IssueTemplateOffscreenSlider(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") + { + } + + public Issue Create(Slider slider, double offscreenTime) => new Issue(slider, this) { Time = offscreenTime }; + } } } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 1e45ea261c..b48d19ae29 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -14,26 +14,6 @@ namespace osu.Game.Rulesets.Edit.Checks private readonly IssueTemplateDoesNotExist templateDoesNotExist; private readonly IssueTemplate[] templates; - private class IssueTemplateNoneSet : IssueTemplate - { - public IssueTemplateNoneSet(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "No background has been set") - { - } - - public Issue Create() => new Issue(this); - } - - private class IssueTemplateDoesNotExist : IssueTemplate - { - public IssueTemplateDoesNotExist(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "The background file \"{0}\" does not exist.") - { - } - - public Issue Create(string filename) => new Issue(this, filename); - } - public CheckBackground() { templates = new IssueTemplate[] @@ -70,5 +50,25 @@ namespace osu.Game.Rulesets.Edit.Checks yield return templateDoesNotExist.Create(beatmap.Metadata.BackgroundFile); } + + private class IssueTemplateNoneSet : IssueTemplate + { + public IssueTemplateNoneSet(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "No background has been set") + { + } + + public Issue Create() => new Issue(this); + } + + private class IssueTemplateDoesNotExist : IssueTemplate + { + public IssueTemplateDoesNotExist(ICheck checkOrigin) + : base(checkOrigin, IssueType.Problem, "The background file \"{0}\" does not exist.") + { + } + + public Issue Create(string filename) => new Issue(this, filename); + } } } From 62c181228284f034168b87c27879fa72b87369b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Apr 2021 23:37:47 +0900 Subject: [PATCH 34/55] Remove redundant parameter naming --- osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs | 6 +----- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index f2a062b6d7..8d4cf2f4f0 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -35,11 +35,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks }; } - public CheckMetadata Metadata { get; } = new CheckMetadata - ( - category: CheckCategory.Compose, - description: "Offscreen hitobjects." - ); + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Offscreen hitobjects"); public IEnumerable PossibleTemplates => templates; diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index b48d19ae29..1a766798cb 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -23,11 +23,7 @@ namespace osu.Game.Rulesets.Edit.Checks }; } - public CheckMetadata Metadata { get; } = new CheckMetadata - ( - category: CheckCategory.Resources, - description: "Missing background." - ); + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Resources, "Missing background"); public IEnumerable PossibleTemplates => templates; From bb720c23a01e6f27b376491abf29e578f4db5bc6 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 17:12:37 +0200 Subject: [PATCH 35/55] Remove check ctors and locals --- .../Edit/Checks/CheckOffscreenObjects.cs | 25 ++++++------------- .../Rulesets/Edit/Checks/CheckBackground.cs | 23 ++++++----------- 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index 8d4cf2f4f0..adaabc0a1d 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -22,22 +22,13 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks // (higher = more performant, but higher false-negative chance). private const int path_step_size = 5; - private readonly IssueTemplateOffscreenCircle templateOffscreenCircle; - private readonly IssueTemplateOffscreenSlider templateOffscreenSlider; - private readonly IssueTemplate[] templates; - - public CheckOffscreenObjects() - { - templates = new IssueTemplate[] - { - templateOffscreenCircle = new IssueTemplateOffscreenCircle(this), - templateOffscreenSlider = new IssueTemplateOffscreenSlider(this) - }; - } - public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Offscreen hitobjects"); - public IEnumerable PossibleTemplates => templates; + public IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplateOffscreenCircle(this), + new IssueTemplateOffscreenSlider(this) + }; public IEnumerable Run(IBeatmap beatmap) { @@ -56,7 +47,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks case HitCircle circle: { if (isOffscreen(circle.StackedPosition, circle.Radius)) - yield return templateOffscreenCircle.Create(circle); + yield return new IssueTemplateOffscreenCircle(this).Create(circle); break; } @@ -82,7 +73,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks // `SpanDuration` ensures we don't include reverses. double time = slider.StartTime + progress * slider.SpanDuration; - yield return templateOffscreenSlider.Create(slider, time); + yield return new IssueTemplateOffscreenSlider(this).Create(slider, time); yield break; } @@ -91,7 +82,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks if (!isOffscreen(slider.StackedEndPosition, slider.Radius)) yield break; - yield return templateOffscreenSlider.Create(slider, slider.EndTime); + yield return new IssueTemplateOffscreenSlider(this).Create(slider, slider.EndTime); } private bool isOffscreen(Vector2 position, double radius) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 1a766798cb..b0c1d6eb4b 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -10,28 +10,19 @@ namespace osu.Game.Rulesets.Edit.Checks { public class CheckBackground : ICheck { - private readonly IssueTemplateNoneSet templateNoneSet; - private readonly IssueTemplateDoesNotExist templateDoesNotExist; - private readonly IssueTemplate[] templates; - - public CheckBackground() - { - templates = new IssueTemplate[] - { - templateNoneSet = new IssueTemplateNoneSet(this), - templateDoesNotExist = new IssueTemplateDoesNotExist(this) - }; - } - public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Resources, "Missing background"); - public IEnumerable PossibleTemplates => templates; + public IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplateNoneSet(this), + new IssueTemplateDoesNotExist(this) + }; public IEnumerable Run(IBeatmap beatmap) { if (beatmap.Metadata.BackgroundFile == null) { - yield return templateNoneSet.Create(); + yield return new IssueTemplateNoneSet(this).Create(); yield break; } @@ -44,7 +35,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (file != null) yield break; - yield return templateDoesNotExist.Create(beatmap.Metadata.BackgroundFile); + yield return new IssueTemplateDoesNotExist(this).Create(beatmap.Metadata.BackgroundFile); } private class IssueTemplateNoneSet : IssueTemplate From 19a154ddf15684f8a3429a642e3f71e555e0d27f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 12 Apr 2021 17:28:12 +0200 Subject: [PATCH 36/55] Rename `checkOrigin` -> `check` More consistent with `Issue.ctor`'s "template". --- .../Edit/Checks/CheckOffscreenObjects.cs | 8 ++++---- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index adaabc0a1d..0a682c4a83 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -93,8 +93,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks private class IssueTemplateOffscreenCircle : IssueTemplate { - public IssueTemplateOffscreenCircle(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") + public IssueTemplateOffscreenCircle(ICheck check) + : base(check, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") { } @@ -103,8 +103,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks private class IssueTemplateOffscreenSlider : IssueTemplate { - public IssueTemplateOffscreenSlider(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") + public IssueTemplateOffscreenSlider(ICheck check) + : base(check, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") { } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index b0c1d6eb4b..463b596120 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -40,8 +40,8 @@ namespace osu.Game.Rulesets.Edit.Checks private class IssueTemplateNoneSet : IssueTemplate { - public IssueTemplateNoneSet(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "No background has been set") + public IssueTemplateNoneSet(ICheck check) + : base(check, IssueType.Problem, "No background has been set") { } @@ -50,8 +50,8 @@ namespace osu.Game.Rulesets.Edit.Checks private class IssueTemplateDoesNotExist : IssueTemplate { - public IssueTemplateDoesNotExist(ICheck checkOrigin) - : base(checkOrigin, IssueType.Problem, "The background file \"{0}\" does not exist.") + public IssueTemplateDoesNotExist(ICheck check) + : base(check, IssueType.Problem, "The background file \"{0}\" does not exist.") { } From d8088777ea0a1177fb3e205530fdab7e04165458 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 01:21:34 +0200 Subject: [PATCH 37/55] Add `Equals` method to `IssueTemplate` This will be useful in tests. --- osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index 97df79ecd8..e21f14f4bc 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -70,5 +70,7 @@ namespace osu.Game.Rulesets.Edit.Checks.Components } } } + + public bool Equals(IssueTemplate other) => other.Type == Type && other.UnformattedMessage == UnformattedMessage; } } From 47cf4bcf2595c672c0f70a85da4c55a36beea182 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 01:22:24 +0200 Subject: [PATCH 38/55] Add `CheckBackground` tests --- .../Editing/Checks/CheckBackgroundTest.cs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs diff --git a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs new file mode 100644 index 0000000000..54d1a116c7 --- /dev/null +++ b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs @@ -0,0 +1,73 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Tests.Editing.Checks +{ + [TestFixture] + public class CheckBackgroundTest + { + private CheckBackground check; + private IBeatmap beatmap; + + [SetUp] + public void Setup() + { + check = new CheckBackground(); + beatmap = new Beatmap + { + BeatmapInfo = new BeatmapInfo + { + Metadata = new BeatmapMetadata { BackgroundFile = "abc123.jpg" }, + BeatmapSet = new BeatmapSetInfo + { + Files = new List(new [] + { + new BeatmapSetFileInfo { Filename = "abc123.jpg" } + }) + } + } + }; + } + + [Test] + public void TestBackgroundSetAndInFiles() + { + var issues = check.Run(beatmap); + + Assert.That(!issues.Any()); + } + + [Test] + public void TestBackgroundSetAndNotInFiles() + { + beatmap.BeatmapInfo.BeatmapSet.Files.Clear(); + + var issues = check.Run(beatmap).ToList(); + var issue = issues.FirstOrDefault(); + + Assert.That(issues.Count == 1); + Assert.That(issue != null); + Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(1))); + } + + [Test] + public void TestBackgroundNotSet() + { + beatmap.Metadata.BackgroundFile = null; + + var issues = check.Run(beatmap).ToList(); + var issue = issues.FirstOrDefault(); + + Assert.That(issues.Count == 1); + Assert.That(issue != null); + Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(0))); + } + } +} From 8a6dfcfae1cb9c8b50e04777648098ef271a8d56 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 01:22:36 +0200 Subject: [PATCH 39/55] Add `CheckOffscreenObjects` tests --- .../Checks/CheckOffscreenObjectsTest.cs | 263 ++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs new file mode 100644 index 0000000000..fa7854765f --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs @@ -0,0 +1,263 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Edit.Checks; +using osu.Game.Rulesets.Osu.Objects; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks +{ + [TestFixture] + public class CheckOffscreenObjectsTest + { + private CheckOffscreenObjects check; + + [SetUp] + public void Setup() + { + check = new CheckOffscreenObjects(); + } + + [Test] + public void TestCircleInCenter() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new HitCircle + { + StartTime = 3000, + Position = new Vector2(320, 240) // Playfield is 640 x 480. + } + } + }; + + var issues = check.Run(beatmap); + + Assert.That(!issues.Any()); + } + + [Test] + public void TestCircleNearEdge() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new HitCircle + { + StartTime = 3000, + Position = new Vector2(5, 5) + } + } + }; + + var issues = check.Run(beatmap); + + Assert.That(!issues.Any()); + } + + [Test] + public void TestCircleNearEdgeStackedOffscreen() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new HitCircle + { + StartTime = 3000, + Position = new Vector2(5, 5), + StackHeight = 5 + } + } + }; + + assertOffscreenCircle(beatmap); + } + + [Test] + public void TestCircleOffscreen() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new HitCircle + { + StartTime = 3000, + Position = new Vector2(0, 0) + } + } + }; + + assertOffscreenCircle(beatmap); + } + + [Test] + public void TestSliderInCenter() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(420, 240), + Path = new SliderPath(new[] + { + new PathControlPoint(new Vector2(0, 0), PathType.Linear), + new PathControlPoint(new Vector2(-100, 0)) + }), + } + } + }; + + var issues = check.Run(beatmap); + + Assert.That(!issues.Any()); + } + + [Test] + public void TestSliderNearEdge() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(320, 240), + Path = new SliderPath(new[] + { + new PathControlPoint(new Vector2(0, 0), PathType.Linear), + new PathControlPoint(new Vector2(0, -235)) + }), + } + } + }; + + var issues = check.Run(beatmap); + + Assert.That(!issues.Any()); + } + + [Test] + public void TestSliderNearEdgeStackedOffscreen() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(320, 240), + Path = new SliderPath(new[] + { + new PathControlPoint(new Vector2(0, 0), PathType.Linear), + new PathControlPoint(new Vector2(0, -235)) + }), + StackHeight = 5 + } + } + }; + + assertOffscreenSlider(beatmap); + } + + [Test] + public void TestSliderOffscreenStart() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(0, 0), + Path = new SliderPath(new[] + { + new PathControlPoint(new Vector2(0, 0), PathType.Linear), + new PathControlPoint(new Vector2(320, 240)) + }), + } + } + }; + + assertOffscreenSlider(beatmap); + } + + [Test] + public void TestSliderOffscreenEnd() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(320, 240), + Path = new SliderPath(new[] + { + new PathControlPoint(new Vector2(0, 0), PathType.Linear), + new PathControlPoint(new Vector2(-320, -240)) + }), + } + } + }; + + assertOffscreenSlider(beatmap); + } + + [Test] + public void TestSliderOffscreenPath() + { + var beatmap = new Beatmap + { + HitObjects = new List + { + new Slider + { + StartTime = 3000, + Position = new Vector2(320, 240), + Path = new SliderPath(new[] + { + // Circular arc shoots over the top of the screen. + new PathControlPoint(new Vector2(0, 0), PathType.PerfectCurve), + new PathControlPoint(new Vector2(-100, -200)), + new PathControlPoint(new Vector2(100, -200)) + }), + } + } + }; + + assertOffscreenSlider(beatmap); + } + + private void assertOneIssue(IBeatmap beatmap, int templateIndex) + { + var issues = check.Run(beatmap).ToList(); + var issue = issues.FirstOrDefault(); + + Assert.That(issues.Count == 1); + Assert.That(issue != null); + Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(templateIndex))); + } + + private void assertOffscreenCircle(IBeatmap beatmap) => assertOneIssue(beatmap, 0); + + private void assertOffscreenSlider(IBeatmap beatmap) => assertOneIssue(beatmap, 1); + } +} From 0bcc39bd36a9a5893f57fcaa8b94f9d3e31356e9 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 02:17:35 +0200 Subject: [PATCH 40/55] Remove redundant space --- osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs index 54d1a116c7..2f309afb6c 100644 --- a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Editing.Checks Metadata = new BeatmapMetadata { BackgroundFile = "abc123.jpg" }, BeatmapSet = new BeatmapSetInfo { - Files = new List(new [] + Files = new List(new[] { new BeatmapSetFileInfo { Filename = "abc123.jpg" } }) From 6d3f9fa9cefdd776d937e0c6b809df8624457e50 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 02:29:25 +0200 Subject: [PATCH 41/55] Use `is` class instead of `Equals` with template index Ensures ordering of `PossibleTemplates` does not affect tests. --- .../Editor/Checks/CheckOffscreenObjectsTest.cs | 14 ++++++++++---- .../Edit/Checks/CheckOffscreenObjects.cs | 4 ++-- .../Editing/Checks/CheckBackgroundTest.cs | 4 ++-- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 4 ++-- .../Edit/Checks/Components/IssueTemplate.cs | 2 -- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs index fa7854765f..33b839650f 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs @@ -246,18 +246,24 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks assertOffscreenSlider(beatmap); } - private void assertOneIssue(IBeatmap beatmap, int templateIndex) + private void assertOffscreenCircle(IBeatmap beatmap) { var issues = check.Run(beatmap).ToList(); var issue = issues.FirstOrDefault(); Assert.That(issues.Count == 1); Assert.That(issue != null); - Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(templateIndex))); + Assert.That(issue.Template is CheckOffscreenObjects.IssueTemplateOffscreenCircle); } - private void assertOffscreenCircle(IBeatmap beatmap) => assertOneIssue(beatmap, 0); + private void assertOffscreenSlider(IBeatmap beatmap) + { + var issues = check.Run(beatmap).ToList(); + var issue = issues.FirstOrDefault(); - private void assertOffscreenSlider(IBeatmap beatmap) => assertOneIssue(beatmap, 1); + Assert.That(issues.Count == 1); + Assert.That(issue != null); + Assert.That(issue.Template is CheckOffscreenObjects.IssueTemplateOffscreenSlider); + } } } diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index 0a682c4a83..c241db7e06 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -91,7 +91,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks position.Y - radius < min_y || position.Y + radius > max_y; } - private class IssueTemplateOffscreenCircle : IssueTemplate + public class IssueTemplateOffscreenCircle : IssueTemplate { public IssueTemplateOffscreenCircle(ICheck check) : base(check, IssueType.Problem, "This circle goes offscreen on a 4:3 aspect ratio.") @@ -101,7 +101,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks public Issue Create(HitCircle circle) => new Issue(circle, this); } - private class IssueTemplateOffscreenSlider : IssueTemplate + public class IssueTemplateOffscreenSlider : IssueTemplate { public IssueTemplateOffscreenSlider(ICheck check) : base(check, IssueType.Problem, "This slider goes offscreen here on a 4:3 aspect ratio.") diff --git a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs index 2f309afb6c..327abcab6c 100644 --- a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs @@ -54,7 +54,7 @@ namespace osu.Game.Tests.Editing.Checks Assert.That(issues.Count == 1); Assert.That(issue != null); - Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(1))); + Assert.That(issue.Template is CheckBackground.IssueTemplateDoesNotExist); } [Test] @@ -67,7 +67,7 @@ namespace osu.Game.Tests.Editing.Checks Assert.That(issues.Count == 1); Assert.That(issue != null); - Assert.That(issue.Template.Equals(check.PossibleTemplates.ElementAt(0))); + Assert.That(issue.Template is CheckBackground.IssueTemplateNoneSet); } } } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 463b596120..4d5069f446 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Edit.Checks yield return new IssueTemplateDoesNotExist(this).Create(beatmap.Metadata.BackgroundFile); } - private class IssueTemplateNoneSet : IssueTemplate + public class IssueTemplateNoneSet : IssueTemplate { public IssueTemplateNoneSet(ICheck check) : base(check, IssueType.Problem, "No background has been set") @@ -48,7 +48,7 @@ namespace osu.Game.Rulesets.Edit.Checks public Issue Create() => new Issue(this); } - private class IssueTemplateDoesNotExist : IssueTemplate + public class IssueTemplateDoesNotExist : IssueTemplate { public IssueTemplateDoesNotExist(ICheck check) : base(check, IssueType.Problem, "The background file \"{0}\" does not exist.") diff --git a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs index e21f14f4bc..97df79ecd8 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/IssueTemplate.cs @@ -70,7 +70,5 @@ namespace osu.Game.Rulesets.Edit.Checks.Components } } } - - public bool Equals(IssueTemplate other) => other.Type == Type && other.UnformattedMessage == UnformattedMessage; } } From 4837cef095558c2ed301881471f3eaf76d6eb209 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 14:44:52 +0900 Subject: [PATCH 42/55] Use static for playfield centre positioning --- .../Checks/CheckOffscreenObjectsTest.cs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs index 33b839650f..4a5e4e18c0 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs @@ -9,6 +9,7 @@ using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Edit.Checks; using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.UI; using osuTK; namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks @@ -16,6 +17,8 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks [TestFixture] public class CheckOffscreenObjectsTest { + private static readonly Vector2 playfield_centre = OsuPlayfield.BASE_SIZE * 0.5f; + private CheckOffscreenObjects check; [SetUp] @@ -34,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks new HitCircle { StartTime = 3000, - Position = new Vector2(320, 240) // Playfield is 640 x 480. + Position = playfield_centre // Playfield is 640 x 480. } } }; @@ -136,11 +139,11 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks new Slider { StartTime = 3000, - Position = new Vector2(320, 240), + Position = playfield_centre, Path = new SliderPath(new[] { new PathControlPoint(new Vector2(0, 0), PathType.Linear), - new PathControlPoint(new Vector2(0, -235)) + new PathControlPoint(new Vector2(0, -playfield_centre.Y + 5)) }), } } @@ -161,11 +164,11 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks new Slider { StartTime = 3000, - Position = new Vector2(320, 240), + Position = playfield_centre, Path = new SliderPath(new[] { new PathControlPoint(new Vector2(0, 0), PathType.Linear), - new PathControlPoint(new Vector2(0, -235)) + new PathControlPoint(new Vector2(0, -playfield_centre.Y + 5)) }), StackHeight = 5 } @@ -189,7 +192,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks Path = new SliderPath(new[] { new PathControlPoint(new Vector2(0, 0), PathType.Linear), - new PathControlPoint(new Vector2(320, 240)) + new PathControlPoint(playfield_centre) }), } } @@ -208,11 +211,11 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks new Slider { StartTime = 3000, - Position = new Vector2(320, 240), + Position = playfield_centre, Path = new SliderPath(new[] { new PathControlPoint(new Vector2(0, 0), PathType.Linear), - new PathControlPoint(new Vector2(-320, -240)) + new PathControlPoint(-playfield_centre) }), } } @@ -231,7 +234,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks new Slider { StartTime = 3000, - Position = new Vector2(320, 240), + Position = playfield_centre, Path = new SliderPath(new[] { // Circular arc shoots over the top of the screen. From b45d7de4eccbd9aecbf621afefc6661ca986fa9e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 15:04:01 +0900 Subject: [PATCH 43/55] Update asserts to use better nunit specifications --- .../Checks/CheckOffscreenObjectsTest.cs | 29 ++++++------------- .../Editing/Checks/CheckBackgroundTest.cs | 16 ++++------ 2 files changed, 14 insertions(+), 31 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs index 4a5e4e18c0..9f9ba0e8db 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Edit.Checks; @@ -42,9 +43,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks } }; - var issues = check.Run(beatmap); - - Assert.That(!issues.Any()); + Assert.That(check.Run(beatmap), Is.Empty); } [Test] @@ -62,9 +61,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks } }; - var issues = check.Run(beatmap); - - Assert.That(!issues.Any()); + Assert.That(check.Run(beatmap), Is.Empty); } [Test] @@ -124,9 +121,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks } }; - var issues = check.Run(beatmap); - - Assert.That(!issues.Any()); + Assert.That(check.Run(beatmap), Is.Empty); } [Test] @@ -149,9 +144,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks } }; - var issues = check.Run(beatmap); - - Assert.That(!issues.Any()); + Assert.That(check.Run(beatmap), Is.Empty); } [Test] @@ -252,21 +245,17 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks private void assertOffscreenCircle(IBeatmap beatmap) { var issues = check.Run(beatmap).ToList(); - var issue = issues.FirstOrDefault(); - Assert.That(issues.Count == 1); - Assert.That(issue != null); - Assert.That(issue.Template is CheckOffscreenObjects.IssueTemplateOffscreenCircle); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.Single().Template is CheckOffscreenObjects.IssueTemplateOffscreenCircle); } private void assertOffscreenSlider(IBeatmap beatmap) { var issues = check.Run(beatmap).ToList(); - var issue = issues.FirstOrDefault(); - Assert.That(issues.Count == 1); - Assert.That(issue != null); - Assert.That(issue.Template is CheckOffscreenObjects.IssueTemplateOffscreenSlider); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.Single().Template is CheckOffscreenObjects.IssueTemplateOffscreenSlider); } } } diff --git a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs index 327abcab6c..635e3bb0f3 100644 --- a/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckBackgroundTest.cs @@ -39,9 +39,7 @@ namespace osu.Game.Tests.Editing.Checks [Test] public void TestBackgroundSetAndInFiles() { - var issues = check.Run(beatmap); - - Assert.That(!issues.Any()); + Assert.That(check.Run(beatmap), Is.Empty); } [Test] @@ -50,11 +48,9 @@ namespace osu.Game.Tests.Editing.Checks beatmap.BeatmapInfo.BeatmapSet.Files.Clear(); var issues = check.Run(beatmap).ToList(); - var issue = issues.FirstOrDefault(); - Assert.That(issues.Count == 1); - Assert.That(issue != null); - Assert.That(issue.Template is CheckBackground.IssueTemplateDoesNotExist); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.Single().Template is CheckBackground.IssueTemplateDoesNotExist); } [Test] @@ -63,11 +59,9 @@ namespace osu.Game.Tests.Editing.Checks beatmap.Metadata.BackgroundFile = null; var issues = check.Run(beatmap).ToList(); - var issue = issues.FirstOrDefault(); - Assert.That(issues.Count == 1); - Assert.That(issue != null); - Assert.That(issue.Template is CheckBackground.IssueTemplateNoneSet); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.Single().Template is CheckBackground.IssueTemplateNoneSet); } } } From 98c25b2e71c1aef8ad2973c0cb7ce96b33faff42 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:33:08 +0200 Subject: [PATCH 44/55] Remove unused import --- .../Editor/Checks/CheckOffscreenObjectsTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs index 9f9ba0e8db..f9445a9a96 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckOffscreenObjectsTest.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; -using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Edit.Checks; From c8cb4286f6e61eeba14b4b0256d5a17d781f6bd9 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:35:06 +0200 Subject: [PATCH 45/55] Add reference for screen bounding box numbers --- osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs index c241db7e06..27cae2ecc1 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckOffscreenObjects.cs @@ -11,8 +11,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks { 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. + // A close approximation for the bounding box of the screen in gameplay on 4:3 aspect ratio. + // Uses gameplay space coordinates (512 x 384 playfield / 640 x 480 screen area). + // See https://github.com/ppy/osu/pull/12361#discussion_r612199777 for reference. private const int min_x = -67; private const int min_y = -60; private const int max_x = 579; From 60c2494b316827c691327a55793750871753a190 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:40:56 +0200 Subject: [PATCH 46/55] Make `BeatmapVerifier` an interface --- .../Edit/OsuBeatmapVerifier.cs | 12 ++---------- osu.Game.Rulesets.Osu/OsuRuleset.cs | 2 +- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 18 ++---------------- osu.Game/Rulesets/Ruleset.cs | 2 +- osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 2 +- 5 files changed, 7 insertions(+), 29 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 2faa239720..1c7ab00bbb 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -10,21 +10,13 @@ using osu.Game.Rulesets.Osu.Edit.Checks; namespace osu.Game.Rulesets.Osu.Edit { - public class OsuBeatmapVerifier : BeatmapVerifier + public class OsuBeatmapVerifier : IBeatmapVerifier { private readonly List checks = new List { new CheckOffscreenObjects() }; - public override IEnumerable Run(IBeatmap beatmap) - { - // Also run mode-invariant checks. - foreach (var issue in base.Run(beatmap)) - yield return issue; - - foreach (var issue in checks.SelectMany(check => check.Run(beatmap))) - yield return issue; - } + public IEnumerable Run(IBeatmap beatmap) => checks.SelectMany(check => check.Run(beatmap)); } } diff --git a/osu.Game.Rulesets.Osu/OsuRuleset.cs b/osu.Game.Rulesets.Osu/OsuRuleset.cs index 63da100a04..d6375fa6e3 100644 --- a/osu.Game.Rulesets.Osu/OsuRuleset.cs +++ b/osu.Game.Rulesets.Osu/OsuRuleset.cs @@ -206,7 +206,7 @@ namespace osu.Game.Rulesets.Osu public override HitObjectComposer CreateHitObjectComposer() => new OsuHitObjectComposer(this); - public override BeatmapVerifier CreateBeatmapVerifier() => new OsuBeatmapVerifier(); + public override IBeatmapVerifier CreateBeatmapVerifier() => new OsuBeatmapVerifier(); public override string Description => "osu!"; diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index 1d0508705a..2bafacefa3 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -2,27 +2,13 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Linq; using osu.Game.Beatmaps; -using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit { - public abstract class BeatmapVerifier + public interface IBeatmapVerifier { - /// - /// 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 - { - new CheckBackground() - }; - - public virtual IEnumerable Run(IBeatmap beatmap) - { - return generalChecks.SelectMany(check => check.Run(beatmap)); - } + public IEnumerable Run(IBeatmap beatmap); } } diff --git a/osu.Game/Rulesets/Ruleset.cs b/osu.Game/Rulesets/Ruleset.cs index 2a29d88c89..b501c55aef 100644 --- a/osu.Game/Rulesets/Ruleset.cs +++ b/osu.Game/Rulesets/Ruleset.cs @@ -202,7 +202,7 @@ namespace osu.Game.Rulesets public virtual HitObjectComposer CreateHitObjectComposer() => null; - public virtual BeatmapVerifier CreateBeatmapVerifier() => null; + public virtual IBeatmapVerifier CreateBeatmapVerifier() => null; public virtual Drawable CreateIcon() => new SpriteIcon { Icon = FontAwesome.Solid.QuestionCircle }; diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index a3d808ce62..a733c9c176 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -21,7 +21,7 @@ namespace osu.Game.Screens.Edit.Verify public class VerifyScreen : EditorScreen { private Ruleset ruleset; - private static BeatmapVerifier beatmapVerifier; + private static IBeatmapVerifier beatmapVerifier; [Cached] private Bindable selectedIssue = new Bindable(); From 304fe5cd341027b19da18b82f394e5fc444a2883 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:41:02 +0200 Subject: [PATCH 47/55] Add `CheckBackground` to `OsuBeatmapVerifier` --- osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 1c7ab00bbb..66ef74ab08 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Osu.Edit.Checks; @@ -14,6 +15,10 @@ namespace osu.Game.Rulesets.Osu.Edit { private readonly List checks = new List { + // General checks + new CheckBackground(), + + // Ruleset-specific checks new CheckOffscreenObjects() }; From aa5fe2e9fcdbe3f2dd6ac1d63328c4f5f4af477d Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 11:02:01 +0200 Subject: [PATCH 48/55] Rename `BeatmapVerifier` -> `IBeatmapVerifier` --- .../Rulesets/Edit/{BeatmapVerifier.cs => IBeatmapVerifier.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename osu.Game/Rulesets/Edit/{BeatmapVerifier.cs => IBeatmapVerifier.cs} (100%) diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/IBeatmapVerifier.cs similarity index 100% rename from osu.Game/Rulesets/Edit/BeatmapVerifier.cs rename to osu.Game/Rulesets/Edit/IBeatmapVerifier.cs From 03ba04e8cec2b335355916d477712e84305ce00e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 19:50:22 +0900 Subject: [PATCH 49/55] Split out general checks into its own verifier class (and remove `static` usage) --- .../Edit/OsuBeatmapVerifier.cs | 5 --- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 24 ++++++++++++++ osu.Game/Screens/Edit/Verify/VerifyScreen.cs | 32 ++++++++----------- 3 files changed, 38 insertions(+), 23 deletions(-) create mode 100644 osu.Game/Rulesets/Edit/BeatmapVerifier.cs diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 66ef74ab08..1c7ab00bbb 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; -using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Osu.Edit.Checks; @@ -15,10 +14,6 @@ namespace osu.Game.Rulesets.Osu.Edit { private readonly List checks = new List { - // General checks - new CheckBackground(), - - // Ruleset-specific checks new CheckOffscreenObjects() }; diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs new file mode 100644 index 0000000000..b1d538bf04 --- /dev/null +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -0,0 +1,24 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Edit.Checks.Components; + +namespace osu.Game.Rulesets.Edit +{ + /// + /// A ruleset-agnostic beatmap converter that identifies issues in common metadata or mapping standards. + /// + public class BeatmapVerifier : IBeatmapVerifier + { + private readonly List checks = new List + { + new CheckBackground(), + }; + + public IEnumerable Run(IBeatmap beatmap) => checks.SelectMany(check => check.Run(beatmap)); + } +} diff --git a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs index a733c9c176..550fbe2950 100644 --- a/osu.Game/Screens/Edit/Verify/VerifyScreen.cs +++ b/osu.Game/Screens/Edit/Verify/VerifyScreen.cs @@ -7,11 +7,9 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; -using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; -using osu.Game.Rulesets; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Checks.Components; using osuTK; @@ -20,9 +18,6 @@ namespace osu.Game.Screens.Edit.Verify { public class VerifyScreen : EditorScreen { - private Ruleset ruleset; - private static IBeatmapVerifier beatmapVerifier; - [Cached] private Bindable selectedIssue = new Bindable(); @@ -31,16 +26,6 @@ namespace osu.Game.Screens.Edit.Verify { } - protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) - { - var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - - ruleset = parent.Get>().Value.BeatmapInfo.Ruleset?.CreateInstance(); - beatmapVerifier = ruleset?.CreateBeatmapVerifier(); - - return dependencies; - } - [BackgroundDependencyLoader] private void load() { @@ -81,9 +66,15 @@ namespace osu.Game.Screens.Edit.Verify [Resolved] private Bindable selectedIssue { get; set; } + private IBeatmapVerifier rulesetVerifier; + private BeatmapVerifier generalVerifier; + [BackgroundDependencyLoader] private void load(OsuColour colours) { + generalVerifier = new BeatmapVerifier(); + rulesetVerifier = Beatmap.BeatmapInfo.Ruleset?.CreateInstance()?.CreateBeatmapVerifier(); + RelativeSizeAxes = Axes.Both; InternalChildren = new Drawable[] @@ -128,9 +119,14 @@ namespace osu.Game.Screens.Edit.Verify private void refresh() { - table.Issues = beatmapVerifier.Run(Beatmap) - .OrderBy(issue => issue.Template.Type) - .ThenBy(issue => issue.Check.Metadata.Category); + var issues = generalVerifier.Run(Beatmap); + + if (rulesetVerifier != null) + issues = issues.Concat(rulesetVerifier.Run(Beatmap)); + + table.Issues = issues + .OrderBy(issue => issue.Template.Type) + .ThenBy(issue => issue.Check.Metadata.Category); } } } From 464fc02875f067aef4bd55f3244aa1df97cc774b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 19:55:17 +0900 Subject: [PATCH 50/55] Fix some styling issues with the verify screen layout --- osu.Game/Screens/Edit/Verify/IssueTable.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 516e1adf44..042d6d84e3 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -38,9 +38,6 @@ namespace osu.Game.Screens.Edit.Verify Padding = new MarginPadding { Horizontal = horizontal_inset }; RowSize = new Dimension(GridSizeMode.Absolute, row_height); - Masking = true; - CornerRadius = 6; - AddInternal(backgroundFlow = new FillFlowContainer { RelativeSizeAxes = Axes.Both, @@ -118,6 +115,17 @@ namespace osu.Game.Screens.Edit.Verify } }; + protected override Drawable CreateHeader(int index, TableColumn column) => new HeaderText(column?.Header ?? string.Empty); + + private class HeaderText : OsuSpriteText + { + public HeaderText(string text) + { + Text = text.ToUpper(); + Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold); + } + } + public class RowBackground : OsuClickableContainer { private readonly Issue issue; From 0d6890243fc7b6308ffdd87e7f87c1551d4ae2fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 20:18:18 +0900 Subject: [PATCH 51/55] Fix typo in xmldoc --- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index b1d538bf04..f9bced7beb 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -10,7 +10,7 @@ using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit { /// - /// A ruleset-agnostic beatmap converter that identifies issues in common metadata or mapping standards. + /// A ruleset-agnostic beatmap verifier that identifies issues in common metadata or mapping standards. /// public class BeatmapVerifier : IBeatmapVerifier { From 69da804f817dac304b7bc36587070d5f129b5eae Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 13 Apr 2021 13:57:56 +0200 Subject: [PATCH 52/55] Add missing period --- osu.Game/Rulesets/Edit/Checks/CheckBackground.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs index 4d5069f446..93da42425c 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckBackground.cs @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Edit.Checks public class IssueTemplateNoneSet : IssueTemplate { public IssueTemplateNoneSet(ICheck check) - : base(check, IssueType.Problem, "No background has been set") + : base(check, IssueType.Problem, "No background has been set.") { } From 0edc1a850d95c1c8bd94c873f12be8b40bcb33d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 23:05:58 +0900 Subject: [PATCH 53/55] Split out common EditorTable base class --- osu.Game/Screens/Edit/EditorTable.cs | 49 +++++++++++++++++ .../Screens/Edit/Timing/ControlPointTable.cs | 44 ++-------------- osu.Game/Screens/Edit/Verify/IssueTable.cs | 52 ++++--------------- 3 files changed, 63 insertions(+), 82 deletions(-) create mode 100644 osu.Game/Screens/Edit/EditorTable.cs diff --git a/osu.Game/Screens/Edit/EditorTable.cs b/osu.Game/Screens/Edit/EditorTable.cs new file mode 100644 index 0000000000..e5e2add384 --- /dev/null +++ b/osu.Game/Screens/Edit/EditorTable.cs @@ -0,0 +1,49 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Graphics; +using osu.Game.Graphics.Sprites; + +namespace osu.Game.Screens.Edit +{ + public abstract class EditorTable : TableContainer + { + private const float horizontal_inset = 20; + + protected const float ROW_HEIGHT = 25; + + protected const int TEXT_SIZE = 14; + + protected readonly FillFlowContainer BackgroundFlow; + + protected EditorTable() + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + Padding = new MarginPadding { Horizontal = horizontal_inset }; + RowSize = new Dimension(GridSizeMode.Absolute, ROW_HEIGHT); + + AddInternal(BackgroundFlow = new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Depth = 1f, + Padding = new MarginPadding { Horizontal = -horizontal_inset }, + Margin = new MarginPadding { Top = ROW_HEIGHT } + }); + } + + protected override Drawable CreateHeader(int index, TableColumn column) => new HeaderText(column?.Header ?? string.Empty); + + private class HeaderText : OsuSpriteText + { + public HeaderText(string text) + { + Text = text.ToUpper(); + Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold); + } + } + } +} diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index a17b431fcc..75e2bb1f5c 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -20,47 +20,24 @@ using osuTK.Graphics; namespace osu.Game.Screens.Edit.Timing { - public class ControlPointTable : TableContainer + public class ControlPointTable : EditorTable { - private const float horizontal_inset = 20; - private const float row_height = 25; - private const int text_size = 14; - - private readonly FillFlowContainer backgroundFlow; - [Resolved] private Bindable selectedGroup { get; set; } - public ControlPointTable() - { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - - Padding = new MarginPadding { Horizontal = horizontal_inset }; - RowSize = new Dimension(GridSizeMode.Absolute, row_height); - - AddInternal(backgroundFlow = new FillFlowContainer - { - RelativeSizeAxes = Axes.Both, - Depth = 1f, - Padding = new MarginPadding { Horizontal = -horizontal_inset }, - Margin = new MarginPadding { Top = row_height } - }); - } - public IEnumerable ControlGroups { set { Content = null; - backgroundFlow.Clear(); + BackgroundFlow.Clear(); if (value?.Any() != true) return; foreach (var group in value) { - backgroundFlow.Add(new RowBackground(group)); + BackgroundFlow.Add(new RowBackground(group)); } Columns = createHeaders(); @@ -86,13 +63,13 @@ namespace osu.Game.Screens.Edit.Timing new OsuSpriteText { Text = $"#{index + 1}", - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold), Margin = new MarginPadding(10) }, new OsuSpriteText { Text = group.Time.ToEditorFormattedString(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold) + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold) }, null, new ControlGroupAttributes(group), @@ -164,17 +141,6 @@ namespace osu.Game.Screens.Edit.Timing } } - protected override Drawable CreateHeader(int index, TableColumn column) => new HeaderText(column?.Header ?? string.Empty); - - private class HeaderText : OsuSpriteText - { - public HeaderText(string text) - { - Text = text.ToUpper(); - Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold); - } - } - public class RowBackground : OsuClickableContainer { private readonly ControlPointGroup controlGroup; diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 042d6d84e3..00570d363b 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -19,47 +19,24 @@ using osuTK.Graphics; namespace osu.Game.Screens.Edit.Verify { - public class IssueTable : TableContainer + public class IssueTable : EditorTable { - private const float horizontal_inset = 20; - private const float row_height = 25; - private const int text_size = 14; - - private readonly FillFlowContainer backgroundFlow; - [Resolved] private Bindable selectedIssue { get; set; } - public IssueTable() - { - RelativeSizeAxes = Axes.X; - AutoSizeAxes = Axes.Y; - - Padding = new MarginPadding { Horizontal = horizontal_inset }; - RowSize = new Dimension(GridSizeMode.Absolute, row_height); - - AddInternal(backgroundFlow = new FillFlowContainer - { - RelativeSizeAxes = Axes.Both, - Depth = 1f, - Padding = new MarginPadding { Horizontal = -horizontal_inset }, - Margin = new MarginPadding { Top = row_height } - }); - } - public IEnumerable Issues { set { Content = null; - backgroundFlow.Clear(); + BackgroundFlow.Clear(); if (value == null) return; foreach (var issue in value) { - backgroundFlow.Add(new RowBackground(issue)); + BackgroundFlow.Add(new RowBackground(issue)); } Columns = createHeaders(); @@ -86,46 +63,35 @@ namespace osu.Game.Screens.Edit.Verify new OsuSpriteText { Text = $"#{index + 1}", - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium), + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Medium), Margin = new MarginPadding { Right = 10 } }, new OsuSpriteText { Text = issue.Template.Type.ToString(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold), Margin = new MarginPadding { Right = 10 }, Colour = issue.Template.Colour }, new OsuSpriteText { Text = issue.GetEditorTimestamp(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold), Margin = new MarginPadding { Right = 10 }, }, new OsuSpriteText { Text = issue.ToString(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Medium) + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Medium) }, new OsuSpriteText { Text = issue.Check.Metadata.Category.ToString(), - Font = OsuFont.GetFont(size: text_size, weight: FontWeight.Bold), + Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold), Margin = new MarginPadding(10) } }; - protected override Drawable CreateHeader(int index, TableColumn column) => new HeaderText(column?.Header ?? string.Empty); - - private class HeaderText : OsuSpriteText - { - public HeaderText(string text) - { - Text = text.ToUpper(); - Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold); - } - } - public class RowBackground : OsuClickableContainer { private readonly Issue issue; @@ -150,7 +116,7 @@ namespace osu.Game.Screens.Edit.Verify this.issue = issue; RelativeSizeAxes = Axes.X; - Height = row_height; + Height = ROW_HEIGHT; AlwaysPresent = true; CornerRadius = 3; Masking = true; From 21e8e5fbcacf799b0485ab0203e66473a5d6e881 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 23:26:19 +0900 Subject: [PATCH 54/55] Move common table layout logic into `EditorTable` abstract class --- osu.Game/Screens/Edit/EditorTable.cs | 95 ++++++++++- .../Screens/Edit/Timing/ControlPointTable.cs | 120 +++----------- osu.Game/Screens/Edit/Verify/IssueTable.cs | 154 +++++------------- 3 files changed, 152 insertions(+), 217 deletions(-) diff --git a/osu.Game/Screens/Edit/EditorTable.cs b/osu.Game/Screens/Edit/EditorTable.cs index e5e2add384..ef1c88db9a 100644 --- a/osu.Game/Screens/Edit/EditorTable.cs +++ b/osu.Game/Screens/Edit/EditorTable.cs @@ -1,10 +1,15 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Input.Events; using osu.Game.Graphics; +using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; +using osuTK.Graphics; namespace osu.Game.Screens.Edit { @@ -16,7 +21,7 @@ namespace osu.Game.Screens.Edit protected const int TEXT_SIZE = 14; - protected readonly FillFlowContainer BackgroundFlow; + protected readonly FillFlowContainer BackgroundFlow; protected EditorTable() { @@ -26,7 +31,7 @@ namespace osu.Game.Screens.Edit Padding = new MarginPadding { Horizontal = horizontal_inset }; RowSize = new Dimension(GridSizeMode.Absolute, ROW_HEIGHT); - AddInternal(BackgroundFlow = new FillFlowContainer + AddInternal(BackgroundFlow = new FillFlowContainer { RelativeSizeAxes = Axes.Both, Depth = 1f, @@ -45,5 +50,91 @@ namespace osu.Game.Screens.Edit Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold); } } + + public class RowBackground : OsuClickableContainer + { + public readonly object Item; + + private const int fade_duration = 100; + + private readonly Box hoveredBackground; + + [Resolved] + private EditorClock clock { get; set; } + + public RowBackground(object item) + { + Item = item; + + RelativeSizeAxes = Axes.X; + Height = 25; + + AlwaysPresent = true; + + CornerRadius = 3; + Masking = true; + + Children = new Drawable[] + { + hoveredBackground = new Box + { + RelativeSizeAxes = Axes.Both, + Alpha = 0, + }, + }; + + // todo delete + Action = () => + { + }; + } + + private Color4 colourHover; + private Color4 colourSelected; + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + hoveredBackground.Colour = colourHover = colours.BlueDarker; + colourSelected = colours.YellowDarker; + } + + private bool selected; + + public bool Selected + { + get => selected; + set + { + if (value == selected) + return; + + selected = value; + updateState(); + } + } + + protected override bool OnHover(HoverEvent e) + { + updateState(); + return base.OnHover(e); + } + + protected override void OnHoverLost(HoverLostEvent e) + { + updateState(); + base.OnHoverLost(e); + } + + private void updateState() + { + hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); + + if (selected || IsHovered) + hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); + else + hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); + } + } } } diff --git a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs index 75e2bb1f5c..dd51056bf1 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointTable.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointTable.cs @@ -8,12 +8,9 @@ using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Input.Events; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Extensions; using osu.Game.Graphics; -using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; using osuTK; using osuTK.Graphics; @@ -25,6 +22,9 @@ namespace osu.Game.Screens.Edit.Timing [Resolved] private Bindable selectedGroup { get; set; } + [Resolved] + private EditorClock clock { get; set; } + public IEnumerable ControlGroups { set @@ -37,7 +37,14 @@ namespace osu.Game.Screens.Edit.Timing foreach (var group in value) { - BackgroundFlow.Add(new RowBackground(group)); + BackgroundFlow.Add(new RowBackground(group) + { + Action = () => + { + selectedGroup.Value = group; + clock.SeekSmoothlyTo(group.Time); + } + }); } Columns = createHeaders(); @@ -45,6 +52,16 @@ namespace osu.Game.Screens.Edit.Timing } } + protected override void LoadComplete() + { + base.LoadComplete(); + + selectedGroup.BindValueChanged(group => + { + foreach (var b in BackgroundFlow) b.Selected = b.Item == group.NewValue; + }, true); + } + private TableColumn[] createHeaders() { var columns = new List @@ -140,100 +157,5 @@ namespace osu.Game.Screens.Edit.Timing return null; } } - - public class RowBackground : OsuClickableContainer - { - private readonly ControlPointGroup controlGroup; - private const int fade_duration = 100; - - private readonly Box hoveredBackground; - - [Resolved] - private EditorClock clock { get; set; } - - [Resolved] - private Bindable selectedGroup { get; set; } - - public RowBackground(ControlPointGroup controlGroup) - { - this.controlGroup = controlGroup; - RelativeSizeAxes = Axes.X; - Height = 25; - - AlwaysPresent = true; - - CornerRadius = 3; - Masking = true; - - Children = new Drawable[] - { - hoveredBackground = new Box - { - RelativeSizeAxes = Axes.Both, - Alpha = 0, - }, - }; - - Action = () => - { - selectedGroup.Value = controlGroup; - clock.SeekSmoothlyTo(controlGroup.Time); - }; - } - - private Color4 colourHover; - private Color4 colourSelected; - - [BackgroundDependencyLoader] - private void load(OsuColour colours) - { - hoveredBackground.Colour = colourHover = colours.BlueDarker; - colourSelected = colours.YellowDarker; - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - selectedGroup.BindValueChanged(group => { Selected = controlGroup == group.NewValue; }, true); - } - - private bool selected; - - protected bool Selected - { - get => selected; - set - { - if (value == selected) - return; - - selected = value; - updateState(); - } - } - - protected override bool OnHover(HoverEvent e) - { - updateState(); - return base.OnHover(e); - } - - protected override void OnHoverLost(HoverLostEvent e) - { - updateState(); - base.OnHoverLost(e); - } - - private void updateState() - { - hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); - - if (selected || IsHovered) - hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); - else - hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); - } - } } } diff --git a/osu.Game/Screens/Edit/Verify/IssueTable.cs b/osu.Game/Screens/Edit/Verify/IssueTable.cs index 00570d363b..44244028c9 100644 --- a/osu.Game/Screens/Edit/Verify/IssueTable.cs +++ b/osu.Game/Screens/Edit/Verify/IssueTable.cs @@ -8,14 +8,10 @@ using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Input.Events; using osu.Game.Graphics; -using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; using osu.Game.Input.Bindings; using osu.Game.Rulesets.Edit.Checks.Components; -using osuTK.Graphics; namespace osu.Game.Screens.Edit.Verify { @@ -24,6 +20,15 @@ namespace osu.Game.Screens.Edit.Verify [Resolved] private Bindable selectedIssue { get; set; } + [Resolved] + private EditorClock clock { get; set; } + + [Resolved] + private EditorBeatmap editorBeatmap { get; set; } + + [Resolved] + private Editor editor { get; set; } + public IEnumerable Issues { set @@ -36,7 +41,25 @@ namespace osu.Game.Screens.Edit.Verify foreach (var issue in value) { - BackgroundFlow.Add(new RowBackground(issue)); + BackgroundFlow.Add(new RowBackground(issue) + { + Action = () => + { + selectedIssue.Value = issue; + + if (issue.Time != null) + { + clock.Seek(issue.Time.Value); + editor.OnPressed(GlobalAction.EditorComposeMode); + } + + if (!issue.HitObjects.Any()) + return; + + editorBeatmap.SelectedHitObjects.Clear(); + editorBeatmap.SelectedHitObjects.AddRange(issue.HitObjects); + }, + }); } Columns = createHeaders(); @@ -44,6 +67,16 @@ namespace osu.Game.Screens.Edit.Verify } } + protected override void LoadComplete() + { + base.LoadComplete(); + + selectedIssue.BindValueChanged(issue => + { + foreach (var b in BackgroundFlow) b.Selected = b.Item == issue.NewValue; + }, true); + } + private TableColumn[] createHeaders() { var columns = new List @@ -91,116 +124,5 @@ namespace osu.Game.Screens.Edit.Verify Margin = new MarginPadding(10) } }; - - public class RowBackground : OsuClickableContainer - { - private readonly Issue issue; - private const int fade_duration = 100; - - private readonly Box hoveredBackground; - - [Resolved] - private EditorClock clock { get; set; } - - [Resolved] - private Editor editor { get; set; } - - [Resolved] - private EditorBeatmap editorBeatmap { get; set; } - - [Resolved] - private Bindable selectedIssue { get; set; } - - public RowBackground(Issue issue) - { - this.issue = issue; - - RelativeSizeAxes = Axes.X; - Height = ROW_HEIGHT; - AlwaysPresent = true; - CornerRadius = 3; - Masking = true; - - Children = new Drawable[] - { - hoveredBackground = new Box - { - RelativeSizeAxes = Axes.Both, - Alpha = 0, - }, - }; - - Action = () => - { - selectedIssue.Value = issue; - - if (issue.Time != null) - { - clock.Seek(issue.Time.Value); - editor.OnPressed(GlobalAction.EditorComposeMode); - } - - if (!issue.HitObjects.Any()) - return; - - editorBeatmap.SelectedHitObjects.Clear(); - editorBeatmap.SelectedHitObjects.AddRange(issue.HitObjects); - }; - } - - private Color4 colourHover; - private Color4 colourSelected; - - [BackgroundDependencyLoader] - private void load(OsuColour colours) - { - hoveredBackground.Colour = colourHover = colours.BlueDarker; - colourSelected = colours.YellowDarker; - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - selectedIssue.BindValueChanged(change => { Selected = issue == change.NewValue; }, true); - } - - private bool selected; - - protected bool Selected - { - get => selected; - set - { - if (value == selected) - return; - - selected = value; - updateState(); - } - } - - protected override bool OnHover(HoverEvent e) - { - updateState(); - return base.OnHover(e); - } - - protected override void OnHoverLost(HoverLostEvent e) - { - updateState(); - base.OnHoverLost(e); - } - - private void updateState() - { - hoveredBackground.FadeColour(selected ? colourSelected : colourHover, 450, Easing.OutQuint); - - if (selected || IsHovered) - hoveredBackground.FadeIn(fade_duration, Easing.OutQuint); - else - hoveredBackground.FadeOut(fade_duration, Easing.OutQuint); - } - } } } From cb4f64133eedde15134e64b9b0a0c584256df3aa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Apr 2021 23:30:20 +0900 Subject: [PATCH 55/55] Add xmldoc to interfaces --- osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs | 3 +++ osu.Game/Rulesets/Edit/IBeatmapVerifier.cs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs b/osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs index f355ae734e..f284240092 100644 --- a/osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs +++ b/osu.Game/Rulesets/Edit/Checks/Components/ICheck.cs @@ -6,6 +6,9 @@ using osu.Game.Beatmaps; namespace osu.Game.Rulesets.Edit.Checks.Components { + /// + /// A specific check that can be run on a beatmap to verify or find issues. + /// public interface ICheck { /// diff --git a/osu.Game/Rulesets/Edit/IBeatmapVerifier.cs b/osu.Game/Rulesets/Edit/IBeatmapVerifier.cs index 2bafacefa3..61d8119635 100644 --- a/osu.Game/Rulesets/Edit/IBeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/IBeatmapVerifier.cs @@ -7,6 +7,9 @@ using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Edit { + /// + /// A class which can run against a beatmap and surface issues to the user which could go against known criteria or hinder gameplay. + /// public interface IBeatmapVerifier { public IEnumerable Run(IBeatmap beatmap);