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

Extend ambit by imperceptible amount to account for potential machine precision errors. #767

Closed
wants to merge 0 commits into from

Conversation

garyyo
Copy link
Contributor

@garyyo garyyo commented Oct 2, 2024

Add 0.00000000001 to "ambit radius squared" calculations to account for potential machine precision errors when calculating the ambit at specific angles.

This fixes not being able to move a sentinel by exactly 16 blocks in any direction. Same fix is applied to the circle sentinel calculation and the player's ambit, and might be relevant to a bound cleric circle but I have not included that.

If you want to test the effect in the current codebase you can use the attached hex to do that (I comment the stack state a lot, if you want to remove that they are marked with a ///).
SentinelStepper.txt

It seems to be most likely to occur at shallow angles like pictured
image

@vgskye
Copy link
Contributor

vgskye commented Oct 9, 2024

why not Math.ulp(1.0) (2.22e-16, also known as double's Machine Epsilon), net.minecraft.world.phys.shapes.Shapes.EPSILON (1e-7), com.mojang.math.Constants.EPSILON (1e-6) or net.minecraft.util.Mth.EPSILON (1e-5)?

@garyyo
Copy link
Contributor Author

garyyo commented Oct 9, 2024

I did not know about them (expect ULP, which is too small). The (non-exhaustively) observed error is ~1e-12, so I chose an order of magnitude higher. If you think those are more appropriate I can use one of them, presumably the smallest one that still works is most appropriate (e.g. net.minecraft.world.phys.shapes.Shapes.EPSILON) but I don't know.

@SamsTheNerd
Copy link
Member

Machine epsilon is smallest precision in range [1,2) i think? then ulp just scales with the power of the double, so in the 16-32 squared range it'd be epsilon * (16^2) or so, which seems to still be a few orders of magnitude smaller than the value used here. I don't see any reason that this needs to be precise and we can't just overshoot it a tad, so i'm fine with using this value.

I suppose the other concern would be very very large coordinate values giving an imprecise distance value? that seems harder to reason about though and so far seems to not be an issue, so i'm fine just going with this to solve the immediate issue of not being able to summon a sentinel 16 blocks away.

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

Successfully merging this pull request may close these issues.

3 participants