Inventory&contains sync

[RCUTANF]

This pull request introduces significant improvements to the inventory and container synchronization system for spectator clients, focusing on more robust handling of container screens, better item syncing, and clearer state management. The changes include new logic for syncing container items, improved handling of inventory open/close events, and refactoring for clarity and correctness in sync controller methods.

Inventory and Container Synchronization Improvements:

• Added logic to synchronize container items in all container screens, not just the player inventory, ensuring all relevant slots are updated with the correct items from the sync data. (AbstractContainerScreenMixin.java)
• Improved handling of container screen opening by detecting container types and sizes, and opening the appropriate synced container screen using the new DummyContainer class. (MenuScreensMixin.java, ScreenSyncController.java, DummyContainer.java) [1] [2] [3]
• Enhanced the logic for syncing player inventory and container items by updating only the relevant slots and supporting both inventory and container screens. (SyncedInventoryScreen.java, AbstractContainerScreenMixin.java) [1] [2]

[Copilot]

Pull request overview

Improves spectator client synchronization for inventories and container screens by adding dedicated container/cursor sync flows, enhancing open/close screen state handling, and refactoring client sync state management to support more GUI types.

Changes:

• Add container and cursor sync packets + server handlers/listeners to keep spectator GUIs updated beyond just the player inventory.
• Refactor client screen sync state/data to support container type/size, incremental item updates, and explicit “screen closed” signaling.
• Update mixins on both server and client to hook menu open/close and cursor changes, and to open appropriate synced container screens.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/packet/ServerboundOpenedInventorySyncPacket.java	Adds explicit open/close boolean state to the C2S “opened inventory” sync packet.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/packet/ClientboundScreenSyncPacket.java	Adds permission-gated send and a “screen closed” flag bit accessor.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/packet/ClientboundScreenCursorSyncPacket.java	Adds permission-gated send for cursor sync packet.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/packet/ClientboundHotbarSyncPacket.java	Introduces an ITEMS_LENGTH constant for hotbar size.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/packet/ClientboundContainerSyncPacket.java	New S2C packet carrying container type/size and item array updates.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ScreenSyncHandler.java	Adds server-side screen open/close sync orchestration for spectators.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/InventorySyncHandler.java	New server tick-based inventory sync with incremental updates.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/CursorSyncHandler.java	New server-side cursor (carried item) sync to spectators.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ContainerSyncHandler.java	New container sync, including slot listeners to push container slot changes.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/SyncPackets.java	Registers the new container sync payload.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/ServerSyncController.java	Initializes new handlers (container/inventory/cursor) and centralizes broadcast/send logic.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/mixin/ServerPlayerMixin.java	Hooks camera changes + menu open/close to manage container sync lifecycle and initial inventory sync.
fabric/src/main/java/com/hpfxd/spectatorplus/fabric/mixin/ServerGamePacketListenerImplMixin.java	Hooks container click handling to sync cursor after interactions.
fabric/src/client/resources/spectatorplus.client.mixins.json	Registers new ScreenMixin on the client.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/util/EffectUtil.java	Updates comments/cleanup in effect utility logic.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/sync/screen/ScreenSyncData.java	Expands screen sync data model to include container type/size/items and helper methods.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/sync/screen/ScreenSyncController.java	Adds container screen opening, container packet handling, and refactors inventory population helpers.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/sync/ClientSyncData.java	Renames/adjusts screen creation helper (createScreenIfNull).
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/sync/ClientSyncController.java	Refactors sync-data lifecycle API (createSyncDataIfNull) and updates packet handlers accordingly.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/screen/ScreenMixin.java	Sends a close notification packet when inventory/container screens are closed.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/screen/MenuScreensMixin.java	Adjusts screen creation interception to open synced container screens using container type/size.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/screen/AbstractContainerScreenMixin.java	Adds per-tick syncing of container slot contents (and cursor) into synced screens.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/MinecraftMixin.java	Updates open-inventory notification packet to include explicit “opened” boolean.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/gui/screens/SyncedInventoryScreen.java	Refactors inventory-slot syncing to use new sync data access patterns.
fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/gui/screens/DummyContainer.java	Reworks dummy container backing storage for variable-sized containers.

Comments suppressed due to low confidence (10)

fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/screen/MenuScreensMixin.java:80

