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.
I should try to compile before I say "this change is okay".
I should try to compile before I say "this change is okay".
I should try to compile before I say "this change is okay".
I should try to compile before I say "this change is okay".
for i in range(100)
This causes the server to generate PrepareItemEnchantEvent even in the
case that an item is already enchanted or otherwise would normally not
be enchantable.
The client resets all formatting after a color code is received, but currently the ANSI codes do not, and so the console does not accurately reflect the appearance of the formatted text. Instead, the ANSI color codes are now set to reset all text attributes.
Currently when dealing with physical interactions with pressure plates
and tripwires we immediately block their activation as soon as a single
entity involved has their event cancelled. We also fire events whenever
an entity intersects the block a wooden button is in even if they aren't
actually pressing it. To correct this we move the button interaction to
the correct place and modify all three to only block the activation if
every entity is blocked from using them instead of just one of them.
CraftServer methods that implement the Server interface will throw an
IllegalArgumentException if a method cannot operate on a null input
and given a null pointer.
This causes methods to fail early and identify that a plugin is
responsible for passing in an invalid argument. This will only
change the exception thrown, if there originally was a thrown
exception. This helps with hunting down legitimate problems
with CraftBukkit.
If the server changes the weather it will set the per-player weather
variable and future changes will not apply. We should only set this
variable when a plugin is requesting per-player weather and not when
the server it doing it.
We used to fall Item.filterData() for this but that method is meant for
converting item data to block data during placement and does the wrong
thing for this case. Instead we just see if the item should have data and
if not set it to zero. We also have to filter wool data explicitly because
clients crash when given invalid wool data.
In Minecraft 1.5 saplings do not grow with a single use of bonemeal anymore.
Our code assumes they will and only takes away bonemeal from the player
when the tree grows successfully (not cancelled by a plugin). Instead we
now always remove a bonemeal even if a plugin is the reason a tree didn't
grow as this matches the vanilla logic more closely.
If a custom TravelAgent is used and returns null for findOrCreate method
a NullPointerException will occur.
Conflicts:
src/main/java/net/minecraft/server/PlayerList.java
Currently, CraftTravelAgent will call s() on the passed-in WorldServer in order to set DEFAULT. However, s() will always return null at this point, because WorldServer.P will still be null, as it is set after the constructor is called. Instead, we set CraftTravelAgent.DEFAULT to the instance that is being constructed.
Recent changes caused PlayerPortalEvent to suddenly return null
unexpectedly and could end up in NPEs resulting that did not before.
This commit addresses that situation by always ensuring a TravelAgent
instance is returned.
The TravelAgent for world 0 is returned arbitrarily in an effort to
compensate for plugins that are implementation dependent and expect some
form of a TravelAgent to be accessible in the event at all times.
Vanilla does not check for blocks in which the player could
suffocate when changing dimension, so portals will happily spawn
players in blocks when using a portal under certain
circumstances. However, we currently check for these instances
and move the player up until they will not suffocate. This means
that players can sometimes be taken to above the target portal,
making it seem as if a portal was not created. Instead, we now
disable this suffocation check when moveToWorld is called from
changeDimension, mirroring vanilla behavior more accurately.
Due to the having to generate new logic to avoid using the customized
PlayerConnection.moveToWorld, entities returning from The End were not
properly calculating their exit target. This commit corrects that
logic.
By having a single function to process BlockPlacement logic, we make
it so that there is consistent behavior throughout all BlockPlace
events. This should allow for easier troubleshooting and less diffs
in source.
This also fixes BUKKIT-3463 by including the correct coordinates that
were clicked to the event.
Also fixes: BUKKIT-3477 and BUKKIT-3488
Minecraft likes to double check that tile entities get set after they
are placed, however we didn't set tile entities until after our event
was called. This caused the world to have multiple tile entities in a
single block location; to fix this we now set tile entities before
the event.
When the skull BlockPlaceEvent was added it was made so the event
would be called after all the data has been set, however this is a
behavior change that is inconsistent with other BlockPlaceEvents.
Instead, if people wish to get the block data they should schedule
a task.
Relates to: BUKKIT-3438
When either of those settings are false, the worlds are not loaded and
therefore will not be targeted for portal exits. Existing worlds are
iterated directly to avoid defaulting to the first world if a direct
dimension match is not found.
Plugins must also specify exit from custom Bukkit worlds to comply with
original commit: https://github.com/Bukkit/CraftBukkit/commit/2dc2af0
This commit introduces a constant to clarify the dependency on the
CraftBukkit implementation of custom worlds having a dimension offset.
By returning the following value (7) we remove the need to special
case pistons in any way (other than the original purpose of this
check, which is to ensure pistons have valid data)
The previous logic was faulty since it lost the logic of "placing" the
block. It was also taking into account data that could have been
changed outside of the processing of this event, which is irrelevant
to the processing of this event.
The javadocs state that a null may be used to remove the currently
playing sound, however this causes a NullPointerException.
It also doesn't process registering the record correctly, along with
processing non-valid items.
By using return 0, we exit the loop prematurely preventing other
creature types from being spawned if one type is set to 0. By using
continue we move on to the other types and allow them to spawn
properly.
Fixes BUKKIT-3408, BUKKIT-3190, BUKKIT-3191, BUKKIT-3407
These changes relate mostly to semantical changes for serialization
contract, exception of changing the map scaling value from byte to boolean,
what it should have been in the first place. Appropriate unit tests were
added for CraftMapMeta, as they were missing.
This makes it so animals (tame or not) will sit properly and not move
around.
Wild animals that are sitting may override the sitting position if
they are attacking.
The 'tag' NBTTagCompound field of the ItemStack assumes that it is OK to
save a reference to an NBT supplied via load() and assumes it is OK to
supply a reference to the internal field during a save(). Neither is true,
as Chunk NBT structures are required to be read-only once created (due to
being written asynchronously off the server thread AND due to the potential
to be passed to a new Chunk if the same chunk is reloaded before the
writing of the NBT is completed by the File I/O thread). Keeping a live
reference to the NBT copy passed in, or to the NBT value passed back
during saving, creates serious thread safety issues which can result in
corrupted data being written to the world data files.
The specific issue here was uncovered by the recent change to use
setName("") on the ItemStack.tag object. When a chunk is being loaded
again before its save is completed, this results in name of the field
in the NBT being set to "". This causes it to be saved as "" instead
of "tag" resulting in it not being properly reloaded in the future which
results in the itemstack losing all of its metadata.
Teleportation should never be processed on dead entities. If you wish
to teleport an entity, do it on a living entity. If you wish to
teleport a player, set their respawn location in PlayerRespawnEvent.
This adds two settings to bukkit.yml, allowing activation and control of
two chunk garbage collection triggering conditions:
chunk-gc/period-in-ticks controls a periodic GC, run once every N ticks
(default is 600); chunk-gc/load-threshold causes the GC to run once
after every N calls to loadChunk() on a given world (this call is an API
call used by plugins, and is distinct from the path taken for routine
player movement-based loading). In both cases, setting to zero will
disable the given GC scheduling strategy.
In either case, the act of doing the GC is simply one of scanning the
loaded chunks, seeing which are NOT being used by one or more players
(due to view-distance) and which are not already queued for unload, and
queueing them for a normal unload. Ultimately, the unload is then
processed the same as if the chunk were unloaded due to leaving the
view-distance range of all players, so the impact on plugins should be
no different (and strategies such as handling the ChunkUnloadEvent in
order to prevent unload will still work).
The initial interval for the periodic GC is randomized on a per-world
basis, in order to avoid all world being GCed at the same time -
minimizing potential lag spikes.
With the persistence api introduced, pets did not have their
persistence flag updated to reflect their persistence. This caused
tame ocelots to not persist under specific conditions.
Slimes and wolves have health that can change based on certain
conditions. So we check if their max health should be updated, and if
it has been customized in any way.
We also scale the wolf's health for their tail
An ItemStack gains the tag name "tag" when the stack is serialized
to NBT, however items don't have a tag *until* they are serialized at
least once. So to solve this, we remove the tag name when loading the
NBT data.
Another problem with NBT are TagLists, when transferring tag lists
between the server and the client the names are lost, and so we
simply don't add a name to the tag.
