No entities were lost in this bug, just we were triggering the add entities
before they were loaded due to an inconsistent order of putting chunk into chunkmap.
Any entity that appeared to be gone on the last build will now be back.
This should not ever be used in production!!
This setting is intended for testing so you can try out converting your world
without actually modifying the world files.
This will add some additional overhead to your world, but you're
just testing anyways so that's not a big deal :)
Will store in a folder named after the current version.
PlayerData and Data folders are copied on server start, so there
may be some delay there, but region files are only copied on demand.
This is highly experiemental so backup your world before relying on this to not modify it
1.13 undesirably changed behavior here that chunk load event fired
before the entities were added to the world.
This means any plugin that spawns entities in chunk load event
causes the entities to be registered to the chunk, and then
added to the world twice.
Moves Entity Add to World to be done anytime a chunk is
registered to the Chunk Map, and ignore other calls.
Fixes#1288
* master:
Always process chunk registration after moving
Always move Entity to its new Chunk even if unloaded
If Entity is added to chunk, look up the chunk if current isnt set
Ignore Dead Entities in entityList iteration
Always process chunk removal in removeEntity
Spigot 1.13 checks if any field (which are manually copied from the ItemStack's "tag" NBT tag) on the ItemMeta class of an ItemStack is set.
We could just check if the "tag" NBT tag is empty, albeit that would break some plugins. The only general tag added on 1.13 is "Damage", and we can just check if the "tag" NBT tag contains any other tag that's not "Damage" (https://minecraft.gamepedia.com/Player.dat_format#Item_structure) making the `hasItemStack` method behave as before.
Check the `ItemMetaTest#testTaggedButNotMeta` method to see how this method behaves. (I also added some extra tests).
`hasItemMeta()` will return true if `ItemStack.getDamage() != 0` or it has the `Damage` tag or any other tag is set.
Closes#1222
This will help guarantee that entities are always in the
chunk that they are currently located at.
Should hopefully also fix Citizens triggering the "Saved to wrong chunk" message
Vanilla logic here would allow us to remvoe an entity from
its current chunk, and if it was going to move into an unloaded
chunk, that entity would not be added to the unloaded chunk.
This is bad because this will result in the entity being lost!
In almost all cases, the chunk will be loaded, but in the event
it wasn't, instead of losing the entity, load the chunk to add
the entity to it.
Hopefully will (f)ix #1280...
I'm suspicious that Citizens isn't calling things in the same order and causes the current
chunk to not be set, which then bugs removals. Though this doesn't make any sense to me,
so this likely won't fix it...
But if the isAddedToChunk is true, we really should be returning a chunk anyways if its loaded.
A spigot change delays removal of entities from the entity list.
This causes a change in behavior from Vanilla where getEntities type
methods will return dead entities that they shouldn't otherwise be doing.
This will ensure that dead entities are skipped from iteration since
they shouldn't of been in the list in the first place.
Spigot might skip chunk registration changes in removeEntity
which can keep them in the chunk when they shouldnt be if done
during entity ticking.
Should fix some cases where "Entity is still in another chunk section"
Related to #1223
Vanilla logic checks unload queue and overwrites if its in it.
we're triggering this if a chunk unloads, and reloads immediately in same tick.
Added check for unload queue to not treat as duplicate
Also fixed the config setting not even loading
Should fix#1280
Citizens hijacks entity map, and im guessing under the right conditions
the result might actually be null during entity creation
Pre the cache patch, the id is looked up on save, so it was fine.
Now, if its null and the save ID is requested, we will try to look
it up again and cache it if found.
While upstream has now made this event cancellable, their changes
result in the vechicle being removed before the event is called,
thus leading cancellation to not behave as expected.
See https://github.com/PaperMC/Paper/issues/1223
Should fix Vanilla bugs
Minecraft is saving invalid entities to the chunk files.
Avoid saving bad data, and also make improvements to handle
loading these chunks. Any invalid entity will be instant killed,
so lets avoid adding it to the world...
This lets us be safer about the dupe UUID resolver too, as now
we can ignore instant killed entities and avoid risk of duplicating
an invalid entity.
This should reduce log occurrences of dupe uuid messages.
Also reduce the logging spam overall.
Setting the flag updates the spawner's delay which stops the spawner from trying to find a new spawn position each tick efter the event was cancelled/aborted which makes it usable for mob stackers/mergers and other plugins that don't actually want any mob to spawn in the spawner cycle but keep the overall behaviour close to vanilla.
This might slightly effect existing plugins that use this event but I doubt anyone really relied on this behaviour, the only possible use case that I can think of is cancelling the event until you find a suitable position in your plugin... and this should be handled by the plugin itself by cancelling and spawning at the position manually.
CraftBukkit added synchronization to read and write methods. This adds
much more contention on this object for accessing region files, as
the entire read and write of NBT data is now a blocking operation.
This causes issues when something then simply needs to check if a chunk exists
on the main thread, causing a block...
However, this synchronization was unnecessary, because there is already
enough synchronization done to keep things safe
1) Obtaining a Region File: Those methods are still static synchronized.
Meaning we can safely obtain a Region File concurrently.
2) RegionFile data access: Methods reading and manipulating data from
a region file are also marked synchronized, ensuring that no 2 processes
are reading or writing data at the same time.
3) Checking a region file for chunkExists: getOffset is also synchronized
ensuring that even if a chunk is currently being written, it will be safe.
By removing these synchronizations, we reduce the locking to only
when data is being write or read.
GZIP compression and NBT Buffer creation will no longer be part of the
synchronized context, reducing lock times.
Ultimately: This brings us back to Vanilla, which has had no indication of region file loss.
Closes#1260
