Skip to content

test(trade): seed one stack per deposit so every transfer is a pick-up and place - #4030

Merged
u9g merged 1 commit into
masterfrom
test/trade-one-stack-per-deposit
Sep 6, 2026
Merged

test(trade): seed one stack per deposit so every transfer is a pick-up and place#4030
u9g merged 1 commit into
masterfrom
test/trade-one-stack-per-deposit

Conversation

@u9g

@u9g u9g commented Aug 30, 2026

Copy link
Copy Markdown
Member

Rebased on master. The first commit is the test half of #3983 (trades = 2): with master's 11 uses, one stack per deposit would need 44 stacks, more than the inventory holds. If #3983 merges first this rebases down to the second commit.

Problem

trade seeds two 64-stacks of emeralds. bot.transfer takes the first stack of the item it finds, so each deposit picks up 64, right-clicks the price in one at a time and puts the rest back — 36 right clicks per deposit for the wooden sword trade. Every click is a server tick, so pre-1.14 the test spends ~5s in 101 clicks.

Changes

Seed one stack of exactly the price for each deposit, in the order the trades consume them ([p0, p0, 2, 2, 1, 1, 36, 36] emeralds and [1, 1] books). transfer then finds the exact stack first and every deposit is pick-up + place. Assertions are unchanged; they only check totals.

The slots are set with Promise.all (as clearInventory already does): on 1.21.3+ (noAckOnCreateSetSlotPacket) there is no ack for set_creative_slot, so each setInventorySlot waits 400ms for a rejection; serialised over 10 slots that is 4s.

Verification

trade on the base branch vs this PR (clicks are window_clicks from the packet trace):

Version Clicks Before After
1.8.8 101 → 34 6976ms 3577ms
1.9.4 101 → 34 7075ms 3625ms
1.13.2 101 → 34 7037ms 3232ms
1.16.5 2348ms
1.21.8 29 → 21 2909ms 2073ms
26.1 29 → 21 3497ms 2618ms

On 1.14+ the server moves whole stacks into the trade slots itself when a trade is selected, so the click count barely changes there; the gain is from the concurrent slot setup. The remaining ~1s of the pre-1.14 runs is the two command-block sleeps (#4029).

@u9g
u9g force-pushed the test/trade-one-stack-per-deposit branch from 8f0c869 to dd9b75c Compare August 30, 2026 18:02
@u9g
u9g changed the base branch from test/trade-fewer-uses to master August 30, 2026 18:03
@u9g
u9g force-pushed the test/trade-one-stack-per-deposit branch 2 times, most recently from 95b80c6 to 5a6f188 Compare September 4, 2026 21:11
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Tests more than 1.5x slower than master (durations are noisy, so this is informational):

   3513ms ->   11076ms  mineflayer_external 1.10.2v nether
   3411ms ->   10486ms  mineflayer_external 1.11.2v nether
   3438ms ->    9826ms  mineflayer_external 1.8.8v nether
   3459ms ->   12114ms  mineflayer_external 1.9.4v nether
   1440ms ->    7176ms  mineflayer_external 1.16.5v activateItem
  12915ms ->   20360ms  mineflayer_external 1.17.1v nether
  12936ms ->   24698ms  mineflayer_external 1.19v nether
   4955ms ->   14772ms  mineflayer_external 1.21.3v nether
   9601ms ->   15542ms  mineflayer_external 1.21.4v nether
   8689ms ->   13726ms  mineflayer_external 1.21.6v nether

@rom1504

rom1504 commented Sep 5, 2026

Copy link
Copy Markdown
Member

Approved, resolved conflict and merge

…p and place

transfer takes the first stack of the item it finds, so with two 64-stacks
of emeralds each deposit picks up a stack, right-clicks the price in one
at a time and puts the rest back: 36 right clicks for the sword trade.
Seeding a stack of exactly the price for each deposit, in the order the
trades consume them, makes every deposit two clicks: 101 -> 34 clicks
before 1.14. On 1.14+ the server moves the stacks itself (29 -> 21).

The slots are set concurrently: on 1.21.3+ there is no creative slot ack
and each set waits 400ms for a rejection, which serialised would cost 4s.
@u9g
u9g force-pushed the test/trade-one-stack-per-deposit branch from 5a6f188 to 4556967 Compare September 6, 2026 13:01
@u9g
u9g merged commit b293d7f into master Sep 6, 2026
11 checks passed
@u9g
u9g deleted the test/trade-one-stack-per-deposit branch September 6, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants