From ad0f02cfa9805e9df0e0aa78fd25f37f564b9b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Wed, 18 Nov 2020 00:50:27 +0100 Subject: [PATCH 01/39] Add checkstyle gradle plugin --- build.gradle | 7 +- minestom_checks.xml | 303 ++++++++++++++++++ .../net/minestom/server/entity/Player.java | 5 +- 3 files changed, 310 insertions(+), 5 deletions(-) create mode 100644 minestom_checks.xml diff --git a/build.gradle b/build.gradle index ab464fc91..ab70c7eb5 100644 --- a/build.gradle +++ b/build.gradle @@ -6,6 +6,7 @@ plugins { id 'maven-publish' id 'net.ltgt.apt' version '0.10' id 'org.jetbrains.kotlin.jvm' version '1.4.10' + id 'checkstyle' } group 'net.minestom.server' @@ -36,7 +37,7 @@ allprojects { maven { url 'https://jitpack.io' } maven { name 'sponge' - url 'http://repo.spongepowered.org/maven' + url 'https://repo.spongepowered.org/maven' } } javadoc { @@ -72,6 +73,10 @@ sourceSets { } } +tasks.withType(Checkstyle) { + configFile file("${projectDir}/minestom_checks.xml") +} + java { // Minestom uses LWJGL libs as optional dependency if interfacing with a GPU is asked registerFeature("lwjgl") { diff --git a/minestom_checks.xml b/minestom_checks.xml new file mode 100644 index 000000000..142f6229b --- /dev/null +++ b/minestom_checks.xml @@ -0,0 +1,303 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 31764aa27..682aa9508 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -70,7 +70,6 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; - /** * Those are the major actors of the server, * they are not necessary backed by a {@link NettyPlayerConnection} as shown by {@link FakePlayer} @@ -119,7 +118,6 @@ public class Player extends LivingEntity implements CommandSender { private long defaultEatingTime = 1000L; private long eatingTime; private boolean isEating; - // Game state (https://wiki.vg/Protocol#Change_Game_State) private boolean enableRespawnScreen; @@ -1119,8 +1117,7 @@ public class Player extends LivingEntity implements CommandSender { /** * Gets the player display name in the tab-list. * - * @return the player display name, - * null means that {@link #getUsername()} is displayed + * @return the player display name, null means that {@link #getUsername()} is displayed */ @Nullable public ColoredText getDisplayName() { From d968946c4495c12e7a062b4fb4cc02e05b20064e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Wed, 18 Nov 2020 00:52:33 +0100 Subject: [PATCH 02/39] Restore deleted empty lines --- src/main/java/net/minestom/server/entity/Player.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 682aa9508..ef8f3365e 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -70,6 +70,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; + /** * Those are the major actors of the server, * they are not necessary backed by a {@link NettyPlayerConnection} as shown by {@link FakePlayer} @@ -118,6 +119,7 @@ public class Player extends LivingEntity implements CommandSender { private long defaultEatingTime = 1000L; private long eatingTime; private boolean isEating; + // Game state (https://wiki.vg/Protocol#Change_Game_State) private boolean enableRespawnScreen; From b5793270d6572a0249c86d8e9053b5dad12834fd Mon Sep 17 00:00:00 2001 From: JesFot Date: Wed, 18 Nov 2020 01:05:15 +0100 Subject: [PATCH 03/39] Add Github action for checking pr code style --- .github/workflows/check-pr-style.yml | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/check-pr-style.yml diff --git a/.github/workflows/check-pr-style.yml b/.github/workflows/check-pr-style.yml new file mode 100644 index 000000000..2f1e35259 --- /dev/null +++ b/.github/workflows/check-pr-style.yml @@ -0,0 +1,30 @@ +name: Check PR code style + +on: + pull_request: + branches: [ master ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Set up JDK 11 + uses: actions/setup-java@v1 + with: + java-version: 1.11 + - name: Run java checkstyle + uses: nikitasavinov/checkstyle-action@0.3.1 + with: + # Report level for reviewdog [info,warning,error] + level: info + # Reporter of reviewdog command [github-pr-check,github-pr-review] + reporter: github-pr-check + # Filtering for the reviewdog command [added,diff_context,file,nofilter]. + filter_mode: added + # Exit code for reviewdog when errors are found [true,false]. + fail_on_error: false + # Checkstyle config file + checkstyle_config: minestom_checks.xml From 980178b02581d8f7c109b681adf9bd008add379f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Wed, 18 Nov 2020 02:52:36 +0100 Subject: [PATCH 04/39] Remove style check for inline '

' --- minestom_checks.xml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/minestom_checks.xml b/minestom_checks.xml index 142f6229b..674c3996f 100644 --- a/minestom_checks.xml +++ b/minestom_checks.xml @@ -268,7 +268,9 @@ - + + + Date: Thu, 19 Nov 2020 04:54:54 +0100 Subject: [PATCH 05/39] Reduced compression level to 3 Signed-off-by: TheMode --- .../minestom/server/network/netty/codec/PacketCompressor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java b/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java index 4e9c8ed8c..eae4f8313 100644 --- a/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java +++ b/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java @@ -33,7 +33,7 @@ public class PacketCompressor extends ByteToMessageCodec { private final byte[] buffer = new byte[8192]; - private final Deflater deflater = new Deflater(); + private final Deflater deflater = new Deflater(3); private final Inflater inflater = new Inflater(); public PacketCompressor(int threshold) { From 828069c6850b301398502a4766da949da7e7a304 Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 07:00:41 +0100 Subject: [PATCH 06/39] Cleanup + reduced default chunk view distance to 8 --- .../java/net/minestom/server/MinecraftServer.java | 2 +- src/main/java/net/minestom/server/entity/Player.java | 12 ++---------- .../listener/manager/PacketListenerManager.java | 8 +++++++- .../minestom/server/network/ConnectionManager.java | 6 +++--- .../server/network/player/NettyPlayerConnection.java | 8 ++++++++ .../server/network/player/PlayerConnection.java | 2 +- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index 7fa3a8946..4c616ee21 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -124,7 +124,7 @@ public final class MinecraftServer { private static boolean initialized; private static boolean started; - private static int chunkViewDistance = 10; + private static int chunkViewDistance = 8; private static int entityViewDistance = 5; private static int compressionThreshold = 256; private static ResponseDataConsumer responseDataConsumer; diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 0ef819fe4..be1b7f9cf 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -1,6 +1,5 @@ package net.minestom.server.entity; -import io.netty.channel.Channel; import net.minestom.server.MinecraftServer; import net.minestom.server.advancements.AdvancementTab; import net.minestom.server.attribute.Attribute; @@ -57,7 +56,6 @@ import net.minestom.server.utils.callback.OptionalCallback; import net.minestom.server.utils.chunk.ChunkCallback; import net.minestom.server.utils.chunk.ChunkUtils; import net.minestom.server.utils.instance.InstanceUtils; -import net.minestom.server.utils.player.PlayerUtils; import net.minestom.server.utils.time.CooldownUtils; import net.minestom.server.utils.time.TimeUnit; import net.minestom.server.utils.time.UpdateOption; @@ -342,14 +340,8 @@ public class Player extends LivingEntity implements CommandSender { @Override public void update(long time) { - // Flush all pending packets - if (PlayerUtils.isNettyClient(this)) { - Channel channel = ((NettyPlayerConnection) playerConnection).getChannel(); - channel.eventLoop().execute(() -> channel.flush()); - } - - // Network tick verification - playerConnection.updateStats(); + // Network tick + this.playerConnection.update(); // Process received packets ClientPlayPacket packet; diff --git a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java index f153afd0c..15341a562 100644 --- a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java +++ b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java @@ -11,6 +11,7 @@ import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -93,8 +94,13 @@ public final class PacketListenerManager { * @return true if the packet is not cancelled, false otherwise */ public boolean processServerPacket(@NotNull T packet, @NotNull Player player) { + final List> consumers = CONNECTION_MANAGER.getSendPacketConsumers(); + if (consumers.isEmpty()) { + return true; + } + final PacketController packetController = new PacketController(); - for (PacketConsumer packetConsumer : CONNECTION_MANAGER.getSendPacketConsumers()) { + for (PacketConsumer packetConsumer : consumers) { packetConsumer.accept(player, packetController, packet); } diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index 4043048b9..a4a5c8753 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -145,11 +145,11 @@ public final class ConnectionManager { /** * Gets all the listeners which are called for each packet received. * - * @return an unmodifiable list of packet's consumers + * @return a list of packet's consumers */ @NotNull public List> getReceivePacketConsumers() { - return Collections.unmodifiableList(receivePacketConsumers); + return receivePacketConsumers; } /** @@ -164,7 +164,7 @@ public final class ConnectionManager { /** * Gets all the listeners which are called for each packet sent. * - * @return an unmodifiable list of packet's consumers + * @return a list of packet's consumers */ @NotNull public List> getSendPacketConsumers() { diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index 9d369635a..fdb21f9ac 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -56,6 +56,14 @@ public class NettyPlayerConnection extends PlayerConnection { this.remoteAddress = channel.remoteAddress(); } + @Override + public void update() { + // Flush + this.channel.eventLoop().execute(() -> channel.flush()); + // Network stats + super.update(); + } + /** * Sets the encryption key and add the codecs to the pipeline. * diff --git a/src/main/java/net/minestom/server/network/player/PlayerConnection.java b/src/main/java/net/minestom/server/network/player/PlayerConnection.java index 6f8c2a705..32200fe8b 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerConnection.java @@ -45,7 +45,7 @@ public abstract class PlayerConnection { /** * Updates values related to the network connection. */ - public void updateStats() { + public void update() { // Check rate limit if (MinecraftServer.getRateLimit() > 0) { tickCounter++; From 69a268aa9dbba83ef76fa73eec9b83cba52620b4 Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 08:05:08 +0100 Subject: [PATCH 07/39] Fixed first player teleportation when the respawn point is not 0 0 0 --- src/main/java/net/minestom/server/entity/Player.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index be1b7f9cf..819d68e55 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -703,7 +703,7 @@ public class Player extends LivingEntity implements CommandSender { sendDimension(instanceDimensionType); } - final long[] visibleChunks = ChunkUtils.getChunksInRange(position, getChunkRange()); + final long[] visibleChunks = ChunkUtils.getChunksInRange(firstSpawn ? getRespawnPoint() : position, getChunkRange()); final int length = visibleChunks.length; AtomicInteger counter = new AtomicInteger(0); @@ -747,11 +747,11 @@ public class Player extends LivingEntity implements CommandSender { */ private void spawnPlayer(Instance instance, boolean firstSpawn) { this.viewableEntities.forEach(entity -> entity.removeViewer(this)); - super.setInstance(instance); if (firstSpawn) { - teleport(getRespawnPoint()); + this.position = getRespawnPoint(); } + super.setInstance(instance); PlayerSpawnEvent spawnEvent = new PlayerSpawnEvent(this, instance, firstSpawn); callEvent(PlayerSpawnEvent.class, spawnEvent); From af730fb73c328eef0d130aec0e4bbdb6df281592 Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 19 Nov 2020 08:11:09 +0100 Subject: [PATCH 08/39] Decrease default synchronization time group to 50 Signed-off-by: TheMode --- src/main/java/net/minestom/server/MinecraftServer.java | 2 +- src/main/java/net/minestom/server/entity/Player.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index 4c616ee21..91f37f2c0 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -77,7 +77,7 @@ public final class MinecraftServer { public static final String THREAD_NAME_TICK = "Ms-Tick"; public static final String THREAD_NAME_BLOCK_BATCH = "Ms-BlockBatchPool"; - public static final int THREAD_COUNT_BLOCK_BATCH = 2; + public static final int THREAD_COUNT_BLOCK_BATCH = 4; public static final String THREAD_NAME_SCHEDULER = "Ms-SchedulerPool"; public static final int THREAD_COUNT_SCHEDULER = 1; diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 819d68e55..b26b531e6 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -81,7 +81,7 @@ public class Player extends LivingEntity implements CommandSender { /** * @see #getPlayerSynchronizationGroup() */ - private static volatile int playerSynchronizationGroup = 100; + private static volatile int playerSynchronizationGroup = 50; /** * For the number of viewers that a player has, the position synchronization packet will be sent From c1e623eddce74ab0f16e4a19cc6c977d9b6b64b1 Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 08:19:16 +0100 Subject: [PATCH 09/39] Fix unmappable character --- src/main/java/net/minestom/server/map/MapColors.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/minestom/server/map/MapColors.java b/src/main/java/net/minestom/server/map/MapColors.java index f158d73fd..4871bde02 100644 --- a/src/main/java/net/minestom/server/map/MapColors.java +++ b/src/main/java/net/minestom/server/map/MapColors.java @@ -123,10 +123,10 @@ public enum MapColors { // From the wiki: https://minecraft.gamepedia.com/Map_item_format // Map Color ID Multiply R,G,B By = Multiplier - //Base Color ID×4 + 0 180 0.71 - //Base Color ID×4 + 1 220 0.86 - //Base Color ID×4 + 2 255 (same color) 1 - //Base Color ID×4 + 3 135 0.53 + //Base Color ID*4 + 0 180 0.71 + //Base Color ID*4 + 1 220 0.86 + //Base Color ID*4 + 2 255 (same color) 1 + //Base Color ID*4 + 3 135 0.53 /** * Returns the color index with RGB multiplied by 0.53, to use on a map From 099a8bafdca4891e00215cb689220088a55b44fa Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 08:29:34 +0100 Subject: [PATCH 10/39] Increase socket send buffer to 1MB --- .../java/net/minestom/server/network/netty/NettyServer.java | 1 + .../minestom/server/network/player/NettyPlayerConnection.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index 35044129d..e1ffef012 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -65,6 +65,7 @@ public class NettyServer { protected void initChannel(@NotNull SocketChannel ch) { ChannelConfig config = ch.config(); config.setOption(ChannelOption.TCP_NODELAY, true); + config.setOption(ChannelOption.SO_SNDBUF, 1_000_000); ChannelPipeline pipeline = ch.pipeline(); diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index fdb21f9ac..c5f3c5922 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -101,9 +101,9 @@ public class NettyPlayerConnection extends PlayerConnection { public void sendPacket(@NotNull ServerPacket serverPacket) { if (shouldSendPacket(serverPacket)) { if (getPlayer() != null) { - channel.write(serverPacket); // Flush on player update + this.channel.write(serverPacket); // Flush on player update } else { - channel.writeAndFlush(serverPacket); + this.channel.writeAndFlush(serverPacket); } } } From 533526d3a5f293c022fe88a57caa1943d1e5e9d7 Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 08:48:33 +0100 Subject: [PATCH 11/39] MainDemo does not require a default write and read speed --- src/test/java/demo/MainDemo.java | 9 --------- src/test/java/demo/commands/TestCommand.java | 4 +++- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/test/java/demo/MainDemo.java b/src/test/java/demo/MainDemo.java index f71101ece..6c0fcd6d6 100644 --- a/src/test/java/demo/MainDemo.java +++ b/src/test/java/demo/MainDemo.java @@ -6,7 +6,6 @@ import net.minestom.server.instance.*; import net.minestom.server.instance.batch.ChunkBatch; import net.minestom.server.instance.block.Block; import net.minestom.server.network.ConnectionManager; -import net.minestom.server.network.netty.NettyServer; import net.minestom.server.utils.Position; import net.minestom.server.world.biomes.Biome; @@ -37,14 +36,6 @@ public class MainDemo { }); }); - // OPTIONAL: optimize networking to prevent having unresponsive threads - { - NettyServer nettyServer = MinecraftServer.getNettyServer(); - // Set the maximum bandwidth out and in to 500KB/s, largely enough for a single client - nettyServer.setWriteLimit(500_000); - nettyServer.setReadLimit(500_000); - } - // Start the server minecraftServer.start("localhost", 25565); } diff --git a/src/test/java/demo/commands/TestCommand.java b/src/test/java/demo/commands/TestCommand.java index 87bb21c39..e58c80e82 100644 --- a/src/test/java/demo/commands/TestCommand.java +++ b/src/test/java/demo/commands/TestCommand.java @@ -5,6 +5,7 @@ import net.minestom.server.command.builder.Arguments; import net.minestom.server.command.builder.Command; import net.minestom.server.command.builder.arguments.Argument; import net.minestom.server.command.builder.arguments.ArgumentType; +import net.minestom.server.entity.Player; public class TestCommand extends Command { @@ -19,11 +20,12 @@ public class TestCommand extends Command { }); setDefaultExecutor((source, args) -> { - System.out.println("DEFAULT"); System.gc(); + source.sendMessage("Explicit GC executed!"); }); addSyntax((source, args) -> { + Player player = (Player) source; System.out.println("ARG 1"); }, test); } From c3c27e8060b6fa3e42ba6628f6cccaa6a16e9ca4 Mon Sep 17 00:00:00 2001 From: JesFot Date: Thu, 19 Nov 2020 11:40:19 +0100 Subject: [PATCH 12/39] Add cache to gradle --- .github/workflows/tests.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6d233db27..e2fc9a4db 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,6 +19,16 @@ jobs: java-version: 1.11 - name: Grant execute permission for gradlew run: chmod +x gradlew + - name: Setup gradle cache + uses: burrunan/gradle-cache-action@v1 + with: + save-generated-gradle-jars: false + save-local-build-cache: false + save-gradle-dependencies-cache: true + save-maven-dependencies-cache: true + # Ignore some of the paths when caching Maven Local repository + maven-local-ignore-paths: | + net/minestom/ - name: Build Minestom run: ./gradlew build - name: Run tests From fc4e8aadf46ea8bf9ee2b21cc9be0e4cf4cb835c Mon Sep 17 00:00:00 2001 From: JesFot Date: Thu, 19 Nov 2020 11:46:18 +0100 Subject: [PATCH 13/39] Add SHA256 checksum for Gradle wrapper jar --- gradle/wrapper/gradle-wrapper.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index a8e4b9ef2..35c7a66ae 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -4,3 +4,4 @@ distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists distributionUrl=https\://services.gradle.org/distributions/gradle-6.3-all.zip +distributionSha256Sum=0f316a67b971b7b571dac7215dcf2591a30994b3450e0629925ffcfe2c68cc5c From ed7df3fc11f0ca40eeb30ddb5a40995fc26bdce0 Mon Sep 17 00:00:00 2001 From: JesFot Date: Thu, 19 Nov 2020 11:50:57 +0100 Subject: [PATCH 14/39] Limit workflow for not triggering tests twice --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e2fc9a4db..7080322d7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -30,6 +30,6 @@ jobs: maven-local-ignore-paths: | net/minestom/ - name: Build Minestom - run: ./gradlew build + run: ./gradlew assemble - name: Run tests run: ./gradlew test From 25cd0727a6aa046881a6df1297d81fdae785899c Mon Sep 17 00:00:00 2001 From: JesFot Date: Thu, 19 Nov 2020 13:12:51 +0100 Subject: [PATCH 15/39] Do not trigger javadoc when running tests --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7080322d7..a3cf9d6c7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -30,6 +30,6 @@ jobs: maven-local-ignore-paths: | net/minestom/ - name: Build Minestom - run: ./gradlew assemble + run: ./gradlew classes testClasses - name: Run tests - run: ./gradlew test + run: ./gradlew check From 018786463dfa7d793efc14d184c74df89c1298d3 Mon Sep 17 00:00:00 2001 From: themode Date: Thu, 19 Nov 2020 15:37:12 +0100 Subject: [PATCH 16/39] Replaced the channel traffic handler to GlobalChannelTrafficShapingHandler --- .../server/network/netty/NettyServer.java | 99 +++++++++---------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index e1ffef012..4008fab3b 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -12,7 +12,8 @@ import io.netty.channel.nio.NioEventLoopGroup; import io.netty.channel.socket.ServerSocketChannel; import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; -import io.netty.handler.traffic.ChannelTrafficShapingHandler; +import io.netty.handler.traffic.GlobalChannelTrafficShapingHandler; +import io.netty.handler.traffic.TrafficCounter; import net.minestom.server.network.PacketProcessor; import net.minestom.server.network.netty.channel.ClientChannel; import net.minestom.server.network.netty.codec.LegacyPingHandler; @@ -22,9 +23,14 @@ import net.minestom.server.network.netty.codec.PacketFramer; import org.jetbrains.annotations.NotNull; import java.net.InetSocketAddress; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; public class NettyServer { + private static final long DEFAULT_CHANNEL_WRITE_LIMIT = 600_000L; + private static final long DEFAULT_CHANNEL_READ_LIMIT = 100_000L; + private final EventLoopGroup boss, worker; private final ServerBootstrap bootstrap; @@ -33,9 +39,12 @@ public class NettyServer { private String address; private int port; - // Options - private long writeLimit = 750_000L; - private long readLimit = 750_000L; + private final GlobalChannelTrafficShapingHandler globalTrafficHandler; + + /** + * Scheduler used by {@code globalTrafficHandler}. + */ + private final ScheduledExecutorService trafficScheduler = Executors.newScheduledThreadPool(1); public NettyServer(@NotNull PacketProcessor packetProcessor) { Class channel; @@ -61,6 +70,18 @@ public class NettyServer { .group(boss, worker) .channel(channel); + this.globalTrafficHandler = new GlobalChannelTrafficShapingHandler(trafficScheduler, 200) { + @Override + protected void doAccounting(TrafficCounter counter) { + // TODO proper monitoring API + //System.out.println("data " + counter.lastWriteThroughput() / 1000 + " " + counter.lastReadThroughput() / 1000); + } + }; + + globalTrafficHandler.setWriteChannelLimit(DEFAULT_CHANNEL_WRITE_LIMIT); + globalTrafficHandler.setReadChannelLimit(DEFAULT_CHANNEL_READ_LIMIT); + + bootstrap.childHandler(new ChannelInitializer() { protected void initChannel(@NotNull SocketChannel ch) { ChannelConfig config = ch.config(); @@ -69,10 +90,7 @@ public class NettyServer { ChannelPipeline pipeline = ch.pipeline(); - ChannelTrafficShapingHandler channelTrafficShapingHandler = - new ChannelTrafficShapingHandler(writeLimit, readLimit, 200); - - pipeline.addLast("traffic-limiter", channelTrafficShapingHandler); + pipeline.addLast("traffic-limiter", globalTrafficHandler); // First check should verify if the packet is a legacy ping (from 1.6 version and earlier) // Removed from the pipeline later in LegacyPingHandler if unnecessary (>1.6) @@ -92,7 +110,13 @@ public class NettyServer { }); } - public void start(String address, int port) { + /** + * Binds the address to start the server. + * + * @param address the server address + * @param port the server port + */ + public void start(@NotNull String address, int port) { this.address = address; this.port = port; @@ -128,58 +152,27 @@ public class NettyServer { } /** - * Gets the server write limit. + * Gets the traffic handler, used to control channel and global bandwidth. *

- * Used when you want to limit the bandwidth used by a single connection. - * Can also prevent the networking threads from being unresponsive. + * The object can be modified as specified by Netty documentation. * - * @return the write limit in bytes + * @return the global traffic handler */ - public long getWriteLimit() { - return writeLimit; + @NotNull + public GlobalChannelTrafficShapingHandler getGlobalTrafficHandler() { + return globalTrafficHandler; } /** - * Changes the server write limit - *

- * WARNING: the change will only apply to new connections, the current ones will not be updated. - * - * @param writeLimit the new write limit in bytes, 0 to disable - * @see #getWriteLimit() + * Stops the server and the various services. */ - public void setWriteLimit(long writeLimit) { - this.writeLimit = writeLimit; - } - - - /** - * Gets the server read limit. - *

- * Used when you want to limit the bandwidth used by a single connection. - * Can also prevent the networking threads from being unresponsive. - * - * @return the read limit in bytes - */ - public long getReadLimit() { - return readLimit; - } - - /** - * Changes the server read limit - *

- * WARNING: the change will only apply to new connections, the current ones will not be updated. - * - * @param readLimit the new read limit in bytes, 0 to disable - * @see #getWriteLimit() - */ - public void setReadLimit(long readLimit) { - this.readLimit = readLimit; - } - public void stop() { - serverChannel.close(); + this.serverChannel.close(); - worker.shutdownGracefully(); - boss.shutdownGracefully(); + this.worker.shutdownGracefully(); + this.boss.shutdownGracefully(); + + this.trafficScheduler.shutdown(); + this.globalTrafficHandler.release(); } } From c1993f67834e6fc48cdd4a5fe9367a7c44da55df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Thu, 19 Nov 2020 15:48:11 +0100 Subject: [PATCH 17/39] Use compatible config file until actions is updated --- .github/workflows/check-pr-style.yml | 2 +- minestom_checks_8.36.xml | 303 +++++++++++++++++++++++++++ 2 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 minestom_checks_8.36.xml diff --git a/.github/workflows/check-pr-style.yml b/.github/workflows/check-pr-style.yml index 2f1e35259..ed1dcbae0 100644 --- a/.github/workflows/check-pr-style.yml +++ b/.github/workflows/check-pr-style.yml @@ -27,4 +27,4 @@ jobs: # Exit code for reviewdog when errors are found [true,false]. fail_on_error: false # Checkstyle config file - checkstyle_config: minestom_checks.xml + checkstyle_config: minestom_checks_8.36.xml diff --git a/minestom_checks_8.36.xml b/minestom_checks_8.36.xml new file mode 100644 index 000000000..eb513ae72 --- /dev/null +++ b/minestom_checks_8.36.xml @@ -0,0 +1,303 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 4b07deae27dc321fd52f436e072441a30255555f Mon Sep 17 00:00:00 2001 From: JesFot Date: Thu, 19 Nov 2020 16:47:16 +0100 Subject: [PATCH 18/39] Use last commit of checkstyle-action --- .github/workflows/check-pr-style.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-pr-style.yml b/.github/workflows/check-pr-style.yml index ed1dcbae0..f13f760b5 100644 --- a/.github/workflows/check-pr-style.yml +++ b/.github/workflows/check-pr-style.yml @@ -16,7 +16,7 @@ jobs: with: java-version: 1.11 - name: Run java checkstyle - uses: nikitasavinov/checkstyle-action@0.3.1 + uses: nikitasavinov/checkstyle-action@61104b8abd3ea7fb471f9b991e5ed71c20a4c9f7 with: # Report level for reviewdog [info,warning,error] level: info From d138acceb1c8dfdfd6361a84841899da40314dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Fri, 20 Nov 2020 01:30:01 +0100 Subject: [PATCH 19/39] Fix gradle checktyle version --- build.gradle | 9 +- minestom_checks.xml | 2 - minestom_checks_8.36.xml | 303 --------------------------------------- 3 files changed, 5 insertions(+), 309 deletions(-) delete mode 100644 minestom_checks_8.36.xml diff --git a/build.gradle b/build.gradle index ab70c7eb5..f3c77a35c 100644 --- a/build.gradle +++ b/build.gradle @@ -46,6 +46,11 @@ allprojects { addBooleanOption('html5', true) } } + + checkstyle { + toolVersion "8.37" + configFile file("${projectDir}/minestom_checks.xml") + } } sourceSets { @@ -73,10 +78,6 @@ sourceSets { } } -tasks.withType(Checkstyle) { - configFile file("${projectDir}/minestom_checks.xml") -} - java { // Minestom uses LWJGL libs as optional dependency if interfacing with a GPU is asked registerFeature("lwjgl") { diff --git a/minestom_checks.xml b/minestom_checks.xml index 674c3996f..eb513ae72 100644 --- a/minestom_checks.xml +++ b/minestom_checks.xml @@ -279,10 +279,8 @@ - - diff --git a/minestom_checks_8.36.xml b/minestom_checks_8.36.xml deleted file mode 100644 index eb513ae72..000000000 --- a/minestom_checks_8.36.xml +++ /dev/null @@ -1,303 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 5fdd62fb58462ee3e24301d7097d10fe65b9860d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=ABsFot?= Date: Fri, 20 Nov 2020 01:33:43 +0100 Subject: [PATCH 20/39] Use last version of checkstyle in github action --- .github/workflows/check-pr-style.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-pr-style.yml b/.github/workflows/check-pr-style.yml index f13f760b5..7ac69d24b 100644 --- a/.github/workflows/check-pr-style.yml +++ b/.github/workflows/check-pr-style.yml @@ -16,7 +16,7 @@ jobs: with: java-version: 1.11 - name: Run java checkstyle - uses: nikitasavinov/checkstyle-action@61104b8abd3ea7fb471f9b991e5ed71c20a4c9f7 + uses: nikitasavinov/checkstyle-action@d87d526a914fc5cb0b003908e35038dbb2d6e1b7 with: # Report level for reviewdog [info,warning,error] level: info @@ -27,4 +27,5 @@ jobs: # Exit code for reviewdog when errors are found [true,false]. fail_on_error: false # Checkstyle config file - checkstyle_config: minestom_checks_8.36.xml + checkstyle_config: minestom_checks.xml + checkstyle_version: "8.37" From 66f038113fbf354f132e83f1c135afb8d364c8b5 Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Thu, 19 Nov 2020 19:53:22 -0500 Subject: [PATCH 21/39] Added EntityLlama --- .../server/entity/type/animal/EntityLlama.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/main/java/net/minestom/server/entity/type/animal/EntityLlama.java diff --git a/src/main/java/net/minestom/server/entity/type/animal/EntityLlama.java b/src/main/java/net/minestom/server/entity/type/animal/EntityLlama.java new file mode 100644 index 000000000..9f3891d5d --- /dev/null +++ b/src/main/java/net/minestom/server/entity/type/animal/EntityLlama.java @@ -0,0 +1,13 @@ +package net.minestom.server.entity.type.animal; + +import net.minestom.server.entity.EntityCreature; +import net.minestom.server.entity.EntityType; +import net.minestom.server.entity.type.Animal; +import net.minestom.server.utils.Position; + +public class EntityLlama extends EntityCreature implements Animal { + public EntityLlama(Position spawnPosition) { + super(EntityType.LLAMA, spawnPosition); + setBoundingBox(0.45f, 0.9375f, 0.45f); + } +} From 8accc82e2b55628e4b8c2814afbd343b559bdbf0 Mon Sep 17 00:00:00 2001 From: JesFot Date: Fri, 20 Nov 2020 02:11:22 +0100 Subject: [PATCH 22/39] Restore gradle test command --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6d233db27..12fd62a60 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -21,5 +21,5 @@ jobs: run: chmod +x gradlew - name: Build Minestom run: ./gradlew build - - name: Run tests + - name: Run Minestom tests run: ./gradlew test From c3b693d7b9e23ae45898c888e6e758fdf275ed62 Mon Sep 17 00:00:00 2001 From: JesFot Date: Fri, 20 Nov 2020 02:20:59 +0100 Subject: [PATCH 23/39] Only compile classes for tests --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ce338e019..c6d8bda7c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -30,6 +30,6 @@ jobs: maven-local-ignore-paths: | net/minestom/ - name: Build Minestom - run: ./gradlew build + run: ./gradlew classes testClasses - name: Run Minestom tests run: ./gradlew test From 0739b57dd1f6fd2088fa92458ab824942a8d9a38 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 03:47:29 +0100 Subject: [PATCH 24/39] Server packet listener now takes a collection of player, for future network optimization --- .../net/minestom/server/entity/Player.java | 1 - ...onsumer.java => ClientPacketConsumer.java} | 12 +++---- .../listener/manager/PacketController.java | 7 +++- .../manager/PacketListenerManager.java | 20 +++++------ .../manager/ServerPacketConsumer.java | 27 +++++++++++++++ .../server/network/ConnectionManager.java | 33 ++++++++----------- .../network/player/PlayerConnection.java | 8 +++-- 7 files changed, 67 insertions(+), 41 deletions(-) rename src/main/java/net/minestom/server/listener/manager/{PacketConsumer.java => ClientPacketConsumer.java} (57%) create mode 100644 src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index cceb40ecd..c03ca9e57 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -339,7 +339,6 @@ public class Player extends LivingEntity implements CommandSender { @Override public void update(long time) { - // Network tick this.playerConnection.update(); diff --git a/src/main/java/net/minestom/server/listener/manager/PacketConsumer.java b/src/main/java/net/minestom/server/listener/manager/ClientPacketConsumer.java similarity index 57% rename from src/main/java/net/minestom/server/listener/manager/PacketConsumer.java rename to src/main/java/net/minestom/server/listener/manager/ClientPacketConsumer.java index 689db0669..fc8110f39 100644 --- a/src/main/java/net/minestom/server/listener/manager/PacketConsumer.java +++ b/src/main/java/net/minestom/server/listener/manager/ClientPacketConsumer.java @@ -2,23 +2,21 @@ package net.minestom.server.listener.manager; import net.minestom.server.entity.Player; import net.minestom.server.network.ConnectionManager; +import net.minestom.server.network.packet.client.ClientPlayPacket; import org.jetbrains.annotations.NotNull; /** - * Interface used to add a listener for incoming/outgoing packets with - * {@link ConnectionManager#onPacketReceive(PacketConsumer)} and {@link ConnectionManager#onPacketSend(PacketConsumer)}. - * - * @param the packet type + * Interface used to add a listener for incoming packets with {@link ConnectionManager#onPacketReceive(ClientPacketConsumer)}. */ @FunctionalInterface -public interface PacketConsumer { +public interface ClientPacketConsumer { /** - * Called when a packet is received/sent from/to a client. + * Called when a packet is received from a client. * * @param player the player concerned by the packet * @param packetController the packet controller, can be used to cancel the packet * @param packet the packet */ - void accept(@NotNull Player player, @NotNull PacketController packetController, @NotNull T packet); + void accept(@NotNull Player player, @NotNull PacketController packetController, @NotNull ClientPlayPacket packet); } diff --git a/src/main/java/net/minestom/server/listener/manager/PacketController.java b/src/main/java/net/minestom/server/listener/manager/PacketController.java index 1716777f3..c628090c3 100644 --- a/src/main/java/net/minestom/server/listener/manager/PacketController.java +++ b/src/main/java/net/minestom/server/listener/manager/PacketController.java @@ -1,9 +1,14 @@ package net.minestom.server.listener.manager; import net.minestom.server.entity.Player; +import net.minestom.server.network.packet.client.ClientPlayPacket; +import net.minestom.server.network.packet.server.ServerPacket; + +import java.util.Collection; /** - * Used to control the output of a packet in {@link PacketConsumer#accept(Player, PacketController, Object)}. + * Used to control the output of a packet in {@link ClientPacketConsumer#accept(Player, PacketController, ClientPlayPacket)} + * and {@link ServerPacketConsumer#accept(Collection, PacketController, ServerPacket)}. */ public class PacketController { diff --git a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java index 15341a562..09594d762 100644 --- a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java +++ b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java @@ -11,6 +11,7 @@ import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -74,8 +75,8 @@ public final class PacketListenerManager { } final PacketController packetController = new PacketController(); - for (PacketConsumer packetConsumer : CONNECTION_MANAGER.getReceivePacketConsumers()) { - packetConsumer.accept(player, packetController, packet); + for (ClientPacketConsumer clientPacketConsumer : CONNECTION_MANAGER.getReceivePacketConsumers()) { + clientPacketConsumer.accept(player, packetController, packet); } if (packetController.isCancel()) @@ -86,22 +87,21 @@ public final class PacketListenerManager { } /** - * Executes the consumers from {@link ConnectionManager#onPacketSend(PacketConsumer)}. + * Executes the consumers from {@link ConnectionManager#onPacketSend(ServerPacketConsumer)}. * - * @param packet the packet to process - * @param player the player which should receive the packet - * @param the packet type + * @param packet the packet to process + * @param players the players which should receive the packet * @return true if the packet is not cancelled, false otherwise */ - public boolean processServerPacket(@NotNull T packet, @NotNull Player player) { - final List> consumers = CONNECTION_MANAGER.getSendPacketConsumers(); + public boolean processServerPacket(@NotNull ServerPacket packet, @NotNull Collection players) { + final List consumers = CONNECTION_MANAGER.getSendPacketConsumers(); if (consumers.isEmpty()) { return true; } final PacketController packetController = new PacketController(); - for (PacketConsumer packetConsumer : consumers) { - packetConsumer.accept(player, packetController, packet); + for (ServerPacketConsumer serverPacketConsumer : consumers) { + serverPacketConsumer.accept(players, packetController, packet); } return !packetController.isCancel(); diff --git a/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java b/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java new file mode 100644 index 000000000..56e4c3948 --- /dev/null +++ b/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java @@ -0,0 +1,27 @@ +package net.minestom.server.listener.manager; + +import net.minestom.server.entity.Player; +import net.minestom.server.network.ConnectionManager; +import net.minestom.server.network.packet.server.ServerPacket; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; + +/** + * Interface used to add a listener for outgoing packets with {@link ConnectionManager#onPacketSend(ServerPacket)}. + * + * @param the packet type + */ +@FunctionalInterface +public interface ServerPacketConsumer { + + /** + * Called when a packet is sent to a client. + * + * @param players the players who will receive the packet + * @param packetController the packet controller, can be used for cancelling + * @param packet the packet to send + */ + void accept(@NotNull Collection players, @NotNull PacketController packetController, @NotNull ServerPacket packet); + +} diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index a4a5c8753..027fcfe87 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -3,10 +3,9 @@ package net.minestom.server.network; import net.minestom.server.chat.JsonMessage; import net.minestom.server.entity.Player; import net.minestom.server.entity.fakeplayer.FakePlayer; -import net.minestom.server.listener.manager.PacketConsumer; -import net.minestom.server.network.packet.client.ClientPlayPacket; +import net.minestom.server.listener.manager.ClientPacketConsumer; +import net.minestom.server.listener.manager.ServerPacketConsumer; import net.minestom.server.network.packet.client.login.LoginStartPacket; -import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.packet.server.login.LoginSuccessPacket; import net.minestom.server.network.packet.server.play.ChatMessagePacket; import net.minestom.server.network.player.PlayerConnection; @@ -29,9 +28,9 @@ public final class ConnectionManager { private final Map connectionPlayerMap = Collections.synchronizedMap(new HashMap<>()); // All the consumers to call once a packet is received - private final List> receivePacketConsumers = new CopyOnWriteArrayList<>(); + private final List receiveClientPacketConsumers = new CopyOnWriteArrayList<>(); // All the consumers to call once a packet is sent - private final List> sendPacketConsumers = new CopyOnWriteArrayList<>(); + private final List sendClientPacketConsumers = new CopyOnWriteArrayList<>(); // The uuid provider once a player login private UuidProvider uuidProvider; // The player provider to have your own Player implementation @@ -148,17 +147,17 @@ public final class ConnectionManager { * @return a list of packet's consumers */ @NotNull - public List> getReceivePacketConsumers() { - return receivePacketConsumers; + public List getReceivePacketConsumers() { + return receiveClientPacketConsumers; } /** * Adds a consumer to call once a packet is received. * - * @param packetConsumer the packet consumer + * @param clientPacketConsumer the packet consumer */ - public void onPacketReceive(@NotNull PacketConsumer packetConsumer) { - this.receivePacketConsumers.add(packetConsumer); + public void onPacketReceive(@NotNull ClientPacketConsumer clientPacketConsumer) { + this.receiveClientPacketConsumers.add(clientPacketConsumer); } /** @@ -167,21 +166,17 @@ public final class ConnectionManager { * @return a list of packet's consumers */ @NotNull - public List> getSendPacketConsumers() { - return Collections.unmodifiableList(sendPacketConsumers); + public List getSendPacketConsumers() { + return Collections.unmodifiableList(sendClientPacketConsumers); } /** * Adds a consumer to call once a packet is sent. - *

- * Be aware that it is possible for the same packet instance to be used multiple time, - * changing the object fields could lead to issues. - * (consider canceling the packet instead and send your own) * - * @param packetConsumer the packet consumer + * @param serverPacketConsumer the packet consumer */ - public void onPacketSend(@NotNull PacketConsumer packetConsumer) { - this.sendPacketConsumers.add(packetConsumer); + public void onPacketSend(@NotNull ServerPacketConsumer serverPacketConsumer) { + this.sendClientPacketConsumers.add(serverPacketConsumer); } /** diff --git a/src/main/java/net/minestom/server/network/player/PlayerConnection.java b/src/main/java/net/minestom/server/network/player/PlayerConnection.java index 32200fe8b..f6d921811 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerConnection.java @@ -4,8 +4,8 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.chat.ChatColor; import net.minestom.server.chat.ColoredText; import net.minestom.server.entity.Player; -import net.minestom.server.listener.manager.PacketConsumer; import net.minestom.server.listener.manager.PacketListenerManager; +import net.minestom.server.listener.manager.ServerPacketConsumer; import net.minestom.server.network.ConnectionManager; import net.minestom.server.network.ConnectionState; import net.minestom.server.network.packet.server.ServerPacket; @@ -15,6 +15,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.net.SocketAddress; +import java.util.Collections; import java.util.concurrent.atomic.AtomicInteger; /** @@ -92,7 +93,7 @@ public abstract class PlayerConnection { /** * Serializes the packet and send it to the client. *

- * Also responsible for executing {@link ConnectionManager#onPacketSend(PacketConsumer)} consumers. + * Also responsible for executing {@link ConnectionManager#onPacketSend(ServerPacketConsumer)} consumers. * * @param serverPacket the packet to send * @see #shouldSendPacket(ServerPacket) @@ -100,7 +101,8 @@ public abstract class PlayerConnection { public abstract void sendPacket(@NotNull ServerPacket serverPacket); protected boolean shouldSendPacket(@NotNull ServerPacket serverPacket) { - return player == null || PACKET_LISTENER_MANAGER.processServerPacket(serverPacket, player); + return player == null || + PACKET_LISTENER_MANAGER.processServerPacket(serverPacket, Collections.singleton(player)); } /** From 2d7159f88858ff21c6ae0f03ff16e54d43649301 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 03:50:41 +0100 Subject: [PATCH 25/39] Fixed javadoc --- .../server/listener/manager/ServerPacketConsumer.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java b/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java index 56e4c3948..24ceae99e 100644 --- a/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java +++ b/src/main/java/net/minestom/server/listener/manager/ServerPacketConsumer.java @@ -8,9 +8,7 @@ import org.jetbrains.annotations.NotNull; import java.util.Collection; /** - * Interface used to add a listener for outgoing packets with {@link ConnectionManager#onPacketSend(ServerPacket)}. - * - * @param the packet type + * Interface used to add a listener for outgoing packets with {@link ConnectionManager#onPacketSend(ServerPacketConsumer)}. */ @FunctionalInterface public interface ServerPacketConsumer { From 153f7215b381e2687b3fbe8ad796391a4b56993c Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 03:57:05 +0100 Subject: [PATCH 26/39] Write packet only once when used with PacketUtils#sendGroupedPacket --- .../minestom/server/utils/PacketUtils.java | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 8ccf4e377..31fcbef75 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -2,8 +2,12 @@ package net.minestom.server.utils; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; +import net.minestom.server.listener.manager.PacketListenerManager; import net.minestom.server.network.packet.server.ServerPacket; +import net.minestom.server.network.player.NettyPlayerConnection; +import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.utils.binary.BinaryWriter; import org.jetbrains.annotations.NotNull; @@ -15,21 +19,32 @@ import java.util.Collection; */ public final class PacketUtils { + private static final PacketListenerManager PACKET_LISTENER_MANAGER = MinecraftServer.getPacketListenerManager(); + private PacketUtils() { } /** - * Sends a {@link ServerPacket} to multiple players. Mostly used for convenience. + * Sends a {@link ServerPacket} to multiple players. *

- * Be aware that this will cause the send packet listeners to be given the exact same packet object. + * Can drastically improve performance since the packet will not have to be processed as much. * * @param players the players to send the packet to * @param packet the packet to send to the players */ public static void sendGroupedPacket(@NotNull Collection players, @NotNull ServerPacket packet) { - for (Player player : players) { - player.getPlayerConnection().sendPacket(packet); + final ByteBuf buffer = writePacket(packet); + final boolean success = PACKET_LISTENER_MANAGER.processServerPacket(packet, players); + if (success) { + for (Player player : players) { + final PlayerConnection playerConnection = player.getPlayerConnection(); + if (playerConnection instanceof NettyPlayerConnection) { + ((NettyPlayerConnection) playerConnection).getChannel().write(buffer.retainedSlice()); + } else { + playerConnection.sendPacket(packet); + } + } } } From e8ddf44c39590676fc09ca680bb6091dfff60462 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 04:36:33 +0100 Subject: [PATCH 27/39] Stop hardcoding handler names --- .../server/network/netty/NettyServer.java | 27 ++++++++++++++----- .../packet/client/login/LoginStartPacket.java | 2 +- .../network/player/NettyPlayerConnection.java | 17 ++++++++---- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index 4008fab3b..4edc6bcdc 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -31,6 +31,21 @@ public class NettyServer { private static final long DEFAULT_CHANNEL_WRITE_LIMIT = 600_000L; private static final long DEFAULT_CHANNEL_READ_LIMIT = 100_000L; + public static final String TRAFFIC_LIMITER_HANDLER_NAME = "traffic-limiter"; + public static final String LEGACY_PING_HANDLER_NAME = "legacy-ping"; + + public static final String COMPRESSOR_HANDLER_NAME = "compressor"; + + public static final String FRAMER_HANDLER_NAME = "framer"; + + public static final String ENCRYPT_HANDLER_NAME = "encrypt"; + public static final String DECRYPT_HANDLER_NAME = "decrypt"; + + + public static final String DECODER_HANDLER_NAME = "decoder"; + public static final String ENCODER_HANDLER_NAME = "encoder"; + public static final String CLIENT_HANDLER_NAME = "handler"; + private final EventLoopGroup boss, worker; private final ServerBootstrap bootstrap; @@ -90,22 +105,22 @@ public class NettyServer { ChannelPipeline pipeline = ch.pipeline(); - pipeline.addLast("traffic-limiter", globalTrafficHandler); + pipeline.addLast(TRAFFIC_LIMITER_HANDLER_NAME, globalTrafficHandler); // First check should verify if the packet is a legacy ping (from 1.6 version and earlier) // Removed from the pipeline later in LegacyPingHandler if unnecessary (>1.6) - pipeline.addLast("legacy-ping", new LegacyPingHandler()); + pipeline.addLast(LEGACY_PING_HANDLER_NAME, new LegacyPingHandler()); // Adds packetLength at start | Reads framed bytebuf - pipeline.addLast("framer", new PacketFramer(packetProcessor)); + pipeline.addLast(FRAMER_HANDLER_NAME, new PacketFramer(packetProcessor)); // Reads bytebuf and creating inbound packet - pipeline.addLast("decoder", new PacketDecoder()); + pipeline.addLast(DECODER_HANDLER_NAME, new PacketDecoder()); // Writes packet to bytebuf - pipeline.addLast("encoder", new PacketEncoder()); + pipeline.addLast(ENCODER_HANDLER_NAME, new PacketEncoder()); - pipeline.addLast("handler", new ClientChannel(packetProcessor)); + pipeline.addLast(CLIENT_HANDLER_NAME, new ClientChannel(packetProcessor)); } }); } diff --git a/src/main/java/net/minestom/server/network/packet/client/login/LoginStartPacket.java b/src/main/java/net/minestom/server/network/packet/client/login/LoginStartPacket.java index 3741eeed7..437f26417 100644 --- a/src/main/java/net/minestom/server/network/packet/client/login/LoginStartPacket.java +++ b/src/main/java/net/minestom/server/network/packet/client/login/LoginStartPacket.java @@ -39,7 +39,7 @@ public class LoginStartPacket implements ClientPreplayPacket { // Compression final int threshold = MinecraftServer.getCompressionThreshold(); if (threshold > 0) { - nettyPlayerConnection.enableCompression(threshold); + nettyPlayerConnection.startCompression(); } } diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index c5f3c5922..9a2192037 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -2,11 +2,13 @@ package net.minestom.server.network.player; import io.netty.channel.Channel; import io.netty.channel.socket.SocketChannel; +import net.minestom.server.MinecraftServer; import net.minestom.server.entity.PlayerSkin; import net.minestom.server.extras.mojangAuth.Decrypter; import net.minestom.server.extras.mojangAuth.Encrypter; import net.minestom.server.extras.mojangAuth.MojangCrypt; import net.minestom.server.network.ConnectionState; +import net.minestom.server.network.netty.NettyServer; import net.minestom.server.network.netty.codec.PacketCompressor; import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.packet.server.login.SetCompressionPacket; @@ -73,21 +75,26 @@ public class NettyPlayerConnection extends PlayerConnection { public void setEncryptionKey(@NotNull SecretKey secretKey) { Check.stateCondition(encrypted, "Encryption is already enabled!"); this.encrypted = true; - channel.pipeline().addBefore("framer", "decrypt", new Decrypter(MojangCrypt.getCipher(2, secretKey))); - channel.pipeline().addBefore("framer", "encrypt", new Encrypter(MojangCrypt.getCipher(1, secretKey))); + channel.pipeline().addBefore(NettyServer.FRAMER_HANDLER_NAME, NettyServer.DECRYPT_HANDLER_NAME, + new Decrypter(MojangCrypt.getCipher(2, secretKey))); + channel.pipeline().addBefore(NettyServer.FRAMER_HANDLER_NAME, NettyServer.ENCRYPT_HANDLER_NAME, + new Encrypter(MojangCrypt.getCipher(1, secretKey))); } /** * Enables compression and add a new codec to the pipeline. * - * @param threshold the threshold for a packet to be compressible * @throws IllegalStateException if encryption is already enabled for this connection */ - public void enableCompression(int threshold) { + public void startCompression() { Check.stateCondition(compressed, "Compression is already enabled!"); + final int threshold = MinecraftServer.getCompressionThreshold(); + Check.stateCondition(threshold == 0, "Compression cannot be enabled because the threshold is equal to 0"); + this.compressed = true; sendPacket(new SetCompressionPacket(threshold)); - channel.pipeline().addAfter("framer", "compressor", new PacketCompressor(threshold)); + channel.pipeline().addAfter(NettyServer.FRAMER_HANDLER_NAME, NettyServer.COMPRESSOR_HANDLER_NAME, + new PacketCompressor(threshold)); } /** From 3455c77eb76189ef4a00bbbbe18f961907315b4f Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 04:48:33 +0100 Subject: [PATCH 28/39] Explain the role of each handlers --- .../server/network/netty/NettyServer.java | 21 +++++++++---------- .../minestom/server/utils/PacketUtils.java | 6 ++++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index 4edc6bcdc..db7ce3b88 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -26,25 +26,24 @@ import java.net.InetSocketAddress; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -public class NettyServer { +public final class NettyServer { private static final long DEFAULT_CHANNEL_WRITE_LIMIT = 600_000L; private static final long DEFAULT_CHANNEL_READ_LIMIT = 100_000L; - public static final String TRAFFIC_LIMITER_HANDLER_NAME = "traffic-limiter"; - public static final String LEGACY_PING_HANDLER_NAME = "legacy-ping"; + public static final String TRAFFIC_LIMITER_HANDLER_NAME = "traffic-limiter"; // Read/write + public static final String LEGACY_PING_HANDLER_NAME = "legacy-ping"; // Read - public static final String COMPRESSOR_HANDLER_NAME = "compressor"; + public static final String ENCRYPT_HANDLER_NAME = "encrypt"; // Write + public static final String DECRYPT_HANDLER_NAME = "decrypt"; // Read - public static final String FRAMER_HANDLER_NAME = "framer"; + public static final String FRAMER_HANDLER_NAME = "framer"; // Read/write - public static final String ENCRYPT_HANDLER_NAME = "encrypt"; - public static final String DECRYPT_HANDLER_NAME = "decrypt"; + public static final String COMPRESSOR_HANDLER_NAME = "compressor"; // Read/write - - public static final String DECODER_HANDLER_NAME = "decoder"; - public static final String ENCODER_HANDLER_NAME = "encoder"; - public static final String CLIENT_HANDLER_NAME = "handler"; + public static final String DECODER_HANDLER_NAME = "decoder"; // Read + public static final String ENCODER_HANDLER_NAME = "encoder"; // Write + public static final String CLIENT_HANDLER_NAME = "handler"; // Read private final EventLoopGroup boss, worker; private final ServerBootstrap bootstrap; diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 31fcbef75..52772b503 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -34,13 +34,15 @@ public final class PacketUtils { * @param packet the packet to send to the players */ public static void sendGroupedPacket(@NotNull Collection players, @NotNull ServerPacket packet) { - final ByteBuf buffer = writePacket(packet); final boolean success = PACKET_LISTENER_MANAGER.processServerPacket(packet, players); if (success) { + final ByteBuf buffer = writePacket(packet); + for (Player player : players) { final PlayerConnection playerConnection = player.getPlayerConnection(); if (playerConnection instanceof NettyPlayerConnection) { - ((NettyPlayerConnection) playerConnection).getChannel().write(buffer.retainedSlice()); + final NettyPlayerConnection nettyPlayerConnection = (NettyPlayerConnection) playerConnection; + nettyPlayerConnection.getChannel().write(buffer.retainedSlice()); } else { playerConnection.sendPacket(packet); } From 4060f8d2903b0a9fffe942b374d77f8fe5983611 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 05:37:13 +0100 Subject: [PATCH 29/39] Added GroupedPacketHandler to prevent rewriting the same packet multiple times --- .../server/network/netty/NettyServer.java | 9 +-- .../netty/codec/GroupedPacketHandler.java | 14 ++++ .../network/netty/codec/PacketCompressor.java | 20 +----- .../network/netty/codec/PacketFramer.java | 13 +--- .../network/netty/packet/FramedPacket.java | 18 +++++ .../network/player/NettyPlayerConnection.java | 2 +- .../minestom/server/utils/PacketUtils.java | 68 ++++++++++++++++++- 7 files changed, 108 insertions(+), 36 deletions(-) create mode 100644 src/main/java/net/minestom/server/network/netty/codec/GroupedPacketHandler.java create mode 100644 src/main/java/net/minestom/server/network/netty/packet/FramedPacket.java diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index db7ce3b88..93bad8b0c 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -16,10 +16,7 @@ import io.netty.handler.traffic.GlobalChannelTrafficShapingHandler; import io.netty.handler.traffic.TrafficCounter; import net.minestom.server.network.PacketProcessor; import net.minestom.server.network.netty.channel.ClientChannel; -import net.minestom.server.network.netty.codec.LegacyPingHandler; -import net.minestom.server.network.netty.codec.PacketDecoder; -import net.minestom.server.network.netty.codec.PacketEncoder; -import net.minestom.server.network.netty.codec.PacketFramer; +import net.minestom.server.network.netty.codec.*; import org.jetbrains.annotations.NotNull; import java.net.InetSocketAddress; @@ -37,6 +34,7 @@ public final class NettyServer { public static final String ENCRYPT_HANDLER_NAME = "encrypt"; // Write public static final String DECRYPT_HANDLER_NAME = "decrypt"; // Read + public static final String GROUPED_PACKET_HANDLER_NAME = "grouped-packet"; // Write public static final String FRAMER_HANDLER_NAME = "framer"; // Read/write public static final String COMPRESSOR_HANDLER_NAME = "compressor"; // Read/write @@ -110,6 +108,9 @@ public final class NettyServer { // Removed from the pipeline later in LegacyPingHandler if unnecessary (>1.6) pipeline.addLast(LEGACY_PING_HANDLER_NAME, new LegacyPingHandler()); + // Used to bypass all the previous handlers by directly sending a framed buffer + pipeline.addLast(GROUPED_PACKET_HANDLER_NAME, new GroupedPacketHandler()); + // Adds packetLength at start | Reads framed bytebuf pipeline.addLast(FRAMER_HANDLER_NAME, new PacketFramer(packetProcessor)); diff --git a/src/main/java/net/minestom/server/network/netty/codec/GroupedPacketHandler.java b/src/main/java/net/minestom/server/network/netty/codec/GroupedPacketHandler.java new file mode 100644 index 000000000..ba23a268b --- /dev/null +++ b/src/main/java/net/minestom/server/network/netty/codec/GroupedPacketHandler.java @@ -0,0 +1,14 @@ +package net.minestom.server.network.netty.codec; + +import io.netty.buffer.ByteBuf; +import io.netty.channel.ChannelHandlerContext; +import io.netty.handler.codec.MessageToByteEncoder; +import net.minestom.server.network.netty.packet.FramedPacket; + +public class GroupedPacketHandler extends MessageToByteEncoder { + + @Override + protected void encode(ChannelHandlerContext ctx, FramedPacket msg, ByteBuf out) { + out.writeBytes(msg.body.retainedSlice()); + } +} diff --git a/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java b/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java index eae4f8313..79f09af67 100644 --- a/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java +++ b/src/main/java/net/minestom/server/network/netty/codec/PacketCompressor.java @@ -21,6 +21,7 @@ import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.ByteToMessageCodec; import io.netty.handler.codec.DecoderException; +import net.minestom.server.utils.PacketUtils; import net.minestom.server.utils.Utils; import java.util.List; @@ -42,24 +43,7 @@ public class PacketCompressor extends ByteToMessageCodec { @Override protected void encode(ChannelHandlerContext ctx, ByteBuf from, ByteBuf to) { - final int packetLength = from.readableBytes(); - - if (packetLength < this.threshold) { - Utils.writeVarIntBuf(to, 0); - to.writeBytes(from); - } else { - Utils.writeVarIntBuf(to, packetLength); - - deflater.setInput(from.nioBuffer()); - deflater.finish(); - - while (!deflater.finished()) { - final int length = deflater.deflate(buffer); - to.writeBytes(buffer, 0, length); - } - - deflater.reset(); - } + PacketUtils.compressBuffer(deflater, buffer, from, to); } @Override diff --git a/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java b/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java index 62af408d0..a342a73e4 100644 --- a/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java +++ b/src/main/java/net/minestom/server/network/netty/codec/PacketFramer.java @@ -7,6 +7,7 @@ import io.netty.handler.codec.CorruptedFrameException; import net.minestom.server.MinecraftServer; import net.minestom.server.network.PacketProcessor; import net.minestom.server.network.player.PlayerConnection; +import net.minestom.server.utils.PacketUtils; import net.minestom.server.utils.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,17 +26,7 @@ public class PacketFramer extends ByteToMessageCodec { @Override protected void encode(ChannelHandlerContext ctx, ByteBuf from, ByteBuf to) { - final int packetSize = from.readableBytes(); - final int headerSize = Utils.getVarIntSize(packetSize); - - if (headerSize > 3) { - throw new IllegalStateException("Unable to fit " + headerSize + " into 3"); - } - - to.ensureWritable(packetSize + headerSize); - - Utils.writeVarIntBuf(to, packetSize); - to.writeBytes(from, from.readerIndex(), packetSize); + PacketUtils.frameBuffer(from, to); } @Override diff --git a/src/main/java/net/minestom/server/network/netty/packet/FramedPacket.java b/src/main/java/net/minestom/server/network/netty/packet/FramedPacket.java new file mode 100644 index 000000000..4466539b7 --- /dev/null +++ b/src/main/java/net/minestom/server/network/netty/packet/FramedPacket.java @@ -0,0 +1,18 @@ +package net.minestom.server.network.netty.packet; + +import io.netty.buffer.ByteBuf; +import org.jetbrains.annotations.NotNull; + +/** + * Represents a packet which is already framed. + * Can be used if you want to send the exact same buffer to multiple clients without processing it more than once. + */ +public class FramedPacket { + + public final ByteBuf body; + + public FramedPacket(@NotNull ByteBuf body) { + this.body = body; + } + +} diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index 9a2192037..7365329a4 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -134,7 +134,7 @@ public class NettyPlayerConnection extends PlayerConnection { @Override public void disconnect() { - channel.close(); + this.channel.close(); } @NotNull diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 52772b503..d8d99d82e 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -5,6 +5,7 @@ import io.netty.buffer.Unpooled; import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; import net.minestom.server.listener.manager.PacketListenerManager; +import net.minestom.server.network.netty.packet.FramedPacket; import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.player.NettyPlayerConnection; import net.minestom.server.network.player.PlayerConnection; @@ -12,6 +13,7 @@ import net.minestom.server.utils.binary.BinaryWriter; import org.jetbrains.annotations.NotNull; import java.util.Collection; +import java.util.zip.Deflater; /** * Utils class for packets. Including writing a {@link ServerPacket} into a {@link ByteBuf} @@ -21,6 +23,9 @@ public final class PacketUtils { private static final PacketListenerManager PACKET_LISTENER_MANAGER = MinecraftServer.getPacketListenerManager(); + private static Deflater deflater = new Deflater(); + private static byte[] buffer = new byte[8192]; + private PacketUtils() { } @@ -36,13 +41,14 @@ public final class PacketUtils { public static void sendGroupedPacket(@NotNull Collection players, @NotNull ServerPacket packet) { final boolean success = PACKET_LISTENER_MANAGER.processServerPacket(packet, players); if (success) { - final ByteBuf buffer = writePacket(packet); + final ByteBuf finalBuffer = createFramedPacket(packet); + final FramedPacket framedPacket = new FramedPacket(finalBuffer); for (Player player : players) { final PlayerConnection playerConnection = player.getPlayerConnection(); if (playerConnection instanceof NettyPlayerConnection) { final NettyPlayerConnection nettyPlayerConnection = (NettyPlayerConnection) playerConnection; - nettyPlayerConnection.getChannel().write(buffer.retainedSlice()); + nettyPlayerConnection.getChannel().write(framedPacket); } else { playerConnection.sendPacket(packet); } @@ -108,4 +114,62 @@ public final class PacketUtils { return writer.getBuffer(); } + public static void frameBuffer(@NotNull ByteBuf from, @NotNull ByteBuf to) { + final int packetSize = from.readableBytes(); + final int headerSize = Utils.getVarIntSize(packetSize); + + if (headerSize > 3) { + throw new IllegalStateException("Unable to fit " + headerSize + " into 3"); + } + + to.ensureWritable(packetSize + headerSize); + + Utils.writeVarIntBuf(to, packetSize); + to.writeBytes(from, from.readerIndex(), packetSize); + } + + public static void compressBuffer(@NotNull Deflater deflater, @NotNull byte[] buffer, @NotNull ByteBuf from, @NotNull ByteBuf to) { + final int packetLength = from.readableBytes(); + + if (packetLength < MinecraftServer.getCompressionThreshold()) { + Utils.writeVarIntBuf(to, 0); + to.writeBytes(from); + } else { + Utils.writeVarIntBuf(to, packetLength); + + deflater.setInput(from.nioBuffer()); + deflater.finish(); + + while (!deflater.finished()) { + final int length = deflater.deflate(buffer); + to.writeBytes(buffer, 0, length); + } + + deflater.reset(); + } + } + + private static ByteBuf createFramedPacket(@NotNull ServerPacket serverPacket) { + ByteBuf packetBuf = writePacket(serverPacket); + + if (MinecraftServer.getCompressionThreshold() > 0) { + + ByteBuf compressedBuf = Unpooled.buffer(); + ByteBuf framedBuf = Unpooled.buffer(); + synchronized (deflater) { + compressBuffer(deflater, buffer, packetBuf, compressedBuf); + } + + frameBuffer(compressedBuf, framedBuf); + + return framedBuf; + } else { + ByteBuf framedBuf = Unpooled.buffer(); + frameBuffer(packetBuf, framedBuf); + + return framedBuf; + } + + } + } From 9f45cf11a30130dd904336de618833d1fe8b84d6 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 05:50:52 +0100 Subject: [PATCH 30/39] Added todo to optimize buffer allocation --- src/main/java/net/minestom/server/utils/PacketUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index d8d99d82e..c40933f43 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -152,6 +152,7 @@ public final class PacketUtils { private static ByteBuf createFramedPacket(@NotNull ServerPacket serverPacket) { ByteBuf packetBuf = writePacket(serverPacket); + // TODO use pooled buffers instead of unpooled ones if (MinecraftServer.getCompressionThreshold() > 0) { ByteBuf compressedBuf = Unpooled.buffer(); From 02eab844a553ad18c9517c9c2b24094eceab5e62 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 07:55:34 +0100 Subject: [PATCH 31/39] Fix logging when compression is disabled (not recommended) --- .../server/network/netty/NettyServer.java | 23 +++++++++++++++---- .../minestom/server/utils/PacketUtils.java | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index 93bad8b0c..1af09b5ee 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -14,6 +14,7 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.handler.traffic.GlobalChannelTrafficShapingHandler; import io.netty.handler.traffic.TrafficCounter; +import net.minestom.server.MinecraftServer; import net.minestom.server.network.PacketProcessor; import net.minestom.server.network.netty.channel.ClientChannel; import net.minestom.server.network.netty.codec.*; @@ -25,8 +26,11 @@ import java.util.concurrent.ScheduledExecutorService; public final class NettyServer { - private static final long DEFAULT_CHANNEL_WRITE_LIMIT = 600_000L; - private static final long DEFAULT_CHANNEL_READ_LIMIT = 100_000L; + private static final long DEFAULT_COMPRESSED_CHANNEL_WRITE_LIMIT = 600_000L; + private static final long DEFAULT_COMPRESSED_CHANNEL_READ_LIMIT = 100_000L; + + private static final long DEFAULT_UNCOMPRESSED_CHANNEL_WRITE_LIMIT = 15_000_000L; + private static final long DEFAULT_UNCOMPRESSED_CHANNEL_READ_LIMIT = 1_000_000L; public static final String TRAFFIC_LIMITER_HANDLER_NAME = "traffic-limiter"; // Read/write public static final String LEGACY_PING_HANDLER_NAME = "legacy-ping"; // Read @@ -90,9 +94,6 @@ public final class NettyServer { } }; - globalTrafficHandler.setWriteChannelLimit(DEFAULT_CHANNEL_WRITE_LIMIT); - globalTrafficHandler.setReadChannelLimit(DEFAULT_CHANNEL_READ_LIMIT); - bootstrap.childHandler(new ChannelInitializer() { protected void initChannel(@NotNull SocketChannel ch) { @@ -132,6 +133,18 @@ public final class NettyServer { * @param port the server port */ public void start(@NotNull String address, int port) { + + { + final boolean compression = MinecraftServer.getCompressionThreshold() != 0; + if (compression) { + globalTrafficHandler.setWriteChannelLimit(DEFAULT_COMPRESSED_CHANNEL_WRITE_LIMIT); + globalTrafficHandler.setReadChannelLimit(DEFAULT_COMPRESSED_CHANNEL_READ_LIMIT); + } else { + globalTrafficHandler.setWriteChannelLimit(DEFAULT_UNCOMPRESSED_CHANNEL_WRITE_LIMIT); + globalTrafficHandler.setReadChannelLimit(DEFAULT_UNCOMPRESSED_CHANNEL_READ_LIMIT); + } + } + this.address = address; this.port = port; diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index c40933f43..463f52838 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -23,7 +23,7 @@ public final class PacketUtils { private static final PacketListenerManager PACKET_LISTENER_MANAGER = MinecraftServer.getPacketListenerManager(); - private static Deflater deflater = new Deflater(); + private static Deflater deflater = new Deflater(3); private static byte[] buffer = new byte[8192]; private PacketUtils() { From e453a0f9b510b5cf2916f46cbb4702d7ca0b3b7b Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 08:39:06 +0100 Subject: [PATCH 32/39] Added Chunk#getLastChangeTime --- .../net/minestom/server/instance/Chunk.java | 11 ++++++ .../server/instance/DynamicChunk.java | 34 ++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index 7b5cad348..92e37c495 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/src/main/java/net/minestom/server/instance/Chunk.java @@ -191,6 +191,17 @@ public abstract class Chunk implements Viewable, DataContainer { @NotNull public abstract Set getBlockEntities(); + /** + * Gets the last time that this chunk changed. + *

+ * "Change" means here data used in {@link ChunkDataPacket}. + * It is necessary to see if the cached version of this chunk can be used + * instead of re writing and compressing everything. + * + * @return the last change time in milliseconds + */ + public abstract long getLastChangeTime(); + /** * Serializes the chunk into bytes. * diff --git a/src/main/java/net/minestom/server/instance/DynamicChunk.java b/src/main/java/net/minestom/server/instance/DynamicChunk.java index 4def0f0b1..80929c9e9 100644 --- a/src/main/java/net/minestom/server/instance/DynamicChunk.java +++ b/src/main/java/net/minestom/server/instance/DynamicChunk.java @@ -57,6 +57,8 @@ public class DynamicChunk extends Chunk { // Block entities protected final Set blockEntities = new CopyOnWriteArraySet<>(); + private long lastChangeTime; + public DynamicChunk(@Nullable Biome[] biomes, int chunkX, int chunkZ, @NotNull PaletteStorage blockPalette, @NotNull PaletteStorage customBlockPalette) { super(biomes, chunkX, chunkZ, true); @@ -86,8 +88,8 @@ public class DynamicChunk extends Chunk { // True if the block is not complete air without any custom block capabilities final boolean hasBlock = blockStateId != 0 || customBlockId != 0; - this.blockPalette.setBlockAt(x, y, z, blockStateId); - this.customBlockPalette.setBlockAt(x, y, z, customBlockId); + setBlockAt(blockPalette, x, y, z, blockStateId); + setBlockAt(customBlockPalette, x, y, z, customBlockId); if (!hasBlock) { // Block has been deleted, clear cache and return @@ -155,23 +157,23 @@ public class DynamicChunk extends Chunk { @Override public short getBlockStateId(int x, int y, int z) { - return this.blockPalette.getBlockAt(x, y, z); + return getBlockAt(blockPalette, x, y, z); } @Override public short getCustomBlockId(int x, int y, int z) { - return customBlockPalette.getBlockAt(x, y, z); + return getBlockAt(customBlockPalette, x, y, z); } @Override protected void refreshBlockValue(int x, int y, int z, short blockStateId, short customBlockId) { - this.blockPalette.setBlockAt(x, y, z, blockStateId); - this.customBlockPalette.setBlockAt(x, y, z, customBlockId); + setBlockAt(blockPalette, x, y, z, blockStateId); + setBlockAt(customBlockPalette, x, y, z, customBlockId); } @Override protected void refreshBlockStateId(int x, int y, int z, short blockStateId) { - this.blockPalette.setBlockAt(x, y, z, blockStateId); + setBlockAt(blockPalette, x, y, z, blockStateId); } @Override @@ -195,6 +197,11 @@ public class DynamicChunk extends Chunk { return blockEntities; } + @Override + public long getLastChangeTime() { + return lastChangeTime; + } + /** * Serialize this {@link Chunk} based on {@link #readChunk(BinaryReader, ChunkCallback)} *

@@ -242,8 +249,8 @@ public class DynamicChunk extends Chunk { for (byte z = 0; z < CHUNK_SIZE_Z; z++) { final int index = getBlockIndex(x, y, z); - final short blockStateId = blockPalette.getBlockAt(x, y, z); - final short customBlockId = customBlockPalette.getBlockAt(x, y, z); + final short blockStateId = getBlockAt(blockPalette, x, y, z); + final short customBlockId = getBlockAt(customBlockPalette, x, y, z); // No block at the position if (blockStateId == 0 && customBlockId == 0) @@ -400,4 +407,13 @@ public class DynamicChunk extends Chunk { return dynamicChunk; } + + private short getBlockAt(@NotNull PaletteStorage paletteStorage, int x, int y, int z) { + return paletteStorage.getBlockAt(x, y, z); + } + + private void setBlockAt(@NotNull PaletteStorage paletteStorage, int x, int y, int z, short blockId) { + paletteStorage.setBlockAt(x, y, z, blockId); + this.lastChangeTime = System.currentTimeMillis(); + } } \ No newline at end of file From 075ff7600a109c930219d553dc15561739ca268a Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 11:14:15 +0100 Subject: [PATCH 33/39] Added a whole new caching system for ChunkDataPacket and UpdateLightPacket --- .../net/minestom/server/MinecraftServer.java | 2 + .../net/minestom/server/instance/Chunk.java | 17 ++++- .../server/instance/DynamicChunk.java | 2 +- .../server/network/netty/NettyServer.java | 4 +- .../packet/server/play/ChunkDataPacket.java | 31 +++++++- .../packet/server/play/UpdateLightPacket.java | 32 +++++++- .../network/player/NettyPlayerConnection.java | 32 +++++++- .../minestom/server/utils/PacketUtils.java | 2 +- .../server/utils/cache/CacheablePacket.java | 45 +++++++++++ .../server/utils/cache/TemporaryCache.java | 75 +++++++++++++++++++ .../utils/cache/TemporaryPacketCache.java | 12 +++ 11 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 src/main/java/net/minestom/server/utils/cache/CacheablePacket.java create mode 100644 src/main/java/net/minestom/server/utils/cache/TemporaryCache.java create mode 100644 src/main/java/net/minestom/server/utils/cache/TemporaryPacketCache.java diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index 91f37f2c0..03f536c03 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -43,6 +43,7 @@ import net.minestom.server.storage.StorageManager; import net.minestom.server.timer.SchedulerManager; import net.minestom.server.utils.MathUtils; import net.minestom.server.utils.PacketUtils; +import net.minestom.server.utils.cache.TemporaryCache; import net.minestom.server.utils.thread.MinestomThread; import net.minestom.server.utils.validate.Check; import net.minestom.server.world.Difficulty; @@ -654,6 +655,7 @@ public final class MinecraftServer { LOGGER.info("Shutting down all thread pools."); benchmarkManager.disable(); commandManager.stopConsoleThread(); + TemporaryCache.REMOVER_SERVICE.shutdown(); MinestomThread.shutdownAll(); LOGGER.info("Minestom server stopped successfully."); } diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index 92e37c495..e2327e266 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/src/main/java/net/minestom/server/instance/Chunk.java @@ -61,6 +61,8 @@ public abstract class Chunk implements Viewable, DataContainer { public static final int BIOME_COUNT = 1024; // 4x4x4 blocks group + private final UUID identifier; + @NotNull protected final Biome[] biomes; protected final int chunkX, chunkZ; @@ -79,6 +81,7 @@ public abstract class Chunk implements Viewable, DataContainer { protected Data data; public Chunk(@Nullable Biome[] biomes, int chunkX, int chunkZ, boolean shouldGenerate) { + this.identifier = UUID.randomUUID(); this.chunkX = chunkX; this.chunkZ = chunkZ; this.shouldGenerate = shouldGenerate; @@ -270,6 +273,18 @@ public abstract class Chunk implements Viewable, DataContainer { return getCustomBlock(x, y, z); } + /** + * Gets the unique identifier of this chunk. + *

+ * WARNING: this UUID is not persistent but randomized once the object is instantiate. + * + * @return the chunk identifier + */ + @NotNull + public UUID getIdentifier() { + return identifier; + } + public Biome[] getBiomes() { return biomes; } @@ -459,7 +474,7 @@ public abstract class Chunk implements Viewable, DataContainer { // TODO do not hardcode light { - UpdateLightPacket updateLightPacket = new UpdateLightPacket(); + UpdateLightPacket updateLightPacket = new UpdateLightPacket(getIdentifier(), getLastChangeTime()); updateLightPacket.chunkX = getChunkX(); updateLightPacket.chunkZ = getChunkZ(); updateLightPacket.skyLightMask = 0x3FFF0; diff --git a/src/main/java/net/minestom/server/instance/DynamicChunk.java b/src/main/java/net/minestom/server/instance/DynamicChunk.java index 80929c9e9..524041707 100644 --- a/src/main/java/net/minestom/server/instance/DynamicChunk.java +++ b/src/main/java/net/minestom/server/instance/DynamicChunk.java @@ -383,7 +383,7 @@ public class DynamicChunk extends Chunk { @NotNull @Override protected ChunkDataPacket createFreshPacket() { - ChunkDataPacket fullDataPacket = new ChunkDataPacket(); + ChunkDataPacket fullDataPacket = new ChunkDataPacket(getIdentifier(), getLastChangeTime()); fullDataPacket.biomes = biomes.clone(); fullDataPacket.chunkX = chunkX; fullDataPacket.chunkZ = chunkZ; diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index 1af09b5ee..c3b94b75f 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -45,7 +45,7 @@ public final class NettyServer { public static final String DECODER_HANDLER_NAME = "decoder"; // Read public static final String ENCODER_HANDLER_NAME = "encoder"; // Write - public static final String CLIENT_HANDLER_NAME = "handler"; // Read + public static final String CLIENT_CHANNEL_NAME = "handler"; // Read private final EventLoopGroup boss, worker; private final ServerBootstrap bootstrap; @@ -121,7 +121,7 @@ public final class NettyServer { // Writes packet to bytebuf pipeline.addLast(ENCODER_HANDLER_NAME, new PacketEncoder()); - pipeline.addLast(CLIENT_HANDLER_NAME, new ClientChannel(packetProcessor)); + pipeline.addLast(CLIENT_CHANNEL_NAME, new ClientChannel(packetProcessor)); } }); } diff --git a/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java b/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java index 0f68d6560..aaae57545 100644 --- a/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java +++ b/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java @@ -13,16 +13,21 @@ import net.minestom.server.network.packet.server.ServerPacketIdentifier; import net.minestom.server.utils.BlockPosition; import net.minestom.server.utils.Utils; import net.minestom.server.utils.binary.BinaryWriter; +import net.minestom.server.utils.cache.CacheablePacket; +import net.minestom.server.utils.cache.TemporaryPacketCache; import net.minestom.server.utils.chunk.ChunkUtils; import net.minestom.server.world.biomes.Biome; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jglrxavpok.hephaistos.nbt.NBTCompound; import java.util.Set; +import java.util.UUID; -public class ChunkDataPacket implements ServerPacket { +public class ChunkDataPacket implements ServerPacket, CacheablePacket { private static final BlockManager BLOCK_MANAGER = MinecraftServer.getBlockManager(); + private static final TemporaryPacketCache CACHE = new TemporaryPacketCache(10000L); public boolean fullChunk; public Biome[] biomes; @@ -40,6 +45,15 @@ public class ChunkDataPacket implements ServerPacket { private static final int MAX_BITS_PER_ENTRY = 16; private static final int MAX_BUFFER_SIZE = (Short.BYTES + Byte.BYTES + 5 * Byte.BYTES + (4096 * MAX_BITS_PER_ENTRY / Long.SIZE * Long.BYTES)) * CHUNK_SECTION_COUNT + 256 * Integer.BYTES; + // Cacheable data + private UUID identifier; + private long lastUpdate; + + public ChunkDataPacket(@Nullable UUID identifier, long lastUpdate) { + this.identifier = identifier; + this.lastUpdate = lastUpdate; + } + @Override public void write(@NotNull BinaryWriter writer) { writer.writeInt(chunkX); @@ -121,4 +135,19 @@ public class ChunkDataPacket implements ServerPacket { public int getId() { return ServerPacketIdentifier.CHUNK_DATA; } + + @Override + public TemporaryPacketCache getCache() { + return CACHE; + } + + @Override + public UUID getIdentifier() { + return identifier; + } + + @Override + public long getLastUpdateTime() { + return lastUpdate; + } } \ No newline at end of file diff --git a/src/main/java/net/minestom/server/network/packet/server/play/UpdateLightPacket.java b/src/main/java/net/minestom/server/network/packet/server/play/UpdateLightPacket.java index 6647d99ae..c2df52b61 100644 --- a/src/main/java/net/minestom/server/network/packet/server/play/UpdateLightPacket.java +++ b/src/main/java/net/minestom/server/network/packet/server/play/UpdateLightPacket.java @@ -3,11 +3,17 @@ package net.minestom.server.network.packet.server.play; import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.packet.server.ServerPacketIdentifier; import net.minestom.server.utils.binary.BinaryWriter; +import net.minestom.server.utils.cache.CacheablePacket; +import net.minestom.server.utils.cache.TemporaryPacketCache; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.List; +import java.util.UUID; -public class UpdateLightPacket implements ServerPacket { +public class UpdateLightPacket implements ServerPacket, CacheablePacket { + + private static final TemporaryPacketCache CACHE = new TemporaryPacketCache(10000L); public int chunkX; public int chunkZ; @@ -23,6 +29,15 @@ public class UpdateLightPacket implements ServerPacket { public List skyLight; public List blockLight; + // Cacheable data + private UUID identifier; + private long lastUpdate; + + public UpdateLightPacket(@Nullable UUID identifier, long lastUpdate) { + this.identifier = identifier; + this.lastUpdate = lastUpdate; + } + @Override public void write(@NotNull BinaryWriter writer) { writer.writeVarInt(chunkX); @@ -53,4 +68,19 @@ public class UpdateLightPacket implements ServerPacket { public int getId() { return ServerPacketIdentifier.UPDATE_LIGHT; } + + @Override + public TemporaryPacketCache getCache() { + return CACHE; + } + + @Override + public UUID getIdentifier() { + return identifier; + } + + @Override + public long getLastUpdateTime() { + return lastUpdate; + } } diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index 7365329a4..d13b10959 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -1,5 +1,6 @@ package net.minestom.server.network.player; +import io.netty.buffer.ByteBuf; import io.netty.channel.Channel; import io.netty.channel.socket.SocketChannel; import net.minestom.server.MinecraftServer; @@ -10,8 +11,12 @@ import net.minestom.server.extras.mojangAuth.MojangCrypt; import net.minestom.server.network.ConnectionState; import net.minestom.server.network.netty.NettyServer; import net.minestom.server.network.netty.codec.PacketCompressor; +import net.minestom.server.network.netty.packet.FramedPacket; import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.packet.server.login.SetCompressionPacket; +import net.minestom.server.utils.PacketUtils; +import net.minestom.server.utils.cache.CacheablePacket; +import net.minestom.server.utils.cache.TemporaryCache; import net.minestom.server.utils.validate.Check; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -108,7 +113,32 @@ public class NettyPlayerConnection extends PlayerConnection { public void sendPacket(@NotNull ServerPacket serverPacket) { if (shouldSendPacket(serverPacket)) { if (getPlayer() != null) { - this.channel.write(serverPacket); // Flush on player update + // Flush happen during #update() + if (serverPacket instanceof CacheablePacket) { + CacheablePacket cacheablePacket = (CacheablePacket) serverPacket; + final UUID identifier = cacheablePacket.getIdentifier(); + + if (identifier == null) { + // This packet explicitly said to do not retrieve the cache + this.channel.write(serverPacket); + } else { + // Try to retrieve the cached buffer + TemporaryCache temporaryCache = cacheablePacket.getCache(); + ByteBuf buffer = temporaryCache.retrieve(identifier); + if (buffer == null) { + // Buffer not found, create and cache it + final long time = System.currentTimeMillis(); + buffer = PacketUtils.createFramedPacket(serverPacket); + temporaryCache.cacheObject(identifier, buffer, time); + } + + FramedPacket framedPacket = new FramedPacket(buffer); + this.channel.write(framedPacket); + } + + } else { + this.channel.write(serverPacket); + } } else { this.channel.writeAndFlush(serverPacket); } diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 463f52838..d45dbbd8a 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -149,7 +149,7 @@ public final class PacketUtils { } } - private static ByteBuf createFramedPacket(@NotNull ServerPacket serverPacket) { + public static ByteBuf createFramedPacket(@NotNull ServerPacket serverPacket) { ByteBuf packetBuf = writePacket(serverPacket); // TODO use pooled buffers instead of unpooled ones diff --git a/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java b/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java new file mode 100644 index 000000000..9c32f6103 --- /dev/null +++ b/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java @@ -0,0 +1,45 @@ +package net.minestom.server.utils.cache; + +import net.minestom.server.network.packet.server.ServerPacket; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.UUID; + +/** + * Implemented by {@link ServerPacket server packets} which can be temporary cached in memory to be re-sent later + * without having to go through all the writing and compression. + *

+ * {@link #getIdentifier()} is to differenciate this packet from the others of the same type, + * and {@link #getLastUpdateTime()} to know if one packet is newer than the previous one. + */ +public interface CacheablePacket { + + /** + * Gets the cache linked to this packet. + *

+ * WARNING: the cache needs to be shared between all the object instances, tips is to make it static. + * + * @return the temporary packet cache + */ + @NotNull + TemporaryPacketCache getCache(); + + /** + * Gets the identifier of this packet. + *

+ * Used to verify if this packet is already cached or not. + * + * @return this packet identifier, null to do not retrieve the cache + */ + @Nullable + UUID getIdentifier(); + + /** + * Gets the last time this packet changed. + * + * @return the last packet update time in milliseconds + */ + long getLastUpdateTime(); + +} diff --git a/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java new file mode 100644 index 000000000..8a8074b3c --- /dev/null +++ b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java @@ -0,0 +1,75 @@ +package net.minestom.server.utils.cache; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * Cache objects with a timeout. + * + * @param the object type to cache + */ +public class TemporaryCache { + + public static final ScheduledExecutorService REMOVER_SERVICE = Executors.newScheduledThreadPool(1); + + // Identifier = Cached object + protected ConcurrentHashMap cache = new ConcurrentHashMap<>(); + // Identifier = time + protected ConcurrentHashMap cacheTime = new ConcurrentHashMap<>(); + + private long keepTime; + + /** + * Creates a new temporary cache. + * + * @param keepTime the time before considering an object unused in milliseconds + * @see #getKeepTime() + */ + public TemporaryCache(long keepTime) { + this.keepTime = keepTime; + REMOVER_SERVICE.scheduleAtFixedRate(() -> { + final boolean removed = cacheTime.values().removeIf(time -> time + keepTime > System.currentTimeMillis()); + if (removed) { + this.cache.entrySet().removeIf(entry -> !cacheTime.containsKey(entry.getKey())); + } + }, keepTime, keepTime, TimeUnit.MILLISECONDS); + } + + /** + * Caches an object + * + * @param identifier the object identifier + * @param value the object to cache + * @param time the current time in milliseconds + */ + public synchronized void cacheObject(@NotNull UUID identifier, T value, long time) { + this.cache.put(identifier, value); + this.cacheTime.put(identifier, time); + } + + /** + * Retrieves an object from cache. + * + * @param identifier the object identifier + * @return the retrieved object or null if not found + */ + @Nullable + public T retrieve(@NotNull UUID identifier) { + return cache.get(identifier); + } + + /** + * Gets the time an object will be kept without being retrieved + * + * @return the keep time in milliseconds + */ + public long getKeepTime() { + return keepTime; + } +} diff --git a/src/main/java/net/minestom/server/utils/cache/TemporaryPacketCache.java b/src/main/java/net/minestom/server/utils/cache/TemporaryPacketCache.java new file mode 100644 index 000000000..4e7b975b0 --- /dev/null +++ b/src/main/java/net/minestom/server/utils/cache/TemporaryPacketCache.java @@ -0,0 +1,12 @@ +package net.minestom.server.utils.cache; + +import io.netty.buffer.ByteBuf; + +/** + * Convenient superclass of {@link TemporaryCache} explicitly for packet to store a {@link ByteBuf}. + */ +public class TemporaryPacketCache extends TemporaryCache { + public TemporaryPacketCache(long keepTime) { + super(keepTime); + } +} From f1c0c9978106ac5873988a42c670c3258e5cf709 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 11:59:58 +0100 Subject: [PATCH 34/39] Fixed TemporaryCache check --- .../java/net/minestom/server/utils/cache/TemporaryCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java index 8a8074b3c..07ea8a51f 100644 --- a/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java +++ b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java @@ -34,7 +34,7 @@ public class TemporaryCache { public TemporaryCache(long keepTime) { this.keepTime = keepTime; REMOVER_SERVICE.scheduleAtFixedRate(() -> { - final boolean removed = cacheTime.values().removeIf(time -> time + keepTime > System.currentTimeMillis()); + final boolean removed = cacheTime.values().removeIf(time -> System.currentTimeMillis() > time + keepTime); if (removed) { this.cache.entrySet().removeIf(entry -> !cacheTime.containsKey(entry.getKey())); } From 8c5d013990bc4965509999f2d38ae84462b04363 Mon Sep 17 00:00:00 2001 From: TheMode Date: Fri, 20 Nov 2020 13:48:45 +0100 Subject: [PATCH 35/39] Flush in the current thread Signed-off-by: TheMode --- .../minestom/server/network/player/NettyPlayerConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index d13b10959..687b52a37 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -66,7 +66,7 @@ public class NettyPlayerConnection extends PlayerConnection { @Override public void update() { // Flush - this.channel.eventLoop().execute(() -> channel.flush()); + this.channel.flush(); // Network stats super.update(); } From 98fe83c605dc3f5a78ee3575547b3eb7ad456e86 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 14:05:22 +0100 Subject: [PATCH 36/39] Comments for PacketUtils --- .../minestom/server/utils/PacketUtils.java | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index d45dbbd8a..3a84ae086 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -114,41 +114,70 @@ public final class PacketUtils { return writer.getBuffer(); } - public static void frameBuffer(@NotNull ByteBuf from, @NotNull ByteBuf to) { - final int packetSize = from.readableBytes(); + /** + * Frames a buffer for it to be understood by a Minecraft client. + *

+ * The content of {@code packetBuffer} can be either a compressed or uncompressed packet buffer, + * it depends of it the client did receive a {@link net.minestom.server.network.packet.server.login.SetCompressionPacket} packet before. + * + * @param packetBuffer the buffer containing compressed or uncompressed packet data + * @param frameTarget the buffer which will receive the framed version of {@code from} + */ + public static void frameBuffer(@NotNull ByteBuf packetBuffer, @NotNull ByteBuf frameTarget) { + final int packetSize = packetBuffer.readableBytes(); final int headerSize = Utils.getVarIntSize(packetSize); if (headerSize > 3) { throw new IllegalStateException("Unable to fit " + headerSize + " into 3"); } - to.ensureWritable(packetSize + headerSize); + frameTarget.ensureWritable(packetSize + headerSize); - Utils.writeVarIntBuf(to, packetSize); - to.writeBytes(from, from.readerIndex(), packetSize); + Utils.writeVarIntBuf(frameTarget, packetSize); + frameTarget.writeBytes(packetBuffer, packetBuffer.readerIndex(), packetSize); } - public static void compressBuffer(@NotNull Deflater deflater, @NotNull byte[] buffer, @NotNull ByteBuf from, @NotNull ByteBuf to) { - final int packetLength = from.readableBytes(); + /** + * Compress using zlib the content of a packet. + *

+ * {@code packetBuffer} needs to be the packet content without any header (if you want to use it to write a Minecraft packet). + * + * @param deflater the deflater for zlib compression + * @param buffer a cached buffer which will be used to store temporary the deflater output + * @param packetBuffer the buffer containing all the packet fields + * @param compressionTarget the buffer which will receive the compressed version of {@code packetBuffer} + */ + public static void compressBuffer(@NotNull Deflater deflater, @NotNull byte[] buffer, @NotNull ByteBuf packetBuffer, @NotNull ByteBuf compressionTarget) { + final int packetLength = packetBuffer.readableBytes(); if (packetLength < MinecraftServer.getCompressionThreshold()) { - Utils.writeVarIntBuf(to, 0); - to.writeBytes(from); + Utils.writeVarIntBuf(compressionTarget, 0); + compressionTarget.writeBytes(packetBuffer); } else { - Utils.writeVarIntBuf(to, packetLength); + Utils.writeVarIntBuf(compressionTarget, packetLength); - deflater.setInput(from.nioBuffer()); + deflater.setInput(packetBuffer.nioBuffer()); deflater.finish(); while (!deflater.finished()) { final int length = deflater.deflate(buffer); - to.writeBytes(buffer, 0, length); + compressionTarget.writeBytes(buffer, 0, length); } deflater.reset(); } } + /** + * Creates a "framed packet" (packet which can be send and understood by a Minecraft client) + * from a server packet. + *

+ * Can be used if you want to store a raw buffer and send it later without the additional writing cost. + * Compression is applied if {@link MinecraftServer#getCompressionThreshold()} is greater than 0. + * + * @param serverPacket the server packet to write + * @return the framed packet from the server one + */ public static ByteBuf createFramedPacket(@NotNull ServerPacket serverPacket) { ByteBuf packetBuf = writePacket(serverPacket); From 871cb993b4f3c915bcd5dc7b5a7d9fa41c0d2560 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 14:14:55 +0100 Subject: [PATCH 37/39] Do not send packet to empty collection --- src/main/java/net/minestom/server/utils/PacketUtils.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 3a84ae086..20f755082 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -39,6 +39,9 @@ public final class PacketUtils { * @param packet the packet to send to the players */ public static void sendGroupedPacket(@NotNull Collection players, @NotNull ServerPacket packet) { + if (players.isEmpty()) + return; + final boolean success = PACKET_LISTENER_MANAGER.processServerPacket(packet, players); if (success) { final ByteBuf finalBuffer = createFramedPacket(packet); From 014bc8b0b5f4680f4b7971513ac9fd418e2f3636 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 14:21:20 +0100 Subject: [PATCH 38/39] Fixed DebugUtils --- .../minestom/server/utils/debug/DebugUtils.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/minestom/server/utils/debug/DebugUtils.java b/src/main/java/net/minestom/server/utils/debug/DebugUtils.java index bdd926e6f..44c43edc9 100644 --- a/src/main/java/net/minestom/server/utils/debug/DebugUtils.java +++ b/src/main/java/net/minestom/server/utils/debug/DebugUtils.java @@ -11,15 +11,11 @@ public final class DebugUtils { public final static Logger LOGGER = LoggerFactory.getLogger(DebugUtils.class); private static final String LINE_SEPARATOR = System.getProperty("line.separator"); - private static final int OFFSET = 2; // Used to do not show DebugUtils in the stack trace /** * Prints the current thread stack trace elements. - * - * @param maxLine the maximum number of stack trace element */ - public static synchronized void printStackTrace(int maxLine) { - maxLine += OFFSET; + public static synchronized void printStackTrace() { StackTraceElement[] elements = Thread.currentThread().getStackTrace(); StringBuilder stringBuilder = new StringBuilder(); @@ -27,7 +23,7 @@ public final class DebugUtils { stringBuilder.append("START STACKTRACE"); stringBuilder.append(LINE_SEPARATOR); - for (int i = OFFSET; i < maxLine; i++) { + for (int i = 0; i < Integer.MAX_VALUE; i++) { if (i >= elements.length) break; @@ -42,11 +38,4 @@ public final class DebugUtils { LOGGER.info(stringBuilder.toString()); } - /** - * Prints the current thread stack trace elements. - */ - public static synchronized void printStackTrace() { - printStackTrace(Integer.MAX_VALUE); - } - } From d7d610ffef63e4b47a5bea4f0871a9d452cf9237 Mon Sep 17 00:00:00 2001 From: themode Date: Fri, 20 Nov 2020 14:39:10 +0100 Subject: [PATCH 39/39] Fixed the player receiving multiple self position packet --- src/main/java/net/minestom/server/entity/Player.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index c03ca9e57..a5ce26e36 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -749,7 +749,12 @@ public class Player extends LivingEntity implements CommandSender { if (firstSpawn) { this.position = getRespawnPoint(); + this.cacheX = position.getX(); + this.cacheY = position.getY(); + this.cacheZ = position.getZ(); + updatePlayerPosition(); } + super.setInstance(instance); PlayerSpawnEvent spawnEvent = new PlayerSpawnEvent(this, instance, firstSpawn); @@ -1372,11 +1377,11 @@ public class Player extends LivingEntity implements CommandSender { *

* Can be altered by the {@link PlayerRespawnEvent#setRespawnPosition(Position)}. * - * @return the default respawn point + * @return a copy of the default respawn point */ @NotNull public Position getRespawnPoint() { - return respawnPoint; + return respawnPoint.copy(); } /**