Moving Elevators

Moving Elevators

7M Downloads

[Bug] Elevator drops redstone on Z axis

KyokoTomato opened this issue · 4 comments

commented

Version Info

  • Minecraft, 1.21.1
  • Moving Elevators, 1.4.8

Are you using OptiFine?: No

Description of the Bug
When placing redstone connected in the Z axis (north/south) on a moving elevator, redstone pieces will drop but nothing breaks. This can create infinite redstone
Doesn't happen for redstone stepping vertically, or redstone along the X axis

Steps to Reproduce
Create a moving elevator with a flat platform, and connect pieces of redstone along the Z axis. Tell the elevator to move to another floor, and it'll drop redstone dust

Screenshots
2024-10-15_13 12 59
2024-10-15_13 13 02

commented

Just... how...
How the hell does the block stay yet also drop as an item...
And why only when connected in a specific direction...
I am genuinely amazed at this bug 🤔

commented

Maybe it has something to do with update order? I think it goes from the negative to positive direction, and X-Y-Z? (Ignoring diagonal updates. I have no clue how those change things. It may as well be magic)

I don't know how helpful this is, but I did some tests that might be related:

  1. When the bug happens, it only happens when the carriage leaves. It pops out redstone dust when it leaves, but does not pop out more when the carriage arrives
  2. The elevator carriage causes different block updates when it leaves vs when it arrives (The thing tested)
    2.1 Leaving: Only causes block updates on non-air blocks within carriage
    2.2 Arriving: Causes block updates for the entire defined carriage area, even in air blocks

Also, images were taken at a lower resolution because I remembered file sizes exist, and didn't want to .jpg them

The test setup

Two budded pistons each at source and destination, one where a solid block is (or will be), and another where there is only air (but next to where the carriage area is/will be). The green wool is to show carriage height

You could also do this with observers or something, but that's harder to screenshot

Before

After: At the source area, only the piston in contact with a solid block received a block update. In the destination area, even the piston next to air received a block update

So, if block updates happen differently, it might have something to do with whether or not the carriage is created (and the blocks that made it up are destroyed) before any block updates can happen? Assuming they're connected and not just, you know, both things related to block updates

Also! Extended pistons drop an item (but stay) if they're facing any axis' negative direction. This probably should have been multiple comments, but I have no clue how notifications work on github, so.

Pistons Positives

Negatives
It might be kind of hard to see, but there are three piston items on the ground

Anyways, as a final thing, you can do mean things to pistons, but I don't know that this is a bug, really, because that's just what you do with pistons

Pistons extended into/out of an elevator Before During After
commented

Current implementation

The way I designed the elevator cabin clearing/placing code is that it does two passes over all blocks in the cabin area. The first pass, it is supposed clear/place all the blocks whilst suppressing all block updates. The second pass, it is supposed to do all the block updates that were suppressed before.
This should prevent any blocks from updating their shape based on a partially removed cabin.

There were two main issues with the current implementation:

  1. To suppress the block updates the code currently just uses specific update flags from the Block class when calling Level#setBlock. However, it seems when Level#setBlock in turn calls LevelChunk#setBlockState on the relevant chunk, it always calls BlockState#onRemove for the current state and BlockState#onPlace for the new state no matter what flags are set.
  2. The second pass when placing the cabin was missing some block updates which would have been called if placed normally with Level#setBlock.

The first leads to pistons and redstone wire being updated when they shouldn't and dropping as items as their respective blocks implement logic in the Block#onRemove method.
The latter led to observers not being updated when the cabin was placed.

PRs

Feedback for the PRs you submitted (and why I haven't accepted them):

#241: The PR reverses the vertical iteration order when clearing blocks such that blocks are removed from top to bottom to prevent redstone wire from breaking and dropping. Redstone wire dropping is the result of some form of block update. During the clearing stage, no block updates should occur. The PR doesn't really address the root cause which is those unwanted block updates. In addition, by reversing the iteration order, the same issue may occur for blocks which depend on the block above them rather than the block below them.
The second part of the PR adds an additional Level#markAndNotifyBlock call for redstone wire. The additional call should ideally be placed during the block update pass rather than the placement pass before it. This does seem to be the correct solution though, however as described above, it seems this was the result of a mismatch between what Level#setBlock does in terms of block updates and what the block update pass had. Thus, this problem wasn't necessarily specific to redstone wire.

#242: Similar to #241, the reason for pistons dropping was unwanted block updates and this PR doesn't really address the root cause.

#243: Observers not being updated properly seems to have been caused by a mismatch between what the block update pass does when placing a cabin and what Level#setBlock would do in terms of block updates when a block is placed normally. Fixing that so the block update pass matches Level#setBlock already seems to make observers update properly without any special casing.

Resulting changes

I added a mixin to suppress the BlockState#onRemove and BlockState#onPlace calls in LevelChunk#setBlockState during the first pass for cabin clearing/placing. During the block update passes for cabin clearing/placing, I then added these BlockState#onRemove and BlockState#onPlace calls. This seems to solve the redstone wire and pistons dropping when they shouldn't and also the weird direction dependent unpowered redstone wire when it should be powered.
As you mentioned, you do end up with headless pistons when an extended piston crosses the edge of a cabin. This is still the case as I don't think there's really a good solution in terms of desired behaviour. Simply unpowering the piston base makes it return to normal, so I don't think this is too much of an issue.

I also made the block updates during the second pass when placing a cabin match what is expected from Level#setBlock. This seems to fix observers not triggering when the cabin arrives.

In addition, I also noticed that blocks indirectly powered by redstone wire across the cabin edge would not get powered/unpowered when the cabin moved. It seems redstone wire only updates indirectly powered blocks whenever its power level changes and not when the block is placed. This can be seen when placing an already powered redstone wire with the /setblock command as any blocks which should get powered by the placed wire are not.
This does seems specific to the redstone wire block and I have added a special case for redstone wire placed on the cabin boundary to replicate the behaviour from when a redstone wire's power level changes.

As far as I can tell, this fixes all the block update and redstone issues.
Thank you for the issue report and excellent help investigating what is going on! Having a bunch of concrete examples and descriptions of where exactly things go wrong makes debugging and fixing these things a lot easier 😊

I have pushed all the changes (dd74d6f, 72a3585, 205637f, 78920f4). I will probably release an update for it tomorrow after porting the changes to all the other Minecraft versions and modloaders.

commented

I released the changes as version 1.4.9 now.
Thank you for reporting the issue and for the help!