From de819c21f282349c1c19eeea42bb877e091f8400 Mon Sep 17 00:00:00 2001 From: Evan Goode <mail@evangoo.de> Date: Sat, 11 Nov 2023 15:05:26 -0500 Subject: [PATCH 1/2] Fix several bugs in villager trading These changes should fix issues with trades that have more than one item in the output stack, trades that cost more than one emerald, and trades with two input items, such as trades for enchanted books and tipped arrows. --- lib/plugins/inventory.js | 17 ++++++++++-- lib/plugins/villager.js | 59 +++++++++++++++------------------------- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/lib/plugins/inventory.js b/lib/plugins/inventory.js index b532b95c2..7412bea05 100644 --- a/lib/plugins/inventory.js +++ b/lib/plugins/inventory.js @@ -391,7 +391,12 @@ function inject (bot, { hideErrors }) { } function tradeMatch (limitItem, targetItem) { - return targetItem.type === limitItem.type && targetItem.count >= limitItem.count + return ( + targetItem !== null && + limitItem !== null && + targetItem.type === limitItem.type && + targetItem.count >= limitItem.count + ) } function expectTradeUpdate (window) { @@ -399,7 +404,7 @@ function inject (bot, { hideErrors }) { const hasItem = !!window.slots[2] if (hasItem !== tradeMatch(trade.inputItem1, window.slots[0])) { - if (trade.hasItems2) { + if (trade.hasItem2) { return hasItem !== tradeMatch(trade.inputItem2, window.slots[1]) } return true @@ -418,7 +423,7 @@ function inject (bot, { hideErrors }) { } } else if (window.type === 'minecraft:merchant') { const toUpdate = [] - if (slot <= 2 && !window.selectedTrade.tradeDisabled && expectTradeUpdate(window)) { + if (slot <= 1 && !window.selectedTrade.tradeDisabled && expectTradeUpdate(window)) { toUpdate.push(once(bot.currentWindow, 'updateSlot:2')) } if (slot === 2) { @@ -427,6 +432,12 @@ function inject (bot, { hideErrors }) { } } await Promise.all(toUpdate) + + if (slot === 2 && !window.selectedTrade.tradeDisabled && expectTradeUpdate(window)) { + // After the trade goes through, if the inputs are still satisfied, + // expect another update in slot 2 + await once(bot.currentWindow, 'updateSlot:2') + } } } diff --git a/lib/plugins/villager.js b/lib/plugins/villager.js index c8f412546..e6fdf5f57 100644 --- a/lib/plugins/villager.js +++ b/lib/plugins/villager.js @@ -141,8 +141,11 @@ function inject (bot, { version }) { itemCount2 = villager.count(Trade.inputItem2.type, Trade.inputItem2.metadata) hasEnoughItem2 = itemCount2 >= Trade.inputItem2.count * count } - if (!hasEnoughItem1 || !hasEnoughItem2) { - throw new Error('Not enough items to trade') + if (!hasEnoughItem1) { + throw new Error('Not enough item 1 to trade') + } + if (!hasEnoughItem2) { + throw new Error('Not enough item 2 to trade') } selectTrade(choice) @@ -152,31 +155,12 @@ function inject (bot, { version }) { if (Trade.hasItem2) proms.push(once(villager, 'updateSlot:1')) if (bot.supportFeature('setSlotAsTransaction')) { proms.push(once(villager, 'updateSlot:2')) - await new Promise((resolve, reject) => { - let countOfItemOneLeftToTake = itemCount1 > 64 ? 64 : itemCount1 - let countOfItemTwoLeftToTake = 0 - if (Trade.hasItem2) { - countOfItemTwoLeftToTake = itemCount2 > 64 ? 64 : itemCount2 - } - const listener = (slot, oldItem, newItem) => { - if (!(slot >= villager.inventoryStart && slot <= villager.inventoryEnd)) return - if (newItem === null) { - if (oldItem.type === Trade.inputItem1.type) countOfItemOneLeftToTake -= oldItem.count - else if (Trade.hasItem2 && oldItem.type === Trade.inputItem2.type) countOfItemTwoLeftToTake -= oldItem.count - } - if (countOfItemOneLeftToTake === 0 && countOfItemTwoLeftToTake === 0) { - villager.off('updateSlot', listener) - resolve() - } - } - villager.on('updateSlot', listener) - }) } await Promise.all(proms) } for (let i = 0; i < count; i++) { - await putRequirements(villager, Trade, count) + await putRequirements(villager, Trade) // ToDo: See if this does anything kappa Trade.nbTradeUses++ if (Trade.maximumNbTradeUses - Trade.nbTradeUses === 0) { @@ -215,24 +199,25 @@ function inject (bot, { version }) { } } - async function putRequirements (window, Trade, count) { + async function putRequirements (window, Trade) { const [slot1, slot2] = window.slots - const { stackSize: stackSize1, type: type1, metadata: metadata1 } = Trade.inputItem1 - const tradeCount1 = Trade.realPrice - const neededCount1 = Math.min(stackSize1, tradeCount1 * count) - - const input1 = !slot1 - ? neededCount1 - : (slot1.count < tradeCount1 ? neededCount1 - slot1.count : 0) - await deposit(window, type1, metadata1, input1, 0) + const { type: type1, metadata: metadata1 } = Trade.inputItem1 + + const input1 = slot1 + ? Math.max(0, Trade.realPrice - slot1.count) + : Trade.realPrice + if (input1) { + await deposit(window, type1, metadata1, input1, 0) + } if (Trade.hasItem2) { - const { count: tradeCount2, stackSize: stackSize2, type: type2, metadata: metadata2 } = Trade.inputItem2 - const needCount2 = Math.min(stackSize2, tradeCount2 * count) + const { count: tradeCount2, type: type2, metadata: metadata2 } = Trade.inputItem2 - const input2 = !slot2 - ? needCount2 - : (slot2.count < tradeCount2 ? needCount2 - slot2.count : 0) - await deposit(window, type2, metadata2, input2, 1) + const input2 = slot2 + ? Math.max(0, tradeCount2 - slot2.count) + : tradeCount2 + if (input2) { + await deposit(window, type2, metadata2, input2, 1) + } } } From b1abd2463777bcf3dbf12f63f3f92ca389519765 Mon Sep 17 00:00:00 2001 From: Evan Goode <mail@evangoo.de> Date: Thu, 21 Dec 2023 17:34:08 -0500 Subject: [PATCH 2/2] Add more tests for villager trading - Add a test trade taking 1 emerald and returning 4 glass - Add a test trade taking [36 emeralds, 1 book] and returning 1 wooden sword (any unstackable should work to accurately simulate an enchanted_book trade) - Increase maximumNbTradeUses to 12 for all test trades --- test/externalTests/trade.js | 71 +++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/test/externalTests/trade.js b/test/externalTests/trade.js index 0718a1f1d..1ad502bc8 100644 --- a/test/externalTests/trade.js +++ b/test/externalTests/trade.js @@ -6,14 +6,20 @@ module.exports = () => async (bot) => { const villagerType = bot.registry.entitiesByName.villager ? 'villager' : 'Villager' const testFluctuations = bot.supportFeature('selectingTradeMovesItems') + const summonCommand = bot.supportFeature('indexesVillagerRecipes') - ? `/summon ${villagerType} ~ ~1 ~ {NoAI:1, Offers:{Recipes:[0:{maxUses:7,buy:{id:"minecraft:emerald",Count:2},sell:{id:"minecraft:pumpkin_pie",Count:2},uses: 1},1:{maxUses:7,buy:{id:"minecraft:emerald",Count:2},buyB:{id:"minecraft:pumpkin_pie",Count:2},sell:{id:"minecraft:wheat",Count:2}, uses:1}]}}` - : `/summon ${villagerType} ~ ~1 ~ {NoAI:1, Offers:{Recipes:[{maxUses:7,buy:{id:"minecraft:emerald",Count:2},sell:{id:"minecraft:pumpkin_pie",Count:2},${testFluctuations ? 'demand:60,priceMultiplier:0.05f,specialPrice:-4,' : ''}uses: 1},{maxUses:7,buy:{id:"minecraft:emerald",Count:2},buyB:{id:"minecraft:pumpkin_pie",Count:2},sell:{id:"minecraft:wheat",Count:2}, uses:1}]}}` + ? `/summon ${villagerType} ~ ~1 ~ {NoAI:1, Offers:{Recipes:[0:{maxUses:12,buy:{id:"minecraft:emerald",Count:2},sell:{id:"minecraft:pumpkin_pie",Count:2},uses: 1},1:{maxUses:12,buy:{id:"minecraft:emerald",Count:2},buyB:{id:"minecraft:pumpkin_pie",Count:2},sell:{id:"minecraft:wheat",Count:2}, uses:1},2:{maxUses:12,buy:{id:"minecraft:emerald",Count:1},sell:{id:"minecraft:glass",Count:4},uses: 1},3:{maxUses:12,buy:{id:"minecraft:emerald",Count:36},buyB:{id:"minecraft:book",Count:1},sell:{id:"minecraft:wooden_sword",Count:1},uses: 1}]}}` + : `/summon ${villagerType} ~ ~1 ~ {NoAI:1, Offers:{Recipes:[{maxUses:12,buy:{id:"minecraft:emerald",Count:2},sell:{id:"minecraft:pumpkin_pie",Count:2},${testFluctuations ? 'demand:60,priceMultiplier:0.05f,specialPrice:-4,' : ''}uses: 1},{maxUses:12,buy:{id:"minecraft:emerald",Count:2},buyB:{id:"minecraft:pumpkin_pie",Count:2},sell:{id:"minecraft:wheat",Count:2}, uses:1},{maxUses:12,buy:{id:"minecraft:emerald",Count:1},sell:{id:"minecraft:glass",Count:4},uses: 1},{maxUses:12,buy:{id:"minecraft:emerald",Count:36},buyB:{id:"minecraft:book",Count:1},sell:{id:"minecraft:wooden_sword",Count:1},uses: 1}]}}` const commandBlockPos = bot.entity.position.offset(0.5, 0, 0.5) const redstoneBlockPos = commandBlockPos.offset(1, 0, 0) - await bot.test.setInventorySlot(36, new Item(bot.registry.itemsByName.emerald.id, 64, 0)) + let shouldHaveEmeralds = 0 + for (let slot = 9; slot <= 17; slot += 1) { + await bot.test.setInventorySlot(slot, new Item(bot.registry.itemsByName.emerald.id, 64, 0)) + shouldHaveEmeralds += 64 + } + await bot.test.setInventorySlot(18, new Item(bot.registry.itemsByName.book.id, 11, 0)) // A command block is needed to spawn the villager due to the chat's character limit in some versions bot.test.sayEverywhere(`/setblock ${commandBlockPos.toArray().join(' ')} command_block`) @@ -40,9 +46,10 @@ module.exports = () => async (bot) => { assert.strictEqual(output.name, 'pumpkin_pie') assert.strictEqual(output.count, 2) - await bot.trade(villager, 0, 6) - assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), testFluctuations ? 64 - 24 : 64 - 12) - assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.pumpkin_pie.id), 12) + await bot.trade(villager, 0, 11) + shouldHaveEmeralds -= testFluctuations ? (2 * 2 * 11) : (2 * 11) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), shouldHaveEmeralds) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.pumpkin_pie.id), 22) } // Handle trade #2 -- takes [2x emerald, 2x pumpkin_pie] and returns 2x wheat @@ -61,15 +68,59 @@ module.exports = () => async (bot) => { assert.strictEqual(output.name, 'wheat') assert.strictEqual(output.count, 2) - await bot.trade(villager, 1, 6) - assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), testFluctuations ? 64 - 36 : 64 - 24) + await bot.trade(villager, 1, 11) + shouldHaveEmeralds -= 11 * 2 + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), shouldHaveEmeralds) assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.pumpkin_pie.id), 0) - assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.wheat.id), 12) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.wheat.id), 22) + } + + // Handle trade #3 -- takes 1x emerald and returns 4x glass + { + const trade = villager.trades[2] + assert.strictEqual(trade.inputs.length, 1, 'Expected single input from villager on first trade') + verifyTrade(trade) + + const [input] = trade.inputs + assert.strictEqual(input.name, 'emerald') + assert.strictEqual(input.count, 1) + + const [output] = trade.outputs + assert.strictEqual(output.name, 'glass') + assert.strictEqual(output.count, 4) + + await bot.trade(villager, 2, 11) + shouldHaveEmeralds -= 11 + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), shouldHaveEmeralds) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.glass.id), 44) + } + + // Handle trade #4 -- takes [36x emerald, 1x book] and returns 1x wooden sword + { + const trade = villager.trades[3] + assert.strictEqual(trade.inputs.length, 2, 'Expected two inputs from villager on second trade') + verifyTrade(trade) + + const [input1, input2] = trade.inputs + assert.strictEqual(input1.name, 'emerald') + assert.strictEqual(input1.count, 36) + assert.strictEqual(input2.name, 'book') + assert.strictEqual(input2.count, 1) + + const [output] = trade.outputs + assert.strictEqual(output.name, 'wooden_sword') + assert.strictEqual(output.count, 1) + + await bot.trade(villager, 3, 11) + shouldHaveEmeralds -= 11 * 36 + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.emerald.id), shouldHaveEmeralds) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.book.id), 0) + assert.strictEqual(bot.currentWindow.count(bot.registry.itemsByName.wooden_sword.id), 11) } function verifyTrade (trade) { assert.strictEqual(trade.nbTradeUses, 1) - assert.strictEqual(trade.maximumNbTradeUses, 7) + assert.strictEqual(trade.maximumNbTradeUses, 12) assert.strictEqual(trade.tradeDisabled, false) const printCountInv = function (item) {