fix: 🐛 Fixed upgrade swaps allowing oversized tank and battery upgrades

Author: P3pp3rF1y

gradle.properties

@@ -13,7 +13,7 @@ loader_version_range=[4,)
 mod_id=sophisticatedcore
 mod_name=Sophisticated Core
 mod_license=All Rights Reserved
-mod_version=1.4.94
+mod_version=1.4.95
 mod_group_id=net.p3pp3rf1y
 mod_authors=P3pp3rF1y
 mod_description=A library / shared functionality mod for Sophisticated Storage and Backpacks

src/main/java/net/p3pp3rf1y/sophisticatedcore/client/ClientEventHandler.java

@@ -203,10 +203,11 @@ private static void onContainerScreenForeground(ContainerScreenEvent.Render.Fore
 		Slot under = containerGui.getSlotUnderMouse();
 		for (Slot s : menu.slots) {
 			ItemStack stack = s.getItem();
-			if (s == under || !s.mayPickup(mc.player) || stack.isEmpty()) {
+			if (s == under || stack.isEmpty()) {
 				continue;
 			}
-			getStashResult(stack, held).ifPresent(stashResult -> renderStashSign(mc, event.getGuiGraphics(), s, stack, stashResult));
+			getStashResult(stack, held).filter(stashResult -> s.mayPickup(mc.player))
+					.ifPresent(stashResult -> renderStashSign(mc, event.getGuiGraphics(), s, stack, stashResult));
 		}
 	}
 

src/main/java/net/p3pp3rf1y/sophisticatedcore/upgrades/battery/BatteryUpgradeItem.java

@@ -36,6 +36,15 @@ public UpgradeType<BatteryUpgradeWrapper> getType() {
 		return TYPE;
 	}
 
+	@Override
+	public UpgradeSlotChangeResult canSwapUpgradeFor(ItemStack upgradeStackToPut, int upgradeSlot, IStorageWrapper storageWrapper, boolean isClientSide) {
+		if (upgradeStackToPut.getItem() == this) {
+			return checkExtraInsertConditions(upgradeStackToPut, storageWrapper, isClientSide, upgradeSlot, this);
+		}
+
+		return super.canSwapUpgradeFor(upgradeStackToPut, upgradeSlot, storageWrapper, isClientSide);
+	}
+
 	@Override
 	public int getInventoryColumnsTaken() {
 		return 2;

src/main/java/net/p3pp3rf1y/sophisticatedcore/upgrades/tank/TankUpgradeItem.java

@@ -50,6 +50,15 @@ public UpgradeType<TankUpgradeWrapper> getType() {
 		return TYPE;
 	}
 
+	@Override
+	public UpgradeSlotChangeResult canSwapUpgradeFor(ItemStack upgradeStackToPut, int upgradeSlot, IStorageWrapper storageWrapper, boolean isClientSide) {
+		if (upgradeStackToPut.getItem() == this) {
+			return checkExtraInsertConditions(upgradeStackToPut, storageWrapper, isClientSide, upgradeSlot, this);
+		}
+
+		return super.canSwapUpgradeFor(upgradeStackToPut, upgradeSlot, storageWrapper, isClientSide);
+	}
+
 	@Override
 	public UpgradeSlotChangeResult checkExtraInsertConditions(ItemStack upgradeStack, IStorageWrapper storageWrapper, boolean isClientSide,
 			@Nullable IUpgradeItem<?> upgradeInSlot) {

src/test/java/net/p3pp3rf1y/sophisticatedcore/upgrades/battery/BatteryUpgradeItemTest.java

@@ -0,0 +1,34 @@
+package net.p3pp3rf1y.sophisticatedcore.upgrades.battery;
+
+import net.minecraft.network.chat.contents.TranslatableContents;
+import net.minecraft.world.item.ItemStack;
+import net.p3pp3rf1y.sophisticatedcore.api.IStorageWrapper;
+import net.p3pp3rf1y.sophisticatedcore.common.gui.UpgradeSlotChangeResult;
+import net.p3pp3rf1y.sophisticatedcore.init.ModCoreDataComponents;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class BatteryUpgradeItemTest {
+	@Test
+	void canSwapUpgradeForRejectsSameBatteryWithEnergyAboveTargetCapacity() {
+		BatteryUpgradeItem batteryUpgrade = mock(BatteryUpgradeItem.class, CALLS_REAL_METHODS);
+		ItemStack upgradeStack = mock(ItemStack.class);
+		when(upgradeStack.getItem()).thenReturn(batteryUpgrade);
+		when(upgradeStack.getOrDefault(ModCoreDataComponents.ENERGY_STORED, 0)).thenReturn(40_001);
+		doReturn(40_000).when(batteryUpgrade).getMaxEnergyStored(any());
+
+		UpgradeSlotChangeResult result = batteryUpgrade.canSwapUpgradeFor(upgradeStack, 0, mock(IStorageWrapper.class), false);
+
+		assertFalse(result.successful());
+		TranslatableContents errorContents = assertInstanceOf(TranslatableContents.class, result.errorMessage().getContents());
+		assertArrayEquals(new Object[]{"1.1"}, errorContents.getArgs());
+	}
+}

src/test/java/net/p3pp3rf1y/sophisticatedcore/upgrades/tank/TankUpgradeItemTest.java

@@ -0,0 +1,59 @@
+package net.p3pp3rf1y.sophisticatedcore.upgrades.tank;
+
+import net.minecraft.SharedConstants;
+import net.minecraft.core.component.DataComponentMap;
+import net.minecraft.core.component.DataComponents;
+import net.minecraft.network.chat.contents.TranslatableContents;
+import net.minecraft.server.Bootstrap;
+import net.minecraft.world.item.Item;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.neoforged.neoforge.fluids.SimpleFluidContent;
+import net.p3pp3rf1y.sophisticatedcore.api.IStorageWrapper;
+import net.p3pp3rf1y.sophisticatedcore.common.gui.UpgradeSlotChangeResult;
+import net.p3pp3rf1y.sophisticatedcore.init.ModCoreDataComponents;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class TankUpgradeItemTest {
+	@BeforeAll
+	static void setup() {
+		SharedConstants.tryDetectVersion();
+		Bootstrap.bootStrap();
+		Bootstrap.validate();
+		bindTestComponents(Items.EXPERIENCE_BOTTLE, Items.POTION, Items.GLASS_BOTTLE);
+	}
+
+	private static void bindTestComponents(Item... items) {
+		DataComponentMap components = DataComponentMap.builder().set(DataComponents.MAX_STACK_SIZE, 64).build();
+		for (Item item : items) {
+			item.builtInRegistryHolder().bindComponents(components);
+		}
+	}
+
+	@Test
+	void canSwapUpgradeForRejectsSameTankWithContentsAboveTargetCapacity() {
+		TankUpgradeItem tankUpgrade = mock(TankUpgradeItem.class, CALLS_REAL_METHODS);
+		ItemStack upgradeStack = mock(ItemStack.class);
+		SimpleFluidContent contents = mock(SimpleFluidContent.class);
+		when(upgradeStack.getItem()).thenReturn(tankUpgrade);
+		when(contents.getAmount()).thenReturn(40_001);
+		when(upgradeStack.getOrDefault(ModCoreDataComponents.FLUID_CONTENTS, SimpleFluidContent.EMPTY)).thenReturn(contents);
+		doReturn(40_000).when(tankUpgrade).getTankCapacity(any());
+
+		UpgradeSlotChangeResult result = tankUpgrade.canSwapUpgradeFor(upgradeStack, 0, mock(IStorageWrapper.class), false);
+
+		assertFalse(result.successful());
+		TranslatableContents errorContents = assertInstanceOf(TranslatableContents.class, result.errorMessage().getContents());
+		assertArrayEquals(new Object[]{"1.1"}, errorContents.getArgs());
+	}
+}