Some more cleanup, address Konica's review comments on old PR

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

@@ -54,7 +54,7 @@ public abstract class SessionLoadResourcePacksEvent extends ConnectionEvent {
     /**
      * Registers a {@link ResourcePack} to be sent to the client.
      *
-     * @param pack a resource pack that will be sent to the client.
+     * @param pack a resource pack that will be sent to the client
      * @return true if the resource pack was added successfully,
      *         or false if already present
      */
@@ -84,7 +84,7 @@ public abstract class SessionLoadResourcePacksEvent extends ConnectionEvent {
      * Returns the subpack options set for a specific resource pack uuid.
      * These are not modifiable.
      *
-     * @param uuid the resourcePack for which the options are set
+     * @param uuid the resource pack for which the options are set
      * @return a list of {@link ResourcePackOption}
      */
     public abstract Collection<ResourcePackOption<?>> options(@NonNull UUID uuid);

api/src/main/java/org/geysermc/geyser/api/event/lifecycle/GeyserDefineResourcePacksEvent.java

@@ -49,7 +49,7 @@ public abstract class GeyserDefineResourcePacksEvent implements Event {
 
     /**
      * Registers a {@link ResourcePack} to be sent to the client, optionally alongside
-     * specific options.
+     * {@link ResourcePackOption} options specifying how it will be applied on clients.
      *
      * @param pack a resource pack that will be sent to the client.
      * @param options {@link ResourcePackOption}'s that specify how clients load the pack
@@ -71,7 +71,7 @@ public abstract class GeyserDefineResourcePacksEvent implements Event {
      * Returns the subpack options set for a specific resource pack uuid.
      * These are not modifiable.
      *
-     * @param uuid the resourcePack for which the options are set
+     * @param uuid the resource pack uuid for which the options are set
      * @return a list of {@link ResourcePackOption}
      */
     public abstract Collection<ResourcePackOption<?>> options(@NonNull UUID uuid);
@@ -88,7 +88,7 @@ public abstract class GeyserDefineResourcePacksEvent implements Event {
     /**
      * Unregisters a {@link ResourcePack} from being sent to clients.
      *
-     * @param uuid the UUID of the resource pack to remove.
+     * @param uuid the uuid of the resource pack to remove.
      * @return true whether the resource pack was removed successfully.
      */
     public abstract boolean unregister(@NonNull UUID uuid);

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

@@ -69,6 +69,7 @@ public interface ResourcePackManifest {
 
     /**
      * Gets the subpacks of the resource pack.
+     * See <a href="https://learn.microsoft.com/en-us/minecraft/creator/documents/utilizingsubpacks">Microsoft's docs</a> for more information.
      *
      * @return the subpacks
      */
@@ -189,12 +190,13 @@ public interface ResourcePackManifest {
     }
 
     /**
-     * Represents a subpack of a resource pack
+     * Represents a subpack of a resource pack.
+     * See <a href="https://learn.microsoft.com/en-us/minecraft/creator/documents/utilizingsubpacks">Micoroft's docs</a> for more information.
      */
     interface Subpack {
 
         /**
-         * Gets the folder name of the subpack.
+         * Gets the folder name of this subpack.
          *
          * @return the folder name
          */
@@ -202,7 +204,7 @@ public interface ResourcePackManifest {
         String folderName();
 
         /**
-         * Gets the name of the subpack.
+         * Gets the name of this subpack. Required for each sub pack to be valid.
          * It can be sent to the Bedrock client alongside the pack
          * to load a particular subpack within a resource pack.
          *
@@ -212,9 +214,9 @@ public interface ResourcePackManifest {
         String name();
 
         /**
-         * Gets the memory tier of the subpack.
-         * One memory tier requires 0.25 GB of free memory
-         * that a device must have to run a sub-pack.
+         * Gets the memory tier of this Subpack, representing how much RAM a device must have to run it.
+         * Each memory tier requires 0.25 GB of RAM. For example, a memory tier of 0 is no requirement,
+         * and a memory tier of 4 requires 1GB of RAM.
          *
          * @return the memory tier
          */

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

@@ -55,7 +55,11 @@ public class GeyserDefineResourcePacksEventImpl extends GeyserDefineResourcePack
 
     @Override
     public boolean register(@NonNull ResourcePack resourcePack, @Nullable ResourcePackOption<?>... options) {
-        GeyserResourcePack pack = validate(resourcePack);
+        Objects.requireNonNull(resourcePack, "resource pack must not be null!");
+        if (!(resourcePack instanceof GeyserResourcePack pack)) {
+            throw new IllegalArgumentException("unknown resource pack implementation: %s".
+                formatted(resourcePack.getClass().getSuperclass().getName()));
+        }
 
         UUID uuid = resourcePack.uuid();
         if (packs.containsKey(uuid)) {
@@ -63,7 +67,7 @@ public class GeyserDefineResourcePacksEventImpl extends GeyserDefineResourcePack
         }
 
         ResourcePackHolder holder = ResourcePackHolder.of(pack);
-        registerOption(holder, options);
+        attemptRegisterOptions(holder, options);
         packs.put(uuid, holder);
         return true;
     }
@@ -78,7 +82,7 @@ public class GeyserDefineResourcePacksEventImpl extends GeyserDefineResourcePack
             throw new IllegalArgumentException("resource pack with uuid " + uuid + " not found, unable to register options");
         }
 
-        registerOption(holder, options);
+        attemptRegisterOptions(holder, options);
     }
 
     @Override
@@ -110,21 +114,11 @@ public class GeyserDefineResourcePacksEventImpl extends GeyserDefineResourcePack
         return packs.remove(uuid) != null;
     }
 
-    private void registerOption(@NonNull ResourcePackHolder holder, @Nullable ResourcePackOption<?>... options) {
+    private void attemptRegisterOptions(@NonNull ResourcePackHolder holder, @Nullable ResourcePackOption<?>... options) {
         if (options == null) {
             return;
         }
 
         holder.optionHolder().validateAndAdd(holder.pack(), options);
     }
-
-    private GeyserResourcePack validate(@NonNull ResourcePack resourcePack) {
-        Objects.requireNonNull(resourcePack);
-        if (resourcePack instanceof GeyserResourcePack geyserResourcePack) {
-            return geyserResourcePack;
-        } else {
-            throw new IllegalArgumentException("unknown resource pack implementation: %s".
-                formatted(resourcePack.getClass().getSuperclass().getName()));
-        }
-    }
 }

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

@@ -87,14 +87,18 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
 
     @Override
     public boolean register(@NonNull ResourcePack resourcePack, @Nullable ResourcePackOption<?>... options) {
-        GeyserResourcePack pack = validate(resourcePack);
+        Objects.requireNonNull(resourcePack);
+        if (!(resourcePack instanceof GeyserResourcePack pack)) {
+            throw new IllegalArgumentException("Unknown resource pack implementation: %s".
+                formatted(resourcePack.getClass().getSuperclass().getName()));
+        }
 
         UUID uuid = resourcePack.uuid();
         if (packs.containsKey(uuid)) {
             return false;
         }
 
-        registerOption(pack, options);
+        attemptRegisterOptions(pack, options);
         packs.put(uuid, ResourcePackHolder.of(pack));
         return true;
     }
@@ -108,7 +112,7 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
             throw new IllegalArgumentException("resource pack with uuid " + uuid + " not found, unable to register options");
         }
 
-        registerOption(holder.pack(), options);
+        attemptRegisterOptions(holder.pack(), options);
     }
 
     @Override
@@ -152,7 +156,7 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
         return packs.remove(uuid) != null;
     }
 
-    private void registerOption(@NonNull GeyserResourcePack pack, @Nullable ResourcePackOption<?>... options) {
+    private void attemptRegisterOptions(@NonNull GeyserResourcePack pack, @Nullable ResourcePackOption<?>... options) {
         if (options == null) {
             return;
         }
@@ -229,16 +233,4 @@ public class SessionLoadResourcePacksEventImpl extends SessionLoadResourcePacksE
     private String subpackName(GeyserResourcePack pack) {
         return value(pack.uuid(), ResourcePackOption.Type.SUBPACK, "");
     }
-
-    // Helper method to validate a pack
-
-    private GeyserResourcePack validate(@NonNull ResourcePack resourcePack) {
-        Objects.requireNonNull(resourcePack);
-        if (resourcePack instanceof GeyserResourcePack geyserResourcePack) {
-            return geyserResourcePack;
-        } else {
-            throw new IllegalArgumentException("Unknown resource pack implementation: %s".
-                formatted(resourcePack.getClass().getSuperclass().getName()));
-        }
-    }
 }