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

Various improvements 2 #77

Merged
merged 12 commits into from
Aug 22, 2024
Merged

Various improvements 2 #77

merged 12 commits into from
Aug 22, 2024

Conversation

Klotzi111
Copy link
Contributor

Hi,
I once again have some changes.

The name of this PR is not that descriptive. But too much happened in this PR to get a good title.

This PR continues adding missing packets that started with #71.
There are now only very few packets missing. For some I could not figure out the PacketType enum.

@Klotzi111
Copy link
Contributor Author

Do you know the PacketType values for? (names from https://wiki.vg/index.php?title=Protocol&oldid=19208):

  • Set Container Property
  • Client Status
  • Click Container Button

@Klotzi111
Copy link
Contributor Author

Klotzi111 commented Aug 19, 2024

For the packets ParticlePacket and ExplosionPacket we need the registry for particle data.
So maybe the next thing we should do is #73 ?

Edit: The class MinecraftData already is the registry. Or if not what is it then?
It would probably enough if this class also has the particle data.

@psu-de
Copy link
Owner

psu-de commented Aug 19, 2024

The PacketType's are the following:

  • Set Container Property: CB_Play_CraftProgressBar
  • Client Status: SB_Play_ClientCommand
  • Click Container Button: SB_Play_EnchantItem

Regarding MinecraftData:
Yes, it kind of is the registry, but doesn't allow to register custom data. This is required for forge, but also if the server uses custom datapacks (i think).
Minecraft-data, our current data source, is providing particle data, so it shouldn't be a problem to implement that. If you want, I can do that today or tomorrow.

I also updated #73 to make it a little bit more clear what really needs to be done.

Edit: I added ParticleData to MinecraftData. But it can only map the particle protocol id to its ParticleType or Identifier, and does not contain any information about the data stored in a particle.

@Klotzi111
Copy link
Contributor Author

I added ParticleData to MinecraftData. But it can only map the particle protocol id to its ParticleType or Identifier, and does not contain any information about the data stored in a particle.

Thanks. That should be fine. I will map the Identifier to the packet parsing logic

@Klotzi111
Copy link
Contributor Author

The only PacketType values we currently have no packet class for are:

  • CB_Configuration_ResourcePackSend
  • CB_Play_ChatPreview
  • CB_Play_FeatureFlags
  • CB_Play_MessageHeader
  • CB_Play_NamedSoundEffect
  • CB_Play_PingResponse
  • CB_Play_ResourcePackSend
  • CB_Play_SculkVibrationSignal
  • CB_Play_ShouldDisplayChatPreview
  • SB_Handshake_LegacyServerListPing
  • SB_Play_ChatPreview

Are these all for older Minecraft versions or am I missing a packet?

@psu-de
Copy link
Owner

psu-de commented Aug 20, 2024

yeah, I think all of these packets only exist for some versions between 1.19 and 1.20.2, but I'll check.

@psu-de
Copy link
Owner

psu-de commented Aug 20, 2024

  • CB_Play_PingResponse was introduced in 1.20.2 (and also exists in 1.20.4)
  • CB_Configuration_ResourcePackSend only exists in 1.20.2, and has the same fields as CB_Play_ResourcePackSend
  • CB_Play_ChatPreview and SB_Play_ChatPreview only exist in 1.19 and 1.19.2
  • CB_Play_FeatureFlags exist in 1.19 to 1.20.2 and was then moved to configuration phase
  • CB_Play_MessageHeader only exists in 1.19.2 and is probably used for chained message verification
  • CB_Play_NamedSoundEffect was removed in 1.19.3
  • CB_Play_ResourcePackSend was removed in 1.20.3
  • CB_Play_SculkVibrationSignal was removed in 1.19
  • CB_Play_ShouldDisplayChatPreview only existed in 1.19 and 1.19.2
  • SB_Handshake_LegacyServerListPing was used before 1.7 to ping the server. It's not part of the protocol anymore, but modern servers should be able to handle it correctly. It also doesn't follow the normal packet format (not length prefixed).

@Klotzi111
Copy link
Contributor Author

With this PR we now have all the packets that exist in 1.20.4.
I think we should merge this PR now so that we can continue with PR #78.
We can implement all the version specific packets with the pattern that PR adds.

@psu-de psu-de merged commit bd6deda into psu-de:main Aug 22, 2024
1 check passed
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.

2 participants