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 for the 1.11 protocol #3480

Merged
merged 2 commits into from
Dec 15, 2016
Merged

Support for the 1.11 protocol #3480

merged 2 commits into from
Dec 15, 2016

Conversation

madmaxoft
Copy link
Member

Basic support for the 1.11 protocol. Doesn't support mobs yet (any mob will D/C clients immediately)

Also changed the protocol classes' names for better legibility, and slightly refactored to allow simple overrides for handling incoming packets.

Ref.: #3479

@madmaxoft
Copy link
Member Author

@NiLSPACE , @sphinxc0re , @Pokechu22 what do you think of the new class names, are they alright?

@Pokechu22
Copy link
Contributor

I like the idea of naming them that way (especially for the file names), but I feel like it might be clearer to name the first versions Protocol_1_9_0 instead of Protocol_1_9. It's not to significant but I think it makes it more obvious that it's intended for the first version. (Of course, mojang doesn't name them that way, so it can go either way)

@sphinxc0re
Copy link
Contributor

I totally agree with @Pokechu22 here. The classnames are much better to read and better to distinguish

@NiLSPACE
Copy link
Member

Yeah, the names are much better.

@madmaxoft madmaxoft force-pushed the proto_1_11 branch 2 times, most recently from 2e72445 to 66a805c Compare December 15, 2016 21:33
@madmaxoft
Copy link
Member Author

I'm not too enthusiastic about 1_9_0 having the _0 suffix, there are iterations that have only a single protocol version, so it does make sense to have the first version without any suffix and additional versions with a suffix.

@NiLSPACE
Copy link
Member

Having no suffix could also indicate that the class is used for all 1.9 protocols, so I'm inclined to keep it with the suffix.

@madmaxoft
Copy link
Member Author

The problem with that is that you don't know whether there will be a new version, until they make the next "major" update, at which point you'll be renaming the class just for the sake of the suffix.

@madmaxoft
Copy link
Member Author

Okay, two against one, I'll rename the classes to include the suffix, and then I'm done with this PR :)

@NiLSPACE
Copy link
Member

Great :)

@NiLSPACE
Copy link
Member

Now the websites needs to be updated.

@sphinxc0re
Copy link
Contributor

@mathiascode to the rescue. Also update the banner

@madmaxoft
Copy link
Member Author

I wouldn't advertise 1.11 support just yet. It's very preliminary, I have a feeling that even when a second player joins the game, both players' clients will crash. I'd wait until #3479 is closed.

@mathiascode
Copy link
Member

Once entities work fine again, I will update the website.

@NiLSPACE
Copy link
Member

It seems the OnPlayerRightClick hook is called twice in 1.11. Is that a bug in our code or did the protocol actually change to do that?

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

Successfully merging this pull request may close these issues.

None yet

5 participants