From ffef6ae1853d84120abf52f3c93382b4863bd556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 28 Feb 2025 13:34:00 +0100 Subject: [PATCH 1/6] Fix possible crash when scaling objects in editor The specific fail case here is when `s.{X,Y}` is 0, and `s{Lower,Upper}Bound` is `Infinity`. Because IEEE math is IEEE math, `0 * Infinity` is `NaN`. `MathHelper.Clamp()` is written the following way: https://github.com/ppy/osuTK/blob/af742f1afd01828efc7bc9fe77536b54aab8b419/src/osuTK/Math/MathHelper.cs#L284-L306 `Math.{Min,Max}` are both documented as reporting `NaN` when any of their operands are `NaN`: https://learn.microsoft.com/en-us/dotnet/api/system.math.min?view=net-8.0#system-math-min(system-single-system-single) https://learn.microsoft.com/en-us/dotnet/api/system.math.max?view=net-8.0#system-math-max(system-single-system-single) which means that if a `NaN` happens to sneak into the bounds, it will start spreading outwards in an uncontrolled manner, and likely crash things. In contrast, the standard library provided `Math.Clamp()` is written like so: https://github.com/dotnet/runtime/blob/577c36cee56480dec4d4610b35605b5d5836888b/src/libraries/System.Private.CoreLib/src/System/Math.cs#L711-L729 With this implementation, if either bound is `NaN`, it will essentially not be checked (because any and all comparisons involving `NaN` return false). This prevents the spread of `NaN`s, all the way to positions of hitobjects, and thus fixes the crash. --- .../Edit/OsuSelectionScaleHandler.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs b/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs index e3ab95c402..4c3db207f2 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs @@ -263,12 +263,12 @@ namespace osu.Game.Rulesets.Osu.Edit { case Axes.X: (sLowerBound, sUpperBound) = computeBounds(lowerBounds - b, upperBounds - b, a); - s.X = MathHelper.Clamp(s.X, sLowerBound, sUpperBound); + s.X = Math.Clamp(s.X, sLowerBound, sUpperBound); break; case Axes.Y: (sLowerBound, sUpperBound) = computeBounds(lowerBounds - a, upperBounds - a, b); - s.Y = MathHelper.Clamp(s.Y, sLowerBound, sUpperBound); + s.Y = Math.Clamp(s.Y, sLowerBound, sUpperBound); break; case Axes.Both: @@ -276,11 +276,11 @@ namespace osu.Game.Rulesets.Osu.Edit // Therefore the ratio s.X / s.Y will be maintained (sLowerBound, sUpperBound) = computeBounds(lowerBounds, upperBounds, a * s.X + b * s.Y); s.X = s.X < 0 - ? MathHelper.Clamp(s.X, s.X * sUpperBound, s.X * sLowerBound) - : MathHelper.Clamp(s.X, s.X * sLowerBound, s.X * sUpperBound); + ? Math.Clamp(s.X, s.X * sUpperBound, s.X * sLowerBound) + : Math.Clamp(s.X, s.X * sLowerBound, s.X * sUpperBound); s.Y = s.Y < 0 - ? MathHelper.Clamp(s.Y, s.Y * sUpperBound, s.Y * sLowerBound) - : MathHelper.Clamp(s.Y, s.Y * sLowerBound, s.Y * sUpperBound); + ? Math.Clamp(s.Y, s.Y * sUpperBound, s.Y * sLowerBound) + : Math.Clamp(s.Y, s.Y * sLowerBound, s.Y * sUpperBound); break; } From 35b0ff80bb6094a32d9c5c2b93203faf491b68fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 28 Feb 2025 13:41:56 +0100 Subject: [PATCH 2/6] Mark `MathHelper.Clamp()` as banned API See previous commit for partial rationale. There's an argument to be made about the `NaN`-spreading semantics being desirable because at least something will loudly fail in that case, but I'm not so sure about that these days. It feels like either way if `NaN`s are produced, then things are outside of any control, and chances are the game can probably continue without crashing. And, this move reduces our dependence on osuTK, which has already been living on borrowed time for years now and is only awaiting someone brave to go excise it. --- CodeAnalysis/BannedSymbols.txt | 3 +++ .../Beatmaps/PippidonBeatmapConverter.cs | 4 ++-- .../Skinning/Argon/ArgonBananaPiece.cs | 3 ++- .../HitCircles/Components/HitCircleOverlapMarker.cs | 3 ++- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 4 ++-- osu.Game/Overlays/NotificationOverlayToastTray.cs | 2 +- osu.Game/Screens/Play/HUDOverlay.cs | 6 +++--- osu.Game/Screens/Utility/CircleGameplay.cs | 4 ++-- .../Utility/SampleComponents/LatencyMovableBox.cs | 9 +++++---- osu.Game/Screens/Utility/ScrollingGameplay.cs | 2 +- 10 files changed, 23 insertions(+), 17 deletions(-) diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt index 550f7c8e11..08b79fc2c0 100644 --- a/CodeAnalysis/BannedSymbols.txt +++ b/CodeAnalysis/BannedSymbols.txt @@ -18,3 +18,6 @@ M:Humanizer.InflectorExtensions.Pascalize(System.String);Humanizer's .Pascalize( M:Humanizer.InflectorExtensions.Camelize(System.String);Humanizer's .Camelize() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToCamelCase() instead. M:Humanizer.InflectorExtensions.Underscore(System.String);Humanizer's .Underscore() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToSnakeCase() instead. M:Humanizer.InflectorExtensions.Kebaberize(System.String);Humanizer's .Kebaberize() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToKebabCase() instead. +M:osuTK.MathHelper.Clamp(System.Int32,System.Int32,System.Int32)~System.Int32;Use Math.Clamp() instead. +M:osuTK.MathHelper.Clamp(System.Single,System.Single,System.Single)~System.Single;This osuTK helper has unsafe semantics when one of the bounds provided is NaN. Use Math.Clamp() instead. +M:osuTK.MathHelper.Clamp(System.Double,System.Double,System.Double)~System.Double;This osuTK helper has unsafe semantics when one of the bounds provided is NaN. Use Math.Clamp() instead. diff --git a/Templates/Rulesets/ruleset-scrolling-example/osu.Game.Rulesets.Pippidon/Beatmaps/PippidonBeatmapConverter.cs b/Templates/Rulesets/ruleset-scrolling-example/osu.Game.Rulesets.Pippidon/Beatmaps/PippidonBeatmapConverter.cs index 0a4fa84ce1..dd8337abee 100644 --- a/Templates/Rulesets/ruleset-scrolling-example/osu.Game.Rulesets.Pippidon/Beatmaps/PippidonBeatmapConverter.cs +++ b/Templates/Rulesets/ruleset-scrolling-example/osu.Game.Rulesets.Pippidon/Beatmaps/PippidonBeatmapConverter.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -9,7 +10,6 @@ using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Pippidon.Objects; using osu.Game.Rulesets.Pippidon.UI; -using osuTK; namespace osu.Game.Rulesets.Pippidon.Beatmaps { @@ -40,7 +40,7 @@ namespace osu.Game.Rulesets.Pippidon.Beatmaps }; } - private int getLane(HitObject hitObject) => (int)MathHelper.Clamp( + private int getLane(HitObject hitObject) => (int)Math.Clamp( (getUsablePosition(hitObject) - minPosition) / (maxPosition - minPosition) * PippidonPlayfield.LANE_COUNT, 0, PippidonPlayfield.LANE_COUNT - 1); private float getUsablePosition(HitObject h) => (h as IHasYPosition)?.Y ?? ((IHasXPosition)h).X; diff --git a/osu.Game.Rulesets.Catch/Skinning/Argon/ArgonBananaPiece.cs b/osu.Game.Rulesets.Catch/Skinning/Argon/ArgonBananaPiece.cs index 8cdb490922..810dc7eed5 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Argon/ArgonBananaPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Argon/ArgonBananaPiece.cs @@ -1,6 +1,7 @@ // 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 osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; @@ -110,7 +111,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Argon double duration = ObjectState.HitObject.StartTime - ObjectState.DisplayStartTime; - fadeContent.Alpha = MathHelper.Clamp( + fadeContent.Alpha = Math.Clamp( Interpolation.ValueAt( Time.Current, 1f, 0f, ObjectState.DisplayStartTime + duration * lens_flare_start, diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/HitCircles/Components/HitCircleOverlapMarker.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/HitCircles/Components/HitCircleOverlapMarker.cs index 8ed9d0476a..7a5b01ce79 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/HitCircles/Components/HitCircleOverlapMarker.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/HitCircles/Components/HitCircleOverlapMarker.cs @@ -3,6 +3,7 @@ #nullable disable +using System; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -76,7 +77,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components if (hasReachedObject && showHitMarkers.Value) { float alpha = Interpolation.ValueAt(editorTime, 0, 1f, hitObjectTime, hitObjectTime + FADE_OUT_EXTENSION, Easing.In); - float ringScale = MathHelper.Clamp(Interpolation.ValueAt(editorTime, 0, 1f, hitObjectTime, hitObjectTime + FADE_OUT_EXTENSION / 2, Easing.OutQuint), 0, 1); + float ringScale = Math.Clamp(Interpolation.ValueAt(editorTime, 0, 1f, hitObjectTime, hitObjectTime + FADE_OUT_EXTENSION / 2, Easing.OutQuint), 0, 1); ring.Scale = new Vector2(1 + 0.1f * ringScale); content.Alpha = 0.9f * (1 - alpha); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 39c0681dba..52575bdd67 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -270,14 +270,14 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders if (adjustVelocity) { proposedVelocity = proposedDistance / oldDuration; - proposedDistance = MathHelper.Clamp(proposedDistance, 0.1 * oldDuration, 10 * oldDuration); + proposedDistance = Math.Clamp(proposedDistance, 0.1 * oldDuration, 10 * oldDuration); } else { double minDistance = distanceSnapProvider?.GetBeatSnapDistance() * oldVelocityMultiplier ?? 1; // Add a small amount to the proposed distance to make it easier to snap to the full length of the slider. proposedDistance = distanceSnapProvider?.FindSnappedDistance((float)proposedDistance + 1, HitObject.StartTime, HitObject) ?? proposedDistance; - proposedDistance = MathHelper.Clamp(proposedDistance, minDistance, HitObject.Path.CalculatedDistance); + proposedDistance = Math.Clamp(proposedDistance, minDistance, HitObject.Path.CalculatedDistance); } if (Precision.AlmostEquals(proposedDistance, HitObject.Path.Distance) && Precision.AlmostEquals(proposedVelocity, HitObject.SliderVelocityMultiplier)) diff --git a/osu.Game/Overlays/NotificationOverlayToastTray.cs b/osu.Game/Overlays/NotificationOverlayToastTray.cs index ddb2e02fb8..dd60e303f6 100644 --- a/osu.Game/Overlays/NotificationOverlayToastTray.cs +++ b/osu.Game/Overlays/NotificationOverlayToastTray.cs @@ -174,7 +174,7 @@ namespace osu.Game.Overlays } height = toastFlow.DrawHeight + 120; - alpha = MathHelper.Clamp(toastFlow.DrawHeight / 41, 0, 1) * maxNotificationAlpha; + alpha = Math.Clamp(toastFlow.DrawHeight / 41, 0, 1) * maxNotificationAlpha; } toastContentBackground.Height = (float)Interpolation.DampContinuously(toastContentBackground.Height, height, 10, Clock.ElapsedFrameTime); diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index 8bfa8dd6ff..19190ac362 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -278,17 +278,17 @@ namespace osu.Game.Screens.Play processDrawables(rulesetComponents); if (lowestTopScreenSpaceRight.HasValue) - TopRightElements.Y = MathHelper.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceRight.Value)).Y, 0, DrawHeight - TopRightElements.DrawHeight); + TopRightElements.Y = Math.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceRight.Value)).Y, 0, DrawHeight - TopRightElements.DrawHeight); else TopRightElements.Y = 0; if (lowestTopScreenSpaceLeft.HasValue) - LeaderboardFlow.Y = MathHelper.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceLeft.Value)).Y, 0, DrawHeight - LeaderboardFlow.DrawHeight); + LeaderboardFlow.Y = Math.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceLeft.Value)).Y, 0, DrawHeight - LeaderboardFlow.DrawHeight); else LeaderboardFlow.Y = 0; if (highestBottomScreenSpace.HasValue) - bottomRightElements.Y = BottomScoringElementsHeight = -MathHelper.Clamp(DrawHeight - ToLocalSpace(highestBottomScreenSpace.Value).Y, 0, DrawHeight - bottomRightElements.DrawHeight); + bottomRightElements.Y = BottomScoringElementsHeight = -Math.Clamp(DrawHeight - ToLocalSpace(highestBottomScreenSpace.Value).Y, 0, DrawHeight - bottomRightElements.DrawHeight); else bottomRightElements.Y = 0; diff --git a/osu.Game/Screens/Utility/CircleGameplay.cs b/osu.Game/Screens/Utility/CircleGameplay.cs index 1f970c5121..0f328d04fb 100644 --- a/osu.Game/Screens/Utility/CircleGameplay.cs +++ b/osu.Game/Screens/Utility/CircleGameplay.cs @@ -201,8 +201,8 @@ namespace osu.Game.Screens.Utility { double preempt = (float)IBeatmapDifficultyInfo.DifficultyRange(SampleApproachRate.Value, 1800, 1200, 450); - approach.Scale = new Vector2(1 + 4 * (float)MathHelper.Clamp((HitTime - Clock.CurrentTime) / preempt, 0, 100)); - Alpha = (float)MathHelper.Clamp((Clock.CurrentTime - HitTime + 600) / 400, 0, 1); + approach.Scale = new Vector2(1 + 4 * (float)Math.Clamp((HitTime - Clock.CurrentTime) / preempt, 0, 100)); + Alpha = (float)Math.Clamp((Clock.CurrentTime - HitTime + 600) / 400, 0, 1); if (Clock.CurrentTime > HitTime + duration) Expire(); diff --git a/osu.Game/Screens/Utility/SampleComponents/LatencyMovableBox.cs b/osu.Game/Screens/Utility/SampleComponents/LatencyMovableBox.cs index dcfcf602bf..ef1b848945 100644 --- a/osu.Game/Screens/Utility/SampleComponents/LatencyMovableBox.cs +++ b/osu.Game/Screens/Utility/SampleComponents/LatencyMovableBox.cs @@ -1,6 +1,7 @@ // 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 osu.Framework.Graphics; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; @@ -55,22 +56,22 @@ namespace osu.Game.Screens.Utility.SampleComponents { case Key.F: case Key.Up: - box.Y = MathHelper.Clamp(box.Y - movementAmount, 0.1f, 0.9f); + box.Y = Math.Clamp(box.Y - movementAmount, 0.1f, 0.9f); break; case Key.J: case Key.Down: - box.Y = MathHelper.Clamp(box.Y + movementAmount, 0.1f, 0.9f); + box.Y = Math.Clamp(box.Y + movementAmount, 0.1f, 0.9f); break; case Key.Z: case Key.Left: - box.X = MathHelper.Clamp(box.X - movementAmount, 0.1f, 0.9f); + box.X = Math.Clamp(box.X - movementAmount, 0.1f, 0.9f); break; case Key.X: case Key.Right: - box.X = MathHelper.Clamp(box.X + movementAmount, 0.1f, 0.9f); + box.X = Math.Clamp(box.X + movementAmount, 0.1f, 0.9f); break; } } diff --git a/osu.Game/Screens/Utility/ScrollingGameplay.cs b/osu.Game/Screens/Utility/ScrollingGameplay.cs index 5038c53b4a..c0264f5734 100644 --- a/osu.Game/Screens/Utility/ScrollingGameplay.cs +++ b/osu.Game/Screens/Utility/ScrollingGameplay.cs @@ -165,7 +165,7 @@ namespace osu.Game.Screens.Utility { double preempt = (float)IBeatmapDifficultyInfo.DifficultyRange(SampleApproachRate.Value, 1800, 1200, 450); - Alpha = (float)MathHelper.Clamp((Clock.CurrentTime - HitTime + 600) / 400, 0, 1); + Alpha = (float)Math.Clamp((Clock.CurrentTime - HitTime + 600) / 400, 0, 1); Y = judgement_position - (float)((HitTime - Clock.CurrentTime) / preempt); if (Clock.CurrentTime > HitTime + duration) From 47747aed3e9feb09c3b6d9f82703cedda8db3035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 3 Mar 2025 08:40:51 +0100 Subject: [PATCH 3/6] Add guards to prevent clamp calls with invalid bounds --- osu.Game/Screens/Play/HUDOverlay.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index 19190ac362..78c602d8f1 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -277,17 +277,17 @@ namespace osu.Game.Screens.Play if (rulesetComponents != null) processDrawables(rulesetComponents); - if (lowestTopScreenSpaceRight.HasValue) + if (lowestTopScreenSpaceRight.HasValue && DrawHeight - TopRightElements.DrawHeight > 0) TopRightElements.Y = Math.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceRight.Value)).Y, 0, DrawHeight - TopRightElements.DrawHeight); else TopRightElements.Y = 0; - if (lowestTopScreenSpaceLeft.HasValue) + if (lowestTopScreenSpaceLeft.HasValue && DrawHeight - LeaderboardFlow.DrawHeight > 0) LeaderboardFlow.Y = Math.Clamp(ToLocalSpace(new Vector2(0, lowestTopScreenSpaceLeft.Value)).Y, 0, DrawHeight - LeaderboardFlow.DrawHeight); else LeaderboardFlow.Y = 0; - if (highestBottomScreenSpace.HasValue) + if (highestBottomScreenSpace.HasValue && DrawHeight - bottomRightElements.DrawHeight > 0) bottomRightElements.Y = BottomScoringElementsHeight = -Math.Clamp(DrawHeight - ToLocalSpace(highestBottomScreenSpace.Value).Y, 0, DrawHeight - bottomRightElements.DrawHeight); else bottomRightElements.Y = 0; From 0a50fb1dfac7b0898c134f98c47a459fbbeb769c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 3 Mar 2025 09:32:27 +0100 Subject: [PATCH 4/6] Add failing test case --- .../Beatmaps/BeatmapExtensionsTest.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 osu.Game.Tests/Beatmaps/BeatmapExtensionsTest.cs diff --git a/osu.Game.Tests/Beatmaps/BeatmapExtensionsTest.cs b/osu.Game.Tests/Beatmaps/BeatmapExtensionsTest.cs new file mode 100644 index 0000000000..1dda2e314d --- /dev/null +++ b/osu.Game.Tests/Beatmaps/BeatmapExtensionsTest.cs @@ -0,0 +1,58 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Timing; +using osu.Game.Rulesets.Osu.Objects; + +namespace osu.Game.Tests.Beatmaps +{ + public class BeatmapExtensionsTest + { + [Test] + public void TestLengthCalculations() + { + var beatmap = new Beatmap + { + HitObjects = + { + new HitCircle { StartTime = 5_000 }, + new HitCircle { StartTime = 300_000 }, + new Spinner { StartTime = 280_000, Duration = 40_000 } + }, + Breaks = + { + new BreakPeriod(50_000, 75_000), + new BreakPeriod(100_000, 150_000), + } + }; + + Assert.That(beatmap.CalculatePlayableBounds(), Is.EqualTo((5_000, 320_000))); + Assert.That(beatmap.CalculatePlayableLength(), Is.EqualTo(315_000)); // 320_000 - 5_000 + Assert.That(beatmap.CalculateDrainLength(), Is.EqualTo(240_000)); // 315_000 - (25_000 + 50_000) = 315_000 - 75_000 + } + + [Test] + public void TestDrainLengthCannotGoNegative() + { + var beatmap = new Beatmap + { + HitObjects = + { + new HitCircle { StartTime = 5_000 }, + new HitCircle { StartTime = 300_000 }, + new Spinner { StartTime = 280_000, Duration = 40_000 } + }, + Breaks = + { + new BreakPeriod(0, 350_000), + } + }; + + Assert.That(beatmap.CalculatePlayableBounds(), Is.EqualTo((5_000, 320_000))); + Assert.That(beatmap.CalculatePlayableLength(), Is.EqualTo(315_000)); // 320_000 - 5_000 + Assert.That(beatmap.CalculateDrainLength(), Is.EqualTo(0)); // break period encompasses entire beatmap + } + } +} From 87fb8da3517ae0f2d0669dc3afa9b233454c49bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 3 Mar 2025 09:35:46 +0100 Subject: [PATCH 5/6] Fix drain length calculation helper method being able to return negative durations This is the principal failure behind https://github.com/ppy/osu-server-beatmap-submission/issues/40. --- osu.Game/Beatmaps/IBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 826d4e19a7..f95fcefd7e 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -161,7 +161,7 @@ namespace osu.Game.Beatmaps /// /// Find the total milliseconds between the first and last hittable objects, excluding any break time. /// - public static double CalculateDrainLength(this IBeatmap beatmap) => CalculatePlayableLength(beatmap.HitObjects) - beatmap.TotalBreakTime; + public static double CalculateDrainLength(this IBeatmap beatmap) => Math.Max(CalculatePlayableLength(beatmap.HitObjects) - beatmap.TotalBreakTime, 0); /// /// Find the timestamps in milliseconds of the start and end of the playable region. From b73a872b94f1053f57612ad32c1d07c88b8d908c Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 4 Mar 2025 14:11:32 +0900 Subject: [PATCH 6/6] Fix broken test --- .../NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs index 959f09361f..4019ff6730 100644 --- a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs +++ b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs @@ -97,16 +97,12 @@ namespace osu.Game.Tests.NonVisual.Multiplayer AddStep("create room with many users", () => { - var newRoom = new Room(); - newRoom.CopyFrom(SelectedRoom.Value!); - - newRoom.RoomID = null; MultiplayerClient.RoomSetupAction = room => { room.Users.AddRange(Enumerable.Range(PLAYER_1_ID, 100).Select(id => new MultiplayerRoomUser(id))); }; - RoomManager.CreateRoom(newRoom); + MultiplayerClient.JoinRoom(MultiplayerClient.ServerSideRooms.Single()).ConfigureAwait(false); }); AddUntilStep("wait for room join", () => RoomJoined);