godotengine
Update Camera2D::set_zoom() function error message
Updated it so that the error condition is accurate to what the error description describes, and the editor won't be flooded with errors if you have a non-zero low zoom level.
There may be a good reason as to why the approximation was used. The comment above gives a good hint about it. Tread carefully
zoom can't be 0, because of zoom_scale, which is 1 / zoom. The change should be safe, but is camera even usable at such small zooms?
I doubt there's any good reason for the zoom to be smaller than 0.00001, and the affine inverse will fail with a determinant of zero which could happen with very tiny but non-zero values which aren't representable, so I don't see any reason to risk it here, it's not expensive to check this
If the is_zero_approx() function is completely necessary and shouldn't be changed then I would argue that the error message itself should be updated from "Zoom level must be different from 0 (can be negative)." to something like "It is not recommmended for zoom level to closely approach zero (although it can be negative)." as I do not appreciate the lack of consistency/accuracy between the error condition and it's message (why settle for less than accurate error messages?).
as I do not appreciate the lack of consistency/accuracy between the error condition and it's message (why settle for less than accurate error messages?).
I'd say that's unnecessarily nitpicky, I'd just leave this alone, but Mickeon's suggestion would be appropriate
It's... a bit nitpicky, yes. Especially given how basic the original message actually is.
In my defense this was my first github pull request and I don't know the exact format for these things; at the same time it was a basic messsage for a pretty basic and self explanatory commit. I guess I could've improved it by researching more about the implications of my code and figuring out whether my change was even necessary, but I'm not that skilled in matrices or computer graphics enough to understand affine inversion so I thought I would leave it up to the reviewers. Though I still contend that the exact wording of the error message is important because it leaves no room for approximation at all and seems very unpolished in that respect (it makes the error message seem like an error itself for people who have a value that is very obviously not zero yet still triggers the error.).