Skip to content
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

Map ByteArray codecs in PacketCodecs #3960

Closed
wants to merge 1 commit into from

Conversation

Octol1ttle
Copy link
Contributor

Fixes #3917

This issue was very weird to me. Other codecs had encode and decode as their method names, but were marked obfuscated and there were no mappings for these methods except for the PacketCodec returned by the byteArray method. I'm not sure why BYTE_ARRAY field and byteArray method require manual mapping.

Also I mapped a constant that defines the maximum size a collection can be initialized with when reading collections. I think it needs to be unpicked to work properly, but I don't know how.

@modmuss50
Copy link
Member

I think it needs to be unpicked to work properly, but I don't know how.

Im not sure if unpick can unpick only within a certain class, as we wouldnt want to unpick this for all Math.min usages across all classes.

@apple502j
Copy link
Contributor

I feel like this has to be a bug in the toolchain, not a mapping issue.

@Juuxel
Copy link
Member

Juuxel commented Aug 23, 2024

Yeah, this feels like an issue with specialised methods, which has happened before (see also #3916 which makes largely the same changes and wasn't merged).

@Octol1ttle
Copy link
Contributor Author

I didn't notice that PR, sorry

@Octol1ttle Octol1ttle closed this Aug 23, 2024
@modmuss50
Copy link
Member

Is it broken in dev? Does this work around it? If so maybe it is fine to merge idk.

@Octol1ttle
Copy link
Contributor Author

See #3917 for how this issue manifests during development

@modmuss50
Copy link
Member

This fix / workaround seems fine then. Unless you want to go digging for the root cause of the issue I dont have a problem with this for now?

@Octol1ttle
Copy link
Contributor Author

I'm not familiar enough with the toolchain to fix this myself. I submitted this PR because I saw that there were already mappings for encode and decode in the byteArray method. If this way works for you, you should be able to re-open and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

method_59799 and method_59800 in PacketCodecs.BYTE_ARRAY don't implement PacketCodec.encode/decode()
4 participants