From 5d8c46c7e61117ede5d6315efc81a4ae498f34ba Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 17 Mar 2024 12:21:21 +0000 Subject: [PATCH] Replace integer instance IDs with UUIDs Here's a fun bug you can try at home: - Create a new world - Spawn in a pocket computer, turn it on, and place it in a chest. - Reload the world - the pocket computer in the chest should now be off. - Spawn in a new pocket computer, and turn it on. The computer in chest will also appear to be on! This bug has been present since pocket computers were added (27th March, 2024). When a pocket computer is added to a player's inventory, it is assigned a unique *per-session* "instance id" , which is used to find the associated computer. Note the "per-session" there - these ids will be reused if you reload the world (or restart the server). In the above bug, we see the following: - The first pocket computer is assigned an instance id of 0. - After reloading, the second pocket computer is assigned an instance id of 0. - If the first pocket computer was in our inventory, it'd be ticked and assigned a new instance id. However, because it's in an inventory, it keeps its old one. - Both computers look up their client-side computer state and get the same value, meaning the first pocket computer mirrors the second! To fix this, we now ensure instance ids are entirely unique (not just per-session). Rather than sequentially assigning an int, we now use a random UUID (we probably could get away with a random long, but this feels more idiomatic). This has a couple of user-visible changes: - /computercraft no longer lists instance ids outside of dumping an individual computer. - The @c[instance=...] selector uses UUIDs. We still use int instance ids for the legacy selector, but that'll be removed in a later MC version. - Pocket computers now store a UUID rather than an int. Related to this change (I made this change first, but then they got kinda mixed up together), we now only create PocketComputerData when receiving server data. This makes the code a little uglier in some places (the data may now be null), but means we don't populate the client-side pocket computer map with computers the server doesn't know about. --- .../cc-tweaked.java-convention.gradle.kts | 2 + .../computercraft/client/ClientRegistry.java | 23 ++++----- .../platform/ClientNetworkContextImpl.java | 8 ++- .../client/pocket/ClientPocketComputers.java | 44 +++++++++++------ .../client/pocket/PocketComputerData.java | 23 ++++----- .../client/render/PocketItemRenderer.java | 26 ++++++---- .../shared/command/CommandComputerCraft.java | 15 ++---- .../command/arguments/ComputerSelector.java | 33 +++++++++---- .../shared/command/text/ChatHelpers.java | 13 +++++ .../blocks/AbstractComputerBlockEntity.java | 9 ++-- .../shared/computer/core/ServerComputer.java | 14 ++++-- .../computer/core/ServerComputerRegistry.java | 49 ++++++++++++------- .../computer/inventory/ViewComputerMenu.java | 2 +- .../network/client/ClientNetworkContext.java | 4 +- .../client/PocketComputerDataMessage.java | 12 +++-- .../PocketComputerDeletedClientMessage.java | 10 ++-- .../pocket/core/PocketServerComputer.java | 2 +- .../pocket/items/PocketComputerItem.java | 21 +++----- .../arguments/ComputerSelectorTest.java | 23 ++++----- .../gametest/Pocket_Computer_Test.kt | 4 +- 20 files changed, 196 insertions(+), 141 deletions(-) diff --git a/buildSrc/src/main/kotlin/cc-tweaked.java-convention.gradle.kts b/buildSrc/src/main/kotlin/cc-tweaked.java-convention.gradle.kts index 981e16de8..ab037b14e 100644 --- a/buildSrc/src/main/kotlin/cc-tweaked.java-convention.gradle.kts +++ b/buildSrc/src/main/kotlin/cc-tweaked.java-convention.gradle.kts @@ -113,6 +113,8 @@ sourceSets.all { option("NullAway:CastToNonNullMethod", "dan200.computercraft.core.util.Nullability.assertNonNull") option("NullAway:CheckOptionalEmptiness") option("NullAway:AcknowledgeRestrictiveAnnotations") + + excludedPaths = ".*/jmh_generated/.*" } } } diff --git a/projects/common/src/client/java/dan200/computercraft/client/ClientRegistry.java b/projects/common/src/client/java/dan200/computercraft/client/ClientRegistry.java index d319f2e90..286343dbe 100644 --- a/projects/common/src/client/java/dan200/computercraft/client/ClientRegistry.java +++ b/projects/common/src/client/java/dan200/computercraft/client/ClientRegistry.java @@ -22,6 +22,7 @@ import dan200.computercraft.shared.ModRegistry; import dan200.computercraft.shared.command.CommandComputerCraft; import dan200.computercraft.shared.common.IColouredItem; +import dan200.computercraft.shared.computer.core.ComputerState; import dan200.computercraft.shared.computer.core.ServerContext; import dan200.computercraft.shared.computer.inventory.AbstractComputerMenu; import dan200.computercraft.shared.computer.inventory.ViewComputerMenu; @@ -91,7 +92,10 @@ public static void registerMainThread() { MenuScreens.>register(ModRegistry.Menus.VIEW_COMPUTER.get(), ComputerScreen::new); registerItemProperty("state", - new UnclampedPropertyFunction((stack, world, player, random) -> ClientPocketComputers.get(stack).getState().ordinal()), + new UnclampedPropertyFunction((stack, world, player, random) -> { + var computer = ClientPocketComputers.get(stack); + return (computer == null ? ComputerState.OFF : computer.getState()).ordinal(); + }), ModRegistry.Items.POCKET_COMPUTER_NORMAL, ModRegistry.Items.POCKET_COMPUTER_ADVANCED ); registerItemProperty("coloured", @@ -155,17 +159,14 @@ public static void registerItemColours(BiConsumer register) } private static int getPocketColour(ItemStack stack, int layer) { - switch (layer) { - case 0: - default: - return 0xFFFFFF; - case 1: // Frame colour - return IColouredItem.getColourBasic(stack); - case 2: { // Light colour - var light = ClientPocketComputers.get(stack).getLightState(); - return light == -1 ? Colour.BLACK.getHex() : light; + return switch (layer) { + default -> 0xFFFFFF; + case 1 -> IColouredItem.getColourBasic(stack); // Frame colour + case 2 -> { // Light colour + var computer = ClientPocketComputers.get(stack); + yield computer == null || computer.getLightState() == -1 ? Colour.BLACK.getHex() : computer.getLightState(); } - } + }; } private static int getTurtleColour(ItemStack stack, int layer) { diff --git a/projects/common/src/client/java/dan200/computercraft/client/platform/ClientNetworkContextImpl.java b/projects/common/src/client/java/dan200/computercraft/client/platform/ClientNetworkContextImpl.java index ebfe918c6..2f1708e90 100644 --- a/projects/common/src/client/java/dan200/computercraft/client/platform/ClientNetworkContextImpl.java +++ b/projects/common/src/client/java/dan200/computercraft/client/platform/ClientNetworkContextImpl.java @@ -67,14 +67,12 @@ public void handlePlayRecord(BlockPos pos, @Nullable SoundEvent sound, @Nullable } @Override - public void handlePocketComputerData(int instanceId, ComputerState state, int lightState, TerminalState terminal) { - var computer = ClientPocketComputers.get(instanceId, terminal.colour); - computer.setState(state, lightState); - if (terminal.hasTerminal()) computer.setTerminal(terminal); + public void handlePocketComputerData(UUID instanceId, ComputerState state, int lightState, TerminalState terminal) { + ClientPocketComputers.setState(instanceId, state, lightState, terminal); } @Override - public void handlePocketComputerDeleted(int instanceId) { + public void handlePocketComputerDeleted(UUID instanceId) { ClientPocketComputers.remove(instanceId); } diff --git a/projects/common/src/client/java/dan200/computercraft/client/pocket/ClientPocketComputers.java b/projects/common/src/client/java/dan200/computercraft/client/pocket/ClientPocketComputers.java index 1905e2af4..120f9d3ea 100644 --- a/projects/common/src/client/java/dan200/computercraft/client/pocket/ClientPocketComputers.java +++ b/projects/common/src/client/java/dan200/computercraft/client/pocket/ClientPocketComputers.java @@ -4,21 +4,26 @@ package dan200.computercraft.client.pocket; -import dan200.computercraft.shared.computer.core.ComputerFamily; +import dan200.computercraft.shared.computer.core.ComputerState; import dan200.computercraft.shared.computer.core.ServerComputer; +import dan200.computercraft.shared.computer.terminal.NetworkedTerminal; +import dan200.computercraft.shared.computer.terminal.TerminalState; import dan200.computercraft.shared.network.client.PocketComputerDataMessage; import dan200.computercraft.shared.pocket.items.PocketComputerItem; -import it.unimi.dsi.fastutil.ints.Int2ObjectMap; -import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import net.minecraft.world.item.ItemStack; +import javax.annotation.Nullable; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; + /** - * Maps {@link ServerComputer#getInstanceID()} to locals {@link PocketComputerData}. + * Maps {@link ServerComputer#getInstanceUUID()} to locals {@link PocketComputerData}. *

* This is populated by {@link PocketComputerDataMessage} and accessed when rendering pocket computers */ public final class ClientPocketComputers { - private static final Int2ObjectMap instances = new Int2ObjectOpenHashMap<>(); + private static final Map instances = new HashMap<>(); private ClientPocketComputers() { } @@ -27,25 +32,32 @@ public static void reset() { instances.clear(); } - public static void remove(int id) { + public static void remove(UUID id) { instances.remove(id); } /** - * Get or create a pocket computer. + * Set the state of a pocket computer. * - * @param instanceId The instance ID of the pocket computer. - * @param advanced Whether this computer has an advanced terminal. - * @return The pocket computer data. + * @param instanceId The instance ID of the pocket computer. + * @param state The computer state of the pocket computer. + * @param lightColour The current colour of the modem light. + * @param terminalData The current terminal contents. */ - public static PocketComputerData get(int instanceId, boolean advanced) { + public static void setState(UUID instanceId, ComputerState state, int lightColour, TerminalState terminalData) { var computer = instances.get(instanceId); - if (computer == null) instances.put(instanceId, computer = new PocketComputerData(advanced)); - return computer; + if (computer == null) { + var terminal = new NetworkedTerminal(terminalData.width, terminalData.height, terminalData.colour); + instances.put(instanceId, computer = new PocketComputerData(state, lightColour, terminal)); + } else { + computer.setState(state, lightColour); + } + + if (terminalData.hasTerminal()) terminalData.apply(computer.getTerminal()); } - public static PocketComputerData get(ItemStack stack) { - var family = stack.getItem() instanceof PocketComputerItem computer ? computer.getFamily() : ComputerFamily.NORMAL; - return get(PocketComputerItem.getInstanceID(stack), family != ComputerFamily.NORMAL); + public static @Nullable PocketComputerData get(ItemStack stack) { + var id = PocketComputerItem.getInstanceID(stack); + return id == null ? null : instances.get(id); } } diff --git a/projects/common/src/client/java/dan200/computercraft/client/pocket/PocketComputerData.java b/projects/common/src/client/java/dan200/computercraft/client/pocket/PocketComputerData.java index 03eb09a95..a5cf16d31 100644 --- a/projects/common/src/client/java/dan200/computercraft/client/pocket/PocketComputerData.java +++ b/projects/common/src/client/java/dan200/computercraft/client/pocket/PocketComputerData.java @@ -4,11 +4,8 @@ package dan200.computercraft.client.pocket; -import dan200.computercraft.core.terminal.Terminal; import dan200.computercraft.shared.computer.core.ComputerState; import dan200.computercraft.shared.computer.terminal.NetworkedTerminal; -import dan200.computercraft.shared.computer.terminal.TerminalState; -import dan200.computercraft.shared.config.Config; import dan200.computercraft.shared.pocket.core.PocketServerComputer; /** @@ -21,20 +18,22 @@ * @see ClientPocketComputers The registry which holds pocket computers. * @see PocketServerComputer The server-side pocket computer. */ -public class PocketComputerData { +public final class PocketComputerData { private final NetworkedTerminal terminal; - private ComputerState state = ComputerState.OFF; - private int lightColour = -1; + private ComputerState state; + private int lightColour; - public PocketComputerData(boolean colour) { - terminal = new NetworkedTerminal(Config.pocketTermWidth, Config.pocketTermHeight, colour); + PocketComputerData(ComputerState state, int lightColour, NetworkedTerminal terminal) { + this.state = state; + this.lightColour = lightColour; + this.terminal = terminal; } public int getLightState() { return state != ComputerState.OFF ? lightColour : -1; } - public Terminal getTerminal() { + public NetworkedTerminal getTerminal() { return terminal; } @@ -42,12 +41,8 @@ public ComputerState getState() { return state; } - public void setState(ComputerState state, int lightColour) { + void setState(ComputerState state, int lightColour) { this.state = state; this.lightColour = lightColour; } - - public void setTerminal(TerminalState state) { - state.apply(terminal); - } } diff --git a/projects/common/src/client/java/dan200/computercraft/client/render/PocketItemRenderer.java b/projects/common/src/client/java/dan200/computercraft/client/render/PocketItemRenderer.java index f305d4e6f..676f13556 100644 --- a/projects/common/src/client/java/dan200/computercraft/client/render/PocketItemRenderer.java +++ b/projects/common/src/client/java/dan200/computercraft/client/render/PocketItemRenderer.java @@ -11,6 +11,7 @@ import dan200.computercraft.client.render.text.FixedWidthFontRenderer; import dan200.computercraft.core.util.Colour; import dan200.computercraft.shared.computer.core.ComputerFamily; +import dan200.computercraft.shared.config.Config; import dan200.computercraft.shared.pocket.items.PocketComputerItem; import net.minecraft.client.renderer.MultiBufferSource; import net.minecraft.world.item.ItemStack; @@ -32,10 +33,16 @@ private PocketItemRenderer() { @Override protected void renderItem(PoseStack transform, MultiBufferSource bufferSource, ItemStack stack, int light) { var computer = ClientPocketComputers.get(stack); - var terminal = computer.getTerminal(); + var terminal = computer == null ? null : computer.getTerminal(); - var termWidth = terminal.getWidth(); - var termHeight = terminal.getHeight(); + int termWidth, termHeight; + if (terminal == null) { + termWidth = Config.pocketTermWidth; + termHeight = Config.pocketTermHeight; + } else { + termWidth = terminal.getWidth(); + termHeight = terminal.getHeight(); + } var width = termWidth * FONT_WIDTH + MARGIN * 2; var height = termHeight * FONT_HEIGHT + MARGIN * 2; @@ -60,14 +67,15 @@ protected void renderItem(PoseStack transform, MultiBufferSource bufferSource, I renderFrame(matrix, bufferSource, family, frameColour, light, width, height); // Render the light - var lightColour = ClientPocketComputers.get(stack).getLightState(); - if (lightColour == -1) lightColour = Colour.BLACK.getHex(); + var lightColour = computer == null || computer.getLightState() == -1 ? Colour.BLACK.getHex() : computer.getLightState(); renderLight(transform, bufferSource, lightColour, width, height); - FixedWidthFontRenderer.drawTerminal( - FixedWidthFontRenderer.toVertexConsumer(transform, bufferSource.getBuffer(RenderTypes.TERMINAL)), - MARGIN, MARGIN, terminal, MARGIN, MARGIN, MARGIN, MARGIN - ); + var quadEmitter = FixedWidthFontRenderer.toVertexConsumer(transform, bufferSource.getBuffer(RenderTypes.TERMINAL)); + if (terminal == null) { + FixedWidthFontRenderer.drawEmptyTerminal(quadEmitter, 0, 0, width, height); + } else { + FixedWidthFontRenderer.drawTerminal(quadEmitter, MARGIN, MARGIN, terminal, MARGIN, MARGIN, MARGIN, MARGIN); + } transform.popPose(); } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/command/CommandComputerCraft.java b/projects/common/src/main/java/dan200/computercraft/shared/command/CommandComputerCraft.java index 9ae5c8f4a..85c95f73d 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/command/CommandComputerCraft.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/command/CommandComputerCraft.java @@ -134,7 +134,7 @@ private static int dump(CommandSourceStack source) { } else if (b.getLevel() == world) { return 1; } else { - return Integer.compare(a.getInstanceID(), b.getInstanceID()); + return a.getInstanceUUID().compareTo(b.getInstanceUUID()); } }); @@ -159,7 +159,8 @@ private static int dump(CommandSourceStack source) { */ private static int dumpComputer(CommandSourceStack source, ServerComputer computer) { var table = new TableBuilder("Dump"); - table.row(header("Instance"), text(Integer.toString(computer.getInstanceID()))); + table.row(header("Instance ID"), text(Integer.toString(computer.getInstanceID()))); + table.row(header("Instance UUID"), text(computer.getInstanceUUID().toString())); table.row(header("Id"), text(Integer.toString(computer.getID()))); table.row(header("Label"), text(computer.getLabel())); table.row(header("On"), bool(computer.isOn())); @@ -338,11 +339,7 @@ private static Component linkComputer(CommandSourceStack source, @Nullable Serve if (computer == null) { out.append("#" + computerId + " ").append(coloured("(unloaded)", ChatFormatting.GRAY)); } else { - out.append(link( - text("#" + computerId + " ").append(coloured("(instance " + computer.getInstanceID() + ")", ChatFormatting.GRAY)), - makeComputerCommand("dump", computer), - Component.translatable("commands.computercraft.dump.action") - )); + out.append(makeComputerDumpCommand(computer)); } // And, if we're a player, some useful links @@ -372,10 +369,6 @@ private static Component linkComputer(CommandSourceStack source, @Nullable Serve return out; } - private static String makeComputerCommand(String command, ServerComputer computer) { - return String.format("/computercraft %s @c[instance=%d]", command, computer.getInstanceID()); - } - private static Component linkPosition(CommandSourceStack context, ServerComputer computer) { if (ModRegistry.Permissions.PERMISSION_TP.test(context)) { return link( diff --git a/projects/common/src/main/java/dan200/computercraft/shared/command/arguments/ComputerSelector.java b/projects/common/src/main/java/dan200/computercraft/shared/command/arguments/ComputerSelector.java index bcec20da7..7d1668b76 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/command/arguments/ComputerSelector.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/command/arguments/ComputerSelector.java @@ -16,7 +16,10 @@ import net.minecraft.advancements.critereon.MinMaxBounds; import net.minecraft.commands.CommandSourceStack; import net.minecraft.commands.SharedSuggestionProvider; +import net.minecraft.commands.arguments.UuidArgument; import net.minecraft.network.chat.Component; +import net.minecraft.network.chat.ComponentContents; +import net.minecraft.network.chat.MutableComponent; import net.minecraft.world.phys.AABB; import net.minecraft.world.phys.Vec3; @@ -30,17 +33,21 @@ import static dan200.computercraft.shared.command.CommandUtils.suggestOnServer; import static dan200.computercraft.shared.command.Exceptions.*; import static dan200.computercraft.shared.command.arguments.ArgumentParserUtils.consume; +import static dan200.computercraft.shared.command.text.ChatHelpers.makeComputerDumpCommand; public record ComputerSelector( String selector, OptionalInt instanceId, + @Nullable UUID instanceUuid, OptionalInt computerId, @Nullable String label, @Nullable ComputerFamily family, @Nullable AABB bounds, @Nullable MinMaxBounds.Doubles range ) { - private static final ComputerSelector all = new ComputerSelector("@c[]", OptionalInt.empty(), OptionalInt.empty(), null, null, null, null); + private static final ComputerSelector all = new ComputerSelector("@c[]", OptionalInt.empty(), null, OptionalInt.empty(), null, null, null, null); + + private static UuidArgument uuidArgument = UuidArgument.uuid(); /** * A {@link ComputerSelector} which matches all computers. @@ -59,8 +66,13 @@ public static ComputerSelector all() { */ public Stream find(CommandSourceStack source) { var context = ServerContext.get(source.getServer()); - if (instanceId.isPresent()) { - var computer = context.registry().get(instanceId.getAsInt()); + if (instanceId().isPresent()) { + var computer = context.registry().get(instanceId().getAsInt()); + return computer != null && matches(source, computer) ? Stream.of(computer) : Stream.of(); + } + + if (instanceUuid() != null) { + var computer = context.registry().get(instanceUuid()); return computer != null && matches(source, computer) ? Stream.of(computer) : Stream.of(); } @@ -79,7 +91,7 @@ public ServerComputer findOne(CommandSourceStack source) throws CommandSyntaxExc if (computers.isEmpty()) throw COMPUTER_ARG_NONE.create(selector); if (computers.size() == 1) return computers.iterator().next(); - var builder = new StringBuilder(); + var builder = MutableComponent.create(ComponentContents.EMPTY); var first = true; for (var computer : computers) { if (first) { @@ -88,12 +100,12 @@ public ServerComputer findOne(CommandSourceStack source) throws CommandSyntaxExc builder.append(", "); } - builder.append(computer.getInstanceID()); + builder.append(makeComputerDumpCommand(computer)); } // We have an incorrect number of computers: throw an error - throw COMPUTER_ARG_MANY.create(selector, builder.toString()); + throw COMPUTER_ARG_MANY.create(selector, builder); } /** @@ -105,6 +117,7 @@ public ServerComputer findOne(CommandSourceStack source) throws CommandSyntaxExc */ public boolean matches(CommandSourceStack source, ServerComputer computer) { return (instanceId().isEmpty() || computer.getInstanceID() == instanceId().getAsInt()) + && (instanceUuid() == null || computer.getInstanceUUID().equals(instanceUuid())) && (computerId().isEmpty() || computer.getID() == computerId().getAsInt()) && (label == null || Objects.equals(computer.getLabel(), label)) && (family == null || computer.getFamily() == family) @@ -127,6 +140,7 @@ public static ComputerSelector parse(StringReader reader) throws CommandSyntaxEx if (consume(reader, "@c[")) { parseSelector(builder, reader); } else { + // TODO(1.20.5): Only parse computer ids here. var kind = reader.peek(); if (kind == '@') { reader.skip(); @@ -143,7 +157,7 @@ public static ComputerSelector parse(StringReader reader) throws CommandSyntaxEx } var selector = reader.getString().substring(start, reader.getCursor()); - return new ComputerSelector(selector, builder.instanceId, builder.computerId, builder.label, builder.family, builder.bounds, builder.range); + return new ComputerSelector(selector, builder.instanceId, builder.instanceUuid, builder.computerId, builder.label, builder.family, builder.bounds, builder.range); } private static void parseSelector(Builder builder, StringReader reader) throws CommandSyntaxException { @@ -260,6 +274,7 @@ private static SuggestionsBuilder suggestions(StringReader reader) { private static final class Builder { private OptionalInt instanceId = OptionalInt.empty(); + private @Nullable UUID instanceUuid = null; private OptionalInt computerId = OptionalInt.empty(); private @Nullable String label; private @Nullable ComputerFamily family; @@ -282,8 +297,8 @@ public static Map options() { var optionList = new Option[]{ new Option( "instance", - (reader, builder) -> builder.instanceId = OptionalInt.of(reader.readInt()), - suggestComputers(c -> Integer.toString(c.getInstanceID())) + (reader, builder) -> builder.instanceUuid = uuidArgument.parse(reader), + suggestComputers(c -> c.getInstanceUUID().toString()) ), new Option( "id", diff --git a/projects/common/src/main/java/dan200/computercraft/shared/command/text/ChatHelpers.java b/projects/common/src/main/java/dan200/computercraft/shared/command/text/ChatHelpers.java index bf3479c5f..a3a743469 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/command/text/ChatHelpers.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/command/text/ChatHelpers.java @@ -4,6 +4,7 @@ package dan200.computercraft.shared.command.text; +import dan200.computercraft.shared.computer.core.ServerComputer; import net.minecraft.ChatFormatting; import net.minecraft.core.BlockPos; import net.minecraft.network.chat.ClickEvent; @@ -73,4 +74,16 @@ public static MutableComponent copy(String text) { .withHoverEvent(new HoverEvent(HoverEvent.Action.SHOW_TEXT, Component.translatable("gui.computercraft.tooltip.copy"))) ); } + + public static String makeComputerCommand(String command, ServerComputer computer) { + return String.format("/computercraft %s @c[instance=%s]", command, computer.getInstanceUUID()); + } + + public static Component makeComputerDumpCommand(ServerComputer computer) { + return link( + text("#" + computer.getID()), + makeComputerCommand("dump", computer), + Component.translatable("commands.computercraft.dump.action") + ); + } } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlockEntity.java b/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlockEntity.java index cd7b27d14..0ad1dc230 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlockEntity.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlockEntity.java @@ -36,13 +36,14 @@ import javax.annotation.Nullable; import java.util.Objects; +import java.util.UUID; public abstract class AbstractComputerBlockEntity extends BlockEntity implements IComputerBlockEntity, Nameable, MenuProvider { private static final String NBT_ID = "ComputerId"; private static final String NBT_LABEL = "Label"; private static final String NBT_ON = "On"; - private int instanceID = -1; + private @Nullable UUID instanceID = null; private int computerID = -1; protected @Nullable String label = null; private boolean on = false; @@ -66,7 +67,7 @@ protected void unload() { var computer = getServerComputer(); if (computer != null) computer.close(); - instanceID = -1; + instanceID = null; } @Override @@ -401,7 +402,7 @@ protected void loadClient(CompoundTag nbt) { } protected void transferStateFrom(AbstractComputerBlockEntity copy) { - if (copy.computerID != computerID || copy.instanceID != instanceID) { + if (copy.computerID != computerID || !Objects.equals(copy.instanceID, instanceID)) { unload(); instanceID = copy.instanceID; computerID = copy.computerID; @@ -411,7 +412,7 @@ protected void transferStateFrom(AbstractComputerBlockEntity copy) { lockCode = copy.lockCode; BlockEntityHelpers.updateBlock(this); } - copy.instanceID = -1; + copy.instanceID = null; } @Override diff --git a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputer.java b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputer.java index 539b2391b..3b9a2e6e3 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputer.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputer.java @@ -26,11 +26,13 @@ import net.minecraft.world.inventory.AbstractContainerMenu; import javax.annotation.Nullable; +import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; public class ServerComputer implements InputHandler, ComputerEnvironment { private final int instanceID; + private final UUID instanceUUID = UUID.randomUUID(); private ServerLevel level; private BlockPos position; @@ -119,9 +121,9 @@ public int pollAndResetChanges() { return computer.pollAndResetChanges(); } - public int register() { - ServerContext.get(level.getServer()).registry().add(instanceID, this); - return instanceID; + public UUID register() { + ServerContext.get(level.getServer()).registry().add(this); + return instanceUUID; } void unload() { @@ -130,7 +132,7 @@ void unload() { public void close() { unload(); - ServerContext.get(level.getServer()).registry().remove(instanceID); + ServerContext.get(level.getServer()).registry().remove(this); } private void sendToAllInteracting(Function> createPacket) { @@ -150,6 +152,10 @@ public int getInstanceID() { return instanceID; } + public UUID getInstanceUUID() { + return instanceUUID; + } + public int getID() { return computer.getID(); } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputerRegistry.java b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputerRegistry.java index 1c53aaabb..34f76b19d 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputerRegistry.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/core/ServerComputerRegistry.java @@ -8,14 +8,14 @@ import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import javax.annotation.Nullable; -import java.util.Collection; -import java.util.Random; +import java.util.*; public class ServerComputerRegistry { private static final Random RANDOM = new Random(); private final int sessionId = RANDOM.nextInt(); - private final Int2ObjectMap computers = new Int2ObjectOpenHashMap<>(); + private final Int2ObjectMap computersByInstanceId = new Int2ObjectOpenHashMap<>(); + private final Map computersByInstanceUuid = new HashMap<>(); private int nextInstanceId; public int getSessionID() { @@ -28,11 +28,16 @@ int getUnusedInstanceID() { @Nullable public ServerComputer get(int instanceID) { - return instanceID >= 0 ? computers.get(instanceID) : null; + return instanceID >= 0 ? computersByInstanceId.get(instanceID) : null; } @Nullable - public ServerComputer get(int sessionId, int instanceId) { + public ServerComputer get(@Nullable UUID instanceID) { + return instanceID != null ? computersByInstanceUuid.get(instanceID) : null; + } + + @Nullable + public ServerComputer get(int sessionId, @Nullable UUID instanceId) { return sessionId == this.sessionId ? get(instanceId) : null; } @@ -50,28 +55,36 @@ void update() { } } - void add(int instanceID, ServerComputer computer) { - remove(instanceID); - computers.put(instanceID, computer); - nextInstanceId = Math.max(nextInstanceId, instanceID + 1); - } + void add(ServerComputer computer) { + var instanceID = computer.getInstanceID(); + var instanceUUID = computer.getInstanceUUID(); - void remove(int instanceID) { - var computer = get(instanceID); - if (computer != null) { - computer.unload(); - computer.onRemoved(); + if (computersByInstanceId.containsKey(instanceID)) { + throw new IllegalStateException("Duplicate computer " + instanceID); } - computers.remove(instanceID); + if (computersByInstanceUuid.containsKey(instanceUUID)) { + throw new IllegalStateException("Duplicate computer " + instanceUUID); + } + + computersByInstanceId.put(instanceID, computer); + computersByInstanceUuid.put(instanceUUID, computer); + } + + void remove(ServerComputer computer) { + computer.unload(); + computer.onRemoved(); + computersByInstanceId.remove(computer.getInstanceID()); + computersByInstanceUuid.remove(computer.getInstanceUUID()); } void close() { for (var computer : getComputers()) computer.unload(); - computers.clear(); + computersByInstanceId.clear(); + computersByInstanceUuid.clear(); } public Collection getComputers() { - return computers.values(); + return computersByInstanceId.values(); } } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ViewComputerMenu.java b/projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ViewComputerMenu.java index 1765c30a3..2dbb56262 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ViewComputerMenu.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/inventory/ViewComputerMenu.java @@ -25,7 +25,7 @@ public ViewComputerMenu(int id, Inventory player, ComputerContainerData data) { private static boolean canInteractWith(ServerComputer computer, Player player) { // If this computer no longer exists then discard it. - if (ServerContext.get(computer.getLevel().getServer()).registry().get(computer.getInstanceID()) != computer) { + if (ServerContext.get(computer.getLevel().getServer()).registry().get(computer.getInstanceUUID()) != computer) { return false; } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/network/client/ClientNetworkContext.java b/projects/common/src/main/java/dan200/computercraft/shared/network/client/ClientNetworkContext.java index cdc598e95..fce3cad03 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/network/client/ClientNetworkContext.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/network/client/ClientNetworkContext.java @@ -30,9 +30,9 @@ public interface ClientNetworkContext { void handlePlayRecord(BlockPos pos, @Nullable SoundEvent sound, @Nullable String name); - void handlePocketComputerData(int instanceId, ComputerState state, int lightState, TerminalState terminal); + void handlePocketComputerData(UUID instanceId, ComputerState state, int lightState, TerminalState terminal); - void handlePocketComputerDeleted(int instanceId); + void handlePocketComputerDeleted(UUID instanceId); void handleSpeakerAudio(UUID source, SpeakerPosition.Message position, float volume, EncodedAudio audio); diff --git a/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDataMessage.java b/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDataMessage.java index 2d032a800..b4be75181 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDataMessage.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDataMessage.java @@ -13,24 +13,26 @@ import dan200.computercraft.shared.pocket.core.PocketServerComputer; import net.minecraft.network.FriendlyByteBuf; +import java.util.UUID; + /** * Provides additional data about a client computer, such as its ID and current state. */ public class PocketComputerDataMessage implements NetworkMessage { - private final int instanceId; + private final UUID clientId; private final ComputerState state; private final int lightState; private final TerminalState terminal; public PocketComputerDataMessage(PocketServerComputer computer, boolean sendTerminal) { - instanceId = computer.getInstanceID(); + clientId = computer.getInstanceUUID(); state = computer.getState(); lightState = computer.getLight(); terminal = sendTerminal ? computer.getTerminalState() : new TerminalState((NetworkedTerminal) null); } public PocketComputerDataMessage(FriendlyByteBuf buf) { - instanceId = buf.readVarInt(); + clientId = buf.readUUID(); state = buf.readEnum(ComputerState.class); lightState = buf.readVarInt(); terminal = new TerminalState(buf); @@ -38,7 +40,7 @@ public PocketComputerDataMessage(FriendlyByteBuf buf) { @Override public void write(FriendlyByteBuf buf) { - buf.writeVarInt(instanceId); + buf.writeUUID(clientId); buf.writeEnum(state); buf.writeVarInt(lightState); terminal.write(buf); @@ -46,7 +48,7 @@ public void write(FriendlyByteBuf buf) { @Override public void handle(ClientNetworkContext context) { - context.handlePocketComputerData(instanceId, state, lightState, terminal); + context.handlePocketComputerData(clientId, state, lightState, terminal); } @Override diff --git a/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDeletedClientMessage.java b/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDeletedClientMessage.java index 6ce365c69..30340139a 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDeletedClientMessage.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/network/client/PocketComputerDeletedClientMessage.java @@ -9,21 +9,23 @@ import dan200.computercraft.shared.network.NetworkMessages; import net.minecraft.network.FriendlyByteBuf; +import java.util.UUID; + public class PocketComputerDeletedClientMessage implements NetworkMessage { - private final int instanceId; + private final UUID instanceId; - public PocketComputerDeletedClientMessage(int instanceId) { + public PocketComputerDeletedClientMessage(UUID instanceId) { this.instanceId = instanceId; } public PocketComputerDeletedClientMessage(FriendlyByteBuf buffer) { - instanceId = buffer.readVarInt(); + instanceId = buffer.readUUID(); } @Override public void write(FriendlyByteBuf buf) { - buf.writeVarInt(instanceId); + buf.writeUUID(instanceId); } @Override diff --git a/projects/common/src/main/java/dan200/computercraft/shared/pocket/core/PocketServerComputer.java b/projects/common/src/main/java/dan200/computercraft/shared/pocket/core/PocketServerComputer.java index b049156c9..16d41012f 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/pocket/core/PocketServerComputer.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/pocket/core/PocketServerComputer.java @@ -190,6 +190,6 @@ protected void onTerminalChanged() { @Override protected void onRemoved() { super.onRemoved(); - ServerNetworking.sendToAllPlayers(new PocketComputerDeletedClientMessage(getInstanceID()), getLevel().getServer()); + ServerNetworking.sendToAllPlayers(new PocketComputerDeletedClientMessage(getInstanceUUID()), getLevel().getServer()); } } diff --git a/projects/common/src/main/java/dan200/computercraft/shared/pocket/items/PocketComputerItem.java b/projects/common/src/main/java/dan200/computercraft/shared/pocket/items/PocketComputerItem.java index c3e731ca3..43f0bfa4a 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/pocket/items/PocketComputerItem.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/pocket/items/PocketComputerItem.java @@ -43,6 +43,7 @@ import javax.annotation.Nullable; import java.util.List; import java.util.Objects; +import java.util.UUID; public class PocketComputerItem extends Item implements IComputerItem, IMedia, IColouredItem { private static final String NBT_UPGRADE = "Upgrade"; @@ -188,10 +189,9 @@ public String getCreatorModId(ItemStack stack) { } public PocketServerComputer createServerComputer(ServerLevel level, Entity entity, @Nullable Container inventory, ItemStack stack) { - var sessionID = getSessionID(stack); var registry = ServerContext.get(level.getServer()).registry(); - var computer = (PocketServerComputer) registry.get(sessionID, getInstanceID(stack)); + var computer = (PocketServerComputer) registry.get(getSessionID(stack), getInstanceID(stack)); if (computer == null) { var computerID = getComputerID(stack); if (computerID < 0) { @@ -201,8 +201,9 @@ public PocketServerComputer createServerComputer(ServerLevel level, Entity entit computer = new PocketServerComputer(level, entity.blockPosition(), getComputerID(stack), getLabel(stack), getFamily()); - setInstanceID(stack, computer.register()); - setSessionID(stack, registry.getSessionID()); + var tag = stack.getOrCreateTag(); + tag.putInt(NBT_SESSION, registry.getSessionID()); + tag.putUUID(NBT_INSTANCE, computer.register()); var upgrade = getUpgrade(stack); @@ -267,13 +268,9 @@ public boolean setLabel(ItemStack stack, @Nullable String label) { return null; } - public static int getInstanceID(ItemStack stack) { + public static @Nullable UUID getInstanceID(ItemStack stack) { var nbt = stack.getTag(); - return nbt != null && nbt.contains(NBT_INSTANCE) ? nbt.getInt(NBT_INSTANCE) : -1; - } - - private static void setInstanceID(ItemStack stack, int instanceID) { - stack.getOrCreateTag().putInt(NBT_INSTANCE, instanceID); + return nbt != null && nbt.hasUUID(NBT_INSTANCE) ? nbt.getUUID(NBT_INSTANCE) : null; } private static int getSessionID(ItemStack stack) { @@ -281,10 +278,6 @@ private static int getSessionID(ItemStack stack) { return nbt != null && nbt.contains(NBT_SESSION) ? nbt.getInt(NBT_SESSION) : -1; } - private static void setSessionID(ItemStack stack, int sessionID) { - stack.getOrCreateTag().putInt(NBT_SESSION, sessionID); - } - private static boolean isMarkedOn(ItemStack stack) { var nbt = stack.getTag(); return nbt != null && nbt.getBoolean(NBT_ON); diff --git a/projects/common/src/test/java/dan200/computercraft/shared/command/arguments/ComputerSelectorTest.java b/projects/common/src/test/java/dan200/computercraft/shared/command/arguments/ComputerSelectorTest.java index 67954bfdf..a88c50d21 100644 --- a/projects/common/src/test/java/dan200/computercraft/shared/command/arguments/ComputerSelectorTest.java +++ b/projects/common/src/test/java/dan200/computercraft/shared/command/arguments/ComputerSelectorTest.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.OptionalInt; +import java.util.UUID; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -39,19 +40,19 @@ public void Parse_basic_inputs(String input, ComputerSelector expected) throws C public static Arguments[] getArgumentTestCases() { return new Arguments[]{ // Legacy selectors - Arguments.of("@some_label", new ComputerSelector("@some_label", OptionalInt.empty(), OptionalInt.empty(), "some_label", null, null, null)), - Arguments.of("~normal", new ComputerSelector("~normal", OptionalInt.empty(), OptionalInt.empty(), null, ComputerFamily.NORMAL, null, null)), - Arguments.of("#123", new ComputerSelector("#123", OptionalInt.empty(), OptionalInt.of(123), null, null, null, null)), - Arguments.of("123", new ComputerSelector("123", OptionalInt.of(123), OptionalInt.empty(), null, null, null, null)), + Arguments.of("@some_label", new ComputerSelector("@some_label", OptionalInt.empty(), null, OptionalInt.empty(), "some_label", null, null, null)), + Arguments.of("~normal", new ComputerSelector("~normal", OptionalInt.empty(), null, OptionalInt.empty(), null, ComputerFamily.NORMAL, null, null)), + Arguments.of("#123", new ComputerSelector("#123", OptionalInt.empty(), null, OptionalInt.of(123), null, null, null, null)), + Arguments.of("123", new ComputerSelector("123", OptionalInt.of(123), null, OptionalInt.empty(), null, null, null, null)), // New selectors - Arguments.of("@c[]", new ComputerSelector("@c[]", OptionalInt.empty(), OptionalInt.empty(), null, null, null, null)), - Arguments.of("@c[instance=123]", new ComputerSelector("@c[instance=123]", OptionalInt.of(123), OptionalInt.empty(), null, null, null, null)), - Arguments.of("@c[id=123]", new ComputerSelector("@c[id=123]", OptionalInt.empty(), OptionalInt.of(123), null, null, null, null)), - Arguments.of("@c[label=\"foo\"]", new ComputerSelector("@c[label=\"foo\"]", OptionalInt.empty(), OptionalInt.empty(), "foo", null, null, null)), - Arguments.of("@c[family=normal]", new ComputerSelector("@c[family=normal]", OptionalInt.empty(), OptionalInt.empty(), null, ComputerFamily.NORMAL, null, null)), + Arguments.of("@c[]", new ComputerSelector("@c[]", OptionalInt.empty(), null, OptionalInt.empty(), null, null, null, null)), + Arguments.of("@c[instance=5e18f505-62f7-46f8-83f3-792f03224724]", new ComputerSelector("@c[instance=5e18f505-62f7-46f8-83f3-792f03224724]", OptionalInt.empty(), UUID.fromString("5e18f505-62f7-46f8-83f3-792f03224724"), OptionalInt.empty(), null, null, null, null)), + Arguments.of("@c[id=123]", new ComputerSelector("@c[id=123]", OptionalInt.empty(), null, OptionalInt.of(123), null, null, null, null)), + Arguments.of("@c[label=\"foo\"]", new ComputerSelector("@c[label=\"foo\"]", OptionalInt.empty(), null, OptionalInt.empty(), "foo", null, null, null)), + Arguments.of("@c[family=normal]", new ComputerSelector("@c[family=normal]", OptionalInt.empty(), null, OptionalInt.empty(), null, ComputerFamily.NORMAL, null, null)), // Complex selectors - Arguments.of("@c[ id = 123 , ]", new ComputerSelector("@c[ id = 123 , ]", OptionalInt.empty(), OptionalInt.of(123), null, null, null, null)), - Arguments.of("@c[id=123,family=normal]", new ComputerSelector("@c[id=123,family=normal]", OptionalInt.empty(), OptionalInt.of(123), null, ComputerFamily.NORMAL, null, null)), + Arguments.of("@c[ id = 123 , ]", new ComputerSelector("@c[ id = 123 , ]", OptionalInt.empty(), null, OptionalInt.of(123), null, null, null, null)), + Arguments.of("@c[id=123,family=normal]", new ComputerSelector("@c[id=123,family=normal]", OptionalInt.empty(), null, OptionalInt.of(123), null, ComputerFamily.NORMAL, null, null)), }; } diff --git a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Pocket_Computer_Test.kt b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Pocket_Computer_Test.kt index d070f8e0f..d45844039 100644 --- a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Pocket_Computer_Test.kt +++ b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Pocket_Computer_Test.kt @@ -38,7 +38,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence { // And ensure its synced to the client. thenIdle(4) thenOnClient { - val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem) + val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem)!! assertEquals(ComputerState.ON, pocketComputer.state) val term = pocketComputer.terminal @@ -54,7 +54,7 @@ fun Sync_state(context: GameTestHelper) = context.sequence { // And ensure the new computer state and terminal are sent. thenIdle(4) thenOnClient { - val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem) + val pocketComputer = ClientPocketComputers.get(minecraft.player!!.mainHandItem)!! assertEquals(ComputerState.BLINKING, pocketComputer.state) val term = pocketComputer.terminal