-
-
Notifications
You must be signed in to change notification settings - Fork 926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update digging.js for state changes #3136
base: master
Are you sure you want to change the base?
Conversation
Made digging.js compatible with state changes, i.e. when the bot switches from being on ground to not on ground and in water, the digging progress is saved and accurately updated.
I have no idea if this would break other people's projects due to variable name changes/complete functionality differences, but the reason for suggesting this fix would be because currently if the bot starts digging a block in one state, i.e. in the water, and then exits the water, or vice versa, the time needed till completion is not adjusted accordingly. |
/fixlint |
please remove commented code |
I ran > [email protected] fix > standard --fix && standard-markdown --fix /home/runner/work/mineflayer/mineflayer/lib/plugins/digging.js:24:30: Expected '!==' and instead saw '!='. (eqeqeq) /home/runner/work/mineflayer/mineflayer/lib/plugins/digging.js:114:11: 'waitTime' is assigned a value but never used. (no-unused-vars) standard: Use JavaScript Standard Style (https://standardjs.com) |
I believe I have made the appropriate changes, though I missed a piece of commented debugging code on line 75. |
lib/plugins/digging.js
Outdated
// bot.referenceTime = window.performance.now(); | ||
waitInterval = setInterval(() => { //! !!! | ||
bot.digPercentage += (50 / bot.digTime(block)) | ||
// bot.digPercentage += (Math.abs(bot.referenceTime - window.performance.now()) / bot.digTime(block)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented code
Clean it up please |
lib/plugins/digging.js
Outdated
0.5 + (i === 'y' ? visibleFaces[i] * 0.5 : 0), | ||
0.5 + (i === 'z' ? visibleFaces[i] * 0.5 : 0) | ||
) | ||
const targetPos = block.position.offset(0.5 + (i === 'x' ? visibleFaces[i] * 0.5 : 0), 0.5 + (i === 'y' ? visibleFaces[i] * 0.5 : 0), 0.5 + (i === 'z' ? visibleFaces[i] * 0.5 : 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
const dx = bot.entity.position.x - (block.position.x + 0.5) | ||
const dy = bot.entity.position.y + bot.entity.height - (block.position.y + 0.5) | ||
const dz = bot.entity.position.z - (block.position.z + 0.5) | ||
const dx = bot.entity.position.x - block.position.x + 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
const dx = bot.entity.position.x - block.position.x + 0.5 | ||
const dy = bot.entity.position.y - block.position.y - 0.5 + bot.entity.height // -0.5 because the bot position | ||
// is calculated from the block position that is inside its feet so 0.5 - 1 = -0.5 | ||
const dz = bot.entity.position.z - block.position.z + 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incorrect
lib/plugins/digging.js
Outdated
const cDist = new Vec3(tPos.x, tPos.y, tPos.z).distanceSquared( | ||
bot.entity.position.offset(0, bot.entity.height, 0) | ||
) | ||
const cDist = new Vec3(tPos.x, tPos.y, tPos.z).distanceSquared(bot.entity.position.offset(0, bot.entity.height, 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
rayPos.y === block.position.y && | ||
rayPos.z === block.position.z | ||
) { | ||
if (rayPos.x === block.position.x && rayPos.y === block.position.y && rayPos.z === block.position.z) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
enchantments, | ||
bot.entity.effects | ||
) | ||
return block.digTime(type, creative, bot.entity.isInWater, !bot.entity.onGround, enchantments, bot.entity.effects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
block.diggable && | ||
block.position.offset(0.5, 0.5, 0.5).distanceTo(bot.entity.position.offset(0, 1.65, 0)) <= 5.1 | ||
) | ||
return block && block.diggable && block.position.offset(0.5, 0.5, 0.5).distanceTo(bot.entity.position.offset(0, 1.65, 0)) <= 5.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
block.position.offset(0.5, 0.5, 0.5).offset(digFace.x * 0.5, digFace.y * 0.5, digFace.z * 0.5), | ||
forceLook | ||
) | ||
await bot.lookAt(block.position.offset(0.5, 0.5, 0.5).offset(digFace.x * 0.5, digFace.y * 0.5, digFace.z * 0.5), forceLook) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only a formatting change, undo
lib/plugins/digging.js
Outdated
@@ -162,13 +184,14 @@ function inject (bot) { | |||
function onBlockUpdate (oldBlock, newBlock) { | |||
// vanilla server never actually interrupt digging, but some server send block update when you start digging | |||
// so ignore block update if not air | |||
// All block update listeners receive (null, null) when the world is unloaded. So newBlock can be null. | |||
if (newBlock?.type !== 0) return | |||
if (!newBlock) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't change anything, see the ? below
you can remove this change
} else { | ||
clearInterval(swingInterval) | ||
} | ||
}, 250) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the change from 350 to 250 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Vakore can you change this back or give w reason to why you changed the interval?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the interval to fix the arm swing bug
waitTimeout = setTimeout(finishDigging, waitTime) | ||
if (waitInterval) { clearInterval(waitInterval) } | ||
waitInterval = setInterval(() => { | ||
bot.digPercentage += (50 / bot.digTime(block)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the vanilla client follow this algorithm when state change while digging ? do you have a reference ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not this exact algorithm but it mimics what I've seen in game, and I've yet to see something abnormal from it.
@@ -162,13 +184,13 @@ function inject (bot) { | |||
function onBlockUpdate (oldBlock, newBlock) { | |||
// vanilla server never actually interrupt digging, but some server send block update when you start digging | |||
// so ignore block update if not air | |||
// All block update listeners receive (null, null) when the world is unloaded. So newBlock can be null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably on accident.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the comment back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a more updated version of digging.js on the discord. Search for attachments from 'Vakore', unable to do stuff with npm rn.
bot.on('heldItemChanged', () => { | ||
let nameToCompare = '' | ||
if (bot.heldItem) { nameToCompare = bot.heldItem.name } | ||
if (bot.lastHeldItemName !== nameToCompare) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think any held item change cancels the dig process? Even increasing the item amount in a slot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly no. If you have two pickaxes with equal durability and/or nbt, and switch between them, or switch between different fist slots while mining, it won't cancel the mining timer. Mining a tree with fists and picking up a log while mining will reset it, though constantly mining with logs while the stack size increases doesn't last time I checked. I'll double check later today but I'm pretty sure this is correct.
Though now that I'm looking at this the system here won't take into account durability/enchant differences, though that's just nbt and can likely be changed easily. This system will still work in most cases though and is better than nothing, but it should be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better way to check if items are equal is the stringify the item and compare that. You could make a string of the item you started mining with and reset the progress if the string of the held item changes.
Btw you don't have to use an external site for linting. Mineflayer comes with its own linter. Just do |
@IceTank do you want to check this one and tell me if it looks good? |
Made digging.js compatible with state changes, i.e. when the bot switches from being on ground to not on ground and in water, the digging progress is saved and accurately updated.