gnembon / fabric-carpet

Fabric Carpet
MIT License
1.71k stars 273 forks source link

Add back unnecessary int to CarpetPayload for backwards compatibility #1837

Open altrisi opened 10 months ago

altrisi commented 10 months ago

This adds back the unnecessary command int to CarpetPayload, to allow it to handle packets from servers running on older versions that are allowing newer clients with something like ViaVersion.

That int was removed in commit cd572695b6.

This also adds checks that command is always DATA, moves CarpetPayload to its own class and adds a constructor without command to it.

Completely untested at the moment.

Fallen-Breath commented 10 months ago

Reviewed the changes and looks good to me

Also made a quick test for the client <-> server commucation in dev environment, all of version log, rule sync and scarpet rendering work fine

cross-mc-version test is not done tho, but I think it's ready to go

ifacify commented 9 months ago

Thank you for your work. Unfortunately upon further investigation there should be no easy way of retaining backwards compatibility given the nature of nameless root nbt introduced in minecraft 1.20.2 update. This completely breaks root nbt compound cross version compatibility and there is no decent way around that. So we may as well take this opportunity to modernize carpet custom payload and keep it stable again in the newer versions.

ifacify commented 9 months ago

Also a carpet custom payload packet translator across 1.20.2+ and 1.20.2- can be made without too much effort.