[Bug] Elevator drops redstone on Z axis
KyokoTomato opened this issue · 4 comments
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
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 🤔
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:
- 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
- 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
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
PositivesNegatives
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
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:
- To suppress the block updates the code currently just uses specific update flags from the
Block
class when callingLevel#setBlock
. However, it seems whenLevel#setBlock
in turn callsLevelChunk#setBlockState
on the relevant chunk, it always callsBlockState#onRemove
for the current state andBlockState#onPlace
for the new state no matter what flags are set. - 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.