Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store angles internally as radians #2976

Closed
3 tasks done
L0laapk3 opened this issue May 5, 2024 · 3 comments · Fixed by #2978
Closed
3 tasks done

Store angles internally as radians #2976

L0laapk3 opened this issue May 5, 2024 · 3 comments · Fixed by #2978

Comments

@L0laapk3
Copy link

L0laapk3 commented May 5, 2024

Prerequisite Checklist

Describe your feature request here

It has been mildly bothering me that angles are internally stored as degrees instead of radians.

It makes more sense to store them internally in radians, since thats what hardware is optimized for. Rather than having to convert them to radians all the time, only to have to convert them back immediately after.

Especially now that in 3.0 angles are unit-aware with sf::Angle, I don't see any reason to use degrees anymore.

Its a fairly trivial change and I'd be happy to PR it myself, but I suspect this is the kind of thing that needs to be discussed first ;)

Use Cases

transform.rotate(vector.angle())

API Example

No response

@vittorioromeo
Copy link
Member

I've done the work so we can discuss pros/cons:
#2978

@ChrisThrasher
Copy link
Member

I'm open to this idea if it means fewer lossy conversions between degrees and radians. The fact that the standard library exclusively uses radians compels me to prefer radians. I can't imagine users noticing or caring what underlying representation we choose.

@kimci86
Copy link
Contributor

kimci86 commented May 6, 2024

I can't imagine users noticing or caring what underlying representation we choose.

Well, no need to imagine, this issue is a proof 😄

On a more serious note, I agree that this is a good idea.

@eXpl0it3r eXpl0it3r added this to the 3.0 milestone May 14, 2024
@eXpl0it3r eXpl0it3r linked a pull request May 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants