diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java index 10baeeb4d..6bfa79982 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDriveBlockEntity.java @@ -6,6 +6,7 @@ package dan200.computercraft.shared.peripheral.diskdrive; import com.google.errorprone.annotations.concurrent.GuardedBy; +import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.api.filesystem.WritableMount; import dan200.computercraft.api.peripheral.IComputerAccess; import dan200.computercraft.api.peripheral.IPeripheral; @@ -47,11 +48,14 @@ private static class MountInfo { private final NonNullList inventory = NonNullList.withSize(1, ItemStack.EMPTY); private MediaStack media = MediaStack.EMPTY; + private @Nullable Mount mount; + private boolean recordPlaying = false; // In order to avoid main-thread calls in the peripheral, we set flags to mark which operation should be performed, // then read them when ticking. private final AtomicReference recordQueued = new AtomicReference<>(null); private final AtomicBoolean ejectQueued = new AtomicBoolean(false); + private final AtomicBoolean mountQueued = new AtomicBoolean(false); public DiskDriveBlockEntity(BlockEntityType type, BlockPos pos, BlockState state) { super(type, pos, state); @@ -109,6 +113,12 @@ var record = media.getAudio(); } } } + + if (mountQueued.get()) { + synchronized (this) { + mountAll(); + } + } } @Override @@ -124,9 +134,9 @@ public void setChanged() { private void updateItem() { var newDisk = getDiskStack(); - if (ItemStack.isSame(newDisk, media.stack)) return; + if (ItemStack.isSameItemSameTags(newDisk, media.stack)) return; - var media = new MediaStack(newDisk.copy()); + var media = MediaStack.of(newDisk); if (newDisk.isEmpty()) { updateBlockState(DiskDriveState.EMPTY); @@ -146,12 +156,10 @@ private void updateItem() { recordPlaying = false; } + mount = null; this.media = media; - // Mount new disk - if (!this.media.stack.isEmpty()) { - for (var computer : computers.entrySet()) mountDisk(computer.getKey(), computer.getValue(), this.media); - } + mountAll(); } } @@ -163,11 +171,30 @@ MediaStack getMedia() { return media; } + /** + * Set the current disk stack, mounting/unmounting if needed. + * + * @param stack The new disk stack. + */ void setDiskStack(ItemStack stack) { setItem(0, stack); setChanged(); } + /** + * Update the current disk stack, assuming the underlying item does not change. Unlike + * {@link #setDiskStack(ItemStack)} this will not change any mounts. + * + * @param stack The new disk stack. + */ + void updateDiskStack(ItemStack stack) { + setItem(0, stack); + if (!ItemStack.isSameItemSameTags(stack, media.stack)) { + media = MediaStack.of(stack); + super.setChanged(); + } + } + @Nullable String getDiskMountPath(IComputerAccess computer) { synchronized (this) { @@ -176,15 +203,21 @@ String getDiskMountPath(IComputerAccess computer) { } } - void mount(IComputerAccess computer) { + /** + * Attach a computer to this disk drive. This sets up the {@link MountInfo} map and flags us to mount next tick. We + * don't mount here, as that might require mutating the current stack. + * + * @param computer The computer to attach. + */ + void attach(IComputerAccess computer) { synchronized (this) { var info = new MountInfo(); computers.put(computer, info); - mountDisk(computer, info, media); + mountQueued.set(true); } } - void unmount(IComputerAccess computer) { + void detach(IComputerAccess computer) { synchronized (this) { unmountDisk(computer, computers.remove(computer)); } @@ -202,10 +235,35 @@ void ejectDisk() { ejectQueued.set(true); } + /** + * Add our mount to all computers. + */ @GuardedBy("this") - private void mountDisk(IComputerAccess computer, MountInfo info, MediaStack disk) { - var mount = disk.getMount((ServerLevel) getLevel()); - if (mount != null) { + private void mountAll() { + doMountAll(); + mountQueued.set(false); + } + + /** + * The worker for {@link #mountAll()}. This is responsible for creating the mount and placing it on all computers. + */ + @GuardedBy("this") + private void doMountAll() { + if (computers.isEmpty() || media.media == null) return; + + if (mount == null) { + var stack = getDiskStack(); + mount = media.media.createDataMount(stack, (ServerLevel) level); + setDiskStack(stack); + } + + if (mount == null) return; + + for (var entry : computers.entrySet()) { + var computer = entry.getKey(); + var info = entry.getValue(); + if (info.mountPath != null) continue; + if (mount instanceof WritableMount writable) { // Try mounting at the lowest numbered "disk" name we can var n = 1; @@ -221,11 +279,9 @@ private void mountDisk(IComputerAccess computer, MountInfo info, MediaStack disk n++; } } - } else { - info.mountPath = null; - } - computer.queueEvent("disk", computer.getAttachmentName()); + computer.queueEvent("disk", computer.getAttachmentName()); + } } private static void unmountDisk(IComputerAccess computer, MountInfo info) { diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java index d698d8132..3aee2a253 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/DiskDrivePeripheral.java @@ -84,11 +84,12 @@ public final void setDiskLabel(Optional label) throws LuaException { var media = diskDrive.getMedia(); if (media.media == null) return; - var stack = media.stack.copy(); + // We're on the main thread so the stack and media should be in sync. + var stack = diskDrive.getDiskStack(); if (!media.media.setLabel(stack, label.map(StringUtil::normaliseLabel).orElse(null))) { throw new LuaException("Disk label cannot be changed"); } - diskDrive.setDiskStack(stack); + diskDrive.updateDiskStack(stack); } /** @@ -178,12 +179,12 @@ public final Object[] getDiskID() { @Override public void attach(IComputerAccess computer) { - diskDrive.mount(computer); + diskDrive.attach(computer); } @Override public void detach(IComputerAccess computer) { - diskDrive.unmount(computer); + diskDrive.detach(computer); } @Override diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/MediaStack.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/MediaStack.java index a8df9641b..9a6edb54b 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/MediaStack.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/diskdrive/MediaStack.java @@ -5,10 +5,8 @@ */ package dan200.computercraft.shared.peripheral.diskdrive; -import dan200.computercraft.api.filesystem.Mount; import dan200.computercraft.api.media.IMedia; import dan200.computercraft.impl.MediaProviders; -import net.minecraft.server.level.ServerLevel; import net.minecraft.sounds.SoundEvent; import net.minecraft.world.item.ItemStack; @@ -17,18 +15,22 @@ /** * An immutable snapshot of the current disk. This allows us to read the stack in a thread-safe manner. */ -class MediaStack { - static final MediaStack EMPTY = new MediaStack(ItemStack.EMPTY); +final class MediaStack { + static final MediaStack EMPTY = new MediaStack(ItemStack.EMPTY, null); final ItemStack stack; final @Nullable IMedia media; - @Nullable - private Mount mount; - - MediaStack(ItemStack stack) { + private MediaStack(ItemStack stack, @Nullable IMedia media) { this.stack = stack; - media = MediaProviders.get(stack); + this.media = media; + } + + public static MediaStack of(ItemStack stack) { + if (stack.isEmpty()) return EMPTY; + + var freshStack = stack.copy(); + return new MediaStack(freshStack, MediaProviders.get(freshStack)); } @Nullable @@ -40,12 +42,4 @@ SoundEvent getAudio() { String getAudioTitle() { return media != null ? media.getAudioTitle(stack) : null; } - - @Nullable - public Mount getMount(ServerLevel level) { - if (media == null) return null; - - if (mount == null) mount = media.createDataMount(stack, level); - return mount; - } } diff --git a/projects/common/src/testMod/java/dan200/computercraft/gametest/core/CCTestCommand.java b/projects/common/src/testMod/java/dan200/computercraft/gametest/core/CCTestCommand.java index 2406a643f..7f04b2bf2 100644 --- a/projects/common/src/testMod/java/dan200/computercraft/gametest/core/CCTestCommand.java +++ b/projects/common/src/testMod/java/dan200/computercraft/gametest/core/CCTestCommand.java @@ -8,6 +8,7 @@ import com.mojang.brigadier.CommandDispatcher; import dan200.computercraft.api.ComputerCraftAPI; import dan200.computercraft.mixin.gametest.TestCommandAccessor; +import dan200.computercraft.shared.ModRegistry; import net.minecraft.ChatFormatting; import net.minecraft.commands.CommandSourceStack; import net.minecraft.gametest.framework.GameTestRegistry; @@ -81,6 +82,27 @@ public static void register(CommandDispatcher dispatcher) { player.getLevel().addFreshEntity(armorStand); return 0; })) + + .then(literal("give-computer").executes(context -> { + var player = context.getSource().getPlayerOrException(); + var pos = StructureUtils.findNearestStructureBlock(player.blockPosition(), 15, player.getLevel()); + if (pos == null) return error(context.getSource(), "No nearby test"); + + var structureBlock = (StructureBlockEntity) player.getLevel().getBlockEntity(pos); + if (structureBlock == null) return error(context.getSource(), "No nearby structure block"); + var info = GameTestRegistry.getTestFunction(structureBlock.getStructurePath()); + + var item = ModRegistry.Items.COMPUTER_ADVANCED.get().create(1, info.getTestName()); + if (!player.getInventory().add(item)) { + var itemEntity = player.drop(item, false); + if (itemEntity != null) { + itemEntity.setNoPickUpDelay(); + itemEntity.setOwner(player.getUUID()); + } + } + + return 1; + })) ); } diff --git a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Disk_Drive_Test.kt b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Disk_Drive_Test.kt index db07bde53..559da776c 100644 --- a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Disk_Drive_Test.kt +++ b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Disk_Drive_Test.kt @@ -8,6 +8,7 @@ import dan200.computercraft.core.apis.FSAPI import dan200.computercraft.gametest.api.* import dan200.computercraft.shared.ModRegistry +import dan200.computercraft.shared.media.items.DiskItem import dan200.computercraft.shared.peripheral.diskdrive.DiskDriveBlock import dan200.computercraft.shared.peripheral.diskdrive.DiskDriveState import dan200.computercraft.test.core.assertArrayEquals @@ -45,10 +46,14 @@ fun Ejects_disk(helper: GameTestHelper) = helper.sequence { thenWaitUntil { helper.assertItemEntityPresent(Items.MUSIC_DISC_13, stackAt, 0.0) } } + /** + * A mount is initially attached, and then removed when the disk is ejected. + */ @GameTest fun Adds_removes_mount(helper: GameTestHelper) = helper.sequence { - thenIdle(2) - thenOnComputer { + thenOnComputer { } // Wait for the computer to start up + thenIdle(2) // Let the disk drive tick once to create the mount + thenOnComputer { // Then actually assert things! getApi().getDrive("disk").assertArrayEquals("right") callPeripheral("right", "ejectDisk") } @@ -56,6 +61,18 @@ fun Adds_removes_mount(helper: GameTestHelper) = helper.sequence { thenOnComputer { assertEquals(null, getApi().getDrive("disk")) } } + /** + * When creating a new mount, the item is with a new disk ID. + */ + @GameTest + fun Creates_disk_id(helper: GameTestHelper) = helper.sequence { + val drivePos = BlockPos(2, 2, 2) + thenWaitUntil { + val drive = helper.getBlockEntity(drivePos, ModRegistry.BlockEntities.DISK_DRIVE.get()) + if (DiskItem.getDiskID(drive.getItem(0)) == -1) helper.fail("Disk has no item", drivePos) + } + } + /** * Check comparators can read the contents of the disk drive */ diff --git a/projects/common/src/testMod/resources/data/cctest/structures/disk_drive_test.creates_disk_id.snbt b/projects/common/src/testMod/resources/data/cctest/structures/disk_drive_test.creates_disk_id.snbt new file mode 100644 index 000000000..dd6b9630f --- /dev/null +++ b/projects/common/src/testMod/resources/data/cctest/structures/disk_drive_test.creates_disk_id.snbt @@ -0,0 +1,138 @@ +{ + DataVersion: 3218, + size: [5, 5, 5], + data: [ + {pos: [0, 0, 0], state: "minecraft:polished_andesite"}, + {pos: [0, 0, 1], state: "minecraft:polished_andesite"}, + {pos: [0, 0, 2], state: "minecraft:polished_andesite"}, + {pos: [0, 0, 3], state: "minecraft:polished_andesite"}, + {pos: [0, 0, 4], state: "minecraft:polished_andesite"}, + {pos: [1, 0, 0], state: "minecraft:polished_andesite"}, + {pos: [1, 0, 1], state: "minecraft:polished_andesite"}, + {pos: [1, 0, 2], state: "minecraft:polished_andesite"}, + {pos: [1, 0, 3], state: "minecraft:polished_andesite"}, + {pos: [1, 0, 4], state: "minecraft:polished_andesite"}, + {pos: [2, 0, 0], state: "minecraft:polished_andesite"}, + {pos: [2, 0, 1], state: "minecraft:polished_andesite"}, + {pos: [2, 0, 2], state: "minecraft:polished_andesite"}, + {pos: [2, 0, 3], state: "minecraft:polished_andesite"}, + {pos: [2, 0, 4], state: "minecraft:polished_andesite"}, + {pos: [3, 0, 0], state: "minecraft:polished_andesite"}, + {pos: [3, 0, 1], state: "minecraft:polished_andesite"}, + {pos: [3, 0, 2], state: "minecraft:polished_andesite"}, + {pos: [3, 0, 3], state: "minecraft:polished_andesite"}, + {pos: [3, 0, 4], state: "minecraft:polished_andesite"}, + {pos: [4, 0, 0], state: "minecraft:polished_andesite"}, + {pos: [4, 0, 1], state: "minecraft:polished_andesite"}, + {pos: [4, 0, 2], state: "minecraft:polished_andesite"}, + {pos: [4, 0, 3], state: "minecraft:polished_andesite"}, + {pos: [4, 0, 4], state: "minecraft:polished_andesite"}, + {pos: [0, 1, 0], state: "minecraft:air"}, + {pos: [0, 1, 1], state: "minecraft:air"}, + {pos: [0, 1, 2], state: "minecraft:air"}, + {pos: [0, 1, 3], state: "minecraft:air"}, + {pos: [0, 1, 4], state: "minecraft:air"}, + {pos: [1, 1, 0], state: "minecraft:air"}, + {pos: [1, 1, 1], state: "minecraft:air"}, + {pos: [1, 1, 2], state: "computercraft:computer_advanced{facing:north,state:on}", nbt: {ComputerId: 1, Label: "disk_drive_test.creates_disk_id", On: 1b, id: "computercraft:computer_advanced"}}, + {pos: [1, 1, 3], state: "minecraft:air"}, + {pos: [1, 1, 4], state: "minecraft:air"}, + {pos: [2, 1, 0], state: "minecraft:air"}, + {pos: [2, 1, 1], state: "minecraft:air"}, + {pos: [2, 1, 2], state: "computercraft:disk_drive{facing:north,state:full}", nbt: {Item: {Count: 1b, id: "computercraft:disk", tag: {Color: 1118481}}, id: "computercraft:disk_drive"}}, + {pos: [2, 1, 3], state: "minecraft:air"}, + {pos: [2, 1, 4], state: "minecraft:air"}, + {pos: [3, 1, 0], state: "minecraft:air"}, + {pos: [3, 1, 1], state: "minecraft:air"}, + {pos: [3, 1, 2], state: "minecraft:air"}, + {pos: [3, 1, 3], state: "minecraft:air"}, + {pos: [3, 1, 4], state: "minecraft:air"}, + {pos: [4, 1, 0], state: "minecraft:air"}, + {pos: [4, 1, 1], state: "minecraft:air"}, + {pos: [4, 1, 2], state: "minecraft:air"}, + {pos: [4, 1, 3], state: "minecraft:air"}, + {pos: [4, 1, 4], state: "minecraft:air"}, + {pos: [0, 2, 0], state: "minecraft:air"}, + {pos: [0, 2, 1], state: "minecraft:air"}, + {pos: [0, 2, 2], state: "minecraft:air"}, + {pos: [0, 2, 3], state: "minecraft:air"}, + {pos: [0, 2, 4], state: "minecraft:air"}, + {pos: [1, 2, 0], state: "minecraft:air"}, + {pos: [1, 2, 1], state: "minecraft:air"}, + {pos: [1, 2, 2], state: "minecraft:air"}, + {pos: [1, 2, 3], state: "minecraft:air"}, + {pos: [1, 2, 4], state: "minecraft:air"}, + {pos: [2, 2, 0], state: "minecraft:air"}, + {pos: [2, 2, 1], state: "minecraft:air"}, + {pos: [2, 2, 2], state: "minecraft:air"}, + {pos: [2, 2, 3], state: "minecraft:air"}, + {pos: [2, 2, 4], state: "minecraft:air"}, + {pos: [3, 2, 0], state: "minecraft:air"}, + {pos: [3, 2, 1], state: "minecraft:air"}, + {pos: [3, 2, 2], state: "minecraft:air"}, + {pos: [3, 2, 3], state: "minecraft:air"}, + {pos: [3, 2, 4], state: "minecraft:air"}, + {pos: [4, 2, 0], state: "minecraft:air"}, + {pos: [4, 2, 1], state: "minecraft:air"}, + {pos: [4, 2, 2], state: "minecraft:air"}, + {pos: [4, 2, 3], state: "minecraft:air"}, + {pos: [4, 2, 4], state: "minecraft:air"}, + {pos: [0, 3, 0], state: "minecraft:air"}, + {pos: [0, 3, 1], state: "minecraft:air"}, + {pos: [0, 3, 2], state: "minecraft:air"}, + {pos: [0, 3, 3], state: "minecraft:air"}, + {pos: [0, 3, 4], state: "minecraft:air"}, + {pos: [1, 3, 0], state: "minecraft:air"}, + {pos: [1, 3, 1], state: "minecraft:air"}, + {pos: [1, 3, 2], state: "minecraft:air"}, + {pos: [1, 3, 3], state: "minecraft:air"}, + {pos: [1, 3, 4], state: "minecraft:air"}, + {pos: [2, 3, 0], state: "minecraft:air"}, + {pos: [2, 3, 1], state: "minecraft:air"}, + {pos: [2, 3, 2], state: "minecraft:air"}, + {pos: [2, 3, 3], state: "minecraft:air"}, + {pos: [2, 3, 4], state: "minecraft:air"}, + {pos: [3, 3, 0], state: "minecraft:air"}, + {pos: [3, 3, 1], state: "minecraft:air"}, + {pos: [3, 3, 2], state: "minecraft:air"}, + {pos: [3, 3, 3], state: "minecraft:air"}, + {pos: [3, 3, 4], state: "minecraft:air"}, + {pos: [4, 3, 0], state: "minecraft:air"}, + {pos: [4, 3, 1], state: "minecraft:air"}, + {pos: [4, 3, 2], state: "minecraft:air"}, + {pos: [4, 3, 3], state: "minecraft:air"}, + {pos: [4, 3, 4], state: "minecraft:air"}, + {pos: [0, 4, 0], state: "minecraft:air"}, + {pos: [0, 4, 1], state: "minecraft:air"}, + {pos: [0, 4, 2], state: "minecraft:air"}, + {pos: [0, 4, 3], state: "minecraft:air"}, + {pos: [0, 4, 4], state: "minecraft:air"}, + {pos: [1, 4, 0], state: "minecraft:air"}, + {pos: [1, 4, 1], state: "minecraft:air"}, + {pos: [1, 4, 2], state: "minecraft:air"}, + {pos: [1, 4, 3], state: "minecraft:air"}, + {pos: [1, 4, 4], state: "minecraft:air"}, + {pos: [2, 4, 0], state: "minecraft:air"}, + {pos: [2, 4, 1], state: "minecraft:air"}, + {pos: [2, 4, 2], state: "minecraft:air"}, + {pos: [2, 4, 3], state: "minecraft:air"}, + {pos: [2, 4, 4], state: "minecraft:air"}, + {pos: [3, 4, 0], state: "minecraft:air"}, + {pos: [3, 4, 1], state: "minecraft:air"}, + {pos: [3, 4, 2], state: "minecraft:air"}, + {pos: [3, 4, 3], state: "minecraft:air"}, + {pos: [3, 4, 4], state: "minecraft:air"}, + {pos: [4, 4, 0], state: "minecraft:air"}, + {pos: [4, 4, 1], state: "minecraft:air"}, + {pos: [4, 4, 2], state: "minecraft:air"}, + {pos: [4, 4, 3], state: "minecraft:air"}, + {pos: [4, 4, 4], state: "minecraft:air"} + ], + entities: [], + palette: [ + "minecraft:polished_andesite", + "minecraft:air", + "computercraft:computer_advanced{facing:north,state:on}", + "computercraft:disk_drive{facing:north,state:full}" + ] +}