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