Framed packet cleanup (#489)

src/main/java/net/minestom/server/Viewable.java

@@ -3,8 +3,10 @@ package net.minestom.server;
 import net.kyori.adventure.audience.Audience;
 import net.minestom.server.adventure.audience.PacketGroupingAudience;
 import net.minestom.server.entity.Player;
+import net.minestom.server.network.packet.FramedPacket;
 import net.minestom.server.network.packet.server.ServerPacket;
 import net.minestom.server.utils.PacketUtils;
+import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.Set;
@@ -60,6 +62,13 @@ public interface Viewable {
         PacketUtils.sendGroupedPacket(getViewers(), packet);
     }
 
+    @ApiStatus.Experimental
+    default void sendPacketToViewers(@NotNull FramedPacket framedPacket) {
+        for (Player viewer : getViewers()) {
+            viewer.sendPacket(framedPacket);
+        }
+    }
+
     /**
      * Sends multiple packets to all viewers.
      * <p>
@@ -85,6 +94,11 @@ public interface Viewable {
         sendPacketToViewers(packet);
     }
 
+    @ApiStatus.Experimental
+    default void sendPacketToViewersAndSelf(@NotNull FramedPacket framedPacket) {
+        sendPacketToViewers(framedPacket);
+    }
+
     /**
      * Gets the result of {@link #getViewers()} as an Adventure Audience.
      *

src/main/java/net/minestom/server/entity/Player.java

@@ -51,6 +51,7 @@ import net.minestom.server.message.Messenger;
 import net.minestom.server.network.ConnectionManager;
 import net.minestom.server.network.ConnectionState;
 import net.minestom.server.network.PlayerProvider;
+import net.minestom.server.network.packet.FramedPacket;
 import net.minestom.server.network.packet.client.ClientPlayPacket;
 import net.minestom.server.network.packet.client.play.ClientChatMessagePacket;
 import net.minestom.server.network.packet.server.ServerPacket;
@@ -274,7 +275,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
         // Recipes end
 
         // Tags
-        this.playerConnection.sendFramedPacket(TagsPacket.DEFAULT_TAGS);
+        this.playerConnection.sendPacket(TagsPacket.DEFAULT_TAGS);
 
         // Some client updates
         this.playerConnection.sendPacket(getPropertiesPacket()); // Send default properties
@@ -486,6 +487,12 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
         super.sendPacketToViewersAndSelf(packet);
     }
 
+    @Override
+    public void sendPacketToViewersAndSelf(@NotNull FramedPacket framedPacket) {
+        this.playerConnection.sendPacket(framedPacket);
+        super.sendPacketToViewersAndSelf(framedPacket);
+    }
+
     /**
      * Changes the player instance and load surrounding chunks if needed.
      * <p>
@@ -1271,6 +1278,11 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
         this.playerConnection.sendPacket(packet);
     }
 
+    @ApiStatus.Experimental
+    public void sendPacket(@NotNull FramedPacket framedPacket) {
+        this.playerConnection.sendPacket(framedPacket);
+    }
+
     /**
      * Gets if the player is online or not.
      *

src/main/java/net/minestom/server/instance/DynamicChunk.java

@@ -10,11 +10,8 @@ import net.minestom.server.entity.pathfinding.PFBlock;
 import net.minestom.server.instance.block.Block;
 import net.minestom.server.instance.block.BlockHandler;
 import net.minestom.server.network.packet.CachedPacket;
-import net.minestom.server.network.packet.FramedPacket;
 import net.minestom.server.network.packet.server.play.ChunkDataPacket;
 import net.minestom.server.network.packet.server.play.UpdateLightPacket;
-import net.minestom.server.network.player.PlayerConnection;
-import net.minestom.server.network.player.PlayerSocketConnection;
 import net.minestom.server.utils.ArrayUtils;
 import net.minestom.server.utils.MathUtils;
 import net.minestom.server.utils.Utils;
@@ -42,7 +39,6 @@ public class DynamicChunk extends Chunk {
     protected final Int2ObjectOpenHashMap<Block> entries = new Int2ObjectOpenHashMap<>();
     protected final Int2ObjectOpenHashMap<Block> tickableMap = new Int2ObjectOpenHashMap<>();
 
-    private volatile long lastChangeTime;
     private final CachedPacket chunkCache = new CachedPacket(this::createChunkPacket);
     private final CachedPacket lightCache = new CachedPacket(this::createLightPacket);
 
@@ -52,7 +48,8 @@ public class DynamicChunk extends Chunk {
 
     @Override
     public void setBlock(int x, int y, int z, @NotNull Block block) {
-        this.lastChangeTime = System.currentTimeMillis();
+        this.chunkCache.updateTimestamp();
+        this.lightCache.updateTimestamp();
         // Update pathfinder
         if (columnarSpace != null) {
             final ColumnarOcclusionFieldList columnarOcclusionFieldList = columnarSpace.occlusionFields();
@@ -121,35 +118,22 @@ public class DynamicChunk extends Chunk {
 
     @Override
     public long getLastChangeTime() {
-        return lastChangeTime;
+        return chunkCache.lastChange();
     }
 
     @Override
     public void sendChunk(@NotNull Player player) {
         if (!isLoaded()) return;
-        final PlayerConnection connection = player.getPlayerConnection();
+        player.sendPacket(lightCache.retrieve());
-        final long lastChange = getLastChangeTime();
+        player.sendPacket(chunkCache.retrieve());
-        final FramedPacket lightPacket = lightCache.retrieveFramedPacket(lastChange);
-        final FramedPacket chunkPacket = chunkCache.retrieveFramedPacket(lastChange);
-        if (connection instanceof PlayerSocketConnection) {
-            PlayerSocketConnection socketConnection = (PlayerSocketConnection) connection;
-            socketConnection.sendFramedPacket(lightPacket);
-            socketConnection.sendFramedPacket(chunkPacket);
-        } else {
-            connection.sendPacket(lightPacket.packet());
-            connection.sendPacket(chunkPacket.packet());
-        }
     }
 
     @Override
     public void sendChunk() {
         if (!isLoaded()) return;
         if (getViewers().isEmpty()) return;
-        final long lastChange = getLastChangeTime();
+        sendPacketToViewers(lightCache.retrieve());
-        final FramedPacket lightPacket = lightCache.retrieveFramedPacket(lastChange);
+        sendPacketToViewers(chunkCache.retrieve());
-        final FramedPacket chunkPacket = chunkCache.retrieveFramedPacket(lastChange);
-        sendPacketToViewers(lightPacket.packet());
-        sendPacketToViewers(chunkPacket.packet());
     }
 
     @NotNull

src/main/java/net/minestom/server/network/packet/CachedPacket.java

@@ -10,6 +10,7 @@ import java.util.function.Supplier;
 @ApiStatus.Internal
 public final class CachedPacket {
     private final Supplier<ServerPacket> supplier;
+    private volatile long lastChange;
     private volatile long packetTimestamp;
     private SoftReference<FramedPacket> packet;
 
@@ -17,7 +18,12 @@ public final class CachedPacket {
         this.supplier = supplier;
     }
 
-    public FramedPacket retrieveFramedPacket(long lastChange) {
+    public void updateTimestamp() {
+        this.lastChange = System.currentTimeMillis();
+    }
+
+    public FramedPacket retrieve() {
+        final long lastChange = this.lastChange;
         final long timestamp = packetTimestamp;
         final var ref = packet;
         FramedPacket cache = ref != null ? ref.get() : null;
@@ -28,4 +34,8 @@ public final class CachedPacket {
         }
         return cache;
     }
+
+    public long lastChange() {
+        return lastChange;
+    }
 }

src/main/java/net/minestom/server/network/player/PlayerConnection.java

@@ -103,7 +103,7 @@ public abstract class PlayerConnection {
     public abstract void sendPacket(@NotNull ServerPacket serverPacket, boolean skipTranslating);
 
     @ApiStatus.Experimental
-    public void sendFramedPacket(@NotNull FramedPacket framedPacket) {
+    public void sendPacket(@NotNull FramedPacket framedPacket) {
         this.sendPacket(framedPacket.packet());
     }
 

src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java

@@ -224,7 +224,7 @@ public class PlayerSocketConnection extends PlayerConnection {
     }
 
     @Override
-    public void sendFramedPacket(@NotNull FramedPacket framedPacket) {
+    public void sendPacket(@NotNull FramedPacket framedPacket) {
         write(framedPacket.body().duplicate().position(0));
     }
 

src/main/java/net/minestom/server/utils/PacketUtils.java

@@ -120,7 +120,7 @@ public final class PacketUtils {
             for (Player player : players) {
                 if (!player.isOnline() || !playerValidator.isValid(player))
                     continue;
-                player.getPlayerConnection().sendFramedPacket(framedPacket);
+                player.sendPacket(framedPacket);
             }
         } else {
             // Write the same packet for each individual players