Pistons forget update when being reloaded

Is this still an issue in the latest snapshot 16w44a? If so please update the affected versions.

This is an automated comment on any open or reopened issue with out-of-date affected versions.

yes. also an issue in 1.12 and very likely in the latest snapshots

This bug is related to the update order of tile entities. When tile entities are saved to disk they are saved using a hashlist. But before they are saved the game keeps a strict FIFO. The order is critical in most instances making the save process lose the proper order in which tile entities should be updated causing issues to both flying machines (generally anything to do with pistons), hoppers and most notably instant wires.

Of course the loading process will lose the order if incorrect order of chunks are loaded but internally inside each chunk the order scrambles as the saving process uses a hash order based on the position. The order can be kept by simply using a linked hash map making anything that relies heavily on the correct order correctly ordered after reload.

The list is found in Chunk.java

private final Map<BlockPos, TileEntity> chunkTileEntityMap;

this.chunkTileEntityMap = Maps.<BlockPos, TileEntity>newHashMap();

The saving happens in AnvilChunkLoader.java

for (TileEntity tileentity : chunkIn.getTileEntityMap().values())
 {
 NBTTagCompound nbttagcompound3 = tileentity.writeToNBT(new NBTTagCompound());
 nbttaglist2.appendTag(nbttagcompound3);
 }

This clearly shows that saving happens based on a hash implementation instead of the order that is used during runtime as FIFO. If the newHashMap is changed to newLinkedHashMap the order is kept making any instant wire function properly.

There is a separate bug related to pistons as well where the proper order of moving blocks aren't saved properly.

Writing to NBT saves the progress by using last progress
compound.setFloat("progress", this.lastProgress )

Reading from NBT then loads it into both progress and last progress creating an inconsistency
this.progress = compound.getFloat("progress");

this.lastProgress = this.progress;

It should be changed to

Writing to NBT
compound.setFloat("progress", this.progress);
 compound.setFloat("lastProgress", this.lastProgress);
Reading from NBT
this.progress = compound.getFloat("progress");
 this.lastProgress = compound.getFloat("lastProgress");

These 2 small fixes will help any inconsistency that happens to pistons after reloads.

 Edit:

Another potential unexplored problem can be the scheduled block events. They are processed after even chunk unloading process and tile entities plus the auto-save in the "sendQueuedBlockEvents()". This means that any block events that were scheduled never gets processed if an auto-save happens.

As this list keeps track of all block events its responsible for both updates to blocks such as pistons and is critical in flying machines and other piston based contraptions. When the chunk is unloaded the list keeps track of any block events until the chunk is reloaded. But if the server shuts down the list is simply lost.

This list should probably be saved to disk in some ways to be recovered after a server restart to not cause issues.

Confirmed on 19w46b.

Can you stick to the full version number when you confirm? Thanks.

@unknown ticket is yours now. You can delete all your comments and update the report yourself in the future.

Confirmed for 1.17 pre-2.

Confirmed for 1.17 pre-3.

Can confirm in 1.17.1 Release Candidate 1.

Can confirm in 1.18.2

I've added 5 images, all done within 22w14a

One baseline before any restarts. The image of the machine stopping is the same as when the fix is applied and stopped after a restart.
Then an image of after a restart, and the machine stopping after the restart.
Notice that the pistons are broken when doing a restart normally.

This is due to the progress and last progress of the piston not being set correctly. Xcom6000's fix was used to demonstate this

can confirm 1.21

The main issue described here is the loss of block events upon a reload (they just arent saved)
Which makes it so if the restart happens in a right tick pistons just wont extend at all cus their corresponding block events didnt save.
The second issue present here is scrambling of block entity ticking order and
this issue affects not only pistons, but all block entities, this causes a lot of sensitive contraptions to break.
Here is a testing setup to replicate the issue:

To replicate follow these steps:
1. Reset the contraption by the note block labeled Reset (next to a gold block).

2. Test the contraption by clicking the note block (next to the diamond block). Observe the pistons extending, only the pistons in the same row activates.

3. Reload the world. After the reload the pistons no longer behave normally. Their behavior changes by the pistons on random side extending.

4. Resetting the contraption fixes it till the area reloads resulting in the scrambled order again.

Noting that the bug is locational. It appears after reload. It affects more than pistons, all block entity's are affected.
This happens due to order in which ticking block entities get ticked is scrambled upon a chunk reload due to the way they are stored.

This bug affects a lot of redstone contraptions, such as storage systems, flying machines, and basically all contraptions which rely on block entity ticking order, so basically any sensitive contraption.

There is a simple way to fix this issue:
in
```
ChunkAccess.java
```
The fix is in done by changing the hash map to a linked hash map

```
protected final Map<BlockPos, BlockEntity> blockEntities = new Object2ObjectOpenHashMap<>();
```
the fix is simply done by changing it to
```
new Object2ObjectLinkedOpenHashMap<>();
```

why are you referring to the piston as male

Confirmed in 1.21.3