From 250a9b43d1e9e551c31c9f55eda461815f7aa88d Mon Sep 17 00:00:00 2001 From: onebeastchris Date: Tue, 13 Aug 2024 01:31:36 +0200 Subject: [PATCH] Check for duplicate options, other fixes --- .../SessionLoadResourcePacksEvent.java | 3 +- .../geyser/api/pack/ResourcePack.java | 2 - .../SessionLoadResourcePacksEventImpl.java | 60 +++++++++++-------- .../geyser/network/UpstreamPacketHandler.java | 1 - .../geyser/pack/GeyserResourcePack.java | 11 ++++ .../geysermc/geyser/registry/Registries.java | 2 +- .../registry/loader/ResourcePackLoader.java | 9 +-- 7 files changed, 55 insertions(+), 33 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 ddb3a3205..d5a8aea87 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 @@ -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); - } diff --git a/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePack.java b/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePack.java index 1bec9af29..29aa340ff 100644 --- a/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePack.java +++ b/api/src/main/java/org/geysermc/geyser/api/pack/ResourcePack.java @@ -36,8 +36,6 @@ import java.util.Collection; *

* This representation of a resource pack only contains what * Geyser requires to send it to the client. - *

- * Optionally, a content key and/or a subpack name to load can be provided. */ public interface ResourcePack { 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 40e142f79..a7f576145 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 @@ -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 packs; - private final Map> options = new HashMap<>(); + private final Map packs; + private final Map> 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 orderedPacks() { - return packs.values().stream() + TreeSet> 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 infoPacketEntries() { List 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 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 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 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; } } 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 fde7e5d70..c998ee4c3 100644 --- a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java +++ b/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()) { diff --git a/core/src/main/java/org/geysermc/geyser/pack/GeyserResourcePack.java b/core/src/main/java/org/geysermc/geyser/pack/GeyserResourcePack.java index 8f209ff66..dbbe9179d 100644 --- a/core/src/main/java/org/geysermc/geyser/pack/GeyserResourcePack.java +++ b/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 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; diff --git a/core/src/main/java/org/geysermc/geyser/registry/Registries.java b/core/src/main/java/org/geysermc/geyser/registry/Registries.java index 30d3c0763..95d2f9f28 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/Registries.java +++ b/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> RESOURCE_PACKS = DeferredRegistry.create(GeyserImpl.getInstance().packDirectory(), SimpleMappedRegistry::create, RegistryLoaders.RESOURCE_PACKS); + public static final DeferredRegistry> RESOURCE_PACKS = DeferredRegistry.create(GeyserImpl.getInstance().packDirectory(), SimpleMappedRegistry::create, RegistryLoaders.RESOURCE_PACKS); /** * A mapped registry holding sound identifiers to their corresponding {@link SoundMapping}. diff --git a/core/src/main/java/org/geysermc/geyser/registry/loader/ResourcePackLoader.java b/core/src/main/java/org/geysermc/geyser/registry/loader/ResourcePackLoader.java index 98ec39375..7df215464 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/loader/ResourcePackLoader.java +++ b/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> { +public class ResourcePackLoader implements RegistryLoader> { static final PathMatcher PACK_MATCHER = FileSystems.getDefault().getPathMatcher("glob:**.{zip,mcpack}"); @@ -64,8 +65,8 @@ public class ResourcePackLoader implements RegistryLoader load(Path directory) { - Map packMap = new HashMap<>(); + public Map load(Path directory) { + Map packMap = new HashMap<>(); if (!Files.exists(directory)) { try { @@ -100,7 +101,7 @@ public class ResourcePackLoader implements RegistryLoader