search

project-imprimis / libprimis

Libprimis: Imprimis' 3D destroyable world engine
https://project-imprimis.github.io/libprimis/index.html
Other
29 stars 8 forks source link

`RAD` does not equal one radian #197

Open no-lex opened 3 years ago

no-lex commented 3 years ago

The radian is a ratio equal to 180/π degrees or 1/(2π) revolutions. Because (of course) tools.h has to do everything its way, the macro RAD is defined as π/180 which is, of course, totally wrong.

The reason that RAD is inverted is for performance (division is slow and multiplication is fast), but this does not excuse the poor naming of the macro.

b-sharman commented 3 years ago

Does this mean that the value of RAD should be changed, or that the macro name RAD should be changed?

no-lex commented 3 years ago

Yes

no-lex commented 3 years ago

(actually, it's probably better to change the name, on the outside case that the division vs multiplication speed matters)

no-lex commented 3 years ago

On further inspection, there are /RAD events where the RAD constant is the divisor, making it not only counterproductive but also confusing to use the inverse radian for RAD.

Duskhorn commented 2 years ago

I've taken a look at this and the codebase has both angle*RAD and angle/RAD as @no-lex said

Is the division vs multiplication thing really that impactful? If so, whould we make an INV_RAD = 180/π constant and change the codebase accordingly?

If not, the value of RAD should be π/180 imho I could start working on this once I have a clearer way of doing this

Duskhorn commented 2 years ago

@no-lex Actually why isn't this issue closed? lol