Skip to content

Fix PositionToValue() for EditorCurve.Handle when curve length <= 1 - #11690

Open
LeDernierPoisson wants to merge 2 commits into
Facepunch:masterfrom
LeDernierPoisson:fix-curve-range-widget
Open

Fix PositionToValue() for EditorCurve.Handle when curve length <= 1#11690
LeDernierPoisson wants to merge 2 commits into
Facepunch:masterfrom
LeDernierPoisson:fix-curve-range-widget

Conversation

@LeDernierPoisson

@LeDernierPoisson LeDernierPoisson commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

…is set

Pull Request

Thanks for contributing to s&box ❤️
Please fill out the sections below to help us review your change efficiently.


Summary

This fixes a bug with the curve editor where if you have only 1 keyframes set , the computed value of PositionToValue() for any consumer would be the square root of what it should actually be

Motivation & Context

This is clearly a bug and makes it really annoying to play with curves. This bug also caused an unstable state to the range curves, even with multiple keyframes set, the computed value was of. Now since you can't have that behaviour again the curve with ranges never gets in this state

Fixes:

Implementation Details

Looking at it I realized it was pretty straightforward. A guarding prevented normalization of the time and value viewport ranges if the curve length was less or equal to 1 . Simply removing the guard fixed it and it didn't introduce any bug on all my tests. It also fixed the same issue for curve ranges so that's a win win

Screenshots / Videos (if applicable)

Before

Bugge-Curve-Range.mp4

After

Bugged-Curve-Range-Fix.mp4

Checklist

  • Code follows existing style and conventions
  • No unnecessary formatting or unrelated changes
  • Public APIs are documented (if applicable)
  • Unit tests added where applicable and all passing
  • I’m okay with this PR being rejected or requested to change 🙂 (if it introduces any issue else , i don't know why it would get rejected)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant