From 936c716870227b3444e151ed258b11b948dc5e78 Mon Sep 17 00:00:00 2001 From: onebeastchris Date: Thu, 19 Sep 2024 18:43:41 +0800 Subject: [PATCH] Some more cleanup, address Konica's review comments on old PR --- .../SessionLoadResourcePacksEvent.java | 4 ++-- .../GeyserDefineResourcePacksEvent.java | 6 ++--- .../geyser/api/pack/ResourcePackManifest.java | 14 ++++++----- .../GeyserDefineResourcePacksEventImpl.java | 22 +++++++---------- .../SessionLoadResourcePacksEventImpl.java | 24 +++++++------------ 5 files changed, 29 insertions(+), 41 deletions(-) diff --git a/api/src/main/java/org/geysermc/geyser/api/event/bedrock/SessionLoadResourcePacksEvent.java b/api/src/main/java/org/geysermc/geyser/api/event/bedrock/SessionLoadResourcePacksEvent.java index efa6b4ea8..19c16f811 100644 --- a/api/src/main/java/org/geysermc/geyser/api/event/bedrock/SessionLoadResourcePacksEvent.java +++ b/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> options(@NonNull UUID uuid); diff --git a/api/src/main/java/org/geysermc/geyser/api/event/lifecycle/GeyserDefineResourcePacksEvent.java b/api/src/main/java/org/geysermc/geyser/api/event/lifecycle/GeyserDefineResourcePacksEvent.java index fbd26d05a..44b158fba 100644 --- a/api/src/main/java/org/geysermc/geyser/api/event/lifecycle/GeyserDefineResourcePacksEvent.java +++ b/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> 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); diff --git a/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePackManifest.java b/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePackManifest.java index 4484afbb2..9cec9a597 100644 --- a/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePackManifest.java +++ b/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 Microsoft's docs 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 Micoroft's docs 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 */ diff --git a/core/src/main/java/org/geysermc/geyser/event/type/GeyserDefineResourcePacksEventImpl.java b/core/src/main/java/org/geysermc/geyser/event/type/GeyserDefineResourcePacksEventImpl.java index 3ddf66e4a..a2369f124 100644 --- a/core/src/main/java/org/geysermc/geyser/event/type/GeyserDefineResourcePacksEventImpl.java +++ b/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())); - } - } } 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 49c5b01e2..278cfe14b 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 @@ -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())); - } - } }