The PluginManager incorrectly used synchronization on firing any event
that was marked as synchronous.
This synchronized did not even protect any concurrency risk as
handlers were already thread safe in terms of mutations during event
dispatch.
The way it was used, has commonly led to deadlocks on the server,
which results in a hard crash.
This change removes the synchronize and adds some protection around enable/disable
Fixes issue #1177
`MapMaker#weakKeys()` makes the `Map` use identity comparison for the keys, while also enabling the automatical removal of dropped classes from the cache.
The changes are the same as in #1399, except now the original patch is modified instead of a new one being created.
Player Movement, Entity Creation and Teleportation move
entities with a very "You are here, no debate" change, making
the server register them as there, regardless if that chunk was
loaded or not.
It appears possible that with hack clients and lag, a player
may be able to move fast enough to move into an unloaded
chunk and get into a buggy state.
To prevent this, we will ensure a chunk is always loaded,
guaranteeing that the entity will be properly registered
into its new home comfortably.
Closes#1316
1) Don't kick in until server has started (the full crash will still kick in before full start)
2) Delay reporting until 10 seconds, then print every 5
3) Make the intervals configurable
4) Make it able to be disabled by setting every interval to <= 0
Particle packets contain a boolean which marks the particle to either force or show normal to the receiver.
Spigot has been sending all particles with the force boolean which overrides client particle settings.
Related changes in this commit;
- Add a force option to the ParticleBuilder API, which defaults to true to keep spigot consistent with existing api.
- Add a new spawnParticle method to support this mode as a parameter. Of course kept existing api methods the same so as to not break them.
Let me know if changes are needed.
* [CI-SKIP] add .editorconfig for base code style settings
* * Created patch 0349 (fixes#471)
* * Made requested modifications
* * Made requested modifications (x2)
* * Made recommended changes (x3)
* * Moved ConcurrentMap return values to Map as no functions specific to ConcurrentMap were used (backing map is still ConcurrentMap)
* Removed ConcurrentMap import
I misinterpreted some code as a risk of entity loss, but now
after deeper study, I see how that code was used more and why
it was adding entities to chunks that they shouldn't have been
in during a world transfer process.
also ensure we never process already valid entities. this shouldnt be possible as of recent
commits as we made the entity slice array safer, but doesn't hurt for this logic to be safe too
incase that patch got dropped in a future version by accident/necessarily
1) Chunk Registration might kill an entity, don't add it to the world if it did!
2) By default, entities are added to the world per slice iteration.
This opens risk of the slices being manipulated during chunk add if an
EntityAddToWorldEvent spawns an entity into this chunk.
Fix this by differing entity add to world for all entities at the same time
3) If a duplicate entity is attempted to add to the world of an entity, and
the original entity is dead, overwrite it as the logic does for unloaod queued entities.
Should hopefully finish up issues with #1223
After witnessing behavior of the regeneration logs, its clear that Vanilla
has had bugs with saving duplicate entities for a while....
Some entities are saved in multiple chunks, and now we are bringing those duplicates
out that use to never surface.
This mode will analyze if the entity appears to be a duplicate (near the other dupe uuid)
and delete the entity instead.
This should reduce regenerations to entities that are nowhere near each other, and
therefore more likely to be subject to real UUID collisions due to our
previous bug, and therefor should survive the chunk load.
Due to the changes in 1.13, clients will send a tab completion request
for all bukkit commands in order to factor in the lack of support for
brigadier and provide backwards support in the API.
Craftbukkit, however; has moved the chat spam limiter to also interact
with the tab completion request, which while good for avoiding abuse,
causes 1.13 clients to easilly be kicked from a server in bukkit due
to this. Removing the spam limit could cause issues for servers, however,
there is no way for servers to manipulate this without blindly cancelling
kick events, which only causes additional complications. This also causes
issues in that the tab spam limit and chat share the same field but different
limits, meaning that a player having typed a long command may be kicked from
the server.
Splitting the field up and making it configurable allows for server owners
to take the burden of this into their own hand without having to rely on
plugins doing unsafe things.
This patch has been applied to 1.12.2 in order to allow people using
plugins which allow clients of newer versions to connect, this is
not a common practice, however is being done as a level of nicety
given the current status of 1.13
We have a result boolean for this already, and this
method was meant to be "Try from cache, if that fails, look it up"
So NPE'ing there just wasn't correct.
ideally this should of never mattered, as it will only
be hit if you teleport out of an unloaded chunk...
But apparently some people are triggering this.
See #1223
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.
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.
Aikar
Max Lee
Gergely Sarkozi
Shane Freeder
cakoyo
Christopher White
Zach Brown
Mystiflow
searchndstroy
kashike
chickeneer
prplz
egg82
miclebrick
Mark Vainomaa