From ffec050d968ded9b8f1862f45279e61a3fff0a56 Mon Sep 17 00:00:00 2001 From: ElliotHume Date: Thu, 6 Aug 2026 14:43:44 -0400 Subject: [PATCH 1/2] bug: fix CardCabinet not properly saving cards between restarts --- CHANGELOG.md | 9 +++ .../block/CardCabinetBlock.java | 11 ++- .../block/entity/CardCabinetBlockEntity.java | 75 ++++++++++++++++--- 3 files changed, 82 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8b0ac17..ad52704a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,15 @@ All major changes brought to **Cobblemon Cards** in this update. * The application mode and modifier maths are now centralized in `CardStatUtil` for every platform. * A config option exists to display non-formatted values, like in previous versions +### 🛠️ Critical Fix — Card Cabinet data loss +* **Card Cabinets no longer lose their contents on restart.** Introduced in 1.0.5; **1.0.4 and earlier are unaffected** and still load normally. +* Load failures are now logged with the offending NBT instead of being swallowed, and a failure to serialise a single stack can no longer abort the whole chunk save. +* Fixed `ItemStack` instances being shared between a cabinet's container and the `BINDER_CONTENTS` component when broken or placed. + + +### Master Album Config for Disabling Stats +* **New Config Option**: `doesMasterBinderProvideStats` — Set to `false` to disable stats from master tier binders. Due to their large card capacity. + --- ## 🚀 Version 1.0.1 (Multiloader & Easter Eggs Update) diff --git a/common/src/main/java/com/howlite/cobblemoncards/block/CardCabinetBlock.java b/common/src/main/java/com/howlite/cobblemoncards/block/CardCabinetBlock.java index 8824e153..8f7f0590 100644 --- a/common/src/main/java/com/howlite/cobblemoncards/block/CardCabinetBlock.java +++ b/common/src/main/java/com/howlite/cobblemoncards/block/CardCabinetBlock.java @@ -147,7 +147,10 @@ protected void onRemove(BlockState state, Level level, BlockPos pos, BlockState boolean isEmpty = true; for (int i = 0; i < cabinetEntity.getContainerSize(); i++) { ItemStack itemStack = cabinetEntity.getItem(i); - items.add(itemStack); + // Copy: the block entity is about to be discarded, but sharing the same + // ItemStack instances between the container and the item component means a + // later mutation on one side silently corrupts the other. + items.add(itemStack.copy()); if (!itemStack.isEmpty()) { isEmpty = false; } @@ -183,11 +186,13 @@ public void setPlacedBy(Level level, BlockPos pos, BlockState state, @Nullable n if (savedItems != null) { net.minecraft.core.NonNullList inventoryList = cabinetEntity.getItems(); for (int i = 0; i < Math.min(savedItems.size(), 12000); i++) { - ItemStack itemStack = savedItems.get(i); - inventoryList.set(i, itemStack); + // Copy: BINDER_CONTENTS is an immutable component list, and its stacks must + // never be handed out by reference to a mutable container. + ItemStack itemStack = savedItems.get(i).copy(); if (itemStack.getCount() > cabinetEntity.getMaxStackSize()) { itemStack.setCount(cabinetEntity.getMaxStackSize()); } + inventoryList.set(i, itemStack); } cabinetEntity.setChanged(); } diff --git a/common/src/main/java/com/howlite/cobblemoncards/block/entity/CardCabinetBlockEntity.java b/common/src/main/java/com/howlite/cobblemoncards/block/entity/CardCabinetBlockEntity.java index 3a94aba3..16eb32ca 100644 --- a/common/src/main/java/com/howlite/cobblemoncards/block/entity/CardCabinetBlockEntity.java +++ b/common/src/main/java/com/howlite/cobblemoncards/block/entity/CardCabinetBlockEntity.java @@ -1,5 +1,6 @@ package com.howlite.cobblemoncards.block.entity; +import com.howlite.cobblemoncards.CobblemonCards; import com.howlite.cobblemoncards.block.CardCabinetBlock; import com.howlite.cobblemoncards.menu.CardCabinetMenu; import net.minecraft.core.BlockPos; @@ -51,33 +52,85 @@ protected void loadAdditional(CompoundTag tag, HolderLookup.Provider registries) public static CompoundTag saveCabinetItems(CompoundTag tag, NonNullList items, HolderLookup.Provider registries) { ListTag listTag = new ListTag(); + int skipped = 0; for (int i = 0; i < items.size(); i++) { ItemStack stack = items.get(i); - if (!stack.isEmpty()) { + if (stack.isEmpty()) { + continue; + } + try { CompoundTag itemTag = new CompoundTag(); itemTag.putInt("Slot", i); - stack.save(registries, itemTag); - listTag.add(itemTag); + // ItemStack#save does NOT mutate the tag it is given: NbtOps#mergeToMap makes a + // shallow copy and returns the merged result. Dropping the return value here + // means only {Slot: n} is written and every card is lost on the next load. + listTag.add(stack.save(registries, itemTag)); + } catch (Exception e) { + // Never let a single bad stack abort the whole chunk save (that would wipe + // far more than the cabinet). + skipped++; + CobblemonCards.LOGGER.error("[CardCabinet] Failed to save item in slot {} ({}), it will be lost", + i, stack, e); } } + if (skipped > 0) { + CobblemonCards.LOGGER.error("[CardCabinet] {} item(s) could not be serialized and were dropped from the save", skipped); + } tag.put("Items", listTag); return tag; } public static void loadCabinetItems(CompoundTag tag, NonNullList items, HolderLookup.Provider registries) { items.clear(); + if (!tag.contains("Items", Tag.TAG_LIST)) { + // No inventory tag at all: either a brand-new cabinet, or the block entity NBT + // never made it to disk. Both are worth knowing about when cards go missing. + CobblemonCards.LOGGER.debug("[CardCabinet] No 'Items' tag found while loading a cabinet"); + return; + } ListTag listTag = tag.getList("Items", Tag.TAG_COMPOUND); + int loaded = 0; + int failed = 0; for (int i = 0; i < listTag.size(); i++) { CompoundTag itemTag = listTag.getCompound(i); int slot; if (itemTag.contains("Slot", Tag.TAG_INT)) { slot = itemTag.getInt("Slot"); } else { + // Written by ContainerHelper.saveAllItems in 1.0.4 and earlier: the slot index is + // a byte, so only the first 256 slots survived that format. slot = itemTag.getByte("Slot") & 0xFF; } - if (slot >= 0 && slot < items.size()) { - items.set(slot, ItemStack.parse(registries, itemTag).orElse(ItemStack.EMPTY)); + if (slot < 0 || slot >= items.size()) { + failed++; + CobblemonCards.LOGGER.error("[CardCabinet] Discarding item with out-of-range slot {} (size {})", slot, items.size()); + continue; + } + // ItemStack.parse() returns Optional.empty() on ANY decoding problem (unknown + // item id, invalid count, a data component whose codec rejected the data...). + // Swallowing that silently is exactly what makes a cabinet look "wiped", so log it. + if (!itemTag.contains("id")) { + // Written by a build affected by the "ItemStack#save return value discarded" + // bug: only {Slot: n} made it to disk, so the card itself is unrecoverable. + failed++; + CobblemonCards.LOGGER.error( + "[CardCabinet] Slot {} was saved without an item id (corrupted by a pre-fix build) and cannot be restored", + slot); + continue; } + java.util.Optional parsed = ItemStack.parse(registries, itemTag); + if (parsed.isEmpty()) { + failed++; + CobblemonCards.LOGGER.error("[CardCabinet] Failed to parse item for slot {}; raw tag = {}", slot, itemTag); + continue; + } + items.set(slot, parsed.get()); + loaded++; + } + if (failed > 0) { + CobblemonCards.LOGGER.error("[CardCabinet] Loaded {} item(s), but {} entrie(s) failed to load and were lost", loaded, failed); + } else { + CobblemonCards.LOGGER.debug("[CardCabinet] Loaded {} item(s)", loaded); } } @@ -89,9 +142,13 @@ public Packet getUpdatePacket() { @Override public CompoundTag getUpdateTag(HolderLookup.Provider registries) { - CompoundTag tag = super.getUpdateTag(registries); - saveAdditional(tag, registries); - return tag; + // IMPORTANT: do NOT send the inventory to clients here. + // This cabinet has 12 000 slots; serialising them into every block-entity update + // packet (and into every chunk packet) can easily produce multi-megabyte payloads, + // which throws "Packet too big" on the encoder and kicks players. + // The client renderer only needs the block state (FACING / FILL_LEVEL) and the + // contents are synced through CardCabinetMenu when the GUI is open. + return super.getUpdateTag(registries); } @Override @@ -105,8 +162,6 @@ public void setChanged() { level.setBlock(getBlockPos(), currentState.setValue(CardCabinetBlock.FILL_LEVEL, newFillLevel), 3); // flag 3 = update + notify neighbours (comparator) - } else { - level.sendBlockUpdated(getBlockPos(), currentState, currentState, 3); } level.updateNeighbourForOutputSignal(getBlockPos(), getBlockState().getBlock()); From 49eb3ea463b10768635ca51cce6fb9989adc228a Mon Sep 17 00:00:00 2001 From: ElliotHume Date: Thu, 6 Aug 2026 14:45:57 -0400 Subject: [PATCH 2/2] feat: add config option to disable stats from master tier binders --- .../com/howlite/cobblemoncards/CobblemonCardsConfig.java | 4 ++++ .../howlite/cobblemoncards/item/custom/BinderItem.java | 9 +++++++++ .../com/howlite/cobblemoncards/util/CardStatUtil.java | 2 +- .../fabric/item/FabricMasterAlbumItem.java | 4 ++++ .../neoforge/item/NeoForgeMasterAlbumItem.java | 4 ++++ 5 files changed, 22 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/com/howlite/cobblemoncards/CobblemonCardsConfig.java b/common/src/main/java/com/howlite/cobblemoncards/CobblemonCardsConfig.java index 43a414f9..9eab527f 100644 --- a/common/src/main/java/com/howlite/cobblemoncards/CobblemonCardsConfig.java +++ b/common/src/main/java/com/howlite/cobblemoncards/CobblemonCardsConfig.java @@ -49,6 +49,10 @@ public class CobblemonCardsConfig extends MidnightConfig { @Entry public static boolean displayPercentStatOnCards = true; + @Entry + public static boolean doesMasterBinderProvideStats = false; + + // --- Per-stat multipliers for the vanilla-attribute / player stats --- // Each of these is applied on top of globalStatMultiplier * playerStatMultiplier. diff --git a/common/src/main/java/com/howlite/cobblemoncards/item/custom/BinderItem.java b/common/src/main/java/com/howlite/cobblemoncards/item/custom/BinderItem.java index 3c6c0334..80953575 100644 --- a/common/src/main/java/com/howlite/cobblemoncards/item/custom/BinderItem.java +++ b/common/src/main/java/com/howlite/cobblemoncards/item/custom/BinderItem.java @@ -1,5 +1,6 @@ package com.howlite.cobblemoncards.item.custom; +import com.howlite.cobblemoncards.CobblemonCardsConfig; import com.howlite.cobblemoncards.component.CardStat; import com.howlite.cobblemoncards.util.PlatformHelper; import net.minecraft.ChatFormatting; @@ -75,6 +76,14 @@ public void appendHoverText(ItemStack stack, TooltipContext context, List statTotals = com.howlite.cobblemoncards.util.CardStatUtil.collectStats(stack); diff --git a/common/src/main/java/com/howlite/cobblemoncards/util/CardStatUtil.java b/common/src/main/java/com/howlite/cobblemoncards/util/CardStatUtil.java index d0c93af0..729ac1be 100644 --- a/common/src/main/java/com/howlite/cobblemoncards/util/CardStatUtil.java +++ b/common/src/main/java/com/howlite/cobblemoncards/util/CardStatUtil.java @@ -31,7 +31,7 @@ public class CardStatUtil { * vanilla {@link DataComponents#CONTAINER} component for saves that predate the migration. */ public static Iterable getBinderContents(ItemStack binderStack) { - if (binderStack == null || binderStack.isEmpty()) { + if (binderStack == null) { return Collections.emptyList(); } List binderItems = binderStack.get(ModDataComponents.BINDER_CONTENTS); diff --git a/fabric/src/main/java/com/howlite/cobblemoncards/fabric/item/FabricMasterAlbumItem.java b/fabric/src/main/java/com/howlite/cobblemoncards/fabric/item/FabricMasterAlbumItem.java index 9db46f08..10a04e29 100644 --- a/fabric/src/main/java/com/howlite/cobblemoncards/fabric/item/FabricMasterAlbumItem.java +++ b/fabric/src/main/java/com/howlite/cobblemoncards/fabric/item/FabricMasterAlbumItem.java @@ -1,5 +1,6 @@ package com.howlite.cobblemoncards.fabric.item; +import com.howlite.cobblemoncards.CobblemonCardsConfig; import com.howlite.cobblemoncards.component.CardStat; import com.howlite.cobblemoncards.item.custom.MasterAlbumItem; import io.wispforest.accessories.api.Accessory; @@ -22,6 +23,9 @@ public FabricMasterAlbumItem(Properties properties) { @Override public void getDynamicModifiers(ItemStack stack, SlotReference reference, AccessoryAttributeBuilder builder) { + // If configured, master tier binders do not provide stats + if (!CobblemonCardsConfig.doesMasterBinderProvideStats) return; + // Shared reader: BINDER_CONTENTS with a fallback to vanilla CONTAINER for unmigrated saves. Map statTotals = com.howlite.cobblemoncards.util.CardStatUtil.collectStats(stack); diff --git a/neoforge/src/main/java/com/howlite/cobblemoncards/neoforge/item/NeoForgeMasterAlbumItem.java b/neoforge/src/main/java/com/howlite/cobblemoncards/neoforge/item/NeoForgeMasterAlbumItem.java index ebeb000c..91d7a5dd 100644 --- a/neoforge/src/main/java/com/howlite/cobblemoncards/neoforge/item/NeoForgeMasterAlbumItem.java +++ b/neoforge/src/main/java/com/howlite/cobblemoncards/neoforge/item/NeoForgeMasterAlbumItem.java @@ -1,5 +1,6 @@ package com.howlite.cobblemoncards.neoforge.item; +import com.howlite.cobblemoncards.CobblemonCardsConfig; import com.howlite.cobblemoncards.item.custom.MasterAlbumItem; import com.howlite.cobblemoncards.component.CardStat; import io.wispforest.accessories.api.Accessory; @@ -22,6 +23,9 @@ public NeoForgeMasterAlbumItem(Properties properties) { @Override public void getDynamicModifiers(ItemStack stack, SlotReference reference, AccessoryAttributeBuilder builder) { + // If configured, master tier binders do not provide stats + if (!CobblemonCardsConfig.doesMasterBinderProvideStats) return; + // Shared reader: BINDER_CONTENTS with a fallback to vanilla CONTAINER for unmigrated saves. Map statTotals = com.howlite.cobblemoncards.util.CardStatUtil.collectStats(stack);