further optimizations, disconnects clients on odd responses

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);

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"),
+        SUBPACK,
-        PRIORITY("priority");
+        PRIORITY
-
-        Type(String name) {
-        }
     }
 
 }

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);
     }
 

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<ResourcePackStackPacket.Entry> orderedPacks() {
+        // TODO sort by priority here
+
+        return new LinkedList<>();
+    }
+
     @Override
     public @NonNull List<ResourcePack> resourcePacks() {
         return List.copyOf(packs.values());

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()) {
+                stackPacket.getResourcePacks().addAll(this.resourcePackLoadEvent.orderedPacks());
-                    ResourcePackManifest.Header header = pack.manifest().header();
-                    stackPacket.getResourcePacks().add(new ResourcePackStackPacket.Entry(header.uuid().toString(), header.version().toString(), ""));
-                }
 
                 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();
 

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() {
+    public GeyserPriorityOption {
-        return priority;
-    }
-
-    public GeyserPriorityOption(int priority) {
         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!");
         }

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 {
+public record GeyserSubpackOption(String subpackName) implements SubpackOption {
-
-    private final String subpackName;
-
-    public GeyserSubpackOption(String subpackName) {
-        this.subpackName = subpackName;
-    }
 
     @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;
-    }
 }