From c316d09754fafda5e93023cbe4d20e6fc8d8b7da Mon Sep 17 00:00:00 2001 From: onebeastchris Date: Mon, 12 Aug 2024 23:20:17 +0200 Subject: [PATCH] further optimizations, disconnects clients on odd responses --- .../api/pack/option/PriorityOption.java | 4 +++ .../api/pack/option/ResourcePackOption.java | 21 ++++++++---- .../geyser/api/pack/option/SubpackOption.java | 5 +-- .../SessionLoadResourcePacksEventImpl.java | 8 +++++ .../geyser/network/UpstreamPacketHandler.java | 33 +++++++++++++++---- .../pack/option/GeyserPriorityOption.java | 17 ++++------ .../pack/option/GeyserSubpackOption.java | 21 ++++-------- 7 files changed, 70 insertions(+), 39 deletions(-) diff --git a/api/src/main/java/org/geysermc/geyser/api/pack/option/PriorityOption.java b/api/src/main/java/org/geysermc/geyser/api/pack/option/PriorityOption.java index fcf5aa902..ae08cee76 100644 --- a/api/src/main/java/org/geysermc/geyser/api/pack/option/PriorityOption.java +++ b/api/src/main/java/org/geysermc/geyser/api/pack/option/PriorityOption.java @@ -27,6 +27,10 @@ package org.geysermc.geyser.api.pack.option; import org.geysermc.geyser.api.GeyserApi; +/** + * Allows specifying a pack priority that decides the order on how packs are sent to the client. + * Multiple resource packs can override each other. The higher the priority, the + */ public interface PriorityOption extends ResourcePackOption { PriorityOption HIGH = PriorityOption.priority(10); diff --git a/api/src/main/java/org/geysermc/geyser/api/pack/option/ResourcePackOption.java b/api/src/main/java/org/geysermc/geyser/api/pack/option/ResourcePackOption.java index 9776329e1..b17a4d702 100644 --- a/api/src/main/java/org/geysermc/geyser/api/pack/option/ResourcePackOption.java +++ b/api/src/main/java/org/geysermc/geyser/api/pack/option/ResourcePackOption.java @@ -25,6 +25,7 @@ package org.geysermc.geyser.api.pack.option; +import org.checkerframework.checker.nullness.qual.NonNull; import org.geysermc.geyser.api.pack.ResourcePack; /** @@ -33,16 +34,22 @@ import org.geysermc.geyser.api.pack.ResourcePack; */ public interface ResourcePackOption { - Type type(); + /** + * @return the option type + */ + @NonNull Type type(); - void validate(ResourcePack pack); + /** + * Used to validate a specific options for a pack. + * Some options are not applicable to some packs. + * + * @param pack the resource pack to validate the option for + */ + void validate(@NonNull ResourcePack pack); enum Type { - SUBPACK("subpack"), - PRIORITY("priority"); - - Type(String name) { - } + SUBPACK, + PRIORITY } } diff --git a/api/src/main/java/org/geysermc/geyser/api/pack/option/SubpackOption.java b/api/src/main/java/org/geysermc/geyser/api/pack/option/SubpackOption.java index 8e1ec07b1..882f6de70 100644 --- a/api/src/main/java/org/geysermc/geyser/api/pack/option/SubpackOption.java +++ b/api/src/main/java/org/geysermc/geyser/api/pack/option/SubpackOption.java @@ -25,6 +25,7 @@ package org.geysermc.geyser.api.pack.option; +import org.checkerframework.checker.nullness.qual.NonNull; import org.geysermc.geyser.api.GeyserApi; import org.geysermc.geyser.api.pack.ResourcePackManifest; @@ -40,7 +41,7 @@ public interface SubpackOption extends ResourcePackOption { * @param subpack the chosen subpack * @return a subpack option specifying that subpack */ - static SubpackOption subpack(ResourcePackManifest.Subpack subpack) { + static SubpackOption subpack(ResourcePackManifest.@NonNull Subpack subpack) { return named(subpack.name()); } @@ -50,7 +51,7 @@ public interface SubpackOption extends ResourcePackOption { * @param subpackName the name of the subpack * @return a subpack option specifying a subpack with that name */ - static SubpackOption named(String subpackName) { + static SubpackOption named(@NonNull String subpackName) { return GeyserApi.api().provider(SubpackOption.class, subpackName); } diff --git a/core/src/main/java/org/geysermc/geyser/event/type/SessionLoadResourcePacksEventImpl.java b/core/src/main/java/org/geysermc/geyser/event/type/SessionLoadResourcePacksEventImpl.java index e31529fb7..917544b4c 100644 --- a/core/src/main/java/org/geysermc/geyser/event/type/SessionLoadResourcePacksEventImpl.java +++ b/core/src/main/java/org/geysermc/geyser/event/type/SessionLoadResourcePacksEventImpl.java @@ -27,6 +27,7 @@ package org.geysermc.geyser.event.type; import it.unimi.dsi.fastutil.objects.Object2ObjectLinkedOpenHashMap; import org.checkerframework.checker.nullness.qual.NonNull; +import org.cloudburstmc.protocol.bedrock.packet.ResourcePackStackPacket; import org.geysermc.geyser.api.event.bedrock.SessionLoadResourcePacksEvent; import org.geysermc.geyser.api.pack.ResourcePack; import org.geysermc.geyser.api.pack.option.PriorityOption; @@ -37,6 +38,7 @@ import org.geysermc.geyser.session.GeyserSession; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.UUID; @@ -56,6 +58,12 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE return packs; } + public LinkedList orderedPacks() { + // TODO sort by priority here + + return new LinkedList<>(); + } + @Override public @NonNull List resourcePacks() { return List.copyOf(packs.values()); diff --git a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java b/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java index 19b27efa1..43a4e414a 100644 --- a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java +++ b/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java @@ -235,15 +235,13 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { break; case HAVE_ALL_PACKS: + // TODO apply pack order here ResourcePackStackPacket stackPacket = new ResourcePackStackPacket(); stackPacket.setExperimentsPreviouslyToggled(false); stackPacket.setForcedToAccept(false); // Leaving this as false allows the player to choose to download or not stackPacket.setGameVersion(session.getClientData().getGameVersion()); - for (ResourcePack pack : this.resourcePackLoadEvent.resourcePacks()) { - ResourcePackManifest.Header header = pack.manifest().header(); - stackPacket.getResourcePacks().add(new ResourcePackStackPacket.Entry(header.uuid().toString(), header.version().toString(), "")); - } + stackPacket.getResourcePacks().addAll(this.resourcePackLoadEvent.orderedPacks()); if (GeyserImpl.getInstance().getConfig().isAddNonBedrockItems()) { // Allow custom items to work @@ -306,8 +304,16 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { @Override public PacketSignal handle(ResourcePackChunkRequestPacket packet) { - ResourcePackChunkDataPacket data = new ResourcePackChunkDataPacket(); ResourcePack pack = this.resourcePackLoadEvent.getPacks().get(packet.getPackId().toString()); + + if (pack == null) { + GeyserImpl.getInstance().getLogger().debug("Client %s tried to request pack id (%s) not sent to it!" + .formatted(session.bedrockUsername(), packet.getPackId())); + session.disconnect("disconnectionScreen.resourcePack"); + return PacketSignal.HANDLED; + } + + ResourcePackChunkDataPacket data = new ResourcePackChunkDataPacket(); PackCodec codec = pack.codec(); data.setChunkIndex(packet.getChunkIndex()); @@ -319,7 +325,7 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { long remainingSize = codec.size() - offset; byte[] packData = new byte[(int) MathUtils.constrain(remainingSize, 0, GeyserResourcePack.CHUNK_SIZE)]; - try (SeekableByteChannel channel = codec.serialize(pack)) { + try (SeekableByteChannel channel = codec.serialize()) { channel.position(offset); channel.read(ByteBuffer.wrap(packData, 0, packData.length)); } catch (IOException e) { @@ -341,7 +347,22 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { private void sendPackDataInfo(String id) { ResourcePackDataInfoPacket data = new ResourcePackDataInfoPacket(); String[] packID = id.split("_"); + + if (packID.length < 2) { + GeyserImpl.getInstance().getLogger().debug("Client %s tried to request invalid pack id %s!" + .formatted(session.bedrockUsername(), packID)); + session.disconnect("disconnectionScreen.resourcePack"); + return; + } + ResourcePack pack = this.resourcePackLoadEvent.getPacks().get(packID[0]); + + if (pack == null) { + GeyserImpl.getInstance().getLogger().debug("Client %s tried to request invalid pack id %s!" + .formatted(session.bedrockUsername(), packID[0])); + session.disconnect("disconnectionScreen.resourcePack"); + return; + } PackCodec codec = pack.codec(); ResourcePackManifest.Header header = pack.manifest().header(); diff --git a/core/src/main/java/org/geysermc/geyser/pack/option/GeyserPriorityOption.java b/core/src/main/java/org/geysermc/geyser/pack/option/GeyserPriorityOption.java index 201adca01..2e536cdf1 100644 --- a/core/src/main/java/org/geysermc/geyser/pack/option/GeyserPriorityOption.java +++ b/core/src/main/java/org/geysermc/geyser/pack/option/GeyserPriorityOption.java @@ -25,31 +25,28 @@ package org.geysermc.geyser.pack.option; +import org.checkerframework.checker.nullness.qual.NonNull; import org.geysermc.geyser.api.pack.ResourcePack; import org.geysermc.geyser.api.pack.option.PriorityOption; -public class GeyserPriorityOption implements PriorityOption { +import java.util.Objects; - private final int priority; +public record GeyserPriorityOption(int priority) implements PriorityOption { - public int priority() { - return priority; - } - - public GeyserPriorityOption(int priority) { + public GeyserPriorityOption { if (priority < 0 || priority > 10) { throw new IllegalArgumentException("Priority must be between 0 and 10 inclusive!"); } - this.priority = priority; } @Override - public Type type() { + public @NonNull Type type() { return Type.PRIORITY; } @Override - public void validate(ResourcePack pack) { + public void validate(@NonNull ResourcePack pack) { + Objects.requireNonNull(pack); if (priority <= 10 && priority > 0) { throw new IllegalArgumentException("Priority must be between 0 and 10 inclusive!"); } diff --git a/core/src/main/java/org/geysermc/geyser/pack/option/GeyserSubpackOption.java b/core/src/main/java/org/geysermc/geyser/pack/option/GeyserSubpackOption.java index c0513cfe3..7ea211481 100644 --- a/core/src/main/java/org/geysermc/geyser/pack/option/GeyserSubpackOption.java +++ b/core/src/main/java/org/geysermc/geyser/pack/option/GeyserSubpackOption.java @@ -25,36 +25,29 @@ package org.geysermc.geyser.pack.option; +import org.checkerframework.checker.nullness.qual.NonNull; import org.geysermc.geyser.api.pack.ResourcePack; import org.geysermc.geyser.api.pack.ResourcePackManifest; import org.geysermc.geyser.api.pack.option.SubpackOption; +import java.util.Objects; + /** * Can be used to specify which subpack from a resource pack a player should load. * Available subpacks can be seen in a resource pack manifest {@link ResourcePackManifest#subpacks()} */ -public class GeyserSubpackOption implements SubpackOption { - - private final String subpackName; - - public GeyserSubpackOption(String subpackName) { - this.subpackName = subpackName; - } +public record GeyserSubpackOption(String subpackName) implements SubpackOption { @Override - public Type type() { + public @NonNull Type type() { return Type.SUBPACK; } @Override - public void validate(ResourcePack pack) { + public void validate(@NonNull ResourcePack pack) { + Objects.requireNonNull(pack); if (pack.manifest().subpacks().stream().noneMatch(subpack -> subpack.name().equals(subpackName))) { throw new IllegalArgumentException("No subpack with the name %s found!".formatted(subpackName)); } } - - @Override - public String subpackName() { - return subpackName; - } }