diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000000..d823c105c1a --- /dev/null +++ b/.gitattributes @@ -0,0 +1,18 @@ +* text=auto eol=lf + +*.bat text eol=crlf + +*.jar binary +*.png binary +*.der binary +*.dat binary +*.mca binary + +*.java diff=java +*.java.peb diff=java +*.kt diff=kotlin +*.kts diff=kotlin +*.md diff=markdown + +/src/autogenerated/** linguist-generated +/src/main/resources/META-INF/native-image/** linguist-generated diff --git a/demo/src/main/java/net/minestom/demo/commands/CookieCommand.java b/demo/src/main/java/net/minestom/demo/commands/CookieCommand.java index 01a71745549..b62309faf91 100644 --- a/demo/src/main/java/net/minestom/demo/commands/CookieCommand.java +++ b/demo/src/main/java/net/minestom/demo/commands/CookieCommand.java @@ -19,7 +19,7 @@ public CookieCommand() { } public static class Store extends Command { - private final Argument keyArg = ArgumentType.ResourceLocation("key"); + private final Argument keyArg = ArgumentType.Key("key"); private final Argument valueArg = ArgumentType.StringArray("value"); public Store() { @@ -40,7 +40,7 @@ private void store(CommandSender sender, CommandContext context) { } public static class Fetch extends Command { - private final Argument keyArg = ArgumentType.ResourceLocation("key"); + private final Argument keyArg = ArgumentType.Key("key"); public Fetch() { super("fetch"); diff --git a/gradlew.bat b/gradlew.bat index 03754c4c3d9..3185a43f78f 100644 --- a/gradlew.bat +++ b/gradlew.bat @@ -1,112 +1,112 @@ -@rem -@rem Copyright 2015 the original author or authors. -@rem -@rem Licensed under the Apache License, Version 2.0 (the "License"); -@rem you may not use this file except in compliance with the License. -@rem You may obtain a copy of the License at -@rem -@rem https://www.apache.org/licenses/LICENSE-2.0 -@rem -@rem Unless required by applicable law or agreed to in writing, software -@rem distributed under the License is distributed on an "AS IS" BASIS, -@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -@rem See the License for the specific language governing permissions and -@rem limitations under the License. -@rem -@rem SPDX-License-Identifier: Apache-2.0 -@rem - -@if "%DEBUG%"=="" @echo off -@rem ########################################################################## -@rem -@rem gradlew startup script for Windows -@rem -@rem ########################################################################## - -@rem Set local scope for the variables, and ensure extensions are enabled -setlocal EnableExtensions - -@rem Catch executions from older scripts and ensure they exit cleanly. -@rem This can be removed once we can be reasonably confident that few people -@rem will be migrating directly to this new wrapper. -goto afterSafetyNet -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: -goto exitWithErrorLevel -:afterSafetyNet - -set DIRNAME=%~dp0 -if "%DIRNAME%"=="" set DIRNAME=. -@rem This is normally unused -set APP_BASE_NAME=%~n0 -set APP_HOME=%DIRNAME% - -@rem Resolve any "." and ".." in APP_HOME to make it shorter. -for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi - -@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m" - -@rem Find java.exe -if defined JAVA_HOME goto findJavaFromJavaHome - -set JAVA_EXE=java.exe -%JAVA_EXE% -version >NUL 2>&1 -if %ERRORLEVEL% equ 0 goto execute - -1>&2 echo. -1>&2 echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. -1>&2 echo. -1>&2 echo Please set the JAVA_HOME variable in your environment to match the -1>&2 echo location of your Java installation. - -"%COMSPEC%" /c exit 1 -goto exitWithErrorLevel - -:findJavaFromJavaHome -set JAVA_HOME=%JAVA_HOME:"=% -set JAVA_EXE=%JAVA_HOME%/bin/java.exe - -if exist "%JAVA_EXE%" goto execute - -1>&2 echo. -1>&2 echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME% -1>&2 echo. -1>&2 echo Please set the JAVA_HOME variable in your environment to match the -1>&2 echo location of your Java installation. - -"%COMSPEC%" /c exit 1 -goto exitWithErrorLevel - -:execute -@rem Setup the command line - - - -@rem Execute gradlew -@rem endlocal doesn't take effect until after the line is parsed and variables are expanded -@rem which allows us to clear the local environment before executing the java command -endlocal & "%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* & call :exitWithErrorLevel & goto exitWithErrorLevel - -@rem This label must not be changed. We rely on old scripts being able to jump to this point. -:exitWithErrorLevel -@rem Use "%COMSPEC%" /c exit to allow operators to work properly in scripts -"%COMSPEC%" /c exit %ERRORLEVEL% +@rem +@rem Copyright 2015 the original author or authors. +@rem +@rem Licensed under the Apache License, Version 2.0 (the "License"); +@rem you may not use this file except in compliance with the License. +@rem You may obtain a copy of the License at +@rem +@rem https://www.apache.org/licenses/LICENSE-2.0 +@rem +@rem Unless required by applicable law or agreed to in writing, software +@rem distributed under the License is distributed on an "AS IS" BASIS, +@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +@rem See the License for the specific language governing permissions and +@rem limitations under the License. +@rem +@rem SPDX-License-Identifier: Apache-2.0 +@rem + +@if "%DEBUG%"=="" @echo off +@rem ########################################################################## +@rem +@rem gradlew startup script for Windows +@rem +@rem ########################################################################## + +@rem Set local scope for the variables, and ensure extensions are enabled +setlocal EnableExtensions + +@rem Catch executions from older scripts and ensure they exit cleanly. +@rem This can be removed once we can be reasonably confident that few people +@rem will be migrating directly to this new wrapper. +goto afterSafetyNet +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: +goto exitWithErrorLevel +:afterSafetyNet + +set DIRNAME=%~dp0 +if "%DIRNAME%"=="" set DIRNAME=. +@rem This is normally unused +set APP_BASE_NAME=%~n0 +set APP_HOME=%DIRNAME% + +@rem Resolve any "." and ".." in APP_HOME to make it shorter. +for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi + +@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. +set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m" + +@rem Find java.exe +if defined JAVA_HOME goto findJavaFromJavaHome + +set JAVA_EXE=java.exe +%JAVA_EXE% -version >NUL 2>&1 +if %ERRORLEVEL% equ 0 goto execute + +1>&2 echo. +1>&2 echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. +1>&2 echo. +1>&2 echo Please set the JAVA_HOME variable in your environment to match the +1>&2 echo location of your Java installation. + +"%COMSPEC%" /c exit 1 +goto exitWithErrorLevel + +:findJavaFromJavaHome +set JAVA_HOME=%JAVA_HOME:"=% +set JAVA_EXE=%JAVA_HOME%/bin/java.exe + +if exist "%JAVA_EXE%" goto execute + +1>&2 echo. +1>&2 echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME% +1>&2 echo. +1>&2 echo Please set the JAVA_HOME variable in your environment to match the +1>&2 echo location of your Java installation. + +"%COMSPEC%" /c exit 1 +goto exitWithErrorLevel + +:execute +@rem Setup the command line + + + +@rem Execute gradlew +@rem endlocal doesn't take effect until after the line is parsed and variables are expanded +@rem which allows us to clear the local environment before executing the java command +endlocal & "%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* & call :exitWithErrorLevel & goto exitWithErrorLevel + +@rem This label must not be changed. We rely on old scripts being able to jump to this point. +:exitWithErrorLevel +@rem Use "%COMSPEC%" /c exit to allow operators to work properly in scripts +"%COMSPEC%" /c exit %ERRORLEVEL% diff --git a/src/main/java/net/minestom/server/command/builder/arguments/ArgumentType.java b/src/main/java/net/minestom/server/command/builder/arguments/ArgumentType.java index ac4e5c9c176..534c8fdcd8d 100644 --- a/src/main/java/net/minestom/server/command/builder/arguments/ArgumentType.java +++ b/src/main/java/net/minestom/server/command/builder/arguments/ArgumentType.java @@ -6,10 +6,10 @@ import net.minestom.server.command.builder.arguments.minecraft.ArgumentFloatRange; import net.minestom.server.command.builder.arguments.minecraft.ArgumentIntRange; import net.minestom.server.command.builder.arguments.minecraft.ArgumentItemStack; +import net.minestom.server.command.builder.arguments.minecraft.ArgumentKey; import net.minestom.server.command.builder.arguments.minecraft.ArgumentNbtCompoundTag; import net.minestom.server.command.builder.arguments.minecraft.ArgumentNbtTag; import net.minestom.server.command.builder.arguments.minecraft.ArgumentResource; -import net.minestom.server.command.builder.arguments.minecraft.ArgumentResourceLocation; import net.minestom.server.command.builder.arguments.minecraft.ArgumentResourceOrTag; import net.minestom.server.command.builder.arguments.minecraft.ArgumentTeamColor; import net.minestom.server.command.builder.arguments.minecraft.ArgumentTime; @@ -183,12 +183,12 @@ public static ArgumentResource Resource(String id, String identifier) { } /** - * Creates a new {@link ArgumentResourceLocation}. + * Creates a new {@link ArgumentKey}. * - * @see ArgumentResourceLocation + * @see ArgumentKey */ - public static ArgumentResourceLocation ResourceLocation(String id) { - return new ArgumentResourceLocation(id); + public static ArgumentKey Key(String id) { + return new ArgumentKey(id); } /** diff --git a/src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentResourceLocation.java b/src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentKey.java similarity index 72% rename from src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentResourceLocation.java rename to src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentKey.java index e30c757b627..55fc6c71f35 100644 --- a/src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentResourceLocation.java +++ b/src/main/java/net/minestom/server/command/builder/arguments/minecraft/ArgumentKey.java @@ -8,23 +8,23 @@ import net.minestom.server.command.builder.exception.ArgumentSyntaxException; /** - * Represents a resource location (namespaced identifier) value. + * Represents a {@link Key} value. *

* Example: {@code minecraft:air} *

*/ -public class ArgumentResourceLocation extends Argument { +public class ArgumentKey extends Argument { public static final int PARSE_ERROR = 1; - public ArgumentResourceLocation(String id) { + public ArgumentKey(String id) { super(id); } @Override public Key parse(CommandSender sender, @KeyPattern String input) throws ArgumentSyntaxException { if (!Key.parseable(input)) - throw new ArgumentSyntaxException("Invalid resource location", input, PARSE_ERROR); + throw new ArgumentSyntaxException("Invalid key", input, PARSE_ERROR); return Key.key(input); } @@ -36,6 +36,6 @@ public ArgumentParserType parser() { @Override public String toString() { - return String.format("ResourceLocation<%s>", getId()); + return String.format("Key<%s>", getId()); } } diff --git a/src/main/java/net/minestom/server/command/builder/parser/ArgumentParser.java b/src/main/java/net/minestom/server/command/builder/parser/ArgumentParser.java index b43d163531c..7345d3d7068 100644 --- a/src/main/java/net/minestom/server/command/builder/parser/ArgumentParser.java +++ b/src/main/java/net/minestom/server/command/builder/parser/ArgumentParser.java @@ -15,9 +15,9 @@ import net.minestom.server.command.builder.arguments.minecraft.ArgumentFloatRange; import net.minestom.server.command.builder.arguments.minecraft.ArgumentIntRange; import net.minestom.server.command.builder.arguments.minecraft.ArgumentItemStack; +import net.minestom.server.command.builder.arguments.minecraft.ArgumentKey; import net.minestom.server.command.builder.arguments.minecraft.ArgumentNbtCompoundTag; import net.minestom.server.command.builder.arguments.minecraft.ArgumentNbtTag; -import net.minestom.server.command.builder.arguments.minecraft.ArgumentResourceLocation; import net.minestom.server.command.builder.arguments.minecraft.ArgumentTeamColor; import net.minestom.server.command.builder.arguments.minecraft.ArgumentTime; import net.minestom.server.command.builder.arguments.minecraft.ArgumentUUID; @@ -59,7 +59,7 @@ public class ArgumentParser { ARGUMENT_FUNCTION_MAP.put("color", ArgumentTeamColor::new); ARGUMENT_FUNCTION_MAP.put("time", ArgumentTime::new); ARGUMENT_FUNCTION_MAP.put("particle", ArgumentParticle::new); - ARGUMENT_FUNCTION_MAP.put("resourcelocation", ArgumentResourceLocation::new); + ARGUMENT_FUNCTION_MAP.put("key", ArgumentKey::new); ARGUMENT_FUNCTION_MAP.put("entitytype", ArgumentEntityType::new); ARGUMENT_FUNCTION_MAP.put("blockstate", ArgumentBlockState::new); ARGUMENT_FUNCTION_MAP.put("intrange", ArgumentIntRange::new); diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 06545cedfd1..e592c2ddb46 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -155,6 +155,7 @@ import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; @@ -200,6 +201,8 @@ public class Player extends LivingEntity implements CommandSender, HoverEventSou private PlayerSkin skin; private @Nullable Instance pendingInstance = null; + // Counts the calls to setInstance, so a spawn waiting for chunks knows when a later call replaces it + private final AtomicInteger instanceRequests = new AtomicInteger(); private int dimensionTypeId; private volatile GameMode gameMode; private WorldPos deathLocation; @@ -671,6 +674,9 @@ public void sendPacketToViewersAndSelf(SendablePacket packet) { *

* Be aware that because chunk operations are expensive, * it is possible for this method to be non-blocking when retrieving chunks is required. + *

+ * A future that waits for chunks completes with a {@link CancellationException} when a later call to this method + * replaces it, and the player stays wherever that later call moves it. * * @param instance the new player instance * @param spawnPosition the new position of the player @@ -681,6 +687,7 @@ public CompletableFuture setInstance(Instance instance, Pos spawnPosition) Check.argCondition(instance.process() != process(), "Instance belongs to another process"); final Instance currentInstance = this.instance; Check.argCondition(currentInstance == instance, "Instance should be different than the current one"); + final int request = instanceRequests.incrementAndGet(); if (SharedInstance.areLinked(currentInstance, instance) && spawnPosition.sameChunk(this.position)) { // The player already has the good version of all the chunks. // We just need to refresh his entity viewing list and add him to the instance @@ -731,6 +738,10 @@ public Void join() { var _ = CompletableFuture.allOf(futures.toArray(CompletableFuture[]::new)) .thenRun(() -> { scheduler.scheduleNextProcess(() -> { + if (instanceRequests.get() != request) { + future.completeExceptionally(new CancellationException("Another setInstance call replaced this one")); + return; + } runnable.accept(instance); future.complete(null); }); diff --git a/src/main/java/net/minestom/server/network/packet/server/CachedPacket.java b/src/main/java/net/minestom/server/network/packet/server/CachedPacket.java index 4fbca8b2053..2832d94eb47 100644 --- a/src/main/java/net/minestom/server/network/packet/server/CachedPacket.java +++ b/src/main/java/net/minestom/server/network/packet/server/CachedPacket.java @@ -2,6 +2,7 @@ import net.minestom.server.network.NetworkBuffer; import net.minestom.server.network.packet.PacketEncodingContext; +import net.minestom.server.property.ServerProperties; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; @@ -27,9 +28,9 @@ * the packet supplier directly. The supplier must therefore be thread safe if * this object may be accessed concurrently while caching is disabled. * - *

One representation is retained, for the most recently used encoding context. + *

One framed representation is retained, for the most recently used encoding context. * Sharing this object across owners, connection states, or compression settings is safe: - * a different context replaces the cached representation. Registry definitions must + * a different context replaces the framed representation and reuses the cached packet. Registry definitions must * remain stable while the representation is cached; call {@link #invalidate()} after * changing definitions or the supplied packet. */ @@ -39,7 +40,7 @@ public final class CachedPacket implements SendablePacket { static { try { - PACKET = MethodHandles.lookup().findVarHandle(CachedPacket.class, "packet", SoftReference.class); + PACKET = MethodHandles.lookup().findVarHandle(CachedPacket.class, "cache", SoftReference.class); } catch (ReflectiveOperationException e) { throw new ExceptionInInitializerError(e); } @@ -49,7 +50,7 @@ public final class CachedPacket implements SendablePacket { // Accessed through PACKET using acquire/release semantics. @SuppressWarnings("unused") - private @Nullable SoftReference packet; + private @Nullable SoftReference cache; /** * Creates a cached packet backed by the given packet supplier. @@ -98,24 +99,28 @@ public void invalidate() { * Returns the packet represented by this cached value. * *

When caching is enabled, this method may initialize or reuse the cached - * framed packet. Supplier invocation and cache computation are serialized. + * packet. Supplier invocation and cache computation are serialized. * *

When caching is disabled, the packet supplier is invoked directly and * may be invoked concurrently by multiple callers. * - * @param context the owner, connection state, and negotiated compression used when framing + * @param context the owner, connection state, and negotiated compression of the caller * @return the packet value */ public ServerPacket packet(PacketEncodingContext context) { - final FramedPacket cache = framed(context); - return cache != null ? cache.packet() : packetSupplier.get(); + final ServerProperties properties = context.properties(); + if (!properties.cachedPacket().get()) return packetSupplier.get(); + final Cache cache = cache(); + return cache != null ? cache.packet() : computePacket(properties); } /** - * Returns the cached framed packet body for the given connection state. + * Returns the cached framed packet body for the given encoding context. * - *

If caching is enabled, this method initializes the cache when necessary. - * Cache computation and supplier invocation are serialized. + *

If caching is enabled, this method frames the packet for + * {@code context} when no body is cached for it yet, reusing the cached + * packet when one exists. Cache computation and supplier invocation are + * serialized. * *

If caching is disabled, no framed body is created and {@code null} is * returned. @@ -124,12 +129,12 @@ public ServerPacket packet(PacketEncodingContext context) { * @return the framed packet body, or {@code null} when caching is disabled */ public @Nullable NetworkBuffer body(PacketEncodingContext context) { - final FramedPacket cache = framed(context); - return cache != null ? cache.body() : null; + final FramedPacket framed = framed(context); + return framed != null ? framed.body() : null; } /** - * Returns the current cached value, computing it when absent. + * Returns the framed packet cached for the given encoding context, computing it when absent. * * @param context the owner, connection state, and negotiated compression used when framing * @return the cached framed packet, or {@code null} when caching is disabled @@ -137,54 +142,73 @@ public ServerPacket packet(PacketEncodingContext context) { public @Nullable FramedPacket framed(PacketEncodingContext context) { if (!context.properties().cachedPacket().get()) return null; - final FramedPacket cache = cachedPacket(); - return cache != null && cache.context().equals(context) ? cache : computeCache(context); + final Cache cache = cache(); + final FramedPacket framed = cache != null ? cache.framed() : null; + return framed != null && framed.context().equals(context) ? framed : computeFramed(context); + } + + /** + * Returns the cached packet, computing it when absent. + * + *

This method is synchronized so that only one thread invokes the packet + * supplier at a time. The cache is checked again after acquiring this + * object's monitor because another thread may have populated it while the + * current thread was waiting. + * + * @param properties the properties of the owner requesting the packet + * @return the existing or newly computed packet + */ + private synchronized ServerPacket computePacket(ServerProperties properties) { + final Cache cache = cache(); + if (cache != null) return cache.packet(); + final ServerPacket packet = packetSupplier.get(); + PACKET.setRelease(this, new SoftReference<>(new Cache(properties, packet, null))); + return packet; } /** - * Returns an existing cached value or computes and publishes a replacement. + * Returns the existing framed packet or computes and publishes one. * *

This method is synchronized so that only one thread invokes the packet - * supplier and computes a replacement cache entry at a time. The cache is - * checked again after acquiring this object's monitor because another thread - * may have populated it while the current thread was waiting. + * supplier and frames a replacement at a time. The cache is checked again + * after acquiring this object's monitor because another thread may have + * populated it while the current thread was waiting. * *

Invalidation does not acquire this monitor. An invalidation occurring * during computation may therefore be followed by publication of the * in progress result. * * @param context the owner, connection state, and negotiated compression used when framing - * @return the existing or newly computed framed packet + * @return the existing or newly framed packet */ - private synchronized FramedPacket computeCache(PacketEncodingContext context) { - final FramedPacket cache = cachedPacket(); - if (cache != null && cache.context().equals(context)) return cache; - - final ServerPacket packet = packetSupplier.get(); - final NetworkBuffer buffer = context.frame(packet); - - final FramedPacket updated = new FramedPacket(context, packet, buffer); - PACKET.setRelease(this, new SoftReference<>(updated)); - return updated; + private synchronized FramedPacket computeFramed(PacketEncodingContext context) { + final Cache cache = cache(); + final FramedPacket cached = cache != null ? cache.framed() : null; + if (cached != null && cached.context().equals(context)) return cached; + + final ServerPacket packet = cache != null ? cache.packet() : packetSupplier.get(); + final FramedPacket framed = new FramedPacket(context, packet, context.frame(packet)); + PACKET.setRelease(this, new SoftReference<>(new Cache(context.properties(), packet, framed))); + return framed; } /** - * Returns the currently referenced cached packet. + * Returns the currently referenced cache. * *

The cached reference is read using acquire semantics. * - * @return the cached framed packet, or {@code null} if no cache exists or - * the soft reference has been cleared + * @return the cache, or {@code null} if no cache exists or the soft + * reference has been cleared */ @SuppressWarnings("unchecked") - private @Nullable FramedPacket cachedPacket() { - final SoftReference ref = (SoftReference) PACKET.getAcquire(this); + private @Nullable Cache cache() { + final SoftReference ref = (SoftReference) PACKET.getAcquire(this); return ref != null ? ref.get() : null; } /** - * Returns whether a framed packet is currently cached and has not been - * reclaimed. + * Returns whether a packet is currently cached and has not been + * reclaimed. The cached packet may not have been framed yet. * *

This result is inherently transient because the underlying soft * reference may be cleared or the cache may be invalidated immediately @@ -194,8 +218,8 @@ private synchronized FramedPacket computeCache(PacketEncodingContext context) { * available */ public boolean isValid() { - final FramedPacket cache = cachedPacket(); - return cache != null && cache.context().properties().cachedPacket().get(); + final Cache cache = cache(); + return cache != null && cache.properties().cachedPacket().get(); } /** @@ -209,6 +233,11 @@ public boolean isValid() { */ @Override public String toString() { - return String.format("CachedPacket{cache=%s}", cachedPacket()); + return String.format("CachedPacket{cache=%s}", cache()); + } + + // The packet, plus its framing for the most recently used encoding context once one requested it. + // Requires identity + private record Cache(ServerProperties properties, ServerPacket packet, @Nullable FramedPacket framed) { } } diff --git a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java index e8ce02a7b4b..e2c7cfbc0d3 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java @@ -57,7 +57,6 @@ *

* It is the implementation used for all network client. */ -@ApiStatus.Internal public class PlayerSocketConnection extends PlayerConnection { private static final Set> IMMEDIATE_PROCESS_PACKETS = Set.of( ClientHandshakePacket.class, // First received packet diff --git a/src/main/java/net/minestom/server/network/socket/ConnectionFactory.java b/src/main/java/net/minestom/server/network/socket/ConnectionFactory.java new file mode 100644 index 00000000000..25d9b908ec7 --- /dev/null +++ b/src/main/java/net/minestom/server/network/socket/ConnectionFactory.java @@ -0,0 +1,21 @@ +package net.minestom.server.network.socket; + +import net.minestom.server.network.player.PlayerSocketConnection; + +import org.jetbrains.annotations.Nullable; + +import java.net.SocketAddress; +import java.nio.channels.SocketChannel; + +@FunctionalInterface +public interface ConnectionFactory { + /** + * Creates the connection for an accepted client. + * The given threads are unstarted; they will be started by the server after this call. + * + * @return the connection to use, or {@code null} to reject the client (its channel will be closed) + */ + @Nullable PlayerSocketConnection create(SocketChannel channel, SocketAddress remoteAddress, + Thread readThread, Thread writeThread); +} + diff --git a/src/main/java/net/minestom/server/network/socket/Server.java b/src/main/java/net/minestom/server/network/socket/Server.java index aa60b370b0a..c55f984af97 100644 --- a/src/main/java/net/minestom/server/network/socket/Server.java +++ b/src/main/java/net/minestom/server/network/socket/Server.java @@ -31,6 +31,7 @@ import java.util.concurrent.atomic.AtomicReference; public final class Server implements ProcessOwned { + private volatile ConnectionFactory connectionFactory; private volatile boolean stop; private final Set connections = new HashSet<>(); private @UnknownNullability Thread acceptThread; @@ -46,6 +47,8 @@ public final class Server implements ProcessOwned { public Server(ServerProcess process, PacketParser.Client packetParser) { this.process = Objects.requireNonNull(process); this.packetParser = packetParser; + this.connectionFactory = (channel, remoteAddress, readThread, writeThread) -> + new PlayerSocketConnection(process, channel, remoteAddress, readThread, writeThread); } @Override @@ -118,7 +121,13 @@ public synchronized void start() { configureSocket(client); Thread readThread = readBuilder.unstarted(() -> playerReadLoop(reference.get())); Thread writeThread = writeBuilder.unstarted(() -> playerWriteLoop(reference.get())); - PlayerSocketConnection connection = new PlayerSocketConnection(process(), client, client.getRemoteAddress(), readThread, writeThread); + PlayerSocketConnection connection = connectionFactory.create(client, client.getRemoteAddress(), readThread, writeThread); + if (connection == null) { + client.close(); // Rejected by the factory + continue; + } + if (connection.process() != process()) + throw new IllegalStateException("Connection belongs to another process"); reference.set(connection); synchronized (this) { if (stop) { @@ -279,6 +288,14 @@ public void stop() { } } + public void setConnectionFactory(ConnectionFactory connectionFactory) { + this.connectionFactory = Objects.requireNonNull(connectionFactory); + } + + public ConnectionFactory connectionFactory() { + return connectionFactory; + } + @ApiStatus.Internal public PacketParser.Client packetParser() { return packetParser; diff --git a/src/main/java/net/minestom/server/recipe/display/SlotDisplay.java b/src/main/java/net/minestom/server/recipe/display/SlotDisplay.java index c1f7ba9644f..a32412edf23 100644 --- a/src/main/java/net/minestom/server/recipe/display/SlotDisplay.java +++ b/src/main/java/net/minestom/server/recipe/display/SlotDisplay.java @@ -5,8 +5,10 @@ import net.minestom.server.component.DataComponent; import net.minestom.server.item.ItemStackTemplate; import net.minestom.server.item.Material; +import net.minestom.server.item.armor.TrimPattern; import net.minestom.server.network.NetworkBuffer; import net.minestom.server.network.NetworkBufferTemplate; +import net.minestom.server.registry.Holder; import net.minestom.server.registry.TagKey; import java.util.ArrayList; @@ -88,19 +90,20 @@ record Dyed(SlotDisplay dye, SlotDisplay target) implements SlotDisplay { record SmithingTrim( SlotDisplay base, SlotDisplay trimMaterial, - SlotDisplay trimPattern + Holder trimPattern ) implements SlotDisplay { public static final NetworkBuffer.Type NETWORK_TYPE = NetworkBufferTemplate.template( SlotDisplay.NETWORK_TYPE, SmithingTrim::base, SlotDisplay.NETWORK_TYPE, SmithingTrim::trimMaterial, - SlotDisplay.NETWORK_TYPE, SmithingTrim::trimPattern, + TrimPattern.NETWORK_TYPE, SmithingTrim::trimPattern, SmithingTrim::new); @Override public List components() { final var components = new ArrayList<>(base.components()); components.addAll(trimMaterial.components()); - components.addAll(trimPattern.components()); + final TrimPattern pattern = trimPattern.asValue(); + if (pattern != null) components.add(pattern.description()); return List.copyOf(components); } @@ -108,7 +111,9 @@ public List components() { public SlotDisplay copyWithOperator(UnaryOperator operator) { return new SmithingTrim(base.copyWithOperator(operator), trimMaterial.copyWithOperator(operator), - trimPattern.copyWithOperator(operator)); + trimPattern instanceof TrimPattern pattern + ? TrimPattern.create(pattern.assetId(), operator.apply(pattern.description()), pattern.isDecal()) + : trimPattern); } } diff --git a/src/test/java/net/minestom/server/command/ArgumentParserTest.java b/src/test/java/net/minestom/server/command/ArgumentParserTest.java index a20fff7d6c0..f8008a8cae4 100644 --- a/src/test/java/net/minestom/server/command/ArgumentParserTest.java +++ b/src/test/java/net/minestom/server/command/ArgumentParserTest.java @@ -27,7 +27,7 @@ public void testArgumentParser() { assertParserEquals("Color", ArgumentType.TeamColor("example")); assertParserEquals("Time", ArgumentType.Time("example")); assertParserEquals("Particle", ArgumentType.Particle("example")); - assertParserEquals("ResourceLocation", ArgumentType.ResourceLocation("example")); + assertParserEquals("Key", ArgumentType.Key("example")); assertParserEquals("EntityType", ArgumentType.EntityType("example")); assertParserEquals("BlockState", ArgumentType.BlockState("example")); assertParserEquals("IntRange", ArgumentType.IntRange("example")); diff --git a/src/test/java/net/minestom/server/command/ArgumentTypeTest.java b/src/test/java/net/minestom/server/command/ArgumentTypeTest.java index 2c92dc3a2a4..8e4b8a7f093 100644 --- a/src/test/java/net/minestom/server/command/ArgumentTypeTest.java +++ b/src/test/java/net/minestom/server/command/ArgumentTypeTest.java @@ -225,14 +225,14 @@ public void testArgumentResource() { } @Test - public void testArgumentResourceLocation() { - var arg = ArgumentType.ResourceLocation("resource_location"); + public void testArgumentKey() { + var arg = ArgumentType.Key("key"); assertArg(arg, Key.key("foo:bar"), "foo:bar"); assertArg(arg, Key.key("minecraft:air"), "air"); assertArg(arg, Key.key("minecraft:foo/bar"), "foo/bar"); - assertInvalidArg(arg, "minecraft:invalid resource location"); + assertInvalidArg(arg, "minecraft:invalid key"); assertInvalidArg(arg, "!"); assertInvalidArg(arg, "a/b:empty"); } diff --git a/src/test/java/net/minestom/server/entity/player/PlayerIntegrationTest.java b/src/test/java/net/minestom/server/entity/player/PlayerIntegrationTest.java index 02870c5ffa9..635047d4122 100644 --- a/src/test/java/net/minestom/server/entity/player/PlayerIntegrationTest.java +++ b/src/test/java/net/minestom/server/entity/player/PlayerIntegrationTest.java @@ -18,6 +18,9 @@ import net.minestom.server.event.player.PlayerChunkUnloadEvent; import net.minestom.server.event.player.PlayerGameModeChangeEvent; import net.minestom.server.event.player.PlayerInputEvent; +import net.minestom.server.instance.Chunk; +import net.minestom.server.instance.ChunkLoader; +import net.minestom.server.instance.Instance; import net.minestom.server.listener.PlayerInputListener; import net.minestom.server.message.ChatMessageType; import net.minestom.server.network.packet.client.common.ClientSettingsPacket; @@ -41,6 +44,7 @@ import net.minestom.testing.Collector; import net.minestom.testing.Env; import net.minestom.testing.EnvTest; +import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -49,6 +53,8 @@ import java.util.EnumSet; import java.util.List; import java.util.Locale; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; import java.util.function.Predicate; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -386,6 +392,43 @@ public void lookAt(Env env) { assertEquals(startingPlayerPos, player.getPosition()); } + @Test + public void laterSetInstanceCancelsSpawnWaitingForChunks(Env env) { + final Player player = env.createPlayer(env.createFlatInstance(), new Pos(0, 40, 0)); + final CountDownLatch release = new CountDownLatch(1); + final Instance slow = env.process().instanceManager().createInstanceContainer(new ChunkLoader() { + @Override + public @Nullable Chunk loadChunk(Instance instance, int chunkX, int chunkZ) { + try { + release.await(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return null; + } + + @Override + public void saveChunk(Chunk chunk) { + } + + @Override + public boolean supportsParallelLoading() { + return true; + } + }); + final Instance target = env.createFlatInstance(); + + final CompletableFuture waiting = player.setInstance(slow, new Pos(0, 40, 0)); + final CompletableFuture moved = player.setInstance(target, new Pos(0, 40, 0)); + assertTrue(env.tickWhile(() -> !moved.isDone(), Duration.ofSeconds(5))); + moved.join(); + release.countDown(); + assertTrue(env.tickWhile(() -> !waiting.isDone(), Duration.ofSeconds(5))); + + assertTrue(waiting.isCancelled()); + assertEquals(target, player.getInstance()); + } + @Test public void removePlayer(Env env) { var instance = env.createFlatInstance(); diff --git a/src/test/java/net/minestom/server/network/ProcessConnectionTest.java b/src/test/java/net/minestom/server/network/ProcessConnectionTest.java index 092fbc23109..31d2e996e25 100644 --- a/src/test/java/net/minestom/server/network/ProcessConnectionTest.java +++ b/src/test/java/net/minestom/server/network/ProcessConnectionTest.java @@ -147,6 +147,37 @@ private static Throwable onlySuppressed(Throwable failure) { return failure.getSuppressed()[0]; } + @Test + void connectionFactoryChoosesRejectsAndCannotAdoptForeignConnections() throws Exception { + try (var pair = new ServerProcessPair()) { + var process = pair.first(); + var server = process.server(); + var created = new AtomicInteger(); + server.setConnectionFactory((channel, remoteAddress, readThread, writeThread) -> { + created.incrementAndGet(); + return new PlayerSocketConnection(process, channel, remoteAddress, readThread, writeThread); + }); + process.start(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0)); + try (var client = new ProtocolClient(process)) { + client.handshake("localhost", ClientHandshakePacket.Intent.STATUS); + client.send(new ClientPingRequestPacket(123)); + assertEquals(new PingResponsePacket(123), client.read()); + assertEquals(1, created.get()); + } + + server.setConnectionFactory((_, _, _, _) -> null); + try (var client = new ProtocolClient(process)) { + assertThrows(EOFException.class, client::read); + } + + server.setConnectionFactory((channel, remoteAddress, readThread, writeThread) -> + new PlayerSocketConnection(pair.second(), channel, remoteAddress, readThread, writeThread)); + try (var client = new ProtocolClient(process)) { + assertThrows(EOFException.class, client::read); + } + } + } + @Test void closingAProcessClosesSocketsWithoutPlayersAndLeavesOtherProcessesListening() throws Exception { try (var pair = new ServerProcessPair()) { diff --git a/src/test/java/net/minestom/server/network/SendablePacketTest.java b/src/test/java/net/minestom/server/network/SendablePacketTest.java index e70253a8ab4..cd936f0ec45 100644 --- a/src/test/java/net/minestom/server/network/SendablePacketTest.java +++ b/src/test/java/net/minestom/server/network/SendablePacketTest.java @@ -46,6 +46,52 @@ public void cached() { } } + @Test + public void cachedBodyReusesCachedPacket() { + try (var pool = new PacketBufferPool(Registries.vanilla())) { + var context = pool.context(ConnectionState.PLAY, 256); + var packet = new SystemChatPacket(Component.text("Hello World!"), false); + var calls = new AtomicInteger(); + var cached = new CachedPacket(() -> { + calls.incrementAndGet(); + return packet; + }); + assertFalse(cached.isValid()); + + // May fail in the very unlikely case where soft references are cleared + assertSame(packet, cached.packet(context)); + assertSame(packet, cached.packet(context)); + assertEquals(1, calls.get()); + assertTrue(cached.isValid()); + + var buffer = context.frame(packet); + var cachedBuffer = cached.body(context); + assertTrue(NetworkBuffer.equals(buffer, cachedBuffer)); + assertSame(cachedBuffer, cached.body(context)); + assertSame(packet, cached.packet(context)); + assertEquals(1, calls.get()); + + cached.invalidate(); + assertFalse(cached.isValid()); + assertTrue(NetworkBuffer.equals(buffer, cached.body(context))); + assertSame(packet, cached.packet(context)); + assertEquals(2, calls.get()); + } + } + + @Test + public void cachedPacketIsNotFramed() { + try (var pool = new PacketBufferPool(Registries.vanilla())) { + // System chat has no packet ID in the login state, so framing it there fails + var packet = new SystemChatPacket(Component.text("Hello World!"), false); + var cached = new CachedPacket(packet); + assertSame(packet, cached.packet(pool.context(ConnectionState.LOGIN, 256))); + assertTrue(cached.isValid()); + var context = pool.context(ConnectionState.PLAY, 256); + assertTrue(NetworkBuffer.equals(context.frame(packet), cached.body(context))); + } + } + @Test @Timeout(10) void concurrentContextsSerializeTheSupplier() throws Exception { @@ -70,12 +116,12 @@ void concurrentContextsSerializeTheSupplier() throws Exception { var second = executor.submit(() -> readConcurrentCache(cached, secondContext, barrier)); first.get(5, TimeUnit.SECONDS); second.get(5, TimeUnit.SECONDS); - assertTrue(calls.get() >= 2); - int previousCalls = calls.get(); + // May fail in the very unlikely case where soft references are cleared + assertEquals(1, calls.get()); cached.invalidate(); assertFalse(cached.isValid()); assertSame(packet, cached.packet(firstContext)); - assertEquals(previousCalls + 1, calls.get()); + assertEquals(2, calls.get()); } }