When converting things in Minecraft to use wall time instead of ticks I
realized we'd run into integer division rounding issues and could have
updates that end up counting as zero ticks. To compensate for this the
code ensures we always process at least one tick. However, every time we
end up with zero ticks the next time we have an extra tick due to rounding
the other way with the leftovers. This means we are going far too fast and
should not have this at least one tick logic at all.
On top of this some potions rely on the number of ticks they run and not
just the amount of time they last and so potions were put back to running
with ticks entirely.
If a chunk has somehow managed to save with arrays that are not 4096
entries long when reading them again we will get exceptions. Checking the
array length and resizing if needed is cheap so we should do this to help
avoid crashing servers due to this error.
When a chunk is being loaded the server checks to ensure the chunk's idea
of where it is located matches where it was located in the region file. If
these two values do not match the chunk's idea of its position is updated
and the chunk is reloaded. In vanilla minecraft this loading involves the
chunk's tile entities as well. With the change to loading player chunks
asynchronously we split loading tile entities to a separate step that takes
place after this check. Because of this tile entities are loaded with
invalid locations that result in trying to fetch block data from negative
or too large positions in the chunk's internal block storage arrays.
Because loading the tile entities is not thread safe we cannot return to
vanilla behavior here. Instead when we detect a misplaced chunk we just
edit the NBT data for the chunk to relocate the tile entities. This results
in them moving correctly with the chunk without having to actually load
them first.
If a hopper is unable to perform any action during a tick it attempts to do
so every tick from then on. Once it is able to do so it then sets a delay
before attempting to do something again. To avoid excessive CPU usage for
hoppers sitting idle we now apply this delay regardless of the success of
the action.
When using the new feature in 1.5 to drop the item in any highlighted slot,
the anvil result slot does not apply the full anvil calculation that picking
up the item does, including the experience calculation.
Currently furnace smelting and the item pickup delay timer use wall time
(aka actual time passed) to emulate a constant tick rate so run at the
same speed regardless of the server's actual tick rate. There are several
other places this makes sense so this commit converts them.
The item despawn timer is converted so now always takes 5 minutes. Users
know this 5 minute number well so keeping this constant helps to avoid
confusion. This also helps alleviate lag because if a large number of item
drops is the reason your server is running slowly having them stay around
longer just means your server is slow longer.
Potion brewing and the zombie villager conversion timer are now constant.
These match the furnace criteria of being useful for hiding lag and not
having a detrimental effect on gameplay.
Potion effects are now also using wall time. The client is told about effect
times in ticks and displays this information to the user as minutes and
seconds assuming a solid 20 ticks per second. The server does have
code for updating the client with the current time remaining to help
avoid skew due to differing tick rates but making this a constant makes
sense due to this display.
Add a check to avoid doing movement work if an entity doesn't move. This
usually will not ever happen in the current server but is useful when it
does and will be more useful in the future.
Only process mob on mob (non-player) collisions every other tick. Players
tend to pack a lot of mobs into a small space (sheep farm, mob grinder, etc)
so they do a lot of work processing collisions. To help alleviate some of
this we only run these calculations every other tick. This has no visible
effect on the client but can be a huge win on the server depending on
circumstances.
Use generic entity inWater checking for squids. Squids have their own logic
currently for determining if they are in water. This check is almost
identical to the generic entity checking which is run anyway. To avoid
doing duplicate work we just remove the squid version. This does not
have any noticeable effect on gameplay since the checks are so similar.
Use HashSet for tile entities instead of ArrayList. Using an ArrayList for
storing tile entities in a world means we have very expensive inserts and
removes that aren't at the end of the array due to the array copy this
causes. This is most noticeable during chunk unload when a large number of
tile entities are removed from the world at once. Using a HashSet here uses
a little more memory but is O(1) for all operations so removes this
bottleneck.
When a player comes into range of an entity in a vehicle they will often be
sent the spawn packet for the rider before receiving one for the vehicle.
This causes the attachment packet to fail client side because it attempts
to attach the rider to a vehicle that doesn't exist on the client. To
correct this we account for both possible orderings and send the
attachment packet appropriately.
Vanilla also sends an new attach packet every 60 ticks even if the vehicle
has not changed. As this packet is a toggle this resulting in players
teleporting around randomly. Since we handle attachments properly now we
simply revert this section to use the 1.4 logic.
When looking up tile entities for a chunk to send to a player we currently
loop through every tile entity in the world checking if it is within the
bounds of the relevant chunk. Instead of doing this we can just use the
tile entities list stored in the chunk to avoid this costly searching.
As a further optimization, we also modify the generic range-based lookup
to use chunks as well. For most lookups this will give a smaller search
pool which will result in faster lookups.
Thanks to @mikeprimm for the idea and most of the implementation.
In certain scenarios a boat can be killed multiple ways in a single tick.
Due to improper guards this can cause the death code to run multiple times
creating item drops. To correct this we insert the appropriate death check.
Two connection status checks were added to setting a scoreboard for a
player. The first checks to see if a player has logged in yet, which
implicates the ability to receive packets. The second checks to affirm
that the CraftPlayer reference is still to a logged in player; setting
it while not logged in would maintain a stale player reference in the
scoreboard manager.
The method's return value was being incorrectly calculated from the
perspective of this.canHurt(that), where it's actually used from the
this.canBeHurtBy(that) perspective.
The method getTeam gets the team from name of, as opposed to getting the
team a player belongs to.
This also addresses BUKKIT-4002 and partially BUKKIT-4044
When a world is created using our API, it does not use secondary world
server and will maintain a reference to its own scoreboard. In vanilla,
this is not an issue as there is only ever one world.
Similarly to maps, an overwrite to the scoreboard reference has been
added for when another world has been created.
This should also address BUKKIT-3982 and BUKKIT-3985
In commit 7710efc5f9 we corrected the handling of large chests as the
destination for hoppers moving items but did not apply the same fix for
large chests being the source or for droppers. This commit updates these
to have the same fix.
This implementation facilitates the correspondence of the Bukkit Scoreboard
API to the internal minecraft implementation.
When the first scoreboard is loaded, the scoreboard manager will be created.
It uses the newly added WeakCollection for handling plugin scoreboard
references to update the respective objectives. When a scoreboard contains no
more active references, it should be garbage collected.
An active reference can be held by a still registered objective, team, and
transitively a score for a still registered objective. An internal reference
will also be kept if a player's specific scoreboard has been set, and will
remain persistent until that player logs out.
A player's specific scoreboard becomes the scoreboard used when determining
team structure for the player's attacking damage and the player's vision.
This class is designed to be an invisible layer between a normal collection,
and one that silently loses entries because they are only weakly referencable.
Some operations have additional overhead to be semantically correct, but it
maintains the equals contract for all entries, as opposed to identity.
It does not support the equals or hash code method as it cannot easily have
the transitive and commutative properties.
This adds calls to BlockRedstoneEvent for the new daylight sensor and
trapped chest blocks. Note that the redstone level for trapped chests
cannot be modified, as it is based on the number of players currently
viewing the chest's inventory.
When an array of an inventory's contents is requested, we loop through the contents of the NMS inventory's ItemStacks in order to return Bukkit ItemStacks that can be used through the API. However, the NMS ItemStack can, in some cases, be larger than the physical size of the inventory. Using the size of the NMS array as a limit on the loop that follows can result in an ArrayIndexOutOfBoundsException because the Bukkit array's length is the actual size of the inventory, and thus will be smaller.
With this commit we use the smaller of the two arrays' length as the limit in the loop, thus eliminating the possibility that the smaller array will be asked for an index higher than its length.
When a block placement happens we currently update physics on the
attempted placement and update again if the placement is cancelled.
To correct the first one we simply set the block without applying
physics. To correct the second we have to add a new method to
BlockState that lets us update without applying physics and use
this method method when putting the block back.
Without this check, any non-null reference to a plugin is considered
'valid' for registering a task in the scheduler. This is obviously
unintentional behavior and has been changed to throw an
IllegalPluginAccessException. It is now consistent with the
SimplePluginManager event registration contract.
This in affect also addresses BUKKIT-3950 for uninitialized plugin
references (ones without a description).
Large chests work in a different fashion as they are a combination of
two other inventories. This causes their getOwner method to always return
null as their is no correct return. To compensate for this for the hopper
events we special case them to use their CraftBukkit counterpart that has
the information we need for the event.
When a splash potion has no applicable effects we currently do not call
PotionSplashEvent. This means plugins are unable to make custom
potions with reliable splash handling as they have to relicate the
functionality themselves.
With this commit we simply make the event fire regardless of the effects
on the potion.
When cloning an item, all references are copied to the new item. For
collections, this makes internal changes become visible in both the old and
new items.
In CraftMetaItem, clone was not making copies of the appropriate collections
and has been fixed for non-null values.
In CraftMetaEnchantedBook and CraftMetaPotion, clone was using possible empty
collection references and has been changed to explicitly null-check instead.
