From acee29c20a5e97dcc05c91e80b0d4cac781b2334 Mon Sep 17 00:00:00 2001 From: themode Date: Mon, 14 Mar 2022 19:01:48 +0100 Subject: [PATCH] Fix player position being wrong inside tests --- .../minestom.common-conventions.gradle.kts | 1 + .../net/minestom/server/entity/Player.java | 4 +-- .../server/network/ConnectionManager.java | 13 +++++-- .../minestom/server/utils/PacketUtils.java | 16 ++------- .../minestom/server/utils/PropertyUtils.java | 18 ++++++++++ .../server/utils/debug/DebugUtils.java | 4 +++ .../java/net/minestom/server/InsideTest.java | 13 +++++++ .../minestom/server/api/TestConnection.java | 8 +---- .../server/api/TestConnectionImpl.java | 35 +++++++------------ .../entity/EntityInstanceIntegrationTest.java | 4 +-- 10 files changed, 66 insertions(+), 50 deletions(-) create mode 100644 src/main/java/net/minestom/server/utils/PropertyUtils.java create mode 100644 src/test/java/net/minestom/server/InsideTest.java diff --git a/build-logic/src/main/kotlin/minestom.common-conventions.gradle.kts b/build-logic/src/main/kotlin/minestom.common-conventions.gradle.kts index 88dd61b90..4065014d4 100644 --- a/build-logic/src/main/kotlin/minestom.common-conventions.gradle.kts +++ b/build-logic/src/main/kotlin/minestom.common-conventions.gradle.kts @@ -23,5 +23,6 @@ tasks { useJUnitPlatform() // Viewable packets make tracking harder. Could be re-enabled later. jvmArgs("-Dminestom.viewable-packet=false") + jvmArgs("-Dminestom.inside-test=true") } } \ No newline at end of file diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index c17817f3f..cd8511266 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -236,7 +236,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, * * @param spawnInstance the player spawn instance (defined in {@link PlayerLoginEvent}) */ - public void UNSAFE_init(@NotNull Instance spawnInstance) { + public CompletableFuture UNSAFE_init(@NotNull Instance spawnInstance) { this.dimensionType = spawnInstance.getDimensionType(); NBTCompound nbt = NBT.Compound(Map.of( @@ -301,7 +301,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, refreshHealth(); // Heal and send health packet refreshAbilities(); // Send abilities packet - setInstance(spawnInstance); + return setInstance(spawnInstance); } /** diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index 012bf081f..84aeffb57 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -15,6 +15,7 @@ import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.network.player.PlayerSocketConnection; import net.minestom.server.utils.StringUtils; import net.minestom.server.utils.async.AsyncUtils; +import net.minestom.server.utils.debug.DebugUtils; import net.minestom.server.utils.validate.Check; import org.jctools.queues.MessagePassingQueue; import org.jctools.queues.MpscUnboundedArrayQueue; @@ -22,6 +23,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; import java.util.function.Function; @@ -213,8 +215,8 @@ public final class ConnectionManager { * @param player the player * @param register true to register the newly created player in {@link ConnectionManager} lists */ - public void startPlayState(@NotNull Player player, boolean register) { - AsyncUtils.runAsync(() -> { + public CompletableFuture startPlayState(@NotNull Player player, boolean register) { + return AsyncUtils.runAsync(() -> { final PlayerConnection playerConnection = player.getPlayerConnection(); // Call pre login event AsyncPlayerPreLoginEvent asyncPlayerPreLoginEvent = new AsyncPlayerPreLoginEvent(player); @@ -290,7 +292,12 @@ public final class ConnectionManager { final Instance spawningInstance = loginEvent.getSpawningInstance(); Check.notNull(spawningInstance, "You need to specify a spawning instance in the PlayerLoginEvent"); // Spawn the player at Player#getRespawnPoint - waitingPlayer.UNSAFE_init(spawningInstance); + if (DebugUtils.INSIDE_TEST) { + // Required to get the exact moment the player spawns + waitingPlayer.UNSAFE_init(spawningInstance).join(); + } else { + waitingPlayer.UNSAFE_init(spawningInstance); + } }); } diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 8e184618a..d970f86e3 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -49,9 +49,9 @@ import java.util.zip.Inflater; public final class PacketUtils { private static final LocalCache LOCAL_DEFLATER = LocalCache.of(Deflater::new); - public static final boolean GROUPED_PACKET = getBoolean("minestom.grouped-packet", true); - public static final boolean CACHED_PACKET = getBoolean("minestom.cached-packet", true); - public static final boolean VIEWABLE_PACKET = getBoolean("minestom.viewable-packet", true); + public static final boolean GROUPED_PACKET = PropertyUtils.getBoolean("minestom.grouped-packet", true); + public static final boolean CACHED_PACKET = PropertyUtils.getBoolean("minestom.cached-packet", true); + public static final boolean VIEWABLE_PACKET = PropertyUtils.getBoolean("minestom.viewable-packet", true); // Viewable packets private static final Cache VIEWABLE_STORAGE_MAP = Caffeine.newBuilder().weakKeys().build(); @@ -339,14 +339,4 @@ public final class PacketUtils { // TODO for non-socket connection } } - - private static boolean getBoolean(String name, boolean defaultValue) { - boolean result = defaultValue; - try { - final String value = System.getProperty(name); - if (value != null) result = Boolean.parseBoolean(value); - } catch (IllegalArgumentException | NullPointerException ignored) { - } - return result; - } } diff --git a/src/main/java/net/minestom/server/utils/PropertyUtils.java b/src/main/java/net/minestom/server/utils/PropertyUtils.java new file mode 100644 index 000000000..aef386e44 --- /dev/null +++ b/src/main/java/net/minestom/server/utils/PropertyUtils.java @@ -0,0 +1,18 @@ +package net.minestom.server.utils; + +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class PropertyUtils { + private PropertyUtils() {} + + public static boolean getBoolean(String name, boolean defaultValue) { + boolean result = defaultValue; + try { + final String value = System.getProperty(name); + if (value != null) result = Boolean.parseBoolean(value); + } catch (IllegalArgumentException | NullPointerException ignored) { + } + return result; + } +} 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 44c43edc9..0b6fc227d 100644 --- a/src/main/java/net/minestom/server/utils/debug/DebugUtils.java +++ b/src/main/java/net/minestom/server/utils/debug/DebugUtils.java @@ -1,12 +1,16 @@ package net.minestom.server.utils.debug; +import net.minestom.server.utils.PropertyUtils; +import org.jetbrains.annotations.ApiStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Utils class useful for debugging purpose. */ +@ApiStatus.Internal public final class DebugUtils { + public static boolean INSIDE_TEST = PropertyUtils.getBoolean("minestom.inside-test", false); public final static Logger LOGGER = LoggerFactory.getLogger(DebugUtils.class); diff --git a/src/test/java/net/minestom/server/InsideTest.java b/src/test/java/net/minestom/server/InsideTest.java new file mode 100644 index 000000000..4fd8bc46b --- /dev/null +++ b/src/test/java/net/minestom/server/InsideTest.java @@ -0,0 +1,13 @@ +package net.minestom.server; + +import net.minestom.server.utils.debug.DebugUtils; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class InsideTest { + @Test + public void inside() { + assertTrue(DebugUtils.INSIDE_TEST); + } +} diff --git a/src/test/java/net/minestom/server/api/TestConnection.java b/src/test/java/net/minestom/server/api/TestConnection.java index a65874466..f79beeaf2 100644 --- a/src/test/java/net/minestom/server/api/TestConnection.java +++ b/src/test/java/net/minestom/server/api/TestConnection.java @@ -7,15 +7,9 @@ import net.minestom.server.network.packet.server.ServerPacket; import org.jetbrains.annotations.NotNull; import java.util.concurrent.CompletableFuture; -import java.util.function.Consumer; public interface TestConnection { - @NotNull CompletableFuture<@NotNull Player> connect(@NotNull Instance instance, @NotNull Pos pos, @NotNull Consumer loginCallback); - - default @NotNull CompletableFuture<@NotNull Player> connect(@NotNull Instance instance, @NotNull Pos pos) { - return connect(instance, pos, (player) -> { - }); - } + @NotNull CompletableFuture<@NotNull Player> connect(@NotNull Instance instance, @NotNull Pos pos); @NotNull Collector trackIncoming(@NotNull Class type); diff --git a/src/test/java/net/minestom/server/api/TestConnectionImpl.java b/src/test/java/net/minestom/server/api/TestConnectionImpl.java index 1e0177c40..286563bf0 100644 --- a/src/test/java/net/minestom/server/api/TestConnectionImpl.java +++ b/src/test/java/net/minestom/server/api/TestConnectionImpl.java @@ -3,7 +3,6 @@ package net.minestom.server.api; import net.minestom.server.ServerProcess; import net.minestom.server.coordinate.Pos; import net.minestom.server.entity.Player; -import net.minestom.server.event.EventListener; import net.minestom.server.event.player.PlayerLoginEvent; import net.minestom.server.instance.Instance; import net.minestom.server.network.packet.server.SendablePacket; @@ -17,8 +16,6 @@ import java.util.List; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; final class TestConnectionImpl implements TestConnection { private final Env env; @@ -27,32 +24,24 @@ final class TestConnectionImpl implements TestConnection { private final List> incomingTrackers = new CopyOnWriteArrayList<>(); - public TestConnectionImpl(Env env) { + TestConnectionImpl(Env env) { this.env = env; this.process = env.process(); } @Override - public @NotNull CompletableFuture connect(@NotNull Instance instance, @NotNull Pos pos, @NotNull Consumer loginCallback) { - AtomicReference> listenerRef = new AtomicReference<>(); - var listener = EventListener.builder(PlayerLoginEvent.class) - .handler(event -> { - if (event.getPlayer().getPlayerConnection() == playerConnection) { - event.setSpawningInstance(instance); - event.getPlayer().setRespawnPoint(pos); - process.eventHandler().removeListener(listenerRef.get()); - loginCallback.accept(event.getPlayer()); - } - }).build(); - listenerRef.set(listener); - process.eventHandler().addListener(listener); + public @NotNull CompletableFuture connect(@NotNull Instance instance, @NotNull Pos pos) { + Player player = new Player(UUID.randomUUID(), "RandName", playerConnection); + player.eventNode().addListener(PlayerLoginEvent.class, event -> { + event.setSpawningInstance(instance); + event.getPlayer().setRespawnPoint(pos); + }); - var player = new Player(UUID.randomUUID(), "RandName", playerConnection); - process.connection().startPlayState(player, true); - while (player.getInstance() != instance) { // TODO replace with proper future - env.tick(); - } - return CompletableFuture.completedFuture(player); + return process.connection().startPlayState(player, true) + .thenApply(unused -> { + process.connection().updateWaitingPlayers(); + return player; + }); } @Override diff --git a/src/test/java/net/minestom/server/entity/EntityInstanceIntegrationTest.java b/src/test/java/net/minestom/server/entity/EntityInstanceIntegrationTest.java index cac7a17a0..b51072a34 100644 --- a/src/test/java/net/minestom/server/entity/EntityInstanceIntegrationTest.java +++ b/src/test/java/net/minestom/server/entity/EntityInstanceIntegrationTest.java @@ -25,9 +25,9 @@ public class EntityInstanceIntegrationTest { @Test public void playerJoin(Env env) { var instance = env.createFlatInstance(); - var connection = env.createConnection(); - var player = connection.connect(instance, new Pos(0, 42, 0)).join(); + var player = env.createPlayer(instance, new Pos(0, 42, 0)); assertEquals(instance, player.getInstance()); + assertEquals(new Pos(0, 42, 0), player.getPosition()); } @Test