From 7ae91a40ecc2ecaf82ed33e8cafa2aa46ab7b458 Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Mon, 16 Aug 2021 20:39:29 -0400 Subject: [PATCH] Ensure every packet is ran on the same thread per player (#2473) This removes a lot of concurrency checking that needs to be done, because there should be no way two packets can be handled at the same time. --- connector/pom.xml | 6 +- .../network/ConnectorServerEventHandler.java | 5 +- .../network/UpstreamPacketHandler.java | 12 ++-- .../network/session/GeyserSession.java | 72 +++++++------------ .../network/session/cache/EntityCache.java | 21 +++--- .../translators/PacketTranslatorRegistry.java | 8 ++- .../bedrock/BedrockAnimateTranslator.java | 2 +- .../BedrockContainerCloseTranslator.java | 40 +++++------ ...BedrockInventoryTransactionTranslator.java | 36 +++++----- .../BedrockItemStackRequestTranslator.java | 2 +- .../entity/BedrockEntityEventTranslator.java | 12 ++-- .../java/JavaRespawnTranslator.java | 8 +-- .../JavaPlayerChangeHeldItemTranslator.java | 14 ++-- .../window/JavaCloseWindowTranslator.java | 7 +- .../java/window/JavaOpenWindowTranslator.java | 58 ++++++++------- .../java/window/JavaSetSlotTranslator.java | 58 ++++++++------- .../window/JavaWindowItemsTranslator.java | 30 ++++---- .../window/JavaWindowPropertyTranslator.java | 16 ++--- .../entity/SkullBlockEntityTranslator.java | 2 +- .../connector/utils/InventoryUtils.java | 22 +++--- 20 files changed, 198 insertions(+), 233 deletions(-) diff --git a/connector/pom.xml b/connector/pom.xml index d32e01806..5e311b5b7 100644 --- a/connector/pom.xml +++ b/connector/pom.xml @@ -13,8 +13,8 @@ 4.8.0 8.5.2 - 2.10.2 - 4.1.59.Final + 2.12.4 + 4.1.66.Final @@ -167,7 +167,7 @@ com.github.GeyserMC PacketLib - 25eb4c4 + 86c9c38 compile diff --git a/connector/src/main/java/org/geysermc/connector/network/ConnectorServerEventHandler.java b/connector/src/main/java/org/geysermc/connector/network/ConnectorServerEventHandler.java index 67fbdbbfd..ccaaac082 100644 --- a/connector/src/main/java/org/geysermc/connector/network/ConnectorServerEventHandler.java +++ b/connector/src/main/java/org/geysermc/connector/network/ConnectorServerEventHandler.java @@ -30,7 +30,9 @@ import com.nukkitx.protocol.bedrock.BedrockServerEventHandler; import com.nukkitx.protocol.bedrock.BedrockServerSession; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.DefaultEventLoopGroup; import io.netty.channel.socket.DatagramPacket; +import io.netty.util.concurrent.DefaultThreadFactory; import org.geysermc.connector.GeyserConnector; import org.geysermc.connector.common.ping.GeyserPingInfo; import org.geysermc.connector.configuration.GeyserConfiguration; @@ -56,6 +58,7 @@ public class ConnectorServerEventHandler implements BedrockServerEventHandler { private static final int MAGIC_RAKNET_LENGTH = 338; private final GeyserConnector connector; + private final DefaultEventLoopGroup eventLoopGroup = new DefaultEventLoopGroup(new DefaultThreadFactory("Geyser player thread")); public ConnectorServerEventHandler(GeyserConnector connector) { this.connector = connector; @@ -162,7 +165,7 @@ public class ConnectorServerEventHandler implements BedrockServerEventHandler { public void onSessionCreation(BedrockServerSession bedrockServerSession) { bedrockServerSession.setLogging(true); bedrockServerSession.setCompressionLevel(connector.getConfig().getBedrock().getCompressionLevel()); - bedrockServerSession.setPacketHandler(new UpstreamPacketHandler(connector, new GeyserSession(connector, bedrockServerSession))); + bedrockServerSession.setPacketHandler(new UpstreamPacketHandler(connector, new GeyserSession(connector, bedrockServerSession, eventLoopGroup.next()))); // Set the packet codec to default just in case we need to send disconnect packets. bedrockServerSession.setPacketCodec(BedrockProtocol.DEFAULT_BEDROCK_CODEC); } diff --git a/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java b/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java index 9213579ed..d008b98ab 100644 --- a/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java +++ b/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java @@ -52,6 +52,11 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { return PacketTranslatorRegistry.BEDROCK_TRANSLATOR.translate(packet.getClass(), packet, session); } + @Override + boolean defaultHandler(BedrockPacket packet) { + return translateAndDefault(packet); + } + @Override public boolean handle(LoginPacket loginPacket) { BedrockPacketCodec packetCodec = BedrockProtocol.getBedrockCodec(loginPacket.getProtocolVersion()); @@ -156,7 +161,7 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { @Override public boolean handle(ModalFormResponsePacket packet) { - session.getFormCache().handleResponse(packet); + session.getEventLoop().execute(() -> session.getFormCache().handleResponse(packet)); return true; } @@ -209,11 +214,6 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { return translateAndDefault(packet); } - @Override - boolean defaultHandler(BedrockPacket packet) { - return translateAndDefault(packet); - } - @Override public boolean handle(ResourcePackChunkRequestPacket packet) { ResourcePackChunkDataPacket data = new ResourcePackChunkDataPacket(); diff --git a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java index 74ff369de..6bd3fca90 100644 --- a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java +++ b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java @@ -57,9 +57,9 @@ import com.nukkitx.protocol.bedrock.data.command.CommandPermission; import com.nukkitx.protocol.bedrock.data.entity.EntityData; import com.nukkitx.protocol.bedrock.data.entity.EntityFlag; import com.nukkitx.protocol.bedrock.packet.*; +import io.netty.channel.EventLoop; import it.unimi.dsi.fastutil.ints.*; import it.unimi.dsi.fastutil.longs.Long2ObjectMap; -import it.unimi.dsi.fastutil.longs.Long2ObjectMaps; import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.ObjectIterator; @@ -102,7 +102,10 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.*; -import java.util.concurrent.*; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @Getter @@ -110,6 +113,10 @@ public class GeyserSession implements CommandSender { private final GeyserConnector connector; private final UpstreamSession upstream; + /** + * The loop where all packets and ticking is processed to prevent concurrency issues. + */ + private final EventLoop eventLoop; private TcpClientSession downstream; @Setter private AuthData authData; @@ -158,11 +165,6 @@ public class GeyserSession implements CommandSender { @Getter(AccessLevel.NONE) private final AtomicInteger itemNetId = new AtomicInteger(2); - @Getter(AccessLevel.NONE) - private final Object inventoryLock = new Object(); - @Getter(AccessLevel.NONE) - private CompletableFuture inventoryFuture; - @Setter private ScheduledFuture craftingGridFuture; @@ -183,8 +185,8 @@ public class GeyserSession implements CommandSender { @Setter private ItemMappings itemMappings; - private final Map skullCache = new ConcurrentHashMap<>(); - private final Long2ObjectMap storedMaps = Long2ObjectMaps.synchronize(new Long2ObjectOpenHashMap<>()); + private final Map skullCache = new Object2ObjectOpenHashMap<>(); + private final Long2ObjectMap storedMaps = new Long2ObjectOpenHashMap<>(); /** * Stores the map between Java and Bedrock biome network IDs. @@ -426,9 +428,10 @@ public class GeyserSession implements CommandSender { private MinecraftProtocol protocol; - public GeyserSession(GeyserConnector connector, BedrockServerSession bedrockServerSession) { + public GeyserSession(GeyserConnector connector, BedrockServerSession bedrockServerSession, EventLoop eventLoop) { this.connector = connector; this.upstream = new UpstreamSession(bedrockServerSession); + this.eventLoop = eventLoop; this.advancementsCache = new AdvancementsCache(this); this.bookEditCache = new BookEditCache(this); @@ -447,7 +450,6 @@ public class GeyserSession implements CommandSender { this.playerInventory = new PlayerInventory(); this.openInventory = null; - this.inventoryFuture = CompletableFuture.completedFuture(null); this.craftingRecipes = new Int2ObjectOpenHashMap<>(); this.unlockedRecipes = new ObjectOpenHashSet<>(); this.lastRecipeNetId = new AtomicInteger(1); @@ -664,7 +666,7 @@ public class GeyserSession implements CommandSender { boolean floodgate = this.remoteAuthType == AuthType.FLOODGATE; // Start ticking - tickThread = connector.getGeneralThreadPool().scheduleAtFixedRate(this::tick, 50, 50, TimeUnit.MILLISECONDS); + tickThread = eventLoop.scheduleAtFixedRate(this::tick, 50, 50, TimeUnit.MILLISECONDS); downstream = new TcpClientSession(this.remoteAddress, this.remotePort, protocol); disableSrvResolving(); @@ -1095,39 +1097,6 @@ public class GeyserSession implements CommandSender { upstream.sendPacket(startGamePacket); } - /** - * Adds a new inventory task. - * Inventory tasks are executed one at a time, in order. - * - * @param task the task to run - */ - public void addInventoryTask(Runnable task) { - synchronized (inventoryLock) { - inventoryFuture = inventoryFuture.thenRun(task).exceptionally(throwable -> { - GeyserConnector.getInstance().getLogger().error("Error processing inventory task", throwable.getCause()); - return null; - }); - } - } - - /** - * Adds a new inventory task with a delay. - * The delay is achieved by scheduling with the Geyser general thread pool. - * Inventory tasks are executed one at a time, in order. - * - * @param task the delayed task to run - * @param delayMillis delay in milliseconds - */ - public void addInventoryTask(Runnable task, long delayMillis) { - synchronized (inventoryLock) { - Executor delayedExecutor = command -> GeyserConnector.getInstance().getGeneralThreadPool().schedule(command, delayMillis, TimeUnit.MILLISECONDS); - inventoryFuture = inventoryFuture.thenRunAsync(task, delayedExecutor).exceptionally(throwable -> { - GeyserConnector.getInstance().getLogger().error("Error processing inventory task", throwable.getCause()); - return null; - }); - } - } - /** * @return the next Bedrock item network ID to use for a new item */ @@ -1229,7 +1198,18 @@ public class GeyserSession implements CommandSender { * @param packet the java edition packet from MCProtocolLib */ public void sendDownstreamPacket(Packet packet) { - if (downstream != null && (protocol.getSubProtocol().equals(SubProtocol.GAME) || packet.getClass() == LoginPluginResponsePacket.class)) { + if (!closed && this.downstream != null) { + EventLoop eventLoop = this.downstream.getChannel().eventLoop(); + if (eventLoop.inEventLoop()) { + sendDownstreamPacket0(packet); + } else { + eventLoop.execute(() -> sendDownstreamPacket0(packet)); + } + } + } + + private void sendDownstreamPacket0(Packet packet) { + if (protocol.getSubProtocol().equals(SubProtocol.GAME) || packet.getClass() == LoginPluginResponsePacket.class) { downstream.send(packet); } else { connector.getLogger().debug("Tried to send downstream packet " + packet.getClass().getSimpleName() + " before connected to the server"); diff --git a/connector/src/main/java/org/geysermc/connector/network/session/cache/EntityCache.java b/connector/src/main/java/org/geysermc/connector/network/session/cache/EntityCache.java index 4d6750bc0..a9d856b9e 100644 --- a/connector/src/main/java/org/geysermc/connector/network/session/cache/EntityCache.java +++ b/connector/src/main/java/org/geysermc/connector/network/session/cache/EntityCache.java @@ -26,6 +26,8 @@ package org.geysermc.connector.network.session.cache; import it.unimi.dsi.fastutil.longs.*; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; import lombok.Getter; import org.geysermc.connector.entity.Tickable; @@ -44,15 +46,15 @@ public class EntityCache { private final GeyserSession session; @Getter - private Long2ObjectMap entities = Long2ObjectMaps.synchronize(new Long2ObjectOpenHashMap<>()); + private final Long2ObjectMap entities = new Long2ObjectOpenHashMap<>(); /** * A list of all entities that must be ticked. */ - private final List tickableEntities = Collections.synchronizedList(new ArrayList<>()); - private Long2LongMap entityIdTranslations = Long2LongMaps.synchronize(new Long2LongOpenHashMap()); - private Map playerEntities = Collections.synchronizedMap(new HashMap<>()); - private Map bossBars = Collections.synchronizedMap(new HashMap<>()); - private final Long2LongMap cachedPlayerEntityLinks = Long2LongMaps.synchronize(new Long2LongOpenHashMap()); + private final List tickableEntities = new ObjectArrayList<>(); + private final Long2LongMap entityIdTranslations = new Long2LongOpenHashMap(); + private final Map playerEntities = new Object2ObjectOpenHashMap<>(); + private final Map bossBars = new Object2ObjectOpenHashMap<>(); + private final Long2LongMap cachedPlayerEntityLinks = new Long2LongOpenHashMap(); @Getter private final AtomicLong nextEntityId = new AtomicLong(2L); @@ -156,13 +158,6 @@ public class EntityCache { bossBars.values().forEach(BossBar::updateBossBar); } - public void clear() { - entities = null; - entityIdTranslations = null; - playerEntities = null; - bossBars = null; - } - public long getCachedPlayerEntityLink(long playerId) { return cachedPlayerEntityLinks.remove(playerId); } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/PacketTranslatorRegistry.java b/connector/src/main/java/org/geysermc/connector/network/translators/PacketTranslatorRegistry.java index d8f726223..c486c2521 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/PacketTranslatorRegistry.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/PacketTranslatorRegistry.java @@ -29,6 +29,7 @@ import com.github.steveice10.mc.protocol.packet.ingame.server.ServerPlayerListDa import com.github.steveice10.mc.protocol.packet.ingame.server.world.ServerUpdateLightPacket; import com.github.steveice10.packetlib.packet.Packet; import com.nukkitx.protocol.bedrock.BedrockPacket; +import io.netty.channel.EventLoop; import it.unimi.dsi.fastutil.objects.ObjectArrayList; import org.geysermc.common.PlatformType; import org.geysermc.connector.GeyserConnector; @@ -89,7 +90,12 @@ public class PacketTranslatorRegistry { try { PacketTranslator

translator = (PacketTranslator

) translators.get(clazz); if (translator != null) { - translator.translate(packet, session); + EventLoop eventLoop = session.getEventLoop(); + if (eventLoop.inEventLoop()) { + translator.translate(packet, session); + } else { + eventLoop.execute(() -> translator.translate(packet, session)); + } return true; } else { if ((GeyserConnector.getInstance().getPlatformType() != PlatformType.STANDALONE || !(packet instanceof BedrockPacket)) && !IGNORED_PACKETS.contains(clazz)) { diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockAnimateTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockAnimateTranslator.java index 6e97a5182..0bae5261c 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockAnimateTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockAnimateTranslator.java @@ -48,7 +48,7 @@ public class BedrockAnimateTranslator extends PacketTranslator { switch (packet.getAction()) { case SWING_ARM: // Delay so entity damage can be processed first - session.getConnector().getGeneralThreadPool().schedule(() -> + session.getEventLoop().schedule(() -> session.sendDownstreamPacket(new ClientPlayerSwingArmPacket(Hand.MAIN_HAND)), 25, TimeUnit.MILLISECONDS diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java index 21bc1e437..88c2206d5 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java @@ -39,29 +39,27 @@ public class BedrockContainerCloseTranslator extends PacketTranslator { - byte windowId = packet.getId(); + byte windowId = packet.getId(); - //Client wants close confirmation - session.sendUpstreamPacket(packet); - session.setClosingInventory(false); + //Client wants close confirmation + session.sendUpstreamPacket(packet); + session.setClosingInventory(false); - if (windowId == -1 && session.getOpenInventory() instanceof MerchantContainer) { - // 1.16.200 - window ID is always -1 sent from Bedrock - windowId = (byte) session.getOpenInventory().getId(); + if (windowId == -1 && session.getOpenInventory() instanceof MerchantContainer) { + // 1.16.200 - window ID is always -1 sent from Bedrock + windowId = (byte) session.getOpenInventory().getId(); + } + + Inventory openInventory = session.getOpenInventory(); + if (openInventory != null) { + if (windowId == openInventory.getId()) { + ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(windowId); + session.sendDownstreamPacket(closeWindowPacket); + InventoryUtils.closeInventory(session, windowId, false); + } else if (openInventory.isPending()) { + InventoryUtils.displayInventory(session, openInventory); + openInventory.setPending(false); } - - Inventory openInventory = session.getOpenInventory(); - if (openInventory != null) { - if (windowId == openInventory.getId()) { - ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(windowId); - session.sendDownstreamPacket(closeWindowPacket); - InventoryUtils.closeInventory(session, windowId, false); - } else if (openInventory.isPending()) { - InventoryUtils.displayInventory(session, openInventory); - openInventory.setPending(false); - } - } - }); + } } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockInventoryTransactionTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockInventoryTransactionTranslator.java index d519f2682..f901e128c 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockInventoryTransactionTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockInventoryTransactionTranslator.java @@ -83,26 +83,24 @@ public class BedrockInventoryTransactionTranslator extends PacketTranslator { - if (session.getPlayerInventory().getHeldItemSlot() != containerAction.getSlot() || - session.getPlayerInventory().getItemInHand().isEmpty()) { - return; - } + if (session.getPlayerInventory().getHeldItemSlot() != containerAction.getSlot() || + session.getPlayerInventory().getItemInHand().isEmpty()) { + return; + } - boolean dropAll = worldAction.getToItem().getCount() > 1; - ClientPlayerActionPacket dropAllPacket = new ClientPlayerActionPacket( - dropAll ? PlayerAction.DROP_ITEM_STACK : PlayerAction.DROP_ITEM, - BlockUtils.POSITION_ZERO, - BlockFace.DOWN - ); - session.sendDownstreamPacket(dropAllPacket); + boolean dropAll = worldAction.getToItem().getCount() > 1; + ClientPlayerActionPacket dropAllPacket = new ClientPlayerActionPacket( + dropAll ? PlayerAction.DROP_ITEM_STACK : PlayerAction.DROP_ITEM, + BlockUtils.POSITION_ZERO, + BlockFace.DOWN + ); + session.sendDownstreamPacket(dropAllPacket); - if (dropAll) { - session.getPlayerInventory().setItemInHand(GeyserItemStack.EMPTY); - } else { - session.getPlayerInventory().getItemInHand().sub(1); - } - }); + if (dropAll) { + session.getPlayerInventory().setItemInHand(GeyserItemStack.EMPTY); + } else { + session.getPlayerInventory().getItemInHand().sub(1); + } } } break; @@ -222,7 +220,7 @@ public class BedrockInventoryTransactionTranslator extends PacketTranslator { + session.setBucketScheduledFuture(session.getEventLoop().schedule(() -> { ClientPlayerUseItemPacket itemPacket = new ClientPlayerUseItemPacket(Hand.MAIN_HAND); session.sendDownstreamPacket(itemPacket); }, 5, TimeUnit.MILLISECONDS)); diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockItemStackRequestTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockItemStackRequestTranslator.java index bdbb88eee..b5e0dd2ce 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockItemStackRequestTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockItemStackRequestTranslator.java @@ -45,6 +45,6 @@ public class BedrockItemStackRequestTranslator extends PacketTranslator translator.translateRequests(session, inventory, packet.getRequests())); + translator.translateRequests(session, inventory, packet.getRequests()); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/entity/BedrockEntityEventTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/entity/BedrockEntityEventTranslator.java index 6267597fd..ced40ca28 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/entity/BedrockEntityEventTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/entity/BedrockEntityEventTranslator.java @@ -37,6 +37,8 @@ import org.geysermc.connector.network.session.GeyserSession; import org.geysermc.connector.network.translators.PacketTranslator; import org.geysermc.connector.network.translators.Translator; +import java.util.concurrent.TimeUnit; + @Translator(packet = EntityEventPacket.class) public class BedrockEntityEventTranslator extends PacketTranslator { @@ -48,12 +50,10 @@ public class BedrockEntityEventTranslator extends PacketTranslator { - ClientSelectTradePacket selectTradePacket = new ClientSelectTradePacket(packet.getData()); - session.sendDownstreamPacket(selectTradePacket); - }); + ClientSelectTradePacket selectTradePacket = new ClientSelectTradePacket(packet.getData()); + session.sendDownstreamPacket(selectTradePacket); - session.addInventoryTask(() -> { + session.getEventLoop().schedule(() -> { Entity villager = session.getPlayerEntity(); Inventory openInventory = session.getOpenInventory(); if (openInventory instanceof MerchantContainer) { @@ -66,7 +66,7 @@ public class BedrockEntityEventTranslator extends PacketTranslator entity.setHealth(entity.getMaxHealth()); entity.getAttributes().put(GeyserAttributeType.HEALTH, entity.createHealthAttribute()); - session.addInventoryTask(() -> { - session.setInventoryTranslator(InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR); - session.setOpenInventory(null); - session.setClosingInventory(false); - }); + session.setInventoryTranslator(InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR); + session.setOpenInventory(null); + session.setClosingInventory(false); SetPlayerGameTypePacket playerGameTypePacket = new SetPlayerGameTypePacket(); playerGameTypePacket.setGamemode(packet.getGamemode().ordinal()); diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerChangeHeldItemTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerChangeHeldItemTranslator.java index 68df63e2b..58ab3bfcd 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerChangeHeldItemTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerChangeHeldItemTranslator.java @@ -36,14 +36,12 @@ public class JavaPlayerChangeHeldItemTranslator extends PacketTranslator { - PlayerHotbarPacket hotbarPacket = new PlayerHotbarPacket(); - hotbarPacket.setContainerId(0); - hotbarPacket.setSelectedHotbarSlot(packet.getSlot()); - hotbarPacket.setSelectHotbarSlot(true); - session.sendUpstreamPacket(hotbarPacket); + PlayerHotbarPacket hotbarPacket = new PlayerHotbarPacket(); + hotbarPacket.setContainerId(0); + hotbarPacket.setSelectedHotbarSlot(packet.getSlot()); + hotbarPacket.setSelectHotbarSlot(true); + session.sendUpstreamPacket(hotbarPacket); - session.getPlayerInventory().setHeldItemSlot(packet.getSlot()); - }); + session.getPlayerInventory().setHeldItemSlot(packet.getSlot()); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java index 9efdee7fc..3c984c0a2 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java @@ -36,9 +36,8 @@ public class JavaCloseWindowTranslator extends PacketTranslator - // Sometimes the server can request a window close of ID 0... when the window isn't even open - // Don't confirm in this instance - InventoryUtils.closeInventory(session, packet.getWindowId(), (session.getOpenInventory() != null && session.getOpenInventory().getId() == packet.getWindowId()))); + // Sometimes the server can request a window close of ID 0... when the window isn't even open + // Don't confirm in this instance + InventoryUtils.closeInventory(session, packet.getWindowId(), (session.getOpenInventory() != null && session.getOpenInventory().getId() == packet.getWindowId())); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java index 79abcc957..c562226c6 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java @@ -31,48 +31,46 @@ import org.geysermc.connector.inventory.Inventory; import org.geysermc.connector.network.session.GeyserSession; import org.geysermc.connector.network.translators.PacketTranslator; import org.geysermc.connector.network.translators.Translator; +import org.geysermc.connector.network.translators.chat.MessageTranslator; import org.geysermc.connector.network.translators.inventory.InventoryTranslator; import org.geysermc.connector.utils.InventoryUtils; import org.geysermc.connector.utils.LocaleUtils; -import org.geysermc.connector.network.translators.chat.MessageTranslator; @Translator(packet = ServerOpenWindowPacket.class) public class JavaOpenWindowTranslator extends PacketTranslator { @Override public void translate(ServerOpenWindowPacket packet, GeyserSession session) { - session.addInventoryTask(() -> { - if (packet.getWindowId() == 0) { - return; - } + if (packet.getWindowId() == 0) { + return; + } - InventoryTranslator newTranslator = InventoryTranslator.INVENTORY_TRANSLATORS.get(packet.getType()); - Inventory openInventory = session.getOpenInventory(); - //No translator exists for this window type. Close all windows and return. - if (newTranslator == null) { - if (openInventory != null) { - InventoryUtils.closeInventory(session, openInventory.getId(), true); - } - ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(packet.getWindowId()); - session.sendDownstreamPacket(closeWindowPacket); - return; - } - - String name = MessageTranslator.convertMessageLenient(packet.getName(), session.getLocale()); - name = LocaleUtils.getLocaleString(name, session.getLocale()); - - Inventory newInventory = newTranslator.createInventory(name, packet.getWindowId(), packet.getType(), session.getPlayerInventory()); + InventoryTranslator newTranslator = InventoryTranslator.INVENTORY_TRANSLATORS.get(packet.getType()); + Inventory openInventory = session.getOpenInventory(); + //No translator exists for this window type. Close all windows and return. + if (newTranslator == null) { if (openInventory != null) { - // If the window type is the same, don't close. - // In rare cases, inventories can do funny things where it keeps the same window type up but change the contents. - if (openInventory.getWindowType() != packet.getType()) { - // Sometimes the server can double-open an inventory with the same ID - don't confirm in that instance. - InventoryUtils.closeInventory(session, openInventory.getId(), openInventory.getId() != packet.getWindowId()); - } + InventoryUtils.closeInventory(session, openInventory.getId(), true); } + ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(packet.getWindowId()); + session.sendDownstreamPacket(closeWindowPacket); + return; + } - session.setInventoryTranslator(newTranslator); - InventoryUtils.openInventory(session, newInventory); - }); + String name = MessageTranslator.convertMessageLenient(packet.getName(), session.getLocale()); + name = LocaleUtils.getLocaleString(name, session.getLocale()); + + Inventory newInventory = newTranslator.createInventory(name, packet.getWindowId(), packet.getType(), session.getPlayerInventory()); + if (openInventory != null) { + // If the window type is the same, don't close. + // In rare cases, inventories can do funny things where it keeps the same window type up but change the contents. + if (openInventory.getWindowType() != packet.getType()) { + // Sometimes the server can double-open an inventory with the same ID - don't confirm in that instance. + InventoryUtils.closeInventory(session, openInventory.getId(), openInventory.getId() != packet.getWindowId()); + } + } + + session.setInventoryTranslator(newTranslator); + InventoryUtils.openInventory(session, newInventory); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaSetSlotTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaSetSlotTranslator.java index b0ce8994e..0d1f6f8d2 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaSetSlotTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaSetSlotTranslator.java @@ -59,39 +59,37 @@ public class JavaSetSlotTranslator extends PacketTranslator @Override public void translate(ServerSetSlotPacket packet, GeyserSession session) { - session.addInventoryTask(() -> { - if (packet.getWindowId() == 255) { //cursor - GeyserItemStack newItem = GeyserItemStack.from(packet.getItem()); - session.getPlayerInventory().setCursor(newItem, session); - InventoryUtils.updateCursor(session); - return; + if (packet.getWindowId() == 255) { //cursor + GeyserItemStack newItem = GeyserItemStack.from(packet.getItem()); + session.getPlayerInventory().setCursor(newItem, session); + InventoryUtils.updateCursor(session); + return; + } + + //TODO: support window id -2, should update player inventory + Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); + if (inventory == null) + return; + + inventory.setStateId(packet.getStateId()); + + InventoryTranslator translator = session.getInventoryTranslator(); + if (translator != null) { + if (session.getCraftingGridFuture() != null) { + session.getCraftingGridFuture().cancel(false); } + session.setCraftingGridFuture(session.getEventLoop().schedule(() -> updateCraftingGrid(session, packet, inventory, translator), 150, TimeUnit.MILLISECONDS)); - //TODO: support window id -2, should update player inventory - Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); - if (inventory == null) - return; - - inventory.setStateId(packet.getStateId()); - - InventoryTranslator translator = session.getInventoryTranslator(); - if (translator != null) { - if (session.getCraftingGridFuture() != null) { - session.getCraftingGridFuture().cancel(false); - } - session.setCraftingGridFuture(session.getConnector().getGeneralThreadPool().schedule(() -> session.addInventoryTask(() -> updateCraftingGrid(session, packet, inventory, translator)), 150, TimeUnit.MILLISECONDS)); - - GeyserItemStack newItem = GeyserItemStack.from(packet.getItem()); - if (packet.getWindowId() == 0 && !(translator instanceof PlayerInventoryTranslator)) { - // In rare cases, the window ID can still be 0 but Java treats it as valid - session.getPlayerInventory().setItem(packet.getSlot(), newItem, session); - InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR.updateSlot(session, session.getPlayerInventory(), packet.getSlot()); - } else { - inventory.setItem(packet.getSlot(), newItem, session); - translator.updateSlot(session, inventory, packet.getSlot()); - } + GeyserItemStack newItem = GeyserItemStack.from(packet.getItem()); + if (packet.getWindowId() == 0 && !(translator instanceof PlayerInventoryTranslator)) { + // In rare cases, the window ID can still be 0 but Java treats it as valid + session.getPlayerInventory().setItem(packet.getSlot(), newItem, session); + InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR.updateSlot(session, session.getPlayerInventory(), packet.getSlot()); + } else { + inventory.setItem(packet.getSlot(), newItem, session); + translator.updateSlot(session, inventory, packet.getSlot()); } - }); + } } private static void updateCraftingGrid(GeyserSession session, ServerSetSlotPacket packet, Inventory inventory, InventoryTranslator translator) { diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowItemsTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowItemsTranslator.java index 4b121229f..7f8cea595 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowItemsTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowItemsTranslator.java @@ -39,25 +39,23 @@ public class JavaWindowItemsTranslator extends PacketTranslator { - Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); - if (inventory == null) - return; + Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); + if (inventory == null) + return; - inventory.setStateId(packet.getStateId()); + inventory.setStateId(packet.getStateId()); - for (int i = 0; i < packet.getItems().length; i++) { - GeyserItemStack newItem = GeyserItemStack.from(packet.getItems()[i]); - inventory.setItem(i, newItem, session); - } + for (int i = 0; i < packet.getItems().length; i++) { + GeyserItemStack newItem = GeyserItemStack.from(packet.getItems()[i]); + inventory.setItem(i, newItem, session); + } - InventoryTranslator translator = session.getInventoryTranslator(); - if (translator != null) { - translator.updateInventory(session, inventory); - } + InventoryTranslator translator = session.getInventoryTranslator(); + if (translator != null) { + translator.updateInventory(session, inventory); + } - session.getPlayerInventory().setCursor(GeyserItemStack.from(packet.getCarriedItem()), session); - InventoryUtils.updateCursor(session); - }); + session.getPlayerInventory().setCursor(GeyserItemStack.from(packet.getCarriedItem()), session); + InventoryUtils.updateCursor(session); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowPropertyTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowPropertyTranslator.java index c31a39029..c5dcc76ca 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowPropertyTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaWindowPropertyTranslator.java @@ -38,15 +38,13 @@ public class JavaWindowPropertyTranslator extends PacketTranslator { - Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); - if (inventory == null) - return; + Inventory inventory = InventoryUtils.getInventory(session, packet.getWindowId()); + if (inventory == null) + return; - InventoryTranslator translator = session.getInventoryTranslator(); - if (translator != null) { - translator.updateProperty(session, inventory, packet.getRawProperty(), packet.getValue()); - } - }); + InventoryTranslator translator = session.getInventoryTranslator(); + if (translator != null) { + translator.updateProperty(session, inventory, packet.getRawProperty(), packet.getValue()); + } } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/world/block/entity/SkullBlockEntityTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/world/block/entity/SkullBlockEntityTranslator.java index e7139f20c..72dee2d94 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/world/block/entity/SkullBlockEntityTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/world/block/entity/SkullBlockEntityTranslator.java @@ -147,7 +147,7 @@ public class SkullBlockEntityTranslator extends BlockEntityTranslator implements if (session.getUpstream().isInitialized()) { player.spawnEntity(session); - SkullSkinManager.requestAndHandleSkin(player, session, (skin -> session.getConnector().getGeneralThreadPool().schedule(() -> { + SkullSkinManager.requestAndHandleSkin(player, session, (skin -> session.getEventLoop().schedule(() -> { // Delay to minimize split-second "player" pop-in player.getMetadata().getFlags().setFlag(EntityFlag.INVISIBLE, false); player.updateBedrockMetadata(session); diff --git a/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java b/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java index c9ddd8ed9..2f4f2456e 100644 --- a/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java +++ b/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java @@ -37,7 +37,6 @@ import com.nukkitx.protocol.bedrock.data.inventory.ContainerId; import com.nukkitx.protocol.bedrock.data.inventory.ItemData; import com.nukkitx.protocol.bedrock.packet.InventorySlotPacket; import com.nukkitx.protocol.bedrock.packet.PlayerHotbarPacket; -import org.geysermc.connector.GeyserConnector; import org.geysermc.connector.common.ChatColor; import org.geysermc.connector.inventory.Container; import org.geysermc.connector.inventory.GeyserItemStack; @@ -80,17 +79,16 @@ public class InventoryUtils { if (translator != null) { translator.prepareInventory(session, inventory); if (translator instanceof DoubleChestInventoryTranslator && !((Container) inventory).isUsingRealBlock()) { - GeyserConnector.getInstance().getGeneralThreadPool().schedule(() -> - session.addInventoryTask(() -> { - Inventory openInv = session.getOpenInventory(); - if (openInv != null && openInv.getId() == inventory.getId()) { - translator.openInventory(session, inventory); - translator.updateInventory(session, inventory); - } else if (openInv != null && openInv.isPending()) { - // Presumably, this inventory is no longer relevant, and the client doesn't care about it - displayInventory(session, openInv); - } - }), 200, TimeUnit.MILLISECONDS); + session.getEventLoop().schedule(() -> { + Inventory openInv = session.getOpenInventory(); + if (openInv != null && openInv.getId() == inventory.getId()) { + translator.openInventory(session, inventory); + translator.updateInventory(session, inventory); + } else if (openInv != null && openInv.isPending()) { + // Presumably, this inventory is no longer relevant, and the client doesn't care about it + displayInventory(session, openInv); + } + }, 200, TimeUnit.MILLISECONDS); } else { translator.openInventory(session, inventory); translator.updateInventory(session, inventory);