* ChunkRegionLoader.c() picks an entry in the save queue, removes that entry from the save queue and then actually writes the entry to the region file. So, between the last two steps, the entry is neither in the save queue nor is it in the region file; if somebody loads the chunk again (with ChunkRegionLoader.loadChunk()) in that gap, they'll get old data. I've delayed the removal until the saving is done.
* ChunkRegionLoader.c() also records the coords of the chunks it's currently saving in this.c. ChunkRegionLoader.a(ChunkCoordIntPair, NBTTagCompound), which adds an entry to the save queue, stops the addition of an entry if its coords are in this.c. Now, I'm guessing that Mojang's intended purpose for this mechanism was to prevent multiple parallel writes for the same chunk. The "stops the addition" bit above should then be something like "block until it's no longer in c"; in fact, the vanilla implementation is "discard the new state of the chunk". I've taken the easy route to solving this, by just making ChunkRegionLoader.c() synchronized (since, in normal use, only the chunk saving thread is in here).
* Actually capture all the data of TileEntities. This is done by creating a copy of the TileEntity. The methods of BlockState which currently directly access the TileEntity reference will modify the data of that TileEntity-snapshot instead.
* With the call to BlockState.update, the captured TileEntity data gets applied to the current TileEntity in the world.
* Methods which trigger block specific actions will use the current TileEntity from the world.
* CraftBlockState does not hand out the wrapped or the snapshot TileEntity directly. Instead, it provides an applyTo method to copy the data to a given TileEntity and a method to directly get a copy of the TileEntity NBT data represented by the BlockState. CraftMetaBlockState was updated to make use of that.
* Added #getSnapshotInventory() to bukkit which allows modifiying the captured inventory snapshots of containers.
* Tried to clarify which methods only work if the BlockState is placed, which methods require the block in the world to still be of the same type (methods which trigger actions), and that .getInventory() directly modifies the inventory of the block in the world if the BlockState is placed and becomes invalid if the block type is changed.
Backwards compatibility
* If the BlockState acts as InventoryHolder, getInventory() will still return the inventory directly backed by the TileEntity in the world (like before), and not the snapshot inventory. This compromise should reduce the potential of these changes to break existing plugins, or craftbukkit's own use of BlockState.
* The snapshot's inventory can be accessed by a new method getSnapshotInventory()
* In case the BlockState is not placed (if it was retrieved from the MetaBlockState of an item), the getInventory() method will however return the snapshot inventory. So that when the BlockState gets applied back to the item, the inventory changes are properly included.
* With the changes to CraftMetaBlockState it is no longer required to call the update method before passing a modified BlockState to the CraftMetaBlockState. For backwards compatibility the update method will simply return true for a non-placed BlockState, without actually doing anything.
Impact on plugins
* Restoring blocks now actually works as expected, properly restoring the TileEntity data, reglardless if the block changed its type in the meantime.
* Plugins are now consistently required to call the update method in order to apply changes to blocks. Though, regarding the Javadoc they should have been required to do so anyways.
* New feature: Plugins can take and modify inventory snapshots.
* Breaking change: If a plugin gets the BlockState of a block in the world, modifies the inventory returned by .getInventory(), and then tries to use the same BlockState to apply the TileEntity data to an ItemStack block meta, the ItemStack will use the snapshot inventory, disregarding the changes made to the inventory returned by .getInventory(). This is the compromise of .getInventory() returning the inventory directly backed by the TileEntity in the world.
Other fixes related to BlockState:
* TileEntityContainer#getLocation() will run into a NPE if the TileEntity is non-placed (ex. when getting the BlockState from a CraftMetaBlockState).
* Beacon.getEntitiesInRange() would previously throw a NPE if called for a non-placed BlockState. It was changed to now require to be placed and use the current TileEntity in the world. If the TileEntity in the world is no longer a beacon, it will return an empty list.
* EndGateway now supports setting and getting the exit location even for non-placed EndGateways (inside BlockStateMeta) by using / returning a location with world being null.
This change properly identifies the creative inventory as one with 5 crafting slots (as that's the default set in ContainerPlayer, and handled properly in other containers), instead of having the same inventory twice (which breaks slot identification).
This happened because "joining" wasn't cleared until the player was ticked. Runnables (presumably) ran _after_ the player list packet was sent, but before the player was ticked; thus, the player list packet was sent, but not cleared. The fix is to replace joining with hasSentListPacket, which is set immediately before sending any player list packets (thus, if hidePlayer is called after, it sees that the list packet has been sent and sends a new one to reset it). With this fix, the player is added to the list and then removed shortly afterwards.
The reason why running /hideall in the example wouldn't fix the invisibility is because the server already thinks the player's been removed from the list (as they're hidden), and thus doesn't want to send another hide packet. This is correct behavior assuming that they get hidden correctly the first time, which they now do.
Cross world teleportation works by taking a copy of an entity and moving it to a new world. After this happens the original entity is marked as dead so as to be removed from the original world, however it still undergoes one further tick in the main world, but with some information from the new world. It is not so easy to break out of this tick cycle if needed, so instead we move the portalling process towards the end of an existing tick. This ensures that the entity will not be spuriously ticked.
This is highly useful for profiling vanilla code, and in some cases plugin code. It is somewhat expensive, though, which is why it was initially disabled.
I chose to use a system property instead of a configuration setting because 1) the MethodProfiler is exclusive to CraftBukkit and not part of the general API (the timings system is the general API equivalent), and 2) using a static final boolean property _may_ allow the JITter to optimize out the methods when disabled (though I'm not sure of it).
There are several changes to fix cases where the profiler code was broken slightly by other craftbukkit changes. All of cases have been fixed, except for the block entity ticking one, due to the cost of the getSimpleName call. For that, a ticking entry is used instead, so that time spent actually ticking the block entities can be compared with time processing the list.
This (effectively) reverts 7dde6cc566.
In some situations, a projectile made collide with the entity that shot
it. This occurs because the game sets the ignored entity incorrectly.
Our fix is to ensure that the shooter is the only entity that gets
ignored by the projectile.
Minecraft does double checking for synchronous generation, but since we generate chunks asynchronously we are required to check the first condition also in case the chunk was loaded between ticks. We leave the other logic to be cleaned up by the loops below.
Previously after a player tick, the player's location would be reset back to the location which they had prior to the tick, only to be (hopefully) corrected at a later point, after the client had attempted (and failed) to acknowledge the location change.
World.addEntity(Entity entity) calls addEntity(entity, SpawnReason.DEFAULT), which contains the code that was originally in addEntity (and some event code).
However, WorldServer previously only had addEntity(Entity entity), so if addEntity(Entity entity, SpawnReason spawnreason) was called directly, the UUID check that's found in it (the call to 'i') is skipped. This happens, among other places, in ChunkRegionLoader.spawnEntity (which /summon uses).
I fixed this by making WorldServer override the SpawnReason version, rather than the regular version. This is safe to do because the World version calls the SpawnReason version - it's not necessary to do the same thing in WorldServer.
Clean up implementation and firing of both of these events by routing
both unload and load behaviors to consistent method calls.
This fixes issues where a few places would not call Load or Unload events
when it should have.
Additionally, reduces diff by moving the neighbor marking code into these
consistent points.
Additional benefits of the change include improving the neighbor marking
methods to use getChunkIfLoaded instead of getLoadedChunkAt in some places,
as the latter will cause chunks to be marked active and not unload.
Finally, this also updates CraftWorld.loadChunk to use the new methods, as the
previous logic did not properly handle the new unload queue.