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

Author: P3pp3rF1y

gradle.properties

@@ -15,7 +15,7 @@ loader_version_range=[4,)
 mod_id=sophisticatedcore
 mod_name=Sophisticated Core
 mod_license=All Rights Reserved
-mod_version=1.4.75
+mod_version=1.4.76
 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

@@ -169,8 +169,8 @@ private static void onDrawScreen(ScreenEvent.Render.Post event) {
 		}
 		ItemStack held = containerGui.getMenu().getCarried();
 		Slot under = containerGui.getSlotUnderMouse();
-		if (!held.isEmpty() && under != null && under.mayPickup(mc.player) && !under.getItem().isEmpty()) {
-			getStashResultAndTooltip(under.getItem(), held)
+		if (!held.isEmpty() && under != null && !under.getItem().isEmpty()) {
+			getStashResultAndTooltip(under.getItem(), held).filter(stashResultAndTooltip -> under.mayPickup(mc.player))
 					.ifPresent(stashResultAndTooltip -> renderSpecialTooltip(event, mc, event.getGuiGraphics(), stashResultAndTooltip));
 		}
 	}
@@ -190,10 +190,10 @@ 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;
 			}
-			getStashResultAndTooltip(stack, held)
+			getStashResultAndTooltip(stack, held).filter(stashResultAndTooltip -> s.mayPickup(mc.player))
 					.ifPresent(stashResultAndTooltip -> renderStashSign(mc, event.getGuiGraphics(), s, stack, stashResultAndTooltip.stashResult()));
 		}
 	}

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

@@ -37,6 +37,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

@@ -51,6 +51,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,37 @@
+package net.p3pp3rf1y.sophisticatedcore.upgrades.battery;
+
+import net.minecraft.core.component.DataComponentMap;
+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() {
+		IStorageWrapper storageWrapper = mock(IStorageWrapper.class);
+		BatteryUpgradeItem batteryUpgrade = mock(BatteryUpgradeItem.class, CALLS_REAL_METHODS);
+		when(batteryUpgrade.asItem()).thenReturn(batteryUpgrade);
+		when(batteryUpgrade.components()).thenReturn(DataComponentMap.EMPTY);
+		doReturn(40_000).when(batteryUpgrade).getMaxEnergyStored(any());
+		ItemStack upgradeStack = new ItemStack(batteryUpgrade);
+		upgradeStack.set(ModCoreDataComponents.ENERGY_STORED, 40_001);
+
+		UpgradeSlotChangeResult result = batteryUpgrade.canSwapUpgradeFor(upgradeStack, 0, storageWrapper, 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,40 @@
+package net.p3pp3rf1y.sophisticatedcore.upgrades.tank;
+
+import net.minecraft.core.component.DataComponentMap;
+import net.minecraft.network.chat.contents.TranslatableContents;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.level.material.Fluids;
+import net.neoforged.neoforge.fluids.FluidStack;
+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.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 {
+	@Test
+	void canSwapUpgradeForRejectsSameTankWithContentsAboveTargetCapacity() {
+		IStorageWrapper storageWrapper = mock(IStorageWrapper.class);
+		TankUpgradeItem tankUpgrade = mock(TankUpgradeItem.class, CALLS_REAL_METHODS);
+		when(tankUpgrade.asItem()).thenReturn(tankUpgrade);
+		when(tankUpgrade.components()).thenReturn(DataComponentMap.EMPTY);
+		doReturn(40_000).when(tankUpgrade).getTankCapacity(any());
+		ItemStack upgradeStack = new ItemStack(tankUpgrade);
+		upgradeStack.set(ModCoreDataComponents.FLUID_CONTENTS, SimpleFluidContent.copyOf(new FluidStack(Fluids.WATER, 40_001)));
+
+		UpgradeSlotChangeResult result = tankUpgrade.canSwapUpgradeFor(upgradeStack, 0, storageWrapper, false);
+
+		assertFalse(result.successful());
+		TranslatableContents errorContents = assertInstanceOf(TranslatableContents.class, result.errorMessage().getContents());
+		assertArrayEquals(new Object[]{"1.1"}, errorContents.getArgs());
+	}
+}