Atomic update of disk drive item stacks

Disk drives have had a long-standing issue with mutating their contents
on the computer thread, potentially leading to all sorts of odd bugs.

We tried to fix this by moving setDiskLabel and the mounting code to run
on the main thread. Unfortunately, this means there is a slight delay to
mounts being attached, breaking disk startup.

This commit implements an alternative solution - we now do mounting on
the computer thread again. If the disk's stack is modified, we update it
in the peripheral-facing item, but not the actual inventory. The next
time the disk drive is ticked, we then sync the two items.

This does mean that there is a fraction of a tick where the two will be
out-of-sync. This isn't ideal - it would potentially be possible to
cycle through disk ids - but I don't really think that's avoidable
without significantly complicating the IMedia API.

Fixes #1649, fixes #1686.

projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Disk_Drive_Test.kt

@@ -5,6 +5,7 @@
 package dan200.computercraft.gametest
 
 import dan200.computercraft.core.apis.FSAPI
+import dan200.computercraft.core.util.Colour
 import dan200.computercraft.gametest.api.*
 import dan200.computercraft.shared.ModRegistry
 import dan200.computercraft.shared.media.items.DiskItem
@@ -19,6 +20,9 @@ import net.minecraft.network.chat.Component
 import net.minecraft.world.item.ItemStack
 import net.minecraft.world.item.Items
 import net.minecraft.world.level.block.RedStoneWireBlock
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.array
+import org.hamcrest.Matchers.equalTo
 import org.junit.jupiter.api.Assertions.assertEquals
 
 class Disk_Drive_Test {
@@ -45,14 +49,48 @@ class Disk_Drive_Test {
         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 Queues_event(helper: GameTestHelper) = helper.sequence {
+        val pos = BlockPos(1, 2, 2)
+
+        var started = false
+        var disk = false
+        var ejected = false
+        thenStartComputer {
+            // thenOnComputer discards events, so instead we need to track our state transitions.
+            started = true
+
+            val diskEvent = pullEvent("disk")
+            assertThat(diskEvent, array(equalTo("disk"), equalTo("right")))
+
+            disk = true
+
+            val ejectEvent = pullEvent("disk_eject")
+            assertThat(ejectEvent, array(equalTo("disk_eject"), equalTo("right")))
+
+            ejected = true
+        }
+
+        thenWaitUntil { helper.assertTrue(started, "Computer not started") }
+        thenExecute { helper.setContainerItem(pos, 0, ItemStack(Items.DIRT)) }
+        thenWaitUntil { helper.assertTrue(disk, "disk not inserted") }
+        thenExecute { helper.setContainerItem(pos, 0, ItemStack.EMPTY) }
+        thenWaitUntil { helper.assertTrue(ejected, "disk not ejected") }
+    }
+
     /**
      * A mount is initially attached, and then removed when the disk is ejected.
      */
     @GameTest
     fun Adds_removes_mount(helper: GameTestHelper) = helper.sequence {
         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!
+        thenExecute {
+            helper.setContainerItem(BlockPos(1, 2, 2), 0, DiskItem.createFromIDAndColour(1, null, Colour.BLACK.hex))
+        }
+        thenOnComputer {
             getApi<FSAPI>().getDrive("disk").assertArrayEquals("right")
             callPeripheral("right", "ejectDisk")
         }

projects/common/src/testMod/kotlin/dan200/computercraft/gametest/api/TestExtensions.kt

@@ -168,6 +168,16 @@ fun <T : Comparable<T>> GameTestHelper.assertBlockHas(pos: BlockPos, property: P
     }
 }
 
+/**
+ * Get a [Container] at a given position.
+ */
+fun GameTestHelper.getContainerAt(pos: BlockPos): Container =
+    when (val container = getBlockEntity(pos)) {
+        is Container -> container
+        null -> failVerbose("Expected a container at $pos, found nothing", pos)
+        else -> failVerbose("Expected a container at $pos, found ${getName(container.type)}", pos)
+    }
+
 /**
  * Assert a container contains exactly these items and no more.
  *
@@ -176,10 +186,7 @@ fun <T : Comparable<T>> GameTestHelper.assertBlockHas(pos: BlockPos, property: P
  * first `n` slots - the remaining are required to be empty.
  */
 fun GameTestHelper.assertContainerExactly(pos: BlockPos, items: List<ItemStack>) =
-    when (val container = getBlockEntity(pos) ?: failVerbose("Expected a container at $pos, found nothing", pos)) {
-        is Container -> assertContainerExactlyImpl(pos, container, items)
-        else -> failVerbose("Expected a container at $pos, found ${getName(container.type)}", pos)
-    }
+    assertContainerExactlyImpl(pos, getContainerAt(pos), items)
 
 /**
  * Assert an container contains exactly these items and no more.
@@ -287,3 +294,13 @@ fun GameTestHelper.setBlock(pos: BlockPos, state: BlockInput) = state.place(leve
 fun GameTestHelper.modifyBlock(pos: BlockPos, modify: (BlockState) -> BlockState) {
     setBlock(pos, modify(getBlockState(pos)))
 }
+
+/**
+ * Update items in the container at [pos], setting the item in the specified [slot] to [item], and then marking it
+ * changed.
+ */
+fun GameTestHelper.setContainerItem(pos: BlockPos, slot: Int, item: ItemStack) {
+    val container = getContainerAt(pos)
+    container.setItem(slot, item)
+    container.setChanged()
+}

projects/common/src/testMod/resources/data/cctest/structures/disk_drive_test.adds_removes_mount.snbt

@@ -34,7 +34,7 @@
         {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:disk_drive{facing:north,state:full}", nbt: {Item: {Count: 1b, id: "computercraft:disk", tag: {Color: 1118481, DiskId: 0}}, id: "computercraft:disk_drive"}},
+        {pos: [1, 1, 2], state: "computercraft:disk_drive{facing:north,state:full}", nbt: {id: "computercraft:disk_drive"}},
         {pos: [1, 1, 3], state: "minecraft:air"},
         {pos: [1, 1, 4], state: "minecraft:air"},
         {pos: [2, 1, 0], state: "minecraft:air"},

projects/common/src/testMod/resources/data/cctest/structures/disk_drive_test.queues_event.snbt

@@ -0,0 +1,138 @@
+{
+    DataVersion: 2975,
+    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:disk_drive{facing:north,state:full}", nbt: {id: "computercraft:disk_drive"}},
+        {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:computer_advanced{facing:north,state:blinking}", nbt: {ComputerId: 1, Label: "disk_drive_test.queues_event", On: 1b, id: "computercraft:computer_advanced"}},
+        {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:disk_drive{facing:north,state:full}",
+        "computercraft:computer_advanced{facing:north,state:blinking}"
+    ]
+}