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

Optional Pack Detection Code #4806

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

letsgoawaydev
Copy link
Contributor

This is the code to detect the Geyser Optional Pack. I was going to use this in the code to add java attack sounds, but I ended up not needing it. It was tested with the force resource pack option disabled and enabled, and works if the pack is declined or accepted, per player. However if the player has the pack loaded client side, this code wont detect it.

In the future this could be used for a better cooldown texture, and for playing sounds that are different for crops on bedrock.

Copy link
Member

@onebeastchris onebeastchris left a comment

Choose a reason for hiding this comment

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

Hey! Thanks for the PR - might be useful to allow GeyserMC/GeyserOptionalPack#55 to be merged :p
However, there are a few functional changes that i thought i should already mention.

@@ -308,10 326,10 @@ public PacketSignal handle(MovePlayerPacket packet) {

@Override
public PacketSignal handle(ResourcePackChunkRequestPacket packet) {
session.setRequestedPacks(true);
Copy link
Member

Choose a reason for hiding this comment

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

This would probably falsely (not) trigger if a pack is already loaded on a client - since it wouldnt request resource packs from the server then

@@ -346,7 364,9 @@ private void sendPackDataInfo(String id) {
ResourcePack pack = this.resourcePackLoadEvent.getPacks().get(packID[0]);
PackCodec codec = pack.codec();
ResourcePackManifest.Header header = pack.manifest().header();

if (header.uuid().toString().equals("e5f5c938-a701-11eb-b2a3-047d7bb283ba")) {
Copy link
Member

Choose a reason for hiding this comment

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

This check doesn't need to be here; should be fine to have it above

@letsgoawaydev
Copy link
Contributor Author

GeyserMC/GeyserOptionalPack#55 doesn't require this optional pack code. I also couldn't figure out how to solve the issue of the pack already being loaded by the client without it breaking other things like force-resource-pack being false

I'll see if i can figure something out again but I'm not really sure what to do about it

@onebeastchris
Copy link
Member

Unfortunately, the client does not let us know about which packs it has installed locally - however, for e.g. the optional pack, that would not be a huge issue as they're likely to be provided by the server (soon likely by default).

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.

None yet

2 participants