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

Support 1.21.2 - 1.21.4 #98

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

BoomEaro
Copy link
Contributor

@BoomEaro BoomEaro commented Oct 23, 2024

Mojang changed the LoginSuccess, JoinGame and PlayerPositionAndLook packets.

And also starting from 1.20.5 it is no longer necessary to send the client a full codec with registers.
Instead, a KnownPacks and UpdateTags packets is sent which synchronizes the registers, just like the vanilla server does.

In addition, all dependencies have been updated and some places in the code have been improved.
I also added github actions to automatically build a jar file when creating a release.

@Quozul Quozul mentioned this pull request Dec 6, 2024
@Quozul
Copy link

Quozul commented Dec 6, 2024

I created #104 to add support for 1.21.4 based on your work, thanks!

@BoomEaro BoomEaro changed the title Support 1.21.2 Support 1.21.2, 1.21.3, 1.21.4 Dec 9, 2024
@BoomEaro
Copy link
Contributor Author

BoomEaro commented Dec 9, 2024

I updated to 1.21.4, but something might have broken on older versions. Let me know if something breaks

@Potothingi
Copy link

1.21.1 - Network protocol error
1.20.6 - World background doesn't appear correctly as black.

@BoomEaro
Copy link
Contributor Author

BoomEaro commented Dec 9, 2024

1.21.1 - Network protocol error 1.20.6 - World background doesn't appear correctly as black.

Fixed 1.21.1. However, I can't reproduce the bug on 1.20.6. How does it look for you? Everything is correct for me.
Tested on vanilla

@BoomEaro BoomEaro changed the title Support 1.21.2, 1.21.3, 1.21.4 Support 1.21.2 - 1.21.4 Dec 9, 2024
@Potothingi
Copy link

1.21.1 - Network protocol error has been fixed.

1.20.6, 1.21.1 - Background still not black.

before:
limbo_before
this PR:
limbo

Copy link

@Potothingi Potothingi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be changed like this?

            if (version.moreOrEqual(Version.V1_21)) {
                msg.writeVarInt(dimensionRegistry.getDimension_1_21().getId());
            }
            else {
                msg.writeVarInt(dimensionRegistry.getDimension_1_20_5().getId());
            }

Applying this will fix the issue in 1.21.1, but will still cause issues in 1.20.5.

@@ -300,6 +300,28 @@ public void encode(ByteMessage msg, Version version) {
msg.writeVarInt(0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^
msg.writeVarInt(dimensionRegistry.getDimension_1_20_5().getId());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

            if (version.moreOrEqual(Version.V1_21)) {
                msg.writeVarInt(dimensionRegistry.getDimension_1_21().getId());
            }
            else {
                msg.writeVarInt(dimensionRegistry.getDimension_1_20_5().getId());
            }

Copy link
Contributor Author

@BoomEaro BoomEaro Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codec 1.20.5 and 1.21 use the same dimension ids, so this is not required

@BoomEaro
Copy link
Contributor Author

BoomEaro commented Dec 9, 2024

1.21.1 - Network protocol error has been fixed.

1.20.6, 1.21.1 - Background still not black.

before: limbo_before this PR: limbo

Ohh, looks like I messed up the dimension IDs. In your screenshot it's the_nether.
I will check

Copy link

@CyberFlameGO CyberFlameGO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

6 participants