-
-
Notifications
You must be signed in to change notification settings - Fork 239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add generated protocol types #1280
Conversation
const fs = require('fs') | ||
const path = require('path') | ||
const mcData = require('minecraft-data') | ||
// const dataPaths = require('../minecraft-data/data/dataPaths.json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented code should be removed
|
||
const packetKeys = [] | ||
// from client | ||
const packets = Object.entries(protocol.play.toServer.types).map(([name, [type, props]]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code should live in node-protodef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but noprotodefde-protodef doesn't have the minecraft-data dependency, what's your proposing workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the code generating typescript types for protodef types should be generic to any protodef type.
Node protodef would then expose a function for generating such typescript types and then node-minecraft-protocol would use it
@@ -0,0 +1,134 @@ | |||
const fs = require('fs') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic should be tested, there are a lot of edge cases so it's probably not fully working as is, tests will make it possible to fix it
node protodef has test values
thanks for the pr; but as said in comments this should live in node-protodef and be more generic node-minecraft-protocol relies on node-protodef for the definition and implementation of its serialization functions |
I'm really interested in this. I do hope to not forget to do this, The last time I looked at it I realized I need more time to implement it there... for now I will just use generated files: https://github.com/zardoy/prismarine-web-client/blob/d36bec02bfb7222ef8ea1b0503371bf16812a9e2/src/generatedServerPackets.ts its really handy |
Add intellisense for packets:
Unfortunately in TS no way to display optionality or jsdoc for overloaded methods, its only possible with js proxied objects.
Alternatively, data can be generated in minecraft-data and reused here as type.
If everything is ok, I'll add the same for events (receive from server data)