Cleanup UpdateManager

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

@@ -1,23 +1,19 @@
 package net.minestom.server;
 
-import net.minestom.server.chat.ChatColor;
-import net.minestom.server.chat.ColoredText;
 import net.minestom.server.entity.EntityManager;
-import net.minestom.server.entity.Player;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.instance.InstanceManager;
-import net.minestom.server.network.ConnectionManager;
-import net.minestom.server.network.packet.server.play.KeepAlivePacket;
 import net.minestom.server.thread.PerGroupChunkProvider;
 import net.minestom.server.thread.ThreadProvider;
 import net.minestom.server.utils.thread.MinestomThread;
 import net.minestom.server.utils.validate.Check;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.List;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
-import java.util.function.DoubleConsumer;
+import java.util.function.LongConsumer;
 
 /**
  * Manager responsible for the server ticks.
@@ -27,37 +23,30 @@ import java.util.function.DoubleConsumer;
  */
 public final class UpdateManager {
 
-    private static final long KEEP_ALIVE_DELAY = 10_000;
-    private static final long KEEP_ALIVE_KICK = 30_000;
-    private static final ColoredText TIMEOUT_TEXT = ColoredText.of(ChatColor.RED + "Timeout");
-
     private final ExecutorService mainUpdate = new MinestomThread(1, MinecraftServer.THREAD_NAME_MAIN_UPDATE);
     private boolean stopRequested;
 
     private ThreadProvider threadProvider;
 
-    private final ConcurrentLinkedQueue<Runnable> tickStartCallbacks = new ConcurrentLinkedQueue<>();
+    private final ConcurrentLinkedQueue<LongConsumer> tickStartCallbacks = new ConcurrentLinkedQueue<>();
-    private final ConcurrentLinkedQueue<DoubleConsumer> tickEndCallbacks = new ConcurrentLinkedQueue<>();
+    private final ConcurrentLinkedQueue<LongConsumer> tickEndCallbacks = new ConcurrentLinkedQueue<>();
 
     {
         // DEFAULT THREAD PROVIDER
-        //threadProvider = new PerInstanceThreadProvider();
         threadProvider = new PerGroupChunkProvider();
     }
 
     /**
-     * Should only be created in MinecraftServer
+     * Should only be created in MinecraftServer.
      */
     protected UpdateManager() {
     }
 
     /**
-     * Starts the server loop in the update thread
+     * Starts the server loop in the update thread.
      */
     protected void start() {
         mainUpdate.execute(() -> {
-
-            final ConnectionManager connectionManager = MinecraftServer.getConnectionManager();
             final EntityManager entityManager = MinecraftServer.getEntityManager();
 
             final long tickDistance = MinecraftServer.TICK_MS * 1000000;
@@ -67,53 +56,19 @@ public final class UpdateManager {
                 final long tickStart = System.currentTimeMillis();
 
                 // Tick start callbacks
-                if (!tickStartCallbacks.isEmpty()) {
+                doTickCallback(tickStartCallbacks, tickStart);
-                    Runnable callback;
-                    while ((callback = tickStartCallbacks.poll()) != null) {
-                        callback.run();
-                    }
-                }
-
-                List<Future<?>> futures;
-
-                // Server tick (instance/chunk/entity)
-                // Synchronize with the update manager instance, like the signal for chunk load/unload
-                synchronized (this) {
-                    futures = threadProvider.update(tickStart);
-                }
 
                 // Waiting players update (newly connected clients waiting to get into the server)
                 entityManager.updateWaitingPlayers();
 
                 // Keep Alive Handling
-                final KeepAlivePacket keepAlivePacket = new KeepAlivePacket(tickStart);
+                entityManager.handleKeepAlive(tickStart);
-                for (Player player : connectionManager.getOnlinePlayers()) {
-                    final long lastKeepAlive = tickStart - player.getLastKeepAlive();
-                    if (lastKeepAlive > KEEP_ALIVE_DELAY && player.didAnswerKeepAlive()) {
-                        player.refreshKeepAlive(tickStart);
-                        player.getPlayerConnection().sendPacket(keepAlivePacket);
-                    } else if (lastKeepAlive >= KEEP_ALIVE_KICK) {
-                        player.kick(TIMEOUT_TEXT);
-                    }
-                }
-
-                for (final Future<?> future : futures) {
-                    try {
-                        future.get();
-                    } catch (Throwable e) {
-                        e.printStackTrace();
-                    }
-                }
 
+                // Server tick (chunks/entities)
+                serverTick(tickStart);
 
                 // Tick end callbacks
-                if (!tickEndCallbacks.isEmpty()) {
+                doTickCallback(tickEndCallbacks, (System.nanoTime() - currentTime) / 1000000);
-                    final double tickEnd = (System.nanoTime() - currentTime) / 1000000D;
-                    DoubleConsumer callback;
-                    while ((callback = tickEndCallbacks.poll()) != null) {
-                        callback.accept(tickEnd);
-                    }
-                }
 
                 // Sleep until next tick
                 final long sleepTime = Math.max(1, (tickDistance - (System.nanoTime() - currentTime)) / 1000000);
@@ -128,6 +83,44 @@ public final class UpdateManager {
         });
     }
 
+    /**
+     * Executes a server tick and returns only once all the futures are completed.
+     *
+     * @param tickStart the time of the tick in milliseconds
+     */
+    private void serverTick(long tickStart) {
+        List<Future<?>> futures;
+
+        // Server tick (instance/chunk/entity)
+        // Synchronize with the update manager instance, like the signal for chunk load/unload
+        synchronized (this) {
+            futures = threadProvider.update(tickStart);
+        }
+
+        for (final Future<?> future : futures) {
+            try {
+                future.get();
+            } catch (Throwable e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    /**
+     * Used to execute tick-related callbacks.
+     *
+     * @param callbacks the callbacks to execute
+     * @param value     the value to give to the consumers
+     */
+    private void doTickCallback(ConcurrentLinkedQueue<LongConsumer> callbacks, long value) {
+        if (!callbacks.isEmpty()) {
+            LongConsumer callback;
+            while ((callback = callbacks.poll()) != null) {
+                callback.accept(value);
+            }
+        }
+    }
+
     /**
      * Gets the current {@link ThreadProvider}.
      *
@@ -206,10 +199,12 @@ public final class UpdateManager {
 
     /**
      * Adds a callback executed at the start of the next server tick.
+     * <p>
+     * The long in the consumer represents the starting time (in ms) of the tick.
      *
      * @param callback the tick start callback
      */
-    public void addTickStartCallback(Runnable callback) {
+    public void addTickStartCallback(@NotNull LongConsumer callback) {
         this.tickStartCallbacks.add(callback);
     }
 
@@ -218,18 +213,18 @@ public final class UpdateManager {
      *
      * @param callback the callback to remove
      */
-    public void removeTickStartCallback(Runnable callback) {
+    public void removeTickStartCallback(@NotNull LongConsumer callback) {
         this.tickStartCallbacks.remove(callback);
     }
 
     /**
      * Adds a callback executed at the end of the next server tick.
      * <p>
-     * The double in the consumer represents the duration (in ms) of the tick.
+     * The long in the consumer represents the duration (in ms) of the tick.
      *
      * @param callback the tick end callback
      */
-    public void addTickEndCallback(DoubleConsumer callback) {
+    public void addTickEndCallback(@NotNull LongConsumer callback) {
         this.tickEndCallbacks.add(callback);
     }
 
@@ -238,12 +233,12 @@ public final class UpdateManager {
      *
      * @param callback the callback to remove
      */
-    public void removeTickEndCallback(DoubleConsumer callback) {
+    public void removeTickEndCallback(@NotNull LongConsumer callback) {
         this.tickEndCallbacks.remove(callback);
     }
 
     /**
-     * Stops the server loop
+     * Stops the server loop.
      */
     public void stop() {
         stopRequested = true;

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

@@ -1,10 +1,15 @@
 package net.minestom.server.entity;
 
 import net.minestom.server.MinecraftServer;
+import net.minestom.server.chat.ChatColor;
+import net.minestom.server.chat.ColoredText;
 import net.minestom.server.event.player.PlayerLoginEvent;
 import net.minestom.server.event.player.PlayerPreLoginEvent;
 import net.minestom.server.instance.Instance;
+import net.minestom.server.network.ConnectionManager;
+import net.minestom.server.network.packet.server.play.KeepAlivePacket;
 import net.minestom.server.utils.validate.Check;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.UUID;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -12,10 +17,16 @@ import java.util.function.Consumer;
 
 public final class EntityManager {
 
+    private static final ConnectionManager CONNECTION_MANAGER = MinecraftServer.getConnectionManager();
+
+    private static final long KEEP_ALIVE_DELAY = 10_000;
+    private static final long KEEP_ALIVE_KICK = 30_000;
+    private static final ColoredText TIMEOUT_TEXT = ColoredText.of(ChatColor.RED + "Timeout");
+
     private final ConcurrentLinkedQueue<Player> waitingPlayers = new ConcurrentLinkedQueue<>();
 
     /**
-     * Connect waiting players
+     * Connects waiting players.
      */
     public void updateWaitingPlayers() {
         // Connect waiting players
@@ -23,7 +34,25 @@ public final class EntityManager {
     }
 
     /**
-     * Add connected clients after the handshake (used to free the networking threads)
+     * Updates keep alive by checking the last keep alive packet and send a new one if needed.
+     *
+     * @param tickStart the time of the update in milliseconds, forwarded to the packet
+     */
+    public void handleKeepAlive(long tickStart) {
+        final KeepAlivePacket keepAlivePacket = new KeepAlivePacket(tickStart);
+        for (Player player : CONNECTION_MANAGER.getOnlinePlayers()) {
+            final long lastKeepAlive = tickStart - player.getLastKeepAlive();
+            if (lastKeepAlive > KEEP_ALIVE_DELAY && player.didAnswerKeepAlive()) {
+                player.refreshKeepAlive(tickStart);
+                player.getPlayerConnection().sendPacket(keepAlivePacket);
+            } else if (lastKeepAlive >= KEEP_ALIVE_KICK) {
+                player.kick(TIMEOUT_TEXT);
+            }
+        }
+    }
+
+    /**
+     * Adds connected clients after the handshake (used to free the networking threads).
      */
     private void waitingPlayersTick() {
         Player waitingPlayer;
@@ -41,14 +70,14 @@ public final class EntityManager {
     }
 
     /**
-     * Call the player initialization callbacks and the event {@link PlayerPreLoginEvent}.
+     * Calls the player initialization callbacks and the event {@link PlayerPreLoginEvent}.
      * If the {@link Player} hasn't been kicked, add him to the waiting list.
      * <p>
      * Can be considered as a pre-init thing.
      *
      * @param player the {@link Player} to add
      */
-    public void addWaitingPlayer(Player player) {
+    public void addWaitingPlayer(@NotNull Player player) {
 
         // Init player (register events)
         for (Consumer<Player> playerInitialization : MinecraftServer.getConnectionManager().getPlayerInitializations()) {