Conversation
| std::uint32_t active_tile_y) { | ||
| int active_tile_z, | ||
| int active_tile_x, | ||
| int active_tile_y) { |
There was a problem hiding this comment.
@mapsam one small note: since std::int32_t is == to int, I prefer the former since it is more explicit. I don't have a citation handy for this being a best practice but I think I've picked it up from @joto. Commenting here just in case it is helpful to know that you could have done std::uint32_t -> std::int32_t. As far as discussing if there is substantial difference between that and int I'd propose bringing that up in slack sometime.
| return utils::CallbackError("'z' value in 'tiles' array item is not a number", callback); | ||
| } | ||
| std::int64_t z = z_val->IntegerValue(); | ||
| int z = z_val->Int32Value(); |
There was a problem hiding this comment.
What happens now if someone passes the equivalent of std::numeric_limits<std::int64_t>::max() (aka 9223372036854775807) in from JS land? What will Int32Value() return? Do we have a test for this? /cc @mapsam
There was a problem hiding this comment.
Good question - @artemp have you covered this in vtcomposite?
There was a problem hiding this comment.
@mapsam @springmeyer - we need to change IsNumber() to IsInt32() to avoid undefined behaviour
|
Thanks @springmeyer for the review! All good points - I may have rushed this one unnecessarily |
3 participants
Avoids casting, and cleans up the z/x/y values to be more readable. #88
cc @artemp @millzpaugh