PrismarineJS / node-minecraft-protocol

Parse and serialize minecraft packets, plus authentication and encryption.
https://prismarinejs.github.io/node-minecraft-protocol/
BSD 3-Clause "New" or "Revised" License
1.23k stars 239 forks source link

Exception in client chat example #343

Closed nsporillo closed 8 years ago

nsporillo commented 8 years ago

Exception: https://gist.github.com/nsporillo/429bddc88896cf147d24

Server version: 1.7.10

Not sure if the client chat example has support for 1.7.10, but I'm getting this exception when I try to connect to a server.

deathcap commented 8 years ago

@nsporillo How are you able to get the client to connect to a 1.7.10 server, which version of node-minecraft-protocol are you using?

With the latest node-minecraft-protocol on git (which supports 1.8.9 and a 1.9 snapshot), connecting to a 1.7.10 vanilla I see this error instead:

node-minecraft-protocol $ node examples/client_chat/client_chat.js localhost 17100 testuser
connecting to localhost:17100
user: testuser
Successfully connected to localhost:17100
disconnected: "Outdated server! I\u0027m still on 1.7.10"
Connection lost
nsporillo commented 8 years ago

Oh I didn't realize latest version doesn't support older mc.

I believe the server I connected to accepts 1.8 clients due to protocol hacks.

I wrote a CLI chat client back in the day (https://gist.github.com/nsporillo/325cf29b3fe9e4b80fd8) and wanted to use it again but don't have my fork of n-m-p from then.

Any ideas?

deathcap commented 8 years ago

Ah, presumably a 1.7.10/1.8 protocol hack from Spigot? There have been other reports of compatibility problems with this server software and PrismarineJS: https://github.com/PrismarineJS/mineflayer/issues/369 (mineflayer uses node-minecraft-protocol).

nsporillo commented 8 years ago

Yes. I was just reading that, however it should be irrelevant. I just need a copy of node-minecraft-protocol from the 1.7 era and my mine chat bot will work with some modifications. I'll close this issue, but you guys should 100% work on adding some sort of compatibility especially if it's only a few extra things.

deathcap commented 8 years ago

@nsporillo can you try this patch:

--- a/examples/client_chat/client_chat.js
+++ b/examples/client_chat/client_chat.js
@@ -171,8 +171,9 @@ function parseChat(chatObj, parentState) {

       chat += color(util.format.apply(this, args), getColorize(parentState));
     }
-    chatObj.extra.forEach(function(i) {
-      chat += parseChat(chatObj.extra[i], parentState);
+    if (chatObj.extra)
+    chatObj.extra.forEach(function(item) {
+      chat += parseChat(item, parentState);
     });
     return chat;
   }

Looks to me the chat example is completely broken (not just with 1.7/1.8 cross-protocol servers), because extra.forEach iterates on the array elements not the indices.

I'm all for supporting all protocol versions, including the 1.7/8 protocol hack. Losing 1.7 compatibility was an unfortunate regression in the 1.8 update, but now nmp has multi-protocol support so adding back 1.7.10 should be conceivably possible. Until then the older versions support 1.7: https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/HISTORY.md#0123

nsporillo commented 8 years ago

Appreciate the effort. I think 1.7.10 support would be nice. I'm using this to spawn simple bots for server-side plugin testing purposes. Hard to drag several people in to test trivial things.

Anyways I'll try the patch and let you know if it works on latest nmp on 1.7.10 w/ the patch

nsporillo commented 8 years ago

@deathcap that worked perfectly. Thank you!