Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Force Relay/Lens movement logic for block structures #4510

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Fabric/src/main/resources/botania.accesswidener
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ accessible class net/minecraft/client/renderer/RenderType$CompositeState
extendable method net/minecraft/world/entity/vehicle/AbstractMinecart getDropItem ()Lnet/minecraft/world/item/Item;
extendable class net/minecraft/world/level/levelgen/NoiseBasedChunkGenerator
accessible class net/minecraft/client/resources/model/ModelManager$ReloadState
mutable field net/minecraft/world/level/block/piston/PistonStructureResolver pistonPos Lnet/minecraft/core/BlockPos;
1 change: 1 addition & 0 deletions Forge/src/main/resources/META-INF/accesstransformer.cfg
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
public net.minecraft.client.gui.screens.worldselection.WorldPreset <init>(Ljava/lang/String;)V
public net.minecraft.client.renderer.RenderType$CompositeRenderType
public net.minecraft.core.registries.BuiltInRegistries$RegistryBootstrap
private-f net.minecraft.world.level.block.piston.PistonStructureResolver pistonPos
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import vazkii.botania.api.BotaniaAPI;
import vazkii.botania.common.handler.BotaniaSounds;
import vazkii.botania.common.helper.ForcePushHelper;
import vazkii.botania.common.item.WandOfTheForestItem;
import vazkii.botania.common.item.lens.ForceLens;
import vazkii.botania.network.EffectType;
Expand All @@ -52,7 +53,9 @@ public void onRemove(@NotNull BlockState state, @NotNull Level world, @NotNull B
var data = WorldData.get(world);

if (isMoving && newState.is(Blocks.MOVING_PISTON)) {
var moveDirection = newState.getValue(MovingPistonBlock.FACING);
var pistonDirection = newState.getValue(MovingPistonBlock.FACING);
// if being moved as part of a retracting sticky piston's block structure, reverse movement direction
var moveDirection = ForcePushHelper.isExtendingMovementContext() ? pistonDirection : pistonDirection.getOpposite();

var destPos = data.mapping.get(pos);
if (destPos != null) {
Expand All @@ -67,7 +70,7 @@ public void onRemove(@NotNull BlockState state, @NotNull Level world, @NotNull B

if (newState.getValue(MovingPistonBlock.TYPE) == PistonType.DEFAULT) {
// Move the actual bound blocks
if (ForceLens.moveBlocks(world, destPos.relative(moveDirection.getOpposite()), moveDirection)) {
if (ForceLens.moveBlocks(world, destPos.relative(moveDirection.getOpposite()), moveDirection, pos)) {
// Move dest side of our binding
data.mapping.put(newSrcPos, data.mapping.get(newSrcPos).relative(moveDirection));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,50 @@
package vazkii.botania.common.helper;

import org.apache.commons.lang3.mutable.MutableInt;
import net.minecraft.core.BlockPos;

import java.util.ArrayDeque;
import java.util.Deque;

public class ForcePushHelper implements AutoCloseable {
private static final ThreadLocal<MutableInt> forcePushCounter = ThreadLocal.withInitial(MutableInt::new);
/**
* Keeps track of push origins for nested force relay moves.
*/
private static final ThreadLocal<Deque<BlockPos>> forcePushOriginStack = ThreadLocal.withInitial(ArrayDeque::new);

/**
* Keeps track of nested block push movement types (extending=true or retracting=false).
*/
private static final ThreadLocal<Deque<Boolean>> movementTypeContextStack = ThreadLocal.withInitial(ArrayDeque::new);

public static boolean isForcePush() {
return forcePushCounter.get().intValue() > 0;
return !forcePushOriginStack.get().isEmpty();
}

public static BlockPos getForcePushOrigin() {
if (!isForcePush()) {
throw new IllegalStateException("Not currently performing a Force Relay or Force Lens push");
}
return forcePushOriginStack.get().peek();
}

public static void pushMovementTypeContext(boolean extending) {
movementTypeContextStack.get().push(extending);
}

public static void popMovementTypeContext() {
movementTypeContextStack.get().pop();
}

public static boolean isExtendingMovementContext() {
return movementTypeContextStack.get().peek() == Boolean.TRUE;
}

public ForcePushHelper() {
forcePushCounter.get().increment();
public ForcePushHelper(BlockPos pushLocation) {
forcePushOriginStack.get().push(pushLocation.immutable());
}

@Override
public void close() {
forcePushCounter.get().decrement();
forcePushOriginStack.get().pop();
}
}
20 changes: 16 additions & 4 deletions Xplat/src/main/java/vazkii/botania/common/item/lens/ForceLens.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,28 @@ public boolean collideBurst(ManaBurst burst, HitResult pos, boolean isManaBlock,
&& !burst.isFake()
&& !isManaBlock) {
BlockHitResult rtr = (BlockHitResult) pos;
moveBlocks(entity.level(), rtr.getBlockPos().relative(rtr.getDirection()), rtr.getDirection().getOpposite());
// mana burst could have been warped here, so don't assume that any block is unmovable
moveBlocks(entity.level(), rtr.getBlockPos().relative(rtr.getDirection()), rtr.getDirection().getOpposite(), ManaBurst.NO_SOURCE);
}

return shouldKill;
}

/**
* Executes a force-push of blocks without an actual piston being present.
*
* @param level The level.
* @param impliedPistonPos Position where to push block from.
* @param direction Direction to move block towards from the impliedPistonPos.
* @param pushSourcePos Position of the push source. The block at this position is considered unmovable,
* and usually it would be the pushing piston.
* @return {@code true} if blocks have started moving, or {@code false} if moving blocks failed,
* e.g. due to exceeded push limit or unmovable blocks being in the way.
*/
@SuppressWarnings("try")
public static boolean moveBlocks(Level level, BlockPos pistonPos, Direction direction) {
try (var ignored = new ForcePushHelper()) {
return ((PistonBaseBlockAccessor) Blocks.PISTON).botania_moveBlocks(level, pistonPos, direction, true);
public static boolean moveBlocks(Level level, BlockPos impliedPistonPos, Direction direction, BlockPos pushSourcePos) {
try (var ignored = new ForcePushHelper(pushSourcePos)) {
return ((PistonBaseBlockAccessor) Blocks.PISTON).botania_moveBlocks(level, impliedPistonPos, direction, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ private void preMoveBlocks(Level level, BlockPos pos, Direction dir, boolean ext
CallbackInfoReturnable<Boolean> cir) {
if (!level.isClientSide()) {
EthicalTntHelper.startTrackingTntEntities();
ForcePushHelper.pushMovementTypeContext(extending);
}
}

Expand All @@ -42,6 +43,7 @@ private void preMoveBlocks(Level level, BlockPos pos, Direction dir, boolean ext
private void postMoveBlocks(Level level, BlockPos pos, Direction dir, boolean extending,
CallbackInfoReturnable<Boolean> cir) {
if (!level.isClientSide()) {
ForcePushHelper.popMovementTypeContext();
EthicalTntHelper.endTrackingTntEntitiesAndCheck();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package vazkii.botania.mixin;

import net.minecraft.core.BlockPos;
import net.minecraft.core.Direction;
import net.minecraft.world.level.Level;
import net.minecraft.world.level.block.piston.PistonStructureResolver;

import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.*;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import vazkii.botania.common.helper.ForcePushHelper;

@Mixin(value = PistonStructureResolver.class)
public class PistonStructureResolverMixin {
// not final, due to access widener/transformer definition:
@SuppressWarnings("ShadowModifiers")
@Shadow
private BlockPos pistonPos;

/**
* Since the pushing piston block is handled separately via its position,
* replace it with the force relay's position when pushing blocks that way.
*/
@Inject(method = "<init>", at = @At(value = "RETURN"))
private void modifyForcePushOrigin(Level level, BlockPos pistonPos, Direction pistonDirection, boolean extending, CallbackInfo ci) {
this.pistonPos = ForcePushHelper.isForcePush() ? ForcePushHelper.getForcePushOrigin() : pistonPos;
}
}
117 changes: 114 additions & 3 deletions Xplat/src/main/java/vazkii/botania/test/block/ForceRelayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import net.minecraft.gametest.framework.GameTest;
import net.minecraft.gametest.framework.GameTestHelper;
import net.minecraft.world.level.block.Blocks;
import net.minecraft.world.level.block.state.BlockState;

import vazkii.botania.common.block.BotaniaBlocks;
import vazkii.botania.common.block.ForceRelayBlock;
Expand Down Expand Up @@ -44,9 +45,8 @@ public void testBasic(GameTestHelper helper) {
}).thenExecuteAfter(4, () -> {
var relayPos = initialRelay.south().west();
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, relayPos);
// Andesite should not have moved
helper.assertBlockPresent(Blocks.ANDESITE, initialAndesite.south());
helper.assertBlockPresent(Blocks.SLIME_BLOCK, initialSlimeUnderAndesite.south());
helper.assertBlockPresent(Blocks.ANDESITE, initialAndesite.south().west());
helper.assertBlockPresent(Blocks.SLIME_BLOCK, initialSlimeUnderAndesite.south().west());
helper.assertBlockPresent(Blocks.DIORITE, initialDiorite.south().west());
helper.assertBlockPresent(Blocks.SLIME_BLOCK, initialSlimeUnderDiorite.south().west());

Expand Down Expand Up @@ -76,4 +76,115 @@ public void testImmovable(GameTestHelper helper) {
"binding to the destination block's original position");
}).thenSucceed();
}

@GameTest(template = "botania:block/piston_relay_sticky_move_no_pull")
public void testStickyPistonMoveNotPropagated(GameTestHelper helper) {
final var initialRelay = new BlockPos(1, 2, 2);
final var initialGranite = new BlockPos(3, 2, 3);
final var stickyPistonButton = new BlockPos(1, 2, 0);
final var nonstickyPistonButton = new BlockPos(1, 2, 5);

final var data = ForceRelayBlock.WorldData.get(helper.getLevel());
data.mapping.put(helper.absolutePos(initialRelay), helper.absolutePos(initialGranite));

helper.startSequence()
.thenExecute(() -> helper.pressButton(stickyPistonButton))
.thenExecuteAfter(4, () -> {
// relay should have moved, but not the bound block
final var relayAfter = initialRelay.south();
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, relayAfter);
helper.assertBlockPresent(Blocks.POLISHED_GRANITE, initialGranite);
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(relayAfter)),
helper.absolutePos(initialGranite),
() -> "If relay is moved directly via sticky piston, it should update its source " +
"location, but retain the binding to the destination block's original position");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay)), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
})
.thenExecuteAfter(20, () -> {
// piston should have retracted by now, nothing else should have changed
final var relayAfter = initialRelay.south();
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, relayAfter);
helper.assertBlockPresent(Blocks.POLISHED_GRANITE, initialGranite);
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(relayAfter)),
helper.absolutePos(initialGranite),
() -> "If a sticky piston retracts from the relay, neither the relay nor its bound " +
"location should change");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay)), null,
() -> "If relay is not moved, its location data should not change");
})
.thenExecute(() -> helper.pressButton(nonstickyPistonButton))
.thenExecuteAfter(4, () -> {
// relay should have moved back and also moved the bound block
final var graniteAfter = initialGranite.north();
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, initialRelay);
helper.assertBlockPresent(Blocks.POLISHED_GRANITE, graniteAfter);
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay)),
helper.absolutePos(graniteAfter),
() -> "If relay is moved directly via non-sticky piston, " +
"it should update its source and bound locations");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay.south())), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
})
.thenSucceed();
}

