diff --git a/MinecraftClient/McClient.cs b/MinecraftClient/McClient.cs index 198d15d0..e19a0260 100644 --- a/MinecraftClient/McClient.cs +++ b/MinecraftClient/McClient.cs @@ -54,6 +54,7 @@ namespace MinecraftClient private readonly List bots = new(); private static readonly List botsOnHold = new(); private static readonly Dictionary inventories = new(); + private static readonly HashSet inventoriesWithFullContents = new(); private readonly Dictionary unlockedRecipes = new(StringComparer.Ordinal); private readonly Dictionary achievements = new(StringComparer.Ordinal); private string? activeAdvancementTab; @@ -2067,12 +2068,12 @@ namespace MinecraftClient firstWindowSlot = -1; lastWindowSlot = -1; - if (inventory.Type == ContainerType.PlayerInventory) + if (inventory.Type is ContainerType.PlayerInventory or ContainerType.Unknown) return false; const int mirroredPlayerInventorySlotCount = 36; int slotCount = inventory.Type.SlotCount(); - if (slotCount < mirroredPlayerInventorySlotCount) + if (slotCount <= mirroredPlayerInventorySlotCount) return false; firstWindowSlot = slotCount - mirroredPlayerInventorySlotCount; @@ -2080,20 +2081,6 @@ namespace MinecraftClient return true; } - private static bool TryGetMirroredPlayerInventorySlot(Container inventory, int windowSlot, out int playerInventorySlot) - { - playerInventorySlot = -1; - - if (!TryGetMirroredPlayerInventoryRange(inventory, out int firstWindowSlot, out int lastWindowSlot)) - return false; - - if (windowSlot < firstWindowSlot || windowSlot > lastWindowSlot) - return false; - - playerInventorySlot = windowSlot - firstWindowSlot + 9; - return true; - } - private static bool AreSameInventorySlot(Item? left, Item? right) { if (left is null || left.IsEmpty) @@ -2113,41 +2100,67 @@ namespace MinecraftClient if (!inventories.TryGetValue(0, out Container? playerInventory)) return false; + if (item is null || item.IsEmpty) + return playerInventory.Items.Remove(playerInventorySlot); + + Item itemClone = item.CloneWithCount(item.Count); + playerInventory.Items.TryGetValue(playerInventorySlot, out Item? previousItem); - if (AreSameInventorySlot(previousItem, item)) + if (AreSameInventorySlot(previousItem, itemClone)) return false; - if (item is null || item.IsEmpty) - playerInventory.Items.Remove(playerInventorySlot); - else - playerInventory.Items[playerInventorySlot] = item; + playerInventory.Items[playerInventorySlot] = itemClone; return true; } - private bool SyncPlayerInventorySlotFromWindow(Container? inventory, int windowSlot) - { - if (inventory is null) - return false; - - if (!TryGetMirroredPlayerInventorySlot(inventory, windowSlot, out int playerInventorySlot)) - return false; - - inventory.Items.TryGetValue(windowSlot, out Item? item); - return SetPlayerInventorySlot(playerInventorySlot, item); - } - private bool SyncPlayerInventorySlotsFromWindow(Container? inventory) { if (inventory is null) return false; + if (!inventoriesWithFullContents.Contains(inventory.ID)) + return false; + if (!TryGetMirroredPlayerInventoryRange(inventory, out int firstWindowSlot, out int lastWindowSlot)) return false; - bool changed = false; + if (!inventories.TryGetValue(0, out Container? playerInventory)) + return false; + + const int firstPlayerInventorySlot = 9; + const int lastPlayerInventorySlot = firstPlayerInventorySlot + 36 - 1; + Dictionary mirroredItems = new(); + for (int windowSlot = firstWindowSlot; windowSlot <= lastWindowSlot; windowSlot++) - changed |= SyncPlayerInventorySlotFromWindow(inventory, windowSlot); + { + if (!inventory.Items.TryGetValue(windowSlot, out Item? item) || item.IsEmpty) + continue; + + int playerInventorySlot = windowSlot - firstWindowSlot + firstPlayerInventorySlot; + mirroredItems[playerInventorySlot] = item.CloneWithCount(item.Count); + } + + bool changed = false; + for (int playerInventorySlot = firstPlayerInventorySlot; playerInventorySlot <= lastPlayerInventorySlot; playerInventorySlot++) + { + playerInventory.Items.TryGetValue(playerInventorySlot, out Item? previousItem); + mirroredItems.TryGetValue(playerInventorySlot, out Item? mirroredItem); + if (AreSameInventorySlot(previousItem, mirroredItem)) + continue; + + changed = true; + break; + } + + if (!changed) + return false; + + for (int playerInventorySlot = firstPlayerInventorySlot; playerInventorySlot <= lastPlayerInventorySlot; playerInventorySlot++) + playerInventory.Items.Remove(playerInventorySlot); + + foreach ((int playerInventorySlot, Item item) in mirroredItems) + playerInventory.Items[playerInventorySlot] = item; return changed; } @@ -2945,7 +2958,10 @@ namespace MinecraftClient if (inventories.ContainsKey(windowId)) { if (windowId != 0) + { inventories.Remove(windowId); + inventoriesWithFullContents.Remove(windowId); + } bool result = handler.SendCloseWindow(windowId); DispatchBotEvent(bot => bot.OnInventoryClose(windowId)); return result; @@ -2966,6 +2982,7 @@ namespace MinecraftClient return InvokeOnMainThread(ClearInventories); inventories.Clear(); + inventoriesWithFullContents.Clear(); inventories[0] = new Container(0, ContainerType.PlayerInventory, "Player Inventory"); ClearUnlockedRecipes(); return true; @@ -3778,6 +3795,7 @@ namespace MinecraftClient /// Inventory ID public void OnInventoryOpen(int inventoryID, Container inventory) { + inventoriesWithFullContents.Remove(inventoryID); inventories[inventoryID] = inventory; if (inventoryID != 0) @@ -3804,9 +3822,15 @@ namespace MinecraftClient if (inventories.ContainsKey(inventoryID)) { if (inventoryID == 0) + { inventories[0].Items.Clear(); // Don't delete player inventory + inventoriesWithFullContents.Clear(); + } else + { inventories.Remove(inventoryID); + inventoriesWithFullContents.Remove(inventoryID); + } } if (inventoryID != 0) @@ -3938,6 +3962,7 @@ namespace MinecraftClient inventories[inventoryID].Items = itemList; inventories[inventoryID].StateID = stateId; + inventoriesWithFullContents.Add(inventoryID); bool playerInventoryChanged = SyncPlayerInventorySlotsFromWindow(inventories[inventoryID]); if (playerInventoryChanged) DispatchBotEvent(bot => bot.OnInventoryUpdate(0)); @@ -3965,7 +3990,7 @@ namespace MinecraftClient inventoryID = 0; // Prevent key not found for some bots relied to this event if (inventories.ContainsKey(0)) { - if (item is not null) + if (item is not null && !item.IsEmpty) inventories[0].Items[-1] = item; else inventories[0].Items.Remove(-1); @@ -3982,7 +4007,7 @@ namespace MinecraftClient } else inventories[inventoryID].Items[slotID] = item; - if (SyncPlayerInventorySlotFromWindow(inventories[inventoryID], slotID)) + if (SyncPlayerInventorySlotsFromWindow(inventories[inventoryID])) DispatchBotEvent(bot => bot.OnInventoryUpdate(0)); } } diff --git a/inventory-sync-issue-report.md b/inventory-sync-issue-report.md new file mode 100644 index 00000000..2d743820 --- /dev/null +++ b/inventory-sync-issue-report.md @@ -0,0 +1,377 @@ +# MCC inventory sync issues report + +## Sources read + +- GitHub issue #3112: https://github.com/MCCTeam/Minecraft-Console-Client/issues/3112 +- GitHub issue #3126: https://github.com/MCCTeam/Minecraft-Console-Client/issues/3126 +- GitHub issue #3124: https://github.com/MCCTeam/Minecraft-Console-Client/issues/3124 +- Related issue referenced by #3124: https://github.com/MCCTeam/Minecraft-Console-Client/issues/3119 +- Commit inspected: `3c59bfe13abc18291506bec59118b5bd7509f4eb` (`Experimental inventory sync`) + +## Short version + +There are three separate but related problems in the reports: + +1. Before commit `3c59bfe`, `/inventory container list` could be correct while `/inventory player list` stayed stale. The script used `GetPlayerInventory()`, so it believed moved or trashed items were still in the player inventory and repeated the same actions. +2. Commit `3c59bfe` tried to fix that by mirroring the player-inventory part of an open container back into inventory id `0`. That addressed the stale player list in the first test, but it introduced or exposed `x0` ghost items because the mirrored player inventory can share the same `Item` object reference as the open container, and later local shift-click prediction can mutate that object to `Count = 0`. +3. Issue #3124 is mainly a Script Scheduler/reconnect duplication issue. It matters here because the posted `WarpAFK.cs` scripts show the user process and the workaround they wrote for the stale inventory problem, but the scheduler bug itself is not fixed by `3c59bfe`. + +## What the issue is exactly + +MCC keeps inventories in a dictionary keyed by server window id. Inventory id `0` is the player inventory. Open containers use separate ids, for example the `/trash` or `/çöp` GUI and the `/is chest N` GUI. + +In Minecraft protocol, an open container window contains both: + +- the container's own top slots +- a mirrored copy of the player's normal 36 inventory slots + +For example: + +- A 27-slot chest (`Generic_9x3`) has 63 total slots in MCC: 27 chest slots plus 36 mirrored player slots. +- A 36-slot trash GUI (`Generic_9x4`) has 72 total slots in MCC: 36 trash slots plus 36 mirrored player slots. +- The mirrored player slots begin after the container's own top slots. + +The user's old script scans `GetPlayerInventory()` to decide what to do, then it clicks the mirrored player slot in the open container: + +- For `/çöp`, the GUI has 36 top slots, so `containerSlot = playerSlot + 27`. Example: player slot `36` maps to container slot `63`. +- For `/is chest N`, the GUI has 27 top slots, so `containerSlot = playerSlot + 18`. Example: player slot `36` maps to container slot `54`. + +Before `3c59bfe`, `DoWindowAction()` updated only the clicked container object. If the script clicked the open trash/chest container, MCC's current container snapshot was updated, but inventory id `0` was not updated from that container's mirrored player slots. Therefore: + +- `/inventory container list` showed the item removed. +- `/inventory player list` still showed the old item. +- `GetPlayerInventory()` returned the stale id `0` state. +- The bot returned to trash/storage mode and clicked the same logical item slots again. + +This is the core bug from #3112. + +## How to reproduce #3112 manually by hand + +These steps avoid the automation script and use only MCC commands. They are based on the user's server and process from #3112. + +Prerequisites: + +- Connect MCC to `play.ronemacraft.com`. +- Use Minecraft `1.20.4`. +- Use a build before the experimental inventory sync fix, for example the reported MCC version 445 or another build before `3c59bfe`. +- Have inventory handling enabled. +- Have at least one item that the server can move into the trash GUI or an island chest. + +Trash GUI reproduction: + +1. Stand still in the Opskyblock/AFK area where the user observed the problem. +2. Put an ore item in the normal player inventory, for example `EmeraldOre`, `IronOre`, `GoldOre`, or `DiamondOre`. +3. Run `/inventory player list`. +4. Note the player slot that contains the ore. For a hotbar item this is usually in slots `36` through `44`. +5. Send `/çöp` or the server's `/trash` alias. +6. Wait until MCC logs that a virtual trash inventory opened. +7. Run `/inventory container list`. +8. Find the same item in the container's mirrored player section. For the 36-slot trash GUI, use `containerSlot = playerSlot + 27`. +9. Run `/inventory container click ShiftClick`. +10. Run `/inventory container list` again. The container view should show that the item was removed or moved. +11. Run `/inventory player list`. +12. On the broken builds, the player list can still show the item in the old player slot. +13. Close the container with `/inventory container close`. +14. Open `/çöp` again and repeat the same click. The server-side item is already gone, but MCC still thinks it exists because id `0` is stale. + +Island chest reproduction: + +1. Put any non-ore item in the player inventory. +2. Run `/inventory player list` and note the player slot. +3. Send `/opskyblock` if needed by the server flow. +4. Send `/is chest 21`. +5. Wait until the chest opens. +6. For the 27-slot chest GUI, compute `containerSlot = playerSlot + 18`. +7. Run `/inventory container click ShiftClick`. +8. Run `/inventory container list`. The open container view should reflect the move. +9. Run `/inventory player list`. On affected builds, the item can still be listed in the player inventory. +10. The user's script then sees `hasOther = true`, tries the next chest, and repeats until its own workaround stops it. + +Why moving or warping seems to fix it: + +- The user said `/spawn` to `/warp afk5` refreshes `/inventory player list`. +- That is consistent with the server resending player inventory or slot state during a world/lobby/teleport transition. +- It is not a useful workaround for the user because leaving the AFK location resets the 30-minute reward counter. + +## How issue #3124 fits in + +Issue #3124 is not the same inventory defect. It is a continuation of #3119, where Script Scheduler can re-run login-triggered scripts during `/reco` or reconnect spam while old script instances keep running. + +The relevant Script Scheduler config from #3124 is: + +- `Trigger_On_Login = true` +- One task sends `/login password`. +- Another task runs `script WarpAFK.cs`. + +Manual scheduler reproduction: + +1. Enable `ChatBot.ScriptScheduler`. +2. Add a login-triggered task that starts a script, like the `WarpAFK.cs` shown in #3124. +3. Connect to the server. +4. Repeatedly run `/reco`, or force several disconnect/reconnect cycles. +5. Watch the log for multiple script instances continuing at once. +6. Expected behavior would be that old script instances are stopped before a new login-triggered instance starts. +7. Actual behavior from #3119/#3124 was repeated commands such as `/opskyblock`, `/pay`, `/is chest 21`, and `/warp afk5`, sometimes before MCC considers itself fully connected. + +Why this matters for the inventory report: + +- The first `WarpAFK.cs` in #3124 contains slot-recorder logic (`doneSlotTypes`, `confirmedPSlots`, `failedPSlots`) specifically because `/inventory player list` was unreliable. +- The later `WarpAFK.cs` posted in #3124 is simpler because the user believed the new player inventory sync worked. It goes back to scanning `GetPlayerInventory()` for `PlayerItems()`. +- That means the experimental inventory sync directly changed how the script could be written, even though the reconnect duplication is a separate bug. + +## What changed in commit 3c59bfe + +Commit `3c59bfe13abc18291506bec59118b5bd7509f4eb` is titled `Experimental inventory sync`. + +Files changed: + +- `MinecraftClient/McClient.cs` +- `MinecraftClient/Resources/Translations/Translations.resx` +- `MinecraftClient/Resources/Translations/Translations.Designer.cs` + +Main inventory changes: + +1. Fixed a merge-order bug in `TryMergeSlot()`. + - Before the commit, the full-merge branch set `item.Count = 0` before adding it to the destination stack. That meant the destination received `0`. + - After the commit, it adds `item.Count` to `curItem.Count` first, then sets `item.Count = 0`. + - Current code: `MinecraftClient/McClient.cs`, around lines 1998-2014. + +2. Added mirrored player-inventory range detection. + - `TryGetMirroredPlayerInventoryRange()` assumes non-player containers end with 36 mirrored player inventory slots. + - `TryGetMirroredPlayerInventorySlot()` maps a container window slot back to a player inventory slot using `playerInventorySlot = windowSlot - firstWindowSlot + 9`. + - Current code: `MinecraftClient/McClient.cs`, around lines 2065-2095. + +3. Added `SetPlayerInventorySlot()`. + - This updates inventory id `0` for a mapped player slot. + - It removes the slot when the incoming item is null or empty. + - It skips work when `AreSameInventorySlot()` says the old and new slot are equivalent. + - Current code: `MinecraftClient/McClient.cs`, around lines 2097-2125. + +4. Added sync from open containers to player inventory id `0`. + - `SyncPlayerInventorySlotFromWindow()` syncs one mapped mirrored slot. + - `SyncPlayerInventorySlotsFromWindow()` syncs all mirrored slots. + - `DoWindowAction()` now calls `SyncPlayerInventorySlotsFromWindow(inventory)` before sending the click packet. + - `OnWindowItems()` now syncs the whole mirrored range and dispatches `OnInventoryUpdate(0)` if changed. + - `OnSetSlot()` now syncs a mapped mirrored slot and dispatches `OnInventoryUpdate(0)` if changed. + - Current code: `MinecraftClient/McClient.cs`, around lines 2128-2152, 2896, and 3931-3989. + +5. Added empty-item filtering to `OnWindowItems()`. + - The commit removes items from incoming full window contents when `Item.IsEmpty` is true. + - Current code: `MinecraftClient/McClient.cs`, around lines 3935-3937. + +6. Added cursor cleanup for left/right click predictions. + - If cursor slot `-1` reaches zero count, it is removed. + - Current code: `MinecraftClient/McClient.cs`, around lines 2219-2221 and 2280-2282. + +7. Added shift-click cleanup. + - If a shift-clicked source item reaches zero count and is still present in the clicked container dictionary, it is removed. + - Current code: `MinecraftClient/McClient.cs`, around lines 2868-2872. + +Translation changes: + +- `cmd.inventory.shiftclick` changed from `Shift clicking slot {0} in window #{1}` to `Shift`. +- `cmd.inventory.shiftrightclick` changed from `Shift right-clicking slot {0} in window #{1}` to `Shift right`. +- This fixes the nested format-string problem from #3126. The outer message is `{0} clicking slot {1} in window #{2}`, so the action label must not contain its own `{0}` and `{1}` placeholders. +- Current command formatting is in `MinecraftClient/Commands/Inventory.cs`, around lines 355-365. + +## Why the commit helped #3112 + +The main stale-list failure was that id `0` did not follow the mirrored player section inside nonzero container windows. + +The new sync code makes this happen: + +1. Open `/çöp` or `/is chest N`. +2. The open container's full slot list includes the player's mirrored 36 slots. +3. `OnWindowItems()` copies that mirrored section into player inventory id `0`. +4. After a click, `DoWindowAction()` locally predicts the container change and syncs mirrored slots into id `0`. +5. Bot scripts reading `GetPlayerInventory()` now see the result of the container click. + +This is why the test build mentioned in #3112 appeared to work for the user's old script. + +## Why the commit did not fully work + +The commit can leave `x0` items in `/inventory player list`. + +The most likely concrete cause is object aliasing plus the equality check in `SetPlayerInventorySlot()`. + +Detailed flow: + +1. `OnWindowItems()` receives an open container, for example `/is chest 21`. +2. `SyncPlayerInventorySlotsFromWindow()` maps the mirrored player slots from that container into inventory id `0`. +3. `SetPlayerInventorySlot()` stores the same `Item` object reference from the container dictionary into the player inventory dictionary. It does not clone the item. +4. The user shift-clicks a mirrored player slot in the open container. +5. If the destination already has the same stackable item, `TryMergeSlot()` fully merges the source stack. +6. In that full-merge branch, the code sets the source `item.Count = 0`. +7. Because player inventory id `0` may hold the same `Item` object reference, the player inventory entry now also has `Count = 0`. +8. `DoWindowAction()` removes the source slot from the open container and calls `SyncPlayerInventorySlotsFromWindow()`. +9. The sync sees the window slot is now empty and tries to set the mapped player slot to `null`. +10. `SetPlayerInventorySlot()` calls `AreSameInventorySlot(previousItem, null)`. +11. `AreSameInventorySlot()` returns true when the previous item is empty and the new item is null. +12. Because it returns true, `SetPlayerInventorySlot()` exits early and does not remove the existing dictionary entry. +13. `/inventory player list` iterates the dictionary directly and prints the item, so it appears as `x0`. + +This matches #3126 very closely: + +- The user bought one grassy soil from `/market` or `/shop`. +- The script stored it in `/is chest`. +- The first item may have moved into an empty chest slot, so no `x0` appeared. +- The user bought the same item again. +- The second item could merge into the existing chest stack, causing `TryMergeSlot()` to set the source count to zero. +- The player inventory dictionary retained the zero-count item, so `/inventory player list` showed `x0`. + +It also explains why the issue is intermittent: + +- Moving into an empty destination slot uses `StoreInNewSlot()`, which removes the source slot but does not set the source item count to zero. +- Fully merging into an existing stack uses `TryMergeSlot()`, which does set the source item count to zero. +- Therefore the visible result depends on whether the item moved into an empty slot or merged into an existing compatible stack. + +## How to reproduce #3126 manually by hand + +Use the experimental build containing `3c59bfe`. + +Server-specific reproduction from the issue: + +1. Connect to `play.ronemacraft.com` on Minecraft `1.20.4`. +2. Go to the Opskyblock area where `/market` or `/shop` is available. +3. Buy exactly one item from `/market` or `/shop`. The user specifically mentioned grassy soil. +4. Run `/inventory player list` and note the player slot containing the item. +5. Open an island chest with `/is chest 21`. +6. Use `/inventory container list` to confirm the item appears in the mirrored player section. +7. Compute the container slot as `playerSlot + 18` for a 27-slot chest. +8. Run `/inventory container click ShiftClick`. +9. Close the container with `/inventory container close`. +10. Run `/inventory player list`. The item may disappear correctly. +11. Buy the same item again, exactly one item. +12. Open the same `/is chest 21` again. +13. Shift-click the second item into the chest using the same slot mapping. +14. Run `/inventory player list`. +15. If the second item merged into the existing stack in the chest, the player list can show that item as `x0`. + +Generic reproduction that should not require this specific server: + +1. Use a build containing `3c59bfe`. +2. Connect to any server where inventory actions work. +3. Put a partial stack of a stackable item in a chest, for example 1 dirt in a chest slot. +4. Put 1 dirt in the player's inventory. +5. Open the chest. +6. Run `/inventory player list` and note the player slot of the dirt. +7. For a normal 27-slot chest, compute `containerSlot = playerSlot + 18`. +8. Run `/inventory container click ShiftClick`. +9. Run `/inventory player list` quickly after the click. +10. The expected correct result is no dirt in the old player slot. +11. The suspected broken result is a retained entry with `x0 Dirt`. + +If this generic reproduction does not show `x0`, add logging or inspect the dictionaries immediately after `DoWindowAction()`. Server correction packets can sometimes mask the display timing, but the code path itself can retain a zero-count item. + +## Is this just an issue on this server? + +The stale `/inventory player list` issue is not purely a RonemaCraft server bug. It is a real MCC state-model gap: + +- MCC had separate snapshots for player inventory id `0` and open container ids. +- Open containers include mirrored player inventory slots. +- Before the commit, MCC did not sync those mirrored slots back into id `0`. +- Any server or plugin GUI that updates only the open container window can expose this mismatch. + +RonemaCraft makes the issue easy to notice because: + +- It uses plugin GUIs for `/çöp`, `/is chest`, `/market`, and daily rewards. +- The user's bot depends on `GetPlayerInventory()` while staying stationary in an AFK area. +- Warping away refreshes inventory but resets the AFK reward counter, so the stale state persists long enough to hurt the workflow. + +The `x0` issue from `3c59bfe` is also not inherently server-only. It follows from MCC's own local prediction and reference sharing. The server's workflow makes it reproducible because buying the same item twice and storing it into the same chest naturally creates a merge-into-existing-stack case. + +The server can still affect how often the bug appears: + +- Some servers send extra full inventory refreshes that mask stale id `0` state. +- Some plugin GUIs send unusual item stacks, custom names, or zero-count placeholders. +- Some server flows delay or omit player-window updates while only updating the open plugin window. + +So the right classification is: + +- #3112: general MCC design gap, exposed strongly by this server's plugin GUI workflow. +- #3126: general bug in the experimental fix, easiest to trigger with this server's repeated market/chest process. +- #3124/#3119: Script Scheduler/reconnect lifecycle bug, separate from inventory sync. + +## How the inventory system worked before this commit + +Before `3c59bfe`, the relevant flow was: + +1. MCC initializes the inventory dictionary with player inventory id `0`. + - `ClearInventories()` creates `inventories[0] = new Container(0, ContainerType.PlayerInventory, "Player Inventory")`. + - Current code is around `MinecraftClient/McClient.cs` lines 2960-2970. + +2. When the server opens a container, MCC adds another `Container` under that server window id. + - The container type comes from the protocol menu type id. + - The title comes from the packet title. + - For 1.20.4, this happens through `OpenWindow` handling in `Protocol18.cs`. + +3. Each `Container` has a flat `Items` dictionary. + - Key: slot id. + - Value: `Item`. + - Empty slots are normally absent from the dictionary. + +4. Container slot counts include the player's mirrored inventory section. + - `Generic_9x3` is 63 total slots: 27 top slots plus 36 player slots. + - `Generic_9x4` is 72 total slots: 36 top slots plus 36 player slots. + - `Generic_9x6` is 90 total slots: 54 top slots plus 36 player slots. + - `PlayerInventory` is 46 total slots. + - Current values are in `MinecraftClient/Inventory/ContainerTypeExtensions.cs`, around lines 10-40. + +5. `/inventory player list` reads only inventory id `0`. + - It iterates `inventory.Items` directly. + - It prints every dictionary entry, including an item whose count is zero. + - Current command code is around `MinecraftClient/Commands/Inventory.cs` lines 320-327. + +6. `/inventory container list` reads the highest open inventory id when no id is supplied. + - This is usually the foreground open server GUI. + - It does not read inventory id `0`. + +7. `OnWindowItems()` replaced only the addressed inventory's item dictionary. + - Before this commit, it did not filter zero-count items. + - Before this commit, it did not sync mirrored player slots into inventory id `0`. + - Before this commit, it dispatched only `OnInventoryUpdate(inventoryID)`. + +8. `OnSetSlot()` updated only the addressed inventory id, except special protocol cases. + - `inventoryID == 254` maps to player inventory id `0`. + - `inventoryID == 255` and slot `-1` maps to the cursor item stored in player inventory slot `-1`. + - Newer 1.21.2+ `SetPlayerInventory` packets map directly to id `0`, but the reported server uses 1.20.4, so that packet path is not available. + +9. `DoWindowAction()` locally predicted the result of clicks before sending the click packet. + - For a nonzero window id, it modified that open container's `Items`. + - It did not also update player inventory id `0`. + - It sent `ClickWindow` with the calculated changed slots and state id. + +10. Bot APIs reflect this model. + - `GetPlayerInventory()` returns inventory id `0`. + - `GetInventories()` returns the full dictionary of player plus open containers. + - `OnInventoryUpdate(int inventoryId)` tells bots which inventory id changed. + - Before the commit, a container mirror change did not imply an `OnInventoryUpdate(0)` event. + +The practical consequence before the commit: + +- If a script needed live item state while a container was open, `/inventory container list` or `GetInventories()[openContainerId]` was more reliable than `GetPlayerInventory()`. +- Scripts had to manually translate mirrored container slots back to player slots, exactly as the user's workaround did. +- `GetPlayerInventory()` was safe only after the server actually refreshed inventory id `0`. + +## Suggested direction for a real fix + +This report is not a patch, but the code evidence points to a safer direction: + +1. Do not store the same mutable `Item` instance in both the open container dictionary and player inventory id `0`. + - Clone items when syncing from a container mirror into player inventory. + +2. Make `SetPlayerInventorySlot()` remove an existing dictionary entry when the new item is null or empty, even if the old entry is already empty. + - The dictionary state matters. An empty item object in the dictionary is not equivalent to no dictionary entry for display and script behavior. + +3. Consider filtering empty items when listing inventories too. + - The data model should not keep `x0` entries, but display code can defensively skip `item.IsEmpty`. + +4. Keep the mirror sync idea, but add tests around these cases: + - Shift-click mirrored player slot into an empty container slot. + - Shift-click mirrored player slot into an existing compatible stack. + - Shift-click one item twice into the same chest stack. + - Open container `WindowItems` sync followed by local `DoWindowAction()`. + - Server `OnSetSlot(windowId, sourceSlot, null, stateId)` after local prediction already changed the source item to zero. + +5. Keep #3124 separate. + - Fixing inventory sync should not be expected to fix Script Scheduler duplicate script instances after reconnect.