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

Add MCP packet names for Bukkit-Forge hybrid server #862

Merged
merged 6 commits into from
Jun 16, 2020

Conversation

NewbieOrange
Copy link

@NewbieOrange NewbieOrange commented Jun 12, 2020

Fix ProtocolLib returning dynamic PacketType on forge servers, which breaks plugin using PacketType enums.

This PR will fix #858.

@NewbieOrange
Copy link
Author

It seems that we cannot get reliable name on forge servers (which is expected). Adding PacketType lookup using packet id as a fallback will fix the issue.

I think adding such a fallback will not hurt reliablity or performance. Any thoughts?

@dmulloy2
Copy link
Owner

it wouldn’t have that much of an impact on performance. my main consideration is that packet ids change just about every version, so the 1.15 packet ids aren’t the same as the forge/1.12 ids

@dmulloy2
Copy link
Owner

it might be worthwhile to generate a mapping of forge packet names

@NewbieOrange
Copy link
Author

I am looking into it and trying to figure out what could be done.

@NewbieOrange
Copy link
Author

Hi @dmulloy2,

I have added some forge packet names and helper functions. I have tested this with FastLogin and it works.

Any thoughts? If you think this is doable, I will finish adding the rest of packet names.

@NewbieOrange NewbieOrange changed the title Add a workaround for Bukkit-Forge hybrid server Add MCP packet names for Bukkit-Forge hybrid server Jun 13, 2020
@NewbieOrange
Copy link
Author

I have added all MCP & Forge packet names up to 1.12.

@NewbieOrange
Copy link
Author

Note that these names only works for minecraft 1.12. MC 1.13 and 1.14 both have different packet names. Considering that CatServer / Magma is still at 1.12 so far, this should be sufficient.

@NewbieOrange
Copy link
Author

I have tested this and it works flawlessly with CatServer. I suppose it should work for Magma too, but not tested.

@dmulloy2 dmulloy2 merged commit 5183bd5 into dmulloy2:master Jun 16, 2020
@NewbieOrange NewbieOrange deleted the fix-forge-compat branch June 17, 2020 08:54
@Luohuayu
Copy link

Luohuayu commented Jul 2, 2020

I have tested this and it works flawlessly with CatServer. I suppose it should work for Magma too, but not tested.

Luohuayu/CatServer#208 (comment)

@Luohuayu
Copy link

Luohuayu commented Jul 2, 2020

@NewbieOrange It seems still some problems

@NewbieOrange
Copy link
Author

NewbieOrange commented Jul 2, 2020

The changes is not in the release version. Try the lastest snapshot (461+). @Luohuayu

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.

ProtocolLib reports incorrect PacketType under CatServer (Bukkit-Forge hybrid server)
3 participants