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] 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);