GeyserMC / Geyser

A bridge/proxy allowing you to connect to Minecraft: Java Edition servers with Minecraft: Bedrock Edition.
https://geysermc.org
MIT License
4.75k stars 687 forks source link

Enable PurchaseReceiptPacket and RefreshEntitlementsPacket #5118

Closed romanalexander closed 2 weeks ago

romanalexander commented 2 weeks ago

These packets are still sent by the client and exclusively used by featured servers. This might be out of scope for Geyser to support though.

Camotoy commented 2 weeks ago

Since most people won't need these, can we put them behind a system property?

Kas-tle commented 2 weeks ago

I am not really ok with enabling purchase receipt for safety reasons, as that packet contains an arbitrary string list. (see https://github.com/CloudburstMC/Protocol/blob/3.0/bedrock-codec/src/main/java/org/cloudburstmc/protocol/bedrock/packet/PurchaseReceiptPacket.java#L15). For some background, we disable the serializers for certain packets because Cloudburst/Protocol does not do serialization on-demand. Instead, it will serialize any packet sent to it for which there is a registered serializer, regardless of whether or not we access data from said packet. Given that, my personal rule has been that if Geyser does not directly handle the packet, we don't enable it.

If this is something you may want for more packets in the future, I'd prefer to make it so they can just override the entire codec (obviously this wouldn't be an official API since you can basically break anything at that point). But that would be somewhat of a pain since GameProtocol is currently static.

romanalexander commented 2 weeks ago

I am not really ok with enabling purchase receipt for safety reasons, as that packet contains an arbitrary string list. (see https://github.com/CloudburstMC/Protocol/blob/3.0/bedrock-codec/src/main/java/org/cloudburstmc/protocol/bedrock/packet/PurchaseReceiptPacket.java#L15). For some background, we disable the serializers for certain packets because Cloudburst/Protocol does not do serialization on-demand. Instead, it will serialize any packet sent to it for which there is a registered serializer, regardless of whether or not we access data from said packet. Given that, my personal rule has been that if Geyser does not directly handle the packet, we don't enable it.

If this is something you may want for more packets in the future, I'd prefer to make it so they can just override the entire codec (obviously this wouldn't be an official API since you can basically break anything at that point). But that would be somewhat of a pain since GameProtocol is currently static.

I agree. I think I'll rework this into first party support and hide it behind a property.