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

Add math.round; fix vector.round #10803

Merged
merged 2 commits into from Apr 1, 2021
Merged

Add math.round; fix vector.round #10803

merged 2 commits into from Apr 1, 2021

Conversation

v-rob
Copy link
Member

@v-rob v-rob commented Jan 14, 2021

Adds math.round to round to the nearest integer (simple, but useful), rounding away from zero at multiples of 0.5, as well as making vector.round follow this behaviour instead of rounding multiples of 0.5 towards +inf.

Fixes #6774

@v-rob v-rob added @ Script API Feature ✨ PRs that add or enhance a feature labels Jan 14, 2021
@Desour
Copy link
Member

Desour commented Jan 15, 2021

  • Imo, it should be documented whether x.5 is rounded towards 0, +inf, -inf or +-inf. Not only for these functions, but also eg. for get_node.
  • I wouldn't call this a "fix" of vector.round. Some mods might rely on the current behaviour.
  • Imho rounding x.5 towards +-inf is not objectively better than towards +inf. But math.floor(x+0.5) is a little faster.
  • Your implementation of math.round rounds -0.5 to math.ceil(-0.5-0.5)=-1, which is "incorrect" according to you.

@v-rob
Copy link
Member Author

v-rob commented Jan 15, 2021

  • Imho rounding x.5 towards +-inf is not objectively better than towards +inf. But math.floor(x+0.5) is a little faster.

Technically, there are multiple ways of rounding a number. However, in practice, I have only ever learned or heard anyone use one rule: to round 4.569 to an integer, look at the first decimal point. If it is <=5, round up. If it is >5, round down. That rule makes no exception for negative numbers. If I'm incorrect and some people use other rules in daily life, please correct me.

Different programming languages handle this differently. C and C++ round away from zero (like this PR), Python rounds towards zero (the opposite of this PR), JavaScript rounds towards positive infinity (like vector.round), etc.

I don't care particularly much; I just chose this implementation because I thought it was the one most people were familiar with. I just thought it would be nice to have a rounding function built in. (I mean, we have vector.round but not math.round? That's kind of ridiculous). I'd rather just get some sane rounding function in without arguing about the most superior rounding rule.

  • Your implementation of math.round rounds -0.5 to math.ceil(-0.5-0.5)=-1, which is "incorrect" according to you.

That's a typo on the PR description; I meant to say that vector.round rounds -0.5 to 0. Fixed.

@ghost
Copy link

ghost commented Jan 28, 2021

I use round a lot. Necessary IMO.

@rubenwardy
Copy link
Member

rubenwardy commented Jan 29, 2021

Following C++'s example makes sense, especially if you want to round node positions

For reference:

Input -0.5 -1.5 -2.5 -3.5 Note
C++ -1 -2 -3 -4 Rounds from 0
Python 0 -2 -2 -4 WTF?
JavaScript 0 -1 -2 -3 Rounds to inf
Lua (+0.5) 0 -1 -2 -3 Rounds to inf
Lua (PR) -1 -2 -3 -4 Rounds from 0

I suggest not following Python's example

@v-rob
Copy link
Member Author

v-rob commented Jan 30, 2021

Added docs saying it rounds away from zero at 0.5.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new vector.round type function that matches engine rounding
5 participants