-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add specific error when encoding packet with no encoder #3734
base: 1.20.5
Are you sure you want to change the base?
Conversation
This does break existing mods such as polymer library (it injects into getCodec method) and carpet (adds to vanilla list). It also requires more specific mixins to work around it. |
You might be able to add a unit test for this, see |
The current behaviour (which may be possible to change) is that attempting to encode a packet without a codec is still an error, so the client will be disconnected. The thrown exception is caught somewhere higher up and the client is disconnected with the "Failed to encode packet" error message, with our RuntimeException being a child. Below is what appears in the logs (at least on my machine) when running
I can't say for sure why the breakpoint isn't working, but I'd think it has something to do with the mixin+anonymous class?
I'll give it a shot! |
Adds a specific error when encoding a packet that doesn't have a registered encoder
To explain a bit more about the issue:
CustomPayload#getCodec returns a dummy decoder using unknownCodecFactory. This is fine when decoding, but not when encoding. When encoding, you'll end up with a confusing exception about casting to DiscardedPayload (or UnknownCustomPayload).
An ideal solution would not use #getCodec for encode, and would instead throw the more helpful exception directly when a codec is missing. Unfortunately I couldn't figure out a good way to do this since map is part of the anonymous class (perhaps someone with more knowledge of mixins could point me in the right direction).
Overwriting the entire #getCodec method is also an option, but I think is generally unfavoured.
For this PR, I just throw the error if the namespace isn't "minecraft", which might be problematic if mods add directly to the map instead of using Fabric's system.