Structure blocks take and apply NBT data of entities and tile entities directly instead of a copy when loading and saving structures
The bug
Structure blocks do not make a copy of the NBT data of an entity or tile entity when they save it in a structure or when they load it from the structure. This results in multiple bugs:
- Loading a structure multiple times results in multiple entities or tile entities "sharing" the same NBT data
- Modifying an entity or tile entity which was saved as part of a structure might modify the NBT data of the respective entity or tile entity in the structure data as well
- Loading a structure with entities and tile entities and modifying them might modify the NBT data of the entity or tile entity which was saved as part of the structure
How to reproduce
- Place a structure block
- Place a mob spawner on top of it and save it as structure
/setblock ~ ~ ~ mob_spawner default replace {SpawnData:{id:"armor_stand"},SpawnCount:0s,SpawnRange:0s,MinSpawnDelay:0s,MaxSpawnDelay:0s,MaxNearbyEntities:1s,RequiredPlayerRange:16s,SpawnPotentials:[{Weight:1,Entity:{id:"bat"}}]}
- Load the structure at multiple positions in the world
- Right click one of the loaded spawners with for example a creeper spawn egg
- Go to the other spawners and check their NBT data (they won't update visually)
/blockdata ~ ~-1 ~ {}→ You will notice that id tag in the SpawnData tag has now the value "minecraft:creeper" (= case 1)
- Load the structure again
→ Even the newly loaded mob spawners will spawn creepers despite the fact that you did not overwrite the structure (= case 2) - Set the time to night and use /blockdata on one of the loaded spawners to make it spawn the SpawnData entity and switch to the SpawnPotentials entry
/time set 14000
/blockdata ~ ~-1 ~ {SpawnCount:1b} - Right click the spawner with for example a chicken spawn egg
- Use /blockdata on the spawner you saved in the structure
/blockdata ~ ~-1 ~ {}→ You will notice that its SpawnPotentials entry changed as well. Its value is now "minecraft:chicken" instead of "minecraft:bat" (= case 3)
Code analysis
Based on 1.11.2 decompiled using MCP 9.35 rc1
The following methods do not make a copy of the NBT data taken from the world
- net.minecraft.world.gen.structure.template.Template.takeEntitiesFromWorld(World, BlockPos, BlockPos)
- net.minecraft.world.gen.structure.template.Template.takeBlocksFromWorld(World, BlockPos, BlockPos, boolean, Block)
The following methods do not make a copy when loading entities respectively tile entities in the world
- net.minecraft.world.gen.structure.template.Template.addEntitiesToWorld(World, BlockPos, Mirror, Rotation, StructureBoundingBox)
- net.minecraft.world.gen.structure.template.Template.addBlocksToWorld(World, BlockPos, ITemplateProcessor, PlacementSettings, int)
Created Issue:
Changing one mob spawner with egg replaces all new spawners' potentials
I have a structure that I'm spawning in with a structure block. This structure has a single mob spawner inside it with a SpawnPotentials list containing two types of entities: skeletons with bows, and skeletons without bows. That works perfectly fine. But, if I spawn the structure in, then change the spawner to, say, spiders with a spawn egg, the next time I load the structure, it will have spiders on its spawn potential list and randomly switch between skeletons and spiders in the cage instead of only the skeletons. In fact, looking at the block entity data shows the first entry in the SpawnPotentials list has its ID replaced with the spider, but retains all the tags of the skeleton that used to be there. The issue is only resolved after removing all spawners, quitting the world, and coming back into the world; then future spawners have their proper SpawnPotentials list.
I don't know if this is a problem only with structure-block-generated spawners or all spawners, but it's a problem; and it doesn't matter where the new spawners are placed, they still keep the spider data from the first spawner even though they shouldn't.
To Reproduce:
1. Create a mob spawner that has a SpawnPotentials list and save it into a structure.2. Remove the spawner, then load the structure.
3. Change the spawner type with a spawn egg, then break the spawner.
4. From now on, whenever you load the spawner from the structure, anywhere in your world, it'll have the new type on its SpawnPotentials list, even though it doesn't have that type in the tags in the structure file.
Code Analysis:
I'm not 100% sure what's causing this, but looking at the 1.11.2 code decompiled in Forge, the only related code I can find that isn't specific to each spawner tile entity instance is the Data Fixer code for mob spawners. So although I haven't pinpointed the exact source of the bug, my best guess is that somewhere, the data fixer is being applied where it shouldn't, "fixing" the IDs of the spawn potentials of the wrong tile entities.
Just speculation, though.
Changing one mob spawner with egg replaces all new spawners' potentialsStructure blocks apply NBT data of entities and tile entities directly instead of a copy when loading a structure
I have a structure that I'm spawning in with a structure block. This structure has a single mob spawner inside it with a SpawnPotentials list containing two types of entities: skeletons with bows, and skeletons without bows. That works perfectly fine. But, if I spawn the structure in, then change the spawner to, say, spiders with a spawn egg, the next time I load the structure, it will have spiders on its spawn potential list and randomly switch between skeletons and spiders in the cage instead of only the skeletons. In fact, looking at the block entity data shows the first entry in the SpawnPotentials list has its ID replaced with the spider, but retains all the tags of the skeleton that used to be there. The issue is only resolved after removing all spawners, quitting the world, and coming back into the world; then future spawners have their proper SpawnPotentials list.
I don't know if this is a problem only with structure-block-generated spawners or all spawners, but it's a problem; and it doesn't matterwherethenew spawners are placed, they still keep the spider data from the first spawner even though they shouldn't.To Reproduce:
1. Create a mob spawner that has a SpawnPotentials list and save it into a structure.2. Remove the spawner, then load the structure.
3. Change the spawner type with a spawn egg, then break the spawner.
4. From now on, whenever you load the spawner from the structure, anywhere in your world, it'll have the new type on its SpawnPotentials list, even though it doesn't have that type in the tags in the structure file.
Code Analysis:
I'm not 100% sure what's causing this, but looking at the 1.11.2 code decompiled in Forge, the only related code I can find that isn't specific to each spawner tile entity instance is the Data Fixer code for mob spawners. So although I haven't pinpointed the exact source of the bug, my best guess is that somewhere, the data fixer is being applied where it shouldn't, "fixing" the IDs of the spawn potentials of the wrong tile entities.
Just speculation, though.
The bug
Structure blocks do not make a copy of the NBT data of an entity or tile entity when they save it in a structure or when they load it from the structure. This results in multiple bugs:
- Loading a structure multiple times results in multiple entities or tile entities "sharing" the same NBT data
- Modifying an entity or tile entity which was saved as part of a structure might modify the NBT data of the respective entity or tile entity in the structure data as well
- Loading a structure with entities and tile entities and modifying them might modify the NBT data of the entity or tile entity which was saved as part of the structure
How to reproduce
- Place a structure block
- Place a mob spawner on top of it and save it as structure
/setblock ~ ~ ~ mob_spawner default replace {SpawnData:{id:"armor_stand"},SpawnCount:0s,SpawnRange:0s,MinSpawnDelay:0s,MaxSpawnDelay:0s,MaxNearbyEntities:1s,RequiredPlayerRange:16s,SpawnPotentials:[{Weight:1,Entity:{id:"bat"}}]}- Load the structure at multiple positions in the world
- Right click one of the loaded spawners with for example a creeper spawn egg
- Go to the other spawners and check their NBT data (they won't update visually)
/blockdata ~ ~-1 ~ {}→ You will notice that id tag in the SpawnData tag has now the value "minecraft:creeper" (= case 1)
- Load the structure again
→ Even the newly loaded mob spawners will spawn creepers despite the fact that you did not overwrite the structure (= case 2)- Set the time to night and use /blockdata on one of the loaded spawners to make it spawn the SpawnData entity and switch to the SpawnPotentials entry
/time set 14000/blockdata ~ ~-1 ~ {SpawnCount:1b}- Right click the spawner with for example a chicken spawn egg
- Use /blockdata on the spawner you saved in the structure
/blockdata ~ ~-1 ~ {}→ You will notice that its SpawnPotentials entry changed as well. Its value is now "minecraft:chicken" instead of "minecraft:bat" (= case 3)
Code analysis
Based on 1.11.2 decompiled using MCP 9.35 rc1
The following methods do not make a copy of the NBT data taken from the world
- net.minecraft.world.gen.structure.template.Template.takeEntitiesFromWorld(World, BlockPos, BlockPos)
- net.minecraft.world.gen.structure.template.Template.takeBlocksFromWorld(World, BlockPos, BlockPos, boolean, Block)
The following methods do not make a copy when loading entities respectively tile entities in the world
- net.minecraft.world.gen.structure.template.Template.addEntitiesToWorld(World, BlockPos, Mirror, Rotation, StructureBoundingBox)
- net.minecraft.world.gen.structure.template.Template.addBlocksToWorld(World, BlockPos, ITemplateProcessor, PlacementSettings, int)
Structure blocks take and apply NBT data of entities and tile entities directly instead of a copy when loading and saving structures
The bug
Some actions in the game apply the same NBT object instance to multiple items, entities or tile entities. This means if you edit it for one of them you basically edit it for all of them.
This could in theory happen with non-NBT data as well, but at least in the current versions this is not the case because either:
- the data is stored in primitive types and therefor no objects are involved
- the data is provided in NBT format but then parsed and stored with objects; therefor no reference to the NBT data exists anymore
Some of the cases listed below might happen when the NBT data is read or written by entities or tile entities. The expected fix is however not to have them always create a copy because that would slow down saving and loading worlds. Instead the methods calling these methods should create a copy before respectively after.
The reason why this report contains a list of all cases, even the ones which are not affected, is to have an overview and make it easier to detect new cases in which this happens.
Actions
Affected key
| Symbol | Meaning |
|---|---|
| Affected | |
| Not affected | |
| Currently not affected, but current behavior is listed as bug and therefor likely affected in the future |
List
Based on 1.11.2 decompiled using MCP 9.35 rc1
Last updated for 1.11.2
Likely incomplete
| Action | Affected | Comment | Affected method |
|---|---|---|---|
| Pick block with NBT | Currently happens client-side, but should happen server-side | Minecraft.storeTEInStack(ItemStack, TileEntity) |
|
| Copying items in Creative | - | - | |
| Crafting: Cloning books | - | - | |
| Crafting: Cloning banners | - | - | |
| Crafting: Cloning maps | - | RecipesMapCloning.getCraftingResult(InventoryCrafting) |
|
| Creating item in Creative with BlockEntityTag | - | NetHandlerPlayServer.processCreativeInventoryAction(CPacketCreativeInventoryAction) |
|
| Using item with EntityTag | Dispenser, spawn eggs, armor stand... | ItemMonsterPlacer.applyItemEntityDataToEntity(World, EntityPlayer, ItemStack, Entity) |
|
| Item with BlockEntityTag placed | - | - | |
| Items from loottables | Uses NBTTagCompound.merge which clones | - | |
| Items from villagers | - | - | |
| Splitting item stacks | - | - | |
| Hopper (minecart) item actions | - | - | |
| Droppers transferring items | - | - | |
| Cleaning banner in cauldron | - | - | |
| Cleaning leather armor in cauldron | Even in Creative stack is replaced and water level reduced, not the case with banners, see MC-114105 | BlockCauldron.onBlockActivated(World, BlockPos, IBlockState, EntityPlayer, EnumHand, EnumFacing, float, float, float) |
|
| Putting item on armor stand | - | - | |
| Using firework | - | - | |
| Putting item in item frame | Only cloned when not empty, but even if empty set as item | EntityItemFrame.setDisplayedItemWithUpdate(ItemStack, boolean) |
|
| Putting chest on donkey or mule | Entity does not store item data but only boolean, see |
- | |
| Putting saddle on pig | Entity does not store item data but only boolean, see |
- | |
| Dispenser / dropper dropping item | - | - | |
| Dispenser placing player skull | GameProfile is read and stored | - | |
| Dispenser shooting tipped arrow | Potion effects are read and stored | - | |
| Dispenser throwing splash or lingering potion | - | - | |
| Dispenser equipping armor (including pumpkin, elytra and skull) | - | - | |
| Dispenser playing shulker box | - | - | |
| /clone | See paragraph "How to reproduce" for reproduction | CommandClone.execute(MinecraftServer, ICommandSender, String[]) |
|
| /fill | Fixed, see |
CommandFill.execute(MinecraftServer, ICommandSender, String[]) |
|
| /entitydata | Runs for each entity seperately and uses NBTTagCompound.merge which clones | - | |
| /replaceitem entity | Runs for each entity seperately | - | |
| Structure blocks loading and saving | See |
- |
How to reproduce
Some of these cases cannot be reproduced because other actions only override the affected data instead of modifying it, or other actions create a copy before you could try to reproduce them. Therefor here are only reproduction steps for some actions.
/clone
- Place a spawner
/setblock ~ ~ ~ spawner{SpawnData:{id:"armor_stand"},SpawnCount:0s,SpawnRange:0s,MinSpawnDelay:0s,MaxSpawnDelay:0s,MaxNearbyEntities:1s,RequiredPlayerRange:16s,SpawnPotentials:[{Weight:1,Entity:{id:"bat"}}]} - Stand on top of the spawner and run the following command
/clone ~ ~-1 ~ ~ ~-1 ~ ~2 ~-1 ~
- For the cloned spawner
- Make it switch to the SpawnPotentials (stand on top of it while running the command)
/data modify block ~ ~-1 ~ SpawnCount set value 1b
- Right click the spawner with a creeper spawn egg
- Make it switch to the SpawnPotentials (stand on top of it while running the command)
- For the original spawner
- Inspect the NBT data
/data get block ~ ~-1 ~ SpawnPotentials
It shows "minecraft:creeper" as entry for the SpawnPotentials instead of "minecraft:bat"
- Inspect the NBT data
/fill
Fixed by MC-163953 ![]()
Different root cause than MC-114030 for pillagers outside outposts, reopening
The problem seems to be that in general structure blocks apply the NBT data stored in the structure file without creating a copy. This means all entities and tile entities created by a structure block basically "share" the same NBT data.
I am going to rewrite the description a little bit, I hope you are fine with that.
@Marcono1234 Sure, that's fine; but I'm having trouble locating that specific code you mentioned. Perhaps Forge has already fixed this bug in its source, but looking through the 1.11.2 source that Forge provides, the takeBlocksFromWorld() method does seem to create a new NBT tag, which it copies the tile entity data to, and then adds that copy of the NBT to a new BlockInfo instance, which is in turn added to the template's block list. So where exactly is it failing to make a copy? (Or, again, perhaps this is something that Forge's developers already fixed and just failed to comment on?)
It is true that it writes the NBT data to a new NBTTagCompound, but the method overriding net.minecraft.tileentity.TileEntity.writeToNBT(NBTTagCompound) do not necessarily copy the NBT data they add to this compound and they probably should not do it anyways because that would only slow down saving a world.
This can for example be seen with banners (net.minecraft.tileentity.TileEntityBanner.writeToNBT(NBTTagCompound)) as they directly set their patterns NBTTagList.