PacketWrapper class improvement/cleanup.

[retrooper]

Is your feature request related to a problem? Please describe. As of now, we mix ServerVersion and ClientVersion inside of PacketWrapper. ServerVersion should probably be renamed to MinecraftVersion and ClientVersion to ProtocolVersion. Examples of minecraft versions could be "1.8.8" or "1.8.9" or "1.8.0", detecting these differences on Spigot could be essential for developers. Just protocol versions are a totally different thing. The protocol version should only be "1.8" as the protocol hadn't changed during those patch updates.

Describe the solution you'd like Now back to PacketWrappers. They should use protocol versions and no server/minecraft versions as they are part of the protocol to avoid confusion. This also simplifies a lot of work. All our mappings should switch to the ProtocolVersion too. There should also be quick conversions from one to another. Just acknoledge that converting a protocol version to a minecraft version can lead to data loss. "1.8" could convert to "1.8.0" as a minecraft version, losing some accuracy.

Describe alternatives you've considered to solve your solution without us adding this as a feature? ❌

Additional context Been thinking of doing this for a while, just gonna halt on this as we have many wrapper pull requests.

[MWHunter]

Your version comparison methods are o(n) despite being ordinals perfectly suited for o(1)

[retrooper]

I'll also rename PacketWrapper#copy(PacketWrapper) to copyFrom(PacketWrapper) to avoid confusion. We copy data from the input wrapper into our own wrapper. We don't modify the input wrapper!

[NoJokeFNA]

Idea

My idea would be to add 1 or 2 class(es), depending on the implementation. Our current way of supporting multiple versions is, in my opinion, just not pretty and in some cases not really readable. For this reason, I would be in favor of having one or two build(er) classes that we can use to simplify this.

Example

ReaderBuilder
  .builder()
  .version(MultiVersion comparison, ServerVersion version)
  .execute(Reader<R> reader)
  .build();
--------
int result = ReaderBuilder
               .builder()
               .version(MultiVersion.NEWER_THAN_OR_EQUALS, ServerVersion.V_1_19_1)
               .execute(PacketWrapper::readInt)
               .version(MultiVersion.OLDER_THAN_OR_EQUALS, ServerVersion.V_1_12_2)
               .execute(PacketWrapper::readByte)
               .build();

This is just an example of how it could look, but I am open to suggestions as well.

[retrooper]

Hmm, comparing that to what we do now, I prefer what we do now. But by improvement I was planning to just remove ServerVersion usage in wrappers and rename ClientVersion to ProtocolVersion so we can use that in all wrappers. ServerVersion should strictly only be for server version detecting.