add a full packet parser

[rom1504]

one way to do #57

It might make more sense to do that since it avoids the queue if we don't need it.

I'll try the "fixed" datatype option though.

(and need an example to test this)

[roblabla]

I'm really unconvinced that's the right way to do it. What's the difference between this class and the existing Parser ? Wouldn't it be possible to merge them ?

[roblabla]

Right, this expects a chunk will never be cut, and always be a single packet. This isn't a good assumption to make. We should assume a stream is a stream, an endless series of bytes.

[rom1504]

We should assume a stream is a stream, an endless series of bytes.

this is only possible is the length is available in the buffer when it gets at the protodef serializer ( #57 (comment) )

[roblabla]

No ? I believe you're constraining your view to minecraft-protocol. ProtoDef serializer is (or should be, I'm not sure where we're at at this point) able to operate on any type of data. For instance, new Serializer(null, types.i32) will give us a stream of i32. Even if they were sent as 1 byte and then 3 bytes. Even if it was sent in bulk.

If minecraft-protocol needs a specific serializer interface for whatever reason, it can easily use "proto.parsePacketBuffer" function to implement it. It doesn't have to use the built-in Serializer.

ProtoDef API for most languages really should reduce to :

• addType
• (parse|create)PacketBuffer
• Whatever facility makes sense for that specific language to plug into the IO stack. In JS that's a TransformStream implementation.

[rom1504]

Yes I meant for minecraft protocol. For minecraft protocol we'd need to put the length in the buffer in order to use that same parser. (+ adding the "fixed" type, in protodef or nmp)
Yes I guess we could also define that FullPacketParser in nmp.

[rom1504]

If we got rid of the splitter in nmp (https://github.com/PrismarineJS/node-minecraft-protocol/blob/master/src/transforms/framing.js#L62 which does basically the same thing as protodef parser (that loop)) things would be much simpler.

[rom1504]

Yes ProtoDef is more general than nmp now. But "more general"(/generic) doesn't only mean "less specific to a given protocol" but also "can handle any protocol especially the hard one such as minecraft protocols"

[roblabla]

I suppose the only problem concerning NMP is that compression is impossible to impl in ProtoDef due to its sync nature. Instead of adding a bunch of hacks, maybe we should reconsider implementing an async interface to ProtoDef ? I'm thinking maybe having an optional async interface might not be too damaging performance-wise. Then compression could be handled in protodef as well

[
  { "name": "len", "type": "varint" },
  { "anon": true, "type": "compressed", typeArgs: {
    "length": "$len",
    "data": [
      // packet here
    ]
  }
]

[rom1504]

Yes having an (semi-?)async interface that would be ideal.
It wasn't only for the perf that I gave up on async, but also because nmp state changing currently work only because stuff are sync (PrismarineJS/node-minecraft-protocol#309 (comment)) and I believe that in order to make it work with an async interface, the state changing needs to be handled by the parser/serializer which is annoying.
Maybe it's possible though.

[roblabla]

Actually, we don't need stuff to be sync, just to be processed in order I think. Am I wrong ? If order is the problem, we can probably think of solutions ^^.

[rom1504]

I'm not sure tbh. If I really knew I would have fixed PrismarineJS/node-minecraft-protocol#309
It's probably possible to make it work, I'll try again sometime. (or a variation of it with only optional async)

[rom1504]

So. 2 ways to go here:

1. put this in node-protodef
2. put this in nmp

I think it's useful in node-protodef because nmp is not the only thing that can assume it's not really a stream.

[rom1504]

let's merge this. can be removed if needed