From 102576cd8c5baa9077e5b1b864b52c83c23c71ae Mon Sep 17 00:00:00 2001 From: Elvendir Date: Sat, 18 Mar 2023 17:53:41 +0100 Subject: [PATCH 01/36] adddede LastPlayed as filter option in beatmap carousel --- .../Select/Carousel/CarouselBeatmap.cs | 1 + osu.Game/Screens/Select/FilterCriteria.cs | 1 + osu.Game/Screens/Select/FilterQueryParser.cs | 53 ++++++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 7e48bc5cdd..5088dfdc02 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -48,6 +48,7 @@ namespace osu.Game.Screens.Select.Carousel match &= !criteria.CircleSize.HasFilter || criteria.CircleSize.IsInRange(BeatmapInfo.Difficulty.CircleSize); match &= !criteria.OverallDifficulty.HasFilter || criteria.OverallDifficulty.IsInRange(BeatmapInfo.Difficulty.OverallDifficulty); match &= !criteria.Length.HasFilter || criteria.Length.IsInRange(BeatmapInfo.Length); + match &= !criteria.LastPlayed.HasFilter || criteria.LastPlayed.IsInRange(BeatmapInfo.LastPlayed); match &= !criteria.BPM.HasFilter || criteria.BPM.IsInRange(BeatmapInfo.BPM); match &= !criteria.BeatDivisor.HasFilter || criteria.BeatDivisor.IsInRange(BeatmapInfo.BeatDivisor); diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 320bfb1b45..a2da98368d 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -31,6 +31,7 @@ namespace osu.Game.Screens.Select public OptionalRange BPM; public OptionalRange BeatDivisor; public OptionalRange OnlineStatus; + public OptionalRange LastPlayed; public OptionalTextFilter Creator; public OptionalTextFilter Artist; diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index c86554ddbc..6b6b2f04a9 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -61,6 +61,10 @@ namespace osu.Game.Screens.Select case "length": return tryUpdateLengthRange(criteria, op, value); + case "played": + case "lastplayed": + return tryUpdateLastPlayedRange(criteria, op, value); + case "divisor": return TryUpdateCriteriaRange(ref criteria.BeatDivisor, op, value, tryParseInt); @@ -109,7 +113,8 @@ namespace osu.Game.Screens.Select value.EndsWith("ms", StringComparison.Ordinal) ? 1 : value.EndsWith('s') ? 1000 : value.EndsWith('m') ? 60000 : - value.EndsWith('h') ? 3600000 : 1000; + value.EndsWith('h') ? 3600000 : + value.EndsWith('d') ? 86400000 : 1000; private static bool tryParseFloatWithPoint(string value, out float result) => float.TryParse(value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out result); @@ -368,5 +373,51 @@ namespace osu.Game.Screens.Select return tryUpdateCriteriaRange(ref criteria.Length, op, totalLength, minScale / 2.0); } + + private static bool tryUpdateLastPlayedRange(FilterCriteria criteria, Operator op, string val) + { + List parts = new List(); + + GroupCollection? match = null; + + match ??= tryMatchRegex(val, @"^((?\d+):)?(?\d+):(?\d+)$"); + match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); + match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); + + if (match == null) + return false; + + if (match["seconds"].Success) + parts.Add(match["seconds"].Value + "s"); + if (match["minutes"].Success) + parts.Add(match["minutes"].Value + "m"); + if (match["hours"].Success) + parts.Add(match["hours"].Value + "h"); + if (match["days"].Success) + parts.Add(match["days"].Value + "d"); + + + double totalLength = 0; + int minScale = 86400000; + + for (int i = 0; i < parts.Count; i++) + { + string part = parts[i]; + string partNoUnit = part.TrimEnd('m', 's', 'h', 'd') ; + if (!tryParseDoubleWithPoint(partNoUnit, out double length)) + return false; + + if (i != parts.Count - 1 && length >= 60) + return false; + if (i != 0 && partNoUnit.Contains('.')) + return false; + + int scale = getLengthScale(part); + totalLength += length * scale; + minScale = Math.Min(minScale, scale); + } + + return tryUpdateCriteriaRange(ref criteria.LastPlayed, op, totalLength, minScale / 2.0); + } } } From 216a88e18d71f74ddccea1b70aff77305ec88920 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Sat, 18 Mar 2023 21:35:10 +0100 Subject: [PATCH 02/36] limited max Date comparison --- .../Screens/Select/Carousel/CarouselBeatmap.cs | 2 +- osu.Game/Screens/Select/FilterQueryParser.cs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 5088dfdc02..069d4f36d6 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -48,7 +48,7 @@ namespace osu.Game.Screens.Select.Carousel match &= !criteria.CircleSize.HasFilter || criteria.CircleSize.IsInRange(BeatmapInfo.Difficulty.CircleSize); match &= !criteria.OverallDifficulty.HasFilter || criteria.OverallDifficulty.IsInRange(BeatmapInfo.Difficulty.OverallDifficulty); match &= !criteria.Length.HasFilter || criteria.Length.IsInRange(BeatmapInfo.Length); - match &= !criteria.LastPlayed.HasFilter || criteria.LastPlayed.IsInRange(BeatmapInfo.LastPlayed); + match &= !criteria.LastPlayed.HasFilter || criteria.LastPlayed.IsInRange(BeatmapInfo.LastPlayed ?? DateTimeOffset.MinValue); match &= !criteria.BPM.HasFilter || criteria.BPM.IsInRange(BeatmapInfo.BPM); match &= !criteria.BeatDivisor.HasFilter || criteria.BeatDivisor.IsInRange(BeatmapInfo.BeatDivisor); diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 6b6b2f04a9..38040c2f79 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -381,7 +381,7 @@ namespace osu.Game.Screens.Select GroupCollection? match = null; match ??= tryMatchRegex(val, @"^((?\d+):)?(?\d+):(?\d+)$"); - match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); + match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); if (match == null) @@ -403,7 +403,7 @@ namespace osu.Game.Screens.Select for (int i = 0; i < parts.Count; i++) { string part = parts[i]; - string partNoUnit = part.TrimEnd('m', 's', 'h', 'd') ; + string partNoUnit = part.TrimEnd('m', 's', 'h', 'd'); if (!tryParseDoubleWithPoint(partNoUnit, out double length)) return false; @@ -417,7 +417,16 @@ namespace osu.Game.Screens.Select minScale = Math.Min(minScale, scale); } - return tryUpdateCriteriaRange(ref criteria.LastPlayed, op, totalLength, minScale / 2.0); + totalLength += minScale / 2; + + // Limits the date to ~2000 years compared to now + // Might want to do it differently before 4000 A.C. + double limit = 86400000; + limit *= 365 * 2000; + totalLength = Math.Min(totalLength, limit); + + DateTimeOffset dateTimeOffset = DateTimeOffset.Now; + return tryUpdateCriteriaRange(ref criteria.LastPlayed, op, dateTimeOffset.AddMilliseconds(-totalLength)); } } } From 6dead81d211a760e185ff0248a080454e6729197 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Sun, 19 Mar 2023 18:43:17 +0100 Subject: [PATCH 03/36] Generalized tryUpdateLastPlayedRange to tryUpdateDateRange and Added tests --- .../Filtering/FilterQueryParserTest.cs | 82 ++++++++++++++ osu.Game/Screens/Select/FilterQueryParser.cs | 107 ++++++++++++------ 2 files changed, 152 insertions(+), 37 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index da32edb8fb..9460228644 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -316,5 +316,87 @@ namespace osu.Game.Tests.NonVisual.Filtering return false; } } + + //Date criteria testing + + private static readonly object[] correct_date_query_examples = + { + new object[] { "600" }, + new object[] { "120:120" }, + new object[] { "48:0:0" }, + new object[] { "0.5s" }, + new object[] { "120m" }, + new object[] { "48h120s" }, + new object[] { "10y24M" }, + new object[] { "10y60d120s" }, + new object[] { "0.1y0.1M2d" }, + new object[] { "0.99y0.99M2d" } + }; + + [Test] + [TestCaseSource(nameof(correct_date_query_examples))] + public void TestValidDateQueries(string dateQuery) + { + string query = $"played={dateQuery} time"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(true, filterCriteria.LastPlayed.HasFilter); + } + + private static readonly object[] incorrect_date_query_examples = + { + new object[] { "7m27" }, + new object[] { "7m7m7m" }, + new object[] { "5s6m" }, + new object[] { "7d7y" }, + new object[] { ":0" }, + new object[] { "0:3:" }, + new object[] { "\"three days\"" } + }; + + [Test] + [TestCaseSource(nameof(incorrect_date_query_examples))] + public void TestInvalidDateQueries(string dateQuery) + { + string query = $"played={dateQuery} time"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(false, filterCriteria.LastPlayed.HasFilter); + } + + private static readonly object[] list_operators = + { + new object[] { "=", false, false, true, true }, + new object[] { ":", false, false, true, true }, + new object[] { "<", false, true, false, false }, + new object[] { "<=", false, true, true, false }, + new object[] { "<:", false, true, true, false }, + new object[] { ">", true, false, false, false }, + new object[] { ">=", true, false, false, true }, + new object[] { ">:", true, false, false, true } + }; + + [Test] + [TestCaseSource(nameof(list_operators))] + public void TestComparisonDateQueries(string ope, bool minIsNull, bool maxIsNull, bool isLowerInclusive, bool isUpperInclusive) + { + string query = $"played{ope}50"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(isLowerInclusive, filterCriteria.LastPlayed.IsLowerInclusive); + Assert.AreEqual(isUpperInclusive, filterCriteria.LastPlayed.IsUpperInclusive); + Assert.AreEqual(maxIsNull, filterCriteria.LastPlayed.Max == null); + Assert.AreEqual(minIsNull, filterCriteria.LastPlayed.Min == null); + } + + [Test] + public void TestOutofrangeDateQuery() + { + const string query = "played=10000y"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(true, filterCriteria.LastPlayed.HasFilter); + Assert.AreEqual(DateTimeOffset.MinValue.AddMilliseconds(1), filterCriteria.LastPlayed.Min); + } } } diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 38040c2f79..f66f1bcd1d 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -63,7 +63,7 @@ namespace osu.Game.Screens.Select case "played": case "lastplayed": - return tryUpdateLastPlayedRange(criteria, op, value); + return tryUpdateDateRange(ref criteria.LastPlayed, op, value); case "divisor": return TryUpdateCriteriaRange(ref criteria.BeatDivisor, op, value, tryParseInt); @@ -374,59 +374,92 @@ namespace osu.Game.Screens.Select return tryUpdateCriteriaRange(ref criteria.Length, op, totalLength, minScale / 2.0); } - private static bool tryUpdateLastPlayedRange(FilterCriteria criteria, Operator op, string val) + private static bool tryUpdateDateRange(ref FilterCriteria.OptionalRange dateRange, Operator op, string val) { - List parts = new List(); - GroupCollection? match = null; match ??= tryMatchRegex(val, @"^((?\d+):)?(?\d+):(?\d+)$"); - match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); + match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)y)?((?\d+(\.\d+)?)M)?((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); if (match == null) return false; - if (match["seconds"].Success) - parts.Add(match["seconds"].Value + "s"); - if (match["minutes"].Success) - parts.Add(match["minutes"].Value + "m"); - if (match["hours"].Success) - parts.Add(match["hours"].Value + "h"); - if (match["days"].Success) - parts.Add(match["days"].Value + "d"); + DateTimeOffset dateTimeOffset = DateTimeOffset.Now; - - double totalLength = 0; - int minScale = 86400000; - - for (int i = 0; i < parts.Count; i++) + try { - string part = parts[i]; - string partNoUnit = part.TrimEnd('m', 's', 'h', 'd'); - if (!tryParseDoubleWithPoint(partNoUnit, out double length)) - return false; + List keys = new List { "seconds", "minutes", "hours", "days", "months", "years" }; - if (i != parts.Count - 1 && length >= 60) - return false; - if (i != 0 && partNoUnit.Contains('.')) - return false; + foreach (string key in keys) + { + if (match[key].Success) + { + if (!tryParseDoubleWithPoint(match[key].Value, out double length)) + return false; - int scale = getLengthScale(part); - totalLength += length * scale; - minScale = Math.Min(minScale, scale); + switch (key) + { + case "seconds": + dateTimeOffset = dateTimeOffset.AddSeconds(-length); + break; + + case "minutes": + dateTimeOffset = dateTimeOffset.AddMinutes(-length); + break; + + case "hours": + dateTimeOffset = dateTimeOffset.AddHours(-length); + break; + + case "days": + dateTimeOffset = dateTimeOffset.AddDays(-length); + break; + + case "months": + dateTimeOffset = dateTimeOffset.AddMonths(-(int)Math.Round(length)); + break; + + case "years": + dateTimeOffset = dateTimeOffset.AddYears(-(int)Math.Round(length)); + break; + } + } + } + } + // If DateTime to compare is out-scope put it to Min + catch (Exception) + { + dateTimeOffset = DateTimeOffset.MinValue; + dateTimeOffset = dateTimeOffset.AddMilliseconds(1); } - totalLength += minScale / 2; + return tryUpdateCriteriaRange(ref dateRange, invert(op), dateTimeOffset); + } - // Limits the date to ~2000 years compared to now - // Might want to do it differently before 4000 A.C. - double limit = 86400000; - limit *= 365 * 2000; - totalLength = Math.Min(totalLength, limit); + // Function to reverse an Operator + private static Operator invert(Operator ope) + { + switch (ope) + { + default: + return Operator.Equal; - DateTimeOffset dateTimeOffset = DateTimeOffset.Now; - return tryUpdateCriteriaRange(ref criteria.LastPlayed, op, dateTimeOffset.AddMilliseconds(-totalLength)); + case Operator.Equal: + return Operator.Equal; + + case Operator.Greater: + return Operator.Less; + + case Operator.GreaterOrEqual: + return Operator.LessOrEqual; + + case Operator.Less: + return Operator.Greater; + + case Operator.LessOrEqual: + return Operator.GreaterOrEqual; + } } } } From 4b053b47852ea46ad104156d81a84e3bdaaf3b04 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Sat, 1 Apr 2023 22:58:25 +0200 Subject: [PATCH 04/36] changed regex match to be inline with standard --- .../NonVisual/Filtering/FilterQueryParserTest.cs | 3 +-- osu.Game/Screens/Select/FilterQueryParser.cs | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 9460228644..b0cc9146d2 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -322,8 +322,6 @@ namespace osu.Game.Tests.NonVisual.Filtering private static readonly object[] correct_date_query_examples = { new object[] { "600" }, - new object[] { "120:120" }, - new object[] { "48:0:0" }, new object[] { "0.5s" }, new object[] { "120m" }, new object[] { "48h120s" }, @@ -350,6 +348,7 @@ namespace osu.Game.Tests.NonVisual.Filtering new object[] { "5s6m" }, new object[] { "7d7y" }, new object[] { ":0" }, + new object[] { "0:3:6" }, new object[] { "0:3:" }, new object[] { "\"three days\"" } }; diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index f66f1bcd1d..f66b8fd377 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -378,9 +378,8 @@ namespace osu.Game.Screens.Select { GroupCollection? match = null; - match ??= tryMatchRegex(val, @"^((?\d+):)?(?\d+):(?\d+)$"); match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)y)?((?\d+(\.\d+)?)M)?((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); - match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); + match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); if (match == null) return false; @@ -417,11 +416,13 @@ namespace osu.Game.Screens.Select break; case "months": - dateTimeOffset = dateTimeOffset.AddMonths(-(int)Math.Round(length)); + dateTimeOffset = dateTimeOffset.AddMonths(-(int)Math.Floor(length)); + dateTimeOffset = dateTimeOffset.AddDays(-30 * (length - Math.Floor(length))); break; case "years": - dateTimeOffset = dateTimeOffset.AddYears(-(int)Math.Round(length)); + dateTimeOffset = dateTimeOffset.AddYears(-(int)Math.Floor(length)); + dateTimeOffset = dateTimeOffset.AddDays(-365 * (length - Math.Floor(length))); break; } } From 52adb99fe5fe4f7f7ea756f0723354f17904203e Mon Sep 17 00:00:00 2001 From: Elvendir <39671719+Elvendir@users.noreply.github.com> Date: Tue, 4 Apr 2023 19:29:37 +0200 Subject: [PATCH 05/36] Update osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index b0cc9146d2..627b44dcd7 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -317,7 +317,6 @@ namespace osu.Game.Tests.NonVisual.Filtering } } - //Date criteria testing private static readonly object[] correct_date_query_examples = { From d6c6507578a0fd3a7a8a502c15febd91b0123069 Mon Sep 17 00:00:00 2001 From: Elvendir <39671719+Elvendir@users.noreply.github.com> Date: Tue, 4 Apr 2023 19:30:13 +0200 Subject: [PATCH 06/36] Update osu.Game/Screens/Select/FilterQueryParser.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Screens/Select/FilterQueryParser.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index f66b8fd377..9582694248 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -428,8 +428,7 @@ namespace osu.Game.Screens.Select } } } - // If DateTime to compare is out-scope put it to Min - catch (Exception) + catch (ArgumentOutOfRangeException) { dateTimeOffset = DateTimeOffset.MinValue; dateTimeOffset = dateTimeOffset.AddMilliseconds(1); From 0c1d6eb89465066285668b619bc73a54c9fa964c Mon Sep 17 00:00:00 2001 From: Elvendir Date: Wed, 5 Apr 2023 11:42:39 +0200 Subject: [PATCH 07/36] - rewrote upper and lower bound tests - removed equality in tests --- .../Filtering/FilterQueryParserTest.cs | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 627b44dcd7..c1678f14a6 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -317,7 +317,6 @@ namespace osu.Game.Tests.NonVisual.Filtering } } - private static readonly object[] correct_date_query_examples = { new object[] { "600" }, @@ -334,7 +333,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCaseSource(nameof(correct_date_query_examples))] public void TestValidDateQueries(string dateQuery) { - string query = $"played={dateQuery} time"; + string query = $"played<{dateQuery} time"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual(true, filterCriteria.LastPlayed.HasFilter); @@ -342,11 +341,11 @@ namespace osu.Game.Tests.NonVisual.Filtering private static readonly object[] incorrect_date_query_examples = { + new object[] { ".5s" }, new object[] { "7m27" }, new object[] { "7m7m7m" }, new object[] { "5s6m" }, new object[] { "7d7y" }, - new object[] { ":0" }, new object[] { "0:3:6" }, new object[] { "0:3:" }, new object[] { "\"three days\"" } @@ -356,41 +355,36 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCaseSource(nameof(incorrect_date_query_examples))] public void TestInvalidDateQueries(string dateQuery) { - string query = $"played={dateQuery} time"; + string query = $"played<{dateQuery} time"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual(false, filterCriteria.LastPlayed.HasFilter); } - private static readonly object[] list_operators = - { - new object[] { "=", false, false, true, true }, - new object[] { ":", false, false, true, true }, - new object[] { "<", false, true, false, false }, - new object[] { "<=", false, true, true, false }, - new object[] { "<:", false, true, true, false }, - new object[] { ">", true, false, false, false }, - new object[] { ">=", true, false, false, true }, - new object[] { ">:", true, false, false, true } - }; - [Test] - [TestCaseSource(nameof(list_operators))] - public void TestComparisonDateQueries(string ope, bool minIsNull, bool maxIsNull, bool isLowerInclusive, bool isUpperInclusive) + public void TestGreaterDateQuery() { - string query = $"played{ope}50"; + const string query = "played>50"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); - Assert.AreEqual(isLowerInclusive, filterCriteria.LastPlayed.IsLowerInclusive); - Assert.AreEqual(isUpperInclusive, filterCriteria.LastPlayed.IsUpperInclusive); - Assert.AreEqual(maxIsNull, filterCriteria.LastPlayed.Max == null); - Assert.AreEqual(minIsNull, filterCriteria.LastPlayed.Min == null); + Assert.AreEqual(false, filterCriteria.LastPlayed.Max == null); + Assert.AreEqual(true, filterCriteria.LastPlayed.Min == null); + } + + [Test] + public void TestLowerDateQuery() + { + const string query = "played<50"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(true, filterCriteria.LastPlayed.Max == null); + Assert.AreEqual(false, filterCriteria.LastPlayed.Min == null); } [Test] public void TestOutofrangeDateQuery() { - const string query = "played=10000y"; + const string query = "played<10000y"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); Assert.AreEqual(true, filterCriteria.LastPlayed.HasFilter); From df170517a85ea1949d20b6df1043143ac028ffec Mon Sep 17 00:00:00 2001 From: Elvendir Date: Wed, 5 Apr 2023 11:59:31 +0200 Subject: [PATCH 08/36] -renamed function inverse() to reverseInequalityOperator() for clarity -changed default case of reverseInequalityOperator() to out of range exception --- osu.Game/Screens/Select/FilterQueryParser.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 9582694248..bd4c29e457 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -434,16 +434,16 @@ namespace osu.Game.Screens.Select dateTimeOffset = dateTimeOffset.AddMilliseconds(1); } - return tryUpdateCriteriaRange(ref dateRange, invert(op), dateTimeOffset); + return tryUpdateCriteriaRange(ref dateRange, reverseInequalityOperator(op), dateTimeOffset); } // Function to reverse an Operator - private static Operator invert(Operator ope) + private static Operator reverseInequalityOperator(Operator ope) { switch (ope) { default: - return Operator.Equal; + throw new ArgumentOutOfRangeException(nameof(ope), $"Unsupported operator {ope}"); case Operator.Equal: return Operator.Equal; From c2f225f0253030ea03e710f1647da26f6b106377 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Wed, 5 Apr 2023 21:25:58 +0200 Subject: [PATCH 09/36] Made Operator.Equal not parse for date filter and added corresponding test --- .../NonVisual/Filtering/FilterQueryParserTest.cs | 9 +++++++++ osu.Game/Screens/Select/FilterQueryParser.cs | 3 +++ 2 files changed, 12 insertions(+) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index c1678f14a6..450e6bdde7 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -381,6 +381,15 @@ namespace osu.Game.Tests.NonVisual.Filtering Assert.AreEqual(false, filterCriteria.LastPlayed.Min == null); } + [Test] + public void TestEqualDateQuery() + { + const string query = "played=50"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.AreEqual(false, filterCriteria.LastPlayed.HasFilter); + } + [Test] public void TestOutofrangeDateQuery() { diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index bd4c29e457..70893c188d 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -376,6 +376,9 @@ namespace osu.Game.Screens.Select private static bool tryUpdateDateRange(ref FilterCriteria.OptionalRange dateRange, Operator op, string val) { + if (op == Operator.Equal) + return false; + GroupCollection? match = null; match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)y)?((?\d+(\.\d+)?)M)?((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); From 928145cdeb3b523833ed6b5f3f85c830d91f4dc8 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Wed, 5 Apr 2023 22:12:15 +0200 Subject: [PATCH 10/36] Enforce integer value before y and M Change impacted Tests --- .../NonVisual/Filtering/FilterQueryParserTest.cs | 8 +++++--- osu.Game/Screens/Select/FilterQueryParser.cs | 12 ++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 450e6bdde7..78b428e7c0 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -325,8 +325,8 @@ namespace osu.Game.Tests.NonVisual.Filtering new object[] { "48h120s" }, new object[] { "10y24M" }, new object[] { "10y60d120s" }, - new object[] { "0.1y0.1M2d" }, - new object[] { "0.99y0.99M2d" } + new object[] { "0y0M2d" }, + new object[] { "1y1M2d" } }; [Test] @@ -348,7 +348,9 @@ namespace osu.Game.Tests.NonVisual.Filtering new object[] { "7d7y" }, new object[] { "0:3:6" }, new object[] { "0:3:" }, - new object[] { "\"three days\"" } + new object[] { "\"three days\"" }, + new object[] { "0.1y0.1M2d" }, + new object[] { "0.99y0.99M2d" } }; [Test] diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 70893c188d..b49f0ba057 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -419,13 +419,17 @@ namespace osu.Game.Screens.Select break; case "months": - dateTimeOffset = dateTimeOffset.AddMonths(-(int)Math.Floor(length)); - dateTimeOffset = dateTimeOffset.AddDays(-30 * (length - Math.Floor(length))); + if (match[key].Value.Contains('.')) + return false; + + dateTimeOffset = dateTimeOffset.AddMonths(-(int)length); break; case "years": - dateTimeOffset = dateTimeOffset.AddYears(-(int)Math.Floor(length)); - dateTimeOffset = dateTimeOffset.AddDays(-365 * (length - Math.Floor(length))); + if (match[key].Value.Contains('.')) + return false; + + dateTimeOffset = dateTimeOffset.AddYears(-(int)length); break; } } From 8e156fdb5102af8cd71aaac32d3c36b3c018d382 Mon Sep 17 00:00:00 2001 From: Elvendir Date: Fri, 7 Apr 2023 00:29:46 +0200 Subject: [PATCH 11/36] Enforce integer through regex match instead --- osu.Game/Screens/Select/FilterQueryParser.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index b49f0ba057..348f663b8e 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -381,7 +381,7 @@ namespace osu.Game.Screens.Select GroupCollection? match = null; - match ??= tryMatchRegex(val, @"^((?\d+(\.\d+)?)y)?((?\d+(\.\d+)?)M)?((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); + match ??= tryMatchRegex(val, @"^((?\d+)y)?((?\d+)M)?((?\d+(\.\d+)?)d)?((?\d+(\.\d+)?)h)?((?\d+(\.\d+)?)m)?((?\d+(\.\d+)?)s)?$"); match ??= tryMatchRegex(val, @"^(?\d+(\.\d+)?)$"); if (match == null) @@ -419,16 +419,10 @@ namespace osu.Game.Screens.Select break; case "months": - if (match[key].Value.Contains('.')) - return false; - dateTimeOffset = dateTimeOffset.AddMonths(-(int)length); break; case "years": - if (match[key].Value.Contains('.')) - return false; - dateTimeOffset = dateTimeOffset.AddYears(-(int)length); break; } From 581ae2f679a00e98ea3e773a9c0c59494b9be8e5 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 12:51:35 +0000 Subject: [PATCH 12/36] handle key presses when watching legacy relax replays --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 24 +++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 3679425389..55d8b6f55f 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -38,12 +38,17 @@ namespace osu.Game.Rulesets.Osu.Mods private ReplayState state = null!; private double lastStateChangeTime; + private DrawableOsuRuleset ruleset = null!; + private bool hasReplay; + private bool legacyReplay; public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { + ruleset = (DrawableOsuRuleset)drawableRuleset; + // grab the input manager for future use. - osuInputManager = ((DrawableOsuRuleset)drawableRuleset).KeyBindingInputManager; + osuInputManager = ruleset.KeyBindingInputManager; } public void ApplyToPlayer(Player player) @@ -51,6 +56,7 @@ namespace osu.Game.Rulesets.Osu.Mods if (osuInputManager.ReplayInputHandler != null) { hasReplay = true; + legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; return; } @@ -59,7 +65,7 @@ namespace osu.Game.Rulesets.Osu.Mods public void Update(Playfield playfield) { - if (hasReplay) + if (hasReplay && !legacyReplay) return; bool requiresHold = false; @@ -125,6 +131,20 @@ namespace osu.Game.Rulesets.Osu.Mods isDownState = down; lastStateChangeTime = time; + // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. + if (legacyReplay) + { + if (!down) + { + osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); + return; + } + + osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + wasLeft = !wasLeft; + return; + } + state = new ReplayState { PressedActions = new List() From 2a02566283f831b469a2b37a4a645aae2cb07bc3 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 17:45:00 +0000 Subject: [PATCH 13/36] refactor `down` and `wasLeft` management into respective `PressHandler` classes --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 67 +++++++++++++++++------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 55d8b6f55f..47b7e543d8 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -39,6 +39,7 @@ namespace osu.Game.Rulesets.Osu.Mods private double lastStateChangeTime; private DrawableOsuRuleset ruleset = null!; + private PressHandler pressHandler = null!; private bool hasReplay; private bool legacyReplay; @@ -56,10 +57,16 @@ namespace osu.Game.Rulesets.Osu.Mods if (osuInputManager.ReplayInputHandler != null) { hasReplay = true; + + Debug.Assert(ruleset.ReplayScore != null); legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; + + pressHandler = new LegacyReplayPressHandler(this); + return; } + pressHandler = new PressHandler(this); osuInputManager.AllowGameplayInputs = false; } @@ -131,20 +138,6 @@ namespace osu.Game.Rulesets.Osu.Mods isDownState = down; lastStateChangeTime = time; - // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. - if (legacyReplay) - { - if (!down) - { - osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); - return; - } - - osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); - wasLeft = !wasLeft; - return; - } - state = new ReplayState { PressedActions = new List() @@ -152,11 +145,53 @@ namespace osu.Game.Rulesets.Osu.Mods if (down) { - state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + pressHandler.HandlePress(wasLeft); wasLeft = !wasLeft; } + else + { + pressHandler.HandleRelease(wasLeft); + } + } + } - state.Apply(osuInputManager.CurrentState, osuInputManager); + private class PressHandler + { + protected readonly OsuModRelax Mod; + + public PressHandler(OsuModRelax mod) + { + Mod = mod; + } + + public virtual void HandlePress(bool wasLeft) + { + Mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + } + + public virtual void HandleRelease(bool wasLeft) + { + Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + } + } + + // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. + private class LegacyReplayPressHandler : PressHandler + { + public LegacyReplayPressHandler(OsuModRelax mod) + : base(mod) + { + } + + public override void HandlePress(bool wasLeft) + { + Mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + } + + public override void HandleRelease(bool wasLeft) + { + Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } } From cc733ea809f3eb1d0887bf8c92605581fd21a9b3 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 18:00:05 +0000 Subject: [PATCH 14/36] add inline comment for supposedly backwards ternary --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 47b7e543d8..a5643e5b49 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -191,6 +191,7 @@ namespace osu.Game.Rulesets.Osu.Mods public override void HandleRelease(bool wasLeft) { + // this intentionally releases right when `wasLeft` is true because `wasLeft` is set at point of press and not at point of release Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } From 5101979ac099b7e590e020020bf3b0a7bf134164 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Tue, 13 Feb 2024 00:34:06 +0000 Subject: [PATCH 15/36] only use `LegacyReplayPressHandler` on legacy replays --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index a5643e5b49..d2e4e0c669 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -61,7 +61,7 @@ namespace osu.Game.Rulesets.Osu.Mods Debug.Assert(ruleset.ReplayScore != null); legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; - pressHandler = new LegacyReplayPressHandler(this); + pressHandler = legacyReplay ? new LegacyReplayPressHandler(this) : new PressHandler(this); return; } From 22e9c4a3b59e414a881ceae5abc885389a0af5b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 13 Feb 2024 10:19:55 +0100 Subject: [PATCH 16/36] Use private interface rather than weird inheritance --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 38 ++++++++++++++--------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index d2e4e0c669..31511c01b8 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.Osu.Mods private double lastStateChangeTime; private DrawableOsuRuleset ruleset = null!; - private PressHandler pressHandler = null!; + private IPressHandler pressHandler = null!; private bool hasReplay; private bool legacyReplay; @@ -155,44 +155,52 @@ namespace osu.Game.Rulesets.Osu.Mods } } - private class PressHandler + private interface IPressHandler { - protected readonly OsuModRelax Mod; + void HandlePress(bool wasLeft); + void HandleRelease(bool wasLeft); + } + + private class PressHandler : IPressHandler + { + private readonly OsuModRelax mod; public PressHandler(OsuModRelax mod) { - Mod = mod; + this.mod = mod; } - public virtual void HandlePress(bool wasLeft) + public void HandlePress(bool wasLeft) { - Mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); - Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + mod.state.Apply(mod.osuInputManager.CurrentState, mod.osuInputManager); } - public virtual void HandleRelease(bool wasLeft) + public void HandleRelease(bool wasLeft) { - Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + mod.state.Apply(mod.osuInputManager.CurrentState, mod.osuInputManager); } } // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. - private class LegacyReplayPressHandler : PressHandler + private class LegacyReplayPressHandler : IPressHandler { + private readonly OsuModRelax mod; + public LegacyReplayPressHandler(OsuModRelax mod) - : base(mod) { + this.mod = mod; } - public override void HandlePress(bool wasLeft) + public void HandlePress(bool wasLeft) { - Mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); } - public override void HandleRelease(bool wasLeft) + public void HandleRelease(bool wasLeft) { // this intentionally releases right when `wasLeft` is true because `wasLeft` is set at point of press and not at point of release - Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); + mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } } From a96a66bc9e071f0b8bc2771f194965a23cc62d95 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 02:04:19 +0300 Subject: [PATCH 17/36] Add failing test case --- .../Navigation/TestSceneScreenNavigation.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index f59fbc75ac..8ff4fd5ecf 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -986,6 +986,29 @@ namespace osu.Game.Tests.Visual.Navigation } } + [Test] + public void TestPresentBeatmapAfterDeletion() + { + BeatmapSetInfo beatmap = null; + + Screens.Select.SongSelect songSelect = null; + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("delete selected beatmap", () => + { + beatmap = Game.Beatmap.Value.BeatmapSetInfo; + Game.BeatmapManager.Delete(Game.Beatmap.Value.BeatmapSetInfo); + }); + + AddUntilStep("nothing selected", () => Game.Beatmap.IsDefault); + AddStep("present deleted beatmap", () => Game.PresentBeatmap(beatmap)); + AddAssert("still nothing selected", () => Game.Beatmap.IsDefault); + } + private Func playToResults() { var player = playToCompletion(); From 35649d137ca9fea993c473b7b08d26f53ba13441 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 01:59:45 +0300 Subject: [PATCH 18/36] Ignore soft-deleted beatmaps when trying to present from notification --- osu.Game/OsuGame.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index c244708385..11798c22ff 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -630,6 +630,12 @@ namespace osu.Game var detachedSet = databasedSet.PerformRead(s => s.Detach()); + if (detachedSet.DeletePending) + { + Logger.Log("The requested beatmap has since been deleted.", LoggingTarget.Information); + return; + } + PerformFromScreen(screen => { // Find beatmaps that match our predicate. From 5267e0abf788da62e6eaaa1fed1ac35be0fcb428 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 03:38:49 +0300 Subject: [PATCH 19/36] Move comment author line to separate component --- .../Overlays/Comments/CommentAuthorLine.cs | 135 ++++++++++++++++++ osu.Game/Overlays/Comments/DrawableComment.cs | 102 +------------ 2 files changed, 139 insertions(+), 98 deletions(-) create mode 100644 osu.Game/Overlays/Comments/CommentAuthorLine.cs diff --git a/osu.Game/Overlays/Comments/CommentAuthorLine.cs b/osu.Game/Overlays/Comments/CommentAuthorLine.cs new file mode 100644 index 0000000000..b6b5dc00e1 --- /dev/null +++ b/osu.Game/Overlays/Comments/CommentAuthorLine.cs @@ -0,0 +1,135 @@ +// 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.Extensions.ObjectExtensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Localisation; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; +using osu.Game.Graphics.Sprites; +using osu.Game.Online.API.Requests.Responses; +using osu.Game.Resources.Localisation.Web; +using osuTK; + +namespace osu.Game.Overlays.Comments +{ + public partial class CommentAuthorLine : FillFlowContainer + { + private readonly Comment comment; + + private OsuSpriteText deletedLabel = null!; + + public CommentAuthorLine(Comment comment) + { + this.comment = comment; + } + + [BackgroundDependencyLoader] + private void load() + { + AutoSizeAxes = Axes.Both; + Direction = FillDirection.Horizontal; + Spacing = new Vector2(4, 0); + + Add(new LinkFlowContainer(s => s.Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold)) + { + AutoSizeAxes = Axes.Both + }.With(username => + { + if (comment.UserId.HasValue) + username.AddUserLink(comment.User); + else + username.AddText(comment.LegacyName!); + })); + + if (comment.Pinned) + Add(new PinnedCommentNotice()); + + Add(new ParentUsername(comment)); + + Add(deletedLabel = new OsuSpriteText + { + Alpha = 0f, + Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold), + Text = CommentsStrings.Deleted + }); + } + + public void MarkDeleted() + { + deletedLabel.Show(); + } + + private partial class PinnedCommentNotice : FillFlowContainer + { + public PinnedCommentNotice() + { + AutoSizeAxes = Axes.Both; + Direction = FillDirection.Horizontal; + Spacing = new Vector2(2, 0); + Children = new Drawable[] + { + new SpriteIcon + { + Icon = FontAwesome.Solid.Thumbtack, + Size = new Vector2(14), + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + }, + new OsuSpriteText + { + Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold), + Text = CommentsStrings.Pinned, + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + } + }; + } + } + + private partial class ParentUsername : FillFlowContainer, IHasTooltip + { + public LocalisableString TooltipText => getParentMessage(); + + private readonly Comment? parentComment; + + public ParentUsername(Comment comment) + { + parentComment = comment.ParentComment; + + AutoSizeAxes = Axes.Both; + Direction = FillDirection.Horizontal; + Spacing = new Vector2(3, 0); + Alpha = comment.ParentId == null ? 0 : 1; + Children = new Drawable[] + { + new SpriteIcon + { + Icon = FontAwesome.Solid.Reply, + Size = new Vector2(14), + }, + new OsuSpriteText + { + Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold, italics: true), + Text = parentComment?.User?.Username ?? parentComment?.LegacyName! + } + }; + } + + private LocalisableString getParentMessage() + { + if (parentComment == null) + return string.Empty; + + return parentComment.HasMessage ? parentComment.Message : parentComment.IsDeleted ? CommentsStrings.Deleted : string.Empty; + } + } + } +} diff --git a/osu.Game/Overlays/Comments/DrawableComment.cs b/osu.Game/Overlays/Comments/DrawableComment.cs index ceae17aa5d..70b1809c3e 100644 --- a/osu.Game/Overlays/Comments/DrawableComment.cs +++ b/osu.Game/Overlays/Comments/DrawableComment.cs @@ -4,12 +4,10 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics; using osu.Game.Graphics; -using osu.Framework.Graphics.Sprites; using osuTK; using osu.Game.Online.API.Requests.Responses; using osu.Game.Users.Drawables; using osu.Game.Graphics.Containers; -using osu.Framework.Graphics.Cursor; using osu.Framework.Bindables; using System.Linq; using osu.Game.Graphics.Sprites; @@ -21,7 +19,6 @@ using osu.Framework.Extensions.IEnumerableExtensions; using System.Collections.Specialized; using System.Diagnostics; using osu.Framework.Extensions.LocalisationExtensions; -using osu.Framework.Localisation; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Graphics.UserInterface; @@ -72,7 +69,7 @@ namespace osu.Game.Overlays.Comments private LinkFlowContainer actionsContainer = null!; private LoadingSpinner actionsLoading = null!; private DeletedCommentsCounter deletedCommentsCounter = null!; - private OsuSpriteText deletedLabel = null!; + private CommentAuthorLine author = null!; private GridContainer content = null!; private VotePill votePill = null!; private Container replyEditorContainer = null!; @@ -98,7 +95,6 @@ namespace osu.Game.Overlays.Comments [BackgroundDependencyLoader] private void load(OverlayColourProvider colourProvider, DrawableComment? parentComment) { - LinkFlowContainer username; FillFlowContainer info; CommentMarkdownContainer message; @@ -174,27 +170,7 @@ namespace osu.Game.Overlays.Comments }, Children = new Drawable[] { - new FillFlowContainer - { - AutoSizeAxes = Axes.Both, - Direction = FillDirection.Horizontal, - Spacing = new Vector2(10, 0), - Children = new[] - { - username = new LinkFlowContainer(s => s.Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold)) - { - AutoSizeAxes = Axes.Both - }, - Comment.Pinned ? new PinnedCommentNotice() : Empty(), - new ParentUsername(Comment), - deletedLabel = new OsuSpriteText - { - Alpha = 0f, - Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold), - Text = CommentsStrings.Deleted - } - } - }, + author = new CommentAuthorLine(Comment), message = new CommentMarkdownContainer { RelativeSizeAxes = Axes.X, @@ -218,7 +194,7 @@ namespace osu.Game.Overlays.Comments { new DrawableDate(Comment.CreatedAt, 12, false) { - Colour = colourProvider.Foreground1 + Colour = colourProvider.Foreground1, } } }, @@ -311,11 +287,6 @@ namespace osu.Game.Overlays.Comments } }; - if (Comment.UserId.HasValue) - username.AddUserLink(Comment.User); - else - username.AddText(Comment.LegacyName!); - if (Comment.EditedAt.HasValue && Comment.EditedUser != null) { var font = OsuFont.GetFont(size: 12, weight: FontWeight.Regular); @@ -400,7 +371,7 @@ namespace osu.Game.Overlays.Comments /// private void makeDeleted() { - deletedLabel.Show(); + author.MarkDeleted(); content.FadeColour(OsuColour.Gray(0.5f)); votePill.Hide(); actionsContainer.Expire(); @@ -547,70 +518,5 @@ namespace osu.Game.Overlays.Comments Top = 10 }; } - - private partial class PinnedCommentNotice : FillFlowContainer - { - public PinnedCommentNotice() - { - AutoSizeAxes = Axes.Both; - Direction = FillDirection.Horizontal; - Spacing = new Vector2(2, 0); - Children = new Drawable[] - { - new SpriteIcon - { - Icon = FontAwesome.Solid.Thumbtack, - Size = new Vector2(14), - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - }, - new OsuSpriteText - { - Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold), - Text = CommentsStrings.Pinned, - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - } - }; - } - } - - private partial class ParentUsername : FillFlowContainer, IHasTooltip - { - public LocalisableString TooltipText => getParentMessage(); - - private readonly Comment? parentComment; - - public ParentUsername(Comment comment) - { - parentComment = comment.ParentComment; - - AutoSizeAxes = Axes.Both; - Direction = FillDirection.Horizontal; - Spacing = new Vector2(3, 0); - Alpha = comment.ParentId == null ? 0 : 1; - Children = new Drawable[] - { - new SpriteIcon - { - Icon = FontAwesome.Solid.Reply, - Size = new Vector2(14), - }, - new OsuSpriteText - { - Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold, italics: true), - Text = parentComment?.User?.Username ?? parentComment?.LegacyName! - } - }; - } - - private LocalisableString getParentMessage() - { - if (parentComment == null) - return string.Empty; - - return parentComment.HasMessage ? parentComment.Message : parentComment.IsDeleted ? CommentsStrings.Deleted : string.Empty; - } - } } } From c4e358044a5f0478119c39309dd7525483847413 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 03:44:32 +0300 Subject: [PATCH 20/36] Add API models for comment page metadata --- .../API/Requests/Responses/CommentBundle.cs | 3 ++ .../API/Requests/Responses/CommentableMeta.cs | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 osu.Game/Online/API/Requests/Responses/CommentableMeta.cs diff --git a/osu.Game/Online/API/Requests/Responses/CommentBundle.cs b/osu.Game/Online/API/Requests/Responses/CommentBundle.cs index ae8b850723..cbff8bf76c 100644 --- a/osu.Game/Online/API/Requests/Responses/CommentBundle.cs +++ b/osu.Game/Online/API/Requests/Responses/CommentBundle.cs @@ -11,6 +11,9 @@ namespace osu.Game.Online.API.Requests.Responses { public class CommentBundle { + [JsonProperty(@"commentable_meta")] + public List CommentableMeta { get; set; } = new List(); + [JsonProperty(@"comments")] public List Comments { get; set; } diff --git a/osu.Game/Online/API/Requests/Responses/CommentableMeta.cs b/osu.Game/Online/API/Requests/Responses/CommentableMeta.cs new file mode 100644 index 0000000000..1084f1c900 --- /dev/null +++ b/osu.Game/Online/API/Requests/Responses/CommentableMeta.cs @@ -0,0 +1,28 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using Newtonsoft.Json; + +namespace osu.Game.Online.API.Requests.Responses +{ + public class CommentableMeta + { + [JsonProperty("id")] + public long Id { get; set; } + + [JsonProperty("owner_id")] + public long? OwnerId { get; set; } + + [JsonProperty("owner_title")] + public string? OwnerTitle { get; set; } + + [JsonProperty("title")] + public string Title { get; set; } = string.Empty; + + [JsonProperty("type")] + public string Type { get; set; } = string.Empty; + + [JsonProperty("url")] + public string Url { get; set; } = string.Empty; + } +} From 72c6134dbff17ba8a7a2ee82a741a61412bdfa1d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 03:46:19 +0300 Subject: [PATCH 21/36] Include commentable object metadata in comments --- osu.Game/Overlays/Comments/CommentsContainer.cs | 8 ++++---- osu.Game/Overlays/Comments/DrawableComment.cs | 4 +++- osu.Game/Overlays/Comments/ReplyCommentEditor.cs | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentsContainer.cs b/osu.Game/Overlays/Comments/CommentsContainer.cs index b4e9a80ff1..2e5f13aa99 100644 --- a/osu.Game/Overlays/Comments/CommentsContainer.cs +++ b/osu.Game/Overlays/Comments/CommentsContainer.cs @@ -301,7 +301,7 @@ namespace osu.Game.Overlays.Comments void addNewComment(Comment comment) { - var drawableComment = GetDrawableComment(comment); + var drawableComment = GetDrawableComment(comment, bundle.CommentableMeta); if (comment.ParentId == null) { @@ -333,7 +333,7 @@ namespace osu.Game.Overlays.Comments if (CommentDictionary.ContainsKey(comment.Id)) continue; - topLevelComments.Add(GetDrawableComment(comment)); + topLevelComments.Add(GetDrawableComment(comment, bundle.CommentableMeta)); } if (topLevelComments.Any()) @@ -351,12 +351,12 @@ namespace osu.Game.Overlays.Comments } } - public DrawableComment GetDrawableComment(Comment comment) + public DrawableComment GetDrawableComment(Comment comment, IReadOnlyList meta) { if (CommentDictionary.TryGetValue(comment.Id, out var existing)) return existing; - return CommentDictionary[comment.Id] = new DrawableComment(comment) + return CommentDictionary[comment.Id] = new DrawableComment(comment, meta) { ShowDeleted = { BindTarget = ShowDeleted }, Sort = { BindTarget = Sort }, diff --git a/osu.Game/Overlays/Comments/DrawableComment.cs b/osu.Game/Overlays/Comments/DrawableComment.cs index 70b1809c3e..afb8bdcc8b 100644 --- a/osu.Game/Overlays/Comments/DrawableComment.cs +++ b/osu.Game/Overlays/Comments/DrawableComment.cs @@ -39,6 +39,7 @@ namespace osu.Game.Overlays.Comments public Action RepliesRequested = null!; public readonly Comment Comment; + public readonly IReadOnlyList Meta; public readonly BindableBool ShowDeleted = new BindableBool(); public readonly Bindable Sort = new Bindable(); @@ -87,9 +88,10 @@ namespace osu.Game.Overlays.Comments [Resolved] private OnScreenDisplay? onScreenDisplay { get; set; } - public DrawableComment(Comment comment) + public DrawableComment(Comment comment, IReadOnlyList meta) { Comment = comment; + Meta = meta; } [BackgroundDependencyLoader] diff --git a/osu.Game/Overlays/Comments/ReplyCommentEditor.cs b/osu.Game/Overlays/Comments/ReplyCommentEditor.cs index dd4c35ef20..8e9e82507d 100644 --- a/osu.Game/Overlays/Comments/ReplyCommentEditor.cs +++ b/osu.Game/Overlays/Comments/ReplyCommentEditor.cs @@ -60,7 +60,7 @@ namespace osu.Game.Overlays.Comments foreach (var comment in cb.Comments) comment.ParentComment = parentComment; - var drawables = cb.Comments.Select(commentsContainer.GetDrawableComment).ToArray(); + var drawables = cb.Comments.Select(c => commentsContainer.GetDrawableComment(c, cb.CommentableMeta)).ToArray(); OnPost?.Invoke(drawables); OnCancel!.Invoke(); From 4d3b605e04d73ff69031d2d4c97ddcd937cb043f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 03:48:45 +0300 Subject: [PATCH 22/36] Add support for displaying "mapper" badges in comments --- .../Overlays/Comments/CommentAuthorLine.cs | 49 ++++++++++++++++++- osu.Game/Overlays/Comments/DrawableComment.cs | 2 +- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentAuthorLine.cs b/osu.Game/Overlays/Comments/CommentAuthorLine.cs index b6b5dc00e1..c269ab4c01 100644 --- a/osu.Game/Overlays/Comments/CommentAuthorLine.cs +++ b/osu.Game/Overlays/Comments/CommentAuthorLine.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; -using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; @@ -23,12 +22,14 @@ namespace osu.Game.Overlays.Comments public partial class CommentAuthorLine : FillFlowContainer { private readonly Comment comment; + private readonly IReadOnlyList meta; private OsuSpriteText deletedLabel = null!; - public CommentAuthorLine(Comment comment) + public CommentAuthorLine(Comment comment, IReadOnlyList meta) { this.comment = comment; + this.meta = meta; } [BackgroundDependencyLoader] @@ -49,6 +50,17 @@ namespace osu.Game.Overlays.Comments username.AddText(comment.LegacyName!); })); + var ownerMeta = meta.FirstOrDefault(m => m.Id == comment.CommentableId && m.Type == comment.CommentableType); + + if (ownerMeta?.OwnerId != null && ownerMeta.OwnerId == comment.UserId) + { + Add(new OwnerTitleBadge(ownerMeta.OwnerTitle ?? string.Empty) + { + // add top space to align with username + Margin = new MarginPadding { Top = 2f }, + }); + } + if (comment.Pinned) Add(new PinnedCommentNotice()); @@ -67,6 +79,39 @@ namespace osu.Game.Overlays.Comments deletedLabel.Show(); } + private partial class OwnerTitleBadge : CircularContainer + { + private readonly string title; + + public OwnerTitleBadge(string title) + { + this.title = title; + } + + [BackgroundDependencyLoader] + private void load(OverlayColourProvider colourProvider) + { + AutoSizeAxes = Axes.Both; + Masking = true; + + InternalChildren = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = colourProvider.Light1, + }, + new OsuSpriteText + { + Text = title, + Font = OsuFont.Default.With(size: 10, weight: FontWeight.Bold), + Margin = new MarginPadding { Vertical = 2, Horizontal = 5 }, + Colour = colourProvider.Background6, + }, + }; + } + } + private partial class PinnedCommentNotice : FillFlowContainer { public PinnedCommentNotice() diff --git a/osu.Game/Overlays/Comments/DrawableComment.cs b/osu.Game/Overlays/Comments/DrawableComment.cs index afb8bdcc8b..afd4b96c68 100644 --- a/osu.Game/Overlays/Comments/DrawableComment.cs +++ b/osu.Game/Overlays/Comments/DrawableComment.cs @@ -172,7 +172,7 @@ namespace osu.Game.Overlays.Comments }, Children = new Drawable[] { - author = new CommentAuthorLine(Comment), + author = new CommentAuthorLine(Comment, Meta), message = new CommentMarkdownContainer { RelativeSizeAxes = Axes.X, From c24af5bfeb65ea125ebdd1c37e8c3e0a296ef78e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 03:49:29 +0300 Subject: [PATCH 23/36] Add test coverage --- .../Online/TestSceneCommentsContainer.cs | 41 ++++++++- .../Visual/Online/TestSceneDrawableComment.cs | 88 ++++++++++--------- .../UserInterface/ThemeComparisonTestScene.cs | 30 ++++--- 3 files changed, 103 insertions(+), 56 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs index 3d8781d902..fd3552f675 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs @@ -170,6 +170,24 @@ namespace osu.Game.Tests.Visual.Online }); } + [Test] + public void TestPostAsOwner() + { + setUpCommentsResponse(getExampleComments()); + AddStep("show comments", () => commentsContainer.ShowComments(CommentableType.Beatmapset, 123)); + + setUpPostResponse(true); + AddStep("enter text", () => editorTextBox.Current.Value = "comm"); + AddStep("submit", () => commentsContainer.ChildrenOfType().Single().ChildrenOfType().First().TriggerClick()); + + AddUntilStep("comment sent", () => + { + string writtenText = editorTextBox.Current.Value; + var comment = commentsContainer.ChildrenOfType().LastOrDefault(); + return comment != null && comment.ChildrenOfType().Any(y => y.Text == writtenText) && comment.ChildrenOfType().Any(y => y.Text == "MAPPER"); + }); + } + private void setUpCommentsResponse(CommentBundle commentBundle) => AddStep("set up response", () => { @@ -183,7 +201,7 @@ namespace osu.Game.Tests.Visual.Online }; }); - private void setUpPostResponse() + private void setUpPostResponse(bool asOwner = false) => AddStep("set up response", () => { dummyAPI.HandleRequest = request => @@ -191,7 +209,7 @@ namespace osu.Game.Tests.Visual.Online if (!(request is CommentPostRequest req)) return false; - req.TriggerSuccess(new CommentBundle + var bundle = new CommentBundle { Comments = new List { @@ -202,9 +220,26 @@ namespace osu.Game.Tests.Visual.Online LegacyName = "FirstUser", CreatedAt = DateTimeOffset.Now, VotesCount = 98, + CommentableId = 2001, + CommentableType = "test", } } - }); + }; + + if (asOwner) + { + bundle.Comments[0].UserId = 1001; + bundle.Comments[0].User = new APIUser { Id = 1001, Username = "FirstUser" }; + bundle.CommentableMeta.Add(new CommentableMeta + { + Id = 2001, + OwnerId = 1001, + OwnerTitle = "MAPPER", + Type = "test", + }); + } + + req.TriggerSuccess(bundle); return true; }; }); diff --git a/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs b/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs index 5e83dd4fb3..6f09e4c1f6 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs @@ -4,62 +4,66 @@ #nullable disable using System; -using NUnit.Framework; -using osu.Framework.Allocation; +using System.Linq; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics.Containers; using osu.Game.Online.API.Requests.Responses; -using osu.Game.Overlays; using osu.Game.Overlays.Comments; +using osu.Game.Tests.Visual.UserInterface; namespace osu.Game.Tests.Visual.Online { - public partial class TestSceneDrawableComment : OsuTestScene + public partial class TestSceneDrawableComment : ThemeComparisonTestScene { - [Cached] - private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Purple); - - private Container container; - - [SetUp] - public void SetUp() => Schedule(() => + public TestSceneDrawableComment() + : base(false) { - Children = new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = colourProvider.Background4, - }, - container = new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - }, - }; - }); - - [TestCaseSource(nameof(comments))] - public void TestComment(string description, string text) - { - AddStep(description, () => - { - comment.Pinned = description == "Pinned"; - comment.Message = text; - container.Add(new DrawableComment(comment)); - }); } - private static readonly Comment comment = new Comment + protected override Drawable CreateContent() => new OsuScrollContainer(Direction.Vertical) { - Id = 1, - LegacyName = "Test User", - CreatedAt = DateTimeOffset.Now, - VotesCount = 0, + RelativeSizeAxes = Axes.Both, + Child = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + ChildrenEnumerable = comments.Select(info => + { + var comment = new Comment + { + Id = 1, + UserId = 1000, + User = new APIUser { Id = 1000, Username = "Someone" }, + CreatedAt = DateTimeOffset.Now, + VotesCount = 0, + Pinned = info[0] == "Pinned", + Message = info[1], + CommentableId = 2001, + CommentableType = "test" + }; + + return new[] + { + new DrawableComment(comment, Array.Empty()), + new DrawableComment(comment, new[] + { + new CommentableMeta + { + Id = 2001, + OwnerId = comment.UserId, + OwnerTitle = "MAPPER", + Type = "test", + }, + new CommentableMeta { Title = "Other Meta" }, + }), + }; + }).SelectMany(c => c) + } }; - private static object[] comments = + private static readonly string[][] comments = { new[] { "Plain", "This is plain comment" }, new[] { "Pinned", "This is pinned comment" }, diff --git a/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs b/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs index 2c894eacab..f0c4b5543f 100644 --- a/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs +++ b/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs @@ -14,31 +14,39 @@ namespace osu.Game.Tests.Visual.UserInterface { public abstract partial class ThemeComparisonTestScene : OsuGridTestScene { - protected ThemeComparisonTestScene() - : base(1, 2) + private readonly bool showNoColourProvider; + + protected ThemeComparisonTestScene(bool showNoColourProvider = true) + : base(1, showNoColourProvider ? 2 : 1) { + this.showNoColourProvider = showNoColourProvider; } [BackgroundDependencyLoader] private void load(OsuColour colours) { - Cell(0, 0).AddRange(new[] + if (showNoColourProvider) { - new Box + Cell(0, 0).AddRange(new[] { - RelativeSizeAxes = Axes.Both, - Colour = colours.GreySeaFoam - }, - CreateContent() - }); + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = colours.GreySeaFoam + }, + CreateContent() + }); + } } protected void CreateThemedContent(OverlayColourScheme colourScheme) { var colourProvider = new OverlayColourProvider(colourScheme); - Cell(0, 1).Clear(); - Cell(0, 1).Add(new DependencyProvidingContainer + int col = showNoColourProvider ? 1 : 0; + + Cell(0, col).Clear(); + Cell(0, col).Add(new DependencyProvidingContainer { RelativeSizeAxes = Axes.Both, CachedDependencies = new (Type, object)[] From 02de9122d4466d2a30a915bbbc19e5f143fec824 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 07:17:05 +0300 Subject: [PATCH 24/36] Remove behaviour of flipping catcher plate on direction change --- .../TestSceneCatchSkinConfiguration.cs | 111 ------------------ .../Skinning/CatchSkinConfiguration.cs | 13 -- .../Legacy/CatchLegacySkinTransformer.cs | 13 -- osu.Game.Rulesets.Catch/UI/Catcher.cs | 10 +- 4 files changed, 1 insertion(+), 146 deletions(-) delete mode 100644 osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs delete mode 100644 osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs deleted file mode 100644 index e2fc31d869..0000000000 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -#nullable disable - -using System.Linq; -using NUnit.Framework; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Testing; -using osu.Framework.Utils; -using osu.Game.Beatmaps; -using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Rulesets.Catch.Judgements; -using osu.Game.Rulesets.Catch.Objects; -using osu.Game.Rulesets.Catch.Objects.Drawables; -using osu.Game.Rulesets.Catch.Skinning; -using osu.Game.Rulesets.Catch.UI; -using osu.Game.Skinning; -using osu.Game.Tests.Visual; -using Direction = osu.Game.Rulesets.Catch.UI.Direction; - -namespace osu.Game.Rulesets.Catch.Tests -{ - public partial class TestSceneCatchSkinConfiguration : OsuTestScene - { - private Catcher catcher; - - private readonly Container container; - - public TestSceneCatchSkinConfiguration() - { - Add(container = new Container { RelativeSizeAxes = Axes.Both }); - } - - [TestCase(false)] - [TestCase(true)] - public void TestCatcherPlateFlipping(bool flip) - { - AddStep("setup catcher", () => - { - var skin = new TestSkin { FlipCatcherPlate = flip }; - container.Child = new SkinProvidingContainer(skin) - { - Child = catcher = new Catcher(new DroppedObjectContainer()) - { - Anchor = Anchor.Centre - } - }; - }); - - Fruit fruit = new Fruit(); - - AddStep("catch fruit", () => catchFruit(fruit, 20)); - - float position = 0; - - AddStep("record fruit position", () => position = getCaughtObjectPosition(fruit)); - - AddStep("face left", () => catcher.VisualDirection = Direction.Left); - - if (flip) - AddAssert("fruit position changed", () => !Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - else - AddAssert("fruit position unchanged", () => Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - - AddStep("face right", () => catcher.VisualDirection = Direction.Right); - - AddAssert("fruit position restored", () => Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - } - - private float getCaughtObjectPosition(Fruit fruit) - { - var caughtObject = catcher.ChildrenOfType().Single(c => c.HitObject == fruit); - return caughtObject.Parent!.ToSpaceOfOtherDrawable(caughtObject.Position, catcher).X; - } - - private void catchFruit(Fruit fruit, float x) - { - fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - var drawableFruit = new DrawableFruit(fruit) { X = x }; - var judgement = fruit.CreateJudgement(); - catcher.OnNewResult(drawableFruit, new CatchJudgementResult(fruit, judgement) - { - Type = judgement.MaxResult - }); - } - - private class TestSkin : TrianglesSkin - { - public bool FlipCatcherPlate { get; set; } - - public TestSkin() - : base(null!) - { - } - - public override IBindable GetConfig(TLookup lookup) - { - if (lookup is CatchSkinConfiguration config) - { - if (config == CatchSkinConfiguration.FlipCatcherPlate) - return SkinUtils.As(new Bindable(FlipCatcherPlate)); - } - - return base.GetConfig(lookup); - } - } - } -} diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs deleted file mode 100644 index ea8d742b1a..0000000000 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs +++ /dev/null @@ -1,13 +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.Rulesets.Catch.Skinning -{ - public enum CatchSkinConfiguration - { - /// - /// Whether the contents of the catcher plate should be visually flipped when the catcher direction is changed. - /// - FlipCatcherPlate - } -} diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index fb8af9bdb6..d1ef47cf17 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -122,19 +122,6 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy result.Value = LegacyColourCompatibility.DisallowZeroAlpha(result.Value); return (IBindable)result; - - case CatchSkinConfiguration config: - switch (config) - { - case CatchSkinConfiguration.FlipCatcherPlate: - // Don't flip catcher plate contents if the catcher is provided by this legacy skin. - if (GetDrawableComponent(new CatchSkinComponentLookup(CatchSkinComponents.Catcher)) != null) - return (IBindable)new Bindable(); - - break; - } - - break; } return base.GetConfig(lookup); diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 147850a9b7..dca01fc61a 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -112,11 +112,6 @@ namespace osu.Game.Rulesets.Catch.UI public Vector2 BodyScale => Scale * body.Scale; - /// - /// Whether the contents of the catcher plate should be visually flipped when the catcher direction is changed. - /// - private bool flipCatcherPlate; - /// /// Width of the area that can be used to attempt catches during gameplay. /// @@ -339,8 +334,6 @@ namespace osu.Game.Rulesets.Catch.UI skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? DEFAULT_HYPER_DASH_COLOUR; - flipCatcherPlate = skin.GetConfig(CatchSkinConfiguration.FlipCatcherPlate)?.Value ?? true; - runHyperDashStateTransition(HyperDashing); } @@ -352,8 +345,7 @@ namespace osu.Game.Rulesets.Catch.UI body.Scale = scaleFromDirection; // Inverse of catcher scale is applied here, as catcher gets scaled by circle size and so do the incoming fruit. - caughtObjectContainer.Scale = (1 / Scale.X) * (flipCatcherPlate ? scaleFromDirection : Vector2.One); - hitExplosionContainer.Scale = flipCatcherPlate ? scaleFromDirection : Vector2.One; + caughtObjectContainer.Scale = new Vector2(1 / Scale.X); // Correct overshooting. if ((hyperDashDirection > 0 && hyperDashTargetPosition < X) || From 2981ebe3d5725e4e26ad2d7b9ec22b9afe87fdf5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 14 Feb 2024 17:13:44 +0900 Subject: [PATCH 25/36] Fix inspection --- .../Rulesets/TestSceneRulesetSkinProvidingContainer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs b/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs index 11f3fe660d..981258e8d1 100644 --- a/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs +++ b/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs @@ -43,13 +43,13 @@ namespace osu.Game.Tests.Rulesets AddStep("setup provider", () => { - var rulesetSkinProvider = new RulesetSkinProvidingContainer(Ruleset.Value.CreateInstance(), Beatmap.Value.Beatmap, Beatmap.Value.Skin); - - rulesetSkinProvider.Add(requester = new SkinRequester()); - + requester = new SkinRequester(); requester.OnLoadAsync += () => textureOnLoad = requester.GetTexture("test-image"); - Child = rulesetSkinProvider; + Child = new RulesetSkinProvidingContainer(Ruleset.Value.CreateInstance(), Beatmap.Value.Beatmap, Beatmap.Value.Skin) + { + requester + }; }); AddAssert("requester got correct initial texture", () => textureOnLoad != null); From d189fa0f6907afbe9f8a83a10c6a712e793a9efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 11:53:41 +0100 Subject: [PATCH 26/36] Rename flag --- .../Visual/UserInterface/ThemeComparisonTestScene.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs b/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs index f0c4b5543f..3177695f44 100644 --- a/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs +++ b/osu.Game.Tests/Visual/UserInterface/ThemeComparisonTestScene.cs @@ -14,18 +14,18 @@ namespace osu.Game.Tests.Visual.UserInterface { public abstract partial class ThemeComparisonTestScene : OsuGridTestScene { - private readonly bool showNoColourProvider; + private readonly bool showWithoutColourProvider; - protected ThemeComparisonTestScene(bool showNoColourProvider = true) - : base(1, showNoColourProvider ? 2 : 1) + protected ThemeComparisonTestScene(bool showWithoutColourProvider = true) + : base(1, showWithoutColourProvider ? 2 : 1) { - this.showNoColourProvider = showNoColourProvider; + this.showWithoutColourProvider = showWithoutColourProvider; } [BackgroundDependencyLoader] private void load(OsuColour colours) { - if (showNoColourProvider) + if (showWithoutColourProvider) { Cell(0, 0).AddRange(new[] { @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Visual.UserInterface { var colourProvider = new OverlayColourProvider(colourScheme); - int col = showNoColourProvider ? 1 : 0; + int col = showWithoutColourProvider ? 1 : 0; Cell(0, col).Clear(); Cell(0, col).Add(new DependencyProvidingContainer From 8312f92b4ea24a981b6916f5207aaaf747471546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 12:24:28 +0100 Subject: [PATCH 27/36] Use better value for alignment --- osu.Game/Overlays/Comments/CommentAuthorLine.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Comments/CommentAuthorLine.cs b/osu.Game/Overlays/Comments/CommentAuthorLine.cs index c269ab4c01..1f6fef4df3 100644 --- a/osu.Game/Overlays/Comments/CommentAuthorLine.cs +++ b/osu.Game/Overlays/Comments/CommentAuthorLine.cs @@ -57,7 +57,7 @@ namespace osu.Game.Overlays.Comments Add(new OwnerTitleBadge(ownerMeta.OwnerTitle ?? string.Empty) { // add top space to align with username - Margin = new MarginPadding { Top = 2f }, + Margin = new MarginPadding { Top = 1f }, }); } From 68247fa022c0ba3cbab5fe3c92457e11fb48c4a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 13:21:37 +0100 Subject: [PATCH 28/36] Fix typo in json property name Would cause the mapper badge to never actually be shown in the real world. --- osu.Game/Online/API/Requests/Responses/Comment.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/API/Requests/Responses/Comment.cs b/osu.Game/Online/API/Requests/Responses/Comment.cs index 907632186c..e6a5559d1f 100644 --- a/osu.Game/Online/API/Requests/Responses/Comment.cs +++ b/osu.Game/Online/API/Requests/Responses/Comment.cs @@ -33,7 +33,7 @@ namespace osu.Game.Online.API.Requests.Responses [JsonProperty(@"votes_count")] public int VotesCount { get; set; } - [JsonProperty(@"commenatble_type")] + [JsonProperty(@"commentable_type")] public string CommentableType { get; set; } = null!; [JsonProperty(@"commentable_id")] From f0f37df67fea4f20cc117012457ffd2c9b3bfec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 15:27:17 +0100 Subject: [PATCH 29/36] Revert unnecessary change --- osu.Game/Screens/Select/FilterQueryParser.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 24580c9e96..5fb5859a3b 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -121,8 +121,7 @@ namespace osu.Game.Screens.Select value.EndsWith("ms", StringComparison.Ordinal) ? 1 : value.EndsWith('s') ? 1000 : value.EndsWith('m') ? 60000 : - value.EndsWith('h') ? 3600000 : - value.EndsWith('d') ? 86400000 : 1000; + value.EndsWith('h') ? 3600000 : 1000; private static bool tryParseFloatWithPoint(string value, out float result) => float.TryParse(value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out result); From d7dfc8b88aa5081a6765898a94097050c8441c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 15:55:57 +0100 Subject: [PATCH 30/36] Add failing test coverage for empty date filter not parsing --- osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 81a73fc99f..814b26a231 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -488,7 +488,8 @@ namespace osu.Game.Tests.NonVisual.Filtering new object[] { "0:3:" }, new object[] { "\"three days\"" }, new object[] { "0.1y0.1M2d" }, - new object[] { "0.99y0.99M2d" } + new object[] { "0.99y0.99M2d" }, + new object[] { string.Empty } }; [Test] From c24328dda347f8cd2196267790e225bb3f038d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 15:56:32 +0100 Subject: [PATCH 31/36] Abandon date filter if no meaningful time bound found during parsing --- osu.Game/Screens/Select/FilterQueryParser.cs | 23 +++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 5fb5859a3b..3612a84ff9 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -394,7 +394,8 @@ namespace osu.Game.Screens.Select if (match == null) return false; - DateTimeOffset dateTimeOffset = DateTimeOffset.Now; + DateTimeOffset? dateTimeOffset = null; + DateTimeOffset now = DateTimeOffset.Now; try { @@ -410,27 +411,27 @@ namespace osu.Game.Screens.Select switch (key) { case "seconds": - dateTimeOffset = dateTimeOffset.AddSeconds(-length); + dateTimeOffset = (dateTimeOffset ?? now).AddSeconds(-length); break; case "minutes": - dateTimeOffset = dateTimeOffset.AddMinutes(-length); + dateTimeOffset = (dateTimeOffset ?? now).AddMinutes(-length); break; case "hours": - dateTimeOffset = dateTimeOffset.AddHours(-length); + dateTimeOffset = (dateTimeOffset ?? now).AddHours(-length); break; case "days": - dateTimeOffset = dateTimeOffset.AddDays(-length); + dateTimeOffset = (dateTimeOffset ?? now).AddDays(-length); break; case "months": - dateTimeOffset = dateTimeOffset.AddMonths(-(int)length); + dateTimeOffset = (dateTimeOffset ?? now).AddMonths(-(int)length); break; case "years": - dateTimeOffset = dateTimeOffset.AddYears(-(int)length); + dateTimeOffset = (dateTimeOffset ?? now).AddYears(-(int)length); break; } } @@ -438,11 +439,13 @@ namespace osu.Game.Screens.Select } catch (ArgumentOutOfRangeException) { - dateTimeOffset = DateTimeOffset.MinValue; - dateTimeOffset = dateTimeOffset.AddMilliseconds(1); + dateTimeOffset = DateTimeOffset.MinValue.AddMilliseconds(1); } - return tryUpdateCriteriaRange(ref dateRange, reverseInequalityOperator(op), dateTimeOffset); + if (!dateTimeOffset.HasValue) + return false; + + return tryUpdateCriteriaRange(ref dateRange, reverseInequalityOperator(op), dateTimeOffset.Value); } // Function to reverse an Operator From a8ae0a032f67900b8466263ac3ceb5f1b79b95d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 15:58:38 +0100 Subject: [PATCH 32/36] Simplify parsing --- osu.Game/Screens/Select/FilterQueryParser.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 3612a84ff9..17af0ee8ba 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -403,9 +403,12 @@ namespace osu.Game.Screens.Select foreach (string key in keys) { - if (match[key].Success) + if (!match.TryGetValue(key, out var group) || !group.Success) + continue; + + if (group.Success) { - if (!tryParseDoubleWithPoint(match[key].Value, out double length)) + if (!tryParseDoubleWithPoint(group.Value, out double length)) return false; switch (key) From f1d69abdc8d56502b23beccf62827518275cdc90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 15:59:07 +0100 Subject: [PATCH 33/36] Rename test --- osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 814b26a231..a5aac0a4ce 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -532,7 +532,7 @@ namespace osu.Game.Tests.NonVisual.Filtering } [Test] - public void TestOutofrangeDateQuery() + public void TestOutOfRangeDateQuery() { const string query = "played<10000y"; var filterCriteria = new FilterCriteria(); From 414066fd34f18db433fc4c3c2fc10f3624989924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 16:07:42 +0100 Subject: [PATCH 34/36] Inline problematic function (and rename things to make more sense) --- osu.Game/Screens/Select/FilterQueryParser.cs | 67 +++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 17af0ee8ba..278ca1ac5f 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -63,7 +63,7 @@ namespace osu.Game.Screens.Select case "played": case "lastplayed": - return tryUpdateDateRange(ref criteria.LastPlayed, op, value); + return tryUpdateDateAgoRange(ref criteria.LastPlayed, op, value); case "divisor": return TryUpdateCriteriaRange(ref criteria.BeatDivisor, op, value, tryParseInt); @@ -381,10 +381,42 @@ namespace osu.Game.Screens.Select return tryUpdateCriteriaRange(ref criteria.Length, op, totalLength, minScale / 2.0); } - private static bool tryUpdateDateRange(ref FilterCriteria.OptionalRange dateRange, Operator op, string val) + /// + /// This function is intended for parsing "days / months / years ago" type filters. + /// + private static bool tryUpdateDateAgoRange(ref FilterCriteria.OptionalRange dateRange, Operator op, string val) { - if (op == Operator.Equal) - return false; + switch (op) + { + case Operator.Equal: + // an equality filter is difficult to define for support here. + // if "3 months 2 days ago" means a single concrete time instant, such a filter is basically useless. + // if it means a range of 24 hours, then that is annoying to write and also comes with its own implications + // (does it mean "time instant 3 months 2 days ago, within 12 hours of tolerance either direction"? + // does it mean "the full calendar day, from midnight to midnight, 3 months 2 days ago"?) + // as such, for simplicity, just refuse to support this. + return false; + + // for the remaining operators, since the value provided to this function is an "ago" type value + // (as in, referring to some amount of time back), + // we'll want to flip the operator, such that `>5d` means "more than five days ago", as in "*before* five days ago", + // as intended by the user. + case Operator.Less: + op = Operator.Greater; + break; + + case Operator.LessOrEqual: + op = Operator.GreaterOrEqual; + break; + + case Operator.Greater: + op = Operator.Less; + break; + + case Operator.GreaterOrEqual: + op = Operator.LessOrEqual; + break; + } GroupCollection? match = null; @@ -448,32 +480,7 @@ namespace osu.Game.Screens.Select if (!dateTimeOffset.HasValue) return false; - return tryUpdateCriteriaRange(ref dateRange, reverseInequalityOperator(op), dateTimeOffset.Value); - } - - // Function to reverse an Operator - private static Operator reverseInequalityOperator(Operator ope) - { - switch (ope) - { - default: - throw new ArgumentOutOfRangeException(nameof(ope), $"Unsupported operator {ope}"); - - case Operator.Equal: - return Operator.Equal; - - case Operator.Greater: - return Operator.Less; - - case Operator.GreaterOrEqual: - return Operator.LessOrEqual; - - case Operator.Less: - return Operator.Greater; - - case Operator.LessOrEqual: - return Operator.GreaterOrEqual; - } + return tryUpdateCriteriaRange(ref dateRange, op, dateTimeOffset.Value); } } } From f7bea00564b7ac537b92a4eb9c4fe395bffb27d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 16:19:32 +0100 Subject: [PATCH 35/36] Improve test coverage --- .../Filtering/FilterQueryParserTest.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index a5aac0a4ce..12d6060351 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -508,8 +508,11 @@ namespace osu.Game.Tests.NonVisual.Filtering const string query = "played>50"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); - Assert.AreEqual(false, filterCriteria.LastPlayed.Max == null); - Assert.AreEqual(true, filterCriteria.LastPlayed.Min == null); + Assert.That(filterCriteria.LastPlayed.Max, Is.Not.Null); + Assert.That(filterCriteria.LastPlayed.Min, Is.Null); + // the parser internally references `DateTimeOffset.Now`, so to not make things too annoying for tests, just assume some tolerance + // (irrelevant in proportion to the actual filter proscribed). + Assert.That(filterCriteria.LastPlayed.Max, Is.EqualTo(DateTimeOffset.Now.AddDays(-50)).Within(TimeSpan.FromSeconds(5))); } [Test] @@ -518,8 +521,25 @@ namespace osu.Game.Tests.NonVisual.Filtering const string query = "played<50"; var filterCriteria = new FilterCriteria(); FilterQueryParser.ApplyQueries(filterCriteria, query); - Assert.AreEqual(true, filterCriteria.LastPlayed.Max == null); - Assert.AreEqual(false, filterCriteria.LastPlayed.Min == null); + Assert.That(filterCriteria.LastPlayed.Max, Is.Null); + Assert.That(filterCriteria.LastPlayed.Min, Is.Not.Null); + // the parser internally references `DateTimeOffset.Now`, so to not make things too annoying for tests, just assume some tolerance + // (irrelevant in proportion to the actual filter proscribed). + Assert.That(filterCriteria.LastPlayed.Min, Is.EqualTo(DateTimeOffset.Now.AddDays(-50)).Within(TimeSpan.FromSeconds(5))); + } + + [Test] + public void TestBothSidesDateQuery() + { + const string query = "played>3M played<1y6M"; + var filterCriteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(filterCriteria, query); + Assert.That(filterCriteria.LastPlayed.Min, Is.Not.Null); + Assert.That(filterCriteria.LastPlayed.Max, Is.Not.Null); + // the parser internally references `DateTimeOffset.Now`, so to not make things too annoying for tests, just assume some tolerance + // (irrelevant in proportion to the actual filter proscribed). + Assert.That(filterCriteria.LastPlayed.Min, Is.EqualTo(DateTimeOffset.Now.AddYears(-1).AddMonths(-6)).Within(TimeSpan.FromSeconds(5))); + Assert.That(filterCriteria.LastPlayed.Max, Is.EqualTo(DateTimeOffset.Now.AddMonths(-3)).Within(TimeSpan.FromSeconds(5))); } [Test] From ae9a2661ace43a96a4fbf26072ed3efd0dc0ba54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Feb 2024 16:20:47 +0100 Subject: [PATCH 36/36] Sprinkle some raw string prefixes --- osu.Game/Screens/Select/FilterQueryParser.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 278ca1ac5f..2c4077dacf 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -431,7 +431,7 @@ namespace osu.Game.Screens.Select try { - List keys = new List { "seconds", "minutes", "hours", "days", "months", "years" }; + List keys = new List { @"seconds", @"minutes", @"hours", @"days", @"months", @"years" }; foreach (string key in keys) { @@ -445,27 +445,27 @@ namespace osu.Game.Screens.Select switch (key) { - case "seconds": + case @"seconds": dateTimeOffset = (dateTimeOffset ?? now).AddSeconds(-length); break; - case "minutes": + case @"minutes": dateTimeOffset = (dateTimeOffset ?? now).AddMinutes(-length); break; - case "hours": + case @"hours": dateTimeOffset = (dateTimeOffset ?? now).AddHours(-length); break; - case "days": + case @"days": dateTimeOffset = (dateTimeOffset ?? now).AddDays(-length); break; - case "months": + case @"months": dateTimeOffset = (dateTimeOffset ?? now).AddMonths(-(int)length); break; - case "years": + case @"years": dateTimeOffset = (dateTimeOffset ?? now).AddYears(-(int)length); break; }