Please require just the minimal interfaces: io.Reader, io.Writer, io.ByteReader, ...

[robinbraemer]

Hey, I'm working on the Bedrock part of Gate proxy and would find myself to copy-paste-modify many functions from your library because many read/write helpers like https://github.com/Sandertv/gophertunnel/blob/10479899eb371fa2f7181377363e69192076c080/minecraft/protocol/varint.go#L52 require a full blown *bytes.Buffer.

Could you please downgrade all such cases to the minimal interface required?

For example as done in util/reader.go & util/writer.go: https://github.com/minekube/gate/tree/af6f2f5ecd4e3f1cdc03e8f3861049d553d1c2f7/pkg/proto/util

[TwistedAsylumMC]

You can use the protocol.Reader and protocol.Writer structs, https://github.com/Sandertv/gophertunnel/blob/10479899eb371fa2f7181377363e69192076c080/minecraft/protocol/reader.go#L398

[robinbraemer]

Thanks for the quick response but I can't use this Reader in my context since this Reader requires a byte slice. One shall consider following better practices and use strong interfaces instead.

[Sandertv]

The main reason these require byte buffers is because they have a ReadByte() and WriteByte() method (granted, it could use an interface for this) which is considerably faster than allocating a byte slice everytime and interface method calls in general. Are the varint functions the only ones you need?

(The same reason counts for the protocol.Reader and protocol.Writer types, which operate on a byte slice because it is easily some 20x faster than consecutive Read() and Write() operations)

[Sandertv]

I may however choose to move the protocol.Reader and protocol.Writer to use io.Reader and io.Writer too as the absolute performance loss isn't that big.

[Sandertv]

The latest commit has all Varint* functions, NewReader and NewWriter, and packet.Header.Read/Write accept io.Writer, io.Reader, io.ByteWriter, io.ByteReader or a combination of those. Let me know if that suffices.

[robinbraemer]

@Sandertv good enough, I might have to write and copy packet types still, thank you!

Another thing I'm very uncomfortable with is panic-ing where an error should have been returned. As in https://github.com/Sandertv/gophertunnel/blob/d3d47c64a1504e11c12148c4e6eec3c9929f8b78/minecraft/protocol/reader.go#L39 I understand the idea that one can simply call all the read and write methods and recover an error in one defer but this is a very bad practice. Callers need to know what's the underlying error and want to unwrap those to handle them appropriately. So while information gets lost, panicing and defering is also a performance hit.

[Sandertv]

There used to be no panicking but that causes bigger issues than this. Previously errors were returned for every read/write operation, but this caused unacceptable boilerplate code (quite literally hundreds to thousands of extra lines) and made the code hard to read and hard to update.

I have also considered the option of storing errors in the Reader/Writer and being able to recover these errors with a method, but this, again, causes unacceptable behaviour where code continues to execute in an undefined state.

In the latest Go versions a deferred panic also causes practically no additional overhead.

If you have any other ideas on how to resolve these points, please let me know.

[Sandertv]

Don't get me wrong by the way, I absolutely agree panicking isn't great, I just haven't personally figured out a cleaner way than what it is right now.