Skip to content

Commit

Permalink
Ensure block entities are not accidentally recreated during Spatial I…
Browse files Browse the repository at this point in the history 
…O move. Fixes #7513
  • Loading branch information
@shartte
shartte committed Dec 30, 2023
1 parent ccfbfab commit 9722aed
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 21 deletions.
5 changes: 3 additions & 2 deletions src/main/java/appeng/api/movable/DefaultBlockEntityMoveStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ public CompoundTag beginMove(BlockEntity blockEntity) {
}

@Override
public boolean completeMove(BlockEntity blockEntity, CompoundTag savedData, Level newLevel, BlockPos newPosition) {
var be = BlockEntity.loadStatic(newPosition, blockEntity.getBlockState(), savedData);
public boolean completeMove(BlockEntity blockEntity, BlockState state, CompoundTag savedData, Level newLevel,
BlockPos newPosition) {
var be = BlockEntity.loadStatic(newPosition, state, savedData);
if (be != null) {
newLevel.setBlockEntity(be);
return true;
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/appeng/api/movable/IBlockEntityMoveStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import net.minecraft.world.level.Level;
import net.minecraft.world.level.block.entity.BlockEntity;
import net.minecraft.world.level.block.entity.BlockEntityType;
import net.minecraft.world.level.block.state.BlockState;

/**
* A strategy for moving block entities in and out of spatial storage.
Expand Down Expand Up @@ -60,11 +61,13 @@ public interface IBlockEntityMoveStrategy {
*
* @param entity The block entity being moved, which has already been removed from the original chunk and
* should not be reused.
* @param state The original block state of the block entity being moved.
* @param savedData Data saved by this strategy in {@link #beginMove(BlockEntity)}.
* @param newLevel Level to moved to
* @param newPosition Position to move to
* @return True if moving succeeded. If false is returned, AE2 will attempt to recover the original entity.
*/
boolean completeMove(BlockEntity entity, CompoundTag savedData, Level newLevel, BlockPos newPosition);
boolean completeMove(BlockEntity entity, BlockState state, CompoundTag savedData, Level newLevel,
BlockPos newPosition);

}
3 changes: 2 additions & 1 deletion src/main/java/appeng/blockentity/crafting/CraftingBlockEntity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import net.minecraft.world.item.ItemStack;
import net.minecraft.world.item.Items;
import net.minecraft.world.level.BlockGetter;
import net.minecraft.world.level.block.Block;
import net.minecraft.world.level.block.entity.BlockEntityType;
import net.minecraft.world.level.block.state.BlockState;
import net.minecraftforge.client.model.data.ModelData;
Expand Down Expand Up @@ -143,7 +144,7 @@ public void updateSubType(boolean updateFormed) {
// Not using flag 2 here (only send to clients, prevent block update) will cause
// infinite loops
// In case there is an inconsistency in the crafting clusters.
this.level.setBlock(this.worldPosition, newState, 2);
this.level.setBlock(this.worldPosition, newState, Block.UPDATE_CLIENTS);
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/main/java/appeng/blockentity/spatial/SpatialAnchorMoveStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import net.minecraft.world.level.Level;
import net.minecraft.world.level.block.entity.BlockEntity;
import net.minecraft.world.level.block.entity.BlockEntityType;
import net.minecraft.world.level.block.state.BlockState;

import appeng.api.movable.DefaultBlockEntityMoveStrategy;
import appeng.core.definitions.AEBlockEntities;
Expand All @@ -51,8 +52,9 @@ public CompoundTag beginMove(BlockEntity blockEntity) {
}

@Override
public boolean completeMove(BlockEntity blockEntity, CompoundTag savedData, Level newLevel, BlockPos newPosition) {
if (!super.completeMove(blockEntity, savedData, newLevel, newPosition)) {
public boolean completeMove(BlockEntity blockEntity, BlockState state, CompoundTag savedData, Level newLevel,
BlockPos newPosition) {
if (!super.completeMove(blockEntity, state, savedData, newLevel, newPosition)) {
return false;
}
// Notify the new block entity
Expand Down
30 changes: 30 additions & 0 deletions src/main/java/appeng/server/testplots/SpatialTestPlots.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,36 @@ public static void controllerInsideScs(PlotBuilder plot) {
});
}

/**
* Validates that crafting CPUs inside of SCSs do not cause crashes. Regression test for
* <a href="https://github.com/AppliedEnergistics/Applied-Energistics-2/issues/7513">issue 7513</a>.
*/
@TestPlot("crafting_cpu_inside_scs")
public static void craftingCpuInsideScs(PlotBuilder plot) {
// Outer network
plot.creativeEnergyCell("0 0 0");
plot.block("[1,10] 0 0", AEBlocks.SPATIAL_PYLON);
plot.block("0 [1,10] 0", AEBlocks.SPATIAL_PYLON);
plot.block("0 0 [1,10]", AEBlocks.SPATIAL_PYLON);
plot.blockEntity("-1 0 0", AEBlocks.SPATIAL_IO_PORT, port -> {
port.getInternalInventory().insertItem(0, AEItems.SPATIAL_CELL128.stack(), false);
});
var leverPos = plot.leverOn(new BlockPos(-1, 0, 0), Direction.WEST);

// Inner network
plot.creativeEnergyCell("3 0 3");
plot.block("[2,4] [1,3] [2,4]", AEBlocks.CRAFTING_STORAGE_64K);

// Woosh!
plot.test(helper -> {
helper.startSequence()
.thenIdle(5)
.thenExecute(() -> helper.pullLever(leverPos))
.thenIdle(5)
.thenSucceed();
});
}

/**
* Tests that entities can be stored and retrieved from spatial I/O without a player being present and loading the
* chunks. <a href="https://github.com/AppliedEnergistics/Applied-Energistics-2/issues/6397">issue 6397</a>.
Expand Down
54 changes: 39 additions & 15 deletions src/main/java/appeng/spatial/CachedPlane.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import net.minecraft.world.ticks.LevelChunkTicks;
import net.minecraft.world.ticks.ScheduledTick;

import it.unimi.dsi.fastutil.ints.Int2ObjectMap;

import appeng.api.ids.AETags;
import appeng.api.movable.BlockEntityMoveStrategies;
import appeng.api.movable.IBlockEntityMoveStrategy;
Expand Down Expand Up @@ -126,20 +128,24 @@ public CachedPlane(ServerLevel level, int minX, int minY, int minZ, int maxX,

var strategy = BlockEntityMoveStrategies.get(blockEntity);
var savedData = strategy.beginMove(blockEntity);
var section = c.getSection(c.getSectionIndex(entry.getKey().getY()));

// Coordinate within the section
int sx = entry.getKey().getX() & (LevelChunkSection.SECTION_WIDTH - 1);
int sy = entry.getKey().getY() & (LevelChunkSection.SECTION_HEIGHT - 1);
int sz = entry.getKey().getZ() & (LevelChunkSection.SECTION_WIDTH - 1);
var state = section.getBlockState(sx, sy, sz);

if (savedData != null) {
this.blockEntities.add(new BlockEntityMoveRecord(strategy, blockEntity, savedData));
this.blockEntities.add(
new BlockEntityMoveRecord(strategy, blockEntity, savedData, entry.getKey(), state));

// Set the state to AIR now since that prevents it from being resurrected recursively
section.setBlockState(sx, sy, sz, Blocks.AIR.defaultBlockState());
c.removeBlockEntity(entry.getKey());
} else {
final BlockStorageData details = new BlockStorageData();
Column column = this.myColumns[pos.getX() - minX][pos.getZ() - minZ];
final int y = pos.getY();
final LevelChunkSection[] storage = column.c.getSections();
final LevelChunkSection extendedblockstorage = storage[y >> 4];

details.state = extendedblockstorage.getBlockState(column.x, y & 15, column.z);

// don't skip air, just let the code replace it...
if (details.state.isAir()) {
if (state.isAir()) {
level.removeBlock(pos, false);
} else {
this.myColumns[pos.getX() - minX][pos.getZ() - minZ].setSkip(pos.getY());
Expand All @@ -158,6 +164,7 @@ public CachedPlane(ServerLevel level, int minX, int minY, int minZ, int maxX,
});
}
}

}

void swap(CachedPlane dst) {
Expand Down Expand Up @@ -251,19 +258,33 @@ private void addTick(BlockPos pos, ScheduledTick<Block> tick) {

private void addBlockEntity(int x, int y, int z, BlockEntityMoveRecord moveRecord) {
try {
var originalPos = moveRecord.pos();

var c = this.myColumns[x][z];
if (!c.doNotSkip(y + this.y_offset)) {
AELog.warn(
"Block entity %s was queued to be moved from %s, but it's position then skipped during the move.",
moveRecord.blockEntity(), moveRecord.blockEntity().getBlockPos());
moveRecord.blockEntity(), originalPos);
return;
}

var newPosition = new BlockPos(x + this.x_offset, y + this.y_offset, z + this.z_offset);

// Restore original block state to the section directly. The chunk would create the BE and notify neighbors.
var chunk = this.level.getChunk(newPosition);
var section = chunk.getSection(chunk.getSectionIndex(newPosition.getY()));
section.setBlockState(
newPosition.getX() & (LevelChunkSection.SECTION_WIDTH - 1),
newPosition.getY() & (LevelChunkSection.SECTION_HEIGHT - 1),
newPosition.getZ() & (LevelChunkSection.SECTION_WIDTH - 1),
moveRecord.state);

var strategy = moveRecord.strategy();
boolean success;
try {
success = strategy.completeMove(moveRecord.blockEntity(), moveRecord.savedData(), this.level,
new BlockPos(x + this.x_offset, y + this.y_offset, z + this.z_offset));
success = strategy.completeMove(moveRecord.blockEntity(), moveRecord.state(), moveRecord.savedData(),
this.level,
newPosition);
} catch (Throwable e) {
AELog.warn(e);
success = false;
Expand Down Expand Up @@ -346,6 +367,7 @@ private static class Column {

private final LevelChunk c;
private List<Integer> skipThese = null;
private Int2ObjectMap<BlockState> savedBlockStates = null;

public Column(LevelChunk chunk, int x, int z) {
this.x = x;
Expand Down Expand Up @@ -374,10 +396,12 @@ public LevelChunkSection getSection(int y) {
}
}

private static record BlockEntityMoveRecord(
private record BlockEntityMoveRecord(
IBlockEntityMoveStrategy strategy,
BlockEntity blockEntity,
CompoundTag savedData) {
CompoundTag savedData,
BlockPos pos,
BlockState state) {
}

}

0 comments on commit 9722aed

Please sign in to comment.