promisify #725

i have not tested it but i think it should work

const mineflayer = require('mineflayer')
const { promisify } = require('util')

const promisifyPlugin = bot => {
  bot.promisify = n => {
    if (!bot[n] || bot[n] instanceof Promise) return
    bot[n] = promisify(bot[n])
  }
  ['wake', ...].forEach(bot.promisify)
}

const bot = mineflayer.createBot(...)
bot.loadPlugin(promisifyPlugin)
bot.once('login', () => {
  bot.wake().then(...)
})

Yes it's of course possible to promisify mineflayer. My point was we could have a mineflayer.promises. That seems like a reasonable choice to support promises without breaking the API

On Thu, Oct 4, 2018, 11:06 plexigras notifications@github.com wrote:

i have not tested it but i think it should work

const mineflayer = require('mineflayer')const { promisify } = require('util') const promisifyPlugin = bot => { bot.promisify = n => { if (!bot[n] || bot[n] instanceof Promise) return bot[n] = promisify(bot[n]) } ['wake', ...].forEach(bot.promisify) } const bot = mineflayer.createBot(...)bot.loadPlugin(promisifyPlugin)bot.once('login', () => { bot.wake().then(...) })

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/mineflayer/issues/725#issuecomment-426942378, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPN_kdtMviOurBWAYwbXBbrJ2u4exTHks5uhc-IgaJpZM4XHDD7 .

I think I can work on that in my spare time

On this something better we can do is support both callback and promise externally. Internally we would use only promises (and async/await)

@rom1504 has funded $2.00 to this issue.

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

@imharvol has funded $2.00 to this issue.

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

@louis030195 has funded $2.00 to this issue.

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

I'm willing to do this, but do you really want to keep supporting callbacks? I think it's beneficial to the quality of the framework to implement this in a breaking way, with everything async/promisified and no more callbacks anywhere.

@ph0t0shop what's the benefit of removing callback for exposed functions ? All internal can use only promises and exposing callback is just something you call on all functions to add it.

That would mean a breaking change and that all the existing projects using mineflayer would have to be updated. IMO it’s not worth a breaking change over, there’s no practical benefit other than a slightly simpler API call.

Example

function callbackify (g) {
return function(a, cb) {
return g.then(r => {cb(r); return r;})
}
}
function f(a) {
const b = a+1
return Promise.resolve(b)
}
module.exports = callbackify(f)

(Callbackify can be generalised to handle any number of arguments easily enough)

My point is it would not even be simpler.

Read https://github.com/PrismarineJS/mineflayer/pull/984 too

On Tue, Dec 1, 2020, 02:16 ph0t0shop notifications@github.com wrote:

I'm willing to do this, but do you really want to keep supporting callbacks? I think it's beneficial to the quality of the framework to implement this in a breaking way, with everything async/promisified and no more callbacks anywhere.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/mineflayer/issues/725#issuecomment-736153065, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR437XIBZAX6BYK5NEYSP3SSQ7X3ANCNFSM4FY4GD5Q .

Yes, but then it can't be the same function so one would have to call a different function depending on whether they want to use the async or the callback version. The way you listed with callbackify will not expose the promise version

Yes it does. The example I did make the function both take a callback and return a promise

These are all the functions that will need to be converted. Non-exposed ones are between parentheses. I'll probably start working on converting them

```bed#wake bed#sleep blocks#waitForChunksToLoad (book#write) (book#modifyBook) book#writeBook book#signBook chat#tabComplete chest#deposit chest#withdraw craft#craft (craft#craftOnce) creative#setInventorySlot creative#flyTo digging#dig dispenser#deposit dispenser#withdraw enchantment_table#enchant enchantment_table#takeTargetItem enchantment_table#putTargetItem enchantment_table#putLapis (furnace#takeSomething) furnace#takeInput furnace#takeFuel furnace#takeOutput (furnace#putSomething) furnace#putInput furnace#putFuel inventory#consume inventory#fish inventory#putSelectedItemRange inventory#activateBlock inventory#activateEntity inventory#activateEntityAt inventory#transfer inventory#placeBlock inventory#clickWindow inventory#putAway inventory#moveSlotItem physics#look physics#lookAt simple_inventory#tossStack simple_inventory#toss simple_inventory#unequip (simple_inventory#equipEmpty) simple_inventory#equip villager#openVillager villager#trade (villager#putRequirements) (villager#deposit)

@rom1504 has rewarded $5.40 to @ph0t0shop. See it on IssueHunt

• :moneybag: Total deposit: $6.00
• :tada: Repository reward(0%): $0.00
• :wrench: Service fee(10%): $0.60

congrats and thanks @ph0t0shop !