ViaVersion compatibility

[kennytv]

Title: Server changes needed to support Via

ViaVersion has a similar effect to ProtocolSupport, but in the other direction - it allows players to connect with a higher client version than the actual server (there is also ViaBackwards and Rewind, which go backwards, but their compat goes hand in hand with ViaVersion).

I was a little bored and played around with Glowstone and Via and actually got it to fully work, but encountered one problem when doing that:
The CodecsHandler is an always changing object, which'd mean Via would have to inject its own handler every single time (without going some absurder routs).
The way I hackily adapted the server for testing purposes was to let it find its protocol itself (https://kennytv.eu/files/bdyfi.png) in a single handler, so that the instance is never replaced.

Probably not needed to be looked at, but here's the custom codecshandler, most of its code is just reused from he other platforms: https://github.com/KennyTV/ViaVersion/blob/glowstone/glowstone/src/main/java/us/myles/ViaVersion/glowstone/handlers/GlowstoneCodecHandler.java
(I also made the CodecsHandler non-final to avoid reflection usage on en-/decoding, but that's not really necessary)

Eitherway, there might be an easy way to circumvent this I just dont know about, but I just wanted to throw this in here.

EDIT: right I totally forgot another thing - since Via already shades its Bukkit and Sponge parts, there needs to be some sort of glowstone.yml that is prioritized over the other ones to actually load. While we could make a common class for Bukkit and Glowstone, that'd go against its current structure.

TL;DR the CodecsHandler instance either has to stay the same or we need a way to instantly reinject new instances/functions during en-/decode + a glowstone.yml that is prioritized over Bukkit's and Sponge's plugin files.

I'm honestly unsure what the best way of handling this would be (since I'm also one of the "newer" Via contributors that haven't been around for too long to know that kind of injecting trickery), but that's why I'm creating this issue 👀

If changes are applicable, it'd be nice they could also be ported in both the 1.13 and 1.12 branches

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[aramperes]

We've done changes to Glowstone in the past to help compatiblity with ProtocolSupport, so I don't see a reason we wouldn't do the same for ViaVersion (as long as the changes don't have a negative impact on Glowstone and are reasonable).

To clarify, the CodecsHandler is being instantiated for each session when it switches protocols. Your patch (the screenshot) makes the registration lookup fallback to all other protocols when the Message subclass is not found in the current proto.

I'm not sure I'm satisfied with having this done in Glowstone, and would rather expose an API to provide/inject CodecsHandler natively. I'm also not clear on how your patch actually fixes the issue of the CodecsHandler being instantiated multiple times.

[kennytv]

Oh forgot to mention; I made the protocol inside the CodecsHandler mutable with a setter and replaced the line you sent with

        CodecsHandler codecs = (CodecsHandler) getChannel().pipeline().get("codecs");
        codecs.setProtocol((GlowProtocol)proto);

.. but again, that was just my hacky way of simply getting it to work to then test it.

Tho some idea came to my mind: have two callbacks/functions with the same args as the de-/encode (either placed in a manager or as static fields in the coder itself), from which one is called at the end of the encode method, the other at the beginning of the decode method if they are present.
With that we wouldn't have to even inject it, and the problem of new instances being created would also be worked around with static callbacks.
Not sure if it's the best, but the only I can come up with right now, maybe you'll have a better idea 👀

[KisaragiEffective]

?

[mastercoms]

This ticket is still valid, even if KennyTV is no longer interested in it.