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

Slider: Add validation considering steps #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PJZ9n
Copy link

@PJZ9n PJZ9n commented Jul 8, 2022

No description provided.

@dktapps
Copy link
Member

dktapps commented Jul 9, 2022

This is unreliable because of floating-point errors. There's a very good chance that this check will fail in valid circumstances because of it.

@PJZ9n
Copy link
Author

PJZ9n commented Jul 10, 2022

I think that it is necessary to calculate with the same accuracy as MCPE to solve it accurately, but as an alternative method, can the problem be solved by aligning the values ​​according to the number of digits of the step? If so, I don't think there is a practical problem.

@dktapps
Copy link
Member

dktapps commented Jul 10, 2022

I think that it is necessary to calculate with the same accuracy as MCPE to solve it accurately, but as an alternative method, can the problem be solved by aligning the values ​​according to the number of digits of the step? If so, I don't think there is a practical problem.

No. It's like the old meme about 0.1 + 0.2 != 0.3. MCPE uses int32 precision, the server uses int64. Therefore there will always be discrepancies.

@PJZ9n
Copy link
Author

PJZ9n commented Jul 10, 2022

$step = 0.001;
$value = 6.006000518798828;
$a = round($value, strlen((string)$step));//float(6.006)
$b = $a / $step;//float(6006)
var_dump(ctype_digit((string)$b));//bool(true)

However, this is a successful calculation. Of course there is a possibility of an error, but is this a practical problem?

@PJZ9n PJZ9n mentioned this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants