Fix negative energy value crash (likely long overflow) (#108)

Fix negative energy value crash when using MI cables (likely long overflow)
Technici4n · Mar 19, 2024 · b68a119 · b68a119
1 parent 738cbe6
commit b68a119
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/src/main/java/dev/technici4n/moderndynamics/network/energy/EnergyCache.java b/src/main/java/dev/technici4n/moderndynamics/network/energy/EnergyCache.java
@@ -113,7 +113,6 @@ public void doTick() {
      * Dispatch a transfer operation among a list of targets. Will not modify the list.
      */
     public static int transferForTargets(TransferOperation operation, List<IEnergyStorage> targets, int maxAmount) {
-        int intMaxAmount = Ints.saturatedCast(maxAmount);
         // Build target list
         List<EnergyTarget> sortableTargets = new ArrayList<>(targets.size());
         for (var target : targets) {
@@ -123,7 +122,7 @@ public static int transferForTargets(TransferOperation operation, List<IEnergySt
         Collections.shuffle(sortableTargets);
         // Simulate the transfer for every target
         for (EnergyTarget target : sortableTargets) {
-            target.simulationResult = operation.transfer(target.target, intMaxAmount, true);
+            target.simulationResult = operation.transfer(target.target, maxAmount, true);
         }
         // Sort from low to high result
         sortableTargets.sort(Comparator.comparingLong(t -> t.simulationResult));

diff --git a/src/main/java/dev/technici4n/moderndynamics/network/mienergy/MIEnergyCache.java b/src/main/java/dev/technici4n/moderndynamics/network/mienergy/MIEnergyCache.java
@@ -18,6 +18,7 @@
  */
 package dev.technici4n.moderndynamics.network.mienergy;
 
+import com.google.common.base.Preconditions;
 import dev.technici4n.moderndynamics.network.NetworkCache;
 import dev.technici4n.moderndynamics.network.NetworkNode;
 import dev.technici4n.moderndynamics.network.energy.EnergyCache;
@@ -27,8 +28,8 @@
 import net.neoforged.neoforge.energy.IEnergyStorage;
 
 public class MIEnergyCache extends NetworkCache<MIEnergyHost, MIEnergyCache> {
-    private int energy = 0;
-    private int maxEnergy = 0;
+    private long energy = 0;
+    private long maxEnergy = 0;
 
     protected MIEnergyCache(ServerLevel level, List<NetworkNode<MIEnergyHost, MIEnergyCache>> networkNodes) {
         super(level, networkNodes);
@@ -39,8 +40,8 @@ protected void doCombine() {
         energy = maxEnergy = 0;
 
         for (var node : nodes) {
-            energy += node.getHost().getEnergy();
-            maxEnergy += node.getHost().getMaxEnergy();
+            energy = saturatedSum(energy, node.getHost().getEnergy());
+            maxEnergy = saturatedSum(maxEnergy, node.getHost().getMaxEnergy());
         }
     }
 
@@ -74,9 +75,21 @@ protected void doTick() {
 
         var tier = nodes.get(0).getHost().tier;
 
+        // tier.getMax() is an int and energy is unsigned, so casting to (int) is safe
         // Extract
-        energy += EnergyCache.transferForTargets(IEnergyStorage::extractEnergy, storages, Math.min(maxEnergy - energy, tier.getMax()));
+        energy += EnergyCache.transferForTargets(IEnergyStorage::extractEnergy, storages, (int) Math.min(maxEnergy - energy, tier.getMax()));
         // Insert
-        energy -= EnergyCache.transferForTargets(IEnergyStorage::receiveEnergy, storages, Math.min(energy, tier.getMax()));
+        energy -= EnergyCache.transferForTargets(IEnergyStorage::receiveEnergy, storages, (int) Math.min(energy, tier.getMax()));
+    }
+
+    // Energy is unsigned. Hence we handle only one case of satured addition (same sign)
+    private static long saturatedSum(long a, long b) {
+        Preconditions.checkArgument(a >= 0, "a >= 0");
+        Preconditions.checkArgument(b >= 0, "b >= 0");
+        var sum = a + b;
+        if (sum < a || sum < b) {
+            return Long.MAX_VALUE;
+        }
+        return sum;
     }
 }