The bug
When running water into string that is attached to a pair of tripwire hooks the string is broken as expected BUT one more than the number broken is dropped.
In other words: if you have two trip wire hooks and between them six string, then seven string are dropped when water runs into and destroys all of the string. However, this behavior (at least in my experience) only happens when the line of string is at least four long.
To duplicate build the device in the screenshots below. (String goes on black wool, and hooks go on block next to and above the yellow wool. I would recommend building everything, putting down the string and then powering the pistons and then putting the water down.
This only works when the water flows in the east-west direction. Testing with the water flowing in the north-south direction will not result in successful reproduction.
Code analysis
Code analysis by @unknown can be found in this comment.
Linked issues
is duplicated by 1
relates to 2
Attachments
Comments 17
confirmed for 1.8.1-pre4, observed the effect for 2-long and 3-long tripwire (does not require 4-long wire as stated in the description)
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.
Full credit for the following code analysis goes to @unknown (from Discord). Thanks!
The following is based upon Yarn 22w05a mappings.
net.minecraft.block.TripwireBlock.java
private void update(World world, BlockPos pos, BlockState blockState) {
for(Direction direction : new Direction[]{Direction.SOUTH, Direction.WEST}) {
for(int i = 1; i < 42; ++i) { //Loop through string next to self
BlockPos blockPos = pos.offset(direction, i); //can be mutable
BlockState state = world.getBlockState(blockPos);
if (state.isOf(this.hookBlock)) {
if (state.get(TripwireHookBlock.FACING) == direction.getOpposite()) this.hookBlock.update(world, blockPos, state, false, true, i, blockState);
break;
}
if (!state.isOf(this)) break;
}
}
}In the tripwire block, the dupe happens due to the updates that are given when the other string are destroyed. The tripwire will loop though all the wires that it is connected to, until it hits a tripwire hook. It then calls the tripwire hook update method. The part that matters in this code is the i & blockState, arguments 6 & 7 of update() Where the tripwire sends its distance & its own block state.
net.minecraft.block.TripwireHookBlock.java
public void update(World world, BlockPos pos, BlockState state, boolean beingRemoved, boolean update, int i, @Nullable BlockState blockState) {
Direction direction = state.get(FACING);
boolean attached = state.get(ATTACHED);
boolean powered = state.get(POWERED);
boolean stayAttached = !beingRemoved;
boolean stayPowered = false;
int distance = 0;
BlockState[] blockStates = new BlockState[42];
for(int k = 1; k < 42; ++k) { //Loop through string in front of hook
BlockState blockState2 = world.getBlockState(pos.offset(direction, k)); //Get blockstate at location
if (blockState2.isOf(Blocks.TRIPWIRE_HOOK)) {
if (blockState2.get(FACING) == direction.getOpposite()) {
distance = k; //Set the ending direction
}
break;
}
if (!blockState2.isOf(Blocks.TRIPWIRE) && k != i) { //If not tripwire & not tripwire that called the update
blockStates[k] = null;
stayAttached = false;
} else {
if (k == i) { //If this is the string that called update, then use cached blockstate
blockState2 = MoreObjects.firstNonNull(blockState, blockState2);
}
boolean disarmed = !blockState2.get(TripwireBlock.DISARMED);
stayPowered |= disarmed && blockState2.get(TripwireBlock.POWERED);
blockStates[k] = blockState2;
if (k == i) {
world.createAndScheduleBlockTick(pos, this, 10);
stayAttached &= disarmed;
}
}
}
stayAttached &= distance > 1; //If another hook was found, facing the opposite direction
stayPowered &= stayAttached; // If there is no gaps between the hooks
//Create blockstate which we might never use V - It's the new state
BlockState blockState3 = this.getDefaultState().with(ATTACHED, Boolean.valueOf(stayAttached)).with(POWERED, Boolean.valueOf(stayPowered));
if (distance > 0) { //If a hook was found, this should not run if nothing has changed between the hooks. Although you do it anyways
BlockPos blockPos = pos.offset(direction, distance);
Direction direction2 = direction.getOpposite();
world.setBlockState(blockPos, blockState3.with(FACING, direction2), Block.NOTIFY_ALL); //Place new hook
this.updateNeighborsOnAxis(world, blockPos, direction2); //Update block around
this.playSound(world, blockPos, stayAttached, stayPowered, attached, powered);
}
this.playSound(world, pos, stayAttached, stayPowered, attached, powered);
if (!beingRemoved) { //Always true for the duping condition. Since the dupe happens due to updates from the other string
world.setBlockState(pos, blockState3.with(FACING, direction), Block.NOTIFY_ALL); //Set this hook to the new state even though it might not have changed
if (update) this.updateNeighborsOnAxis(world, pos, direction); //If it should update. Which is always true for the duping conditions
}
if (attached != stayAttached) { //If it's detaching itself
for(int l = 1; l < distance; ++l) {
BlockPos blockPos2 = pos.offset(direction, l);
BlockState blockState4 = blockStates[l];
if (blockState4 != null) {
world.setBlockState(blockPos2, blockState4.with(ATTACHED, Boolean.valueOf(stayAttached)), Block.NOTIFY_ALL); //Let all blocks to be detached
if (!world.getBlockState(blockPos2).isAir()) {} //Hello there, if the setblock was in here, it would fix MC-129055 but not this dupe
}
}
}
}The bug
The water comes along and breaks all 3 string. Although 1 string was broken first, that string calls the update. The update will loop through the strings, although there are none... Except the block state was cached while it still existed, and there is no check to make sure that it's still there. So it places it back. Secondly, the last check would fix MC-129055, although the reason it does not fix this bug is because it checks for air. Although this block would be water, instead it should be checking to make sure it's a tripwire. Thirdly, I keep mentioning how you are replacing the blocks that did not change state. The reason I am mentioning this is because those blocks might not even exist. Since you are giving updates before that, and shape updates can break tripwire hooks during the call, that's the tripwire hook dupe.
Fix
There are 3 bugs (2 being duplication exploits) in this code, they can be fixed simply. To fix this issue and MC-129055, make the last setBlockState() check if the block is a tripwire, as long as it's the tripwire that called the update.
To fix the tripwire hook dupe, you need to check that the tripwire is still there before doing setBlock on it as long as any updates were given before it. Or, you could move the updates to happen last in order to preserve there states. The latter I don't recommend since it will cause the strings to do another search in some instances.
Fundamental issue
The way that disarming tripwire works currently is by using this mechanic to replace itself. Which is terribly scary. Thankfully there is a way to solve this: you need to tell the method that this tripwire just changed to disarmed. This can be done in onStateReplaced and passed as an argument over to the tripwire hook update() method. Doing so will allow you to skip the check to see if it's still there, while also changing its state to disarmed. This would also allow for greatly simplifying most of the logic for both the tripwire and the tripwire hook, such as not having to schedule a block tick if its not disarming. All other calls to update() should be false for disarming.
@unknown's opinion: I still don't like this idea and think that instead it should be revamped, either by preventing the break and just doing a setBlockState or by moving the update outside of weird calls such as onStateReplaced.
Fixed code
net.minecraft.block.TripwireBlock.java
//Change: Added disarming argument for update()
private void update(World world, BlockPos pos, BlockState blockState, boolean disarming) {
for(Direction direction : new Direction[]{Direction.SOUTH, Direction.WEST}) {
for(int i = 1; i < 42; ++i) {
BlockPos blockPos = pos.offset(direction, i);
BlockState state = world.getBlockState(blockPos);
if (state.isOf(this.hookBlock)) {
if (state.get(TripwireHookBlock.FACING) == direction.getOpposite()) {
//Change: Added disarming
this.hookBlock.update(world, blockPos, state, false, true, disarming, i, blockState);
}
break;
}
if (!state.isOf(this)) break;
}
}
}
public void onStateReplaced(BlockState state, World world, BlockPos pos, BlockState newState, boolean moved) {
if (!moved && !state.isOf(newState.getBlock())) {
this.update(world, pos, state.with(POWERED, true), state.get(DISARMED) && newState.isAir());
}
}net.minecraft.block.TripwireHookBlock.java
//Change: Added disarming argument for update()
public void update(World world, BlockPos pos, BlockState state, boolean beingRemoved, boolean update, boolean disarming, int i, @Nullable BlockState blockState) {
Direction direction = state.get(FACING);
boolean attached = state.get(ATTACHED);
boolean powered = state.get(POWERED);
boolean stayAttached = !beingRemoved;
boolean stayPowered = false;
int distance = 0;
BlockState[] blockStates = new BlockState[42];
for(int k = 1; k < 42; ++k) {
BlockState blockState2 = world.getBlockState(pos.offset(direction, k));
if (blockState2.isOf(Blocks.TRIPWIRE_HOOK)) {
if (blockState2.get(FACING) == direction.getOpposite()) distance = k;
break;
} else if (!blockState2.isOf(Blocks.TRIPWIRE) && k != i) {
blockStates[k] = null;
stayAttached = false;
} else {
if (k == i) blockState2 = MoreObjects.firstNonNull(blockState, blockState2);
boolean disarmed = !blockState2.get(TripwireBlock.DISARMED);
stayPowered |= disarmed && blockState2.get(TripwireBlock.POWERED);
blockStates[k] = blockState2;
if (k == i) {
world.createAndScheduleBlockTick(pos, this, 10);
stayAttached &= disarmed;
}
}
}
stayAttached &= distance > 1;
stayPowered &= stayAttached;
BlockState blockState3 = this.getDefaultState().with(ATTACHED, stayAttached).with(POWERED, stayPowered);
if (distance > 0) {
BlockPos blockPos = pos.offset(direction, distance);
Direction direction2 = direction.getOpposite();
world.setBlockState(blockPos, blockState3.with(FACING, direction2), Block.NOTIFY_ALL);
this.updateNeighborsOnAxis(world, blockPos, direction2); //Possible hook break
this.playSound(world, blockPos, stayAttached, stayPowered, attached, powered);
}
this.playSound(world, pos, stayAttached, stayPowered, attached, powered);
//Change: added state check, due to possible hook break above
if (!beingRemoved && world.getBlockState(pos).isOf(this)) {
world.setBlockState(pos, blockState3.with(FACING, direction), Block.NOTIFY_ALL);
if (update) this.updateNeighborsOnAxis(world, pos, direction);
}
if (attached != stayAttached) {
for(int l = 1; l < distance; ++l) {
BlockState blockState4 = blockStates[l];
if (blockState4 != null) {
BlockPos blockPos2 = pos.offset(direction, l);
//Change: If this is the tripwire that called the update or its disarming or its a tripwire, then place it with the new states
if (l != i || disarming || world.getBlockState(blockPos2).isOf(Blocks.TRIPWIRE)) {
world.setBlockState(blockPos2, blockState4.with(ATTACHED, stayAttached), Block.NOTIFY_ALL);
}
}
}
}
}Shouldn't be fixed, it's a good bug that helps people get geared up faster and build what they want in their worlds, instead of spending hours upon hours grinding. Minecraft is a game about building what you want, not spending all your time gearing up.
I also don't think this bug should be fixed unless a suitable replacement is found like the one for TNT replication bugs
[Mod] markderickson
Confirmed for 14w26c but only when tripwire is connected to tripwire hooks