Allow to create custom client & communication between clients #1254
Conversation
src/client.js
Outdated
| @@ -13,9 +13,11 @@ const createDecipher = require('./transforms/encryption').createDecipher | |||
| const closeTimeout = 30 * 1000 | |||
|
|
|||
| class Client extends EventEmitter { | |||
| constructor (isServer, version, customPackets, hideErrors = false) { | |||
| constructor (isServer, version, customPackets, hideErrors = false, customCommunication = undefined) { | |||
There was a problem hiding this comment.
Hm, pretty sure it would be better if it was an object like
| constructor (isServer, version, customPackets, hideErrors = false, customCommunication = undefined) { | |
| constructor (isServer, version, additionalOptions) { |
There was a problem hiding this comment.
I don't think this whole change makes a lot of sense.
What about instead letting the user provide its own Client class ?
This is overriding pretty much everything so I don't think it really fits in Client
src/client.js
Outdated
| this.serializer.write({ name, params }) | ||
|
|
||
| if (this.customCommunication) { | ||
| this.customCommunication.sendData({ name, params, state: this.state }) |
There was a problem hiding this comment.
It doesn't support writeBundle and writeRaw as I'm not sure how flying-squid could use it
|
please open a pr on flying squid showing how you use it |
Oke! I've included some other important changes to the same branch, so I have to rebase it first I guess. Will look into it tomorrow! |
src/createClient.js
Outdated
| @@ -32,7 +32,8 @@ function createClient (options) { | |||
| options.protocolVersion = version.version | |||
| const hideErrors = options.hideErrors || false | |||
|
|
|||
| const client = new Client(false, version.minecraftVersion, options.customPackets, hideErrors) | |||
| const Client = options.customClient ?? DefaultClient | |||
There was a problem hiding this comment.
Why isn't that option enough? Seems like it is to me
There was a problem hiding this comment.
Isn't it to allow users to provide custom implementation of the Client class? Like you said in #1254 (comment)
There was a problem hiding this comment.
I think I got it. I refactored all it. See latest commits
|
Also FYI |
|
Why can't you implement this functionality with a custom |
|
You are totally right! I'm sorry I missed the intention to have custom client implementation outside of nmp (node minecraft protocol) in #1254 (comment). Since then we can have just custom server and client options here. And yes, I don't skip auth currently, most probably I could just emit login event directly.. |
|
done. I simply moved this custom client to pwc, I'm still not sure about skipping auth even in the context of the local server, I feel it can break mineflayer... |
| @@ -34,7 +34,8 @@ automatically logged in and validated against mojang's auth. | |||
| * enforceSecureProfile (optional) : Kick clients that do not have chat signing keys from Mojang (1.19+) | |||
| * generatePreview (optional) : Function to generate chat previews. Takes the raw message string and should return the message preview as a string. (1.19-1.19.2) | |||
| * socketType (optional) : either `tcp` or `ipc`. Switches from a tcp connection to a ipc socket connection (or named pipes on windows). With the `ipc` option `host` becomes the path off the ipc connection on the local filesystem. Example: `\\.\pipe\minecraft-ipc` (Windows) `/tmp/minecraft-ipc.sock` (unix based systems). See the ipcConnection example for an example. | |||
|
|
|||
| * Server : You can pass a custom server class to use instead of the default one. | |||
There was a problem hiding this comment.
it would be really good to say here what methods this class must expose
| * realms : An object which should contain one of the following properties: `realmId` or `pickRealm`. When defined will attempt to join a Realm without needing to specify host/port. **The authenticated account must either own the Realm or have been invited to it** | ||
| * realmId : The id of the Realm to join. | ||
| * pickRealm(realms) : A function which will have an array of the user Realms (joined/owned) passed to it. The function should return a Realm. | ||
| * Client : You can pass a custom client class to use instead of the default one, which would allow you to create completely custom communication. Also note that you can use the `stream` option instead where you can supply custom duplex, but this will still use serialization/deserialization of packets. |
There was a problem hiding this comment.
it would be good to say here what methods this class must expose
There was a problem hiding this comment.
I guess there is the typing for that (with all signatures etc)
|
left a comment regarding the doc but otherwise lgtm |
|
ok |
For PrismarineJS/flying-squid#491
Also allow creating custom clients so don't need to patch it with webpack