bedrock-protocol
bedrock-protocol copied to clipboard
Allow CLI Options for Examples
A really basic CLI parsing setup that allows for out-of-the-box running of these examples.
Motiivation: I use these to test changes on bedrock-server and I've actually keep some secondary scripts with my own server details around to run these. With these flags though, no code changes are needed for simple demos.
Also this will reduce/eliminate the chance of personal info accidentally being entered into bedrock-protocol code.
Three concerns I see with this:
- A lot of the logic that separated these files is now gone. For instance, createRelay and realmRelay can do the same thing. I kept realmRelay for its demonstrative value though.
- Not crazy about the parse function living in createClient...
- The argument parsing is very limited as I didn't want to add a new dependency for such a minor feature. I would love to expand this further for some other examples I have (I have a pretty good relay that allows packet inspection that I would like to include), but that would likely stretch the arg-parsing logic to the point where it should have a dedicated JS file in
examples/(sayexamples/lib/args.js). This would also solve the above createClient concern.
The objective of the example code is to give provide something that users can copy paste into their projects and work on top of (and understand the library). It's not necessarily intended to be a standalone tool. I think we can certainly accept command line arguments, but I think no to anything that adds excessive boilerplate to the examples.
Okay. What if this functionality is moved to the tools directory?
I think if you want something like this it makes sense as an independent example or better as a separate package. I agree that some of the examples that do the same thing can be removed.
As for the tools/ folder, I believe @rom1504 position was that we should not have one. As of right now it's just tools for development (eg start a server) and helper scripts for tests to run. If we do keep it then I think it should stay for internal development purposes.
Something like this I think is ok :
https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/examples/client_chat/client_chat.js
It accepts arguments and console input but no excessive boilerplate.
But we also have a simpler example - https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/examples/client_auto/client_auto.js
Those examples look very similar to what I was going for... But if the general attitude is against things like this then its probably better to drop this.
However one thing I think this could be applied to is revisiting that tools/relay.js suggestion intended for inspection and packet debugging. I think this would be valuable for 1) general development, but 2) when someone is looking for support, the first thing we usually tell them is to pop up a relay and debug the packets, but that's just one more step they have to do and if they're already struggling... So something that can ease development and provide tooling for struggling users.
Would that be more in line with the views of this project?
(The boilerplate is the added parseAddress as a library API.) In those linked examples the only boilerplate is the extra events which we could probably have inside bedrock-protocol also. The remaining boilerplate inside the nmp examples is required for function (eg, usage of prismarine chat). We don't currently need prismarine chat inside bedrock which makes the examples much simpler.
But yes : as I noted prior I think it's ok as a separate example just as we have more complex examples in nmp, or this could be a separate package users can start with npx, just like https://github.com/extremeheat/minecraft-bedrock-server (we should remove the tools/ script) or https://github.com/extremeheat/prismarine-inspector.
let's not