From 292273edbe6e14671c6ef03e6a44504b20736c23 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Sat, 23 Sep 2023 06:31:26 +0300 Subject: [PATCH 01/51] Add test scene --- osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs b/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs index ac80463d3a..97e1cae11c 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneDrawableComment.cs @@ -64,7 +64,8 @@ namespace osu.Game.Tests.Visual.Online new[] { "Plain", "This is plain comment" }, new[] { "Pinned", "This is pinned comment" }, new[] { "Link", "Please visit https://osu.ppy.sh" }, - + new[] { "Big Image", "![](Backgrounds/bg1)" }, + new[] { "Small Image", "![](Cursor/cursortrail)" }, new[] { "Heading", @"# Heading 1 From 793d1bf9706dcbf6b26363b3085283898bc3033c Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Mon, 25 Sep 2023 12:52:49 +0200 Subject: [PATCH 02/51] Add ability to search for difficulty names --- .../Select/Carousel/CarouselBeatmap.cs | 2 + osu.Game/Screens/Select/FilterCriteria.cs | 47 ++++++++++++------- osu.Game/Screens/Select/FilterQueryParser.cs | 19 ++++++++ 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 6917bd1da2..f433e71cc3 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -64,6 +64,8 @@ namespace osu.Game.Screens.Select.Carousel if (!match) return false; + match &= criteria.DifficultySearchTerms.All(term => term.Matches(BeatmapInfo.DifficultyName)); + if (criteria.SearchTerms.Length > 0) { var searchableTerms = BeatmapInfo.GetSearchableTerms(); diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index a2ae114126..aa3909fa0e 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -46,6 +46,7 @@ namespace osu.Game.Screens.Select }; public OptionalTextFilter[] SearchTerms = Array.Empty(); + public OptionalTextFilter[] DifficultySearchTerms = Array.Empty(); public RulesetInfo? Ruleset; public bool AllowConvertedBeatmaps; @@ -64,24 +65,7 @@ namespace osu.Game.Screens.Select { searchText = value; - List terms = new List(); - - string remainingText = value; - - // First handle quoted segments to ensure we keep inline spaces in exact matches. - foreach (Match quotedSegment in Regex.Matches(searchText, "(\"[^\"]+\"[!]?)")) - { - terms.Add(new OptionalTextFilter { SearchTerm = quotedSegment.Value }); - remainingText = remainingText.Replace(quotedSegment.Value, string.Empty); - } - - // Then handle the rest splitting on any spaces. - terms.AddRange(remainingText.Split(' ', StringSplitOptions.RemoveEmptyEntries).Select(s => new OptionalTextFilter - { - SearchTerm = s - })); - - SearchTerms = terms.ToArray(); + SearchTerms = getTermsFromSearchText(value); SearchNumber = null; @@ -90,6 +74,11 @@ namespace osu.Game.Screens.Select } } + public string DifficultySearchText + { + set => DifficultySearchTerms = getTermsFromSearchText(value); + } + /// /// Hashes from the to filter to. /// @@ -97,6 +86,28 @@ namespace osu.Game.Screens.Select public IRulesetFilterCriteria? RulesetCriteria { get; set; } + private static OptionalTextFilter[] getTermsFromSearchText(string searchText) + { + List terms = new List(); + + string remainingText = searchText; + + // First handle quoted segments to ensure we keep inline spaces in exact matches. + foreach (Match quotedSegment in Regex.Matches(searchText, "(\"[^\"]+\"[!]?)")) + { + terms.Add(new OptionalTextFilter { SearchTerm = quotedSegment.Value }); + remainingText = remainingText.Replace(quotedSegment.Value, string.Empty); + } + + // Then handle the rest splitting on any spaces. + terms.AddRange(remainingText.Split(' ', StringSplitOptions.RemoveEmptyEntries).Select(s => new OptionalTextFilter + { + SearchTerm = s + })); + + return terms.ToArray(); + } + public struct OptionalRange : IEquatable> where T : struct { diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 1238173b41..797493189d 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -19,8 +19,27 @@ namespace osu.Game.Screens.Select @"\b(?\w+)(?(:|=|(>|<)(:|=)?))(?("".*""[!]?)|(\S*))", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex difficulty_query_syntax_regex = new Regex( + @"(\s|^)((\[(?>\[(?)|[^[\]]+|\](?<-level>))*(?(level)(?!))\](\s|$))|(\[.*))", + RegexOptions.Compiled); + internal static void ApplyQueries(FilterCriteria criteria, string query) { + foreach (Match match in difficulty_query_syntax_regex.Matches(query)) + { + // Trim the first character because it's always '[' (ignoring spaces) + string cleanDifficultyQuery = match.Value.Trim(' ')[1..]; + + if (cleanDifficultyQuery.EndsWith(']')) + cleanDifficultyQuery = cleanDifficultyQuery[..^1]; + + criteria.DifficultySearchText = cleanDifficultyQuery; + + // Insert whitespace if necessary so that the words before and after the difficulty query aren't joined together. + bool insertWhitespace = match.Value.StartsWith(' ') && match.Value.EndsWith(' '); + query = query.Replace(match.Value, insertWhitespace ? " " : ""); + } + foreach (Match match in query_syntax_regex.Matches(query)) { string key = match.Groups["key"].Value.ToLowerInvariant(); From ab57cbf6f5a1d438963469f4dea27974755559e7 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Mon, 25 Sep 2023 12:53:17 +0200 Subject: [PATCH 03/51] Add `TestSceneDifficultySearch` --- .../SongSelect/TestSceneDifficultySearch.cs | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs diff --git a/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs b/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs new file mode 100644 index 0000000000..63963d1101 --- /dev/null +++ b/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs @@ -0,0 +1,71 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Carousel; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.SongSelect +{ + public partial class TestSceneDifficultySearch : OsuTestScene + { + private static readonly (string title, string difficultyName)[] beatmaps = + { + ("Title1", "Diff1"), + ("Title1", "Diff2"), + ("My[Favourite]Song", "Expert"), + ("Title", "My Favourite Diff"), + ("Another One", "diff ]with [[] brackets]]]"), + }; + + [TestCase("[1]", new[] { 0 })] + [TestCase("[1", new[] { 0 })] + [TestCase("My[Favourite", new[] { 2 })] + [TestCase("My[Favourite]", new[] { 2 })] + [TestCase("My[Favourite]Song", new[] { 2 })] + [TestCase("Favourite]", new[] { 2 })] + [TestCase("[Diff", new[] { 0, 1, 3, 4 })] + [TestCase("[Diff]", new[] { 0, 1, 3, 4 })] + [TestCase("[Favourite]", new[] { 3 })] + [TestCase("Title1 [Diff]", new[] { 0, 1 })] + [TestCase("Title1[Diff]", new int[] { })] + [TestCase("[diff ]with]", new[] { 4 })] + [TestCase("[diff ]with [[] brackets]]]]", new[] { 4 })] + public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) + { + var carouselBeatmaps = createCarouselBeatmaps().ToList(); + + AddStep("filter beatmaps", () => + { + var criteria = new FilterCriteria(); + FilterQueryParser.ApplyQueries(criteria, query); + carouselBeatmaps.ForEach(b => b.Filter(criteria)); + }); + + AddAssert("filtered correctly", () => carouselBeatmaps.All(b => + { + int index = carouselBeatmaps.IndexOf(b); + + bool filtered = b.Filtered.Value; + + return filtered != expectedBeatmapIndexes.Contains(index); + })); + } + + private static IEnumerable createCarouselBeatmaps() + { + return beatmaps.Select(info => new CarouselBeatmap(new BeatmapInfo + { + Metadata = new BeatmapMetadata + { + Title = info.title + }, + DifficultyName = info.difficultyName + })); + } + } +} From 74896fd6b2f271dfd5ec01c01aa04526a72125c3 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Mon, 25 Sep 2023 13:38:47 +0200 Subject: [PATCH 04/51] Fix multiple difficulty search queries not working in some cases. --- osu.Game/Screens/Select/FilterQueryParser.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 797493189d..58e25adb5b 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -25,8 +25,12 @@ namespace osu.Game.Screens.Select internal static void ApplyQueries(FilterCriteria criteria, string query) { - foreach (Match match in difficulty_query_syntax_regex.Matches(query)) + while (true) { + var match = difficulty_query_syntax_regex.Matches(query).FirstOrDefault(); + + if (match is null) break; + // Trim the first character because it's always '[' (ignoring spaces) string cleanDifficultyQuery = match.Value.Trim(' ')[1..]; From 23c20ca5f44c6eb1a56ce5f6c932dd951e5945b5 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 29 Sep 2023 13:44:11 +0200 Subject: [PATCH 05/51] Add xmldocs --- osu.Game/Screens/Select/FilterCriteria.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index aa3909fa0e..1bcff601e5 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -46,6 +46,10 @@ namespace osu.Game.Screens.Select }; public OptionalTextFilter[] SearchTerms = Array.Empty(); + + /// + /// Search terms that are used for searching difficulty names. + /// public OptionalTextFilter[] DifficultySearchTerms = Array.Empty(); public RulesetInfo? Ruleset; @@ -74,6 +78,10 @@ namespace osu.Game.Screens.Select } } + /// + /// Extracts the search terms from the provided + /// and stores them in . + /// public string DifficultySearchText { set => DifficultySearchTerms = getTermsFromSearchText(value); From c927f90a4834ccf2d5f7c8108feeebfb6ebbfd66 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 29 Sep 2023 13:44:42 +0200 Subject: [PATCH 06/51] Replace regex with a custom algorithm and update test scene. --- .../SongSelect/TestSceneDifficultySearch.cs | 5 +- osu.Game/Screens/Select/FilterQueryParser.cs | 90 ++++++++++++++----- 2 files changed, 71 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs b/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs index 63963d1101..f013d5c19a 100644 --- a/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs +++ b/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs @@ -19,7 +19,7 @@ namespace osu.Game.Tests.SongSelect ("Title1", "Diff2"), ("My[Favourite]Song", "Expert"), ("Title", "My Favourite Diff"), - ("Another One", "diff ]with [[] brackets]]]"), + ("Another One", "diff ]with [[ brackets]]]"), }; [TestCase("[1]", new[] { 0 })] @@ -34,7 +34,8 @@ namespace osu.Game.Tests.SongSelect [TestCase("Title1 [Diff]", new[] { 0, 1 })] [TestCase("Title1[Diff]", new int[] { })] [TestCase("[diff ]with]", new[] { 4 })] - [TestCase("[diff ]with [[] brackets]]]]", new[] { 4 })] + [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] + [TestCase("[diff] another [brackets]", new[] { 4 })] public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) { var carouselBeatmaps = createCarouselBeatmaps().ToList(); diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 58e25adb5b..08fe86713a 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -19,30 +19,9 @@ namespace osu.Game.Screens.Select @"\b(?\w+)(?(:|=|(>|<)(:|=)?))(?("".*""[!]?)|(\S*))", RegexOptions.Compiled | RegexOptions.IgnoreCase); - private static readonly Regex difficulty_query_syntax_regex = new Regex( - @"(\s|^)((\[(?>\[(?)|[^[\]]+|\](?<-level>))*(?(level)(?!))\](\s|$))|(\[.*))", - RegexOptions.Compiled); - internal static void ApplyQueries(FilterCriteria criteria, string query) { - while (true) - { - var match = difficulty_query_syntax_regex.Matches(query).FirstOrDefault(); - - if (match is null) break; - - // Trim the first character because it's always '[' (ignoring spaces) - string cleanDifficultyQuery = match.Value.Trim(' ')[1..]; - - if (cleanDifficultyQuery.EndsWith(']')) - cleanDifficultyQuery = cleanDifficultyQuery[..^1]; - - criteria.DifficultySearchText = cleanDifficultyQuery; - - // Insert whitespace if necessary so that the words before and after the difficulty query aren't joined together. - bool insertWhitespace = match.Value.StartsWith(' ') && match.Value.EndsWith(' '); - query = query.Replace(match.Value, insertWhitespace ? " " : ""); - } + criteria.DifficultySearchText = extractDifficultySearchText(ref query); foreach (Match match in query_syntax_regex.Matches(query)) { @@ -57,6 +36,73 @@ namespace osu.Game.Screens.Select criteria.SearchText = query; } + /// + /// Extracts and returns the difficulty search text between square brackets. + /// + /// The search query. The difficulty search text will be removed from the query. + /// The difficulty search text (without the square brackets). + private static string extractDifficultySearchText(ref string query) + { + var openingBracketIndexes = new List(); + var closingBracketIndexes = new List(); + + populateIndexLists(ref query); + + return performExtraction(ref query); + + void populateIndexLists(ref string query) + { + bool currentlyBetweenBrackets = false; + + for (int i = 0; i < query.Length; i++) + { + switch (query[i]) + { + case '[' when !currentlyBetweenBrackets && (i == 0 || query[i - 1] == ' '): + currentlyBetweenBrackets = true; + openingBracketIndexes.Add(i + 1); + break; + + case ']' when currentlyBetweenBrackets && (i == query.Length - 1 || query[i + 1] == ' '): + currentlyBetweenBrackets = false; + closingBracketIndexes.Add(i); + break; + } + } + + if (currentlyBetweenBrackets) + { + // If there is no "]" closing the current difficulty search query, append it. + query += ']'; + closingBracketIndexes.Add(query.Length - 1); + } + } + + string performExtraction(ref string query) + { + var searchTexts = new List(); + string originalQuery = query; + + for (int i = 0; i < openingBracketIndexes.Count; i++) + { + int startIndex = openingBracketIndexes[i]; + int endIndex = closingBracketIndexes[i]; + + string searchText = originalQuery[startIndex..endIndex]; + + searchTexts.Add(searchText); + + query = query + .Replace($" [{searchText}]", "") + .Replace($"[{searchText}] ", "") + .Replace($"[{searchText}]", "") + .Replace($" [{searchText}] ", " "); + } + + return string.Join(' ', searchTexts); + } + } + private static bool tryParseKeywordCriteria(FilterCriteria criteria, string key, string value, Operator op) { switch (key) From af7180a5b544570cbcaea0c172cf6a2c8e486956 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 16 Oct 2023 19:07:03 +0900 Subject: [PATCH 07/51] Add `SpinnerSpinHistory` and tests --- .../SpinnerSpinHistoryTest.cs | 132 +++++++++++++++++ .../TestSceneSpinnerInput.cs | 5 - .../Judgements/OsuSpinnerJudgementResult.cs | 30 ++-- .../Objects/Drawables/SpinnerSpinHistory.cs | 138 ++++++++++++++++++ .../Default/SpinnerRotationTracker.cs | 8 +- 5 files changed, 282 insertions(+), 31 deletions(-) create mode 100644 osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs create mode 100644 osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs diff --git a/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs new file mode 100644 index 0000000000..9fee5382ec --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs @@ -0,0 +1,132 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Rulesets.Osu.Objects.Drawables; + +namespace osu.Game.Rulesets.Osu.Tests +{ + [TestFixture] + public class SpinnerSpinHistoryTest + { + private SpinnerSpinHistory history = null!; + + [SetUp] + public void Setup() + { + history = new SpinnerSpinHistory(); + } + + [TestCase(0, 0)] + [TestCase(10, 10)] + [TestCase(180, 180)] + [TestCase(350, 350)] + [TestCase(360, 360)] + [TestCase(370, 370)] + [TestCase(540, 540)] + [TestCase(720, 720)] + // --- + [TestCase(-0, 0)] + [TestCase(-10, 10)] + [TestCase(-180, 180)] + [TestCase(-350, 350)] + [TestCase(-360, 360)] + [TestCase(-370, 370)] + [TestCase(-540, 540)] + [TestCase(-720, 720)] + public void TestSpinOneDirection(float spin, float expectedRotation) + { + history.ReportDelta(500, spin); + Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation)); + } + + [TestCase(0, 0, 0, 0)] + // --- + [TestCase(10, -10, 0, 10)] + [TestCase(-10, 10, 0, 10)] + // --- + [TestCase(10, -20, 0, 10)] + [TestCase(-10, 20, 0, 10)] + // --- + [TestCase(20, -10, 0, 20)] + [TestCase(-20, 10, 0, 20)] + // --- + [TestCase(10, -360, 0, 350)] + [TestCase(-10, 360, 0, 350)] + // --- + [TestCase(360, -10, 0, 370)] + [TestCase(360, 10, 0, 370)] + [TestCase(-360, 10, 0, 370)] + [TestCase(-360, -10, 0, 370)] + // --- + [TestCase(10, 10, 10, 30)] + [TestCase(10, 10, -10, 20)] + [TestCase(10, -10, 10, 10)] + [TestCase(-10, -10, -10, 30)] + [TestCase(-10, -10, 10, 20)] + [TestCase(-10, 10, 10, 10)] + // --- + [TestCase(10, -20, -350, 360)] + [TestCase(10, -20, 350, 340)] + [TestCase(-10, 20, 350, 360)] + [TestCase(-10, 20, -350, 340)] + public void TestSpinMultipleDirections(float spin1, float spin2, float spin3, float expectedRotation) + { + history.ReportDelta(500, spin1); + history.ReportDelta(1000, spin2); + history.ReportDelta(1500, spin3); + Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation)); + } + + // One spin + [TestCase(370, -50, 320)] + [TestCase(-370, 50, 320)] + // Two spins + [TestCase(740, -420, 320)] + [TestCase(-740, 420, 320)] + public void TestRemoveAndCrossFullSpin(float deltaToAdd, float deltaToRemove, float expectedRotation) + { + history.ReportDelta(1000, deltaToAdd); + history.ReportDelta(500, deltaToRemove); + Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation)); + } + + // One spin + partial + [TestCase(400, -30, -50, 320)] + [TestCase(-400, 30, 50, 320)] + // Two spins + partial + [TestCase(800, -430, -50, 320)] + [TestCase(-800, 430, 50, 320)] + public void TestRemoveAndCrossFullAndPartialSpins(float deltaToAdd1, float deltaToAdd2, float deltaToRemove, float expectedRotation) + { + history.ReportDelta(1000, deltaToAdd1); + history.ReportDelta(1500, deltaToAdd2); + history.ReportDelta(500, deltaToRemove); + Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation)); + } + + [Test] + public void TestRewindMultipleFullSpins() + { + history.ReportDelta(500, 360); + history.ReportDelta(1000, 720); + + Assert.That(history.TotalRotation, Is.EqualTo(1080)); + + history.ReportDelta(250, -180); + + Assert.That(history.TotalRotation, Is.EqualTo(180)); + } + + [Test] + public void TestRewindIntoSegmentThatHasNotCrossedZero() + { + history.ReportDelta(1000, -180); + history.ReportDelta(1500, 90); + history.ReportDelta(2000, 450); + history.ReportDelta(1750, -45); + + Assert.That(history.TotalRotation, Is.EqualTo(180)); + } + } +} diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs index c4bf0d4e2e..67c7dbdb66 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs @@ -53,7 +53,6 @@ namespace osu.Game.Rulesets.Osu.Tests /// While off-centre, vibrates backwards and forwards on the x-axis, from centre-50 to centre+50, every 50ms. /// [Test] - [Ignore("An upcoming implementation will fix this case")] public void TestVibrateWithoutSpinningOffCentre() { List frames = new List(); @@ -81,7 +80,6 @@ namespace osu.Game.Rulesets.Osu.Tests /// While centred on the slider, vibrates backwards and forwards on the x-axis, from centre-50 to centre+50, every 50ms. /// [Test] - [Ignore("An upcoming implementation will fix this case")] public void TestVibrateWithoutSpinningOnCentre() { List frames = new List(); @@ -130,7 +128,6 @@ namespace osu.Game.Rulesets.Osu.Tests /// No ticks should be hit since the total rotation is -0.5 (0.5 CW + 1 CCW = 0.5 CCW). /// [Test] - [Ignore("An upcoming implementation will fix this case")] public void TestSpinHalfBothDirections() { performTest(new SpinFramesGenerator(time_spinner_start) @@ -149,7 +146,6 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(-180, 540, 1)] [TestCase(180, -900, 2)] [TestCase(-180, 900, 2)] - [Ignore("An upcoming implementation will fix this case")] public void TestSpinOneDirectionThenChangeDirection(float direction1, float direction2, int expectedTicks) { performTest(new SpinFramesGenerator(time_spinner_start) @@ -162,7 +158,6 @@ namespace osu.Game.Rulesets.Osu.Tests } [Test] - [Ignore("An upcoming implementation will fix this case")] public void TestRewind() { AddStep("set manual clock", () => manualClock = new ManualClock { Rate = 1 }); diff --git a/osu.Game.Rulesets.Osu/Judgements/OsuSpinnerJudgementResult.cs b/osu.Game.Rulesets.Osu/Judgements/OsuSpinnerJudgementResult.cs index c5e15d63ea..e8fc1d99bc 100644 --- a/osu.Game.Rulesets.Osu/Judgements/OsuSpinnerJudgementResult.cs +++ b/osu.Game.Rulesets.Osu/Judgements/OsuSpinnerJudgementResult.cs @@ -4,6 +4,7 @@ using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Judgements { @@ -15,28 +16,15 @@ namespace osu.Game.Rulesets.Osu.Judgements public Spinner Spinner => (Spinner)HitObject; /// - /// The total rotation performed on the spinner disc, disregarding the spin direction, - /// adjusted for the track's playback rate. + /// The total amount that the spinner was rotated. /// - /// - /// - /// This value is always non-negative and is monotonically increasing with time - /// (i.e. will only increase if time is passing forward, but can decrease during rewind). - /// - /// - /// The rotation from each frame is multiplied by the clock's current playback rate. - /// The reason this is done is to ensure that spinners give the same score and require the same number of spins - /// regardless of whether speed-modifying mods are applied. - /// - /// - /// - /// Assuming no speed-modifying mods are active, - /// if the spinner is spun 360 degrees clockwise and then 360 degrees counter-clockwise, - /// this property will return the value of 720 (as opposed to 0). - /// If Double Time is active instead (with a speed multiplier of 1.5x), - /// in the same scenario the property will return 720 * 1.5 = 1080. - /// - public float TotalRotation; + public float TotalRotation => History.TotalRotation; + + /// + /// Stores the spinning history of the spinner.
+ /// Instants of movement deltas may be added or removed from this in order to calculate the total rotation for the spinner. + /// + public readonly SpinnerSpinHistory History = new SpinnerSpinHistory(); /// /// Time instant at which the spin was started (the first user input which caused an increase in spin). diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs new file mode 100644 index 0000000000..0464ed422f --- /dev/null +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs @@ -0,0 +1,138 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Diagnostics; + +namespace osu.Game.Rulesets.Osu.Objects.Drawables +{ + /// + /// Stores the spinning history of a single spinner.
+ /// Instants of movement deltas may be added or removed from this in order to calculate the total rotation for the spinner. + /// + /// + /// A single, full rotation of the spinner is defined as a 360-degree rotation of the spinner, starting from 0, going in a single direction.
+ /// + /// + /// If the player spins 90-degrees clockwise, then changes direction, they need to spin 90-degrees counter-clockwise to return to 0 + /// and then continue rotating the spinner for another 360-degrees in the same direction. + /// + public class SpinnerSpinHistory + { + /// + /// The sum of all complete spins and any current partial spin, in degrees. + /// + /// + /// This is the final scoring value. + /// + public float TotalRotation => 360 * segments.Count + currentMaxRotation; + + /// + /// The list of all segments where either: + /// + /// The spinning direction was changed. + /// A full spin of 360 degrees was performed in either direction. + /// + /// + private readonly Stack segments = new Stack(); + + /// + /// The total accumulated rotation. + /// + private float currentAbsoluteRotation; + + private float lastCompletionAbsoluteRotation; + + /// + /// For the current spin, represents the maximum rotation (from 0..360) achieved by the user. + /// + private float currentMaxRotation; + + /// + /// The current spin, from -360..360. + /// + private float currentRotation => currentAbsoluteRotation - lastCompletionAbsoluteRotation; + + private double lastReportTime = double.NegativeInfinity; + + /// + /// Report a delta update based on user input. + /// + /// The current time. + /// The delta of the angle moved through since the last report. + public void ReportDelta(double currentTime, float delta) + { + // TODO: Debug.Assert(Math.Abs(delta) < 180); + // This will require important frame guarantees. + + currentAbsoluteRotation += delta; + + if (currentTime >= lastReportTime) + addDelta(currentTime, delta); + else + rewindDelta(currentTime, delta); + + lastReportTime = currentTime; + } + + private void addDelta(double currentTime, float delta) + { + if (delta == 0) + return; + + currentMaxRotation = Math.Max(currentMaxRotation, Math.Abs(currentRotation)); + + while (currentMaxRotation >= 360) + { + int direction = Math.Sign(currentRotation); + + segments.Push(new SpinSegment(currentTime, direction)); + + lastCompletionAbsoluteRotation += direction * 360; + currentMaxRotation = Math.Abs(currentRotation); + } + } + + private void rewindDelta(double currentTime, float delta) + { + while (segments.TryPeek(out var segment) && segment.StartTime > currentTime) + { + segments.Pop(); + lastCompletionAbsoluteRotation -= segment.Direction * 360; + currentMaxRotation = Math.Abs(currentRotation); + } + + currentMaxRotation = Math.Abs(currentRotation); + } + + /// + /// Represents a single segment of history. + /// + /// + /// Each time the player changes direction, a new segment is recorded. + /// A segment stores the current absolute angle of rotation. Generally this would be either -360 or 360 for a completed spin, or + /// a number representing the last incomplete spin. + /// + private class SpinSegment + { + /// + /// The start time of this segment, when the direction change occurred. + /// + public readonly double StartTime; + + /// + /// The direction this segment started in. + /// + public readonly int Direction; + + public SpinSegment(double startTime, int direction) + { + Debug.Assert(direction == -1 || direction == 1); + + StartTime = startTime; + Direction = direction; + } + } + } +} diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/SpinnerRotationTracker.cs b/osu.Game.Rulesets.Osu/Skinning/Default/SpinnerRotationTracker.cs index 69c2bf3dd0..4bdcc4b381 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/SpinnerRotationTracker.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/SpinnerRotationTracker.cs @@ -101,15 +101,13 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default rotationTransferred = true; } - currentRotation += delta; - double rate = gameplayClock?.GetTrueGameplayRate() ?? Clock.Rate; + delta = (float)(delta * Math.Abs(rate)); Debug.Assert(Math.Abs(delta) <= 180); - // rate has to be applied each frame, because it's not guaranteed to be constant throughout playback - // (see: ModTimeRamp) - drawableSpinner.Result.TotalRotation += (float)(Math.Abs(delta) * rate); + currentRotation += delta; + drawableSpinner.Result.History.ReportDelta(Time.Current, delta); } private void resetState(DrawableHitObject obj) From 9011170c3e4df5ccf26ff762da2972c7f4d0bd0c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 20:10:31 +0900 Subject: [PATCH 08/51] Adjust tests in line with new expectations --- .../SpinnerSpinHistoryTest.cs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs index 9fee5382ec..c20a7bbfd9 100644 --- a/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs @@ -113,7 +113,7 @@ namespace osu.Game.Rulesets.Osu.Tests Assert.That(history.TotalRotation, Is.EqualTo(1080)); - history.ReportDelta(250, -180); + history.ReportDelta(250, -900); Assert.That(history.TotalRotation, Is.EqualTo(180)); } @@ -122,11 +122,27 @@ namespace osu.Game.Rulesets.Osu.Tests public void TestRewindIntoSegmentThatHasNotCrossedZero() { history.ReportDelta(1000, -180); - history.ReportDelta(1500, 90); - history.ReportDelta(2000, 450); - history.ReportDelta(1750, -45); - Assert.That(history.TotalRotation, Is.EqualTo(180)); + history.ReportDelta(1500, 90); + Assert.That(history.TotalRotation, Is.EqualTo(180)); + history.ReportDelta(2000, 450); + Assert.That(history.TotalRotation, Is.EqualTo(360)); + history.ReportDelta(1750, -45); + Assert.That(history.TotalRotation, Is.EqualTo(315)); + } + + [Test] + public void TestRewindOverDirectionChange() + { + history.ReportDelta(1000, 40); // max is now CW 40 degrees + Assert.That(history.TotalRotation, Is.EqualTo(40)); + history.ReportDelta(1100, -90); // max is now CCW 50 degrees + Assert.That(history.TotalRotation, Is.EqualTo(50)); + history.ReportDelta(1200, 110); // max is now CW 60 degrees + Assert.That(history.TotalRotation, Is.EqualTo(60)); + + history.ReportDelta(1000, -20); + Assert.That(history.TotalRotation, Is.EqualTo(40)); } } } From 80ac0c46d9895e40171445552551f76ff7f2b118 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 19:12:12 +0900 Subject: [PATCH 09/51] Fix final test This looks to have been a WIP test, so I've brought it up to the point of passing. I haven't given it much more attention than that, though. --- .../TestSceneSpinnerInput.cs | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs index 67c7dbdb66..662734ddcd 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs @@ -162,13 +162,20 @@ namespace osu.Game.Rulesets.Osu.Tests { AddStep("set manual clock", () => manualClock = new ManualClock { Rate = 1 }); - List frames = new SpinFramesGenerator(time_spinner_start) - .Spin(360, 500) // 2000ms -> 1 full CW spin - .Spin(-180, 500) // 2500ms -> 0.5 CCW spins - .Spin(90, 500) // 3000ms -> 0.25 CW spins - .Spin(450, 500) // 3500ms -> 1 full CW spin - .Spin(180, 500) // 4000ms -> 0.5 CW spins - .Build(); + List frames = + new SpinFramesGenerator(time_spinner_start) + // 1500ms start + .Spin(360, 500) + // 2000ms -> 1 full CW spin + .Spin(-180, 500) + // 2500ms -> 1 full CW spin + 0.5 CCW spins + .Spin(90, 500) + // 3000ms -> 1 full CW spin + 0.25 CCW spins + .Spin(450, 500) + // 3500ms -> 2 full CW spins + .Spin(180, 500) + // 4000ms -> 2 full CW spins + 0.5 CW spins + .Build(); loadPlayer(frames); @@ -189,11 +196,11 @@ namespace osu.Game.Rulesets.Osu.Tests assertTotalRotation(3750, 810); assertTotalRotation(3500, 720); assertTotalRotation(3250, 530); - assertTotalRotation(3000, 540); + assertTotalRotation(3000, 450); assertTotalRotation(2750, 540); assertTotalRotation(2500, 540); - assertTotalRotation(2250, 360); - assertTotalRotation(2000, 180); + assertTotalRotation(2250, 450); + assertTotalRotation(2000, 360); assertTotalRotation(1500, 0); void assertTotalRotation(double time, float expected) @@ -206,7 +213,8 @@ namespace osu.Game.Rulesets.Osu.Tests void addSeekStep(double time) { AddStep($"seek to {time}", () => clock.Seek(time)); - AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time)); + // Lenience is required due to interpolation running slightly ahead on a stalled clock. + AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time).Within(40)); } } From 4f0d55e1a900026ee43c678614cedf4598dcd4f7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 19:39:15 +0900 Subject: [PATCH 10/51] Clean up code and add xmldoc and inline doc --- .../Objects/Drawables/SpinnerSpinHistory.cs | 130 ++++++++++-------- 1 file changed, 69 insertions(+), 61 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs index 0464ed422f..3212ddc1ef 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs @@ -26,33 +26,33 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// /// This is the final scoring value. /// - public float TotalRotation => 360 * segments.Count + currentMaxRotation; + public float TotalRotation => 360 * completedSpins.Count + currentSpinMaxRotation; - /// - /// The list of all segments where either: - /// - /// The spinning direction was changed. - /// A full spin of 360 degrees was performed in either direction. - /// - /// - private readonly Stack segments = new Stack(); + private readonly Stack completedSpins = new Stack(); /// /// The total accumulated rotation. /// - private float currentAbsoluteRotation; + private float totalAbsoluteRotation; - private float lastCompletionAbsoluteRotation; + private float totalAbsoluteRotationsAtLastCompletion; /// - /// For the current spin, represents the maximum rotation (from 0..360) achieved by the user. + /// For the current spin, represents the maximum absolute rotation (from 0..360) achieved by the user. /// - private float currentMaxRotation; + /// + /// This is used to report in the case a user spins backwards. + /// Basically it allows us to not reduce the total rotation in such a case. + /// + /// This also stops spinner "cheese" where a user may rapidly change directions and cause an increase + /// in rotations. + /// + private float currentSpinMaxRotation; /// /// The current spin, from -360..360. /// - private float currentRotation => currentAbsoluteRotation - lastCompletionAbsoluteRotation; + private float currentSpinRotation => totalAbsoluteRotation - totalAbsoluteRotationsAtLastCompletion; private double lastReportTime = double.NegativeInfinity; @@ -63,74 +63,82 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// The delta of the angle moved through since the last report. public void ReportDelta(double currentTime, float delta) { - // TODO: Debug.Assert(Math.Abs(delta) < 180); - // This will require important frame guarantees. + if (delta == 0) + return; - currentAbsoluteRotation += delta; + // Importantly, outside of tests the max delta entering here is 180 degrees. + // If it wasn't for tests, we could add this line: + // + // Debug.Assert(Math.Abs(delta) < 180); + // + // For this to be 101% correct, we need to add the ability for important frames to be + // created based on gameplay intrinsics (ie. there should be one frame for any spinner delta 90 < n < 180 degrees). + // + // But this can come later. + + totalAbsoluteRotation += delta; if (currentTime >= lastReportTime) - addDelta(currentTime, delta); + { + currentSpinMaxRotation = Math.Max(currentSpinMaxRotation, Math.Abs(currentSpinRotation)); + + // Handle the case where the user has completed another spin. + // Note that this does could be an `if` rather than `while` if the above assertion held true. + // It is a `while` loop to handle tests which throw larger values at this method. + while (currentSpinMaxRotation >= 360) + { + int direction = Math.Sign(currentSpinRotation); + + completedSpins.Push(new CompletedSpin(currentTime, direction)); + + // Incrementing the last completion point will cause `currentSpinRotation` to + // hold the remaining spin that needs to be considered. + totalAbsoluteRotationsAtLastCompletion += direction * 360; + + // Reset the current max as we are entering a new spin. + // Importantly, carry over the remainder (which is now stored in `currentSpinRotation`). + currentSpinMaxRotation = Math.Abs(currentSpinRotation); + } + } else - rewindDelta(currentTime, delta); + { + // When rewinding, the main thing we care about is getting `totalAbsoluteRotationsAtLastCompletion` + // to the correct value. We can used the stored history for this. + while (completedSpins.TryPeek(out var segment) && segment.CompletionTime > currentTime) + { + completedSpins.Pop(); + totalAbsoluteRotationsAtLastCompletion -= segment.Direction * 360; + } + + // This is a best effort. We may not have enough data to match this 1:1, but that's okay. + // We know that the player is somewhere in a spin. + // In the worst case, this will be lower than expected, and recover in forward playback. + currentSpinMaxRotation = Math.Abs(currentSpinRotation); + } lastReportTime = currentTime; } - private void addDelta(double currentTime, float delta) - { - if (delta == 0) - return; - - currentMaxRotation = Math.Max(currentMaxRotation, Math.Abs(currentRotation)); - - while (currentMaxRotation >= 360) - { - int direction = Math.Sign(currentRotation); - - segments.Push(new SpinSegment(currentTime, direction)); - - lastCompletionAbsoluteRotation += direction * 360; - currentMaxRotation = Math.Abs(currentRotation); - } - } - - private void rewindDelta(double currentTime, float delta) - { - while (segments.TryPeek(out var segment) && segment.StartTime > currentTime) - { - segments.Pop(); - lastCompletionAbsoluteRotation -= segment.Direction * 360; - currentMaxRotation = Math.Abs(currentRotation); - } - - currentMaxRotation = Math.Abs(currentRotation); - } - /// - /// Represents a single segment of history. + /// Represents a single completed spin. /// - /// - /// Each time the player changes direction, a new segment is recorded. - /// A segment stores the current absolute angle of rotation. Generally this would be either -360 or 360 for a completed spin, or - /// a number representing the last incomplete spin. - /// - private class SpinSegment + private class CompletedSpin { /// - /// The start time of this segment, when the direction change occurred. + /// The time at which this spin completion occurred. /// - public readonly double StartTime; + public readonly double CompletionTime; /// - /// The direction this segment started in. + /// The direction this spin completed in. /// public readonly int Direction; - public SpinSegment(double startTime, int direction) + public CompletedSpin(double completionTime, int direction) { Debug.Assert(direction == -1 || direction == 1); - StartTime = startTime; + CompletionTime = completionTime; Direction = direction; } } From 92524d42990747e5545194c4def191f073ee7e39 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 17:44:01 +0900 Subject: [PATCH 11/51] Remove incorrect plural from variable --- .../Objects/Drawables/SpinnerSpinHistory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs index 3212ddc1ef..1cf511f526 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs @@ -35,7 +35,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// private float totalAbsoluteRotation; - private float totalAbsoluteRotationsAtLastCompletion; + private float totalAbsoluteRotationAtLastCompletion; /// /// For the current spin, represents the maximum absolute rotation (from 0..360) achieved by the user. @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// /// The current spin, from -360..360. /// - private float currentSpinRotation => totalAbsoluteRotation - totalAbsoluteRotationsAtLastCompletion; + private float currentSpinRotation => totalAbsoluteRotation - totalAbsoluteRotationAtLastCompletion; private double lastReportTime = double.NegativeInfinity; @@ -93,7 +93,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables // Incrementing the last completion point will cause `currentSpinRotation` to // hold the remaining spin that needs to be considered. - totalAbsoluteRotationsAtLastCompletion += direction * 360; + totalAbsoluteRotationAtLastCompletion += direction * 360; // Reset the current max as we are entering a new spin. // Importantly, carry over the remainder (which is now stored in `currentSpinRotation`). @@ -107,7 +107,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables while (completedSpins.TryPeek(out var segment) && segment.CompletionTime > currentTime) { completedSpins.Pop(); - totalAbsoluteRotationsAtLastCompletion -= segment.Direction * 360; + totalAbsoluteRotationAtLastCompletion -= segment.Direction * 360; } // This is a best effort. We may not have enough data to match this 1:1, but that's okay. From 6ae82c5e73d27d91ad4d11d381cc59d3044784c6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 18:35:08 +0900 Subject: [PATCH 12/51] Fix circle scale not matching stable due to missing multiplier This multiplier has to exist. I'm not guaranteeing that the rest is correct here. Should we be doing proper cross-testing on this? Maybe, but it's going to be hard to get right. We could likely check as far as "game pixels", but there's still a chance that the osu-framework could be doing something weird in the rest of the hierarchy where playfield scale is involved. Closes https://github.com/ppy/osu/issues/25162. Tested to fix the linked replay. --- osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs index 0bdbfaa760..80d3de75ea 100644 --- a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs @@ -155,7 +155,17 @@ namespace osu.Game.Rulesets.Osu.Objects // This adjustment is necessary for AR>10, otherwise TimePreempt can become smaller leading to hitcircles not fully fading in. TimeFadeIn = 400 * Math.Min(1, TimePreempt / PREEMPT_MIN); - Scale = (1.0f - 0.7f * (difficulty.CircleSize - 5) / 5) / 2; + // The following comment is copied verbatim from osu-stable: + // + // Builds of osu! up to 2013-05-04 had the gamefield being rounded down, which caused incorrect radius calculations + // in widescreen cases. This ratio adjusts to allow for old replays to work post-fix, which in turn increases the lenience + // for all plays, but by an amount so small it should only be effective in replays. + // + // To match expectations of gameplay we need to apply this multiplier to circle scale. It's weird but is what it is. + // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. + const float broken_gamefield_rounding_allowance = 1.00041f; + + Scale = (1.0f - 0.7f * (difficulty.CircleSize - 5) / 5) / 2 * broken_gamefield_rounding_allowance; } protected override HitWindows CreateHitWindows() => new OsuHitWindows(); From 3a5490892ce669bc8d7ae66f1bbc90e33d2e1ed9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 19:50:42 +0900 Subject: [PATCH 13/51] Centralise and repair circle size calculations game-wide --- .../Objects/CatchHitObject.cs | 2 +- osu.Game.Rulesets.Catch/UI/Catcher.cs | 9 ++----- .../TestSceneSliderFollowCircleInput.cs | 2 +- osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs | 12 +--------- .../Statistics/AccuracyHeatmap.cs | 3 +-- osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs | 24 ++++++++++++++++++- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs index f4bd515995..1d536eb5cb 100644 --- a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs @@ -151,7 +151,7 @@ namespace osu.Game.Rulesets.Catch.Objects TimePreempt = (float)IBeatmapDifficultyInfo.DifficultyRange(difficulty.ApproachRate, 1800, 1200, 450); - Scale = (1.0f - 0.7f * (difficulty.CircleSize - 5) / 5) / 2; + Scale = IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize); } protected override HitWindows CreateHitWindows() => HitWindows.Empty; diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index c5c9556ed7..236a7290da 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Catch.UI Size = new Vector2(BASE_SIZE); if (difficulty != null) - Scale = calculateScale(difficulty); + Scale = new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize)); CatchWidth = CalculateCatchWidth(Scale); @@ -182,11 +182,6 @@ namespace osu.Game.Rulesets.Catch.UI /// public Drawable CreateProxiedContent() => caughtObjectContainer.CreateProxy(); - /// - /// Calculates the scale of the catcher based off the provided beatmap difficulty. - /// - private static Vector2 calculateScale(IBeatmapDifficultyInfo difficulty) => new Vector2(1.0f - 0.7f * (difficulty.CircleSize - 5) / 5); - /// /// Calculates the width of the area used for attempting catches in gameplay. /// @@ -197,7 +192,7 @@ namespace osu.Game.Rulesets.Catch.UI /// Calculates the width of the area used for attempting catches in gameplay. /// /// The beatmap difficulty. - public static float CalculateCatchWidth(IBeatmapDifficultyInfo difficulty) => CalculateCatchWidth(calculateScale(difficulty)); + public static float CalculateCatchWidth(IBeatmapDifficultyInfo difficulty) => CalculateCatchWidth(new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize))); /// /// Determine if this catcher can catch a in the current position. diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs index 5adc50859f..0e5f8a8cf6 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs @@ -35,7 +35,7 @@ namespace osu.Game.Rulesets.Osu.Tests { const double time_slider_start = 1000; - float circleRadius = OsuHitObject.OBJECT_RADIUS * (1.0f - 0.7f * (circleSize - 5) / 5) / 2; + float circleRadius = OsuHitObject.OBJECT_RADIUS * IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(circleSize); float followCircleRadius = circleRadius * 1.2f; performTest(new Beatmap diff --git a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs index 80d3de75ea..0b1f413362 100644 --- a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs @@ -155,17 +155,7 @@ namespace osu.Game.Rulesets.Osu.Objects // This adjustment is necessary for AR>10, otherwise TimePreempt can become smaller leading to hitcircles not fully fading in. TimeFadeIn = 400 * Math.Min(1, TimePreempt / PREEMPT_MIN); - // The following comment is copied verbatim from osu-stable: - // - // Builds of osu! up to 2013-05-04 had the gamefield being rounded down, which caused incorrect radius calculations - // in widescreen cases. This ratio adjusts to allow for old replays to work post-fix, which in turn increases the lenience - // for all plays, but by an amount so small it should only be effective in replays. - // - // To match expectations of gameplay we need to apply this multiplier to circle scale. It's weird but is what it is. - // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. - const float broken_gamefield_rounding_allowance = 1.00041f; - - Scale = (1.0f - 0.7f * (difficulty.CircleSize - 5) / 5) / 2 * broken_gamefield_rounding_allowance; + Scale = IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize); } protected override HitWindows CreateHitWindows() => new OsuHitWindows(); diff --git a/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs b/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs index 6564a086fe..8a2c7ff13a 100644 --- a/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs +++ b/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs @@ -208,8 +208,7 @@ namespace osu.Game.Rulesets.Osu.Statistics if (score.HitEvents.Count == 0) return; - // Todo: This should probably not be done like this. - float radius = OsuHitObject.OBJECT_RADIUS * (1.0f - 0.7f * (playableBeatmap.Difficulty.CircleSize - 5) / 5) / 2; + float radius = OsuHitObject.OBJECT_RADIUS * IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(playableBeatmap.Difficulty.CircleSize); foreach (var e in score.HitEvents.Where(e => e.HitObject is HitCircle && !(e.HitObject is SliderTailCircle))) { diff --git a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs index 78234a9dd9..af943b62e7 100644 --- a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs +++ b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs @@ -44,6 +44,21 @@ namespace osu.Game.Beatmaps /// double SliderTickRate { get; } + static float CalculateScaleFromCircleSize(float circleSize) + { + // The following comment is copied verbatim from osu-stable: + // + // Builds of osu! up to 2013-05-04 had the gamefield being rounded down, which caused incorrect radius calculations + // in widescreen cases. This ratio adjusts to allow for old replays to work post-fix, which in turn increases the lenience + // for all plays, but by an amount so small it should only be effective in replays. + // + // To match expectations of gameplay we need to apply this multiplier to circle scale. It's weird but is what it is. + // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. + const float broken_gamefield_rounding_allowance = 1.00041f; + + return (float)(1.0f - 0.7f * DifficultyRange(circleSize)) / 2 * broken_gamefield_rounding_allowance; + } + /// /// Maps a difficulty value [0, 10] to a two-piece linear range of values. /// @@ -55,13 +70,20 @@ namespace osu.Game.Beatmaps static double DifficultyRange(double difficulty, double min, double mid, double max) { if (difficulty > 5) - return mid + (max - mid) * (difficulty - 5) / 5; + return mid + (max - mid) * DifficultyRange(difficulty); if (difficulty < 5) return mid - (mid - min) * (5 - difficulty) / 5; return mid; } + /// + /// Maps a difficulty value [0, 10] to a linear range of [-1, 1]. + /// + /// The difficulty value to be mapped. + /// Value to which the difficulty value maps in the specified range. + static double DifficultyRange(double difficulty) => (difficulty - 5) / 5; + /// /// Maps a difficulty value [0, 10] to a two-piece linear range of values. /// From 040d0970f219fbb58c10f72fca9c3c6b80c53a51 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 23:54:18 +0900 Subject: [PATCH 14/51] Use `DifficultyRange` helper method in one more place --- osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs index af943b62e7..19944afa1c 100644 --- a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs +++ b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs @@ -72,7 +72,7 @@ namespace osu.Game.Beatmaps if (difficulty > 5) return mid + (max - mid) * DifficultyRange(difficulty); if (difficulty < 5) - return mid - (mid - min) * (5 - difficulty) / 5; + return mid + (mid - min) * DifficultyRange(difficulty); return mid; } From 42087037aeea9409fac0723b9a9d05b9fe2d7e98 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 23:56:15 +0900 Subject: [PATCH 15/51] Fix `Catcher` code change not accounting for double sizing requirement --- osu.Game.Rulesets.Catch/UI/Catcher.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 236a7290da..f09614816d 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Catch.UI Size = new Vector2(BASE_SIZE); if (difficulty != null) - Scale = new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize)); + Scale = calculateScale(difficulty); CatchWidth = CalculateCatchWidth(Scale); @@ -192,7 +192,7 @@ namespace osu.Game.Rulesets.Catch.UI /// Calculates the width of the area used for attempting catches in gameplay. /// /// The beatmap difficulty. - public static float CalculateCatchWidth(IBeatmapDifficultyInfo difficulty) => CalculateCatchWidth(new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize))); + public static float CalculateCatchWidth(IBeatmapDifficultyInfo difficulty) => CalculateCatchWidth(calculateScale(difficulty)); /// /// Determine if this catcher can catch a in the current position. @@ -466,6 +466,11 @@ namespace osu.Game.Rulesets.Catch.UI d.Expire(); } + /// + /// Calculates the scale of the catcher based off the provided beatmap difficulty. + /// + private static Vector2 calculateScale(IBeatmapDifficultyInfo difficulty) => new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize) * 2); + private enum DroppedObjectAnimation { Drop, From f16400929d7eaddaddce0e0c56c30ae5254497e9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 15:37:51 +0900 Subject: [PATCH 16/51] Update bindable flow to expose every spin, even after max bonus is reached --- .../Objects/Drawables/DrawableSpinner.cs | 29 +++++++++---------- .../Skinning/Argon/ArgonSpinner.cs | 8 ++--- .../Skinning/Default/DefaultSpinner.cs | 8 ++--- .../Skinning/Legacy/LegacySpinner.cs | 8 ++--- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index 9fa180cf93..72bf7bf1e9 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs @@ -48,9 +48,16 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// /// The amount of bonus score gained from spinning after the required number of spins, for display purposes. /// - public IBindable GainedBonus => gainedBonus; + public double CurrentBonusScore => score_per_tick * (completedFullSpins.Value - HitObject.SpinsRequiredForBonus); - private readonly Bindable gainedBonus = new BindableDouble(); + /// + /// The maximum amount of bonus score which can be achieved from extra spins. + /// + public double MaximumBonusScore => score_per_tick * HitObject.MaximumBonusSpins; + + public IBindable CompletedFullSpins => completedFullSpins; + + private readonly Bindable completedFullSpins = new Bindable(); /// /// The number of spins per minute this spinner is spinning at, for display purposes. @@ -286,8 +293,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private static readonly int score_per_tick = new SpinnerBonusTick.OsuSpinnerBonusTickJudgement().MaxNumericResult; - private int completedFullSpins; - private void updateBonusScore() { if (ticks.Count == 0) @@ -295,27 +300,21 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables int spins = (int)(Result.TotalRotation / 360); - if (spins < completedFullSpins) + if (spins < completedFullSpins.Value) { // rewinding, silently handle - completedFullSpins = spins; + completedFullSpins.Value = spins; return; } - while (completedFullSpins != spins) + while (completedFullSpins.Value != spins) { var tick = ticks.FirstOrDefault(t => !t.Result.HasResult); // tick may be null if we've hit the spin limit. - if (tick != null) - { - tick.TriggerResult(true); + tick?.TriggerResult(true); - if (tick is DrawableSpinnerBonusTick) - gainedBonus.Value = score_per_tick * (spins - HitObject.SpinsRequiredForBonus); - } - - completedFullSpins++; + completedFullSpins.Value++; } } } diff --git a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs index d5a9cf46c5..dc701aef53 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs @@ -85,17 +85,17 @@ namespace osu.Game.Rulesets.Osu.Skinning.Argon }; } - private IBindable gainedBonus = null!; + private IBindable completedSpins = null!; private IBindable spinsPerMinute = null!; protected override void LoadComplete() { base.LoadComplete(); - gainedBonus = drawableSpinner.GainedBonus.GetBoundCopy(); - gainedBonus.BindValueChanged(bonus => + completedSpins = drawableSpinner.CompletedFullSpins.GetBoundCopy(); + completedSpins.BindValueChanged(bonus => { - bonusCounter.Text = bonus.NewValue.ToString(NumberFormatInfo.InvariantInfo); + bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); bonusCounter.FadeOutFromOne(1500); bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); }); diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs index 071fbe6add..e001aeceae 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs @@ -80,17 +80,17 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default }); } - private IBindable gainedBonus = null!; + private IBindable completedSpins = null!; private IBindable spinsPerMinute = null!; protected override void LoadComplete() { base.LoadComplete(); - gainedBonus = drawableSpinner.GainedBonus.GetBoundCopy(); - gainedBonus.BindValueChanged(bonus => + completedSpins = drawableSpinner.CompletedFullSpins.GetBoundCopy(); + completedSpins.BindValueChanged(bonus => { - bonusCounter.Text = bonus.NewValue.ToString(NumberFormatInfo.InvariantInfo); + bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); bonusCounter.FadeOutFromOne(1500); bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); }); diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs index d8f837ae5e..456f041554 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs @@ -107,7 +107,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy }); } - private IBindable gainedBonus = null!; + private IBindable completedSpins = null!; private IBindable spinsPerMinute = null!; private readonly Bindable completed = new Bindable(); @@ -116,10 +116,10 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { base.LoadComplete(); - gainedBonus = DrawableSpinner.GainedBonus.GetBoundCopy(); - gainedBonus.BindValueChanged(bonus => + completedSpins = DrawableSpinner.CompletedFullSpins.GetBoundCopy(); + completedSpins.BindValueChanged(bonus => { - bonusCounter.Text = bonus.NewValue.ToString(NumberFormatInfo.InvariantInfo); + bonusCounter.Text = DrawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); bonusCounter.FadeOutFromOne(800, Easing.Out); bonusCounter.ScaleTo(SPRITE_SCALE * 2f).Then().ScaleTo(SPRITE_SCALE * 1.28f, 800, Easing.Out); }); From 0da0855692ddc73c519a62aaa2dddf7355926de7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 16:10:50 +0900 Subject: [PATCH 17/51] Add ability to change spin speed in spinner test --- .../TestSceneSpinner.cs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs index b1d9c453d6..ea57a6a1b5 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Testing; using osu.Game.Audio; @@ -26,6 +27,15 @@ namespace osu.Game.Rulesets.Osu.Tests private TestDrawableSpinner drawableSpinner; + private readonly BindableDouble spinRate = new BindableDouble(); + + protected override void LoadComplete() + { + base.LoadComplete(); + + AddSliderStep("Spin rate", 0.5, 5, 1, val => spinRate.Value = val); + } + [TestCase(true)] [TestCase(false)] public void TestVariousSpinners(bool autoplay) @@ -86,7 +96,7 @@ namespace osu.Game.Rulesets.Osu.Tests spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { OverallDifficulty = od }); - return drawableSpinner = new TestDrawableSpinner(spinner, true) + return drawableSpinner = new TestDrawableSpinner(spinner, true, spinRate) { Anchor = Anchor.Centre, Depth = depthIndex++, @@ -114,7 +124,7 @@ namespace osu.Game.Rulesets.Osu.Tests spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = circleSize }); - drawableSpinner = new TestDrawableSpinner(spinner, auto) + drawableSpinner = new TestDrawableSpinner(spinner, auto, spinRate) { Anchor = Anchor.Centre, Depth = depthIndex++, @@ -130,18 +140,20 @@ namespace osu.Game.Rulesets.Osu.Tests private partial class TestDrawableSpinner : DrawableSpinner { private readonly bool auto; + private readonly BindableDouble spinRate; - public TestDrawableSpinner(Spinner s, bool auto) + public TestDrawableSpinner(Spinner s, bool auto, BindableDouble spinRate) : base(s) { this.auto = auto; + this.spinRate = spinRate; } protected override void Update() { base.Update(); if (auto) - RotationTracker.AddRotation((float)(Clock.ElapsedFrameTime * 2)); + RotationTracker.AddRotation((float)(Clock.ElapsedFrameTime * spinRate.Value)); } } } From 3986cec949cac13b70a16d3d99d5211ce0e54516 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 15:57:13 +0900 Subject: [PATCH 18/51] Cap bonus score more correctly --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index 72bf7bf1e9..d4e257d754 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs @@ -48,7 +48,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// /// The amount of bonus score gained from spinning after the required number of spins, for display purposes. /// - public double CurrentBonusScore => score_per_tick * (completedFullSpins.Value - HitObject.SpinsRequiredForBonus); + public double CurrentBonusScore => score_per_tick * Math.Clamp(completedFullSpins.Value - HitObject.SpinsRequiredForBonus, 0, HitObject.MaximumBonusSpins); /// /// The maximum amount of bonus score which can be achieved from extra spins. From 181a98e8efd346e8f21c744af18239c402bc8f63 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 16:05:29 +0900 Subject: [PATCH 19/51] Remove duplicate definition of bonus text in `ArgonSpinner` --- osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs index dc701aef53..b667c415fe 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs @@ -35,14 +35,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Argon InternalChildren = new Drawable[] { - bonusCounter = new OsuSpriteText - { - Alpha = 0, - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Font = OsuFont.Default.With(size: 24), - Y = -120, - }, new ArgonSpinnerDisc { RelativeSizeAxes = Axes.Both, From ca3a3f600e61165d7f662ef63798e76d3dc18803 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 16:11:09 +0900 Subject: [PATCH 20/51] Update visuals of spinner implementations to show MAX score differently --- .../Skinning/Argon/ArgonSpinner.cs | 22 ++++++++++++++---- .../Skinning/Default/DefaultSpinner.cs | 23 ++++++++++++++++--- .../Skinning/Legacy/LegacySpinner.cs | 16 +++++++++++-- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs index b667c415fe..ee9f228137 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Argon/ArgonSpinner.cs @@ -85,11 +85,25 @@ namespace osu.Game.Rulesets.Osu.Skinning.Argon base.LoadComplete(); completedSpins = drawableSpinner.CompletedFullSpins.GetBoundCopy(); - completedSpins.BindValueChanged(bonus => + completedSpins.BindValueChanged(_ => { - bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); - bonusCounter.FadeOutFromOne(1500); - bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); + if (drawableSpinner.CurrentBonusScore <= 0) + return; + + if (drawableSpinner.CurrentBonusScore == drawableSpinner.MaximumBonusScore) + { + bonusCounter.Text = "MAX"; + bonusCounter.ScaleTo(1.5f).Then().ScaleTo(2.8f, 1000, Easing.OutQuint); + + bonusCounter.FlashColour(Colour4.FromHex("FC618F"), 400); + bonusCounter.FadeOutFromOne(500); + } + else + { + bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); + bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); + bonusCounter.FadeOutFromOne(1500); + } }); spinsPerMinute = drawableSpinner.SpinsPerMinute.GetBoundCopy(); diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs index e001aeceae..4a76a1aec4 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs @@ -24,6 +24,9 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default private Container spmContainer = null!; private OsuSpriteText spmCounter = null!; + [Resolved] + private OsuColour colours { get; set; } = null!; + public DefaultSpinner() { RelativeSizeAxes = Axes.Both; @@ -90,9 +93,23 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default completedSpins = drawableSpinner.CompletedFullSpins.GetBoundCopy(); completedSpins.BindValueChanged(bonus => { - bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); - bonusCounter.FadeOutFromOne(1500); - bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); + if (drawableSpinner.CurrentBonusScore <= 0) + return; + + if (drawableSpinner.CurrentBonusScore == drawableSpinner.MaximumBonusScore) + { + bonusCounter.Text = "MAX"; + bonusCounter.ScaleTo(1.5f).Then().ScaleTo(2.8f, 1000, Easing.OutQuint); + + bonusCounter.FlashColour(colours.YellowLight, 400); + bonusCounter.FadeOutFromOne(500); + } + else + { + bonusCounter.Text = drawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); + bonusCounter.FadeOutFromOne(1500); + bonusCounter.ScaleTo(1.5f).Then().ScaleTo(1f, 1000, Easing.OutQuint); + } }); spinsPerMinute = drawableSpinner.SpinsPerMinute.GetBoundCopy(); diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs index 456f041554..28acb4a996 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySpinner.cs @@ -119,9 +119,21 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy completedSpins = DrawableSpinner.CompletedFullSpins.GetBoundCopy(); completedSpins.BindValueChanged(bonus => { + if (DrawableSpinner.CurrentBonusScore <= 0) + return; + bonusCounter.Text = DrawableSpinner.CurrentBonusScore.ToString(NumberFormatInfo.InvariantInfo); - bonusCounter.FadeOutFromOne(800, Easing.Out); - bonusCounter.ScaleTo(SPRITE_SCALE * 2f).Then().ScaleTo(SPRITE_SCALE * 1.28f, 800, Easing.Out); + + if (DrawableSpinner.CurrentBonusScore == DrawableSpinner.MaximumBonusScore) + { + bonusCounter.ScaleTo(1.4f).Then().ScaleTo(1.8f, 1000, Easing.Out); + bonusCounter.FadeOutFromOne(500, Easing.Out); + } + else + { + bonusCounter.FadeOutFromOne(800, Easing.Out); + bonusCounter.ScaleTo(SPRITE_SCALE * 2f).Then().ScaleTo(SPRITE_SCALE * 1.28f, 800, Easing.Out); + } }); spinsPerMinute = DrawableSpinner.SpinsPerMinute.GetBoundCopy(); From 3fb74cb5f936f4fb8380f29cf1430c7e4bda5da7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 18:57:14 +0900 Subject: [PATCH 21/51] Move helper method to `LegacyRulesetExtensions` and stop applying rounding allowance to catch As discussed, it isn't used in stable like this. Was a mistake. --- .../Objects/CatchHitObject.cs | 3 ++- osu.Game.Rulesets.Catch/UI/Catcher.cs | 3 ++- .../TestSceneSliderFollowCircleInput.cs | 3 ++- osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs | 3 ++- .../Statistics/AccuracyHeatmap.cs | 3 ++- osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs | 15 --------------- .../Objects/Legacy/LegacyRulesetExtensions.cs | 19 +++++++++++++++++++ 7 files changed, 29 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs index 1d536eb5cb..b9fef6bf8c 100644 --- a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs @@ -8,6 +8,7 @@ using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; using osuTK; @@ -151,7 +152,7 @@ namespace osu.Game.Rulesets.Catch.Objects TimePreempt = (float)IBeatmapDifficultyInfo.DifficultyRange(difficulty.ApproachRate, 1800, 1200, 450); - Scale = IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize); + Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize); } protected override HitWindows CreateHitWindows() => HitWindows.Empty; diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index f09614816d..0c2c157d10 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -17,6 +17,7 @@ using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Catch.Objects.Drawables; using osu.Game.Rulesets.Catch.Skinning; using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Skinning; using osuTK; using osuTK.Graphics; @@ -469,7 +470,7 @@ namespace osu.Game.Rulesets.Catch.UI /// /// Calculates the scale of the catcher based off the provided beatmap difficulty. /// - private static Vector2 calculateScale(IBeatmapDifficultyInfo difficulty) => new Vector2(IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize) * 2); + private static Vector2 calculateScale(IBeatmapDifficultyInfo difficulty) => new Vector2(LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize) * 2); private enum DroppedObjectAnimation { diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs index 0e5f8a8cf6..d4bb789a12 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderFollowCircleInput.cs @@ -10,6 +10,7 @@ using osu.Game.Beatmaps; using osu.Game.Replays; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Replays; @@ -35,7 +36,7 @@ namespace osu.Game.Rulesets.Osu.Tests { const double time_slider_start = 1000; - float circleRadius = OsuHitObject.OBJECT_RADIUS * IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(circleSize); + float circleRadius = OsuHitObject.OBJECT_RADIUS * LegacyRulesetExtensions.CalculateScaleFromCircleSize(circleSize, true); float followCircleRadius = circleRadius * 1.2f; performTest(new Beatmap diff --git a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs index 0b1f413362..d74d28c748 100644 --- a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs @@ -7,6 +7,7 @@ using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Scoring; using osu.Game.Rulesets.Scoring; @@ -155,7 +156,7 @@ namespace osu.Game.Rulesets.Osu.Objects // This adjustment is necessary for AR>10, otherwise TimePreempt can become smaller leading to hitcircles not fully fading in. TimeFadeIn = 400 * Math.Min(1, TimePreempt / PREEMPT_MIN); - Scale = IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(difficulty.CircleSize); + Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize, true); } protected override HitWindows CreateHitWindows() => new OsuHitWindows(); diff --git a/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs b/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs index 8a2c7ff13a..83bab7dc01 100644 --- a/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs +++ b/osu.Game.Rulesets.Osu/Statistics/AccuracyHeatmap.cs @@ -13,6 +13,7 @@ using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; +using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Scoring; using osuTK; @@ -208,7 +209,7 @@ namespace osu.Game.Rulesets.Osu.Statistics if (score.HitEvents.Count == 0) return; - float radius = OsuHitObject.OBJECT_RADIUS * IBeatmapDifficultyInfo.CalculateScaleFromCircleSize(playableBeatmap.Difficulty.CircleSize); + float radius = OsuHitObject.OBJECT_RADIUS * LegacyRulesetExtensions.CalculateScaleFromCircleSize(playableBeatmap.Difficulty.CircleSize, true); foreach (var e in score.HitEvents.Where(e => e.HitObject is HitCircle && !(e.HitObject is SliderTailCircle))) { diff --git a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs index 19944afa1c..e7a3d87d0a 100644 --- a/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs +++ b/osu.Game/Beatmaps/IBeatmapDifficultyInfo.cs @@ -44,21 +44,6 @@ namespace osu.Game.Beatmaps /// double SliderTickRate { get; } - static float CalculateScaleFromCircleSize(float circleSize) - { - // The following comment is copied verbatim from osu-stable: - // - // Builds of osu! up to 2013-05-04 had the gamefield being rounded down, which caused incorrect radius calculations - // in widescreen cases. This ratio adjusts to allow for old replays to work post-fix, which in turn increases the lenience - // for all plays, but by an amount so small it should only be effective in replays. - // - // To match expectations of gameplay we need to apply this multiplier to circle scale. It's weird but is what it is. - // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. - const float broken_gamefield_rounding_allowance = 1.00041f; - - return (float)(1.0f - 0.7f * DifficultyRange(circleSize)) / 2 * broken_gamefield_rounding_allowance; - } - /// /// Maps a difficulty value [0, 10] to a two-piece linear range of values. /// diff --git a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs index 6cff4b12c4..f006d5e998 100644 --- a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs +++ b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Objects.Types; @@ -38,5 +39,23 @@ namespace osu.Game.Rulesets.Objects.Legacy return timingControlPoint.BeatLength * bpmMultiplier; } + + /// + /// Calculates scale from a CS value, with an optional fudge that was historically applied to the osu! ruleset. + /// + public static float CalculateScaleFromCircleSize(float circleSize, bool applyFudge = false) + { + // The following comment is copied verbatim from osu-stable: + // + // Builds of osu! up to 2013-05-04 had the gamefield being rounded down, which caused incorrect radius calculations + // in widescreen cases. This ratio adjusts to allow for old replays to work post-fix, which in turn increases the lenience + // for all plays, but by an amount so small it should only be effective in replays. + // + // To match expectations of gameplay we need to apply this multiplier to circle scale. It's weird but is what it is. + // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. + const float broken_gamefield_rounding_allowance = 1.00041f; + + return (float)(1.0f - 0.7f * IBeatmapDifficultyInfo.DifficultyRange(circleSize)) / 2 * broken_gamefield_rounding_allowance; + } } } From 1be49f5b93ed4c2738fb411b0a3d99ad34730f27 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 19:04:24 +0900 Subject: [PATCH 22/51] Update osu! difficulty calculator tests to factor in fractional changes --- .../OsuDifficultyCalculatorTest.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs index f1c1c4734d..7b7deb9c67 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs @@ -15,22 +15,22 @@ namespace osu.Game.Rulesets.Osu.Tests { protected override string ResourceAssembly => "osu.Game.Rulesets.Osu"; - [TestCase(6.7115569159190587d, 206, "diffcalc-test")] - [TestCase(1.4391311903612753d, 45, "zero-length-sliders")] + [TestCase(6.710442985146793d, 206, "diffcalc-test")] + [TestCase(1.4386882251130073d, 45, "zero-length-sliders")] [TestCase(0.42506480230838789d, 2, "very-fast-slider")] [TestCase(0.14102693012101306d, 1, "nan-slider")] public void Test(double expectedStarRating, int expectedMaxCombo, string name) => base.Test(expectedStarRating, expectedMaxCombo, name); - [TestCase(8.9757300665532966d, 206, "diffcalc-test")] + [TestCase(8.9742952703071666d, 206, "diffcalc-test")] [TestCase(0.55071082800473514d, 2, "very-fast-slider")] - [TestCase(1.7437232654020756d, 45, "zero-length-sliders")] + [TestCase(1.743180218215227d, 45, "zero-length-sliders")] public void TestClockRateAdjusted(double expectedStarRating, int expectedMaxCombo, string name) => Test(expectedStarRating, expectedMaxCombo, name, new OsuModDoubleTime()); - [TestCase(6.7115569159190587d, 239, "diffcalc-test")] + [TestCase(6.710442985146793d, 239, "diffcalc-test")] [TestCase(0.42506480230838789d, 4, "very-fast-slider")] - [TestCase(1.4391311903612753d, 54, "zero-length-sliders")] + [TestCase(1.4386882251130073d, 54, "zero-length-sliders")] public void TestClassicMod(double expectedStarRating, int expectedMaxCombo, string name) => Test(expectedStarRating, expectedMaxCombo, name, new OsuModClassic()); From 137a1d948dad5382af169b4660d57e7f31605253 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 20:04:11 +0900 Subject: [PATCH 23/51] Allow interacting with the carousel anywhere in empty space at song select --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 16ae54b413..eb47a7201a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -1197,6 +1197,8 @@ namespace osu.Game.Screens.Select { private bool rightMouseScrollBlocked; + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; + public CarouselScrollContainer() { // size is determined by the carousel itself, due to not all content necessarily being loaded. From 0ae0b0c353618b7289d6fe6b6aac7c34ad9b9886 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 20:04:26 +0900 Subject: [PATCH 24/51] Fix carousel "reset position" marging not scaling with UI scale correctly --- osu.Game/Screens/Select/SongSelect.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index bac84b8134..f792f3e4f5 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -171,11 +171,6 @@ namespace osu.Game.Screens.Select AddRangeInternal(new Drawable[] { - new ResetScrollContainer(() => Carousel.ScrollToSelected()) - { - RelativeSizeAxes = Axes.Y, - Width = 250, - }, new VerticalMaskingContainer { Children = new Drawable[] @@ -243,6 +238,10 @@ namespace osu.Game.Screens.Select Padding = new MarginPadding { Top = left_area_padding }, Children = new Drawable[] { + new ResetScrollContainer(() => Carousel.ScrollToSelected()) + { + RelativeSizeAxes = Axes.Both, + }, beatmapInfoWedge = new BeatmapInfoWedge { Height = WEDGE_HEIGHT, From 906b700acac611deabd13f6c62ea0d467311940d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 20:18:14 +0900 Subject: [PATCH 25/51] Fix fudge not being applied --- osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs index f006d5e998..53cf835248 100644 --- a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs +++ b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs @@ -55,7 +55,7 @@ namespace osu.Game.Rulesets.Objects.Legacy // It works out to under 1 game pixel and is generally not meaningful to gameplay, but is to replay playback accuracy. const float broken_gamefield_rounding_allowance = 1.00041f; - return (float)(1.0f - 0.7f * IBeatmapDifficultyInfo.DifficultyRange(circleSize)) / 2 * broken_gamefield_rounding_allowance; + return (float)(1.0f - 0.7f * IBeatmapDifficultyInfo.DifficultyRange(circleSize)) / 2 * (applyFudge ? broken_gamefield_rounding_allowance : 1); } } } From b321d556b68691e24183c366fccfed0eced5a81a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Oct 2023 14:18:32 +0200 Subject: [PATCH 26/51] Enforce minimum gameplay sample volume of 5% --- .../Objects/Drawables/DrawableHoldNote.cs | 6 ++++- .../Objects/Drawables/DrawableSlider.cs | 6 ++++- .../Objects/Drawables/DrawableSpinner.cs | 1 + .../Objects/Drawables/DrawableHitObject.cs | 25 ++++++++++++++++++- .../UI/GameplaySampleTriggerSource.cs | 6 ++++- osu.Game/Skinning/SkinnableSound.cs | 8 +++++- 6 files changed, 47 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs index 86920927dc..3490d50871 100644 --- a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs @@ -108,7 +108,11 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables RelativeSizeAxes = Axes.X }, tailContainer = new Container { RelativeSizeAxes = Axes.Both }, - slidingSample = new PausableSkinnableSound { Looping = true } + slidingSample = new PausableSkinnableSound + { + Looping = true, + MinimumSampleVolume = MINIMUM_SAMPLE_VOLUME, + } }); maskedContents.AddRange(new[] diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 09973ce5fd..b682d14879 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -107,7 +107,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables headContainer = new Container { RelativeSizeAxes = Axes.Both }, OverlayElementContainer = new Container { RelativeSizeAxes = Axes.Both, }, Ball, - slidingSample = new PausableSkinnableSound { Looping = true } + slidingSample = new PausableSkinnableSound + { + Looping = true, + MinimumSampleVolume = MINIMUM_SAMPLE_VOLUME, + } }); PositionBindable.BindValueChanged(_ => Position = HitObject.StackedPosition); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index 9fa180cf93..1135ecc7d5 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs @@ -99,6 +99,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables spinningSample = new PausableSkinnableSound { Volume = { Value = 0 }, + MinimumSampleVolume = MINIMUM_SAMPLE_VOLUME, Looping = true, Frequency = { Value = spinning_sample_initial_frequency } } diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 3bb0e3dfb8..ce6475d3ce 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -159,6 +159,26 @@ namespace osu.Game.Rulesets.Objects.Drawables /// internal bool IsInitialized; + /// + /// The minimum allowable volume for sample playback. + /// quieter than that will be forcibly played at this volume instead. + /// + /// + /// + /// Drawable hitobjects adding their own custom samples, or other sample playback sources + /// (i.e. ) must enforce this themselves. + /// + /// + /// This sample volume floor is present in stable, although it is set at 8% rather than 5%. + /// See: https://github.com/peppy/osu-stable-reference/blob/3ea48705eb67172c430371dcfc8a16a002ed0d3d/osu!/Audio/AudioEngine.cs#L1070, + /// https://github.com/peppy/osu-stable-reference/blob/3ea48705eb67172c430371dcfc8a16a002ed0d3d/osu!/Audio/AudioEngine.cs#L1404-L1405. + /// The reason why it is 5% here is that the 8% cap was enforced in a silent manner + /// (i.e. the minimum selectable volume in the editor was 5%, but it would be played at 8% anyways), + /// which is confusing and arbitrary, so we're just doing 5% here at the cost of sacrificing strict parity. + /// + /// + public const int MINIMUM_SAMPLE_VOLUME = 5; + /// /// Creates a new . /// @@ -181,7 +201,10 @@ namespace osu.Game.Rulesets.Objects.Drawables comboColourBrightness.BindTo(gameplaySettings.ComboColourNormalisationAmount); // Explicit non-virtual function call in case a DrawableHitObject overrides AddInternal. - base.AddInternal(Samples = new PausableSkinnableSound()); + base.AddInternal(Samples = new PausableSkinnableSound + { + MinimumSampleVolume = MINIMUM_SAMPLE_VOLUME + }); CurrentSkin = skinSource; CurrentSkin.SourceChanged += skinSourceChanged; diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index fed262868d..b61e8d9674 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -7,6 +7,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics.Containers; using osu.Game.Audio; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play; using osu.Game.Skinning; @@ -45,7 +46,10 @@ namespace osu.Game.Rulesets.UI Child = hitSounds = new Container { Name = "concurrent sample pool", - ChildrenEnumerable = Enumerable.Range(0, max_concurrent_hitsounds).Select(_ => new PausableSkinnableSound()) + ChildrenEnumerable = Enumerable.Range(0, max_concurrent_hitsounds).Select(_ => new PausableSkinnableSound + { + MinimumSampleVolume = DrawableHitObject.MINIMUM_SAMPLE_VOLUME + }) } }; } diff --git a/osu.Game/Skinning/SkinnableSound.cs b/osu.Game/Skinning/SkinnableSound.cs index 59b3799e0a..f866a4f8ec 100644 --- a/osu.Game/Skinning/SkinnableSound.cs +++ b/osu.Game/Skinning/SkinnableSound.cs @@ -20,6 +20,12 @@ namespace osu.Game.Skinning /// public partial class SkinnableSound : SkinReloadableDrawable, IAdjustableAudioComponent { + /// + /// The minimum allowable volume for . + /// that specify a lower will be forcibly pulled up to this volume. + /// + public int MinimumSampleVolume { get; set; } + public override bool RemoveWhenNotAlive => false; public override bool RemoveCompletedTransforms => false; @@ -156,7 +162,7 @@ namespace osu.Game.Skinning { var sample = samplePool?.GetPooledSample(s) ?? new PoolableSkinnableSample(s); sample.Looping = Looping; - sample.Volume.Value = s.Volume / 100.0; + sample.Volume.Value = Math.Max(s.Volume, MinimumSampleVolume) / 100.0; samplesContainer.Add(sample); } From 12282fff5c23fefcbee4907c8d4e917801a8cf38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 20 Oct 2023 14:21:11 +0200 Subject: [PATCH 27/51] Disallow setting sample volume lower than 5% in editor --- .../Edit/Compose/Components/Timeline/SamplePointPiece.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs index b02cfb505e..28841fc9e5 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs @@ -16,6 +16,7 @@ using osu.Game.Audio; using osu.Game.Graphics; using osu.Game.Graphics.UserInterfaceV2; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Screens.Edit.Timing; using osuTK; using osuTK.Graphics; @@ -101,7 +102,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline }, volume = new IndeterminateSliderWithTextBoxInput("Volume", new BindableInt(100) { - MinValue = 0, + MinValue = DrawableHitObject.MINIMUM_SAMPLE_VOLUME, MaxValue = 100, }) } From 71718bd761ffb237cd4f8d733352733a43aaf003 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Sat, 23 Sep 2023 07:04:51 +0300 Subject: [PATCH 28/51] Limit comment sprite height --- .../Comments/CommentMarkdownContainer.cs | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs index 278cef9112..c17e48ef8d 100644 --- a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs +++ b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs @@ -1,9 +1,17 @@ // 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 Markdig.Syntax; +using Markdig.Syntax.Inlines; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Containers.Markdown; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.Textures; using osu.Game.Graphics.Containers.Markdown; +using osuTK; namespace osu.Game.Overlays.Comments { @@ -16,6 +24,8 @@ namespace osu.Game.Overlays.Comments protected override MarkdownHeading CreateHeading(HeadingBlock headingBlock) => new CommentMarkdownHeading(headingBlock); + public override MarkdownTextFlowContainer CreateTextFlow() => new CommentMarkdownTextFlowContainer(); + private partial class CommentMarkdownHeading : OsuMarkdownHeading { public CommentMarkdownHeading(HeadingBlock headingBlock) @@ -40,5 +50,64 @@ namespace osu.Game.Overlays.Comments } } } + + private partial class CommentMarkdownTextFlowContainer : MarkdownTextFlowContainer + { + protected override void AddImage(LinkInline linkInline) => AddDrawable(new CommentMarkdownImage(linkInline.Url)); + + private partial class CommentMarkdownImage : MarkdownImage + { + public CommentMarkdownImage(string url) + : base(url) + { + } + + private DelayedLoadWrapper wrapper; + + protected override Drawable CreateContent(string url) => wrapper = new DelayedLoadWrapper(CreateImageContainer(url)); + + protected override ImageContainer CreateImageContainer(string url) + { + var container = new CommentImageContainer(url); + container.OnLoadComplete += d => + { + // The size of DelayedLoadWrapper depends on AutoSizeAxes of it's content. + // But since it's set to None, we need to specify the size here manually. + wrapper.Size = container.Size; + d.FadeInFromZero(300, Easing.OutQuint); + }; + return container; + } + + private partial class CommentImageContainer : ImageContainer + { + // https://github.com/ppy/osu-web/blob/3bd0f406dc78d60b356d955cd4201f8c3e1cca09/resources/css/bem/osu-md.less#L36 + // Web version defines max height in em units (6 em), which assuming default pixel size as 14 results in 84 px, + // which also seems to match my observations upon expecting the web element. + private const float max_height = 84f; + + public CommentImageContainer(string url) + : base(url) + { + AutoSizeAxes = Axes.None; + } + + protected override Sprite CreateImageSprite() => new Sprite + { + RelativeSizeAxes = Axes.Both + }; + + protected override Texture GetImageTexture(TextureStore textures, string url) + { + Texture t = base.GetImageTexture(textures, url); + + if (t != null) + Size = t.Height > 96 ? new Vector2(max_height / t.Height * t.Width, max_height) : t.Size; + + return t; + } + } + } + } } } From e1e9c3d7b377a574bc0dc32f7868683c858219a4 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Sun, 22 Oct 2023 00:33:26 +0300 Subject: [PATCH 29/51] Fix incorrect height limit --- osu.Game/Overlays/Comments/CommentMarkdownContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs index c17e48ef8d..39894c642e 100644 --- a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs +++ b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs @@ -102,7 +102,7 @@ namespace osu.Game.Overlays.Comments Texture t = base.GetImageTexture(textures, url); if (t != null) - Size = t.Height > 96 ? new Vector2(max_height / t.Height * t.Width, max_height) : t.Size; + Size = t.Height > max_height ? new Vector2(max_height / t.Height * t.Width, max_height) : t.Size; return t; } From fc1254ba47268bb51a547b0dd26945f744e17047 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 16:15:56 +0900 Subject: [PATCH 30/51] Update resources --- osu.Game/osu.Game.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index c289ae89ee..848e17af1a 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -37,7 +37,7 @@ - + From 4e057b446a475e7b2886df925e6d70fafeb8e979 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 16:46:15 +0900 Subject: [PATCH 31/51] Rename accumulated rotation variable --- .../Objects/Drawables/SpinnerSpinHistory.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs index 1cf511f526..1c6c5b5d02 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SpinnerSpinHistory.cs @@ -31,11 +31,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private readonly Stack completedSpins = new Stack(); /// - /// The total accumulated rotation. + /// The total accumulated (absolute) rotation. /// - private float totalAbsoluteRotation; + private float totalAccumulatedRotation; - private float totalAbsoluteRotationAtLastCompletion; + private float totalAccumulatedRotationAtLastCompletion; /// /// For the current spin, represents the maximum absolute rotation (from 0..360) achieved by the user. @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// /// The current spin, from -360..360. /// - private float currentSpinRotation => totalAbsoluteRotation - totalAbsoluteRotationAtLastCompletion; + private float currentSpinRotation => totalAccumulatedRotation - totalAccumulatedRotationAtLastCompletion; private double lastReportTime = double.NegativeInfinity; @@ -76,7 +76,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables // // But this can come later. - totalAbsoluteRotation += delta; + totalAccumulatedRotation += delta; if (currentTime >= lastReportTime) { @@ -93,7 +93,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables // Incrementing the last completion point will cause `currentSpinRotation` to // hold the remaining spin that needs to be considered. - totalAbsoluteRotationAtLastCompletion += direction * 360; + totalAccumulatedRotationAtLastCompletion += direction * 360; // Reset the current max as we are entering a new spin. // Importantly, carry over the remainder (which is now stored in `currentSpinRotation`). @@ -107,7 +107,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables while (completedSpins.TryPeek(out var segment) && segment.CompletionTime > currentTime) { completedSpins.Pop(); - totalAbsoluteRotationAtLastCompletion -= segment.Direction * 360; + totalAccumulatedRotationAtLastCompletion -= segment.Direction * 360; } // This is a best effort. We may not have enough data to match this 1:1, but that's okay. From ac1783fa94ca445320044723f82daf3726d39364 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 23 Oct 2023 13:26:59 +0300 Subject: [PATCH 32/51] Fix comment --- osu.Game/Overlays/Comments/CommentMarkdownContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs index 39894c642e..2ec2313500 100644 --- a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs +++ b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs @@ -82,8 +82,8 @@ namespace osu.Game.Overlays.Comments private partial class CommentImageContainer : ImageContainer { // https://github.com/ppy/osu-web/blob/3bd0f406dc78d60b356d955cd4201f8c3e1cca09/resources/css/bem/osu-md.less#L36 - // Web version defines max height in em units (6 em), which assuming default pixel size as 14 results in 84 px, - // which also seems to match my observations upon expecting the web element. + // Web version defines max height in em units (6 em), which assuming default font size as 14 results in 84 px, + // which also seems to match observations upon inspecting the web element. private const float max_height = 84f; public CommentImageContainer(string url) From b18a5e63b74ac6122fe9c30109ee7fe757b07c8a Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Mon, 23 Oct 2023 13:29:46 +0300 Subject: [PATCH 33/51] Remove nullable disable --- osu.Game/Overlays/Comments/CommentMarkdownContainer.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs index 2ec2313500..e48a52c787 100644 --- a/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs +++ b/osu.Game/Overlays/Comments/CommentMarkdownContainer.cs @@ -1,8 +1,6 @@ // 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 Markdig.Syntax; using Markdig.Syntax.Inlines; using osu.Framework.Graphics; @@ -62,7 +60,7 @@ namespace osu.Game.Overlays.Comments { } - private DelayedLoadWrapper wrapper; + private DelayedLoadWrapper wrapper = null!; protected override Drawable CreateContent(string url) => wrapper = new DelayedLoadWrapper(CreateImageContainer(url)); @@ -104,7 +102,7 @@ namespace osu.Game.Overlays.Comments if (t != null) Size = t.Height > max_height ? new Vector2(max_height / t.Height * t.Width, max_height) : t.Size; - return t; + return t!; } } } From 08b7c3d5ad30bed5353c171cf52b74fbb31a20a5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 20:01:23 +0900 Subject: [PATCH 34/51] Remove test which makes less sense with new algorith --- .../SpinnerSpinHistoryTest.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs index c20a7bbfd9..cd54873d36 100644 --- a/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs @@ -118,19 +118,6 @@ namespace osu.Game.Rulesets.Osu.Tests Assert.That(history.TotalRotation, Is.EqualTo(180)); } - [Test] - public void TestRewindIntoSegmentThatHasNotCrossedZero() - { - history.ReportDelta(1000, -180); - Assert.That(history.TotalRotation, Is.EqualTo(180)); - history.ReportDelta(1500, 90); - Assert.That(history.TotalRotation, Is.EqualTo(180)); - history.ReportDelta(2000, 450); - Assert.That(history.TotalRotation, Is.EqualTo(360)); - history.ReportDelta(1750, -45); - Assert.That(history.TotalRotation, Is.EqualTo(315)); - } - [Test] public void TestRewindOverDirectionChange() { From 15c1fcbf96cf1e64637ce7f583dde98333f1ca2e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 20:28:12 +0900 Subject: [PATCH 35/51] Set `ManualClock` rate to 0 to avoid interpolation causing test errors --- osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs | 8 ++++++-- osu.Game/Tests/Visual/OsuTestScene.cs | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs index 662734ddcd..531f88dff8 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs @@ -160,7 +160,11 @@ namespace osu.Game.Rulesets.Osu.Tests [Test] public void TestRewind() { - AddStep("set manual clock", () => manualClock = new ManualClock { Rate = 1 }); + AddStep("set manual clock", () => manualClock = new ManualClock + { + // Avoids interpolation trying to run ahead during testing. + Rate = 0 + }); List frames = new SpinFramesGenerator(time_spinner_start) @@ -214,7 +218,7 @@ namespace osu.Game.Rulesets.Osu.Tests { AddStep($"seek to {time}", () => clock.Seek(time)); // Lenience is required due to interpolation running slightly ahead on a stalled clock. - AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time).Within(40)); + AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time)); } } diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index 096917cf4f..e1bbddd340 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -432,6 +432,8 @@ namespace osu.Game.Tests.Visual private bool running; + public override double Rate => referenceClock.Rate; + public TrackVirtualManual(IFrameBasedClock referenceClock, string name = "virtual") : base(name) { From d08ba764ddfb233125ade630490ef6e09ddaea6c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 20:29:46 +0900 Subject: [PATCH 36/51] Add extended testing of rewind --- .../TestSceneSpinnerInput.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs index 531f88dff8..5a473409a4 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs @@ -196,7 +196,7 @@ namespace osu.Game.Rulesets.Osu.Tests DrawableSpinner drawableSpinner = null!; AddUntilStep("get spinner", () => (drawableSpinner = currentPlayer.ChildrenOfType().Single()) != null); - assertTotalRotation(4000, 900); + assertFinalRotationCorrect(); assertTotalRotation(3750, 810); assertTotalRotation(3500, 720); assertTotalRotation(3250, 530); @@ -207,6 +207,26 @@ namespace osu.Game.Rulesets.Osu.Tests assertTotalRotation(2000, 360); assertTotalRotation(1500, 0); + // same thing but always returning to final time to check. + assertFinalRotationCorrect(); + assertTotalRotation(3750, 810); + assertFinalRotationCorrect(); + assertTotalRotation(3500, 720); + assertFinalRotationCorrect(); + assertTotalRotation(3250, 530); + assertFinalRotationCorrect(); + assertTotalRotation(3000, 450); + assertFinalRotationCorrect(); + assertTotalRotation(2750, 540); + assertFinalRotationCorrect(); + assertTotalRotation(2500, 540); + assertFinalRotationCorrect(); + assertTotalRotation(2250, 450); + assertFinalRotationCorrect(); + assertTotalRotation(2000, 360); + assertFinalRotationCorrect(); + assertTotalRotation(1500, 0); + void assertTotalRotation(double time, float expected) { addSeekStep(time); @@ -220,6 +240,8 @@ namespace osu.Game.Rulesets.Osu.Tests // Lenience is required due to interpolation running slightly ahead on a stalled clock. AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time)); } + + void assertFinalRotationCorrect() => assertTotalRotation(4000, 900); } private void assertTicksHit(int count) From baf4130f3adf314c3eac7deef89cde013cb9ba28 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 22:18:38 +0900 Subject: [PATCH 37/51] Fix test clock no longer accounting for rate changes --- osu.Game/Tests/Visual/OsuTestScene.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index e1bbddd340..4860d3954e 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -432,7 +432,10 @@ namespace osu.Game.Tests.Visual private bool running; - public override double Rate => referenceClock.Rate; + public override double Rate => base.Rate + // This is mainly to allow some tests to override the rate to zero + // and avoid interpolation. + * referenceClock.Rate; public TrackVirtualManual(IFrameBasedClock referenceClock, string name = "virtual") : base(name) From cfd8d05fde5cb5cf0b45ff9284ae1182305e5c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 23 Oct 2023 21:26:25 +0200 Subject: [PATCH 38/51] Fix spinner ticks not playing samples correctly sometimes Noticed this during work on https://github.com/ppy/osu/pull/25185. In some circumstances, it seemed that spinner bonus ticks (and mostly them specifically) would not always play with the correct volume. Hours of debugging later pointed at a trace at `DrawableAudioWrapper.refreshLayoutFromParent()` not firing sometimes. Initially I thought it to be some sort of framework bug, but after preparing a diff and running final checks, I noticed that sometimes the sample was being played *by a `PoolableSkinnableSample` that wasn't loaded*. And determining why *that* is the case turned out with this diff. As it happens, spinner ticks get assigned a start time proportionally, i.e. the 1st of 10 ticks is placed at 10% of the duration, the 2nd at 20%, and so on. The start time generally shouldn't matter, because the spinner is manually judging the ticks. *However*, the ticks *still* receive a lifetime start / end in the same way normal objects do, which means that in some cases they can *not be alive* when they're hit, which means that the `DrawableAudioWrapper` flow *hasn't had a chance to run*, and rightly so. To fix, ensure that all spinner ticks are alive throughout the entirety of the spinner's duration. --- .../Objects/Drawables/DrawableSpinnerTick.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index 34253e3d4f..20fe7172ad 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -4,6 +4,8 @@ #nullable disable using osu.Framework.Graphics; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Objects.Drawables { @@ -25,6 +27,26 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables Origin = Anchor.Centre; } + protected override void OnApply() + { + base.OnApply(); + + // the tick can be theoretically judged at any point in the spinner's duration, + // so it must be alive throughout the spinner's entire lifetime. + // this mostly matters for correct sample playback. + LifetimeStart = DrawableSpinner.HitObject.StartTime; + } + + protected override void UpdateHitStateTransforms(ArmedState state) + { + base.UpdateHitStateTransforms(state); + + // the tick can be theoretically judged at any point in the spinner's duration, + // so it must be alive throughout the spinner's entire lifetime (or until hit, whichever applies). + // this mostly matters for correct sample playback. + LifetimeEnd = (Result?.IsHit == true ? Result.TimeAbsolute : DrawableSpinner.HitObject.GetEndTime()) + (Samples?.Length ?? 0); + } + /// /// Apply a judgement result. /// From c5339f440e4a3c968a62235fc439b01539536dc4 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Mon, 23 Oct 2023 23:30:54 +0200 Subject: [PATCH 39/51] Apply code review suggestions --- osu.Game/Screens/Select/FilterQueryParser.cs | 40 +++++++++++--------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 08fe86713a..33ddd08e09 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -46,11 +46,11 @@ namespace osu.Game.Screens.Select var openingBracketIndexes = new List(); var closingBracketIndexes = new List(); - populateIndexLists(ref query); + populateIndexLists(query); return performExtraction(ref query); - void populateIndexLists(ref string query) + void populateIndexLists(string query) { bool currentlyBetweenBrackets = false; @@ -58,24 +58,25 @@ namespace osu.Game.Screens.Select { switch (query[i]) { - case '[' when !currentlyBetweenBrackets && (i == 0 || query[i - 1] == ' '): - currentlyBetweenBrackets = true; - openingBracketIndexes.Add(i + 1); + case '[': + if (!currentlyBetweenBrackets && (i == 0 || query[i - 1] == ' ')) + { + currentlyBetweenBrackets = true; + openingBracketIndexes.Add(i + 1); + } + break; - case ']' when currentlyBetweenBrackets && (i == query.Length - 1 || query[i + 1] == ' '): - currentlyBetweenBrackets = false; - closingBracketIndexes.Add(i); + case ']': + if (currentlyBetweenBrackets && (i == query.Length - 1 || query[i + 1] == ' ')) + { + currentlyBetweenBrackets = false; + closingBracketIndexes.Add(i); + } + break; } } - - if (currentlyBetweenBrackets) - { - // If there is no "]" closing the current difficulty search query, append it. - query += ']'; - closingBracketIndexes.Add(query.Length - 1); - } } string performExtraction(ref string query) @@ -86,7 +87,10 @@ namespace osu.Game.Screens.Select for (int i = 0; i < openingBracketIndexes.Count; i++) { int startIndex = openingBracketIndexes[i]; - int endIndex = closingBracketIndexes[i]; + + int endIndex = closingBracketIndexes.Count > 0 + ? closingBracketIndexes[Math.Min(i, closingBracketIndexes.Count - 1)] + : query.Length; string searchText = originalQuery[startIndex..endIndex]; @@ -96,7 +100,9 @@ namespace osu.Game.Screens.Select .Replace($" [{searchText}]", "") .Replace($"[{searchText}] ", "") .Replace($"[{searchText}]", "") - .Replace($" [{searchText}] ", " "); + .Replace($" [{searchText}] ", " ") + .Replace($" [{searchText}", "") + .Replace($"[{searchText}", ""); } return string.Join(' ', searchTexts); From 127482919329740ef82670e0760c467c1d2e3718 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 14:51:40 +0900 Subject: [PATCH 40/51] Block scroll and click operations on the left side of song select --- osu.Game/Screens/Select/SongSelect.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index f792f3e4f5..d5ec94ad71 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -238,7 +238,7 @@ namespace osu.Game.Screens.Select Padding = new MarginPadding { Top = left_area_padding }, Children = new Drawable[] { - new ResetScrollContainer(() => Carousel.ScrollToSelected()) + new LeftSideInteractionContainer(() => Carousel.ScrollToSelected()) { RelativeSizeAxes = Axes.Both, }, @@ -1016,18 +1016,25 @@ namespace osu.Game.Screens.Select } } - private partial class ResetScrollContainer : Container + /// + /// Handles mouse interactions required when moving away from the carousel. + /// + private partial class LeftSideInteractionContainer : Container { - private readonly Action? onHoverAction; + private readonly Action? resetCarouselPosition; - public ResetScrollContainer(Action onHoverAction) + public LeftSideInteractionContainer(Action resetCarouselPosition) { - this.onHoverAction = onHoverAction; + this.resetCarouselPosition = resetCarouselPosition; } + protected override bool OnScroll(ScrollEvent e) => true; + + protected override bool OnMouseDown(MouseDownEvent e) => true; + protected override bool OnHover(HoverEvent e) { - onHoverAction?.Invoke(); + resetCarouselPosition?.Invoke(); return base.OnHover(e); } } From 383d7153788152c5dd6cdc4abe1149e03bfb3d7c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 14:59:21 +0900 Subject: [PATCH 41/51] Fix incorrectly named test --- .../{TestSceneDifficultySearch.cs => DifficultySearchTest.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osu.Game.Tests/SongSelect/{TestSceneDifficultySearch.cs => DifficultySearchTest.cs} (97%) diff --git a/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs b/osu.Game.Tests/SongSelect/DifficultySearchTest.cs similarity index 97% rename from osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs rename to osu.Game.Tests/SongSelect/DifficultySearchTest.cs index f013d5c19a..770e40d672 100644 --- a/osu.Game.Tests/SongSelect/TestSceneDifficultySearch.cs +++ b/osu.Game.Tests/SongSelect/DifficultySearchTest.cs @@ -11,7 +11,7 @@ using osu.Game.Tests.Visual; namespace osu.Game.Tests.SongSelect { - public partial class TestSceneDifficultySearch : OsuTestScene + public partial class DifficultySearchTest : OsuTestScene { private static readonly (string title, string difficultyName)[] beatmaps = { From b16ece32f42373a0dde565a830f19b414e21966e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 15:14:30 +0900 Subject: [PATCH 42/51] Put tests in more correct place --- .../Filtering/FilterQueryParserTest.cs | 50 +++++++++++++ .../SongSelect/DifficultySearchTest.cs | 72 ------------------- 2 files changed, 50 insertions(+), 72 deletions(-) delete mode 100644 osu.Game.Tests/SongSelect/DifficultySearchTest.cs diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index ce95e921b9..d453954ae0 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -2,10 +2,12 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Rulesets.Filter; using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Carousel; using osu.Game.Screens.Select.Filter; namespace osu.Game.Tests.NonVisual.Filtering @@ -382,6 +384,54 @@ namespace osu.Game.Tests.NonVisual.Filtering Assert.AreEqual("unrecognised=keyword", filterCriteria.SearchText.Trim()); } + [TestCase("[1]", new[] { 0 })] + [TestCase("[1", new[] { 0 })] + [TestCase("My[Favourite", new[] { 2 })] + [TestCase("My[Favourite]", new[] { 2 })] + [TestCase("My[Favourite]Song", new[] { 2 })] + [TestCase("Favourite]", new[] { 2 })] + [TestCase("[Diff", new[] { 0, 1, 3, 4 })] + [TestCase("[Diff]", new[] { 0, 1, 3, 4 })] + [TestCase("[Favourite]", new[] { 3 })] + [TestCase("Title1 [Diff]", new[] { 0, 1 })] + [TestCase("Title1[Diff]", new int[] { })] + [TestCase("[diff ]with]", new[] { 4 })] + [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] + [TestCase("[diff] another [brackets]", new[] { 4 })] + public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) + { + var carouselBeatmaps = (((string title, string difficultyName)[])new[] + { + ("Title1", "Diff1"), + ("Title1", "Diff2"), + ("My[Favourite]Song", "Expert"), + ("Title", "My Favourite Diff"), + ("Another One", "diff ]with [[ brackets]]]"), + }).Select(info => new CarouselBeatmap(new BeatmapInfo + { + Metadata = new BeatmapMetadata + { + Title = info.title + }, + DifficultyName = info.difficultyName + })).ToList(); + + var criteria = new FilterCriteria(); + + FilterQueryParser.ApplyQueries(criteria, query); + carouselBeatmaps.ForEach(b => b.Filter(criteria)); + + Assert.That(carouselBeatmaps.All(b => + { + int index = carouselBeatmaps.IndexOf(b); + + bool shouldBeVisible = expectedBeatmapIndexes.Contains(index); + bool isVisible = !b.Filtered.Value; + + return isVisible == shouldBeVisible; + })); + } + private class CustomFilterCriteria : IRulesetFilterCriteria { public string? CustomValue { get; set; } diff --git a/osu.Game.Tests/SongSelect/DifficultySearchTest.cs b/osu.Game.Tests/SongSelect/DifficultySearchTest.cs deleted file mode 100644 index 770e40d672..0000000000 --- a/osu.Game.Tests/SongSelect/DifficultySearchTest.cs +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using System.Linq; -using NUnit.Framework; -using osu.Game.Beatmaps; -using osu.Game.Screens.Select; -using osu.Game.Screens.Select.Carousel; -using osu.Game.Tests.Visual; - -namespace osu.Game.Tests.SongSelect -{ - public partial class DifficultySearchTest : OsuTestScene - { - private static readonly (string title, string difficultyName)[] beatmaps = - { - ("Title1", "Diff1"), - ("Title1", "Diff2"), - ("My[Favourite]Song", "Expert"), - ("Title", "My Favourite Diff"), - ("Another One", "diff ]with [[ brackets]]]"), - }; - - [TestCase("[1]", new[] { 0 })] - [TestCase("[1", new[] { 0 })] - [TestCase("My[Favourite", new[] { 2 })] - [TestCase("My[Favourite]", new[] { 2 })] - [TestCase("My[Favourite]Song", new[] { 2 })] - [TestCase("Favourite]", new[] { 2 })] - [TestCase("[Diff", new[] { 0, 1, 3, 4 })] - [TestCase("[Diff]", new[] { 0, 1, 3, 4 })] - [TestCase("[Favourite]", new[] { 3 })] - [TestCase("Title1 [Diff]", new[] { 0, 1 })] - [TestCase("Title1[Diff]", new int[] { })] - [TestCase("[diff ]with]", new[] { 4 })] - [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] - [TestCase("[diff] another [brackets]", new[] { 4 })] - public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) - { - var carouselBeatmaps = createCarouselBeatmaps().ToList(); - - AddStep("filter beatmaps", () => - { - var criteria = new FilterCriteria(); - FilterQueryParser.ApplyQueries(criteria, query); - carouselBeatmaps.ForEach(b => b.Filter(criteria)); - }); - - AddAssert("filtered correctly", () => carouselBeatmaps.All(b => - { - int index = carouselBeatmaps.IndexOf(b); - - bool filtered = b.Filtered.Value; - - return filtered != expectedBeatmapIndexes.Contains(index); - })); - } - - private static IEnumerable createCarouselBeatmaps() - { - return beatmaps.Select(info => new CarouselBeatmap(new BeatmapInfo - { - Metadata = new BeatmapMetadata - { - Title = info.title - }, - DifficultyName = info.difficultyName - })); - } - } -} From 794c3a2473024aa8119cd708acfa4fddebd10dc8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 15:22:27 +0900 Subject: [PATCH 43/51] Add a couple more tests for sanity --- .../NonVisual/Filtering/FilterQueryParserTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index d453954ae0..499300ebf8 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -390,14 +390,16 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("My[Favourite]", new[] { 2 })] [TestCase("My[Favourite]Song", new[] { 2 })] [TestCase("Favourite]", new[] { 2 })] - [TestCase("[Diff", new[] { 0, 1, 3, 4 })] - [TestCase("[Diff]", new[] { 0, 1, 3, 4 })] + [TestCase("[Diff", new[] { 0, 1, 3, 4, 6 })] + [TestCase("[Diff]", new[] { 0, 1, 3, 4, 6 })] [TestCase("[Favourite]", new[] { 3 })] [TestCase("Title1 [Diff]", new[] { 0, 1 })] [TestCase("Title1[Diff]", new int[] { })] [TestCase("[diff ]with]", new[] { 4 })] [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] [TestCase("[diff] another [brackets]", new[] { 4 })] + [TestCase("[Diff in title]", new int[] { })] + [TestCase("[Diff in diff]", new int[] { 6 })] public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) { var carouselBeatmaps = (((string title, string difficultyName)[])new[] @@ -407,6 +409,8 @@ namespace osu.Game.Tests.NonVisual.Filtering ("My[Favourite]Song", "Expert"), ("Title", "My Favourite Diff"), ("Another One", "diff ]with [[ brackets]]]"), + ("Diff in title", "a"), + ("a", "Diff in diff"), }).Select(info => new CarouselBeatmap(new BeatmapInfo { Metadata = new BeatmapMetadata From 6865d8894de02cf68466d809875e45508d839bb1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 16:01:32 +0900 Subject: [PATCH 44/51] Simplify implementation and remove unsupported test coverage --- .../Filtering/FilterQueryParserTest.cs | 1 - .../Select/Carousel/CarouselBeatmap.cs | 4 +- osu.Game/Screens/Select/FilterCriteria.cs | 71 +++++++++--------- osu.Game/Screens/Select/FilterQueryParser.cs | 75 ------------------- 4 files changed, 36 insertions(+), 115 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 499300ebf8..508251f046 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -397,7 +397,6 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("Title1[Diff]", new int[] { })] [TestCase("[diff ]with]", new[] { 4 })] [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] - [TestCase("[diff] another [brackets]", new[] { 4 })] [TestCase("[Diff in title]", new int[] { })] [TestCase("[Diff in diff]", new int[] { 6 })] public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index f433e71cc3..ff781db680 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -59,13 +59,13 @@ namespace osu.Game.Screens.Select.Carousel criteria.Artist.Matches(BeatmapInfo.Metadata.ArtistUnicode); match &= !criteria.Title.HasFilter || criteria.Title.Matches(BeatmapInfo.Metadata.Title) || criteria.Title.Matches(BeatmapInfo.Metadata.TitleUnicode); + match &= !criteria.DifficultyName.HasFilter || criteria.DifficultyName.Matches(BeatmapInfo.DifficultyName) || + criteria.DifficultyName.Matches(BeatmapInfo.Metadata.TitleUnicode); match &= !criteria.UserStarDifficulty.HasFilter || criteria.UserStarDifficulty.IsInRange(BeatmapInfo.StarRating); if (!match) return false; - match &= criteria.DifficultySearchTerms.All(term => term.Matches(BeatmapInfo.DifficultyName)); - if (criteria.SearchTerms.Length > 0) { var searchableTerms = BeatmapInfo.GetSearchableTerms(); diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 1bcff601e5..5666f9d1d4 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -38,6 +38,7 @@ namespace osu.Game.Screens.Select public OptionalTextFilter Creator; public OptionalTextFilter Artist; public OptionalTextFilter Title; + public OptionalTextFilter DifficultyName; public OptionalRange UserStarDifficulty = new OptionalRange { @@ -47,11 +48,6 @@ namespace osu.Game.Screens.Select public OptionalTextFilter[] SearchTerms = Array.Empty(); - /// - /// Search terms that are used for searching difficulty names. - /// - public OptionalTextFilter[] DifficultySearchTerms = Array.Empty(); - public RulesetInfo? Ruleset; public bool AllowConvertedBeatmaps; @@ -69,7 +65,39 @@ namespace osu.Game.Screens.Select { searchText = value; - SearchTerms = getTermsFromSearchText(value); + List terms = new List(); + + string remainingText = value; + + // Match either an open difficulty tag to the end of string, + // or match a closed one with a whitespace after it. + // + // To keep things simple, the closing ']' may be included in the match group, + // and is trimmer post-match. + foreach (Match quotedSegment in Regex.Matches(value, "(^|\\s)\\[(.*)(\\]\\s|$)")) + { + DifficultyName = new OptionalTextFilter + { + SearchTerm = quotedSegment.Groups[2].Value.Trim(']') + }; + + remainingText = remainingText.Replace(quotedSegment.Value, string.Empty); + } + + // First handle quoted segments to ensure we keep inline spaces in exact matches. + foreach (Match quotedSegment in Regex.Matches(value, "(\"[^\"]+\"[!]?)")) + { + terms.Add(new OptionalTextFilter { SearchTerm = quotedSegment.Value }); + remainingText = remainingText.Replace(quotedSegment.Value, string.Empty); + } + + // Then handle the rest splitting on any spaces. + terms.AddRange(remainingText.Split(' ', StringSplitOptions.RemoveEmptyEntries).Select(s => new OptionalTextFilter + { + SearchTerm = s + })); + + SearchTerms = terms.ToArray(); SearchNumber = null; @@ -78,15 +106,6 @@ namespace osu.Game.Screens.Select } } - /// - /// Extracts the search terms from the provided - /// and stores them in . - /// - public string DifficultySearchText - { - set => DifficultySearchTerms = getTermsFromSearchText(value); - } - /// /// Hashes from the to filter to. /// @@ -94,28 +113,6 @@ namespace osu.Game.Screens.Select public IRulesetFilterCriteria? RulesetCriteria { get; set; } - private static OptionalTextFilter[] getTermsFromSearchText(string searchText) - { - List terms = new List(); - - string remainingText = searchText; - - // First handle quoted segments to ensure we keep inline spaces in exact matches. - foreach (Match quotedSegment in Regex.Matches(searchText, "(\"[^\"]+\"[!]?)")) - { - terms.Add(new OptionalTextFilter { SearchTerm = quotedSegment.Value }); - remainingText = remainingText.Replace(quotedSegment.Value, string.Empty); - } - - // Then handle the rest splitting on any spaces. - terms.AddRange(remainingText.Split(' ', StringSplitOptions.RemoveEmptyEntries).Select(s => new OptionalTextFilter - { - SearchTerm = s - })); - - return terms.ToArray(); - } - public struct OptionalRange : IEquatable> where T : struct { diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 33ddd08e09..1238173b41 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -21,8 +21,6 @@ namespace osu.Game.Screens.Select internal static void ApplyQueries(FilterCriteria criteria, string query) { - criteria.DifficultySearchText = extractDifficultySearchText(ref query); - foreach (Match match in query_syntax_regex.Matches(query)) { string key = match.Groups["key"].Value.ToLowerInvariant(); @@ -36,79 +34,6 @@ namespace osu.Game.Screens.Select criteria.SearchText = query; } - /// - /// Extracts and returns the difficulty search text between square brackets. - /// - /// The search query. The difficulty search text will be removed from the query. - /// The difficulty search text (without the square brackets). - private static string extractDifficultySearchText(ref string query) - { - var openingBracketIndexes = new List(); - var closingBracketIndexes = new List(); - - populateIndexLists(query); - - return performExtraction(ref query); - - void populateIndexLists(string query) - { - bool currentlyBetweenBrackets = false; - - for (int i = 0; i < query.Length; i++) - { - switch (query[i]) - { - case '[': - if (!currentlyBetweenBrackets && (i == 0 || query[i - 1] == ' ')) - { - currentlyBetweenBrackets = true; - openingBracketIndexes.Add(i + 1); - } - - break; - - case ']': - if (currentlyBetweenBrackets && (i == query.Length - 1 || query[i + 1] == ' ')) - { - currentlyBetweenBrackets = false; - closingBracketIndexes.Add(i); - } - - break; - } - } - } - - string performExtraction(ref string query) - { - var searchTexts = new List(); - string originalQuery = query; - - for (int i = 0; i < openingBracketIndexes.Count; i++) - { - int startIndex = openingBracketIndexes[i]; - - int endIndex = closingBracketIndexes.Count > 0 - ? closingBracketIndexes[Math.Min(i, closingBracketIndexes.Count - 1)] - : query.Length; - - string searchText = originalQuery[startIndex..endIndex]; - - searchTexts.Add(searchText); - - query = query - .Replace($" [{searchText}]", "") - .Replace($"[{searchText}] ", "") - .Replace($"[{searchText}]", "") - .Replace($" [{searchText}] ", " ") - .Replace($" [{searchText}", "") - .Replace($"[{searchText}", ""); - } - - return string.Join(' ', searchTexts); - } - } - private static bool tryParseKeywordCriteria(FilterCriteria criteria, string key, string value, Operator op) { switch (key) From fcb366af4db6736e4786c705de7a3cee59dc0e1d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 16:09:05 +0900 Subject: [PATCH 45/51] Add `diff=` support for more advanced usages --- .../NonVisual/Filtering/FilterQueryParserTest.cs | 16 ++++++++-------- osu.Game/Screens/Select/FilterQueryParser.cs | 3 +++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index 508251f046..74d47b43af 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -399,6 +399,10 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] [TestCase("[Diff in title]", new int[] { })] [TestCase("[Diff in diff]", new int[] { 6 })] + [TestCase("diff=Diff", new[] { 0, 1, 3, 4, 6 })] + [TestCase("diff=Diff1", new[] { 0 })] + [TestCase("diff=\"Diff\"", new[] { 3, 4, 6 })] + [TestCase("diff=!\"Diff\"", new int[] {})] public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) { var carouselBeatmaps = (((string title, string difficultyName)[])new[] @@ -424,15 +428,11 @@ namespace osu.Game.Tests.NonVisual.Filtering FilterQueryParser.ApplyQueries(criteria, query); carouselBeatmaps.ForEach(b => b.Filter(criteria)); - Assert.That(carouselBeatmaps.All(b => - { - int index = carouselBeatmaps.IndexOf(b); + int[] visibleBeatmaps = carouselBeatmaps + .Where(b => !b.Filtered.Value) + .Select(b => carouselBeatmaps.IndexOf(b)).ToArray(); - bool shouldBeVisible = expectedBeatmapIndexes.Contains(index); - bool isVisible = !b.Filtered.Value; - - return isVisible == shouldBeVisible; - })); + Assert.That(visibleBeatmaps, Is.EqualTo(expectedBeatmapIndexes)); } private class CustomFilterCriteria : IRulesetFilterCriteria diff --git a/osu.Game/Screens/Select/FilterQueryParser.cs b/osu.Game/Screens/Select/FilterQueryParser.cs index 1238173b41..0d8905347b 100644 --- a/osu.Game/Screens/Select/FilterQueryParser.cs +++ b/osu.Game/Screens/Select/FilterQueryParser.cs @@ -76,6 +76,9 @@ namespace osu.Game.Screens.Select case "title": return TryUpdateCriteriaText(ref criteria.Title, op, value); + case "diff": + return TryUpdateCriteriaText(ref criteria.DifficultyName, op, value); + default: return criteria.RulesetCriteria?.TryParseCustomKeywordCriteria(key, op, value) ?? false; } From b4cc12ab5aee4682359f874548f937793a075d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 24 Oct 2023 10:00:58 +0200 Subject: [PATCH 46/51] Fix formatting --- 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 74d47b43af..bfdd5f92a2 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -402,7 +402,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("diff=Diff", new[] { 0, 1, 3, 4, 6 })] [TestCase("diff=Diff1", new[] { 0 })] [TestCase("diff=\"Diff\"", new[] { 3, 4, 6 })] - [TestCase("diff=!\"Diff\"", new int[] {})] + [TestCase("diff=!\"Diff\"", new int[] { })] public void TestDifficultySearch(string query, int[] expectedBeatmapIndexes) { var carouselBeatmaps = (((string title, string difficultyName)[])new[] From abfe2ce0fe28c8efc99ef622a6724fe399bb4ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 24 Oct 2023 10:05:20 +0200 Subject: [PATCH 47/51] Fix comment --- osu.Game/Screens/Select/FilterCriteria.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 5666f9d1d4..812a16c484 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -73,7 +73,7 @@ namespace osu.Game.Screens.Select // or match a closed one with a whitespace after it. // // To keep things simple, the closing ']' may be included in the match group, - // and is trimmer post-match. + // and is trimmed post-match. foreach (Match quotedSegment in Regex.Matches(value, "(^|\\s)\\[(.*)(\\]\\s|$)")) { DifficultyName = new OptionalTextFilter From c9161a7bfcbf09a9ce577e174c5867475e21476d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 17:32:01 +0900 Subject: [PATCH 48/51] Remove incorrect copy paste fail --- osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index ff781db680..1d40862df7 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -59,9 +59,7 @@ namespace osu.Game.Screens.Select.Carousel criteria.Artist.Matches(BeatmapInfo.Metadata.ArtistUnicode); match &= !criteria.Title.HasFilter || criteria.Title.Matches(BeatmapInfo.Metadata.Title) || criteria.Title.Matches(BeatmapInfo.Metadata.TitleUnicode); - match &= !criteria.DifficultyName.HasFilter || criteria.DifficultyName.Matches(BeatmapInfo.DifficultyName) || - criteria.DifficultyName.Matches(BeatmapInfo.Metadata.TitleUnicode); - + match &= !criteria.DifficultyName.HasFilter || criteria.DifficultyName.Matches(BeatmapInfo.DifficultyName); match &= !criteria.UserStarDifficulty.HasFilter || criteria.UserStarDifficulty.IsInRange(BeatmapInfo.StarRating); if (!match) return false; From a0c8158033fcb59459bd10e7ceaba11310241ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 24 Oct 2023 10:42:03 +0200 Subject: [PATCH 49/51] Remove unnecessary `LifetimeEnd` specification --- .../Objects/Drawables/DrawableSpinnerTick.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index 20fe7172ad..b2de4da01b 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -5,7 +5,6 @@ using osu.Framework.Graphics; using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Objects.Drawables { @@ -37,16 +36,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables LifetimeStart = DrawableSpinner.HitObject.StartTime; } - protected override void UpdateHitStateTransforms(ArmedState state) - { - base.UpdateHitStateTransforms(state); - - // the tick can be theoretically judged at any point in the spinner's duration, - // so it must be alive throughout the spinner's entire lifetime (or until hit, whichever applies). - // this mostly matters for correct sample playback. - LifetimeEnd = (Result?.IsHit == true ? Result.TimeAbsolute : DrawableSpinner.HitObject.GetEndTime()) + (Samples?.Length ?? 0); - } - /// /// Apply a judgement result. /// From 68397f0e816f131dda8f20b6959298403d7a87b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 18:01:24 +0900 Subject: [PATCH 50/51] Remove unused using --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index b2de4da01b..a5785dd1f6 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -4,7 +4,6 @@ #nullable disable using osu.Framework.Graphics; -using osu.Game.Rulesets.Objects; namespace osu.Game.Rulesets.Osu.Objects.Drawables { From 966decb0089631032a5eb29a1f547faf16e83cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 24 Oct 2023 11:25:37 +0200 Subject: [PATCH 51/51] Remove redundant explicit array type specification --- 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 bfdd5f92a2..739a72df08 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -398,7 +398,7 @@ namespace osu.Game.Tests.NonVisual.Filtering [TestCase("[diff ]with]", new[] { 4 })] [TestCase("[diff ]with [[ brackets]]]]", new[] { 4 })] [TestCase("[Diff in title]", new int[] { })] - [TestCase("[Diff in diff]", new int[] { 6 })] + [TestCase("[Diff in diff]", new[] { 6 })] [TestCase("diff=Diff", new[] { 0, 1, 3, 4, 6 })] [TestCase("diff=Diff1", new[] { 0 })] [TestCase("diff=\"Diff\"", new[] { 3, 4, 6 })]