Conversation
A server or proxy that sends cookie_request (1.20.5+) in the login, configuration or play state never got a reply, so the connection hung. Keep a per-connection cookie map on client._cookies, fill it from store_cookie packets and from the new `cookies` option (for carrying cookies over a transfer), and answer every cookie_request with the stored value or an absent one, as vanilla does.
…e vanilla The vanilla client answers login_acknowledged with a minecraft:brand custom_payload followed by Client Information, and sends neither again when a server moves it back to configuration from play: it only replies with configuration_acknowledged. nmp sent no brand, re-sent the settings on every configuration entry and defaulted the view distance to 10 where vanilla uses 12. Send the brand (`brand` option, default 'vanilla') and the settings on the first entry only, with the vanilla defaults.
The chat session was only created for the first login packet. After a server switch through re-configuration the next login packet left the old session in place, while the vanilla client discards its session on every login and announces a new one (fresh session UUID, index 0) with the same profile key pair. Do the same.
The vanilla client answers select_known_packs with the packs from the server's list that it has locally (minecraft:core at its own version on a vanilla server), and the server then omits the registry entries of those packs. nmp always replied with no packs. Add a `knownPacks` option listing the client's local packs and reply with their intersection with the server's list. It defaults to [] so the server keeps sending its full registry data, which consumers such as mineflayer depend on.
rom1504
left a comment
rom1504
left a comment
There was a problem hiding this comment.
Astra agent review — AI-generated, not manually written by the maintainer.
I checked the current handshake changes and public declarations, ran the 40 focused handshake tests successfully, and reproduced the TypeScript errors below against this head. The existing cookie-schema limitation is already documented and is not repeated here. No vanilla server or full suite was run.
Skills used: prismarine-code-quality-review helped check the documented public API against its declarations; prismarine-review helped verify the current head and avoid duplicate feedback.
| * validateChannelProtocol (optional) : whether or not to enable protocol validation for custom protocols using plugin channels. Defaults to true | ||
| * disableChatSigning (optional) : Don't try obtaining chat signing keys from Mojang (1.19+) | ||
| * clientSettings (optional) : Client Information (settings) sent to the server during the configuration phase (1.20.2+). All fields are optional and default to vanilla-safe values: | ||
| * cookies (optional) : cookies to answer `cookie_request` packets with, as an object or Map of key to Buffer (1.20.5+). Pass the previous connection's `client._cookies` when following a `transfer` packet, like the vanilla client does |
There was a problem hiding this comment.
Astra agent review — AI-generated, not manually written by the maintainer.
The newly documented options are missing from src/index.d.ts: compiling createClient({ username: 'bot', brand: 'custom' }) against this head gives TS2353, and the same happens for cookies and knownPacks. The documented transfer example also gives TS2339 for previousClient._cookies. Could this change add those three options to ClientOptions and declare the cookie map used by this documented transfer path? This lets TypeScript consumers use the new API without casts.
Skills used: prismarine-code-quality-review helped check the documented public API against its declarations; prismarine-review helped verify the current head and avoid duplicate feedback.
2 participants
Audit of nmp's client against the vanilla 1.21.4 client (
ClientHandshakePacketListenerImpl,ClientCommonPacketListenerImpl,ClientPacketListener,KnownPacksManager,Options) turned up four places where nmp diverges in ways that servers and proxies notice. One commit each:cookie_request(1.20.5+). nmp never replied, so any server or proxy that sends one (Velocity/Bungee modern forwarding, transfer flows) hangs the connection. A newsrc/client/cookies.jskeepsclient._cookies, fills it fromstore_cookie, and answerscookie_requestin login, configuration and play with the stored value or an absent one, exactly like vanilla. Acookiesoption seeds the map so a consumer following atransferpacket can carry cookies over like vanilla'sTransferState.login_acknowledgedwithcustom_payload minecraft:brandfollowed by Client Information, and does not repeat either when a server sends it back to configuration (onlyconfiguration_acknowledged). nmp sent no brand, re-sent the settings on every configuration entry, and used view distance 10 where vanilla's default is 12. Newbrandoption (default'vanilla'; mineflayer'soptions.brandflows through),clientSettingsdefaults are now the vanilla ones.loginpacket. After a proxy server switch the secondloginpacket left the old session in place; vanilla discards it and announces a new session (new UUID, index 0) with the same key pair.client.once('login')→client.on('login').select_known_packs. Vanilla replies with the packs from the server's list it has locally. NewknownPacksoption ([{ namespace, id, version }]) answered with the intersection; the default stays[]so servers keep sending full registry data, which mineflayer/prismarine-registry rely on.Docs updated in
docs/API.md. In-process tests added intest/serverTest.jsfor cookie replies in all three states (including a stored-cookie echo and an absent reply), brand-then-settings sent exactly once across astart_configurationround trip, and the known-packs intersection.Note: minecraft-data's 1.21.8
protocol.jsondeclarescookie_response.valueas a non-optionalByteArray(the 1.21.8 client writes it nullable like every other version). The absent reply is still wire-correct there (an emptyByteArrayand an absent option both serialize as a single0x00), but echoing a stored cookie needs a minecraft-data fix; the stored-cookie test is gated on the schema having the option wrapper.