major NBT cleanup

- no more default nbt tags on items (even in creative/jei) - don't write tags with default values on them (e.g. false booleans)
desht · Jan 17, 2024 · 5c62c59 · 5c62c59
1 parent 29fca81
commit 5c62c59
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 74 deletions.
diff --git a/src/main/java/me/desht/modularrouters/block/tile/ModularRouterBlockEntity.java b/src/main/java/me/desht/modularrouters/block/tile/ModularRouterBlockEntity.java
@@ -282,17 +282,17 @@ public void saveAdditional(CompoundTag nbt) {
         if (redstoneBehaviour != RouterRedstoneBehaviour.ALWAYS) nbt.putString(NBT_REDSTONE_MODE, redstoneBehaviour.name());
         if (energyStorage.getCapacity() > 0) nbt.put(NBT_ENERGY, energyStorage.serializeNBT());
         if (energyDirection != EnergyDirection.FROM_ROUTER) nbt.putString(NBT_ENERGY_DIR, energyDirection.name());
-        nbt.putBoolean(NBT_ACTIVE, active);
-        nbt.putInt(NBT_ACTIVE_TIMER, activeTimer);
-        nbt.putBoolean(NBT_ECO_MODE, ecoMode);
+        if (active) nbt.putBoolean(NBT_ACTIVE, true);
+        if (activeTimer != 0) nbt.putInt(NBT_ACTIVE_TIMER, activeTimer);
+        if (ecoMode) nbt.putBoolean(NBT_ECO_MODE, true);
 
         CompoundTag ext = new CompoundTag();
         CompoundTag ext1 = getExtensionData();
         for (String key : ext1.getAllKeys()) {
             //noinspection ConstantConditions
             ext.put(key, ext1.get(key));
         }
-        nbt.put(NBT_EXTRA, ext);
+        if (!ext.isEmpty()) nbt.put(NBT_EXTRA, ext);
     }
 
     private boolean hasItems(IItemHandler handler) {
@@ -1074,9 +1074,9 @@ public int getTransferRate() {
         @Override
         public Tag serializeNBT() {
             CompoundTag tag = new CompoundTag();
-            tag.putInt("Energy", energy);
-            tag.putInt("Capacity", capacity);
-            tag.putInt("Excess", excess);
+            if (energy > 0) tag.putInt("Energy", energy);
+            if (capacity > 0) tag.putInt("Capacity", capacity);
+            if (excess > 0) tag.putInt("Excess", excess);
             return tag;
         }
 

diff --git a/src/main/java/me/desht/modularrouters/container/handler/AugmentHandler.java b/src/main/java/me/desht/modularrouters/container/handler/AugmentHandler.java
@@ -22,7 +22,7 @@ public AugmentHandler(ItemStack holderStack, ModularRouterBlockEntity router) {
         Validate.isTrue(holderStack.getItem() instanceof ModuleItem, "holder stack must be a module!");
 
         this.holderStack = holderStack;
-        deserializeNBT(holderStack.getOrCreateTagElement(ModularRouters.MODID).getCompound(ModuleHelper.NBT_AUGMENTS));
+        deserializeNBT(ModuleHelper.validateNBT(holderStack).getCompound(ModuleHelper.NBT_AUGMENTS));
     }
 
     public ItemStack getHolderStack() {
@@ -57,7 +57,7 @@ protected void onContentsChanged(int slot) {
     }
 
     private void save() {
-        holderStack.getOrCreateTagElement(ModularRouters.MODID).put(ModuleHelper.NBT_AUGMENTS, serializeNBT());
+        ModuleHelper.validateNBTForWriting(holderStack).put(ModuleHelper.NBT_AUGMENTS, serializeNBT());
         if (router != null) {
             router.recompileNeeded(ModularRouterBlockEntity.COMPILE_MODULES);
         }

diff --git a/src/main/java/me/desht/modularrouters/container/handler/BaseModuleHandler.java b/src/main/java/me/desht/modularrouters/container/handler/BaseModuleHandler.java
@@ -22,7 +22,7 @@ public BaseModuleHandler(ItemStack holderStack, ModularRouterBlockEntity router,
         this.router = router;
         this.tagName = tagName;
 
-        deserializeNBT(holderStack.getOrCreateTagElement(ModularRouters.MODID).getCompound(tagName));
+        deserializeNBT(ModuleHelper.validateNBT(holderStack).getCompound(tagName));
     }
 
     /**
@@ -70,7 +70,7 @@ public static int getFilterSize(ItemStack holderStack, String tagName) {
      * Save the contents of the item handler onto the holder item stack's NBT
      */
     public void save() {
-        holderStack.getOrCreateTagElement(ModularRouters.MODID).put(tagName, serializeNBT());
+        ModuleHelper.validateNBTForWriting(holderStack).put(tagName, serializeNBT());
     }
 
     public static class BulkFilterHandler extends BaseModuleHandler {

diff --git a/src/main/java/me/desht/modularrouters/item/module/IPickaxeUser.java b/src/main/java/me/desht/modularrouters/item/module/IPickaxeUser.java
@@ -1,6 +1,6 @@
 package me.desht.modularrouters.item.module;
 
-import me.desht.modularrouters.ModularRouters;
+import me.desht.modularrouters.util.ModuleHelper;
 import net.minecraft.nbt.CompoundTag;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.Items;
@@ -9,7 +9,7 @@ public interface IPickaxeUser {
     String NBT_PICKAXE = "Pickaxe";
 
     default ItemStack getPickaxe(ItemStack moduleStack) {
-        CompoundTag tag = moduleStack.getOrCreateTagElement(ModularRouters.MODID);
+        CompoundTag tag = ModuleHelper.validateNBT(moduleStack);
         if (tag.contains(NBT_PICKAXE)) {
             return ItemStack.of(tag.getCompound(NBT_PICKAXE));
         } else {
@@ -18,8 +18,7 @@ default ItemStack getPickaxe(ItemStack moduleStack) {
     }
 
     default ItemStack setPickaxe(ItemStack moduleStack, ItemStack pickaxeStack) {
-        CompoundTag tag = moduleStack.getOrCreateTagElement(ModularRouters.MODID);
-        tag.put(NBT_PICKAXE, pickaxeStack.serializeNBT());
+        ModuleHelper.validateNBTForWriting(pickaxeStack).put(NBT_PICKAXE, pickaxeStack.serializeNBT());
         return moduleStack;
     }
 }
diff --git a/src/main/java/me/desht/modularrouters/item/module/ModuleItem.java b/src/main/java/me/desht/modularrouters/item/module/ModuleItem.java
@@ -327,8 +327,7 @@ private void addFilterInformation(ItemStack itemstack, List<Component> list) {
     @Nonnull
     public InteractionResultHolder<ItemStack> use(Level world, Player player, InteractionHand hand) {
         ItemStack stack = player.getItemInHand(hand);
-        ModuleHelper.validateNBT(stack);
-        if (!player.isSteppingCarefully()) {
+        if (!player.isCrouching()) {
             if (!world.isClientSide) {
                 MFLocator locator = MFLocator.heldModule(hand);
                 NetworkHooks.openScreen((ServerPlayer) player, new ModuleMenuProvider(player, locator), locator::writeBuf);

diff --git a/src/main/java/me/desht/modularrouters/item/module/TargetedModule.java b/src/main/java/me/desht/modularrouters/item/module/TargetedModule.java
@@ -13,6 +13,7 @@
 import me.desht.modularrouters.util.MiscUtil;
 import me.desht.modularrouters.util.ModuleHelper;
 import net.minecraft.ChatFormatting;
+import net.minecraft.Util;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Direction;
 import net.minecraft.core.GlobalPos;
@@ -175,14 +176,13 @@ private static void setTarget(ItemStack stack, Level world, BlockPos pos, Direct
             ModularRouters.LOGGER.warn("TargetModule.setTarget() should not be called client-side!");
             return;
         }
-        CompoundTag compound = ModuleHelper.validateNBT(stack);
+        CompoundTag compound = ModuleHelper.validateNBTForWriting(stack);
         if (pos == null) {
             compound.remove(NBT_TARGET);
         } else {
             ModuleTarget mt = new ModuleTarget(MiscUtil.makeGlobalPos(world, pos), face, BlockUtil.getBlockName(world, pos));
             compound.put(NBT_TARGET, mt.toNBT());
         }
-        stack.getOrCreateTag().put(ModularRouters.MODID, compound);
     }
 
     /**
@@ -221,7 +221,7 @@ public static ModuleTarget getTarget(ItemStack stack, boolean checkBlockName) {
      *
      * @param stack the module item stack
      * @param checkBlockName verify the name of the target block - only works server-side
-     * @return a list of targets for the module
+     * @return a set of targets for the module
      */
     public static Set<ModuleTarget> getTargets(ItemStack stack, boolean checkBlockName) {
         Set<ModuleTarget> result = Sets.newHashSet();
@@ -243,13 +243,8 @@ public static Set<ModuleTarget> getTargets(ItemStack stack, boolean checkBlockNa
     }
 
     private static void setTargets(ItemStack stack, Set<ModuleTarget> targets) {
-        CompoundTag compound = ModuleHelper.validateNBT(stack);
-        ListTag list = new ListTag();
-        for (ModuleTarget target : targets) {
-            list.add(target.toNBT());
-        }
-        compound.put(NBT_MULTI_TARGET, list);
-        stack.getOrCreateTag().put(ModularRouters.MODID, compound);
+        CompoundTag compound = ModuleHelper.validateNBTForWriting(stack);
+        compound.put(NBT_MULTI_TARGET, Util.make(new ListTag(), l -> targets.forEach(t -> l.add(t.toNBT()))));
     }
 
     private static ModuleTarget updateTargetBlockName(ItemStack stack, ModuleTarget target) {

diff --git a/src/main/java/me/desht/modularrouters/logic/compiled/CompiledDetectorModule.java b/src/main/java/me/desht/modularrouters/logic/compiled/CompiledDetectorModule.java
@@ -18,8 +18,8 @@ public class CompiledDetectorModule extends CompiledModule {
     public CompiledDetectorModule(ModularRouterBlockEntity router, ItemStack stack) {
         super(router, stack);
 
-        CompoundTag compound = setupNBT(stack);
-        signalLevel = compound.getByte(NBT_SIGNAL_LEVEL);
+        CompoundTag compound = ModuleHelper.validateNBT(stack);
+        signalLevel = compound.contains(NBT_SIGNAL_LEVEL) ? compound.getByte(NBT_SIGNAL_LEVEL) : 15;
         strongSignal = compound.getBoolean(NBT_STRONG_SIGNAL);
     }
 
@@ -41,17 +41,6 @@ public boolean execute(@Nonnull ModularRouterBlockEntity router) {
         return true;
     }
 
-    private CompoundTag setupNBT(ItemStack stack) {
-        CompoundTag compound = ModuleHelper.validateNBT(stack);
-        if (!compound.contains(NBT_SIGNAL_LEVEL)) {
-            compound.putInt(NBT_SIGNAL_LEVEL, 15);
-        }
-        if (!compound.contains(NBT_STRONG_SIGNAL)) {
-            compound.putBoolean(NBT_STRONG_SIGNAL, false);
-        }
-        return compound;
-    }
-
     public int getSignalLevel() {
         return signalLevel;
     }

diff --git a/src/main/java/me/desht/modularrouters/logic/compiled/CompiledFlingerModule.java b/src/main/java/me/desht/modularrouters/logic/compiled/CompiledFlingerModule.java
@@ -17,7 +17,6 @@
 import net.minecraft.world.item.ItemStack;
 
 import javax.annotation.Nonnull;
-import java.util.stream.Stream;
 
 public class CompiledFlingerModule extends CompiledDropperModule {
     public static final String NBT_SPEED = "Speed";
@@ -30,10 +29,6 @@ public CompiledFlingerModule(ModularRouterBlockEntity router, ItemStack stack) {
         super(router, stack);
 
         CompoundTag compound = ModuleHelper.validateNBT(stack);
-        Stream.of(NBT_SPEED, NBT_PITCH, NBT_YAW)
-                .filter(key -> !compound.contains(key))
-                .forEach(key -> compound.putFloat(key, 0.0f));
-
         speed = compound.getFloat(NBT_SPEED);
         pitch = compound.getFloat(NBT_PITCH);
         yaw = compound.getFloat(NBT_YAW);

diff --git a/src/main/java/me/desht/modularrouters/logic/compiled/CompiledFluidModule1.java b/src/main/java/me/desht/modularrouters/logic/compiled/CompiledFluidModule1.java
@@ -45,7 +45,7 @@ public class CompiledFluidModule1 extends CompiledModule {
     public CompiledFluidModule1(ModularRouterBlockEntity router, ItemStack stack) {
         super(router, stack);
 
-        CompoundTag compound = setupNBT(stack);
+        CompoundTag compound = ModuleHelper.validateNBT(stack);
         maxTransfer = compound.getInt(NBT_MAX_TRANSFER);
         fluidDirection = FluidDirection.values()[compound.getByte(NBT_FLUID_DIRECTION)];
         forceEmpty = compound.getBoolean(NBT_FORCE_EMPTY);
@@ -211,7 +211,7 @@ private boolean doTransfer(ModularRouterBlockEntity router, IFluidHandler src, I
                 return false;
             }
         }
-        int amount = Math.min(maxTransfer, router.getCurrentFluidTransferAllowance(direction));
+        int amount = Math.min(getMaxTransfer(), router.getCurrentFluidTransferAllowance(direction));
         FluidStack newStack = FluidUtil.tryFluidTransfer(dest, src, amount, false);
         if (!newStack.isEmpty() && getFilter().testFluid(newStack.getFluid())) {
             newStack = FluidUtil.tryFluidTransfer(dest, src, newStack.getAmount(), true);
@@ -240,23 +240,12 @@ private int checkFluidInTank(IFluidHandler handler) {
         }
     }
 
-    private CompoundTag setupNBT(ItemStack stack) {
-        CompoundTag compound = ModuleHelper.validateNBT(stack);
-        if (!compound.contains(NBT_MAX_TRANSFER)) {
-            compound.putInt(NBT_MAX_TRANSFER, BUCKET_VOLUME);
-        }
-        if (!compound.contains(NBT_FLUID_DIRECTION)) {
-            compound.putByte(NBT_FLUID_DIRECTION, (byte) FluidDirection.IN.ordinal());
-        }
-        return compound;
-    }
-
     public FluidDirection getFluidDirection() {
         return fluidDirection;
     }
 
     public int getMaxTransfer() {
-        return maxTransfer;
+        return maxTransfer == 0 ? BUCKET_VOLUME : maxTransfer;
     }
 
     public boolean isForceEmpty() {

diff --git a/src/main/java/me/desht/modularrouters/network/ModuleSettingsMessage.java b/src/main/java/me/desht/modularrouters/network/ModuleSettingsMessage.java
@@ -46,7 +46,7 @@ public void handle(Supplier<NetworkEvent.Context> ctx) {
                 ItemStack moduleStack = locator.getModuleStack(player);
 
                 if (moduleStack.getItem() instanceof ModuleItem) {
-                    CompoundTag compound = ModuleHelper.validateNBT(moduleStack);
+                    CompoundTag compound = ModuleHelper.validateNBTForWriting(moduleStack);
                     for (String key : payload.getAllKeys()) {
                         compound.put(key, Objects.requireNonNull(payload.get(key)));
                     }

diff --git a/src/main/java/me/desht/modularrouters/recipe/ResetModuleRecipe.java b/src/main/java/me/desht/modularrouters/recipe/ResetModuleRecipe.java
@@ -46,10 +46,9 @@ public ItemStack assemble(CraftingContainer inv, RegistryAccess registryAccess)
 
         if (!moduleStack.isEmpty()) {
             ItemStack newStack = new ItemStack(moduleStack.getItem());
-            ModuleHelper.validateNBT(newStack);
-            if (moduleStack.getItem() instanceof IPickaxeUser) {
-                ItemStack pick = ((IPickaxeUser) moduleStack.getItem()).getPickaxe(moduleStack);
-                if (!pick.isEmpty()) ((IPickaxeUser) moduleStack.getItem()).setPickaxe(newStack, pick);
+            if (moduleStack.getItem() instanceof IPickaxeUser pickaxeUser) {
+                ItemStack pick = pickaxeUser.getPickaxe(moduleStack);
+                if (!pick.isEmpty()) pickaxeUser.setPickaxe(newStack, pick);
             }
             return newStack;
         } else {

diff --git a/src/main/java/me/desht/modularrouters/util/ModuleHelper.java b/src/main/java/me/desht/modularrouters/util/ModuleHelper.java
@@ -29,11 +29,12 @@ public class ModuleHelper {
 
     @Nonnull
     public static CompoundTag validateNBT(ItemStack stack) {
-        CompoundTag compound = stack.getOrCreateTagElement(ModularRouters.MODID);
-        if (compound.getTagType(NBT_FILTER) != Tag.TAG_COMPOUND) {
-            compound.put(NBT_FILTER, new CompoundTag());
-        }
-        return compound;
+        return stack.hasTag() ? stack.getTag().getCompound(ModularRouters.MODID) : new CompoundTag();
+    }
+
+    @Nonnull
+    public static CompoundTag validateNBTForWriting(ItemStack stack) {
+        return stack.getOrCreateTagElement(ModularRouters.MODID);
     }
 
     public static boolean isBlacklist(ItemStack stack) {
@@ -64,18 +65,17 @@ public static Termination getTermination(ItemStack stack) {
                     Termination.valueOf(compound.getString(ModuleHelper.NBT_TERMINATION)) :
                     Termination.NONE;
         } catch (IllegalArgumentException e) {
-            compound.putString(ModuleHelper.NBT_TERMINATION, Termination.NONE.toString());
             return Termination.NONE;
         }
     }
 
     public static RelativeDirection getRelativeDirection(ItemStack stack) {
-        if (stack.getItem() instanceof ModuleItem && ((ModuleItem) stack.getItem()).isDirectional()) {
+        if (stack.getItem() instanceof ModuleItem m && m.isDirectional()) {
             CompoundTag compound = validateNBT(stack);
             try {
-                return RelativeDirection.valueOf(compound.getString(NBT_DIRECTION));
+                String dirStr = compound.getString(NBT_DIRECTION);
+                return dirStr.isEmpty() ? RelativeDirection.NONE : RelativeDirection.valueOf(dirStr);
             } catch (IllegalArgumentException e) {
-                compound.putString(NBT_DIRECTION, RelativeDirection.NONE.toString());
                 return RelativeDirection.NONE;
             }
         } else {
@@ -111,7 +111,7 @@ public static boolean isMatchAll(ItemStack stack) {
     }
 
     public static void setRoundRobinCounter(ItemStack moduleStack, int counter) {
-        CompoundTag tag = validateNBT(moduleStack);
+        CompoundTag tag = validateNBTForWriting(moduleStack);
         tag.putInt(NBT_RR_COUNTER, counter);
     }