Aikar
- aikar
- aikar
- America/New_York
- Yes
- No
MC-4024![[Mojang] Jeb (Jens Bergensten)](../../../data/assets/gravatar/jeb.jpeg){kind=small}
![[Mojang] Grum (Erik Broes)](../../../data/assets/gravatar/grum.png){kind=small}
Normal![[Mojang] Georgii Gavrichev](../../../data/avatars/users/22428.png){kind=small}
clones
This happens from time to time when playing SMP with a friend on my local minecraft server.
The chest seems to remain on the open state (regarding the animation) and closes once I rightclick it to view its inventory, so basically it's inverted. The issue doesn't seem to resolve itself after relogging, so it must be a server-side issue.
It usually happens after playing for a while, after frequently using a chest. Have been able to reproduce this for a few times, so I'll provide a screenshot whenever it happens again.
Cloning to reopen as this is not fixed in 1.4.5
----------------
Better Description
----------------
If a user manages to right click a chest 2 or more times before the server has registered it and sent the user the Container Open packet, the user chest the server thinks the user has open will be the first click, where as the client will think its the second click.This causes the server to ignore everything the user does with that chest, so any items pulled out will stay in the chest, and any items put in will stay in players inventory.
As previously reported, the chest animations get into an inverted state.
This is 100% reproduceable by simply disabling your internet connection while on SMP, click the chest 2+ times, reenable internet.
The server will catch up to your packets once your internet reconnects and trigger the issue.
For players on slower network connections, this happens accidently often, causing lots of confusion on why their items are not where they put them.
Confirmed in 1.4.5 Vanilla and Craftbukkit. This issue has existed since 1.3.1
----------------
Original Description
----------------This happens from time to time when playing SMP with a friend on my local minecraft server.
The chest seems to remain on the open state (regarding the animation) and closes once I rightclick it to view its inventory, so basically it's inverted. The issue doesn't seem to resolve itself after relogging, so it must be a server-side issue.
It usually happens after playing for a while, after frequently using a chest. Have been able to reproduce this for a few times, so I'll provide a screenshot whenever it happens again.
Windows 7 Ultimate 32-bit, Java 1.7.0_09
n/a
is cloned by
is duplicated by
CLONE -Chest glitch - Server/Client desync
Cloning to reopen as this is not fixed in 1.4.5
----------------
Better Description
----------------
If a user manages to right click a chest 2 or more times before the server has registered it and sent the user the Container Open packet, the user chest the server thinks the user has open will be the first click, where as the client will think its the second click.This causes the server to ignore everything the user does with that chest, so any items pulled out will stay in the chest, and any items put in will stay in players inventory.
As previously reported, the chest animations get into an inverted state.
This is 100% reproduceable by simply disabling your internet connection while on SMP, click the chest 2+ times, reenable internet.
The server will catch up to your packets once your internet reconnects and trigger the issue.
For players on slower network connections, this happens accidently often, causing lots of confusion on why their items are not where they put them.
Confirmed in 1.4.5 Vanilla and Craftbukkit. This issue has existed since 1.3.1
----------------
Original Description
----------------This happens from time to time when playing SMP with a friend on my local minecraft server.
The chest seems to remain on the open state (regarding the animation) and closes once I rightclick it to view its inventory, so basically it's inverted. The issue doesn't seem to resolve itself after relogging, so it must be a server-side issue.
It usually happens after playing for a while, after frequently using a chest. Have been able to reproduce this for a few times, so I'll provide a screenshot whenever it happens again.
Cloning to reopen as this is not fixed in 1.4.5, as the animations can still show this same behavior as reported in this ticket, but there is more damages than the animations alone that I feel previous reporters likely missed and didn't understand the 2 were related.
----------------
Better Description
----------------
If a user manages to right click a chest 2 or more times before the server has registered it and sent the user the Container Open packet, the user chest the server thinks the user has open will be the first click, where as the client will think its the second click.This causes the server to ignore everything the user does with that chest, so any items pulled out will stay in the chest, and any items put in will stay in players inventory.
As previously reported, the chest animations get into an inverted state.
This is 100% reproduceable by simply disabling your internet connection while on SMP, click the chest 2+ times, reenable internet.
The server will catch up to your packets once your internet reconnects and trigger the issue.
For players on slower network connections, this happens accidently often, causing lots of confusion on why their items are not where they put them.
Confirmed in 1.4.5 Vanilla and Craftbukkit. This issue has existed since 1.3.1
----------------
Original Description
----------------This happens from time to time when playing SMP with a friend on my local minecraft server.
The chest seems to remain on the open state (regarding the animation) and closes once I rightclick it to view its inventory, so basically it's inverted. The issue doesn't seem to resolve itself after relogging, so it must be a server-side issue.
It usually happens after playing for a while, after frequently using a chest. Have been able to reproduce this for a few times, so I'll provide a screenshot whenever it happens again.
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
duplicates
@kumasasa
Dupe bugs ARE compromising to a server and should be private...
Sign rendering has been limited in 1.8 based on the resource pack itself how it renders the text.
That is fine and all, but it also impacts simply setting up the sign.
For example, a user with a custom resource pack using larger fonts can't write as many characters on a sign line than another resource pack can.
Truncating the message on sign viewing is accepted, but the client should not be as strict on sign creation, as a sign created on one resource pack may
tover flow but be perfectly visible on another.Even more so, users using the vanilla resource pack are reporting being unable to write their character name on a sign due to this.
name: Merek_Shadowmere
Suggestion: Client should allow writing up to 32 characters, and then let viewing the sign enforce the rendering based calculations.
Then activities that read the sign data can at least see full name, even if client can not.
I run a decent sized Minecraft server that tries to not restrict players as much as possible.
My players love to build redstone contraptions, including item sorters and movers, resulting in LOTS of Hoppers, sometimes hitting 40,000+ Tile Entities, or 2000 Hoppers in a single area.
This has drastic impact to server performance causing every players experience to be degraded by loss of TPS.
After EXTENSIVE research using a profiler and studying the code, I've identified numerous issues with the hopper system.
- Hoppers by proper design, call the .update() method on an inventory when it has been modified in some way, to trigger physics and comparators.
However, many inventory manipulation methods also call .update()
This results in .update being called many times, which then triggers comparators and other physics events. This single handedly crushes servers with hoppers.
There is also a literal duplicate .update() call in the method that calls the .update() after flag = true. Only 1 is needed.- Unnecessary ItemStack cloning
The server calls itemstack.clone() on EVERY item stack it 'trys'. Take TileEntityHopper.a(IHopper) for example.
If an inventory of say 50 items in a chest is trying to be moved to a hopper, and that hopper only has 1 free slot and nothing matches the chests inventory, this method will result in every one of those item stacks being cloned every tick, only for it to fail to be moved and now an instance of the itemstack will be GC'd- Inefficient Item Suck In behavior
Hoppers will out-number ItemStacks in any matured world, and has potential to be loaded longer term than an item stack. It does not make sense that hoppers perform AABB lookups for Entities.Solutions:
First I will reference my patch on how I quick patched in solutions, though not always perfect:
https://bitbucket.org/starlis/empirecraft/src/05f5bb0a3bb9576430e97ea6ef32a9785e8e6f6f/patches/craftbukkit/0097-Fix-Minecraft-Fix-Hoppers.patch?fileviewer=file-view-default
- For cloning issue, I suggest using itemstack count manipulation like I am doing in my patch. This avoids calls to .update() from split stack methods which partially resolves issue #1 also
- For the .update call, if switching off using the manipulation methods, some of the .update() calls are now handled, but in TileEntityHopper.c(IInventory, ItemStack, int, EnumDirection) method where the actual inventory manipulation takes place, iinventory.setItem() where itemstack1 is null can also trigger .update() on chests. adding some method to setWithoutUpdate instead on the interface is possibly the most ideal instead of my hack method.
- For Hopper Suck In, Instead, ItemStacks should instead look for hoppers to ping them to suck them in, which can do much more efficient Tile Entity BlockPosition lookups. But would still need an Entity AABB for the Minecarts, but the overall occurrence would drastically be reduced.
I run a decent sized Minecraft server that tries to not restrict players as much as possible.
My players love to build redstone contraptions, including item sorters and movers, resulting in LOTS of Hoppers, sometimes hitting 40,000+ Tile Entities, or 2000 Hoppers in a single area.
This has drastic impact to server performance causing every players experience to be degraded by loss of TPS.
After EXTENSIVE research using a profiler and studying the code, I've identified numerous issues with the hopper system.
- Hoppers by proper design, call the .update() method on an inventory when it has been modified in some way, to trigger physics and comparators.
However, many inventory manipulation methods also call .update()
This results in .update being called many times, which then triggers comparators and other physics events. This single handedly crushes servers with hoppers.
There is also a literal duplicate .update() call in the method that calls the .update() after flag = true. Only 1 is needed.- Unnecessary ItemStack cloning
The server calls itemstack.clone() on EVERY item stack it 'trys'. Take TileEntityHopper.a(IHopper) for example.
If an inventory of say 50 items in a chest is trying to be moved to a hopper, and that hopper only has 1 free slot and nothing matches the chests inventory, this method will result in every one of those item stacks being cloned every tick, only for it to fail to be moved and now an instance of the itemstack will be GC'd- Inefficient Item Suck In behavior
Hoppers will out-number ItemStacks in any matured world, and has potential to be loaded longer term than an item stack. It does not make sense that hoppers perform AABB lookups for Entities.Solutions:
First I will reference my patch on how I quick patched in solutions, though not always perfect:
https://bitbucket.org/starlis/empirecraft/src/05f5bb0a3bb9576430e97ea6ef32a9785e8e6f6f/patches/craftbukkit/0097-Fix-Minecraft-Fix-Hoppers.patch?fileviewer=file-view-default
- For cloning issue, I suggest using itemstack count manipulation like I am doing in my patch. This avoids calls to .update() from split stack methods which partially resolves issue #1 also
- For the .update call, if switching off using the manipulation methods, some of the .update() calls are now handled, but in TileEntityHopper.c(IInventory, ItemStack, int, EnumDirection) method where the actual inventory manipulation takes place, iinventory.setItem() where itemstack1 is null can also trigger .update() on chests.
adding some method to setWithoutUpdate instead on the interface is possibly the most ideal instead of my hack method.For Hopper Suck In, Instead, ItemStacks should instead look for hoppers to ping them to suck them in, which can do much more efficient Tile Entity BlockPosition lookups. But would still need an Entity AABB for the Minecarts, but the overall occurrence would drastically be reduced.I run a decent sized Minecraft server that tries to not restrict players as much as possible.
My players love to build redstone contraptions, including item sorters and movers, resulting in LOTS of Hoppers, sometimes hitting 40,000+ Tile Entities, or 2000 Hoppers in a single area.
This has drastic impact to server performance causing every players experience to be degraded by loss of TPS.
After EXTENSIVE research using a profiler and studying the code, I've identified numerous issues with the hopper system.
- Hoppers by proper design, call the .update() method on an inventory when it has been modified in some way, to trigger physics and comparators.
However, many inventory manipulation methods also call .update()
This results in .update being called many times, which then triggers comparators and other physics events. This single handedly crushes servers with hoppers.
There is also a literal duplicate .update() call in the method that calls the .update() after flag = true. Only 1 is needed.- Unnecessary ItemStack cloning
The server calls itemstack.clone() on EVERY item stack it 'trys'. Take TileEntityHopper.a(IHopper) for example.
If an inventory of say 50 items in a chest is trying to be moved to a hopper, and that hopper only has 1 free slot and nothing matches the chests inventory, this method will result in every one of those item stacks being cloned every tick, only for it to fail to be moved and now an instance of the itemstack will be GC'd- Inefficient Item Suck In behavior
Hoppers will out-number ItemStacks in any matured world, and has potential to be loaded longer term than an item stack. It does not make sense that hoppers perform AABB lookups for Entities.
As of 1.13-1.15, this now also does 2 lookups per hopper due to the advanced hitbox. This needs to be a simple lookup again.
Also, no reason to suck in if there is a full solid occluding block above the hopper.- Usage of streams hurts performance due to overhead, complex call stacks. JVM does better at optimizing non stream code, and Java 8 is not receiving stream optimizations.
Solutions:
First I will reference my patch on how I quick patched in solutions, though not always perfect - note there are some non vanilla matters here but will lay out the vanilla relevant elements:
https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch
- For cloning issue, I suggest using itemstack count manipulation like I am doing in my patch. This avoids calls to .update() from split stack methods which partially resolves issue #1 also. CraftBukkit made the cloning problem even worse, but Vanilla does pre clone too, so the concept is valid in Vanilla
- For the .update call, if switching off using the manipulation methods, some of the .update() calls are now handled, but in TileEntityHopper.c(IInventory, ItemStack, int, EnumDirection) method where the actual inventory manipulation takes place, iinventory.setItem() where itemstack1 is null can also trigger .update() on chests. adding some method to setWithoutUpdate instead on the interface is possibly the most ideal instead of my hack method.
- For Hopper Suck In, Instead, ItemStacks could instead look for hoppers to ping them to suck them in, which can do much more efficient Tile Entity BlockPosition lookups. But would still need an Entity AABB for the Minecarts, but the overall occurrence would drastically be reduced.
- Hoppers are able to engage the automatic loot filling on a chest. However no reason to run that check multiple times, just do it on first index only
- Hoppers in 1.13+ (or at least 1.15) are now sucking in twice, once for each part of the complex shape of a Hopper. Bowl level pixel perfect bounding boxes are unnecessary. Restore the single AABB lookup from 1.12
- Removal of all usages of Streams. hoppers are hot. Streams have overhead that added up quite a lot and also seems to hurt JVM optimizations. Since MC runs on java 8, which is not receiving performance improvements to Streams/JIT, this hurts. This includes isEmpty() in Lootable.
- Skip item suck in behavior if the block above the hopper is full cuboid solid at all. Items collide with blocks and this scenario should never naturally occur where an item phases into a solid block and then gets sucked in. ( https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch#L453 )
- Since .getItem() on some inventories (Lootable types) have additional overhead, keep result from first call and pass it to dependent methods
I run a decent sized Minecraft server that tries to not restrict players as much as possible.
My players love to build redstone contraptions, including item sorters and movers, resulting in LOTS of Hoppers, sometimes hitting 40,000+ Tile Entities, or 2000 Hoppers in a single area.
This has drastic impact to server performance causing every players experience to be degraded by loss of TPS.
After EXTENSIVE research using a profiler and studying the code, I've identified numerous issues with the hopper system.
- Hoppers by proper design, call the .update() method on an inventory when it has been modified in some way, to trigger physics and comparators.
However, many inventory manipulation methods also call .update()
This results in .update being called many times, which then triggers comparators and other physics events. This single handedly crushes servers with hoppers.
There is also a literal duplicate .update() call in the method that calls the .update() after flag = true. Only 1 is needed.- Unnecessary ItemStack cloning
The server calls itemstack.clone() on EVERY item stack it 'trys'. Take TileEntityHopper.a(IHopper) for example.
If an inventory of say 50 items in a chest is trying to be moved to a hopper, and that hopper only has 1 free slot and nothing matches the chests inventory, this method will result in every one of those item stacks being cloned every tick, only for it to fail to be moved and now an instance of the itemstack will be GC'd- Inefficient Item Suck In behavior
Hoppers will out-number ItemStacks in any matured world, and has potential to be loaded longer term than an item stack. It does not make sense that hoppers perform AABB lookups for Entities.
As of 1.13-1.15, this now also does 2 lookups per hopper due to the advanced hitbox. This needs to be a simple lookup again.
Also, no reason to suck in if there is a full solid occluding block above the hopper.- Usage of streams hurts performance due to overhead, complex call stacks. JVM does better at optimizing non stream code, and Java 8 is not receiving stream optimizations.
Solutions:
First I will reference my patch on how I quick patched in solutions, though not always perfect - note there are some non vanilla matters here but will lay out the vanilla relevant elements:
https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch
- For cloning issue, I suggest using itemstack count manipulation like I am doing in my patch. This avoids calls to .update() from split stack methods which partially resolves issue #1 also. CraftBukkit made the cloning problem even worse, but Vanilla does pre clone too, so the concept is valid in Vanilla
- For the .update call, if switching off using the manipulation methods, some of the .update() calls are now handled, but in TileEntityHopper.c(IInventory, ItemStack, int, EnumDirection) method where the actual inventory manipulation takes place, iinventory.setItem() where itemstack1 is null can also trigger .update() on chests. adding some method to setWithoutUpdate instead on the interface is possibly the most ideal instead of my hack method.
- For Hopper Suck In, Instead, ItemStacks could instead look for hoppers to ping them to suck them in, which can do much more efficient Tile Entity BlockPosition lookups. But would still need an Entity AABB for the Minecarts, but the overall occurrence would drastically be reduced.
- Hoppers are able to engage the automatic loot filling on a chest. However no reason to run that check multiple times, just do it on first index only
- Hoppers in 1.13+ (or at least 1.15) are now sucking in twice, once for each part of the complex shape of a Hopper. Bowl level pixel perfect bounding boxes are unnecessary. Restore the single AABB lookup from 1.12
- Removal of all usages of Streams. hoppers are hot. Streams have overhead that added up quite a lot and also seems to hurt JVM optimizations. Since MC runs on java 8, which is not receiving performance improvements to Streams/JIT, this hurts. This includes isEmpty() in Lootable.
- Skip item suck in behavior if the block above the hopper is full cuboid solid at all. Items collide with blocks and this scenario should never naturally occur where an item phases into a solid block and then gets sucked in. ( https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch#L453 )
- Since .getItem() on some inventories (Lootable types) have additional overhead, keep result from first call and pass it to dependent methods
I run a decent sized Minecraft server that tries to not restrict players as much as possible.
My players love to build redstone contraptions, including item sorters and movers, resulting in LOTS of Hoppers, sometimes hitting 40,000+ Tile Entities, or 2000 Hoppers in a single area.
This has drastic impact to server performance causing every players experience to be degraded by loss of TPS.
After EXTENSIVE research using a profiler and studying the code, I've identified numerous issues with the hopper system.
- Hoppers by proper design, call the .update() method on an inventory when it has been modified in some way, to trigger physics and comparators.
However, many inventory manipulation methods also call .update()
This results in .update being called many times, which then triggers comparators and other physics events. This single handedly crushes servers with hoppers.- Unnecessary ItemStack cloning
The server calls itemstack.clone() on EVERY item stack it 'trys'. Take TileEntityHopper.a(IHopper) for example.
If an inventory of say 50 items in a chest is trying to be moved to a hopper, and that hopper only has 1 free slot and nothing matches the chests inventory, this method will result in every one of those item stacks being cloned every tick, only for it to fail to be moved and now an instance of the itemstack will be GC'd- Inefficient Item Suck In behavior
Hoppers will out-number ItemStacks in any matured world, and has potential to be loaded longer term than an item stack. It does not make sense that hoppers perform AABB lookups for Entities.
As of 1.13-1.15, this now also does 2 lookups per hopper due to the advanced hitbox. This needs to be a simple lookup again.
Also, no reason to suck in if there is a full solid occluding block above the hopper.- Usage of streams hurts performance due to overhead, complex call stacks. JVM does better at optimizing non stream code, and Java 8 is not receiving stream optimizations.
Solutions:
First I will reference my patch on how I quick patched in solutions, though not always perfect - note there are some non vanilla matters here but will lay out the vanilla relevant elements:
https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch
- For cloning issue, I suggest using itemstack count manipulation like I am doing in my patch. This avoids calls to .update() from split stack methods which partially resolves issue #1 also. CraftBukkit made the cloning problem even worse, but Vanilla does pre clone too, so the concept is valid in Vanilla
- For the .update call, if switching off using the manipulation methods, some of the .update() calls are now handled, but in TileEntityHopper.c(IInventory, ItemStack, int, EnumDirection) method where the actual inventory manipulation takes place, iinventory.setItem() where itemstack1 is null can also trigger .update() on chests. adding some method to setWithoutUpdate instead on the interface is possibly the most ideal instead of my hack method.
- For Hopper Suck In, Instead, ItemStacks could instead look for hoppers to ping them to suck them in, which can do much more efficient Tile Entity BlockPosition lookups. But would still need an Entity AABB for the Minecarts, but the overall occurrence would drastically be reduced.
- Hoppers are able to engage the automatic loot filling on a chest. However no reason to run that check multiple times, just do it on first index only
- Hoppers in 1.13+ (or at least 1.15) are now sucking in twice, once for each part of the complex shape of a Hopper. Bowl level pixel perfect bounding boxes are unnecessary. Restore the single AABB lookup from 1.12
- Removal of all usages of Streams. hoppers are hot. Streams have overhead that added up quite a lot and also seems to hurt JVM optimizations. Since MC runs on java 8, which is not receiving performance improvements to Streams/JIT, this hurts. This includes isEmpty() in Lootable.
- Skip item suck in behavior if the block above the hopper is full cuboid solid at all. Items collide with blocks and this scenario should never naturally occur where an item phases into a solid block and then gets sucked in. ( https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch#L453 )
- Since .getItem() on some inventories (Lootable types) have additional overhead, keep result from first call and pass it to dependent methods
A feature long used by Minecraft maps and servers is to add the "ench" tag to an item in order to make it glow.
Pre 1.11, one could put an empty enchant list on the item to make it glow.
Now in 1.11, the client requires the list is not empty. This can be worked around by actually giving an item the enchant and then use the HideEnchants flag.
However, the client still will not render any Block Entity itemstack with the glow.
This relates to (in MCP naming)
RenderItem#renderItem(ItemStack, IBakedModel);if (model.isBuiltInRenderer()) {}
This rendering path does not process the same effects applying process.
This is devastating to servers that have built up custom items with lore and glowing effects to mark them as special.
These items now do not stand out against their normal counterparts.
The client should keep the same behavior previous versions have and render any item that is enchanted with the glow effect (and preferably not require the list to be non empty, to make it simpler to send the effect)
Stick: - WORKS
/give @a minecraft:stick 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}Chest: - DOESNT WORK
/give @a minecraft:chest 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}
A feature long used by Minecraft maps and servers is to add the "ench" tag to an item in order to make it glow.
Pre 1.11, one could put an empty enchant list on the item to make it glow.
Now in 1.11, the client requires the list is not empty. This can be worked around by actually giving an item the enchant and then use the HideEnchants flag.
However, the client still will not render any Block Entity itemstack with the glow.
This relates to (in MCP naming)
RenderItem#renderItem(ItemStack, IBakedModel);if (model.isBuiltInRenderer()) {}
This rendering path does not process the same effects applying process.
This is devastating to servers that have built up custom items with lore and glowing effects to mark them as special.
These items now do not stand out against their normal counterparts.
The client should keep the same behavior previous versions have and render any item that
is enchanted with the glow effect (and preferablynot require the list to be non empty,tomake it simpler to send the effect)Stick: - WORK
S/give @a minecraft:stick 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}Chest: - DOESNT WORK
/give @a minecraft:chest 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}A feature long used by Minecraft maps and servers is to add the "ench" tag to an item in order to make it glow.
Pre 1.11, one could put an empty enchant list on the item to make it glow.
Now in 1.11, the client requires the list is not empty. This can be worked around by actually giving an item the enchant and then use the HideEnchants flag.
However, the client still will not render any Block Entity itemstack with the glow.
This relates to (in MCP naming)
RenderItem#renderItem(ItemStack, IBakedModel);if (model.isBuiltInRenderer()) {}
This rendering path does not process the same effects applying process.
This is devastating to servers that have built up custom items with lore and glowing effects to mark them as special.
These items now do not stand out against their normal counterparts.
The client should keep the same behavior previous versions have and render any item that has the ench tag not require the list to be non empty, so that itemstacks created expecting the previous behavior still function the same.
It would be good if Tile Entities also were able to receive the effect (was not applying in past either, but requesting it be fixed)
Stick: - WORKS
/give @a minecraft:stick 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}Stick: - USE TO WORK
/give @a minecraft:stick 1 0 {ench:[]}Chest: - DOESNT WORK
/give @a minecraft:chest 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}
Experience orbs operate on integer values, and players can use vanilla commands to spawn an exp orb with an exact value.
Spawning an orb at 100k is valid, however if that orb is unloaded, the NBT data saves it as a short, corrupting the value.
Fixing is as trivial as changing the NBT set/get calls to Int for Value instead of Short
Experience orbs operate on integer values, and players can use vanilla commands to spawn an exp orb with an exact value.
Spawning an orb at 100k is valid, however if that orb is unloaded, the NBT data saves it as a short, corrupting the value.
Fixing is as trivial as changing the NBT set/get calls to Int for Value instead of Short
[Developer Report]
ThreadedAnvilChunkStorage overuses synchronization, resulting in massive performance drops to the server.
Because most of the methods are synchronized, the File IO Thread writing a chunk out is done under a synchronized context, blocking the entire class.
The main thread then will hit this class to load chunks, save chunks, check if chunks exists, etc, and will be blocked while any asynchronous chunk saving is occurring.
This totally defeats the entire purpose of having asynchronous IO, and drastically impacts performance.
Please see my patch (licensed MIT, or WTFPL if that makes it even easier legally) for my improvements to the Chunk Save process that has worked fine for us for years: https://github.com/PaperMC/Paper/blob/
e2c75e81f7580440f1b6c1a02a13a2f61b9c67ec/Spigot-Server-Patches/0062-Chunk-save-queue-improvements.patch[Developer Report]
ThreadedAnvilChunkStorage overuses synchronization, resulting in massive performance drops to the server.
Because most of the methods are synchronized, the File IO Thread writing a chunk out is done under a synchronized context, blocking the entire class.
The main thread then will hit this class to load chunks, save chunks, check if chunks exists, etc, and will be blocked while any asynchronous chunk saving is occurring.
This totally defeats the entire purpose of having asynchronous IO, and drastically impacts performance.
Please see my patch (licensed MIT, or WTFPL if that makes it even easier legally) for my improvements to the Chunk Save process that has worked fine for us for years: https://github.com/PaperMC/Paper/blob/ffd9c779233fb6e5bb428d865e87c4f3d46607e0/Spigot-Server-Patches/0062-Chunk-save-queue-improvements.patch
[Developer Report]Currently, the server saves all "pending"/"generating" chunks to the region during the last method in the chunk gen scheduler, during auto save and unload all chunks phases.
These chunks appear to be pretty meaningless to save, as the loadChunk method ignores them due to invalid chunk status, and retriggers a new generation to begin with.
Additionally, chunks saved like this then breaks .chunkExists() method, making it look like they exist when they don't
The server should just not even save these chunks, reducing load on the server, and keeping .chunkExists() to logically represent achunkis able to be loaded.
Thisalso causes issues if Mojang was to ever go the next step and add Async Chunk IO Loading as the modded platforms have directly to the game.Saving of current progress of generating chunks appears to be a pretty wasteful process.
This causes a lot more activity to occur on the main thread, for data that becomes irrelevant very shortly after.All saving of proto chunks even helps with is resuming a chunk that was cancelled mid generation at server shutdown.
That's a pretty extreme use case, when you could simply not save them, and allow the generation to start over the next request.
I really recommend removing this, so only Level staged proto chunks are saved.
This gets rid of the entire method at the chunk scheduler that processes these, saving a ton of work.
Server saves pendingchunks[performance] Server saves proto chunks
Going to use spigots mapping names as not sure on each of those for Mojang mappingsIn
analyzing heap memory usage, it wasnoticed thatlong[] was a serious contender...In tracing it back, it was found that BitSet's used for Carving Masks were the heaviest user, stored as a reference from ProtoChunk, which was linked to the ChunkHolder.
The ChunkHolder is holding a ref from the status future cache.
This is due to the fact that during Chunk Saving, the carving masks are saved regardless of chunk status, so saving creates the 65k BitSet's and saves them, even at status's like StructureStart which we have TONS of....
Then when that ProtoChunk is loaded again, those 65k bitsets are loaded into memory and kept around in the cache even when that chunk isn't promoting to FULL.
Fix is pretty simple, don't write CarvingMasks unless they've actually been generated when saving.
for (int l = 0; l < k; ++l) {
WorldGenStage.Features worldgenstage_features = aworldgenstage_features[l]; // Paper start - don't create carving mask bitsets if not even at that chunk status yet BitSet mask = protochunk.getCarvingMaskIfSet(worldgenstage_features); if (mask != null) {
nbttagcompound3.setByteArray(worldgenstage_features.toString(), mask.toByteArray()); }
// Paper end}In analyzing heap memory usage, it was noticed that long[] was a serious contender...
In tracing it back, it was found that BitSet's used for Carving Masks were the heaviest user, stored as a reference from ProtoChunk, which was linked to the ChunkHolder.
The ChunkHolder is holding a ref from the status future cache.
This is due to the fact that during Chunk Saving, the carving masks are saved regardless of chunk status, so saving creates the 65k BitSet's and saves them, even at status's like StructureStart which we have TONS of....
Then when that ProtoChunk is loaded again, those 65k bitsets are loaded into memory and kept around in the cache even when that chunk isn't promoting to FULL.
Fix is pretty simple, don't write CarvingMasks unless they've actually been generated when saving.
for (int l = 0; l < k; ++l) {
WorldGenStage.Features worldgenstage_features = aworldgenstage_features[l]; // Paper start - don't create carving mask bitsets if not even at that chunk status yet BitSet mask = protochunk.getCarvingMaskIfSet(worldgenstage_features); if (mask != null) {
nbttagcompound3.setByteArray(worldgenstage_features.toString(), mask.toByteArray()); }
// Paper end}
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)
Jigsaw World Gen Features is triggering codec errors like the following:
{axis:"y"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat
e_provider:{state:{Properties:,Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}]
{axis:"y"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat
e_provider:{state:{Properties:,Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}]
{type:"minecraft:simple_block_placer"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64
,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:,state_provider:
{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]:
Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76
Spigot named this file odd, as I don't think it directly relates to generating Pillager Outposts,
Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));
Unsure about data corruption, but can be spammy.
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)
Jigsaw World Gen Features is triggering codec errors like the following:
{axis:"y"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat
e_provider:{state:{Properties:,Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}]
{axis:"y"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat
e_provider:{state:{Properties:,Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}]
{type:"minecraft:simple_block_placer"}
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64
,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:,state_provider:
{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]:
Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76
Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));
Unsure about data corruption, but can be spammy.
The bug
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)Jigsaw World Gen Features is triggering codec errors like the following:
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64 ,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:{type:"minecraft:simple_block_placer"},state_provider:{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));Unsure about data corruption, but can be spammy.
Reproduction steps
- Create a world in 1.15.2 and teleport to a village; or use the attached world MC-197883.zip
- Open the world in the latest version
Multiple errors are logged
Root Cause
The Root Cause is that there are items in the FEATURE registry that are specifying "name" as their identifier instead of "type", and these are mixed into the same collection.
This code in Spigot mappings is the source point:
public class WorldGenFeatureConfigured<FC extends WorldGenFeatureConfiguration, F extends WorldGenerator<FC>> implements IDecoratable<WorldGenFeatureConfigured<?, ?>> { public static final Codec<WorldGenFeatureConfigured<?, ?>> a = IRegistry.FEATURE.dispatch((worldgenfeatureconfigured) -> { return worldgenfeatureconfigured.e; }, WorldGenerator::a); public static final Codec<Supplier<WorldGenFeatureConfigured<?, ?>>> b = RegistryFileCodec.a(IRegistry.au, WorldGenFeatureConfigured.a); public static final Codec<List<Supplier<WorldGenFeatureConfigured<?, ?>>>> c = RegistryFileCodec.b(IRegistry.au, WorldGenFeatureConfigured.a); public static final Logger LOGGER = LogManager.getLogger();
Changing this to .dispatch("name", (....
Fixes the errors for the ones listed in the ticket, however then breaks every thing using type.
The data sets need to be updated to consistently use type.
The bug
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)Jigsaw World Gen Features is triggering codec errors like the following:
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64 ,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:{type:"minecraft:simple_block_placer"},state_provider:{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));Unsure about data corruption, but can be spammy.
Reproduction steps
- Create a world in 1.15.2 and teleport to a village; or use the attached world MC-197883.zip
- Open the world in the latest version
Root Cause
The Root Cause is that there are items in the FEATURE registry that are specifying "name" as their identifier instead of "type", and these are mixed into the same collection.
This code in Spigot mappings is the source point:
public class WorldGenFeatureConfigured<FC extends WorldGenFeatureConfiguration, F extends WorldGenerator<FC>> implements IDecoratable<WorldGenFeatureConfigured<?, ?>> { public static final Codec<WorldGenFeatureConfigured<?, ?>> a = IRegistry.FEATURE.dispatch((worldgenfeatureconfigured) -> { return worldgenfeatureconfigured.e; }, WorldGenerator::a); public static final Codec<Supplier<WorldGenFeatureConfigured<?, ?>>> b = RegistryFileCodec.a(IRegistry.au, WorldGenFeatureConfigured.a); public static final Codec<List<Supplier<WorldGenFeatureConfigured<?, ?>>>> c = RegistryFileCodec.b(IRegistry.au, WorldGenFeatureConfigured.a); public static final Logger LOGGER = LogManager.getLogger();
Changing this to .dispatch("name", (....
Fixes the errors for the ones listed in the ticket, however then breaks every thing using type.
The data sets need to be updated to consistently use type.
The bug
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)Jigsaw World Gen Features is triggering codec errors like the following:
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64 ,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:{type:"minecraft:simple_block_placer"},state_provider:{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));Unsure about data corruption, but can be spammy.
Reproduction steps
- Create a world in 1.15.2 and teleport to a village; or use the attached world MC-197883.zip
- Open the world in the latest version
Root Cause
The Root Cause is that there are items in the FEATURE registry that are specifying "name" as their identifier instead of "type", and these are mixed into the same collection.
This code in Spigot mappings is the source point:
public class WorldGenFeatureConfigured<FC extends WorldGenFeatureConfiguration, F extends WorldGenerator<FC>> implements IDecoratable<WorldGenFeatureConfigured<?, ?>> { public static final Codec<WorldGenFeatureConfigured<?, ?>> a = IRegistry.FEATURE.dispatch((worldgenfeatureconfigured) -> { return worldgenfeatureconfigured.e; }, WorldGenerator::a); public static final Codec<Supplier<WorldGenFeatureConfigured<?, ?>>> b = RegistryFileCodec.a(IRegistry.au, WorldGenFeatureConfigured.a); public static final Codec<List<Supplier<WorldGenFeatureConfigured<?, ?>>>> c = RegistryFileCodec.b(IRegistry.au, WorldGenFeatureConfigured.a); public static final Logger LOGGER = LogManager.getLogger();
Changing this to .dispatch("name", (....
Fixes the errors for the ones listed in the ticket, however then breaks every thing using type.
The data sets need to be updated to consistently use type.
The bug
Similar to
MC-191388but a new issue (this is a not a duplicate or a "wasn't fixed correctly" - this is a new issue)Jigsaw World Gen Features is triggering codec errors like the following:
[19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:block_pile",config:{stat e_provider:{state:{Properties:{axis:"y"},Name:"minecraft:hay_block"},type:"minecraft:simple_state_provider"}}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[{name:"minecraft:flower",config:{tries:64 ,yspread:3,xspread:7,need_water:0b,zspread:7,blacklist:[L;],project:1b,block_placer:{type:"minecraft:simple_block_placer"},state_provider:{type:"minecraft:plain_flower_provider"},whitelist:[L;],can_replace:0b}}] [19:34:04] [Server thread/ERROR]: Input does not contain a key [type]: MapLike[\{name:"minecraft:no_op",config:{}}]Using Spigot/Paper mappings, the callstack of where this is triggered is
https://gist.github.com/aikar/56b0c2beff6d8c70e18893a72889eb76Code triggering it is this:
this.b = new BlockPosition(nbttagcompound.getInt("PosX"), nbttagcompound.getInt("PosY"), nbttagcompound.getInt("PosZ")); this.e = nbttagcompound.getInt("ground_level_delta"); DataResult dataresult = WorldGenFeatureDefinedStructurePoolStructure.e.parse(DynamicOpsNBT.a, nbttagcompound.getCompound("pool_element"));Unsure about data corruption, but can be spammy.
Reproduction steps
- Create a world in 1.15.2 and teleport to a village; or use the attached world MC-197883.zip
- Open the world in the latest version
Root Cause
The Root Cause is that there are items in the FEATURE registry that are specifying "name" as their identifier instead of "type", and these are mixed into the same collection.
This code in Spigot mappings is the source point:
public class WorldGenFeatureConfigured<FC extends WorldGenFeatureConfiguration, F extends WorldGenerator<FC>> implements IDecoratable<WorldGenFeatureConfigured<?, ?>> { public static final Codec<WorldGenFeatureConfigured<?, ?>> a = IRegistry.FEATURE.dispatch((worldgenfeatureconfigured) -> { return worldgenfeatureconfigured.e; }, WorldGenerator::a); public static final Codec<Supplier<WorldGenFeatureConfigured<?, ?>>> b = RegistryFileCodec.a(IRegistry.au, WorldGenFeatureConfigured.a); public static final Codec<List<Supplier<WorldGenFeatureConfigured<?, ?>>>> c = RegistryFileCodec.b(IRegistry.au, WorldGenFeatureConfigured.a); public static final Logger LOGGER = LogManager.getLogger();
Changing this to .dispatch("name", (....
Fixes the errors for the ones listed in the ticket, however then breaks every thing using type.
The data sets need to be updated to consistently use type.
The bug
When you get a player head, it's supposed to retain the skin that you had when it was created (see this comment by [Mojang] Nathan Adams in MC-52806). However it fails to retain the skin if there are multiple heads for the same player loaded into the world.
Therefore, a single player can only have one player head. Every head linked to their name that they find afterwards will continue using that skin until they exit the game, regardless of what skin the head should display.
This is a visual / caching bug only, the skin texture is correctly saved in the NBT data.
To reproduce
- Create a world.
- Obtain your player head ("Head A") with
/give @p player_head{SkullOwner:<your player name>} - Place Head A in the world
- Exit the game and change your skin.
- Re-enter the world and give yourself another player head using the same command. This head is Head B. You may notice that Head B has the old skin.
- Place the Head B in the world
- Exit the game, reopen, and re-enter the world
- Both Head A and Head B will now use the same skin. Depending on which skull is loaded first (this seems to be related to coordinates), they'll either have your old or your new skin.
- If both heads have your new skin, destroy Head B, remove it from your inventory, and relog once more. Then, Head A will once again display your old skin correctly.
This bug also results in issues between worlds:
- Perform steps 1-4 above.
- Create a new world, create and place a head. Think of this as Skull C.
- Now, if you load the first world with Skull A, it will display the skin of Skull C.
- If you close and reopen the game and load Skull A's world before loading the Skull C's world, then Skull C will display the skin of Skull A.
I should note more clearly what is happening with the in-between worlds issue. The skull that loads first will be the only one displayed on your player skulls until you fully exit the game. Once you re enter the game after fully closing it, the first of your player skulls that you load next will then be the only one.
Current Workaround
Using the /data command, you can change the Name or UUID of the owner to be something different than your (or any other player's) Name/UUID and also be destinct between all the player heads you want to use. This will trick the game into the thinking the head belongs to another player who doesn't yet have a player head so that it properly updates to the skin that it was assigned.
Cuase
According to Aikar on MC-68487, the cache for skins uses GameProfile.equals for comparison, which only checks for name and UUID. As such, distinct names or UUIDs allow separate skins, but the same name and UUID means different skins cannot be used.
![[Mod] Torabi](../../../data/assets/gravatar/torabi.png){kind=small}
Actually, us developers will look at code, see 2 things that appear to do the same thing, but miss the slight difference in the 2, or worse, don't clearly see what happens deeper in the flow.
So we then say "let's simplify things! Delete the extra". We test, and everything works... So it seems like a good idea. Deleting code usually is a good thing.
If you have two functions that do mostly the same thing, then your code is poorly structured, and it's likely that changes or fixes that should be applied to both will only be applied to one, causing bugs. Mojang may not have considered the performance impact of consolidating the two functions, and may not have consolidated them correctly, but consolidating them was still a good action to take. Maybe an additional argument should have been added, or maybe the unload queue shouldn't be managed within that function at all.
sativaphil, I didn't say that this wasn't a high-priority issue, only that it wasn't the only one. Everyone always thinks their favorite issue is the most important, and that the developers should drop everything else to work on it, but it's clearly impossible for them all to be right. In this case, I would argue that the official Minecraft server software is generally not sufficient for servers with a large number of players, and that most of them run alternative server software, such as the Paper server Aikar has mentioned.
Aikar- Stick to managing what's in/out with SpigotMC and also to what you "know" vs "think". As that reads "I think" vs "i know", as until you can substantiate the opinion provided with provable / tested code that illustrates the ROI failures in this equation, then really, it's about offering alternative options to just "bug fixes" whilst highlighting ways a refactor or deferred by design can occur. Fact is there's a cloud of CPU's available to hand off workable tasks to clients, even if its discrete calculations that are obfuscated.
If anything why SpigotMC hasn't done an out of band patch to this issue (thus reducing constant bottle necks currently been seen) probably summarises the peak abilities being on offer here.
Aikar well three things:
Firstly, see what happens when you think outside the narrowminded box - https://www.youtube.com/watch?v=JjSKqQI6Cr0&feature=youtu.be - you end up with moments like these.
Secondly, in 2008 had the same assnine response to how HD video can't work because of the roundtrip latency involved in clients negotiating between server<->client on how much CPU, GPU and Bandwidth can be consumed at intervals that reduces load on server/cloud infrastructure (and to prevent buffering)... oh...you now call this HD video online as a result of the team i was in via Microsoft (FYI we got it working to over 280million PC's during the beijing olympics and it was extremely rare you saw buffering symbols). Furthermore in SCADA style solutions where mission critical response times need to occur (think support operator staring at a screen looking for breakdowns) a 500ms-3 second response time for a visual catchup is undetected unless there's a deliberate interrupt in the users experience. So if an industrial grade emergency response solution like these can live with the existence of a 500ms - 3 in cognitive load to reactionary response... we can as well (assuming your limited theory on such matter were to come true, which it actuall wouldn't but this would involve an education process on threading, network latency compression etc etc)
The lifespan of a users behaviour at ahead-of-time chunk loading is as follows. The client makes the prediction ahead of time, generating chunks according to the intended seed/behaviour (and you can obfuscate the seed or encrypt that data better as well - but since MCP exists any client-side code is still open to hacking but there's more ways around that as well). While that client generates the behaviour client-side, it then sends back its findings BEFORE AND/OR AFTER the visual queue has been rendered or during such procedure.
All the server is doing is polling clients for verification of the truth, in that the clients return back the index associated within the Chunk with its findings, it then acts in the same capacity as Rendering nodes do in 3D rendering pipelines (you won't find this in your web developer textbooks btw, so may need to upgrade your Amazon account). The reality is the consequences vs likelihood of a client being without trust is actually lower than one would think, but just in case the server continues to apply verification protocols to ensure that the truth is always held to higher standard, in the event the truth fails and culprits are found it also can increase banning behaviour management whilst at the same time the server can then re-generate the suspicious chunk anyway only during non-peak-processing times.
That is to say, using crowd sourcing approaches you can furthermore generate chunks ahead of time while CPU's are idle, that is the client can tack onto the existing chatty packets that go between client/server that there are in a semi-idle state or what we called "refractory processing time", During this periods the server can predict players movements based on basic square root math(s) and historical data "User is x:1000,y:128, z:1000" therefore it can assume that another 1000 block radius is likely to be rendered in the next N amount of ticks, it can then ask spare CPU cycles to generate such information / data even if the user doesn't action that 1000 block radius. Keeping in mind its really a 1x time call per server per chunk ratio.
Lastly,
If i wanted a Wordpress blog update or fix, i'd respect your attempt at asserting authority, in the mean time, you play with your JavaScript/PHP toys and leave the hard thinking to the rest of us who look beyond what blind ignorant obedience has to offer.
As to how it relates to the topic at head.. simple... the chunk creation queue needs refactoring period. As all this does is put a band-aid upon a broken limb.
Aikar I will check your patch on Monday, but without a reproducible case of massive lag, I can't say if the problem is solved or not.
The bug
Trying to rename 2 items or more will not work in survival.
Steps to reproduce
- Make sure you're in survival mode
- Place an anvil
- Get two or more items
- Put the items in the anvil and rename them
- Try to take out the new items
Additional information by Aikar in this comment.
I upgrade from 133 1.16.1 too 16.2 148 and get MapLike errors.
So the fix from Aikar fix only some of the issues.
You can still get MapLike errors.
@benjamin I have removed your attachment form this report because it seems to be unrelated. However, it is quite interesting nonetheless that Watchdog shuts down the server while it is in BlockableEventLoop.waitForTasks() (indicating that it is idle?). Feel free to create a separate report (ideally also including reproduction steps) about that issue.
@Aikar, I think it would be best to create a separate issue for that (especially if it only appears after applying your fix in Paper); also please make sure that this is indeed a vanilla Minecraft issue and not an issue with the fix you implemented for Paper.
The first logged errors you are reporting appear to be indeed exactly MC-197883.
The second log file (https://pastebin.com/shetqD5e) contains different errors, however since this is caused by a modified server (Paper is not a vanilla server) this won't be accepted here unless someone (ideally Aikar) can explain based on the unmodified Minecraft code that this is indeed a problem caused by Minecraft and not the changes introduced by Paper.
this issue is killing our server... Fireworks also seem to be able to "bundle up" in some cases as I loaded a chunk with 3000 fireworks that did not want to detonate either, killing the client in lag.
and to make it clear-- water isnt required to reproduce this.
Having a firework dispenser launcher firing and moving away so the fireworks enter unloaded chunks does the same.
Something about the server stops sending updates about the firework to the client, and the client keeps the firework and it continues to propel farther than it was intended to.
I have found a solution to part of this bug (server side)
in World.tickEntities:
The fireworks are being skipped for ticks if they are less than 32 blocks from an unloaded chunk. The server then never detonates the firework, causing the client to shoot it into unrenderable areas.
The client should still be fixed to not lag out when a firework hits unrenderable areas.
My commit to CB here: https://github.com/aikar/EMC-CraftBukkit/commit/1c3abba02a76b08f38bfd03c095345652327924e
Lewis, book issue was a craftbukkit issue specifically which is now fixed.
I made this change to my Anvils to alleviate this bug: https://github.com/aikar/EMC-CraftBukkit/commit/244a1bb98b27e78135381101726876dd22cc7d9a
I did, this is not a duplicate. Please reopen and raise the priority of this.
This issue has nothing to do with that issue. This is a server issue, that is a client issue.
This needs to be fixed for 1.5 so please open and get it pushed to proper people ASAP.
Also, I have found another issue in the code.
The boolean logic for the "is an item frame and once every 10 ticks" is broken.
The update counter is never incremented, so this code will run EVERY tick, not one in ten.
The code needs to look something like this:
Id also recommend lowering the default itemframe tracking range so that so many people arent tracking them so far away.... at distances they cant even see the things really.
I contributed to the Spigot project with this fix, and multiple server owners are now running this slightly modified (to only send the map once) with great results.
Previously this issue was tearing servers apart.
It needs to be fixed ASAP before its wide spread to the public that you can take down servers and waste massive amounts of bandwidth with very little effort.
@kumasasa
Dupe bugs ARE compromising to a server and should be private...
couldnt triggering physics on chunk load essentially fix this?
Yes I just looked at 1.5.1 client and server code, still has the problem.
public String getItemDisplayName(ItemStack par1ItemStack)
{ return ("" + StringTranslate.getInstance().translateNamedKey(this.getLocalizedName(par1ItemStack))).trim(); }It's using getItemDisplayName to fill in the input box for the anvil window, but when user is using non english language, this text will differ from the server and will be considered a rename.
simple steps to reproduce:
Switch to pirate language
put redstone lamp in anvil, do not type a name
youll notice it lists the item immediately as processable by the anvil. this is due to rename.
if you switch to english, item will not be processible.
client simply needs to check english name != current typed name before sending MC|ItemName custom channel in GuiRepair:sendSlotContents
I added some debug code that prints the name of the host sent from the client upon connection using the BungeeCord software:
@EventHandler
{ System.out.println(event.getConnection().getVirtualHost().getHostString()); }public void onLogin(final LoginEvent event)
This shows the trailing .
However connecting directly without the SRV record entry, shows no .
18:08:40 [INFO] play.emc.gs
18:08:40 [INFO] [Aikar] <-> ServerConnector [smp3] has connected
18:08:44 [INFO] play.emc.gs.
18:08:44 [INFO] [archer530] <-> ServerConnector [smp8] has connected
This was not a duplicate of that issue, but this is likely improved slightly in 1.7.
Pictures showing 2 different frames
Well it is the same overall issue, but in this case I'm desiring to keep the invisible effect.
Maybe add a new flag to hide entity on client and only show the name?
Sonic, thats the point. Asking to reverse that statement.
If we just accept everything Mojang says, and never ask for it to be the way we would prefer it, how would the game be made better for us?
Don't just accept a bad answer as the final word, push for it to be fixed
The issue lies in that the server is calculating DEEP_BIOME based on the 1.8 World Generator and NOT the worlds ACTUAL biome from previous versions.
I had a report of a biome in OCEAN (Not Deep), and when I generated a 1.8 world with exact same seed, sure enough that location was DEEP_OCEAN in that seed.
The Monument Structure check uses 2 different Biome lookup methods, and the one that does DEEP_BIOME check uses the World Gen calculations, and then the followup check for "Surrounding biomes" uses the actual worlds data.
I temp fixed for my server with the following change:
This issue has more flaws than this report states as it also causes monuments to spawn in the middle of Rivers, Frozen Rivers and Frozen Oceans, which is quite odd to stroll through the mountains and find a monument.
With this being marked as fixed, can there least be a gamerule to keep this behavior? I was happy to see the name tags and thought it was intentional.
for a non PvP Server, would love to keep name tags showing.
Mustek that is related but not duplicate.
That issue is about sign rendering, and is for the current intent "working as intended". This ticket is asking about sign creation, not sign rendering.
[Mod] Neko as mentioned in the ticket, that comment from [Mojang] Searge (Michael Stoyke) is about sign RENDERING, which is what my original ticket was closed as duplicate of.
Fully understand that rendering limited by resource pack is "Working as designed". But its also affecting sign creation in a way that even a vanilla client with vanilla resource pack can't write a player's name on the sign when it should be able to.
Client should let you write the name, and then clients that have a resource pack that can fit all 16 characters can see it, and those that can't, can't.
The limit being enforced both directions makes it difficult to create signs.
Forcing a user to use a custom resource pack just to create a sign is silly.
reaverofdarknes1 the fix only applies to newly generated chunks. Any created before 1.7, and visited during 1.8 with the bug will forever be marked as a water temple.
It just stops NEW cases of the bug.
A new world would not have this bug to begin with. It applies to pre 1.8 chunks being visited on 1.8+ worldgen.
Can be fixed by simply adding a isDead check to the damageEntity method.
I've reported fix to Grum.
[Mojang] Grum (Erik Broes) Can this please not be closed? I get that its probably not something you want to spend time on, but it's still a problem regardless.
Can we leave it open incase Searge or someone else feels like fixing it?
[Mojang] Grum (Erik Broes) Not asking for a high amount, but a mere 32 for example would be a sane limit
As said, there are cases where players with 15-16 character names can not write their name on the sign due to character width.
I really believe its beneficial to let them write the name and let the client truncate as it does for any sign created pre 1.8 with names that no longer can be typed.
For servers that read data off the signs, its game breaking when some players can not have their name written on the sign without resorting to third party resource packs.
All I'm asking is to remove the need to use 3rd party resource packs to achieve a sane result of a mere 16 characters on a sign.
Then theres also the benefit of servers that translate & into color codes, a 32 limit would let you write colored text on the sign that RENDERS within the signs limits, but has more characters than could be typed.
for ex &aW&bW&cWW&eW&dW&4W
would not be possible to type on sign as it is today, but when translated into color codes, it will fit on the signs rendering.
The Papa that's not relevant to this report. This has nothing to do with a client. It's a developer to developer report, which we devs do not have an alternate tracker to report to so this is only place.
The java OS and drivers would be listed as "All"
[Mod] Torabi
"The change that caused this issue was made deliberately, and presumably for a good reason. The fix provided may help Mojang in understanding and fixing the issue, but probably can't just be dropped into the original code due to obfuscation, and they may not want to fix it in that way for the same reason they made the change in the first place."
Actually, us developers will look at code, see 2 things that appear to do the same thing, but miss the slight difference in the 2, or worse, don't clearly see what happens deeper in the flow.
So we then say "let's simplify things! Delete the extra". We test, and everything works... So it seems like a good idea. Deleting code usually is a good thing.
But in this case, the problem isn't seen until larger servers, and until Mojang starts testing updates against 100 player servers...It's going to rely on the 3rd party community to clean up the mistakes, as we're the ones that have in depth profiler data on how the code behaves under heavy load.
The devs don't copy and paste our patches directly of course, but they'll know how to apply it based on logic (hince "paper patch 78" was mine [except for the part that got modified and caused bugs] as well as 2 other performance changes I contributed that landed in 1.9)
You can use Paper server which contains the latest fixes to 1.9 performance problems (some spigot doesn't have atm)
https://aquifermc.org/threads/paper-for-1-9-official-release.51/
[~scott.barnes@gmail.com] that doesn't have anything to do with this issue and would not be a good way to improve the server. That would extremely over complicate the design and make it even harder to make real optimizations.
[Mod] Torabi I wasn't saying simplfying code was wrong. I totally agree that when you see things that appear to do the same thing, to condense it. I was saying that this regression was very unlikely to be intentional, and a mere side effect of the change, as you previously implied they intentionally changed the behavior of the code to always touch the unload queue, and I was saying I disagree and I believe the unload queue behavior change was merely an unintended side effect.
I do know it's a bad idea.
Minecraft servers operate on a 50ms tick rate. Network latency for any given client is going to be on average at least 100ms. Then add in round trip, processor lag on the client, delay between processing the network queues on the server, and then your suggested peer review.
Then factor in data synchronization, that the client then has to know more about data the server normally keeps to itself, increasing load and bandwidth on the network queue.
Now you're easily approaching 700 ms+ to do a single action that normally takes 1/10th of a millisecond.
This ticket is about how to modify a data structure to remove data when it no longer needs to be there. There's nothing a client can participate in logic about here.
This discussion ends here
My fix patch can be seen here: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/CraftBukkit/commits/f92e01ba5cf
I don't believe so, as this memory leak also affects 1.8.
I wonder if
MC-94438has to do with Grass blocks triggering Chunk Loads for Light Level checks.As that is another flaw we've recently found.
My quick bandaid for that in paper is to return 0 for unloaded chunks light level... Which should only ever happen for blocks that gather light data from neighbor, and that would result in it using 1 of the other 3-4 neighbors.
Maybe thats good enough for Vanilla to do?
I think this has to do with Grass blocks triggering Chunk Loads for Light Level checks. As the profiler report in this report shows high tickPending.
As that is another flaw we've recently found and fixed in Paper (Spigot) Server.
My quick bandaid for that in paper is to return 0 for unloaded chunks light level... Which should only ever happen for blocks that gather light data from neighbor, and that would result in it using 1 of the other 3-4 neighbors.
Maybe that's good enough for Vanilla to do?
[Mod] md_5 It's not a change in grass itself, but grass blocks can choose a random block that's on the edge of an unloaded chunk. Then that calls getLightLevel, and the light level check will check its neighboring blocks light level to calculate what it should be.
The big difference in 1.8 vs 1.9 here, is that pre 1.9, there was a boolean in ChunkProviderServer that controlled whether or not stuff like this could trigger chunk loads. It was located between the Chunk Loader and the Chunk hashmap
That boolean was removed in 1.9, so anything acting on unloaded chunks, triggers a chunk load, where as in 1.8 and before, it was limited and did not always load chunks.
Maybe bringing back the empty chunk thing from 1.8 will help solve many of these issues.
[Mod] redstonehelper Yeah I'm saying
MC-100341and alsoMC-98994might be related to the cause ofMC-94438, since the latter is a very generic "I'm Lagging".[Mod] Neko My
MC-100382has nothing to directly do with performance issues of the pathfinding system itself. and would not directly relate toMC-17630That bug fixed in 16w14a is a slow leak, that's been in Minecraft for years, and plenty of servers run fine for hours/days on end.
SuperKawaii no, it is not. This bug has been in Minecraft for years, and not something players can intentionally trigger at a scale to crash a server.
ProfMobius (Thomas Guimbretiere) If this is pathfinding related, here is another patch of mine I've been using in production just fine for a few months now:
https://github.com/PaperMC/Paper/blob/44a1d43781efe9792299457685731847b1a93e92/Spigot-Server-Patches/0061-Optimize-Pathfinding.patch
Drastically helped with failed pathfinding.
I too am seeing this, Ubuntu 14.04.
updated versions.
with the nature of the bug, any chunk that loaded with the bug will be permanently 'bugged'.
The fix only applies to future chunk loads.
I gave Grum the fix to this bug, and it's still in place in 1.10.2.
To clarify more, the code says "is this block a monument" during structure generation.
These chunks loaded, and saved into the monument data file that there is a monument at that location when the logic for determining monuments was broken.
you would have to some how 'unprepare' the structures for that area and make it reprocess, which is not possible with vanilla tools/commands.
No, already generated chunks are near impossible to 'fix', as the game incorrectly registered a structure into the structure data files.
That code is only ran on the chunks first generation, which the previous code had faulty determination logic for.
That data has been saved to the file, and the only way to stop the spawns now is to delete the data marking that area as a monument.
Deleting the chunk/region isn't even enough to do that, as the data for structures is stored in 1 shared file.
I don't think the devs can realistically fix existing worlds that had monuments registered to the wrong location. But the problem that caused it to happen was fixed.
Stick: - WORKS
/give @a minecraft:stick 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}Chest: - DOESNT WORK
/give @a minecraft:chest 1 0 {ench:[{id:0,lvl:1}],HideFlags:1}Hmm, ok it appears it was non empty part that mainly got me. I still think the client should support the empty list again so that every itemstack that was created in that format will not be broken in 'desired effect'
But also requesting Entity Blocks stacks to get the effect.
I've updated ticket on that note. (Note: the [] version doesn't work on Bukkit by command, just Vanilla)
I looked into this, and from my understanding there's not an actual "problem".
The code pretty much is behaving as it should, but Mojang added a "warning" here even though this code path should be expected.
To "avoid" this would defeat the purpose of the mutable positions as you would have to defensive clone it before passing it to another method.
My suggestion is to remove this debug, and ensure anytime a block position is set to a field, to do something like
pos.cloneIfMutable(); where immutable pos will return self and Mutable overrides and clones.
Then we have assurance that mutables will not be stored into fields.
Ok, debugged this and it appears the MC|ItemName packet to reset the name back to default is being sent too early, before the server receives the click event.
This is causing the server to reset the UI and unset the result, causing it to fail.
The client does not send this code if only 1 in the stack due to the left hand stack becoming empty:
I very hackily fixed this on the server by ignoring any incoming packet to set the name to empty if the anvil still has an itemstack in the result slot.
It seems they changed anvils in general in 1.11.1, which
MC-111744seems to be an intended behavior change, but possibly introduced this bug.MC-112017likely was another behavior change that was incomplete.This appears to have been a deliberate change.
Code was added to intentionally achieve a 1 output result.
Likely to handle the case of renaming stacks of 64 being a little overpowered to just cost 1, but scaling cost is ineffective due to the cap.
Kumasasa can you reopen this per my last comment? This isn't the same thing as the ticket it's marked duplicate as.
I forgot I opened that since this one was wrongly closed as duplicate.
The duplicate Hopper.update() call was resolved, but the other issues are still an issue (confirmed as of 1.12.2)
in the method that pushes items into another inventory
have I mentioned how much I hate that environment is a text field above description?....
Confirmed yes still an issue on 1.13.
I've fixed this bug in Paper by randomizing the UUID's a bit before sending to the client. Client really needs to cache based on Textures payload instead of UUID.
Attached screenshots demonstrating problem.
On creating 2nd skull in "created" screenshot, witness both the same.
Reopen client and witness "relogged" screenshot, where textures switch to the other skull.
"payload based" is with my server side fix to send different UUID's to the client.
Attached screenshots demonstrating problem.
On creating 2nd skull in "created" screenshot, witness both the same.
Reopen client and witness "relogged" screenshot, where textures switch to the other skull.
"payload based" is with my server side fix to send different UUID's to the client based on payload data
Copying from
MC-91214:If a world contains 2 player skulls, each of the same players UUID, but each with a different texture ID (Obtained at 2 different time periods), the client will show alternating skin textures on the skulls.
The root issue lies that the skin cache in the client checks by GameProfile for cache key which the equals method only checks by name and uuid.
So when the client looks at a skull after the 15s cache expires, it will get one of the skins, cache it, and use it for both.
The client should show the correct skin for each skull, and simplest way to do that is to make the texture id also part of the cache key.
Issue still remains in 1.13.1 because a new synchronize was added to the FileIO thread, resulting in the exact same problem.
Please see https://github.com/PaperMC/Paper/blob/ffd9c779233fb6e5bb428d865e87c4f3d46607e0/Spigot-Server-Patches/0062-Chunk-save-queue-improvements.patch for my work on optimizing this area of the code.
Ultimately, I would love to see all of this work brought into vanilla (all of my work is licensed MIT).
After many hours of taking cracks at this, I finally figured it out
This is now fixed in Paper version 275+
https://papermc.io/ci/job/Paper-1.13/
I sent to Dinnerbone and Grum, but for anyone else interested, here is the fix:
https://gist.github.com/aikar/5da731bc0fb3dbd322fac5d15689ffef
my patch is based on mc-dev mappings, not mcp.
I believe that
MC-119971is only fixed due to the fact that this file is over synchronized and blocks the getChunkAt call until done....As soon as they fix this issue, they would reintroduce your issue.
They really need to implement my entire patch, which uses a queue to know what work to do, and then uses the map to track latest version of what to save.
I have carefully placed synchronizes to ensure that the removed entry from the map is actually the entry that just got saved, because there is another race condition in this code otherwise:
Chunk can be saved
Chunk is polled on IO thread, saving, but a new version of the Chunk is queued up at same time
IO Thread deletes entry from map, erasing the new one instead of old, making further getChunkAt calls load stale data.
My patch as linked solves all of these issues.
@Urielsalis it is resolved in 1.13 at least. Likely was on 1.12 too as I don't recall dropping this fix in 1.13, we must of dropped it 1 or 2 versions ago.
To add an alternative fix – I realized a patch I added in https://bugs.mojang.com/browse/MC-136995 requires a field CraftBukkit added, to retain if the entity is in the world or not, which I used to fix this bug in Paper:
Then this code only needs a very minor change:
The downside to Xcom's fix is that dimension teleporting will trigger chunk loads to an unnecessary chunk as it first updates the position of the entity before snapshotting it to copy into the new dimension, and that would trigger the chunk to load.
however, the entity being removed from the world before its moved would set valid=false and then not load the chunk with my supplied version.
Paper bug report: https://github.com/PaperMC/Paper/issues/1880
Please see https://bugs.mojang.com/browse/MC-145752 for my input on how to solve this implementation wise.
ultimately what is needed is a targetRange attribute.
Back in pre 1.6 days, internal had target range and follow range as separate values. a zombie would aggro at 16 (targetRange) but follow you up to 40.
But 1.6 introduced attributes and merged these values, resulting in zombies now aggroing at 40 (now 35?)
It seems like mojang realized that was bad and has once again separated the values, but currently hard coded.
Just give a target range attribute and everythings good.
Target Range controls the distance a monster will choose to attack you, and then follow would be how far you have to run from it before it gives up chase.
And if you want both of those values to be high/same, then set both to the same.
As reported by Connor, that wasn't what was needed to fix it. It needed to be made an int instead to solve the overflow issue.
there is no benefit to try to be conservative on 2 bytes here. This isn't a heavy packet.
Is it not possible to spawn an entity and supply its nbt data, passing a large int?
Connors comment implies they did.
This is expected with the new chunk system. The server no longer sleeps, and instead spinwaits.
If you want to learn more, jump on the PaperMC Discord.
Yes, fix for this is also related to:
MC-136995It is my understanding this is resolved now too.
Latest 1.15 patch, these notes are for most part all still relevant: https://github.com/PaperMC/Paper/blob/10502558e92f478e7e1343193dd8ace8dbf8ac78/Spigot-Server-Patches/0414-Optimize-Hoppers.patch
I have updated the ticket with lots of 1.15 relevant information.
That's called cheating and not something you should do on a survival multi-player server
FYI Discussion thread for this issue is at https://www.reddit.com/user/sliced_lime/comments/gzpo5p/some_words_on_things_in_116/
The above comment is false, please don't disable that config it doesn't relate to this issue
@TPEHEP You don't have to keep asking me this. The developers will update this ticket if they've made progress.
It's up to the developers to note if they fully solved each item I raised. I will update ticket if I detect they made progress as I apply our patches.
@nighter Why is this (which is a same issue as
MC-191388) marked Won't Fix?FYI This is patched (worked around) in Paper 1.15+ (https://papermc.io for those unaware).
The bug lies in the client, but we sent extra light data to the client to avoid this issue.
https://github.com/PaperMC/Paper/blob/b6925c36afa7565cac8f9fe9d3432fe658ca1f77/Spigot-Server-Patches/0496-Workaround-for-Client-Lag-Spikes-MC-162253.patch
Attached a screenshot from a user, appears this is still the same damage as
MC-191388To reproduce, creating a world in 1.15 and finding a village, and I'm not sure if partial generation applies or not, but if it does, stop moving soon as part of a village is seen, and then open that world in 1.16.2
I will try to attach some files I have reproducing this every time after work too, just need to try to filter it to just needed files first and get coords.
I have one report that it was related to partial generation, but in my own test world I get it at my "Home" location which is a village and i know it was fully generated.
Another suspect is if a village doesn't have a pillar outpost, and my village does not... (likely created in 1.12).
Hope this helps.
I've got a report now it even impacts a fresh 1.16.1 generated chunk too.... so impact is a bit higher as it covers everyone doing a point release, and people really dont want to regen worlds for a point release so fast.
There is a new issue for the new error:
MC-197883Context of the issue has changed, but end results are similar.
However the bug now also impacts 1.16.1 -> 1.16.2 too.
Root issue appears that there are registries that are specifying "name" as their identifier instead of "type"
We tried updating the .dispatch() call for Configured World Gen Features to be name instead, and it solved THIS specific issues, but caused others that do actually use "type"...
so we have a mixed data set.
Mojang will have to correct the data points to be consistent as type.
We're going to bandaid it by checking if type doesnt exists, try name
So it's clear for those that didn't interpret it from my last reply providing what the actual problem is here
This issue is fixed in Paper 1.16.2 - https://papermc.io/downloads
Those errors you saw are 100% a completely different bug. I assume
Mojang, I don't know if a new issue is warranted for this one, but I don't have any more context since can't reproduce my self:
https://pastebin.com/shetqD5e
@K.Geraghty Welcome to Paper
This is what our project is known for, is improving minecrafts performance and security as well as a ton of new features for server owners.
We work pretty closely with Mojang to help bring our improvements to the base game, but sadly that process can be a bit slow...
@Broken-arrow because the logs you pasted in that ticket is in fact a duplicate of this one.
The logs you pasted on Paper's issue tracker looks to be a different issue. the key is "position_predicate" vs "type"
I linked the logs in my last reply
@Marcono, I can say with confidence my fix can not cause a regression, as it only deals with type/name. I question if Michaels issue is a similar issue but different data set with same problem though.
I cant create a ticket for that yet as michael has not provided me enough information to reproduce it yet.
I am a creator of Paper, It is a vanilla issue.
So what? It will be in it when it is.
Fix for this issue here: https://github.com/PaperMC/Paper/commit/01b1971a43f56da6418f3f623dc972f46dcff373
int j = this.c.firstPartial(itemstack); if (j == -1) { j = this.c.getFirstEmptySlotIndex(); } // Paper start if (j == -1) { this.c.player.drop(itemstack.cloneItemStack(), false); break; } // Paper end ItemStack itemstack1 = itemstack.cloneItemStack(); itemstack1.setCount(1); if (!this.c.c(j, itemstack1)) { AutoRecipe.LOGGER.error("Can't find any space for item in the inventory"); }Incorrect, it is a server side bug that generating chunks if the right scenarios come into play, will crash the server.
@Pcobald can you remove that comment? It's spam and does not add anything of value to the conversation.
Unless Mojang asks for more information, no more needs to be supplied for this issue.
So please avoid replying with more stuff until Mojang asks, though last I talked with Mojang on this issue, they seem to understand what the issue is.
This is unlikely to be fixed before 1.17.