Breaking or blowing up campfires sometimes still make smoke particles

This also happens when they are removed with structure blocks, I made a report of that here:

MC-141916

Also happens (sometimes) when you put water over them, then take it away and delete the campfires manually.

the bug has NOT been solved yet. also happening to me

I can recreate this bug reliably by generating a lot of lag in a single chunk (e.g. with observerclock + redstone). With all fireplaces inside the 'lag-chunk' this bug occures, but with fireplaces in other chunks it does not (see image below)

Also I attached my Test-World for easy reproduction of this bug.

This also happens when removed with the fill command.

Also happens if you have viewdistance < 8 and you  place a campfire, fly out of range that the chunk unloads, fly back and destroy the campfire.

Affects 19w04b, and also occurs when /fill is used to remove campfires

Affects 19w05a.

PhiPro and I did some investigation into the cause of this bug and think we know both the cause and an easy fix. This hopefully also resolves MC-142214 and MC-142332. The basic issue is that when the client processes a ChunkDataClientPacket it does not properly remove the tile entities that are in that chunk. In particular, the tile entities are left in a few key lists like World.tickingBlockEntities, and therefore they continue to tick and produce smoke. This can lead to many duplicated tile entities at the same location (causing lag by creating many particles) or smoke particles being created from blocks that no longer contain campfires. In agreement with the other bug reports, this requires many blocks within the same chunk as the campfire to be updated (to trigger the ChunkDataClientPacket).

The relevant code is below (using the mappings from fabric for 19w05a):

In WorldChunk.method_12224(PacketByteBuf, CompoundTag, int, boolean), (called by ClientPlayNetworkHandler.onChunkData > ClientChunkManager.method_16020 > WorldChunk.method_12224) the following code appears:

Iterator iterator_1;
int int_2;
if (boolean_1) {
   this.blockEntityMap.clear();
} else {
   iterator_1 = this.blockEntityMap.keySet().iterator();        
   while(iterator_1.hasNext()) {
      BlockPos blockPos_1 = (BlockPos)iterator_1.next();
      int_2 = blockPos_1.getY() >> 4;
      if ((int_1 & 1 << int_2) != 0) {
         iterator_1.remove();
      }
   }
}

This code removes the entities from the chunk's block entity map, but leaves them in a few important lists. To fix this, we should call world.removeBlockEntity on the positions for each of the tile entities instead of using the iterator.remove method. To avoid ConcurrentModificationException s, one possibility is to make a list of the affected coordinates. Alternatively, it might be possible to call world.removeBlockEntity immediately after iterator_1.remove()

Confirmed in 19w06a.