@GameTest(template = "botania:block/piston_relay_structure_pull")
public void testMovingInPistonStructure(GameTestHelper helper) {
final var button = new BlockPos(0, 2, 1);
final var initialRelay1 = new BlockPos(3, 2, 2);
final var initialRelay2 = new BlockPos(3, 2, 5);
final var initialBoundSlime = new BlockPos(2, 2, 4);
final var initialAndesite = new BlockPos(3, 4, 3);

final var data = ForceRelayBlock.WorldData.get(helper.getLevel());
data.mapping.put(helper.absolutePos(initialRelay1), helper.absolutePos(initialBoundSlime));
data.mapping.put(helper.absolutePos(initialRelay2), helper.absolutePos(initialAndesite));

helper.startSequence()
.thenExecute(() -> helper.pressButton(button))
.thenExecuteAfter(4, () -> {
// sticky piston should have moved the block structure with the first relay,
// which should have moved the other block structure with the second relay,
// which should have moved the andesite block
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, initialRelay1.east());
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, initialRelay2.east());
helper.assertBlockState(initialBoundSlime, BlockState::isAir,
() -> "Second slime block structure should have moved, " +
"leaving behind an air block at the originally bound location");
helper.assertBlockPresent(Blocks.POLISHED_ANDESITE, initialAndesite.east());
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay1.east())),
helper.absolutePos(initialBoundSlime.east()),
() -> "Even if the structure was pushed by a sticky piston, the relay and bound " +
"locations should have been updated, since the relay was not pushed directly.");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay2.east())),
helper.absolutePos(initialAndesite.east()),
() -> "If a relay is moved indirectly as part of a block structure, its location and the " +
"bound block's location data should have been updated.");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay1)), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay2)), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
})
.thenExecuteAfter(24, () -> {
// the sticky piston should have retracted by now and everything should have moved back
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, initialRelay1);
helper.assertBlockPresent(BotaniaBlocks.pistonRelay, initialRelay2);
helper.assertBlockPresent(Blocks.SLIME_BLOCK, initialBoundSlime);
helper.assertBlockPresent(Blocks.POLISHED_ANDESITE, initialAndesite);
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay1)),
helper.absolutePos(initialBoundSlime),
() -> "Even if the structure was pushed by a sticky piston, the relay and bound " +
"locations should have been updated, since the relay was not pushed directly.");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay2)),
helper.absolutePos(initialAndesite),
() -> "If a relay is moved indirectly as part of a block structure, its location and the " +
"bound block's location data should have been updated.");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay1.east())), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
TestingUtil.assertEquals(data.mapping.get(helper.absolutePos(initialRelay2.east())), null,
() -> "If relay is moved for whatever reason, its location data should be updated");
})
.thenSucceed();
}
}
1 change: 1 addition & 0 deletions Xplat/src/main/resources/botania_xplat.accesswidener
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ accessible class net/minecraft/client/renderer/RenderType$CompositeRenderType
accessible class net/minecraft/client/renderer/RenderType$CompositeState
extendable method net/minecraft/world/entity/vehicle/AbstractMinecart getDropItem ()Lnet/minecraft/world/item/Item;
extendable class net/minecraft/world/level/levelgen/NoiseBasedChunkGenerator
mutable field net/minecraft/world/level/block/piston/PistonStructureResolver pistonPos Lnet/minecraft/core/BlockPos;
1 change: 1 addition & 0 deletions Xplat/src/main/resources/botania_xplat.mixins.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"NearestAttackableTargetGoalAccessor",
"PistonBaseBlockAccessor",
"PistonBaseBlockMixin",
"PistonStructureResolverMixin",
"PlayerMixin",
"PollinateGoalMixin",
"RecipeManagerAccessor",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
DataVersion: 3465,
size: [5, 3, 6],
data: [
{pos: [1, 1, 0], state: "minecraft:polished_blackstone_button{face:wall,facing:north,powered:false}"},
{pos: [1, 1, 1], state: "minecraft:sticky_piston{extended:false,facing:south}"},
{pos: [1, 1, 2], state: "botania:piston_relay"},
{pos: [1, 1, 4], state: "minecraft:piston{extended:false,facing:north}"},
{pos: [1, 1, 5], state: "minecraft:polished_blackstone_button{face:wall,facing:south,powered:false}"},
{pos: [3, 1, 3], state: "minecraft:polished_granite"}
],
entities: [],
palette: [
"minecraft:sticky_piston{extended:false,facing:south}",
"botania:piston_relay",
"minecraft:piston{extended:false,facing:north}",
"minecraft:polished_granite",
"minecraft:polished_blackstone_button{face:wall,facing:north,powered:false}",
"minecraft:polished_blackstone_button{face:wall,facing:south,powered:false}"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
DataVersion: 3465,
size: [7, 5, 7],
data: [
{pos: [0, 1, 1], state: "minecraft:polished_blackstone_button{face:wall,facing:west,powered:false}"},
{pos: [1, 1, 1], state: "minecraft:sticky_piston{extended:false,facing:east}"},
{pos: [2, 1, 1], state: "minecraft:slime_block"},
{pos: [2, 1, 2], state: "minecraft:polished_diorite"},
{pos: [2, 1, 4], state: "minecraft:slime_block"},
{pos: [2, 1, 5], state: "minecraft:polished_granite"},
{pos: [3, 1, 1], state: "minecraft:slime_block"},
{pos: [3, 1, 2], state: "botania:piston_relay"},
{pos: [3, 1, 4], state: "minecraft:slime_block"},
{pos: [3, 1, 5], state: "botania:piston_relay"},
{pos: [4, 1, 1], state: "minecraft:slime_block"},
{pos: [4, 1, 2], state: "minecraft:polished_diorite"},
{pos: [4, 1, 4], state: "minecraft:slime_block"},
{pos: [4, 1, 5], state: "minecraft:polished_granite"},
{pos: [3, 3, 3], state: "minecraft:polished_andesite"}
],
entities: [],
palette: [
"minecraft:sticky_piston{extended:false,facing:east}",
"minecraft:slime_block",
"minecraft:polished_diorite",
"minecraft:polished_granite",
"botania:piston_relay",
"minecraft:polished_andesite",
"minecraft:polished_blackstone_button{face:wall,facing:west,powered:false}"
]
}
Loading