Clamp :ScaleTo values to be greater than zero #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fraktality
reviewed
Sep 24, 2023
spr.lua
Outdated
| @@ -629,7 +629,7 @@ local PSEUDO_PROPERTIES: PropertyOverride = { | |||
| return inst:GetScale() | |||
| end, | |||
| set = function(inst: Model, value: number) | |||
| inst:ScaleTo(value) | |||
| inst:ScaleTo(math.clamp(value, 1e-4, math.huge)) | |||
There was a problem hiding this comment.
ScaleTo apparently allows any input as long as it's not zero. Can we make sure this allows the smallest possible floating point value that isn't subnormal?
We should also think about clamping at some value below infinity to avoid warnings. 2^53 should work.
|
ScaleTo will accept values up to 10^-45 before rounding to zero. However, if we use that as our epsilon, there are cases where value gets rounded to zero anyway (ex. when the spring sleeps). 10^-35 seems to not round to zero, so we can use that as our epsilon. |
Fraktality
reviewed
Oct 1, 2023
| @@ -37,6 +37,7 @@ local SLEEP_ROTATION_OFFSET = math.rad(0.01) -- rad | |||
| local SLEEP_ROTATION_VELOCITY = math.rad(0.1) -- rad/s | |||
| local EPS = 1e-5 -- epsilon for stability checks around pathological frequency/damping values | |||
| local AXIS_MATRIX_EPS = 1e-6 -- epsilon for converting from axis-angle to matrix | |||
| local SCALE_TO_EPS = 1e-35 -- epsilon used for clamping ScaleTo calls above 0 | |||
There was a problem hiding this comment.
Can we make this a table or NumberRange? e.g. SCALE_TO_RANGE = {1e-35, 2^53}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes a spring can go over its target and call :ScaleTo with values <= 0, which results in an error. Fixed by clamping the setter to be always greater than zero.