PrismarineJS / mineflayer

Create Minecraft bots with a powerful, stable, and high level JavaScript API.
https://prismarinejs.github.io/mineflayer/
MIT License
4.95k stars 904 forks source link

TypeError: Cannot set property 'heldItem' of undefined in inventory plugin - occasionally occurs when connecting to Glowstone++ server #353

Closed deathcap closed 8 years ago

deathcap commented 8 years ago

Attempting to use latest mineflayer (c3f67f9 from git) with latest Glowstone++ (65111e) from git), usually I hit another issue and cannot login: https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/issues/146 (update: no longer hit this issue with build 511) - but after retrying several times, sometimes consistently mineflayer gets further but then crashes in the "inventory" plugin:

mineflayer $ NODE_DEBUG=minecraft-protocol node examples/echo.js  localhost 25565
MC-PROTO: 64949 Old-styling packet
MC-PROTO: 64949 writing packetId handshaking.set_protocol (0x0)
MC-PROTO: 64949 { protocolVersion: 47,
  serverHost: 'localhost',
  serverPort: 25565,
  nextState: 2 }
MC-PROTO: 64949 Old-styling packet
MC-PROTO: 64949 writing packetId login.login_start (0x0)
MC-PROTO: 64949 { username: 'echo' }
MC-PROTO: 64949 read packetId login.compress (0x3)
MC-PROTO: 64949 { id: 3, state: 'login', threshold: 256 }
MC-PROTO: 64949 read packetId login.success (0x2)
MC-PROTO: 64949 { id: 2,
  state: 'login',
  uuid: 'c39fa4c6-85a3-3582-8167-db3805dbe851',
  username: 'echo' }
MC-PROTO: 64949 read packetId play.window_items (0x30)
MC-PROTO: 64949 { id: 48,
  state: 'play',
  windowId: 0,
  count: 45,
  items: 
   [ { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -1 },
     { blockId: -
/Users/admin/games/voxeljs/mineflayer/lib/plugins/inventory.js:279
    bot.entity.heldItem = bot.heldItem;
                        ^

TypeError: Cannot set property 'heldItem' of undefined
    at updateHeldItem (/Users/admin/games/voxeljs/mineflayer/lib/plugins/inventory.js:279:25)
    at Client.<anonymous> (/Users/admin/games/voxeljs/mineflayer/lib/plugins/inventory.js:446:5)
    at emitOne (events.js:77:13)
    at Client.emit (events.js:169:7)
    at afterParse (/Users/admin/games/voxeljs/node-minecraft-protocol/dist/client.js:97:10)
    at parseNewStylePacket (/Users/admin/games/voxeljs/node-minecraft-protocol/dist/protocol.js:1080:5)
    at prepareParse (/Users/admin/games/voxeljs/node-minecraft-protocol/dist/client.js:113:9)
    at Socket.<anonymous> (/Users/admin/games/voxeljs/node-minecraft-protocol/dist/client.js:124:5)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)

unsure if this is a client (mineflayer) or server (Glowstone++) bug, but at first glance it seems to me to be a possible timing/synchronization or packet ordering issue?

(Originally saw this when using mineflayer through https://github.com/vogonistic/mineflayer-voxel/pull/1)

rom1504 commented 8 years ago

bot.entity is defined when the login packet (http://prismarinejs.github.io/minecraft-data/?d=protocol#toClient_login http://wiki.vg/Protocol#Join_Game ) is received (https://github.com/PrismarineJS/mineflayer/blob/master/lib/plugins/entities.js#L67)

it seems like glowstone send window_items before this packet.

To handle this we'd have to define bot.entity before I guess, but that would be a bit weird because we don't know the entity id before getting that login packet.

I guess I'll try running mineflayer on glowstone and try some things.

deathcap commented 8 years ago

Dug into this some more, I believe it is a Glowstone++ server bug.

It sends window_items, login, and then another window_items. The first window_items appears to be an error, because it is called in the constructor and is sent in join() anyway. Removing it allows mineflayer to login, and we still get updated inventories: https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/pull/147 - if my analysis is correct (awaiting feedback on the Glowstone++ PR), then this issue on mineflayer can be closed.

deathcap commented 8 years ago

Fixed in Glowstone++ https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/pull/147