From 52e4767247d48b83da9f6881eea0ca2de9e7de78 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 29 May 2021 20:05:13 +0100 Subject: [PATCH] Fix screen handler data syncing, also rewrite it while im at it to reduce the number of packets sent. --- .../screen/builder/BuiltScreenHandler.java | 177 ++++++------------ .../ExtendedScreenHandlerListener.java | 44 ----- .../screen/builder/ScreenHandlerBuilder.java | 2 +- .../client/screen/builder/Syncable.java | 2 +- .../blockentity/RedstoneConfiguration.java | 2 +- .../network/ClientBoundPacketHandlers.java | 36 +++- .../common/network/ClientBoundPackets.java | 10 +- .../java/reborncore/common/util/Tank.java | 2 +- .../mixin/common/MixinServerPlayerEntity.java | 18 ++ .../ServerPlayerEntityScreenHandler.java | 8 + .../resources/reborncore.common.mixins.json | 3 +- 11 files changed, 128 insertions(+), 176 deletions(-) delete mode 100644 RebornCore/src/main/java/reborncore/client/screen/builder/ExtendedScreenHandlerListener.java create mode 100644 RebornCore/src/main/java/reborncore/mixin/common/MixinServerPlayerEntity.java create mode 100644 RebornCore/src/main/java/reborncore/mixin/ifaces/ServerPlayerEntityScreenHandler.java diff --git a/RebornCore/src/main/java/reborncore/client/screen/builder/BuiltScreenHandler.java b/RebornCore/src/main/java/reborncore/client/screen/builder/BuiltScreenHandler.java index 16b42864e..f48600d9a 100644 --- a/RebornCore/src/main/java/reborncore/client/screen/builder/BuiltScreenHandler.java +++ b/RebornCore/src/main/java/reborncore/client/screen/builder/BuiltScreenHandler.java @@ -24,6 +24,8 @@ package reborncore.client.screen.builder; +import it.unimi.dsi.fastutil.ints.Int2ObjectMap; +import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import net.minecraft.entity.player.PlayerEntity; import net.minecraft.inventory.CraftingInventory; import net.minecraft.inventory.Inventory; @@ -34,17 +36,23 @@ import net.minecraft.screen.ScreenHandlerType; import net.minecraft.screen.slot.Slot; import net.minecraft.util.math.BlockPos; import org.apache.commons.lang3.Range; -import org.apache.commons.lang3.tuple.MutableTriple; import org.apache.commons.lang3.tuple.Pair; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import reborncore.common.blockentity.MachineBaseBlockEntity; +import reborncore.common.network.ClientBoundPackets; +import reborncore.common.network.NetworkManager; import reborncore.common.util.ItemUtils; import reborncore.mixin.common.AccessorScreenHandler; +import reborncore.mixin.ifaces.ServerPlayerEntityScreenHandler; -import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.*; -public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenHandlerListener { +public class BuiltScreenHandler extends ScreenHandler { + private static final Logger LOGGER = LogManager.getLogger(); private final String name; @@ -52,9 +60,10 @@ public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenH private final List> playerSlotRanges; private final List> blockEntitySlotRanges; - private final ArrayList> shortValues; - private final ArrayList> integerValues; - private final ArrayList> objectValues; + // Holds the syncpair along with the last value + private final Map syncPairCache = new HashMap<>(); + private final Int2ObjectMap syncPairIdLookup = new Int2ObjectOpenHashMap<>(); + private List> craftEvents; private Integer[] integerParts; @@ -71,36 +80,17 @@ public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenH this.playerSlotRanges = playerSlotRange; this.blockEntitySlotRanges = blockEntitySlotRange; - this.shortValues = new ArrayList<>(); - this.integerValues = new ArrayList<>(); - this.objectValues = new ArrayList<>(); - this.blockEntity = blockEntity; } - public void addShortSync(final List> syncables) { - - for (final Pair syncable : syncables) { - this.shortValues.add(MutableTriple.of(syncable.getLeft(), syncable.getRight(), (short) 0)); + public void addObjectSync(final List, Consumer>> syncables) { + for (final Pair, Consumer> syncable : syncables) { + // Add a new sync pair to the cache with a null value + int id = syncPairCache.size() + 1; + var syncPair = new SyncPair(syncable.getLeft(), syncable.getRight(), id); + this.syncPairCache.put(syncPair, null); + this.syncPairIdLookup.put(id, syncPair); } - this.shortValues.trimToSize(); - } - - public void addIntegerSync(final List> syncables) { - - for (final Pair syncable : syncables) { - this.integerValues.add(MutableTriple.of(syncable.getLeft(), syncable.getRight(), 0)); - } - this.integerValues.trimToSize(); - this.integerParts = new Integer[this.integerValues.size()]; - } - - public void addObjectSync(final List> syncables) { - - for (final Pair syncable : syncables) { - this.objectValues.add(MutableTriple.of(syncable.getLeft(), syncable.getRight(), null)); - } - this.objectValues.trimToSize(); } public void addCraftEvents(final List> craftEvents) { @@ -124,44 +114,7 @@ public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenH super.sendContentUpdates(); for (final ScreenHandlerListener listener : ((AccessorScreenHandler) (this)).getListeners()) { - - int i = 0; - if (!this.shortValues.isEmpty()) { - for (final MutableTriple value : this.shortValues) { - final short supplied = (short) value.getLeft().getAsInt(); - if (supplied != value.getRight()) { - - listener.onPropertyUpdate(this, i, supplied); - value.setRight(supplied); - } - i++; - } - } - - if (!this.integerValues.isEmpty()) { - for (final MutableTriple value : this.integerValues) { - final int supplied = value.getLeft().getAsInt(); - if (supplied != value.getRight()) { - - listener.onPropertyUpdate(this, i, supplied >> 16); - listener.onPropertyUpdate(this, i + 1, (short) (supplied & 0xFFFF)); - value.setRight(supplied); - } - i += 2; - } - } - - if (!this.objectValues.isEmpty()) { - int objects = 0; - for (final MutableTriple value : this.objectValues) { - final Object supplied = value.getLeft().get(); - if (supplied != value.getRight()) { - sendObject(listener, this, objects, supplied); - value.setRight(supplied); - } - objects++; - } - } + sendContentUpdatePacketToListener(listener); } } @@ -169,57 +122,45 @@ public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenH public void addListener(final ScreenHandlerListener listener) { super.addListener(listener); - int i = 0; - if (!this.shortValues.isEmpty()) { - for (final MutableTriple value : this.shortValues) { - final short supplied = (short) value.getLeft().getAsInt(); - - listener.onPropertyUpdate(this, i, supplied); - value.setRight(supplied); - i++; - } - } - - if (!this.integerValues.isEmpty()) { - for (final MutableTriple value : this.integerValues) { - final int supplied = value.getLeft().getAsInt(); - - listener.onPropertyUpdate(this, i, supplied >> 16); - listener.onPropertyUpdate(this, i + 1, (short) (supplied & 0xFFFF)); - value.setRight(supplied); - i += 2; - } - } - - if (!this.objectValues.isEmpty()) { - int objects = 0; - for (final MutableTriple value : this.objectValues) { - final Object supplied = value.getLeft(); - sendObject(listener, this, objects, ((Supplier) supplied).get()); - value.setRight(supplied); - objects++; - } - } + sendContentUpdatePacketToListener(listener); } - @Override - public void handleObject(int var, Object value) { - this.objectValues.get(var).getMiddle().accept(value); + private void sendContentUpdatePacketToListener(final ScreenHandlerListener listener) { + Int2ObjectMap updatedValues = new Int2ObjectOpenHashMap<>(); + + this.syncPairCache.replaceAll((syncPair, cached) -> { + final Object value = syncPair.supplier().get(); + + if (value != cached) { + updatedValues.put(syncPair.id, value); + return value; + } + return null; + }); + + sendUpdatedValues(listener, this, updatedValues); } - @Override - public void setProperty(int id, int value) { - if (id < this.shortValues.size()) { - this.shortValues.get(id).getMiddle().accept((short) value); - this.shortValues.get(id).setRight((short) value); - } else if (id - this.shortValues.size() < this.integerValues.size() * 2) { - - if ((id - this.shortValues.size()) % 2 == 0) { - this.integerParts[(id - this.shortValues.size()) / 2] = value; - } else { - this.integerValues.get((id - this.shortValues.size()) / 2).getMiddle().accept( - (this.integerParts[(id - this.shortValues.size()) / 2] & 0xFFFF) << 16 | value & 0xFFFF); + public void handleUpdateValues(Int2ObjectMap updatedValues) { + updatedValues.int2ObjectEntrySet().forEach(entry -> { + SyncPair syncPair = syncPairIdLookup.get(entry.getIntKey()); + if (syncPair == null) { + LOGGER.warn("Unknown sync pair id: " + entry.getIntKey()); + return; } + + // TODO check the object type here? + syncPair.consumer().accept(entry.getValue()); + }); + } + + private void sendUpdatedValues(ScreenHandlerListener screenHandlerListener, ScreenHandler screenHandler, Int2ObjectMap updatedValues) { + if (updatedValues.isEmpty()) { + return; + } + + if (screenHandlerListener instanceof ServerPlayerEntityScreenHandler serverPlayerEntityScreenHandler) { + NetworkManager.sendToPlayer(ClientBoundPackets.createPacketSendObject(screenHandler, updatedValues), serverPlayerEntityScreenHandler.rc_getServerPlayerEntity()); } } @@ -370,4 +311,8 @@ public class BuiltScreenHandler extends ScreenHandler implements ExtendedScreenH public ScreenHandlerType getType() { return type; } + + private record SyncPair(Supplier supplier, Consumer consumer, int id) { + + } } diff --git a/RebornCore/src/main/java/reborncore/client/screen/builder/ExtendedScreenHandlerListener.java b/RebornCore/src/main/java/reborncore/client/screen/builder/ExtendedScreenHandlerListener.java deleted file mode 100644 index 8584bca52..000000000 --- a/RebornCore/src/main/java/reborncore/client/screen/builder/ExtendedScreenHandlerListener.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * This file is part of RebornCore, licensed under the MIT License (MIT). - * - * Copyright (c) 2021 TeamReborn - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -package reborncore.client.screen.builder; - -import net.minecraft.screen.ScreenHandler; -import net.minecraft.screen.ScreenHandlerListener; -import net.minecraft.server.network.ServerPlayerEntity; -import reborncore.common.network.ClientBoundPackets; -import reborncore.common.network.NetworkManager; - -public interface ExtendedScreenHandlerListener { - - default void sendObject(ScreenHandlerListener screenHandlerListener, ScreenHandler screenHandler, int var, Object value) { - if (screenHandlerListener instanceof ServerPlayerEntity) { - NetworkManager.sendToPlayer(ClientBoundPackets.createPacketSendObject(var, value, screenHandler), (ServerPlayerEntity) screenHandlerListener); - } - } - - default void handleObject(int var, Object value) { - - } -} diff --git a/RebornCore/src/main/java/reborncore/client/screen/builder/ScreenHandlerBuilder.java b/RebornCore/src/main/java/reborncore/client/screen/builder/ScreenHandlerBuilder.java index a41928df2..b21663a82 100644 --- a/RebornCore/src/main/java/reborncore/client/screen/builder/ScreenHandlerBuilder.java +++ b/RebornCore/src/main/java/reborncore/client/screen/builder/ScreenHandlerBuilder.java @@ -47,7 +47,7 @@ public class ScreenHandlerBuilder { final List slots; final List> playerInventoryRanges, blockEntityInventoryRanges; - final List> objectValues; + final List, Consumer>> objectValues; final List> craftEvents; diff --git a/RebornCore/src/main/java/reborncore/client/screen/builder/Syncable.java b/RebornCore/src/main/java/reborncore/client/screen/builder/Syncable.java index 62e700627..eefa1b3b0 100644 --- a/RebornCore/src/main/java/reborncore/client/screen/builder/Syncable.java +++ b/RebornCore/src/main/java/reborncore/client/screen/builder/Syncable.java @@ -32,6 +32,6 @@ import java.util.function.Supplier; public interface Syncable { - void getSyncPair(List> pairList); + void getSyncPair(List, Consumer>> pairList); } diff --git a/RebornCore/src/main/java/reborncore/common/blockentity/RedstoneConfiguration.java b/RebornCore/src/main/java/reborncore/common/blockentity/RedstoneConfiguration.java index f916c785c..81bbe80f6 100644 --- a/RebornCore/src/main/java/reborncore/common/blockentity/RedstoneConfiguration.java +++ b/RebornCore/src/main/java/reborncore/common/blockentity/RedstoneConfiguration.java @@ -177,7 +177,7 @@ public class RedstoneConfiguration implements NBTSerializable, Syncable { } @Override - public void getSyncPair(List> pairList) { + public void getSyncPair(List, Consumer>> pairList) { pairList.add(Pair.of(this::write, (Consumer) this::read)); } diff --git a/RebornCore/src/main/java/reborncore/common/network/ClientBoundPacketHandlers.java b/RebornCore/src/main/java/reborncore/common/network/ClientBoundPacketHandlers.java index 85bc72447..7b9b52f4f 100644 --- a/RebornCore/src/main/java/reborncore/common/network/ClientBoundPacketHandlers.java +++ b/RebornCore/src/main/java/reborncore/common/network/ClientBoundPacketHandlers.java @@ -24,6 +24,8 @@ package reborncore.common.network; +import it.unimi.dsi.fastutil.ints.Int2ObjectMap; +import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import net.fabricmc.api.EnvType; import net.fabricmc.api.Environment; import net.minecraft.block.entity.BlockEntity; @@ -35,9 +37,11 @@ import net.minecraft.screen.ScreenHandler; import net.minecraft.util.Identifier; import net.minecraft.util.math.BlockPos; import net.minecraft.world.World; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import reborncore.RebornCore; import reborncore.client.ClientChunkManager; -import reborncore.client.screen.builder.ExtendedScreenHandlerListener; +import reborncore.client.screen.builder.BuiltScreenHandler; import reborncore.common.blockentity.FluidConfiguration; import reborncore.common.blockentity.MachineBaseBlockEntity; import reborncore.common.blockentity.SlotConfiguration; @@ -47,6 +51,7 @@ import java.util.List; @Environment(EnvType.CLIENT) public class ClientBoundPacketHandlers { + private static final Logger LOGGER = LogManager.getLogger(); public static void init() { NetworkManager.registerClientBoundHandler(new Identifier("reborncore", "custom_description"), (client, handler, packetBuffer, responseSender) -> { @@ -103,15 +108,30 @@ public class ClientBoundPacketHandlers { }); NetworkManager.registerClientBoundHandler(new Identifier("reborncore", "send_object"), (client, handler, packetBuffer, responseSender) -> { - int id = packetBuffer.readInt(); - Object value = new ExtendedPacketBuffer(packetBuffer).readObject(); - String container = packetBuffer.readString(packetBuffer.readInt()); + int size = packetBuffer.readInt(); + ExtendedPacketBuffer epb = new ExtendedPacketBuffer(packetBuffer); + Int2ObjectMap updatedValues = new Int2ObjectOpenHashMap<>(); + + for (int i = 0; i < size; i++) { + int id = packetBuffer.readInt(); + Object value = epb.readObject(); + updatedValues.put(id, value); + } + + String name = packetBuffer.readString(packetBuffer.readInt()); + client.execute(() -> { Screen gui = MinecraftClient.getInstance().currentScreen; - if (gui instanceof HandledScreen) { - ScreenHandler screenHandler = ((HandledScreen) gui).getScreenHandler(); - if (screenHandler instanceof ExtendedScreenHandlerListener) { - ((ExtendedScreenHandlerListener) screenHandler).handleObject(id, value); + if (gui instanceof HandledScreen handledScreen) { + ScreenHandler screenHandler = handledScreen.getScreenHandler(); + if (screenHandler instanceof BuiltScreenHandler builtScreenHandler) { + String shName = screenHandler.getClass().getName(); + if (!shName.equals(name)) { + LOGGER.warn("Received packet for {} but screen handler {} is open!", name, shName); + return; + } + + builtScreenHandler.handleUpdateValues(updatedValues); } } }); diff --git a/RebornCore/src/main/java/reborncore/common/network/ClientBoundPackets.java b/RebornCore/src/main/java/reborncore/common/network/ClientBoundPackets.java index e9cb48432..eebad58fd 100644 --- a/RebornCore/src/main/java/reborncore/common/network/ClientBoundPackets.java +++ b/RebornCore/src/main/java/reborncore/common/network/ClientBoundPackets.java @@ -24,6 +24,7 @@ package reborncore.common.network; +import it.unimi.dsi.fastutil.ints.Int2ObjectMap; import net.minecraft.block.entity.BlockEntity; import net.minecraft.nbt.NbtCompound; import net.minecraft.screen.ScreenHandler; @@ -62,10 +63,13 @@ public class ClientBoundPackets { }); } - public static IdentifiedPacket createPacketSendObject(int id, Object value, ScreenHandler screenHandler) { + public static IdentifiedPacket createPacketSendObject(ScreenHandler screenHandler, Int2ObjectMap updatedValues) { return NetworkManager.createClientBoundPacket(new Identifier("reborncore", "send_object"), packetBuffer -> { - packetBuffer.writeInt(id); - packetBuffer.writeObject(value); + packetBuffer.writeInt(updatedValues.size()); + updatedValues.forEach((integer, o) -> { + packetBuffer.writeInt(integer); + packetBuffer.writeObject(o); + }); packetBuffer.writeInt(screenHandler.getClass().getName().length()); packetBuffer.writeString(screenHandler.getClass().getName()); }); diff --git a/RebornCore/src/main/java/reborncore/common/util/Tank.java b/RebornCore/src/main/java/reborncore/common/util/Tank.java index b8f4b9901..237c06b50 100644 --- a/RebornCore/src/main/java/reborncore/common/util/Tank.java +++ b/RebornCore/src/main/java/reborncore/common/util/Tank.java @@ -133,7 +133,7 @@ public class Tank implements GenericFluidContainer, Syncable { } @Override - public void getSyncPair(List> pairList) { + public void getSyncPair(List, Consumer>> pairList) { pairList.add(Pair.of(() -> Registry.FLUID.getId(fluidInstance.getFluid()).toString(), (Consumer) o -> fluidInstance.setFluid(Registry.FLUID.get(new Identifier(o))))); pairList.add(Pair.of(() -> fluidInstance.getAmount(), o -> fluidInstance.setAmount((FluidValue) o))); } diff --git a/RebornCore/src/main/java/reborncore/mixin/common/MixinServerPlayerEntity.java b/RebornCore/src/main/java/reborncore/mixin/common/MixinServerPlayerEntity.java new file mode 100644 index 000000000..e3663fd1f --- /dev/null +++ b/RebornCore/src/main/java/reborncore/mixin/common/MixinServerPlayerEntity.java @@ -0,0 +1,18 @@ +package reborncore.mixin.common; + +import net.minecraft.server.network.ServerPlayerEntity; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import reborncore.mixin.ifaces.ServerPlayerEntityScreenHandler; + +@Mixin(targets = "net.minecraft.server.network.ServerPlayerEntity$2") +public class MixinServerPlayerEntity implements ServerPlayerEntityScreenHandler { + + @Shadow + ServerPlayerEntity field_29183; + + @Override + public ServerPlayerEntity rc_getServerPlayerEntity() { + return field_29183; + } +} diff --git a/RebornCore/src/main/java/reborncore/mixin/ifaces/ServerPlayerEntityScreenHandler.java b/RebornCore/src/main/java/reborncore/mixin/ifaces/ServerPlayerEntityScreenHandler.java new file mode 100644 index 000000000..74a828931 --- /dev/null +++ b/RebornCore/src/main/java/reborncore/mixin/ifaces/ServerPlayerEntityScreenHandler.java @@ -0,0 +1,8 @@ +package reborncore.mixin.ifaces; + +import net.minecraft.server.network.ServerPlayerEntity; + +public interface ServerPlayerEntityScreenHandler { + + ServerPlayerEntity rc_getServerPlayerEntity(); +} diff --git a/RebornCore/src/main/resources/reborncore.common.mixins.json b/RebornCore/src/main/resources/reborncore.common.mixins.json index eae6819e9..58d629f6c 100644 --- a/RebornCore/src/main/resources/reborncore.common.mixins.json +++ b/RebornCore/src/main/resources/reborncore.common.mixins.json @@ -15,7 +15,8 @@ "MixinItemStack", "MixinLivingEntity", "MixinPlayerEntity", - "MixinRecipeManager" + "MixinRecipeManager", + "MixinServerPlayerEntity" ], "injectors": { "defaultRequire": 1