EcoCityCraft/Spigot — pull request #21 by mikeprimm (2013-02-28 02:06:46+00:00)
Title: Tile entities in chunk tile entity list can be invalid - make sure to check

The optimization for getTileEntities() correctly improves performance by getting tile entities from the chunks (usually chunk) corresponding to the volume requested, but incorrectly assumes that all the TileEntities in the chunk's list are valid. TEs that are invalidated are (for whatever reason) immediately removed from the world TE list, but not the chunk's TE list - so including all the TEs from the chunk has the possibility of including invalid TEs.

The MinecraftForge folks, as it turns out, had previously implemented this same fix, and found that the isInvalid() check is necessary - encoding already invalid TEs isn't necessarily safe, although problems will be very hard to reproduce since the call is only used for sending full chunks or sending sections of chunks when 64+ updates have occurred (explosions and such). Consequently, the second optimization (just doing addAll on the chunk TE list, versus calling getTileEntitites(), for full chunk encodes) is not ultimately correct/equivalent to normal behavior, and may not be safe. As the true drivers for the change are avoiding traversing the world TE list, and per-chunk TE lists tend to be relatively short, 99+% of the performance benefit of the change will still be achieved.