MC-2025 /r/mojira post

Title:  Final and proper fix to MC-2025, posted to Mojira

**MC-2025 is dead, or it should be.  Someone named Kademlia figured out the correct solution TWO YEARS AGO.  About a year ago, I figured out the underlying cause (IEEE floating point rounding artifacts).  Then we spent the the next year arguing about better solutions.  The one Kademlia suggested is the correct one, and it should have been implemented that way a long time ago.**

*The Mojira entry is over-spammed, so this reddit post is to serve as the only place for further discussion.  However, as far as I am concerned, the discussion should be over.*

I have added strikethrough to my recent work on MC-2025 that addresses the rounding error problem directly so that entity position and bounding box can be losslessly inter-converted.  That solution is possibly intersting but unnecessarily complex and pointless for anything other than understanding IEEE floating point rounding effects.  I was comfortable tinkering with alternate solutions because I'd heard rumors that MC-2025 had already been fixed.  But we can't find any evidence of a fix from decompiling 1.13-pre1, so it's time for us to get practical here.

Along with plenty of other people, I, Xcom, and MrGrim (Michael Kreitzer), and Kademlia came up with basically the same solution independently.  Xcom's has been in Carpet Mod for ages, and MrGrim has been using it in his own custom JAR for probably about as long.  We should all be really embarrassed that Kademlia figured it out about two years ago, well before any of the rest of us, but we basically ignored it and argued.  (Links to comments at the end of this post.)

Here is what people have to understand about this bug.  There is no and can never be perfect stability in axis-aligned bounding box (AABB) dimensions.  The exact size of the AABB depends on where in the world the entity is located, due to limits of floating point precision.  To begin with, the typical mob width is 0.6, which is (a) not representable in floating point, and (b) stored as a 32-bit float, so when converted to double-precision, you get something more like 0.6000000238418579.  However, it's not particularly important that it's this weird number.  I demonstrated in a comment on my last reddit post that even something that you **can** store exactly in floating point (e.g. 0.59375) is just as vulnerable to rounding drift.  I had developed a method for computing center coordinates and AABB faces so that they are losslessly inter-convertible, but the AABB dimensions end up varying from 0.6000000238418579 depending on the world coordinates, so you **basically never get an AABB of that size in the first place.**

The underlying cause of MC-2025 is that **sometimes** an AABB ends up **slightly smaller** than the desired width.  If this happens before the entity is pushed up against a boundary (i.e. blocks or walls), then upon chunk save and reload, the AABB size will be recomputed such that it is intersecting the wall, allowing the entity to be pushed into the wall, suffocate, and die.  You can see my Mojira comment and previous /r/mojira post for what the numbers looks like.  Although the rounding artifacts get larger at larger world coordinates, the drift we see is miniscule.  Closer to the world origin, we have seen error on the order of 2^(-46).  Compare this to the fact that (due to MC-4), entity coordinates send to clients are quantized to multiples of 1/4096 (2^(-12)).

But, OMG, do the rounding errors mean that AABB's accumulate shrinkage over time?  **Actually, no.**  The statistics on IEEE rounding do not have that kind of bias.  What has not been stated is that the AABB is just as likely to end up **larger** than the expected width; on save and reload, the entity ends up slightly away from the wall, and we don't notice any problem.  In reality, the AABB size ends up undergoing random wobble around the expected value **all the time**, and that wobble isn't functionally any different from the kind we'd get even if we tried to force the AABB size to be stable!

This reasoning leads us to one clear conclusion:  **The simplest, least invasive, and most correct solution is to just store the AABB in NBT data on chunk save and restore the AABB exactly as it was saved upon reload.**  The solution is as follows.

Based on MCP symbols for 1.12.2, the following methods on Entity need to be modified:

* `public NBTTagCompound writeToNBT(NBTTagCompound compound)`
* `public void readFromNBT(NBTTagCompound compound)`

# Saving the AABB on chunk unload

In `writeToNBT`, somewhere within the `try` block, do the following:

    AxisAlignedBB aabb = this.getEntityBoundingBox();
    compound.setTag("AABB", this.newDoubleNBTList(
        aabb.minX, aabb.minY, aabb.minZ, aabb.maxX, aabb.maxY, aabb.maxZ));

# Restoring the AABB after chunk reload

There are two places in `readFromNBT` that call `setPosition`, which resets the AABB based on entity width.  It is **very important** that this new code be inserted **after** those calls.  This needs to be the **very last thing** inside of the `try` block.

So right **after** this:

    if (this.shouldSetPosAfterLoading())
    {
        this.setPosition(this.posX, this.posY, this.posZ);
    }

Insert this code:

    if (compound.hasKey("AABB")) {
		NBTTagList aabb = compound.getTagList("AABB", 6);
        this.setEntityBoundingBox(new AxisAlignedBB(aabb.getDoubleAt(0), aabb.getDoubleAt(1),
        								            aabb.getDoubleAt(2), aabb.getDoubleAt(3),
        								            aabb.getDoubleAt(4), aabb.getDoubleAt(5)));
    }


- [Kademlia's Mojira comment with the original correct solution](https://bugs.mojang.com/browse/MC-2025?focusedCommentId=317274&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-317274)
- [My Mojira comment that explains the underlying cause](https://bugs.mojang.com/browse/MC-2025?focusedCommentId=408078&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-408078)