Check for duplicate options, other fixes

api/src/main/java/org/geysermc/geyser/api/event/bedrock/SessionLoadResourcePacksEvent.java

@@ -64,6 +64,7 @@ public abstract class SessionLoadResourcePacksEvent extends ConnectionEvent {
      * specific options.
      *
      * @param resourcePack a resource pack that will be sent to the client.
+     * @param resourcePackOptions {@link ResourcePackOption}'s that specify how clients load the pack
      * @return true if the resource pack was added successfully,
      *         or false if already present
      */
@@ -81,6 +82,7 @@ public abstract class SessionLoadResourcePacksEvent extends ConnectionEvent {
 
     /**
      * Returns the subpack options set for a specific resource pack uuid.
+     * These are not modifiable.
      *
      * @param resourcePack the resourcePack for which the options are set
      * @return a list of {@link ResourcePackOption}
@@ -94,5 +96,4 @@ public abstract class SessionLoadResourcePacksEvent extends ConnectionEvent {
      * @return true whether the resource pack was removed from the list of resource packs.
      */
     public abstract boolean unregister(@NonNull UUID uuid);
-
 }

api/src/main/java/org/geysermc/geyser/api/pack/ResourcePack.java

@@ -36,8 +36,6 @@ import java.util.Collection;
  * <p>
  * This representation of a resource pack only contains what
  * Geyser requires to send it to the client.
- * <p>
- * Optionally, a content key and/or a subpack name to load can be provided.
  */
 public interface ResourcePack {
 

core/src/main/java/org/geysermc/geyser/event/type/SessionLoadResourcePacksEventImpl.java

@@ -43,54 +43,61 @@ import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.UUID;
 import java.util.stream.Collectors;
 
 public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksEvent {
 
     @Getter
-    private final Map<String, ResourcePack> packs;
-    private final Map<String, Collection<ResourcePackOption>> options = new HashMap<>();
+    private final Map<UUID, ResourcePack> packs;
+    private final Map<UUID, Collection<ResourcePackOption>> options = new HashMap<>();
 
     public SessionLoadResourcePacksEventImpl(GeyserSession session) {
         super(session);
         this.packs = new Object2ObjectLinkedOpenHashMap<>(Registries.RESOURCE_PACKS.get());
         this.packs.values().forEach(
-            pack -> options.put(pack.manifest().header().uuid().toString(), pack.defaultOptions())
+            pack -> options.put(pack.manifest().header().uuid(), pack.defaultOptions())
         );
     }
 
     public LinkedList<ResourcePackStackPacket.Entry> orderedPacks() {
-        return packs.values().stream()
+        TreeSet<Map.Entry<ResourcePack, Integer>> sortedPacks = packs.values().stream()
             // Map each ResourcePack to a pair of (ResourcePack, Priority)
             .map(pack -> new AbstractMap.SimpleEntry<>(pack, getPriority(pack)))
-            // Sort by priority in descending order (higher priority first)
-            .sorted((entry1, entry2) -> Integer.compare(entry2.getValue(), entry1.getValue()))
-            // Extract the ResourcePack from the sorted entries
+            // Sort by priority in descending order
+            .collect(Collectors.toCollection(() -> new TreeSet<>(Map.Entry.comparingByValue(Comparator.naturalOrder()))));
+
+        // Convert the sorted entries to a LinkedList of ResourcePackStackPacket.Entry
+        return sortedPacks.stream()
             .map(entry -> {
-                ResourcePack pack = entry.getKey();
-                ResourcePackManifest.Header header = pack.manifest().header();
-                return new ResourcePackStackPacket.Entry(header.uuid().toString(), header.version().toString(), getSubpackName(header.uuid()));
+                ResourcePackManifest.Header header = entry.getKey().manifest().header();
+                return new ResourcePackStackPacket.Entry(
+                    header.uuid().toString(),
+                    header.version().toString(),
+                    getSubpackName(header.uuid())
+                );
             })
-            // Collect to a LinkedList
             .collect(Collectors.toCollection(LinkedList::new));
     }
 
     // Helper method to get the priority of a ResourcePack
     private int getPriority(ResourcePack pack) {
-        return options.get(pack.manifest().header().uuid().toString()).stream()
-            // Filter to find the PriorityOption
+        return options.get(pack.manifest().header().uuid()).stream()
             .filter(option -> option instanceof PriorityOption)
-            // Map to the priority value
             .mapToInt(option -> ((PriorityOption) option).priority())
-            // Get the highest priority (or a default value, if none found)
-            .max().orElse(PriorityOption.NORMAL.priority());
+            .max()
+            .orElse(PriorityOption.NORMAL.priority());
     }
 
+
     public List<ResourcePacksInfoPacket.Entry> infoPacketEntries() {
         List<ResourcePacksInfoPacket.Entry> entries = new ArrayList<>();
 
@@ -106,14 +113,13 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
     }
 
     private String getSubpackName(UUID uuid) {
-        return options.get(uuid.toString()).stream()
+        return options.get(uuid).stream()
             .filter(option -> option instanceof SubpackOption)
             .map(option -> ((SubpackOption) option).subpackName())
             .findFirst()
             .orElse("");  // Return an empty string if none is found
     }
 
-
     @Override
     public @NonNull List<ResourcePack> resourcePacks() {
         return List.copyOf(packs.values());
@@ -126,16 +132,22 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
 
     @Override
     public boolean register(@NonNull ResourcePack resourcePack, ResourcePackOption... resourcePackOptions) {
+
+        // Validate options, check for duplicates
+        Set<ResourcePackOption.Type> types = new HashSet<>();
         for (ResourcePackOption option : resourcePackOptions) {
             option.validate(resourcePack);
+            if (!types.add(option.type())) {
+                throw new IllegalArgumentException("Duplicate resource pack option " + option + "!");
+            }
         }
+        types.clear();
 
-        String packID = resourcePack.manifest().header().uuid().toString();
-        if (packs.containsValue(resourcePack) || packs.containsKey(packID)) {
+        UUID uuid = resourcePack.manifest().header().uuid();
+        if (packs.containsValue(resourcePack) || packs.containsKey(uuid)) {
             return false;
         }
 
-        String uuid = resourcePack.manifest().header().uuid().toString();
         packs.put(uuid, resourcePack);
         options.put(uuid, List.of(resourcePackOptions));
         return true;
@@ -143,12 +155,12 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
 
     @Override
     public Collection<ResourcePackOption> options(UUID uuid) {
-        return Collections.unmodifiableCollection(options.get(uuid.toString()));
+        return Collections.unmodifiableCollection(options.get(uuid));
     }
 
     @Override
     public boolean unregister(@NonNull UUID uuid) {
-        options.remove(uuid.toString());
-        return packs.remove(uuid.toString()) != null;
+        options.remove(uuid);
+        return packs.remove(uuid) != null;
     }
 }

core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java

@@ -234,7 +234,6 @@ public class UpstreamPacketHandler extends LoggingPacketHandler {
                 stackPacket.setExperimentsPreviouslyToggled(false);
                 stackPacket.setForcedToAccept(false); // Leaving this as false allows the player to choose to download or not
                 stackPacket.setGameVersion(session.getClientData().getGameVersion());
-
                 stackPacket.getResourcePacks().addAll(this.resourcePackLoadEvent.orderedPacks());
 
                 if (GeyserImpl.getInstance().getConfig().isAddNonBedrockItems()) {

core/src/main/java/org/geysermc/geyser/pack/GeyserResourcePack.java

@@ -36,8 +36,10 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 
 public record GeyserResourcePack(
     PackCodec codec,
@@ -105,6 +107,15 @@ public record GeyserResourcePack(
         }
 
         public GeyserResourcePack build() {
+            // Check for duplicates
+            Set<ResourcePackOption.Type> types = new HashSet<>();
+            for (ResourcePackOption option : defaultOptions) {
+                if (!types.add(option.type())) {
+                    throw new IllegalArgumentException("Duplicate resource pack option " + option + "!");
+                }
+            }
+            types.clear();
+
             GeyserResourcePack pack = new GeyserResourcePack(codec, manifest, contentKey, defaultOptions);
             defaultOptions.forEach(option -> option.validate(pack));
             return pack;

core/src/main/java/org/geysermc/geyser/registry/Registries.java

@@ -145,7 +145,7 @@ public final class Registries {
     /**
      * A mapped registry holding {@link ResourcePack}'s with the pack uuid as keys.
      */
-    public static final DeferredRegistry<Map<String, ResourcePack>> RESOURCE_PACKS = DeferredRegistry.create(GeyserImpl.getInstance().packDirectory(), SimpleMappedRegistry::create, RegistryLoaders.RESOURCE_PACKS);
+    public static final DeferredRegistry<Map<UUID, ResourcePack>> RESOURCE_PACKS = DeferredRegistry.create(GeyserImpl.getInstance().packDirectory(), SimpleMappedRegistry::create, RegistryLoaders.RESOURCE_PACKS);
 
     /**
      * A mapped registry holding sound identifiers to their corresponding {@link SoundMapping}.

core/src/main/java/org/geysermc/geyser/registry/loader/ResourcePackLoader.java

@@ -45,6 +45,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -54,7 +55,7 @@ import java.util.zip.ZipFile;
 /**
  * Loads {@link ResourcePack}s within a {@link Path} directory, firing the {@link GeyserLoadResourcePacksEvent}.
  */
-public class ResourcePackLoader implements RegistryLoader<Path, Map<String, ResourcePack>> {
+public class ResourcePackLoader implements RegistryLoader<Path, Map<UUID, ResourcePack>> {
 
     static final PathMatcher PACK_MATCHER = FileSystems.getDefault().getPathMatcher("glob:**.{zip,mcpack}");
 
@@ -64,8 +65,8 @@ public class ResourcePackLoader implements RegistryLoader<Path, Map<String, Reso
      * Loop through the packs directory and locate valid resource pack files
      */
     @Override
-    public Map<String, ResourcePack> load(Path directory) {
-        Map<String, ResourcePack> packMap = new HashMap<>();
+    public Map<UUID, ResourcePack> load(Path directory) {
+        Map<UUID, ResourcePack> packMap = new HashMap<>();
 
         if (!Files.exists(directory)) {
             try {
@@ -100,7 +101,7 @@ public class ResourcePackLoader implements RegistryLoader<Path, Map<String, Reso
         for (Path path : event.resourcePacks()) {
             try {
                 GeyserResourcePack pack = readPack(path).build();
-                packMap.put(pack.manifest().header().uuid().toString(), pack);
+                packMap.put(pack.manifest().header().uuid(), pack);
             } catch (Exception e) {
                 e.printStackTrace();
             }