• In the container-screen branch, the method doesn’t return after openContainerScreen(...), so execution falls through to the “Unable to open” cleanup and sends ServerboundContainerClosePacket, clears sync state, and resets isPendingOpen. This will immediately close the container you just opened. Add a return (or restructure the control flow) once the container screen is successfully opened.

                        // Handle container screens
                        if (ClientSyncController.syncData.screen.containerType != null) {
                            // Open container screen with proper type and size
                            ScreenSyncController.openContainerScreen(mc,
                                ClientSyncController.syncData.screen.containerType,
                                ClientSyncController.syncData.screen.containerSize);
                        } else {
                            // Fallback to original screen creation for unknown container types
                            final Inventory inventory;
                            if (hasInventory) {
                                inventory = ScreenSyncController.syncedInventory;
                            } else {
                                inventory = mc.player.getInventory();
                            }

                            final M menu = type.create(windowId, inventory);
                            final S screen = screenConstructor.create(menu, inventory, title);

                            ScreenSyncController.handleNewSyncedScreen(mc, screen);
                        }
                    }
                }
            }

            // Unable to open, immediately tell the server we've closed this screen.
            mc.getConnection().send(new ServerboundContainerClosePacket(windowId));
            ClientSyncController.syncData.screen = null;
            ScreenSyncController.isPendingOpen = false;

fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/mixin/screen/ScreenMixin.java:24

