Legacy support for old packet structures

[JarvisCraft]

Feature proposed in #59 whose main point is to allow PacketWrapper to be used on lower versions in which structures of packet objects vary from these now. This is reached by adding conditional statements which check minor version of minecraft server (this is stored as a static constant of AbstractPacket as an int primitive so the overhead of such approach seems to be close to zero). Another enhancement being part of this PR is storing ProtocolManager instance from ProtocolLibrary.getProtocolManager() as a static constant of AbstractPacket which both guarantees no mismatches and performs a sort of microoptimization.

[JarvisCraft]

@dmulloy2, waiting for your point on whether or not this PR should be continued.

[JarvisCraft]

Another thing to discuss is what to do with those methods which don't have equivalents on older versions such as setUniqueId(UUID). Should these throw fail-fast exceptions on those versions or return some stubs instead?

[dmulloy2]

I like it. As for the methods without equivalents it should throw an UnsupportedOperationException

[JarvisCraft]

@dmulloy2, it seems that WrapperPlayServerRelEntityMove and WrapperPlayServerRelEntityMoveLook have different types for setDx/y/z and getDx/y/z as the first one uses raw int value which should be manually converted by the user and the second one uses value converted to double using / 4096D so I will change the method signature in EntityMove so it follows the standard.

[JarvisCraft]

Also will extend packets as in NMS so that Entity is parent of RelEntityMove RelEntityMoveLook and EntityLook.

[JarvisCraft]

@dmulloy2, could you please review 7dd117acb10e135e966c2c591312058faca0d585

[dmulloy2]

@JarvisCraft sorry for the late reply on this, it's been a busy year.

What's the status of this PR? I looked over the diff and it looks good, but it is still a draft PR.

[JarvisCraft]

@dmulloy2, hi there I will try to finish it soon (had a big delay due to university and other stuff) <3

[JarvisCraft]

Welp, I'm weird thus it took me another year to sit and start doing it intensively.

Now all WrapperPlayClient* part is backwards compatible.

Significant details:

• I've added @BackwardsCompatible annotation for easier tracking of backwards compatibility
• WrapperPlayClientUseItem contains a compound structure MovingObjectPosition which should probably be implemented in ProtocolLib itself
• Tests won't run now (RIP) as AbstractPacket depends on ProtocolLibrary.getProtocolManager() which is null when running tests

The next stage is doing the same job for WrapperPlayServer*

[JarvisCraft]

Also, there was code-style mess as some classes were using tabs and some were using spaces thus I've used tabs everywhere (as it was more common among default classes) thus if spaces are the preferred indents (I personally prefer them) it won't be a problem to use them instead.

[JarvisCraft]

As another point for discussion, there are some legacy details which may be also influenced if you support it:

• I suggest removing legacy methods with typos which were replaced with correct ones. As PacketWrapper is mostly used as a shaded dependency it should not cause much problems on existing servers.
• There are some incorrect method names such as getEntityID instead of getEntityId which stand against Java's code conventions and mess with methods like getKeepAliveId thus I also suggest removing them
• Some boolean accessors use is prefix like isInvulnerable() while others use get and some simply omit these, I think that it's worth using get everywhere as, while I prefer is for booleans, here this are not just getters bu reflective accessors

[JarvisCraft]

Another big job to be done is Advancements as they seem to be broken as I remember. I will probably do the same job as done with MovingObjectPosition: implement wrappers in PacketWrapper leaving an opportunity for you to move it into ProtocolLib.

[JarvisCraft]

I have temporarily configured the branch of my fork to deploy artifacts to GH Package Registry so that it is possible to depend on this patch while it is not implemented. Once it becomes non-draft I will tag you and reset 330d078

[JarvisCraft]

@dmulloy2 could you please allow me to temporarily deploy current legacy-support builds under my coordinates (i.e. ru.progrm-jarvis.minecraft:PacketWrapper) at Sonatype OSSRH so that in can be used as a dependency now until the patch is fully ready to be meged into upstream.

At current it gets deployed to GitHub Package Registry for the same purpose but it requires user to provide access credentials (even though the repo is public) which seems to be an issue: for example I have minecraft-utils depending on this, then custom-stuff depends on minecraft-utils and than multiple projects depend on custom-stuff and yet each of them has to provide GitHub credentials to be able to compile just because the top-level dependency (minecraft-utils uses GitHub Package Registry's PacketWrapper).

[dmulloy2]

@JarvisCraft yeah you're absolutely free to deploy your own version under your own coordinates

[JarvisCraft]

@JarvisCraft yeah you're absolutely free to deploy your own version under your own coordinates

Thanks! Now it is available via Sonatype OSSRH. Once the job on this PR is ready (once it will happen, haha) I will also open a PR to enable deployment under your coordinates to Sonatye OSSRH and possibly Maven Central.

[dmulloy2]

Also you should be good to go back to using my maven repo

[dmulloy2]

@JarvisCraft any update on this?