From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 19 Apr 2020 00:05:46 -0400 Subject: [PATCH] Fix Longstanding Broken behavior of PlayerJoinEvent For years, plugin developers have had to delay many things they do inside of the PlayerJoinEvent by 1 tick to make it actually work. This all boiled down to 1 reason why: The event fired before the player was fully ready and joined to the world! Additionally, if that player logged out on a vehicle, the event fired before the vehicle was even loaded, so that plugins had no access to the vehicle during this event either. This change finally fixes this issue, fully preparing the player into the world as a fully ready entity, vehicle included. There should be no plugins that break because of this change, but might improve consistency with other plugins instead. For example, if 2 plugins listens to this event, and the first one teleported the player in the event, then the 2nd plugin actually would be getting a valid player! This was very non deterministic. This change will ensure every plugin receives a deterministic result, and should no longer require 1 tick delays anymore. diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java index dbe60f5d24fb39be52c3cb8f933371b1626951df..b3ca4300b280a24f3ed2acaffdd6ae2cdffd140d 100644 --- a/src/main/java/net/minecraft/server/level/ChunkMap.java +++ b/src/main/java/net/minecraft/server/level/ChunkMap.java @@ -1584,7 +1584,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider }); } - protected void addEntity(Entity entity) { + public void addEntity(Entity entity) { // Paper - protected -> public org.spigotmc.AsyncCatcher.catchOp("entity track"); // Spigot // Paper start - ignore and warn about illegal addEntity calls instead of crashing server if (!entity.valid || entity.level != this.level || this.entityMap.containsKey(entity.getId())) { @@ -1593,6 +1593,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider .printStackTrace(); return; } + if (entity instanceof ServerPlayer && ((ServerPlayer) entity).supressTrackerForLogin) return; // Delay adding to tracker until after list packets // Paper end if (!(entity instanceof EnderDragonPart)) { EntityType entitytypes = entity.getType(); diff --git a/src/main/java/net/minecraft/server/level/ServerPlayer.java b/src/main/java/net/minecraft/server/level/ServerPlayer.java index aa979d17c264840ebd528708df3d6118e69fec68..75a095e0c2177dc1b46b080597ff8f12f1480acc 100644 --- a/src/main/java/net/minecraft/server/level/ServerPlayer.java +++ b/src/main/java/net/minecraft/server/level/ServerPlayer.java @@ -237,6 +237,7 @@ public class ServerPlayer extends Player implements ContainerListener { public double maxHealthCache; public boolean joining = true; public boolean sentListPacket = false; + public boolean supressTrackerForLogin = false; // Paper public Integer clientViewDistance; // CraftBukkit end public PlayerNaturallySpawnCreaturesEvent playerNaturallySpawnedEvent; // Paper diff --git a/src/main/java/net/minecraft/server/players/PlayerList.java b/src/main/java/net/minecraft/server/players/PlayerList.java index e043722436492140162940770c22be47690fb47f..454d60566743e02e7e55868c7bb45e30583dfa8f 100644 --- a/src/main/java/net/minecraft/server/players/PlayerList.java +++ b/src/main/java/net/minecraft/server/players/PlayerList.java @@ -272,6 +272,12 @@ public abstract class PlayerList { this.playersByUUID.put(player.getUUID(), player); // this.sendAll(new PacketPlayOutPlayerInfo(PacketPlayOutPlayerInfo.EnumPlayerInfoAction.ADD_PLAYER, new EntityPlayer[]{entityplayer})); // CraftBukkit - replaced with loop below + // Paper start - correctly register player BEFORE PlayerJoinEvent, so the entity is valid and doesn't require tick delay hacks + player.supressTrackerForLogin = true; + worldserver1.addNewPlayer(player); + this.server.getCustomBossEvents().onPlayerConnect(player); // see commented out section below worldserver.addPlayerJoin(entityplayer); + mountSavedVehicle(player, worldserver1, nbttagcompound); + // Paper end // CraftBukkit start PlayerJoinEvent playerJoinEvent = new org.bukkit.event.player.PlayerJoinEvent(cserver.getPlayer(player), PaperAdventure.asAdventure(chatmessage)); // Paper - Adventure cserver.getPluginManager().callEvent(playerJoinEvent); @@ -307,6 +313,8 @@ public abstract class PlayerList { player.connection.send(new ClientboundPlayerInfoPacket(ClientboundPlayerInfoPacket.Action.ADD_PLAYER, new ServerPlayer[] { entityplayer1})); } player.sentListPacket = true; + player.supressTrackerForLogin = false; // Paper + ((ServerLevel)player.level).getChunkSource().chunkMap.addEntity(player); // Paper - track entity now // CraftBukkit end player.connection.send(new ClientboundSetEntityDataPacket(player.getId(), player.getEntityData(), true)); // CraftBukkit - BungeeCord#2321, send complete data to self on spawn @@ -332,6 +340,11 @@ public abstract class PlayerList { playerconnection.send(new ClientboundUpdateMobEffectPacket(player.getId(), mobeffect)); } + // Paper start - move vehicle into method so it can be called above - short circuit around that code + onPlayerJoinFinish(player, worldserver1, s1); + } + private void mountSavedVehicle(ServerPlayer entityplayer, ServerLevel worldserver1, CompoundTag nbttagcompound) { + // Paper end if (nbttagcompound != null && nbttagcompound.contains("RootVehicle", 10)) { CompoundTag nbttagcompound1 = nbttagcompound.getCompound("RootVehicle"); // CraftBukkit start @@ -354,20 +367,20 @@ public abstract class PlayerList { Entity entity1; if (entity.getUUID().equals(uuid)) { - player.startRiding(entity, true); + entityplayer.startRiding(entity, true); } else { iterator1 = entity.getIndirectPassengers().iterator(); while (iterator1.hasNext()) { entity1 = (Entity) iterator1.next(); if (entity1.getUUID().equals(uuid)) { - player.startRiding(entity1, true); + entityplayer.startRiding(entity1, true); break; } } } - if (!player.isPassenger()) { + if (!entityplayer.isPassenger()) { PlayerList.LOGGER.warn("Couldn't reattach entity to player"); worldserver1.despawn(entity); iterator1 = entity.getIndirectPassengers().iterator(); @@ -380,16 +393,20 @@ public abstract class PlayerList { } } - player.initMenu(); + // Paper start + } + public void onPlayerJoinFinish(ServerPlayer entityplayer, ServerLevel worldserver1, String s1) { + // Paper end + entityplayer.initMenu(); // Paper start - Add to collideRule team if needed final Scoreboard scoreboard = this.getServer().getLevel(Level.OVERWORLD).getScoreboard(); final PlayerTeam collideRuleTeam = scoreboard.getTeam(collideRuleTeamName); - if (this.collideRuleTeamName != null && collideRuleTeam != null && player.getTeam() == null) { - scoreboard.addPlayerToTeam(player.getScoreboardName(), collideRuleTeam); + if (this.collideRuleTeamName != null && collideRuleTeam != null && entityplayer.getTeam() == null) { + scoreboard.addPlayerToTeam(entityplayer.getScoreboardName(), collideRuleTeam); } // Paper end // CraftBukkit - Moved from above, added world - PlayerList.LOGGER.info("{}[{}] logged in with entity id {} at ([{}]{}, {}, {})", player.getName().getString(), s1, player.getId(), worldserver1.worldDataServer.getLevelName(), player.getX(), player.getY(), player.getZ()); + PlayerList.LOGGER.info("{}[{}] logged in with entity id {} at ([{}]{}, {}, {})", entityplayer.getName().getString(), s1, entityplayer.getId(), worldserver1.worldDataServer.getLevelName(), entityplayer.getX(), entityplayer.getY(), entityplayer.getZ()); } public void updateEntireScoreboard(ServerScoreboard scoreboard, ServerPlayer player) {