diff --git a/api/src/main/java/mrtjp/projectred/api/BlockMover.java b/api/src/main/java/mrtjp/projectred/api/BlockMover.java index 8e1620224..a2e8c6e01 100755 --- a/api/src/main/java/mrtjp/projectred/api/BlockMover.java +++ b/api/src/main/java/mrtjp/projectred/api/BlockMover.java @@ -1,14 +1,24 @@ package mrtjp.projectred.api; -import net.minecraft.core.Direction; import net.minecraft.core.BlockPos; +import net.minecraft.core.Direction; import net.minecraft.world.level.Level; /** * Interface for an object that manages the movement of blocks and tiles that are * registered to it. This class should be registered through the {@link IExpansionAPI}. + *

+ * The order of calls are: + *

    + *
  1. Server receives {@link #canMove(Level, BlockPos)}
    2. + *
  2. Server receives {@link #move(Level, BlockPos, Direction)}
    3. + *
  3. Client receives {@link #move(Level, BlockPos, Direction)}
    4. + *
  4. Client receives {@link #postMove(Level, BlockPos)}
    5. + *
  5. Server receives {@link #postMove(Level, BlockPos)}
    6. + *
*/ public interface BlockMover { + /** * Used to check if the block at the given position can move. This * method is only called if the specified block is tied to this @@ -16,38 +26,45 @@ public interface BlockMover { * for this handler. This is called before actually moving. If * everything is able to move, an animation will start. *

- * Called server-side only when determining what to move. + * Called server-side only. * - * @param w The world. - * @param pos The position of the block to move. + * @param w The world. + * @param pos The position of the block before movement. * @return True if the block at the given position is able to move. */ boolean canMove(Level w, BlockPos pos); /** * Method used to actually move the tile. Called after the animation - * has run. This is where you should tell the tile that it is time - * to move, and peform any extra checks or calls. This should also - * move the block and tile as well. This is called - * on every block in the moving structure sequentially. + * has run. This is where you should tell the tile that it is time + * to move, and perform any extra checks or calls. This should also + * move the block and tile as well. This is called on every block in + * the moving structure sequentially. *

- * Called on both server and client. + * Called on server then client. Note that after server call, block + * positions of client/server are out of sync. Do not send packets + * to client referencing blocks by position, update neighbors, mark + * chunk for re-render, etc. * * @param w The world. - * @param pos The position of the block to move. + * @param pos The position of the block before movement. * @param dir The ForgeDirection the structure is moving in. */ void move(Level w, BlockPos pos, Direction dir); /** - * Called after all blocks in the group have moved to their - * new locations. This is where you would reload your tile, - * tell it to refresh or reacknowledge its new position. + * Called after all blocks in the group have moved to their new locations. + * This is where you would reload your tile, tell it to refresh or + * re-acknowledge its new position. No need to update neighbors, since + * they are batch-notified after all blocks (moved and adjacent to moved) + * get postMove call. *

- * Called on both server and client. + * Called on client then server. At this point, both client and server + * blocks are in sync at new positions. It is safe to send packets + * with position references. * - * @param w The world. - * @param pos The position of the block to move. + * @param w The world. + * @param pos The position of the block after movement. */ void postMove(Level w, BlockPos pos); } diff --git a/api/src/main/java/mrtjp/projectred/api/MovementDescriptor.java b/api/src/main/java/mrtjp/projectred/api/MovementDescriptor.java index 14202b691..94fa72d18 100644 --- a/api/src/main/java/mrtjp/projectred/api/MovementDescriptor.java +++ b/api/src/main/java/mrtjp/projectred/api/MovementDescriptor.java @@ -13,21 +13,30 @@ public interface MovementDescriptor { enum MovementStatus { /** - * Failed to begin movement (collision, unmovable block in structure, etc) + * Movement pending start. Default state when first created */ - FAILED, + PENDING_START, /** - * Movement in progress + * Movement animation in progress while block remains in initial position */ MOVING, /** - * Movement finished successfully + * Movement animation has completed but still awaiting execution of movement. + * Block is still in initial position. + */ + PENDING_FINALIZATION, + /** + * Movement finished successfully and block is in final position */ FINISHED, /** * Movement cancelled before completion (chunk was unloaded, etc) */ CANCELLED, + /** + * Failed to begin movement (collision, unmovable block in structure, etc) + */ + FAILED, /** * Unknown status */ diff --git a/core/src/main/java/mrtjp/projectred/core/tile/IConnectableTile.java b/core/src/main/java/mrtjp/projectred/core/tile/IConnectableTile.java index 28eaef819..bf6e5d72f 100644 --- a/core/src/main/java/mrtjp/projectred/core/tile/IConnectableTile.java +++ b/core/src/main/java/mrtjp/projectred/core/tile/IConnectableTile.java @@ -6,9 +6,9 @@ import codechicken.multipart.block.TileMultipart; import codechicken.multipart.util.PartMap; import mrtjp.projectred.api.IConnectable; +import mrtjp.projectred.core.CenterLookup; import net.minecraft.core.BlockPos; import net.minecraft.core.Direction; -import net.minecraft.world.level.block.entity.BlockEntity; import net.minecraft.world.level.block.state.BlockState; import javax.annotation.Nullable; @@ -208,9 +208,9 @@ default boolean discoverCorner(int s, int edgeRot) { } default boolean discoverStraightCenterOverride(int s) { - BlockPos pos = posOfStraight(s); - BlockEntity tile = getBlockLevel().getBlockEntity(pos); - if (tile instanceof IConnectable connectable) { + CenterLookup lookup = CenterLookup.lookupStraightCenter(getBlockLevel(), getBlockPosition(), s); + + if (lookup.tile instanceof IConnectable connectable) { return canConnectPart(connectable, s, -1) && connectable.connectStraight(this, s, -1); } diff --git a/expansion/src/main/java/mrtjp/projectred/expansion/MovementManager.java b/expansion/src/main/java/mrtjp/projectred/expansion/MovementManager.java index 6a514bf6c..4c82d1335 100644 --- a/expansion/src/main/java/mrtjp/projectred/expansion/MovementManager.java +++ b/expansion/src/main/java/mrtjp/projectred/expansion/MovementManager.java @@ -40,6 +40,7 @@ import java.util.*; import java.util.function.Consumer; +import static mrtjp.projectred.api.MovementDescriptor.MovementStatus.*; import static mrtjp.projectred.expansion.ProjectRedExpansion.LOGGER; public class MovementManager { @@ -85,16 +86,20 @@ public static void onChunkUnwatchEvent(ChunkWatchEvent.UnWatch event) { } public static void onChunkUnloadEvent(ChunkEvent.Unload event) { +// LOGGER.debug("Chunk {} unloaded", event.getLevel()); if (event.getLevel() instanceof Level level) { getInstance(level).cancelMovementsInChunk(level, event.getChunk().getPos()); } } public static void onLevelUnload(LevelEvent.Unload event) { - // Note: Client unloads levels as player changes dimensions, but + // Note: Client unloads levels as player changes dimensions or leaves server, but // server appears to always have all dims loaded and unloads only // on shutdown LOGGER.debug("Level {} unloaded", event.getLevel()); + if (event.getLevel() instanceof Level level) { + getInstance(level).cancelMovementsOnUnload(level); + } } public static void onLevelLoad(LevelEvent.Load event) { @@ -186,6 +191,16 @@ private void removeChunkWatcher(ChunkPos pos, ServerPlayer player) { if (watchingPlayersSet != null) watchingPlayersSet.remove(pos); } + private void cancelMovementsOnUnload(Level level) { + // Note: Call this on both sides + LOGGER.debug("Cancelling {} movements on level {} unload", structures.size(), level); + for (var structure : structures.values()) { + structure.cancelMove(level); // prob doesn't matter at this point + } + structures.clear(); + nextStructureId = 0; + } + private void cancelMovementsInChunk(Level level, ChunkPos pos) { if (level.isClientSide) { return; @@ -220,7 +235,7 @@ private void tick(Level level) { for (var e : newWatchers.entrySet()) { ServerPlayer player = e.getKey(); Set posSet = e.getValue(); - LOGGER.debug("Sending descriptions to player {} for {} chunks", player.getName().getString(), posSet.size()); +// LOGGER.debug("Sending descriptions to player {} for {} chunks", player.getName().getString(), posSet.size()); sendDescriptionsOnWatch(player, posSet); // Promote player to a watcher watchingPlayers.computeIfAbsent(player, p -> new HashSet<>()).addAll(posSet); @@ -231,12 +246,18 @@ private void tick(Level level) { List removed = new LinkedList<>(); for (var e : structures.entrySet()) { MovingStructure structure = e.getValue(); - if (structure.isFinished()) { + if (structure.getStatus() == PENDING_FINALIZATION) { LOGGER.debug("Executing move {}", structure.toString()); - // Execute move - structure.executeMove(level); - // Tell client + + // Execute pre-move, which does silent block modifications + structure.executePreMove(level); + + // Tell client to execute both pre-move and post-move sendExecuteMove(structure); + + // Execute post-move. Block updates can be done here since client has moved blocks already + structure.executePostMove(level); + // Remove removed.add(e.getKey()); } @@ -264,13 +285,13 @@ public MovementDescriptor beginMove(Level level, Set blocks, int dir, MovingStructure structure = new MovingStructure(getNextStructureId(), speed, dir, movingRows); if (!structure.canMove(level)) return structure; - // Add structure and begin move + // Add structure and send to client structures.put(structure.id, structure); - structure.beginMove(level); - - // Send to client sendNewStructureDescription(structure); + // Begin move (client does this when structure received from above call) + structure.beginMove(level); + return structure; } @@ -322,16 +343,22 @@ private void readNewStructure(MCDataInput input, Level level) { } private void readStructureExecution(MCDataInput input, Level level) { - // Generate a new structure in case it was stale on client - MovingStructure structure = MovingStructure.fromDesc(input); - - // Execute move - structure.executeMove(level); + int id = input.readUShort(); + var structure = structures.get(id); - // Delete struct that *should* have been on the client - if (structures.remove(structure.id) == null) { - LOGGER.warn("Move executed for unknown structure id {}", structure.id); + // Client would have received this structure already + if (structure == null) { + LOGGER.error("Pre-move executed for unknown structure id {}. Adding it for post-move.", id); + return; } + + // Execute pre-move and post-move operations. Server has only done pre-move so far. + // It will do post-move after the client. + structure.executePreMove(level); + structure.executePostMove(level); + + // Full movement complete on client-side. Remove structure + structures.remove(id); } private void readStructureCancellation(MCDataInput input, Level level) { @@ -374,7 +401,7 @@ private void sendNewStructureDescription(MovingStructure structure) { private void sendExecuteMove(MovingStructure structure) { PacketCustom packet = createPacket(KEY_EXECUTE_MOVE); - structure.writeDesc(packet); + packet.writeShort(structure.id); for (ServerPlayer player : playersWatchingStructure(structure)) { packet.sendToPlayer(player); @@ -429,21 +456,21 @@ private static class MovingStructure implements InternalMovementInfo { private final LazyValue> intersectingChunks = new LazyValue<>(this::computeIntersectingChunks); private final LazyValue> renderChunks = new LazyValue<>(this::computeRenderChunks); - private boolean started = false; - private boolean cancelled = false; + private MovementStatus status; private double progress; - public MovingStructure(int id, double speed, int dir, List rows, double progress) { + public MovingStructure(int id, double speed, int dir, List rows, MovementStatus status, double progress) { this.id = id; this.speed = speed; this.dir = dir; this.rows = Collections.unmodifiableList(rows); + this.status = status; this.progress = progress; this.totalSize = FastStream.of(rows).intSum(r -> r.size); } public MovingStructure(int id, double speed, int dir, List rows) { - this(id, speed, dir, rows, 0D); + this(id, speed, dir, rows, PENDING_START, 0D); } //region Network @@ -456,6 +483,7 @@ public void writeDesc(MCDataOutput output) { output.writePos(row.pos); output.writeShort(row.size); } + output.writeByte(status.ordinal()); output.writeDouble(progress); //TODO use integers instead } @@ -468,24 +496,22 @@ public static MovingStructure fromDesc(MCDataInput input) { for (int i = 0; i < size; i++) { rows.add(new MovingRow(input.readPos(), dir, input.readUShort())); } + MovementStatus status = MovementStatus.values()[input.readUByte()]; double progress = input.readDouble(); - return new MovingStructure(id, speed, dir, rows, progress); + return new MovingStructure(id, speed, dir, rows, status, progress); } //endregion //region Movement description @Override public MovementStatus getStatus() { - if (!started) return MovementStatus.FAILED; - if (cancelled) return MovementStatus.CANCELLED; - if (progress >= 1D) return MovementStatus.FINISHED; - return MovementStatus.MOVING; + return status; } @Override public boolean isMoving() { - return getStatus() == MovementStatus.MOVING; + return getStatus() == MOVING || getStatus() == PENDING_FINALIZATION; } @Override @@ -500,7 +526,8 @@ public int getSize() { @Override public Vector3 getRenderOffset(float partialTicks) { - return Vector3.fromBlockPos(BlockPos.ZERO.relative(Direction.values()[dir])).multiply(progress + speed * partialTicks); + double p = Math.min(progress + speed * partialTicks, 1D); + return Vector3.fromBlockPos(BlockPos.ZERO.relative(Direction.values()[dir])).multiply(p); } //endregion @@ -520,12 +547,18 @@ public boolean contains(BlockPos pos) { } public void tickProgress(Level level) { - progress = Math.min(progress + speed, 1D); - FastStream.of(rows).forEach(r -> r.pushEntities(level, progress)); - } - public boolean isFinished() { - return progress >= 1D; + // Should not be ticking progress otherwise + assert status == MOVING || status == PENDING_FINALIZATION; + + if (status == MOVING) { + progress = Math.min(progress + speed, 1D); + FastStream.of(rows).forEach(r -> r.pushEntities(level, progress)); + + if (progress >= 1D) { + status = PENDING_FINALIZATION; + } + } } public boolean canMove(Level level) { @@ -536,7 +569,9 @@ public boolean canMove(Level level) { } public void beginMove(Level level) { - started = true; + assert status == MovementStatus.PENDING_START; + status = MOVING; + FastStream.of(rows).forEach(r -> r.beginMove(level)); if (level.isClientSide) { @@ -545,10 +580,13 @@ public void beginMove(Level level) { } } - public void executeMove(Level level) { - - // Run movement phases + public void executePreMove(Level level) { + // Silently moves blocks to new position FastStream.of(rows).forEach(r -> r.moveBlocks(level)); + } + + public void executePostMove(Level level) { + // Completes the movement by alerting the tile itself, etc FastStream.of(rows).forEach(r -> r.postMove(level)); FastStream.of(rows).forEach(r -> r.endMove(level)); @@ -572,11 +610,14 @@ public void executeMove(Level level) { } //TODO Tick rescheduling + status = FINISHED; } public void cancelMove(Level level) { // Shouldn't need to do anything. Nothing happens until the animation is finished - cancelled = true; + // TODO MovementController notification for this? + assert status == MOVING || status == PENDING_FINALIZATION; + status = CANCELLED; } @OnlyIn(Dist.CLIENT) @@ -662,6 +703,10 @@ public boolean canMove(Level level) { Iterator it = iteratePreMove(); while (it.hasNext()) { BlockPos pos = it.next(); + + BlockMover mover = MovementRegistry.getMover(level, pos); + if (!mover.canMove(level, pos)) return false; + MovementController controller = MovementRegistry.getMovementController(level, pos); // Allow hooks to conditionally block movement if (controller != null && !controller.isMovable(level, pos, Direction.values()[dir])) return false;