fix: clear a chest's lid open count when its block is replaced; make useChests await its fill commands

lib/plugins/block_actions.js

@@ -22,6 +22,13 @@ function inject (bot) {
   // Stores how many players have currently open a container at a certain position
   const openCountByPos = {}
 
+  // The server drops the closing block event of a container that is no
+  // longer there, so a replaced container must not keep its open count.
+  bot.on('blockUpdate', (oldBlock, newBlock) => {
+    if (!newBlock || oldBlock?.name === newBlock.name) return
+    delete openCountByPos[newBlock.position]
+  })
+
   function parseChestMetadata (chestBlock) {
     const chestTypes = ['single', 'right', 'left']
 

test/externalTests/plugins/testCommon.js

@@ -36,6 +36,7 @@ function inject (bot, wrap) {
   bot.test.fly = fly
   bot.test.teleport = teleport
   bot.test.resetState = resetState
+  bot.test.awaitCommandsProcessed = awaitCommandsProcessed
   bot.test.setInventorySlot = setInventorySlot
   bot.test.placeBlock = placeBlock
   bot.test.runExample = runExample
@@ -93,13 +94,7 @@ function inject (bot, wrap) {
     // The marker echo only proves the fills executed: command feedback is
     // sent immediately while block changes flush at tick end, so the client
     // can still hold pre-fill blocks after the echo.
-    const marker = 'superflat-reset-done'
-    const echo = onceWithCleanup(bot, 'messagestr', {
-      timeout: 5000,
-      checkCondition: (message) => message.includes(marker)
-    })
-    bot.chat(marker)
-    await echo
+    await awaitCommandsProcessed('superflat-reset-done')
     const staleBlock = () => {
       for (let y = groundY + 4; y >= groundY - 1; y--) {
         const realY = y + bot.test.groundY - 4
@@ -139,6 +134,21 @@ function inject (bot, wrap) {
     }
   }
 
+  // Chat and commands run in order on the server's main thread, so the echo
+  // of a message sent after a batch of commands proves the batch has executed.
+  // Command feedback is sent immediately while block changes flush at tick
+  // end, so a caller reading blocks a command just changed still has to wait
+  // for them; but block interaction packets are not ordered behind commands
+  // on 1.21.9+, so this must precede acting on such a block.
+  async function awaitCommandsProcessed (marker) {
+    const echo = onceWithCleanup(bot, 'messagestr', {
+      timeout: 5000,
+      checkCondition: (message) => message.includes(marker)
+    })
+    bot.chat(marker)
+    await echo
+  }
+
   async function placeBlock (slot, position) {
     bot.setQuickBarSlot(slot - 36)
     // always place the block on the top of the block below it, i guess.

test/externalTests/useChests.js

@@ -161,7 +161,7 @@ module.exports = () => async (bot) => {
   // Write the slots server side. Filling them by clicking leaves the result at
   // the mercy of the client's predicted state, which from 1.17 is not confirmed
   // per click, and a lost move is invisible until an assertion reads the slot.
-  function fillChest (pos) {
+  async function fillChest (pos) {
     const at = `${pos.x} ${pos.y} ${pos.z}`
     for (const { slot, name, count } of layout) {
       const item = bot.registry.itemsByName[name]
@@ -174,6 +174,7 @@ module.exports = () => async (bot) => {
         bot.chat(`/replaceitem block ${at} container.${slot} ${name} ${count}`)
       }
     }
+    await bot.test.awaitCommandsProcessed('chest-filled')
   }
 
   // Each left/right click resolves differently depending on whether the cursor
@@ -255,7 +256,7 @@ module.exports = () => async (bot) => {
     bot.chat(`/setblock ${largeChestLocations[1].x} ${largeChestLocations[1].y} ${largeChestLocations[1].z} chest`)
   }
 
-  fillChest(largeChestLocations[0])
+  await fillChest(largeChestLocations[0])
   const window = await bot.openContainer(bot.blockAt(largeChestLocations[0]))
   // Which half of a double chest the window lists first depends on the version.
   for (const { name, count } of layout) {

test/internalTest.js

@@ -706,6 +706,35 @@ for (const supportedVersion of mineflayer.testedVersions) {
       })
     })
 
+    describe('block actions', () => {
+      it('emits chestLidMove again once an open chest has been replaced', async () => {
+        const pos = vec3(1, 65, 1)
+        const chestId = bot.registry.blocksByName.chest.id
+        const location = { x: pos.x, y: pos.y, z: pos.z }
+        const [client] = await once(server, 'playerJoin')
+        client.write('login', bot.test.generateLoginPacket())
+        const chunk = bot.test.buildChunk()
+        chunk.setBlockType(pos, chestId)
+        client.write('map_chunk', generateChunkPacket(chunk))
+        await once(bot, 'chunkColumnLoad')
+        const chestStateId = bot.blockAt(pos).stateId
+
+        const opened = onceWithCleanup(bot, 'chestLidMove', { timeout: 2000 })
+        client.write('block_action', { location, byte1: 1, byte2: 1, blockId: chestId })
+        await opened
+
+        // Breaking the chest while it is open never yields a closing block
+        // action, so the open count must not survive the block change.
+        client.write('block_change', { location, type: 0 })
+        client.write('block_change', { location, type: chestStateId })
+        const reopened = onceWithCleanup(bot, 'chestLidMove', { timeout: 2000 })
+        client.write('block_action', { location, byte1: 1, byte2: 1, blockId: chestId })
+        const [block, isOpen] = await reopened
+        assert.ok(block.position.equals(pos))
+        assert.strictEqual(isOpen, 1)
+      })
+    })
+
     describe('entities', () => {
       it('entity id changes on login', (done) => {
         const loginPacket = bot.test.generateLoginPacket()