From b8fa18a155ef3c07517eeb0f97e5dd53ddd0cdf8 Mon Sep 17 00:00:00 2001 From: onebeastchris Date: Mon, 19 Feb 2024 21:26:35 +0100 Subject: [PATCH] start: don't try to delete broken packs while we are still delivering them --- .../geyser/network/UpstreamPacketHandler.java | 9 ++++----- .../geyser/pack/url/GeyserUrlPackCodec.java | 4 ++-- .../geyser/registry/loader/ResourcePackLoader.java | 8 ++++---- .../java/org/geysermc/geyser/util/WebUtils.java | 13 ++++++++----- 4 files changed, 18 insertions(+), 16 deletions(-) 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 da3f0c071..f52ec378e 100644 --- a/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java +++ b/core/src/main/java/org/geysermc/geyser/network/UpstreamPacketHandler.java @@ -77,16 +77,15 @@ import java.nio.channels.SeekableByteChannel; import java.util.ArrayDeque; import java.util.Deque; import java.util.HashMap; -import java.util.HashSet; +import java.util.Map; import java.util.OptionalInt; -import java.util.Set; import java.util.UUID; public class UpstreamPacketHandler extends LoggingPacketHandler { private boolean networkSettingsRequested = false; private final Deque packsToSent = new ArrayDeque<>(); - private final Set brokenResourcePacks = new HashSet<>(); + private final Map brokenResourcePacks = new HashMap<>(); private final CompressionStrategy compressionStrategy; private SessionLoadResourcePacksEventImpl resourcePackLoadEvent; @@ -315,8 +314,8 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { // If a remote pack ends up here, that usually implies that a platform was not able to download the pack if (codec instanceof UrlPackCodec urlPackCodec) { // Ensure we don't a. spam console, and b. spam download/check requests - if (!brokenResourcePacks.contains(packet.getPackId())) { - brokenResourcePacks.add(packet.getPackId()); + if (!brokenResourcePacks.containsKey(packet.getPackId())) { + brokenResourcePacks.put(packet.getPackId(), ""); GeyserImpl.getInstance().getLogger().warning("Received a request for a remote pack that the client should have already downloaded! " + "Is the pack at the URL " + urlPackCodec.url() + " still available?"); // not actually interested in using the download, but this does all the checks we need diff --git a/core/src/main/java/org/geysermc/geyser/pack/url/GeyserUrlPackCodec.java b/core/src/main/java/org/geysermc/geyser/pack/url/GeyserUrlPackCodec.java index 04bb70663..0b0ce32e3 100644 --- a/core/src/main/java/org/geysermc/geyser/pack/url/GeyserUrlPackCodec.java +++ b/core/src/main/java/org/geysermc/geyser/pack/url/GeyserUrlPackCodec.java @@ -88,9 +88,9 @@ public class GeyserUrlPackCodec extends UrlPackCodec { try { final Path downloadedPack = ResourcePackLoader.downloadPack(url, false).whenComplete((pack, throwable) -> { if (throwable != null) { - GeyserImpl.getInstance().getLogger().error("Failed to download pack from " + url, throwable); + GeyserImpl.getInstance().getLogger().error("Failed to download pack from " + url + " due to " + throwable.getMessage()); if (GeyserImpl.getInstance().getConfig().isDebugMode()) { - throwable.printStackTrace(); + GeyserImpl.getInstance().getLogger().error("full error: " + throwable); } } }).join(); 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 e41ae7608..bc784b85d 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 @@ -231,8 +231,8 @@ public class ResourcePackLoader implements RegistryLoader downloadPack(String url, boolean force) throws IllegalArgumentException { - return WebUtils.checkUrlAndDownloadRemotePack(url, force).whenCompleteAsync((cachedPath, throwable) -> { + public static CompletableFuture<@Nullable Path> downloadPack(String url, boolean checking) throws IllegalArgumentException { + return WebUtils.checkUrlAndDownloadRemotePack(url, checking).whenCompleteAsync((cachedPath, throwable) -> { if (cachedPath == null) { // already warned about in WebUtils return; @@ -257,9 +257,9 @@ public class ResourcePackLoader implements RegistryLoader checkUrlAndDownloadRemotePack(String url, boolean force) { @@ -124,7 +124,7 @@ public class WebUtils { con.setConnectTimeout(10000); con.setReadTimeout(10000); con.setRequestProperty("User-Agent", "Geyser-" + GeyserImpl.getInstance().getPlatformType().platformName() + "/" + GeyserImpl.VERSION); - con.setInstanceFollowRedirects(false); // TODO verify + con.setInstanceFollowRedirects(true); int responseCode = con.getResponseCode(); if (responseCode >= 400) { @@ -140,11 +140,13 @@ public class WebUtils { return null; } + // This doesn't seem to be a requirement (anymore?) Logging to debug might be interesting though. if (type == null || !type.equals("application/zip")) { - GeyserImpl.getInstance().getLogger().error(String.format("Invalid application type from remote pack URL: %s (type: %s)", url, type)); - return null; + GeyserImpl.getInstance().getLogger().debug(String.format("Application type from remote pack URL: %s (type: %s)", url, type)); } + // TODO: add logic here to *not* delete the cached pack (and only at shutdown). + Path packLocation = REMOTE_PACK_CACHE.resolve(url.hashCode() + ".zip"); Path packMetadata = packLocation.resolveSibling(url.hashCode() + ".metadata"); @@ -170,7 +172,7 @@ public class WebUtils { if (Files.size(packLocation) != size) { GeyserImpl.getInstance().getLogger().error(String.format("Size mismatch with resource pack at url: %s. Downloaded pack has %s bytes, expected %s bytes!", url, Files.size(packLocation), size)); Files.delete(packLocation); - return null; + //return null; } try { @@ -179,6 +181,7 @@ public class WebUtils { GeyserImpl.getInstance().getLogger().error("Failed to write cached pack metadata: " + e.getMessage()); } + GeyserImpl.getInstance().getLogger().info("debug: pack downloaded"); return packLocation; } catch (MalformedURLException e) { throw new IllegalArgumentException("Malformed URL: " + url);