From 298f3393762c25001c45a49e4209585553ce24fe Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 28 Nov 2021 20:03:27 +0000 Subject: [PATCH] Invalidate peripherals during the computer's tick instead - Capability invalidation and tile/block entity changes set a dirty bit instead of refetching the peripheral immediately. - Then on the block's tick we recompute the peripheral if the dirty bit is set. Fixes #696 and probably fixes #882. Some way towards #893, but not everything yet. This is probably going to break things horribly. Let's find out! --- .../computer/blocks/BlockComputerBase.java | 2 +- .../computer/blocks/TileComputerBase.java | 174 +++++++++--------- .../peripheral/modem/wired/TileCable.java | 15 +- .../modem/wired/TileWiredModemFull.java | 21 ++- .../shared/turtle/blocks/TileTurtle.java | 4 +- .../shared/turtle/core/TurtleBrain.java | 23 +-- .../shared/util/RedstoneUtil.java | 27 +++ 7 files changed, 162 insertions(+), 104 deletions(-) diff --git a/src/main/java/dan200/computercraft/shared/computer/blocks/BlockComputerBase.java b/src/main/java/dan200/computercraft/shared/computer/blocks/BlockComputerBase.java index b9febd5e6..1904484a2 100644 --- a/src/main/java/dan200/computercraft/shared/computer/blocks/BlockComputerBase.java +++ b/src/main/java/dan200/computercraft/shared/computer/blocks/BlockComputerBase.java @@ -54,7 +54,7 @@ public abstract class BlockComputerBase extends Bloc super.onPlace( state, world, pos, oldState, isMoving ); TileEntity tile = world.getBlockEntity( pos ); - if( tile instanceof TileComputerBase ) ((TileComputerBase) tile).updateInput(); + if( tile instanceof TileComputerBase ) ((TileComputerBase) tile).updateInputsImmediately( ); } @Override diff --git a/src/main/java/dan200/computercraft/shared/computer/blocks/TileComputerBase.java b/src/main/java/dan200/computercraft/shared/computer/blocks/TileComputerBase.java index 9afcebff5..1a2a9f07f 100644 --- a/src/main/java/dan200/computercraft/shared/computer/blocks/TileComputerBase.java +++ b/src/main/java/dan200/computercraft/shared/computer/blocks/TileComputerBase.java @@ -19,9 +19,6 @@ import dan200.computercraft.shared.util.DirectionUtil; import dan200.computercraft.shared.util.RedstoneUtil; import joptsimple.internal.Strings; import net.minecraft.block.BlockState; -import net.minecraft.block.Blocks; -import net.minecraft.block.RedstoneDiodeBlock; -import net.minecraft.block.RedstoneWireBlock; import net.minecraft.entity.player.PlayerEntity; import net.minecraft.inventory.container.INamedContainerProvider; import net.minecraft.item.ItemStack; @@ -38,7 +35,6 @@ import net.minecraft.util.math.BlockRayTraceResult; import net.minecraft.util.text.ITextComponent; import net.minecraft.util.text.StringTextComponent; import net.minecraft.util.text.TranslationTextComponent; -import net.minecraft.world.World; import net.minecraftforge.common.util.NonNullConsumer; import javax.annotation.Nonnull; @@ -57,6 +53,8 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT private boolean on = false; boolean startOn = false; private boolean fresh = false; + + private int invalidSides = 0; private final NonNullConsumer[] invalidate; private final ComputerFamily family; @@ -71,7 +69,8 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT NonNullConsumer[] invalidate = this.invalidate = new NonNullConsumer[6]; for( Direction direction : Direction.values() ) { - invalidate[direction.ordinal()] = o -> updateInput( direction ); + int mask = 1 << direction.ordinal(); + invalidate[direction.ordinal()] = o -> invalidSides |= mask; } } @@ -143,45 +142,51 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT @Override public void onNeighbourChange( @Nonnull BlockPos neighbour ) { - updateInput( neighbour ); + updateInputAt( neighbour ); } @Override public void onNeighbourTileEntityChange( @Nonnull BlockPos neighbour ) { - updateInput( neighbour ); + updateInputAt( neighbour ); } @Override public void tick() { - if( !getLevel().isClientSide ) + if( getLevel().isClientSide ) return; + + ServerComputer computer = createServerComputer(); + + if( invalidSides != 0 ) { - ServerComputer computer = createServerComputer(); - if( computer == null ) return; - - // If the computer isn't on and should be, then turn it on - if( startOn || (fresh && on) ) + for( Direction direction : DirectionUtil.FACINGS ) { - computer.turnOn(); - startOn = false; + if( (invalidSides & (1 << direction.ordinal())) != 0 ) refreshPeripheral( computer, direction ); } - - computer.keepAlive(); - - fresh = false; - computerID = computer.getID(); - label = computer.getLabel(); - on = computer.isOn(); - - // Update the block state if needed. We don't fire a block update intentionally, - // as this only really is needed on the client side. - updateBlockState( computer.getState() ); - - // TODO: This should ideally be split up into label/id/on (which should save NBT and sync to client) and - // redstone (which should update outputs) - if( computer.hasOutputChanged() ) updateOutput(); } + + // If the computer isn't on and should be, then turn it on + if( startOn || (fresh && on) ) + { + computer.turnOn(); + startOn = false; + } + + computer.keepAlive(); + + fresh = false; + computerID = computer.getID(); + label = computer.getLabel(); + on = computer.isOn(); + + // Update the block state if needed. We don't fire a block update intentionally, + // as this only really is needed on the client side. + updateBlockState( computer.getState() ); + + // TODO: This should ideally be split up into label/id/on (which should save NBT and sync to client) and + // redstone (which should update outputs) + if( computer.hasOutputChanged() ) updateOutput(); } protected abstract void updateBlockState( ComputerState newState ); @@ -226,89 +231,80 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT return localSide; } - private void updateSideInput( ServerComputer computer, Direction dir, BlockPos offset ) + private void updateRedstoneInput( @Nonnull ServerComputer computer, Direction dir, BlockPos targetPos ) { Direction offsetSide = dir.getOpposite(); ComputerSide localDir = remapToLocalSide( dir ); - computer.setRedstoneInput( localDir, getRedstoneInput( level, offset, dir ) ); - computer.setBundledRedstoneInput( localDir, BundledRedstone.getOutput( getLevel(), offset, offsetSide ) ); - if( !isPeripheralBlockedOnSide( localDir ) ) - { - IPeripheral peripheral = Peripherals.getPeripheral( getLevel(), offset, offsetSide, invalidate[dir.ordinal()] ); - computer.setPeripheral( localDir, peripheral ); - } + computer.setRedstoneInput( localDir, RedstoneUtil.getRedstoneInput( level, targetPos, dir ) ); + computer.setBundledRedstoneInput( localDir, BundledRedstone.getOutput( getLevel(), targetPos, offsetSide ) ); + } + + private void refreshPeripheral( @Nonnull ServerComputer computer, Direction dir ) + { + invalidSides &= ~(1 << dir.ordinal()); + + ComputerSide localDir = remapToLocalSide( dir ); + if( isPeripheralBlockedOnSide( localDir ) ) return; + + Direction offsetSide = dir.getOpposite(); + IPeripheral peripheral = Peripherals.getPeripheral( getLevel(), getBlockPos().relative( dir ), offsetSide, invalidate[dir.ordinal()] ); + computer.setPeripheral( localDir, peripheral ); + } + + public void updateInputsImmediately() + { + ServerComputer computer = getServerComputer(); + if( computer != null ) updateInputsImmediately( computer ); } /** - * Gets the redstone input for an adjacent block. + * Update all redstone and peripherals. * - * @param world The world we exist in - * @param pos The position of the neighbour - * @param side The side we are reading from - * @return The effective redstone power - * @see RedstoneDiodeBlock#calculateInputStrength(World, BlockPos, BlockState) + * This should only be really be called when the computer is being ticked (though there are some cases where it + * won't be), as peripheral scanning requires adjacent tiles to be in a "correct" state - which may not be the case + * if they're still updating! + * + * @param computer The current computer instance. */ - protected static int getRedstoneInput( World world, BlockPos pos, Direction side ) + private void updateInputsImmediately( @Nonnull ServerComputer computer ) { - int power = world.getSignal( pos, side ); - if( power >= 15 ) return power; - - BlockState neighbour = world.getBlockState( pos ); - return neighbour.getBlock() == Blocks.REDSTONE_WIRE - ? Math.max( power, neighbour.getValue( RedstoneWireBlock.POWER ) ) - : power; - } - - public void updateInput() - { - if( getLevel() == null || getLevel().isClientSide ) return; - - // Update all sides - ServerComputer computer = getServerComputer(); - if( computer == null ) return; - - BlockPos pos = computer.getPosition(); + BlockPos pos = getBlockPos(); for( Direction dir : DirectionUtil.FACINGS ) { - updateSideInput( computer, dir, pos.relative( dir ) ); + updateRedstoneInput( computer, dir, pos.relative( dir ) ); + refreshPeripheral( computer, dir ); } } - private void updateInput( BlockPos neighbour ) + private void updateInputAt( @Nonnull BlockPos neighbour ) { - if( getLevel() == null || getLevel().isClientSide ) return; - ServerComputer computer = getServerComputer(); if( computer == null ) return; for( Direction dir : DirectionUtil.FACINGS ) { - BlockPos offset = worldPosition.relative( dir ); + BlockPos offset = getBlockPos().relative( dir ); if( offset.equals( neighbour ) ) { - updateSideInput( computer, dir, offset ); + updateRedstoneInput( computer, dir, offset ); + invalidSides |= 1 << dir.ordinal(); return; } } - // If the position is not any adjacent one, update all inputs. - updateInput(); - } - - private void updateInput( Direction dir ) - { - if( getLevel() == null || getLevel().isClientSide ) return; - - ServerComputer computer = getServerComputer(); - if( computer == null ) return; - - updateSideInput( computer, dir, worldPosition.relative( dir ) ); + // If the position is not any adjacent one, update all inputs. This is pretty terrible, but some redstone mods + // handle this incorrectly. + BlockPos pos = getBlockPos(); + for( Direction dir : DirectionUtil.FACINGS ) updateRedstoneInput( computer, dir, pos.relative( dir ) ); + invalidSides = (1 << 6) - 1; // Mark all peripherals as dirty. } + /** + * Update the block's state and propagate redstone output. + */ public void updateOutput() { - // Update redstone updateBlock(); for( Direction dir : DirectionUtil.FACINGS ) { @@ -358,9 +354,10 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT return family; } + @Nonnull public ServerComputer createServerComputer() { - if( getLevel().isClientSide ) return null; + if( getLevel().isClientSide ) throw new IllegalStateException( "Cannot access server computer on the client." ); boolean changed = false; if( instanceID < 0 ) @@ -368,18 +365,21 @@ public abstract class TileComputerBase extends TileGeneric implements IComputerT instanceID = ComputerCraft.serverComputerRegistry.getUnusedInstanceID(); changed = true; } - if( !ComputerCraft.serverComputerRegistry.contains( instanceID ) ) + + ServerComputer computer = ComputerCraft.serverComputerRegistry.get( instanceID ); + if( computer == null ) { - ServerComputer computer = createComputer( instanceID, computerID ); + computer = createComputer( instanceID, computerID ); ComputerCraft.serverComputerRegistry.add( instanceID, computer ); fresh = true; changed = true; } - if( changed ) updateInput(); - return ComputerCraft.serverComputerRegistry.get( instanceID ); + if( changed ) updateInputsImmediately( computer ); + return computer; } + @Nullable public ServerComputer getServerComputer() { return getLevel().isClientSide ? null : ComputerCraft.serverComputerRegistry.get( instanceID ); diff --git a/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileCable.java b/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileCable.java index 3a0f03ddf..394e55f0d 100644 --- a/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileCable.java +++ b/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileCable.java @@ -77,8 +77,9 @@ public class TileCable extends TileGeneric } } + private boolean invalidPeripheral; private boolean peripheralAccessAllowed; - private final WiredModemLocalPeripheral peripheral = new WiredModemLocalPeripheral( this::refreshPeripheral ); + private final WiredModemLocalPeripheral peripheral = new WiredModemLocalPeripheral( this::queueRefreshPeripheral ); private boolean destroyed = false; @@ -239,12 +240,20 @@ public class TileCable extends TileGeneric if( !level.isClientSide && peripheralAccessAllowed ) { Direction facing = getDirection(); - if( getBlockPos().relative( facing ).equals( neighbour ) ) refreshPeripheral(); + if( getBlockPos().relative( facing ).equals( neighbour ) ) queueRefreshPeripheral(); } } + private void queueRefreshPeripheral() + { + if( invalidPeripheral ) return; + invalidPeripheral = true; + TickScheduler.schedule( this ); + } + private void refreshPeripheral() { + invalidPeripheral = false; if( level != null && !isRemoved() && peripheral.attach( level, getBlockPos(), getDirection() ) ) { updateConnectedPeripherals(); @@ -324,6 +333,8 @@ public class TileCable extends TileGeneric elementCap = CapabilityUtil.invalidate( elementCap ); } + if( invalidPeripheral ) refreshPeripheral(); + if( modem.getModemState().pollChanged() ) updateBlockState(); if( !connectionsFormed ) diff --git a/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileWiredModemFull.java b/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileWiredModemFull.java index e8a2c0728..1f2523fe8 100644 --- a/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileWiredModemFull.java +++ b/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileWiredModemFull.java @@ -108,13 +108,15 @@ public class TileWiredModemFull extends TileGeneric private final NonNullConsumer> connectedNodeChanged = x -> connectionsChanged(); + private int invalidSides = 0; + public TileWiredModemFull( TileEntityType type ) { super( type ); for( int i = 0; i < peripherals.length; i++ ) { Direction facing = Direction.from3DDataValue( i ); - peripherals[i] = new WiredModemLocalPeripheral( () -> refreshPeripheral( facing ) ); + peripherals[i] = new WiredModemLocalPeripheral( () -> queueRefreshPeripheral( facing ) ); } } @@ -173,13 +175,20 @@ public class TileWiredModemFull extends TileGeneric { for( Direction facing : DirectionUtil.FACINGS ) { - if( getBlockPos().relative( facing ).equals( neighbour ) ) refreshPeripheral( facing ); + if( getBlockPos().relative( facing ).equals( neighbour ) ) queueRefreshPeripheral( facing ); } } } + private void queueRefreshPeripheral( @Nonnull Direction facing ) + { + if( invalidSides == 0 ) TickScheduler.schedule( this ); + invalidSides |= 1 << facing.ordinal(); + } + private void refreshPeripheral( @Nonnull Direction facing ) { + invalidSides &= ~(1 << facing.ordinal()); WiredModemLocalPeripheral peripheral = peripherals[facing.ordinal()]; if( level != null && !isRemoved() && peripheral.attach( level, getBlockPos(), facing ) ) { @@ -262,6 +271,14 @@ public class TileWiredModemFull extends TileGeneric { if( getLevel().isClientSide ) return; + if( invalidSides != 0 ) + { + for( Direction direction : DirectionUtil.FACINGS ) + { + if( (invalidSides & (1 << direction.ordinal())) != 0 ) refreshPeripheral( direction ); + } + } + if( modemState.pollChanged() ) updateBlockState(); if( !connectionsFormed ) diff --git a/src/main/java/dan200/computercraft/shared/turtle/blocks/TileTurtle.java b/src/main/java/dan200/computercraft/shared/turtle/blocks/TileTurtle.java index fb89ae33d..7b0ce1994 100644 --- a/src/main/java/dan200/computercraft/shared/turtle/blocks/TileTurtle.java +++ b/src/main/java/dan200/computercraft/shared/turtle/blocks/TileTurtle.java @@ -322,8 +322,10 @@ public class TileTurtle extends TileComputerBase implements ITurtleTile, Default { if( dir.getAxis() == Direction.Axis.Y ) dir = Direction.NORTH; level.setBlockAndUpdate( worldPosition, getBlockState().setValue( BlockTurtle.FACING, dir ) ); + updateOutput(); - updateInput(); + updateInputsImmediately(); + onTileEntityChange(); } diff --git a/src/main/java/dan200/computercraft/shared/turtle/core/TurtleBrain.java b/src/main/java/dan200/computercraft/shared/turtle/core/TurtleBrain.java index 7678f0663..1af843524 100644 --- a/src/main/java/dan200/computercraft/shared/turtle/core/TurtleBrain.java +++ b/src/main/java/dan200/computercraft/shared/turtle/core/TurtleBrain.java @@ -333,16 +333,17 @@ public class TurtleBrain implements ITurtleAccess TileTurtle newTurtle = (TileTurtle) newTile; newTurtle.setLevelAndPosition( world, pos ); newTurtle.transferStateFrom( oldOwner ); - newTurtle.createServerComputer().setWorld( world ); - newTurtle.createServerComputer().setPosition( pos ); + + ServerComputer computer = newTurtle.createServerComputer(); + computer.setWorld( world ); + computer.setPosition( pos ); // Remove the old turtle oldWorld.removeBlock( oldPos, false ); // Make sure everybody knows about it - newTurtle.updateBlock(); - newTurtle.updateInput(); newTurtle.updateOutput(); + newTurtle.updateInputsImmediately(); return true; } } @@ -614,16 +615,16 @@ public class TurtleBrain implements ITurtleAccess @Override public void setUpgrade( @Nonnull TurtleSide side, ITurtleUpgrade upgrade ) { - if( !setUpgradeDirect( side, upgrade ) ) return; + if( !setUpgradeDirect( side, upgrade ) || owner.getLevel() == null ) return; // This is a separate function to avoid updating the block when reading the NBT. We don't need to do this as // either the block is newly placed (and so won't have changed) or is being updated with /data, which calls // updateBlock for us. - if( owner.getLevel() != null ) - { - owner.updateBlock(); - owner.updateInput(); - } + owner.updateBlock(); + + // Recompute peripherals in case an upgrade being removed has exposed a new peripheral. + // TODO: Only update peripherals, or even only two sides? + owner.updateInputsImmediately(); } private boolean setUpgradeDirect( @Nonnull TurtleSide side, ITurtleUpgrade upgrade ) @@ -645,7 +646,7 @@ public class TurtleBrain implements ITurtleAccess if( upgrade != null ) upgrades.put( side, upgrade ); // Notify clients and create peripherals - if( owner.getLevel() != null ) + if( owner.getLevel() != null && !owner.getLevel().isClientSide ) { updatePeripherals( owner.createServerComputer() ); } diff --git a/src/main/java/dan200/computercraft/shared/util/RedstoneUtil.java b/src/main/java/dan200/computercraft/shared/util/RedstoneUtil.java index 41c632dc9..021032d87 100644 --- a/src/main/java/dan200/computercraft/shared/util/RedstoneUtil.java +++ b/src/main/java/dan200/computercraft/shared/util/RedstoneUtil.java @@ -6,6 +6,9 @@ package dan200.computercraft.shared.util; import net.minecraft.block.BlockState; +import net.minecraft.block.Blocks; +import net.minecraft.block.RedstoneDiodeBlock; +import net.minecraft.block.RedstoneWireBlock; import net.minecraft.util.Direction; import net.minecraft.util.math.BlockPos; import net.minecraft.world.World; @@ -15,6 +18,30 @@ import java.util.EnumSet; public final class RedstoneUtil { + private RedstoneUtil() + { + } + + /** + * Gets the redstone input for an adjacent block. + * + * @param world The world we exist in + * @param pos The position of the neighbour + * @param side The side we are reading from + * @return The effective redstone power + * @see RedstoneDiodeBlock#getInputSignal(World, BlockPos, BlockState) + */ + public static int getRedstoneInput( World world, BlockPos pos, Direction side ) + { + int power = world.getSignal( pos, side ); + if( power >= 15 ) return power; + + BlockState neighbour = world.getBlockState( pos ); + return neighbour.getBlock() == Blocks.REDSTONE_WIRE + ? Math.max( power, neighbour.getValue( RedstoneWireBlock.POWER ) ) + : power; + } + public static void propagateRedstoneOutput( World world, BlockPos pos, Direction side ) { // Propagate ordinary output. See BlockRedstoneDiode.notifyNeighbors