• This mixin sends ServerboundOpenedInventorySyncPacket(false) for any AbstractContainerScreen close. For non-inventory containers, the server already detects open/close via ServerPlayer.openMenu / doCloseContainer injections, so this produces duplicate close events (and extra network traffic). Consider limiting this packet to closing the player inventory (e.g., InventoryScreen / InventoryMenu) or otherwise gating it so container closes aren’t reported twice.

    @Inject(method = "onClose()V", at = @At("HEAD"))
    private void spectatorplus$onScreenClose(CallbackInfo ci) {
        // Check if this is an inventory or container screen being closed
        if ((Object) this instanceof InventoryScreen || (Object) this instanceof AbstractContainerScreen) {
            // Notify server that we closed our inventory/container
            if (ClientPlayNetworking.canSend(ServerboundOpenedInventorySyncPacket.TYPE)) {
                ClientPlayNetworking.send(new ServerboundOpenedInventorySyncPacket(false));
            }
        }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ScreenSyncHandler.java:69

• syncScreenOpened iterates spectators and then calls helper methods that themselves broadcast to all spectators (InventorySyncHandler.sendPacket/syncPlayerInventoryScreen and ContainerSyncHandler.sendPacket). This results in the same packets being sent N times when N spectators are watching. Refactor so per-spectator work uses ServerSyncController.sendPacket(spectator, ...), and broadcast packets are sent once per target screen change (outside the loop).

    public static void syncScreenOpened(ServerPlayer target) {
        var spectators = ServerSyncController.getSpectators(target);
        if (spectators.isEmpty()) {
            return;
        }

        for (ServerPlayer spectator : spectators) {
            // Determine what type of screen the target player has open
            if (target.containerMenu != target.inventoryMenu) {
                // Player has a container open (chest, crafting table, furnace, etc.)
                ContainerSyncHandler.sendPacket(spectator, target);

            } else {
                // Player has their regular inventory open (survival/creative inventory)
                syncPlayerInventoryScreen(spectator, target);
            }
        }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ScreenSyncHandler.java:91

• syncScreenClosed broadcasts the close ClientboundScreenSyncPacket inside the spectator loop. Since broadcastPacketToSpectators already loops spectators, this sends duplicate close packets (N times). Compute flags once and broadcast once after unsubscribing listeners (or send directly to the specific spectator).

        for (ServerPlayer spectator : spectators) {
            ContainerSyncHandler.unsubscribeFromContainer(spectator, target);

            // Send screen sync packet to indicate inventory/container was closed
            int flags = 0; // No flags means close screen
            flags |= (1 << 3); // Screen closed flag
            ServerSyncController.broadcastPacketToSpectators(target, new ClientboundScreenSyncPacket(target.getUUID(), flags));
        }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/InventorySyncHandler.java:88

• sendPacket(ServerPlayer spectator, ServerPlayer target) ignores the spectator parameter and broadcasts the inventory to all spectators of target. This is surprising at call sites (e.g., request-open handling) and also contributes to duplicate sends when called inside spectator loops. Either rename the method to reflect broadcasting, or change it to ServerSyncController.sendPacket(spectator, ...) and keep a separate broadcast helper.

    //this for send full inventory packet immediately
    public static void sendPacket(ServerPlayer spectator, ServerPlayer target) {
        final ItemStack[] slots = extractPlayerInventory(target);
        ServerSyncController.broadcastPacketToSpectators(target, new ClientboundInventorySyncPacket(target.getUUID(), slots));
    }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ContainerSyncHandler.java:62

• sendPacket(ServerPlayer spectator, ServerPlayer target) broadcasts container + screen sync packets to all spectators of target, even though it takes a specific spectator and is called from per-spectator loops. This amplifies traffic and can result in each spectator receiving the same update multiple times. Consider sending the sync packets directly to spectator (via ServerSyncController.sendPacket) and only broadcasting when that’s explicitly intended.

    public static void sendPacket(ServerPlayer spectator, ServerPlayer target) {
        InventorySyncHandler.sendPacket(spectator, target);
        var containerMenu = target.containerMenu;
        if (containerMenu == target.inventoryMenu) {
            return; // No container open, just player inventory
        }
        ItemStack[] containerItems = extractContainerItems(containerMenu);
        ServerSyncController.broadcastPacketToSpectators(target, new ClientboundContainerSyncPacket(
            target.getUUID(),
            containerMenu.getType(),
            containerItems.length,
            containerItems
        ));

        int flags = 0; // Container screen (not survival inventory)
        flags |= (1 << 2); // Has dummy slots flag for container screens
        ServerSyncController.broadcastPacketToSpectators(target, new ClientboundScreenSyncPacket(target.getUUID(), flags));

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/ContainerSyncHandler.java:31

• The disconnect handler only removes the listener from containerListeners, but doesn’t detach it from the target containerMenu. If a spectator disconnects while the target container remains open, the AbstractContainerMenu will retain a stale listener until the container closes. Consider storing enough context (e.g., target/menu reference) to call removeSlotListener on disconnect as well.

    public static void init() {
        // Clean up listeners when a player disconnects.
        ServerPlayConnectionEvents.DISCONNECT.register((handler, server) -> {
            containerListeners.remove(handler.getPlayer().getUUID());
        });

fabric/src/client/java/com/hpfxd/spectatorplus/fabric/client/sync/screen/ScreenSyncController.java:121

• closeSyncedInventory() unconditionally does mc.setScreen(null). If the spectator currently has a different GUI open when a close-sync packet arrives, this will unexpectedly close that unrelated screen. Consider only closing when mc.screen == syncedScreen (or when isPendingOpen/a synced screen is active), and otherwise just reset the internal sync state.

    public static void closeSyncedInventory() {
        final Minecraft mc = Minecraft.getInstance();
        mc.setScreen(null);
        syncedScreen = null;

        syncedInventory = null;
        syncedWindowId = Integer.MIN_VALUE;
        isPendingOpen = false;
        syncData.screen = null;
    }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/sync/handler/CursorSyncHandler.java:56

• Cursor change detection uses ItemStack.isSameItem(oldCursor, newCursor), which ignores count and components/NBT. This can miss updates (e.g., stack size changes while dragging, or NBT changes). Use a comparison consistent with the rest of syncing (e.g., ItemStack.matches or isSameItemSameComponents plus count) so spectators see accurate cursor state.

    public static void onCursorChanged(ServerPlayer player, ItemStack newCursor, int originSlot) {
        ItemStack oldCursor = playerCursors.get(player.getUUID());
        if  (oldCursor == null) {
            oldCursor = ItemStack.EMPTY;
        }
        if (newCursor == null) {
            newCursor = ItemStack.EMPTY;
        }

        if (!ItemStack.isSameItem(oldCursor, newCursor)) {
            playerCursors.put(player.getUUID(), newCursor.copy());

            ClientboundScreenCursorSyncPacket packet = new ClientboundScreenCursorSyncPacket(
                    player.getUUID(),
                    newCursor,
                    originSlot
            );

            ServerSyncController.broadcastPacketToSpectators(player, packet);
        }

fabric/src/main/java/com/hpfxd/spectatorplus/fabric/mixin/ServerPlayerMixin.java:7

• This introduces a wildcard import (com.hpfxd.spectatorplus.fabric.sync.handler.*). Elsewhere in the same package mixins use explicit handler imports (e.g., LivingEntityMixin imports EffectsSyncHandler directly), so keeping explicit imports here would better match existing style and avoid pulling in unused types.

import com.hpfxd.spectatorplus.fabric.sync.ServerSyncController;
import com.hpfxd.spectatorplus.fabric.sync.handler.*;
import com.hpfxd.spectatorplus.fabric.sync.packet.ClientboundExperienceSyncPacket;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

clearContent() calls this.items.clear(), which shrinks the backing list to size 0. This breaks the Container contract (and makes getContainerSize() change), likely leading to index errors in menus. Instead, keep the list size and set all entries to ItemStack.EMPTY (or reinitialize items to the original size).

Suggested change

	this.items.clear();
	for (int i = 0; i < this.items.size(); i++) {
	this.items.set(i, ItemStack.EMPTY);
	}

[Copilot]

createContainerListener is registered once per spectator, but slotChanged calls broadcastPacketToSpectators. With multiple spectators, each slot change will be broadcast multiple times (once per listener), causing O(N) duplicate packets per update. Either broadcast from a single shared listener per target, or have each per-spectator listener send only to its own spectator (ServerSyncController.sendPacket).