From e0a97cfac533487450b8067656674858cef958a0 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 20 Sep 2019 18:35:15 +0900 Subject: [PATCH 01/16] Add a LabelledComponent base class --- .../TestSceneLabelledComponent.cs | 89 +++++++++++++ .../LabelledComponents/LabelledComponent.cs | 121 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneLabelledComponent.cs create mode 100644 osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledComponent.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledComponent.cs new file mode 100644 index 0000000000..73e0191adb --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledComponent.cs @@ -0,0 +1,89 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics.Sprites; +using osu.Game.Screens.Edit.Setup.Components.LabelledComponents; +using osuTK.Graphics; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public class TestSceneLabelledComponent : OsuTestScene + { + [TestCase(false)] + [TestCase(true)] + public void TestPadded(bool hasDescription) => createPaddedComponent(hasDescription); + + [TestCase(false)] + [TestCase(true)] + public void TestNonPadded(bool hasDescription) => createPaddedComponent(hasDescription, false); + + private void createPaddedComponent(bool hasDescription = false, bool padded = true) + { + AddStep("create component", () => + { + LabelledComponent component; + + Child = new Container + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Width = 500, + AutoSizeAxes = Axes.Y, + Child = component = padded ? (LabelledComponent)new PaddedLabelledComponent() : new NonPaddedLabelledComponent(), + }; + + component.Label = "a sample component"; + component.Description = hasDescription ? "this text describes the component" : string.Empty; + }); + } + + private class PaddedLabelledComponent : LabelledComponent + { + public PaddedLabelledComponent() + : base(true) + { + } + + protected override Drawable CreateComponent() => new OsuSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Colour = Color4.Red, + Text = @"(( Component ))" + }; + } + + private class NonPaddedLabelledComponent : LabelledComponent + { + public NonPaddedLabelledComponent() + : base(false) + { + } + + protected override Drawable CreateComponent() => new Container + { + RelativeSizeAxes = Axes.X, + Height = 40, + Children = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = Color4.SlateGray + }, + new OsuSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Colour = Color4.Red, + Text = @"(( Component ))" + } + } + }; + } + } +} diff --git a/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs new file mode 100644 index 0000000000..b4c91d6d57 --- /dev/null +++ b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs @@ -0,0 +1,121 @@ +// 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; +using osuTK; + +namespace osu.Game.Screens.Edit.Setup.Components.LabelledComponents +{ + public abstract class LabelledComponent : CompositeDrawable + { + protected const float CONTENT_PADDING_VERTICAL = 10; + protected const float CONTENT_PADDING_HORIZONTAL = 15; + protected const float CORNER_RADIUS = 15; + + protected readonly Drawable Component; + + private readonly OsuTextFlowContainer labelText; + private readonly OsuTextFlowContainer descriptionText; + + protected LabelledComponent(bool padded) + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + CornerRadius = CORNER_RADIUS; + Masking = true; + + InternalChildren = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = OsuColour.FromHex("1c2125"), + }, + new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Padding = padded + ? new MarginPadding { Horizontal = CONTENT_PADDING_HORIZONTAL, Vertical = CONTENT_PADDING_VERTICAL } + : new MarginPadding { Left = CONTENT_PADDING_HORIZONTAL }, + Spacing = new Vector2(0, 12), + Children = new Drawable[] + { + new GridContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Content = new[] + { + new Drawable[] + { + labelText = new OsuTextFlowContainer(s => s.Font = OsuFont.GetFont(weight: FontWeight.Bold)) + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + AutoSizeAxes = Axes.Both, + Padding = new MarginPadding { Right = 20 } + }, + new Container + { + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Child = Component = CreateComponent().With(d => + { + d.Anchor = Anchor.CentreRight; + d.Origin = Anchor.CentreRight; + }) + } + }, + }, + RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) }, + ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) } + }, + descriptionText = new OsuTextFlowContainer(s => s.Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold, italics: true)) + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Padding = new MarginPadding { Bottom = padded ? 0 : CONTENT_PADDING_VERTICAL }, + Alpha = 0, + } + } + } + }; + } + + [BackgroundDependencyLoader] + private void load(OsuColour osuColour) + { + descriptionText.Colour = osuColour.Yellow; + } + + public string Label + { + set => labelText.Text = value; + } + + public string Description + { + set + { + descriptionText.Text = value; + + if (!string.IsNullOrEmpty(value)) + descriptionText.Show(); + else + descriptionText.Hide(); + } + } + + protected abstract Drawable CreateComponent(); + } +} From 523272edab145dbd08b40980ca71a9aba16fe9d6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 20 Sep 2019 18:39:04 +0900 Subject: [PATCH 02/16] Add xmldocs --- .../LabelledComponents/LabelledComponent.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs index b4c91d6d57..19e9c329d6 100644 --- a/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs +++ b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledComponent.cs @@ -17,11 +17,18 @@ namespace osu.Game.Screens.Edit.Setup.Components.LabelledComponents protected const float CONTENT_PADDING_HORIZONTAL = 15; protected const float CORNER_RADIUS = 15; + /// + /// The component that is being displayed. + /// protected readonly Drawable Component; private readonly OsuTextFlowContainer labelText; private readonly OsuTextFlowContainer descriptionText; + /// + /// Creates a new . + /// + /// Whether the component should be padded or should be expanded to the bounds of this . protected LabelledComponent(bool padded) { RelativeSizeAxes = Axes.X; @@ -116,6 +123,10 @@ namespace osu.Game.Screens.Edit.Setup.Components.LabelledComponents } } + /// + /// Creates the component that should be displayed. + /// + /// The component. protected abstract Drawable CreateComponent(); } } From 33c51d5178fc5191d143fd77407042e5dc6b6687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 20:55:30 +0200 Subject: [PATCH 03/16] Extract parsing filter queries to class For the sake of testability without having to spin up visual tests, extract methods related to parsing filter queries from FilterControl to a static FilterQueryParser class. --- osu.Game/Screens/Select/FilterControl.cs | 131 +----------------- osu.Game/Screens/Select/FilterQueryParser.cs | 138 +++++++++++++++++++ 2 files changed, 139 insertions(+), 130 deletions(-) create mode 100644 osu.Game/Screens/Select/FilterQueryParser.cs diff --git a/osu.Game/Screens/Select/FilterControl.cs b/osu.Game/Screens/Select/FilterControl.cs index e3c23f7e22..91f1ca0307 100644 --- a/osu.Game/Screens/Select/FilterControl.cs +++ b/osu.Game/Screens/Select/FilterControl.cs @@ -16,8 +16,6 @@ using Container = osu.Framework.Graphics.Containers.Container; using osu.Framework.Graphics.Shapes; using osu.Game.Configuration; using osu.Game.Rulesets; -using System.Text.RegularExpressions; -using osu.Game.Beatmaps; namespace osu.Game.Screens.Select { @@ -47,10 +45,7 @@ namespace osu.Game.Screens.Select Ruleset = ruleset.Value }; - applyQueries(criteria, ref query); - - criteria.SearchText = query; - + FilterQueryParser.ApplyQueries(criteria, query); return criteria; } @@ -181,129 +176,5 @@ namespace osu.Game.Screens.Select } private void updateCriteria() => FilterChanged?.Invoke(CreateCriteria()); - - private static readonly Regex query_syntax_regex = new Regex( - @"\b(?stars|ar|dr|cs|divisor|length|objects|bpm|status)(?[=:><]+)(?\S*)", - RegexOptions.Compiled | RegexOptions.IgnoreCase); - - private void applyQueries(FilterCriteria criteria, ref string query) - { - foreach (Match match in query_syntax_regex.Matches(query)) - { - var key = match.Groups["key"].Value.ToLower(); - var op = match.Groups["op"].Value; - var value = match.Groups["value"].Value; - - switch (key) - { - case "stars" when float.TryParse(value, out var stars): - updateCriteriaRange(ref criteria.StarDifficulty, op, stars); - break; - - case "ar" when float.TryParse(value, out var ar): - updateCriteriaRange(ref criteria.ApproachRate, op, ar); - break; - - case "dr" when float.TryParse(value, out var dr): - updateCriteriaRange(ref criteria.DrainRate, op, dr); - break; - - case "cs" when float.TryParse(value, out var cs): - updateCriteriaRange(ref criteria.CircleSize, op, cs); - break; - - case "bpm" when double.TryParse(value, out var bpm): - updateCriteriaRange(ref criteria.BPM, op, bpm); - break; - - case "length" when double.TryParse(value.TrimEnd('m', 's', 'h'), out var length): - var scale = - value.EndsWith("ms") ? 1 : - value.EndsWith("s") ? 1000 : - value.EndsWith("m") ? 60000 : - value.EndsWith("h") ? 3600000 : 1000; - - updateCriteriaRange(ref criteria.Length, op, length * scale, scale / 2.0); - break; - - case "divisor" when int.TryParse(value, out var divisor): - updateCriteriaRange(ref criteria.BeatDivisor, op, divisor); - break; - - case "status" when Enum.TryParse(value, true, out var statusValue): - updateCriteriaRange(ref criteria.OnlineStatus, op, statusValue); - break; - } - - query = query.Replace(match.ToString(), ""); - } - } - - private void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, float value, float tolerance = 0.05f) - { - updateCriteriaRange(ref range, op, value); - - switch (op) - { - case "=": - case ":": - range.Min = value - tolerance; - range.Max = value + tolerance; - break; - } - } - - private void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, double value, double tolerance = 0.05) - { - updateCriteriaRange(ref range, op, value); - - switch (op) - { - case "=": - case ":": - range.Min = value - tolerance; - range.Max = value + tolerance; - break; - } - } - - private void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, T value) - where T : struct, IComparable - { - switch (op) - { - default: - return; - - case "=": - case ":": - range.IsInclusive = true; - range.Min = value; - range.Max = value; - break; - - case ">": - range.IsInclusive = false; - range.Min = value; - break; - - case ">=": - case ">:": - range.IsInclusive = true; - range.Min = value; - break; - - case "<": - range.IsInclusive = false; - range.Max = value; - break; - - case "<=": - case "<:": - range.IsInclusive = true; - range.Max = value; - break; - } - } } } diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs new file mode 100644 index 0000000000..4e2b591fc9 --- /dev/null +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -0,0 +1,138 @@ +// 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.Text.RegularExpressions; +using osu.Game.Beatmaps; + +namespace osu.Game.Screens.Select +{ + internal static class FilterQueryParser + { + private static readonly Regex query_syntax_regex = new Regex( + @"\b(?stars|ar|dr|cs|divisor|length|objects|bpm|status)(?[=:><]+)(?\S*)", + RegexOptions.Compiled | RegexOptions.IgnoreCase); + + internal static void ApplyQueries(FilterCriteria criteria, string query) + { + foreach (Match match in query_syntax_regex.Matches(query)) + { + var key = match.Groups["key"].Value.ToLower(); + var op = match.Groups["op"].Value; + var value = match.Groups["value"].Value; + + switch (key) + { + case "stars" when float.TryParse(value, out var stars): + updateCriteriaRange(ref criteria.StarDifficulty, op, stars); + break; + + case "ar" when float.TryParse(value, out var ar): + updateCriteriaRange(ref criteria.ApproachRate, op, ar); + break; + + case "dr" when float.TryParse(value, out var dr): + updateCriteriaRange(ref criteria.DrainRate, op, dr); + break; + + case "cs" when float.TryParse(value, out var cs): + updateCriteriaRange(ref criteria.CircleSize, op, cs); + break; + + case "bpm" when double.TryParse(value, out var bpm): + updateCriteriaRange(ref criteria.BPM, op, bpm); + break; + + case "length" when double.TryParse(value.TrimEnd('m', 's', 'h'), out var length): + var scale = + value.EndsWith("ms") ? 1 : + value.EndsWith("s") ? 1000 : + value.EndsWith("m") ? 60000 : + value.EndsWith("h") ? 3600000 : 1000; + + updateCriteriaRange(ref criteria.Length, op, length * scale, scale / 2.0); + break; + + case "divisor" when int.TryParse(value, out var divisor): + updateCriteriaRange(ref criteria.BeatDivisor, op, divisor); + break; + + case "status" when Enum.TryParse(value, true, out var statusValue): + updateCriteriaRange(ref criteria.OnlineStatus, op, statusValue); + break; + } + + query = query.Replace(match.ToString(), ""); + } + + criteria.SearchText = query; + } + + private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, float value, float tolerance = 0.05f) + { + updateCriteriaRange(ref range, op, value); + + switch (op) + { + case "=": + case ":": + range.Min = value - tolerance; + range.Max = value + tolerance; + break; + } + } + + private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, double value, double tolerance = 0.05) + { + updateCriteriaRange(ref range, op, value); + + switch (op) + { + case "=": + case ":": + range.Min = value - tolerance; + range.Max = value + tolerance; + break; + } + } + + private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, T value) + where T : struct, IComparable + { + switch (op) + { + default: + return; + + case "=": + case ":": + range.IsInclusive = true; + range.Min = value; + range.Max = value; + break; + + case ">": + range.IsInclusive = false; + range.Min = value; + break; + + case ">=": + case ">:": + range.IsInclusive = true; + range.Min = value; + break; + + case "<": + range.IsInclusive = false; + range.Max = value; + break; + + case "<=": + case "<:": + range.IsInclusive = true; + range.Max = value; + break; + } + } + } +} From dddd94684bd233e1377211d086e0f47c736fb934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 21:34:38 +0200 Subject: [PATCH 04/16] Split out lower and upper interval inclusivity A single IsInclusive field causes unexpected issues when trying to formulate a half-open interval query. Split out IsInclusive into two fields, Is{Lower,Upper}Inclusive and update usages accordingly. --- osu.Game/Screens/Select/FilterCriteria.cs | 10 ++++++---- osu.Game/Screens/Select/FilterQueryParser.cs | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index a3fa1b10ca..97a7f12724 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Select if (comparison < 0) return false; - if (comparison == 0 && !IsInclusive) + if (comparison == 0 && !IsLowerInclusive) return false; } @@ -64,7 +64,7 @@ namespace osu.Game.Screens.Select if (comparison > 0) return false; - if (comparison == 0 && !IsInclusive) + if (comparison == 0 && !IsUpperInclusive) return false; } @@ -73,12 +73,14 @@ namespace osu.Game.Screens.Select public T? Min; public T? Max; - public bool IsInclusive; + public bool IsLowerInclusive; + public bool IsUpperInclusive; public bool Equals(OptionalRange other) => Min.Equals(other.Min) && Max.Equals(other.Max) - && IsInclusive.Equals(other.IsInclusive); + && IsLowerInclusive.Equals(other.IsLowerInclusive) + && IsUpperInclusive.Equals(other.IsUpperInclusive); } } } diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 4e2b591fc9..800f1afd03 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -106,30 +106,30 @@ namespace osu.Game.Screens.Select case "=": case ":": - range.IsInclusive = true; + range.IsLowerInclusive = range.IsUpperInclusive = true; range.Min = value; range.Max = value; break; case ">": - range.IsInclusive = false; + range.IsLowerInclusive = false; range.Min = value; break; case ">=": case ">:": - range.IsInclusive = true; + range.IsLowerInclusive = true; range.Min = value; break; case "<": - range.IsInclusive = false; + range.IsUpperInclusive = false; range.Max = value; break; case "<=": case "<:": - range.IsInclusive = true; + range.IsUpperInclusive = true; range.Max = value; break; } From f5f5094611257f50fc778becf402105868e9757f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 22:10:46 +0200 Subject: [PATCH 05/16] Take culture into account when parsing filters Culture was not taken into account when parsing filters, which meant that in cultures that use the comma (,) as a decimal delimiter, it would conflict with the comma used to delimit search criteria. To remove any ambiguity, introduce local helper functions that allow the decimal point to be utilised, using the invariant culture. This also matches stable behaviour. The decision to not reuse osu.Game.Beatmaps.Formats.Parsing was deliberate due to differing semantics (it's not really sane to throw exceptions on receiving user-facing input). --- osu.Game/Screens/Select/FilterQueryParser.cs | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 800f1afd03..d6d19c8650 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Globalization; using System.Text.RegularExpressions; using osu.Game.Beatmaps; @@ -23,27 +24,27 @@ namespace osu.Game.Screens.Select switch (key) { - case "stars" when float.TryParse(value, out var stars): + case "stars" when parseFloatWithPoint(value, out var stars): updateCriteriaRange(ref criteria.StarDifficulty, op, stars); break; - case "ar" when float.TryParse(value, out var ar): + case "ar" when parseFloatWithPoint(value, out var ar): updateCriteriaRange(ref criteria.ApproachRate, op, ar); break; - case "dr" when float.TryParse(value, out var dr): + case "dr" when parseFloatWithPoint(value, out var dr): updateCriteriaRange(ref criteria.DrainRate, op, dr); break; - case "cs" when float.TryParse(value, out var cs): + case "cs" when parseFloatWithPoint(value, out var cs): updateCriteriaRange(ref criteria.CircleSize, op, cs); break; - case "bpm" when double.TryParse(value, out var bpm): + case "bpm" when parseDoubleWithPoint(value, out var bpm): updateCriteriaRange(ref criteria.BPM, op, bpm); break; - case "length" when double.TryParse(value.TrimEnd('m', 's', 'h'), out var length): + case "length" when parseDoubleWithPoint(value.TrimEnd('m', 's', 'h'), out var length): var scale = value.EndsWith("ms") ? 1 : value.EndsWith("s") ? 1000 : @@ -53,7 +54,7 @@ namespace osu.Game.Screens.Select updateCriteriaRange(ref criteria.Length, op, length * scale, scale / 2.0); break; - case "divisor" when int.TryParse(value, out var divisor): + case "divisor" when parseInt(value, out var divisor): updateCriteriaRange(ref criteria.BeatDivisor, op, divisor); break; @@ -68,6 +69,15 @@ namespace osu.Game.Screens.Select criteria.SearchText = query; } + private static bool parseFloatWithPoint(string value, out float result) => + float.TryParse(value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out result); + + private static bool parseDoubleWithPoint(string value, out double result) => + double.TryParse(value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out result); + + private static bool parseInt(string value, out int result) => + int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out result); + private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, float value, float tolerance = 0.05f) { updateCriteriaRange(ref range, op, value); From d11d932a8747273f56f4128dbcf8b31bd6a329c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 22:19:45 +0200 Subject: [PATCH 06/16] Add filter parsing tests Introduce unit tests covering parsing for the originally introduced filtering features. The introduced improvements (lower and upper interval and decimal point support) also tested. --- .../Filtering/FilterQueryParserTest.cs | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs new file mode 100644 index 0000000000..f98ad1fc43 --- /dev/null +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -0,0 +1,129 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Screens.Select; + +namespace osu.Game.Tests.NonVisual.Filtering +{ + [TestFixture] + public class FilterQueryParserTest + { + [Test] + public void TestApplyQueriesBareWords() + { + const string query = "looking for a beatmap"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("looking for a beatmap", filterCriteria.SearchText); + Assert.AreEqual(4, filterCriteria.SearchTerms.Length); + } + + [Test] + public void TestApplyStarQueries() + { + const string query = "stars<4 easy"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("easy", filterCriteria.SearchText.Trim()); + Assert.AreEqual(1, filterCriteria.SearchTerms.Length); + Assert.AreEqual(4.0f, filterCriteria.StarDifficulty.Max); + Assert.IsFalse(filterCriteria.StarDifficulty.IsUpperInclusive); + Assert.IsNull(filterCriteria.StarDifficulty.Min); + } + + [Test] + public void TestApplyApproachRateQueries() + { + const string query = "ar>=9 difficult"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("difficult", filterCriteria.SearchText.Trim()); + Assert.AreEqual(1, filterCriteria.SearchTerms.Length); + Assert.AreEqual(9.0f, filterCriteria.ApproachRate.Min); + Assert.IsTrue(filterCriteria.ApproachRate.IsLowerInclusive); + Assert.IsNull(filterCriteria.ApproachRate.Max); + } + + [Test] + public void TestApplyDrainRateQueries() + { + const string query = "dr>2 quite specific dr<:6"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("quite specific", filterCriteria.SearchText.Trim()); + Assert.AreEqual(2, filterCriteria.SearchTerms.Length); + Assert.AreEqual(2.0f, filterCriteria.DrainRate.Min); + Assert.IsFalse(filterCriteria.DrainRate.IsLowerInclusive); + Assert.AreEqual(6.0f, filterCriteria.DrainRate.Max); + Assert.IsTrue(filterCriteria.DrainRate.IsUpperInclusive); + } + + [Test] + public void TestApplyBPMQueries() + { + const string query = "bpm>:200 gotta go fast"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("gotta go fast", filterCriteria.SearchText.Trim()); + Assert.AreEqual(3, filterCriteria.SearchTerms.Length); + Assert.AreEqual(200d, filterCriteria.BPM.Min); + Assert.IsTrue(filterCriteria.BPM.IsLowerInclusive); + Assert.IsNull(filterCriteria.BPM.Max); + } + + private static object[] lengthQueryExamples = + { + new object[] { "6ms", TimeSpan.FromMilliseconds(6), TimeSpan.FromMilliseconds(1) }, + new object[] { "23s", TimeSpan.FromSeconds(23), TimeSpan.FromSeconds(1) }, + new object[] { "9m", TimeSpan.FromMinutes(9), TimeSpan.FromMinutes(1) }, + new object[] { "0.25h", TimeSpan.FromHours(0.25), TimeSpan.FromHours(1) }, + new object[] { "70", TimeSpan.FromSeconds(70), TimeSpan.FromSeconds(1) }, + }; + + [Test] + [TestCaseSource(nameof(lengthQueryExamples))] + public void TestApplyLengthQueries(string lengthQuery, TimeSpan expectedLength, TimeSpan scale) + { + string query = $"length={lengthQuery} time"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("time", filterCriteria.SearchText.Trim()); + Assert.AreEqual(1, filterCriteria.SearchTerms.Length); + Assert.AreEqual(expectedLength.TotalMilliseconds - scale.TotalMilliseconds / 2.0, filterCriteria.Length.Min); + Assert.IsTrue(filterCriteria.Length.IsLowerInclusive); + Assert.AreEqual(expectedLength.TotalMilliseconds + scale.TotalMilliseconds / 2.0, filterCriteria.Length.Max); + Assert.IsTrue(filterCriteria.Length.IsUpperInclusive); + } + + [Test] + public void TestApplyDivisorQueries() + { + const string query = "that's a time signature alright! divisor:12"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("that's a time signature alright!", filterCriteria.SearchText.Trim()); + Assert.AreEqual(5, filterCriteria.SearchTerms.Length); + Assert.AreEqual(12, filterCriteria.BeatDivisor.Min); + Assert.IsTrue(filterCriteria.BeatDivisor.IsLowerInclusive); + Assert.AreEqual(12, filterCriteria.BeatDivisor.Max); + Assert.IsTrue(filterCriteria.BeatDivisor.IsUpperInclusive); + } + + [Test] + public void TestApplyStatusQueries() + { + const string query = "I want the pp status=ranked"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("I want the pp", filterCriteria.SearchText.Trim()); + Assert.AreEqual(4, filterCriteria.SearchTerms.Length); + Assert.AreEqual(BeatmapSetOnlineStatus.Ranked, filterCriteria.OnlineStatus.Min); + Assert.IsTrue(filterCriteria.OnlineStatus.IsLowerInclusive); + Assert.AreEqual(BeatmapSetOnlineStatus.Ranked, filterCriteria.OnlineStatus.Max); + Assert.IsTrue(filterCriteria.OnlineStatus.IsUpperInclusive); + } + } +} From 41569fd2b63f82aa5a19f4f026120d0d2f2717ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 22:48:30 +0200 Subject: [PATCH 07/16] Add filter evaluating unit tests Introduce unit tests covering the actual evaluation of filters for beatmaps. Partially covers most scenarios. --- .../NonVisual/Filtering/FilterMatchingTest.cs | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs new file mode 100644 index 0000000000..24e735310d --- /dev/null +++ b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs @@ -0,0 +1,136 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Carousel; + +namespace osu.Game.Tests.NonVisual.Filtering +{ + [TestFixture] + public class FilterMatchingTest + { + private readonly BeatmapInfo exampleBeatmapInfo = new BeatmapInfo + { + Ruleset = new RulesetInfo { ID = 5 }, + StarDifficulty = 4.0d, + BaseDifficulty = new BeatmapDifficulty + { + ApproachRate = 5.0f, + DrainRate = 3.0f, + CircleSize = 2.0f, + }, + Metadata = new BeatmapMetadata + { + Artist = "The Artist", + ArtistUnicode = "The Artist", + Title = "Title goes here", + TitleUnicode = "Title goes here", + AuthorString = "Author", + Source = "unit tests", + Tags = "look for tags too", + }, + Version = "version as well", + Length = 2500, + BPM = 160, + BeatDivisor = 12, + Status = BeatmapSetOnlineStatus.Loved + }; + + [Test] + public void TestCriteriaMatchingNoRuleset() + { + var criteria = new FilterCriteria(); + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.IsFalse(carouselItem.Filtered.Value); + } + + [Test] + public void TestCriteriaMatchingSpecificRuleset() + { + var criteria = new FilterCriteria + { + Ruleset = new RulesetInfo { ID = 6 } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.IsTrue(carouselItem.Filtered.Value); + } + + [Test] + public void TestCriteriaMatchingConvertedBeatmaps() + { + var criteria = new FilterCriteria + { + Ruleset = new RulesetInfo { ID = 6 }, + AllowConvertedBeatmaps = true + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.IsFalse(carouselItem.Filtered.Value); + } + + [Test] + [TestCase(true)] + [TestCase(false)] + public void TestCriteriaMatchingRangeMin(bool inclusive) + { + var criteria = new FilterCriteria + { + Ruleset = new RulesetInfo { ID = 6 }, + AllowConvertedBeatmaps = true, + ApproachRate = new FilterCriteria.OptionalRange + { + IsLowerInclusive = inclusive, + Min = 5.0f + } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(!inclusive, carouselItem.Filtered.Value); + } + + [Test] + [TestCase(true)] + [TestCase(false)] + public void TestCriteriaMatchingRangeMax(bool inclusive) + { + var criteria = new FilterCriteria + { + Ruleset = new RulesetInfo { ID = 6 }, + AllowConvertedBeatmaps = true, + BPM = new FilterCriteria.OptionalRange + { + IsUpperInclusive = inclusive, + Max = 160d + } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(!inclusive, carouselItem.Filtered.Value); + } + + [Test] + [TestCase("artist", false)] + [TestCase("artist title author", false)] + [TestCase("an artist", true)] + [TestCase("tags too", false)] + [TestCase("version", false)] + [TestCase("an auteur", true)] + public void TestCriteriaMatchingTerms(string terms, bool filtered) + { + var criteria = new FilterCriteria + { + Ruleset = new RulesetInfo { ID = 6 }, + AllowConvertedBeatmaps = true, + SearchText = terms + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(filtered, carouselItem.Filtered.Value); + } + } +} From 51509f6be03523771e61269311086d6fdb49c7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Sep 2019 23:06:20 +0200 Subject: [PATCH 08/16] Add filter steps to carousel visual test Just a couple of steps for added coverage in visual tests. Very on-the-surface, the unit tests are supposed to cover the gory details. --- .../Visual/SongSelect/TestSceneBeatmapCarousel.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 6669ec7da3..71399106f4 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -242,6 +242,21 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); AddAssert("Selection is non-null", () => currentSelection != null); + + setSelected(1, 3); + AddStep("Apply a range filter", () => carousel.Filter(new FilterCriteria + { + SearchText = "#3", + StarDifficulty = new FilterCriteria.OptionalRange + { + Min = 2, + Max = 5.5, + IsLowerInclusive = true + } + }, false)); + checkSelected(3, 2); + + AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); } /// From b262ba13cd5d2ea1c9a908bd542f7c28758299fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 21 Sep 2019 23:16:23 +0200 Subject: [PATCH 09/16] Add creator= and artist= filters To match stable, add creator= and artist= filters to the beatmap carousel on song select screen. Contrary to stable, this implementation supports phrase queries with spaces within using double quotes. The quote handling is not entirely correct (can't nest), but quotes should rarely happen within names, and it is an edge case of an edge case - leaving best-effort as is. Test coverage also included. --- .../NonVisual/Filtering/FilterMatchingTest.cs | 71 ++++++++++++++++++- .../Filtering/FilterQueryParserTest.cs | 44 ++++++++++++ .../Select/Carousel/CarouselBeatmap.cs | 4 ++ osu.Game/Screens/Select/FilterCriteria.cs | 21 ++++++ osu.Game/Screens/Select/FilterQueryParser.cs | 21 +++++- 5 files changed, 157 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs index 24e735310d..30686cb947 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs @@ -12,7 +12,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestFixture] public class FilterMatchingTest { - private readonly BeatmapInfo exampleBeatmapInfo = new BeatmapInfo + private BeatmapInfo getExampleBeatmap() => new BeatmapInfo { Ruleset = new RulesetInfo { ID = 5 }, StarDifficulty = 4.0d, @@ -25,10 +25,10 @@ namespace osu.Game.Tests.NonVisual.Filtering Metadata = new BeatmapMetadata { Artist = "The Artist", - ArtistUnicode = "The Artist", + ArtistUnicode = "check unicode too", Title = "Title goes here", TitleUnicode = "Title goes here", - AuthorString = "Author", + AuthorString = "The Author", Source = "unit tests", Tags = "look for tags too", }, @@ -42,6 +42,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [Test] public void TestCriteriaMatchingNoRuleset() { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria(); var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); carouselItem.Filter(criteria); @@ -51,6 +52,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [Test] public void TestCriteriaMatchingSpecificRuleset() { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria { Ruleset = new RulesetInfo { ID = 6 } @@ -63,6 +65,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [Test] public void TestCriteriaMatchingConvertedBeatmaps() { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria { Ruleset = new RulesetInfo { ID = 6 }, @@ -78,6 +81,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase(false)] public void TestCriteriaMatchingRangeMin(bool inclusive) { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria { Ruleset = new RulesetInfo { ID = 6 }, @@ -98,6 +102,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase(false)] public void TestCriteriaMatchingRangeMax(bool inclusive) { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria { Ruleset = new RulesetInfo { ID = 6 }, @@ -122,6 +127,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("an auteur", true)] public void TestCriteriaMatchingTerms(string terms, bool filtered) { + var exampleBeatmapInfo = getExampleBeatmap(); var criteria = new FilterCriteria { Ruleset = new RulesetInfo { ID = 6 }, @@ -132,5 +138,64 @@ namespace osu.Game.Tests.NonVisual.Filtering carouselItem.Filter(criteria); Assert.AreEqual(filtered, carouselItem.Filtered.Value); } + + [Test] + [TestCase("", false)] + [TestCase("The", false)] + [TestCase("THE", false)] + [TestCase("author", false)] + [TestCase("the author", false)] + [TestCase("the author AND then something else", true)] + [TestCase("unknown", true)] + public void TestCriteriaMatchingCreator(string creatorName, bool filtered) + { + var exampleBeatmapInfo = getExampleBeatmap(); + var criteria = new FilterCriteria + { + Creator = new FilterCriteria.OptionalTextFilter { SearchTerm = creatorName } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(filtered, carouselItem.Filtered.Value); + } + + [Test] + [TestCase("", false)] + [TestCase("The", false)] + [TestCase("THE", false)] + [TestCase("artist", false)] + [TestCase("the artist", false)] + [TestCase("the artist AND then something else", true)] + [TestCase("unicode too", false)] + [TestCase("unknown", true)] + public void TestCriteriaMatchingArtist(string artistName, bool filtered) + { + var exampleBeatmapInfo = getExampleBeatmap(); + var criteria = new FilterCriteria + { + Artist = new FilterCriteria.OptionalTextFilter { SearchTerm = artistName } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(filtered, carouselItem.Filtered.Value); + } + + [Test] + [TestCase("", false)] + [TestCase("artist", false)] + [TestCase("unknown", true)] + public void TestCriteriaMatchingArtistWithNullUnicodeName(string artistName, bool filtered) + { + var exampleBeatmapInfo = getExampleBeatmap(); + exampleBeatmapInfo.Metadata.ArtistUnicode = null; + + var criteria = new FilterCriteria + { + Artist = new FilterCriteria.OptionalTextFilter { SearchTerm = artistName } + }; + var carouselItem = new CarouselBeatmap(exampleBeatmapInfo); + carouselItem.Filter(criteria); + Assert.AreEqual(filtered, carouselItem.Filtered.Value); + } } } diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index f98ad1fc43..daab690a84 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -125,5 +125,49 @@ namespace osu.Game.Tests.NonVisual.Filtering Assert.AreEqual(BeatmapSetOnlineStatus.Ranked, filterCriteria.OnlineStatus.Max); Assert.IsTrue(filterCriteria.OnlineStatus.IsUpperInclusive); } + + [Test] + public void TestApplyCreatorQueries() + { + const string query = "beatmap specifically by creator=my_fav"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("beatmap specifically by", filterCriteria.SearchText.Trim()); + Assert.AreEqual(3, filterCriteria.SearchTerms.Length); + Assert.AreEqual("my_fav", filterCriteria.Creator.SearchTerm); + } + + [Test] + public void TestApplyArtistQueries() + { + const string query = "find me songs by artist=singer please"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("find me songs by please", filterCriteria.SearchText.Trim()); + Assert.AreEqual(5, filterCriteria.SearchTerms.Length); + Assert.AreEqual("singer", filterCriteria.Artist.SearchTerm); + } + + [Test] + public void TestApplyArtistQueriesWithSpaces() + { + const string query = "really like artist=\"name with space\" yes"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("really like yes", filterCriteria.SearchText.Trim()); + Assert.AreEqual(3, filterCriteria.SearchTerms.Length); + Assert.AreEqual("name with space", filterCriteria.Artist.SearchTerm); + } + + [Test] + public void TestApplyArtistQueriesOneDoubleQuote() + { + const string query = "weird artist=double\"quote"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual("weird", filterCriteria.SearchText.Trim()); + Assert.AreEqual(1, filterCriteria.SearchTerms.Length); + Assert.AreEqual("double\"quote", filterCriteria.Artist.SearchTerm); + } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 9cc84c8bdd..6c3c9d20f3 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -39,6 +39,10 @@ namespace osu.Game.Screens.Select.Carousel match &= criteria.BeatDivisor.IsInRange(Beatmap.BeatDivisor); match &= criteria.OnlineStatus.IsInRange(Beatmap.Status); + match &= criteria.Creator.Matches(Beatmap.Metadata.AuthorString); + match &= criteria.Artist.Matches(Beatmap.Metadata.Artist) || + criteria.Artist.Matches(Beatmap.Metadata.ArtistUnicode); + if (match) foreach (var criteriaTerm in criteria.SearchTerms) match &= diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 97a7f12724..c2cbac905e 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -23,6 +23,8 @@ namespace osu.Game.Screens.Select public OptionalRange BPM; public OptionalRange BeatDivisor; public OptionalRange OnlineStatus; + public OptionalTextFilter Creator; + public OptionalTextFilter Artist; public string[] SearchTerms = Array.Empty(); @@ -82,5 +84,24 @@ namespace osu.Game.Screens.Select && IsLowerInclusive.Equals(other.IsLowerInclusive) && IsUpperInclusive.Equals(other.IsUpperInclusive); } + + public struct OptionalTextFilter : IEquatable + { + public bool Matches(string value) + { + if (string.IsNullOrEmpty(SearchTerm)) + return true; + + // search term is guaranteed to be non-empty, so if the string we're comparing is empty, it's not matching + if (string.IsNullOrEmpty(value)) + return false; + + return value.IndexOf(SearchTerm, StringComparison.InvariantCultureIgnoreCase) >= 0; + } + + public string SearchTerm; + + public bool Equals(OptionalTextFilter other) => SearchTerm?.Equals(other.SearchTerm) ?? true; + } } } diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index d6d19c8650..b9281c5d6f 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -11,7 +11,7 @@ namespace osu.Game.Screens.Select internal static class FilterQueryParser { private static readonly Regex query_syntax_regex = new Regex( - @"\b(?stars|ar|dr|cs|divisor|length|objects|bpm|status)(?[=:><]+)(?\S*)", + @"\b(?stars|ar|dr|cs|divisor|length|objects|bpm|status|creator|artist)(?[=:><]+)(?("".*"")|(\S*))", RegexOptions.Compiled | RegexOptions.IgnoreCase); internal static void ApplyQueries(FilterCriteria criteria, string query) @@ -61,6 +61,14 @@ namespace osu.Game.Screens.Select case "status" when Enum.TryParse(value, true, out var statusValue): updateCriteriaRange(ref criteria.OnlineStatus, op, statusValue); break; + + case "creator": + updateCriteriaText(ref criteria.Creator, op, value); + break; + + case "artist": + updateCriteriaText(ref criteria.Artist, op, value); + break; } query = query.Replace(match.ToString(), ""); @@ -78,6 +86,17 @@ namespace osu.Game.Screens.Select private static bool parseInt(string value, out int result) => int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out result); + private static void updateCriteriaText(ref FilterCriteria.OptionalTextFilter textFilter, string op, string value) + { + switch (op) + { + case "=": + case ":": + textFilter.SearchTerm = value.Trim('"'); + break; + } + } + private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, float value, float tolerance = 0.05f) { updateCriteriaRange(ref range, op, value); From 70842f71f4b54484ea1ef22c19379950c2a01a0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 22 Sep 2019 00:11:13 +0200 Subject: [PATCH 10/16] Fix floating point handling in filter intervals Due to floating-point rounding and representation errors, filters could wrongly display results incongruous with the wedge display text (ie. a beatmap with the BPM of 139.99999 would be displayed as having 140 BPM and also pass the bpm<140 filter). Apply tolerance when parsing floating-point constraints. The tolerance chosen is half of what the UI displays for the particular values (so for example half of 0.1 for AR/DR/CS, 0.01 for stars, etc.) Tests updated accordingly. --- .../Filtering/FilterQueryParserTest.cs | 35 ++++++++---- osu.Game/Screens/Select/FilterQueryParser.cs | 56 ++++++++++++++++--- 2 files changed, 70 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index daab690a84..9869ddde41 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -21,6 +21,16 @@ namespace osu.Game.Tests.NonVisual.Filtering Assert.AreEqual(4, filterCriteria.SearchTerms.Length); } + /* + * The following tests have been written a bit strangely (they don't check exact + * bound equality with what the filter says). + * This is to account for floating-point arithmetic issues. + * For example, specifying a bpm<140 filter would previously match beatmaps with BPM + * of 139.99999, which would be displayed in the UI as 140. + * Due to this the tests check the last tick inside the range and the first tick + * outside of the range. + */ + [Test] public void TestApplyStarQueries() { @@ -29,8 +39,9 @@ namespace osu.Game.Tests.NonVisual.Filtering FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual("easy", filterCriteria.SearchText.Trim()); Assert.AreEqual(1, filterCriteria.SearchTerms.Length); - Assert.AreEqual(4.0f, filterCriteria.StarDifficulty.Max); - Assert.IsFalse(filterCriteria.StarDifficulty.IsUpperInclusive); + Assert.IsNotNull(filterCriteria.StarDifficulty.Max); + Assert.Greater(filterCriteria.StarDifficulty.Max, 3.99d); + Assert.Less(filterCriteria.StarDifficulty.Max, 4.00d); Assert.IsNull(filterCriteria.StarDifficulty.Min); } @@ -42,8 +53,9 @@ namespace osu.Game.Tests.NonVisual.Filtering FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual("difficult", filterCriteria.SearchText.Trim()); Assert.AreEqual(1, filterCriteria.SearchTerms.Length); - Assert.AreEqual(9.0f, filterCriteria.ApproachRate.Min); - Assert.IsTrue(filterCriteria.ApproachRate.IsLowerInclusive); + Assert.IsNotNull(filterCriteria.ApproachRate.Min); + Assert.Greater(filterCriteria.ApproachRate.Min, 8.9f); + Assert.Less(filterCriteria.ApproachRate.Min, 9.0f); Assert.IsNull(filterCriteria.ApproachRate.Max); } @@ -55,10 +67,10 @@ namespace osu.Game.Tests.NonVisual.Filtering FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual("quite specific", filterCriteria.SearchText.Trim()); Assert.AreEqual(2, filterCriteria.SearchTerms.Length); - Assert.AreEqual(2.0f, filterCriteria.DrainRate.Min); - Assert.IsFalse(filterCriteria.DrainRate.IsLowerInclusive); - Assert.AreEqual(6.0f, filterCriteria.DrainRate.Max); - Assert.IsTrue(filterCriteria.DrainRate.IsUpperInclusive); + Assert.Greater(filterCriteria.DrainRate.Min, 2.0f); + Assert.Less(filterCriteria.DrainRate.Min, 2.1f); + Assert.Greater(filterCriteria.DrainRate.Max, 6.0f); + Assert.Less(filterCriteria.DrainRate.Min, 6.1f); } [Test] @@ -69,8 +81,9 @@ namespace osu.Game.Tests.NonVisual.Filtering FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual("gotta go fast", filterCriteria.SearchText.Trim()); Assert.AreEqual(3, filterCriteria.SearchTerms.Length); - Assert.AreEqual(200d, filterCriteria.BPM.Min); - Assert.IsTrue(filterCriteria.BPM.IsLowerInclusive); + Assert.IsNotNull(filterCriteria.BPM.Min); + Assert.Greater(filterCriteria.BPM.Min, 199.99d); + Assert.Less(filterCriteria.BPM.Min, 200.00d); Assert.IsNull(filterCriteria.BPM.Max); } @@ -93,9 +106,7 @@ namespace osu.Game.Tests.NonVisual.Filtering Assert.AreEqual("time", filterCriteria.SearchText.Trim()); Assert.AreEqual(1, filterCriteria.SearchTerms.Length); Assert.AreEqual(expectedLength.TotalMilliseconds - scale.TotalMilliseconds / 2.0, filterCriteria.Length.Min); - Assert.IsTrue(filterCriteria.Length.IsLowerInclusive); Assert.AreEqual(expectedLength.TotalMilliseconds + scale.TotalMilliseconds / 2.0, filterCriteria.Length.Max); - Assert.IsTrue(filterCriteria.Length.IsUpperInclusive); } [Test] diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index b9281c5d6f..3ee704201e 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -25,23 +25,23 @@ namespace osu.Game.Screens.Select switch (key) { case "stars" when parseFloatWithPoint(value, out var stars): - updateCriteriaRange(ref criteria.StarDifficulty, op, stars); + updateCriteriaRange(ref criteria.StarDifficulty, op, stars, 0.01f / 2); break; case "ar" when parseFloatWithPoint(value, out var ar): - updateCriteriaRange(ref criteria.ApproachRate, op, ar); + updateCriteriaRange(ref criteria.ApproachRate, op, ar, 0.1f / 2); break; case "dr" when parseFloatWithPoint(value, out var dr): - updateCriteriaRange(ref criteria.DrainRate, op, dr); + updateCriteriaRange(ref criteria.DrainRate, op, dr, 0.1f / 2); break; case "cs" when parseFloatWithPoint(value, out var cs): - updateCriteriaRange(ref criteria.CircleSize, op, cs); + updateCriteriaRange(ref criteria.CircleSize, op, cs, 0.1f / 2); break; case "bpm" when parseDoubleWithPoint(value, out var bpm): - updateCriteriaRange(ref criteria.BPM, op, bpm); + updateCriteriaRange(ref criteria.BPM, op, bpm, 0.01d / 2); break; case "length" when parseDoubleWithPoint(value.TrimEnd('m', 's', 'h'), out var length): @@ -99,29 +99,67 @@ namespace osu.Game.Screens.Select private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, float value, float tolerance = 0.05f) { - updateCriteriaRange(ref range, op, value); - switch (op) { + default: + return; + case "=": case ":": range.Min = value - tolerance; range.Max = value + tolerance; break; + + case ">": + range.Min = value + tolerance; + break; + + case ">=": + case ">:": + range.Min = value - tolerance; + break; + + case "<": + range.Max = value - tolerance; + break; + + case "<=": + case "<:": + range.Max = value + tolerance; + break; } } private static void updateCriteriaRange(ref FilterCriteria.OptionalRange range, string op, double value, double tolerance = 0.05) { - updateCriteriaRange(ref range, op, value); - switch (op) { + default: + return; + case "=": case ":": range.Min = value - tolerance; range.Max = value + tolerance; break; + + case ">": + range.Min = value + tolerance; + break; + + case ">=": + case ">:": + range.Min = value - tolerance; + break; + + case "<": + range.Max = value - tolerance; + break; + + case "<=": + case "<:": + range.Max = value + tolerance; + break; } } From 96c0c80dc58622af64cba3b020b2434b60132bb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 22 Sep 2019 21:20:50 +0200 Subject: [PATCH 11/16] Factor out methods in FilterQueryParser Factor FilterQueryParser.ApplyQueries into shorter methods to reduce method complexity. --- osu.Game/Screens/Select/FilterQueryParser.cs | 102 ++++++++++--------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 3ee704201e..ffe1258168 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -22,54 +22,7 @@ namespace osu.Game.Screens.Select var op = match.Groups["op"].Value; var value = match.Groups["value"].Value; - switch (key) - { - case "stars" when parseFloatWithPoint(value, out var stars): - updateCriteriaRange(ref criteria.StarDifficulty, op, stars, 0.01f / 2); - break; - - case "ar" when parseFloatWithPoint(value, out var ar): - updateCriteriaRange(ref criteria.ApproachRate, op, ar, 0.1f / 2); - break; - - case "dr" when parseFloatWithPoint(value, out var dr): - updateCriteriaRange(ref criteria.DrainRate, op, dr, 0.1f / 2); - break; - - case "cs" when parseFloatWithPoint(value, out var cs): - updateCriteriaRange(ref criteria.CircleSize, op, cs, 0.1f / 2); - break; - - case "bpm" when parseDoubleWithPoint(value, out var bpm): - updateCriteriaRange(ref criteria.BPM, op, bpm, 0.01d / 2); - break; - - case "length" when parseDoubleWithPoint(value.TrimEnd('m', 's', 'h'), out var length): - var scale = - value.EndsWith("ms") ? 1 : - value.EndsWith("s") ? 1000 : - value.EndsWith("m") ? 60000 : - value.EndsWith("h") ? 3600000 : 1000; - - updateCriteriaRange(ref criteria.Length, op, length * scale, scale / 2.0); - break; - - case "divisor" when parseInt(value, out var divisor): - updateCriteriaRange(ref criteria.BeatDivisor, op, divisor); - break; - - case "status" when Enum.TryParse(value, true, out var statusValue): - updateCriteriaRange(ref criteria.OnlineStatus, op, statusValue); - break; - - case "creator": - updateCriteriaText(ref criteria.Creator, op, value); - break; - - case "artist": - updateCriteriaText(ref criteria.Artist, op, value); - break; - } + parseKeywordCriteria(criteria, key, value, op); query = query.Replace(match.ToString(), ""); } @@ -77,6 +30,59 @@ namespace osu.Game.Screens.Select criteria.SearchText = query; } + private static void parseKeywordCriteria(FilterCriteria criteria, string key, string value, string op) + { + switch (key) + { + case "stars" when parseFloatWithPoint(value, out var stars): + updateCriteriaRange(ref criteria.StarDifficulty, op, stars, 0.01f / 2); + break; + + case "ar" when parseFloatWithPoint(value, out var ar): + updateCriteriaRange(ref criteria.ApproachRate, op, ar, 0.1f / 2); + break; + + case "dr" when parseFloatWithPoint(value, out var dr): + updateCriteriaRange(ref criteria.DrainRate, op, dr, 0.1f / 2); + break; + + case "cs" when parseFloatWithPoint(value, out var cs): + updateCriteriaRange(ref criteria.CircleSize, op, cs, 0.1f / 2); + break; + + case "bpm" when parseDoubleWithPoint(value, out var bpm): + updateCriteriaRange(ref criteria.BPM, op, bpm, 0.01d / 2); + break; + + case "length" when parseDoubleWithPoint(value.TrimEnd('m', 's', 'h'), out var length): + var scale = getLengthScale(value); + updateCriteriaRange(ref criteria.Length, op, length * scale, scale / 2.0); + break; + + case "divisor" when parseInt(value, out var divisor): + updateCriteriaRange(ref criteria.BeatDivisor, op, divisor); + break; + + case "status" when Enum.TryParse(value, true, out var statusValue): + updateCriteriaRange(ref criteria.OnlineStatus, op, statusValue); + break; + + case "creator": + updateCriteriaText(ref criteria.Creator, op, value); + break; + + case "artist": + updateCriteriaText(ref criteria.Artist, op, value); + break; + } + } + + private static int getLengthScale(string value) => + value.EndsWith("ms") ? 1 : + value.EndsWith("s") ? 1000 : + value.EndsWith("m") ? 60000 : + value.EndsWith("h") ? 3600000 : 1000; + private static bool parseFloatWithPoint(string value, out float result) => float.TryParse(value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out result); From ffbab2535856f37dacdb6f49f494aa1ed20f3ccf Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 23 Sep 2019 16:12:43 +0300 Subject: [PATCH 12/16] Fix incorrect icon margin in ChangelogOverlay --- osu.Game/Overlays/Changelog/ChangelogBuild.cs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/osu.Game/Overlays/Changelog/ChangelogBuild.cs b/osu.Game/Overlays/Changelog/ChangelogBuild.cs index 11dc2049fd..05bf56bc33 100644 --- a/osu.Game/Overlays/Changelog/ChangelogBuild.cs +++ b/osu.Game/Overlays/Changelog/ChangelogBuild.cs @@ -15,6 +15,7 @@ using osu.Game.Users; using osuTK.Graphics; using osu.Framework.Allocation; using System.Net; +using osuTK; namespace osu.Game.Overlays.Changelog { @@ -67,21 +68,31 @@ namespace osu.Game.Overlays.Changelog foreach (APIChangelogEntry entry in categoryEntries) { - LinkFlowContainer title = new LinkFlowContainer + LinkFlowContainer title; + + Container titleContainer = new Container { - Direction = FillDirection.Full, - RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, + RelativeSizeAxes = Axes.X, Margin = new MarginPadding { Vertical = 5 }, + Child = title = new LinkFlowContainer + { + Direction = FillDirection.Full, + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + } }; var entryColour = entry.Major ? colours.YellowLight : Color4.White; - title.AddIcon(entry.Type == ChangelogEntryType.Fix ? FontAwesome.Solid.Check : FontAwesome.Solid.Plus, t => + titleContainer.Add(new SpriteIcon { - t.Font = fontSmall; - t.Colour = entryColour; - t.Padding = new MarginPadding { Left = -17, Right = 5 }; + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreRight, + Size = new Vector2(fontSmall.Size), + Icon = entry.Type == ChangelogEntryType.Fix ? FontAwesome.Solid.Check : FontAwesome.Solid.Plus, + Colour = entryColour, + Margin = new MarginPadding { Right = 5 }, }); title.AddText(entry.Title, t => @@ -139,7 +150,7 @@ namespace osu.Game.Overlays.Changelog t.Colour = entryColour; }); - ChangelogEntries.Add(title); + ChangelogEntries.Add(titleContainer); if (!string.IsNullOrEmpty(entry.MessageHtml)) { From 5c4dfe0809c6865adf8f8daec0756935200f1ee0 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 23 Sep 2019 17:05:19 +0300 Subject: [PATCH 13/16] Apply suggested change --- osu.Game/Overlays/Changelog/ChangelogBuild.cs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/osu.Game/Overlays/Changelog/ChangelogBuild.cs b/osu.Game/Overlays/Changelog/ChangelogBuild.cs index 05bf56bc33..bce1be5941 100644 --- a/osu.Game/Overlays/Changelog/ChangelogBuild.cs +++ b/osu.Game/Overlays/Changelog/ChangelogBuild.cs @@ -68,6 +68,8 @@ namespace osu.Game.Overlays.Changelog foreach (APIChangelogEntry entry in categoryEntries) { + var entryColour = entry.Major ? colours.YellowLight : Color4.White; + LinkFlowContainer title; Container titleContainer = new Container @@ -75,26 +77,26 @@ namespace osu.Game.Overlays.Changelog AutoSizeAxes = Axes.Y, RelativeSizeAxes = Axes.X, Margin = new MarginPadding { Vertical = 5 }, - Child = title = new LinkFlowContainer + Children = new Drawable[] { - Direction = FillDirection.Full, - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, + new SpriteIcon + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreRight, + Size = new Vector2(fontSmall.Size), + Icon = entry.Type == ChangelogEntryType.Fix ? FontAwesome.Solid.Check : FontAwesome.Solid.Plus, + Colour = entryColour, + Margin = new MarginPadding { Right = 5 }, + }, + title = new LinkFlowContainer + { + Direction = FillDirection.Full, + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + } } }; - var entryColour = entry.Major ? colours.YellowLight : Color4.White; - - titleContainer.Add(new SpriteIcon - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreRight, - Size = new Vector2(fontSmall.Size), - Icon = entry.Type == ChangelogEntryType.Fix ? FontAwesome.Solid.Check : FontAwesome.Solid.Plus, - Colour = entryColour, - Margin = new MarginPadding { Right = 5 }, - }); - title.AddText(entry.Title, t => { t.Font = fontLarge; From 53603b559176e4ddbc62e5e3e9d54f67785b2d62 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2019 20:15:51 +0000 Subject: [PATCH 14/16] Bump System.ComponentModel.Annotations from 4.5.0 to 4.6.0 Bumps [System.ComponentModel.Annotations](https://github.com/dotnet/corefx) from 4.5.0 to 4.6.0. - [Release notes](https://github.com/dotnet/corefx/releases) - [Commits](https://github.com/dotnet/corefx/commits) Signed-off-by: dependabot-preview[bot] --- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index a27a94b8f9..a699217503 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -30,6 +30,6 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index a6516e6d1b..7803ea1e49 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -123,7 +123,7 @@ - + From 78ce62b187798d8e194c0c89ea6ee3a29154bc3b Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2019 20:29:36 +0000 Subject: [PATCH 15/16] Bump System.IO.Packaging from 4.5.0 to 4.6.0 Bumps [System.IO.Packaging](https://github.com/dotnet/corefx) from 4.5.0 to 4.6.0. - [Release notes](https://github.com/dotnet/corefx/releases) - [Commits](https://github.com/dotnet/corefx/commits) Signed-off-by: dependabot-preview[bot] --- osu.Desktop/osu.Desktop.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 538aaf2d7a..03b002add7 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -26,7 +26,7 @@ - + From 50dcb70342c22f97398e9c45b2f7c8576f52807d Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2019 21:04:20 +0000 Subject: [PATCH 16/16] Bump Microsoft.Win32.Registry from 4.5.0 to 4.6.0 Bumps [Microsoft.Win32.Registry](https://github.com/dotnet/corefx) from 4.5.0 to 4.6.0. - [Release notes](https://github.com/dotnet/corefx/releases) - [Commits](https://github.com/dotnet/corefx/commits) Signed-off-by: dependabot-preview[bot] --- osu.Desktop/osu.Desktop.csproj | 2 +- osu.Game.Tournament/osu.Game.Tournament.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 538aaf2d7a..9e8bb431c0 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -23,7 +23,7 @@ - + diff --git a/osu.Game.Tournament/osu.Game.Tournament.csproj b/osu.Game.Tournament/osu.Game.Tournament.csproj index 4790fcbcde..bddaff0a80 100644 --- a/osu.Game.Tournament/osu.Game.Tournament.csproj +++ b/osu.Game.Tournament/osu.Game.Tournament.csproj @@ -11,6 +11,6 @@ - + \ No newline at end of file