Improve remainder distribution to try and split the remainder as evenly as possible between the various destinations before falling back to sending to the first one it will fit in (#6617) (#8062)

pupnewfster · thiakil · web-flow · commit c53a41c10f0e · 2024-04-20T10:37:04.000-05:00
Co-authored-by: Thiakil &lt;thiakil@users.noreply.github.com&gt;
diff --git a/src/main/java/mekanism/common/lib/distribution/FloatingLongSplitInfo.java b/src/main/java/mekanism/common/lib/distribution/FloatingLongSplitInfo.java
@@ -20,10 +20,18 @@ public void send(FloatingLong amountNeeded) {
         //If we are giving it, then lower the amount we are checking/splitting
         boolean recalculate;
         if (amountNeeded.isZero()) {
+            if (!decrementTargets) {
+                //If we are not decrementing targets, then don't remove that as a valid target, or update how much there is per target
+                return;
+            }
             recalculate = true;
         } else {
             amountToSplit = amountToSplit.minusEqual(amountNeeded);
             sentSoFar = sentSoFar.plusEqual(amountNeeded);
+            if (!decrementTargets) {
+                //If we are not decrementing targets, then don't remove that as a valid target, or update how much there is per target
+                return;
+            }
             recalculate = !amountNeeded.equals(amountPerTarget);
         }
         toSplitAmong--;
@@ -50,6 +58,16 @@ public FloatingLong getRemainderAmount() {
         return amountPerTarget;
     }
 
+    @Override
+    public FloatingLong getUnsent() {
+        return amountToSplit;
+    }
+
+    @Override
+    public boolean isZero(FloatingLong value) {
+        return value.isZero();
+    }
+
     @Override
     public FloatingLong getTotalSent() {
         return sentSoFar;
diff --git a/src/main/java/mekanism/common/lib/distribution/IntegerSplitInfo.java b/src/main/java/mekanism/common/lib/distribution/IntegerSplitInfo.java
@@ -5,24 +5,36 @@ public class IntegerSplitInfo extends SplitInfo<Integer> {
     private int amountToSplit;
     private int amountPerTarget;
     private int sentSoFar;
+    private int remainder;
 
     public IntegerSplitInfo(int amountToSplit, int totalTargets) {
         super(totalTargets);
         this.amountToSplit = amountToSplit;
         amountPerTarget = toSplitAmong == 0 ? 0 : amountToSplit / toSplitAmong;
+        remainder = toSplitAmong == 0 ? 0 : amountToSplit % toSplitAmong;
     }
 
     @Override
     public void send(Integer amountNeeded) {
         //If we are giving it, then lower the amount we are checking/splitting
         amountToSplit -= amountNeeded;
         sentSoFar += amountNeeded;
+        if (!decrementTargets) {
+            //If we are not decrementing targets, then don't remove that as a valid target, or update how much there is per target
+            int difference = amountNeeded - amountPerTarget;
+            if (difference > 0) {
+                //If we removed more than we have per target, we need to remove the excess from our remainder
+                remainder -= difference;
+            }
+            return;
+        }
         toSplitAmong--;
         //Only recalculate it if it is not willing to accept/doesn't want the
         // full per side split
         if (amountNeeded != amountPerTarget && toSplitAmong != 0) {
             int amountPerLast = amountPerTarget;
             amountPerTarget = amountToSplit / toSplitAmong;
+            remainder = amountToSplit % toSplitAmong;
             if (!amountPerChanged && amountPerTarget != amountPerLast) {
                 amountPerChanged = true;
             }
@@ -31,15 +43,29 @@ public void send(Integer amountNeeded) {
 
     @Override
     public Integer getShareAmount() {
+        //TODO: Should we make this return a + 1 if there is a remainder, so that we can factor out those cases that can accept exactly amountPerTarget + 1
+        // while doing our initial loop rather than handling it via getRemainderAmount?
         return amountPerTarget;
     }
 
     @Override
     public Integer getRemainderAmount() {
-        //Add to the remainder amount the entire remainder so that we try to use it up if we can
-        // The remainder then if it cannot be fully accepted slowly shrinks across each target we are distributing to
-        //TODO: Evaluate making a more even distribution of the remainder
-        return toSplitAmong == 0 ? amountPerTarget : amountPerTarget + (amountToSplit % toSplitAmong);
+        if (toSplitAmong != 0 && remainder > 0) {
+            //If we have a remainder, be willing to provide a single unit as the remainder
+            // so that we split the remainder more evenly across the targets.
+            return amountPerTarget + 1;
+        }
+        return amountPerTarget;
+    }
+
+    @Override
+    public Integer getUnsent() {
+        return amountToSplit;
+    }
+
+    @Override
+    public boolean isZero(Integer value) {
+        return value == 0;
     }
 
     @Override
diff --git a/src/main/java/mekanism/common/lib/distribution/LongSplitInfo.java b/src/main/java/mekanism/common/lib/distribution/LongSplitInfo.java
@@ -5,24 +5,36 @@ public class LongSplitInfo extends SplitInfo<Long> {
     private long amountToSplit;
     private long amountPerTarget;
     private long sentSoFar;
+    private long remainder;
 
     public LongSplitInfo(long amountToSplit, int totalTargets) {
         super(totalTargets);
         this.amountToSplit = amountToSplit;
         amountPerTarget = toSplitAmong == 0 ? 0 : amountToSplit / toSplitAmong;
+        remainder = toSplitAmong == 0 ? 0 : amountToSplit % toSplitAmong;
     }
 
     @Override
     public void send(Long amountNeeded) {
         //If we are giving it, then lower the amount we are checking/splitting
         amountToSplit -= amountNeeded;
         sentSoFar += amountNeeded;
+        if (!decrementTargets) {
+            //If we are not decrementing targets, then don't remove that as a valid target, or update how much there is per target
+            long difference = amountNeeded - amountPerTarget;
+            if (difference > 0) {
+                //If we removed more than we have per target, we need to remove the excess from our remainder
+                remainder -= difference;
+            }
+            return;
+        }
         toSplitAmong--;
         //Only recalculate it if it is not willing to accept/doesn't want the
         // full per side split
         if (amountNeeded != amountPerTarget && toSplitAmong != 0) {
             long amountPerLast = amountPerTarget;
             amountPerTarget = amountToSplit / toSplitAmong;
+            remainder = amountToSplit % toSplitAmong;
             if (!amountPerChanged && amountPerTarget != amountPerLast) {
                 amountPerChanged = true;
             }
@@ -36,10 +48,22 @@ public Long getShareAmount() {
 
     @Override
     public Long getRemainderAmount() {
-        //Add to the remainder amount the entire remainder so that we try to use it up if we can
-        // The remainder then if it cannot be fully accepted slowly shrinks across each target we are distributing to
-        //TODO: Evaluate making a more even distribution of the remainder
-        return toSplitAmong == 0 ? amountPerTarget : amountPerTarget + (amountToSplit % toSplitAmong);
+        if (toSplitAmong != 0 && remainder > 0) {
+            //If we have a remainder, be willing to provide a single unit as the remainder
+            // so that we split the remainder more evenly across the targets.
+            return amountPerTarget + 1;
+        }
+        return amountPerTarget;
+    }
+
+    @Override
+    public Long getUnsent() {
+        return remainder;
+    }
+
+    @Override
+    public boolean isZero(Long value) {
+        return value == 0;
     }
 
     @Override
diff --git a/src/main/java/mekanism/common/lib/distribution/SplitInfo.java b/src/main/java/mekanism/common/lib/distribution/SplitInfo.java
@@ -2,18 +2,63 @@
 
 public abstract class SplitInfo<TYPE extends Number & Comparable<TYPE>> {
 
+    /**
+     * Number of targets to split the contents among.
+     */
     protected int toSplitAmong;
+    /**
+     * Represents whether the amount per target distribution has changed. This may happen if a target doesn't need as much as we are willing to offer it in the split.
+     */
     public boolean amountPerChanged = false;
+    /**
+     * Determines whether the number of targets to split amount should be decreased.
+     *
+     * @implNote This is only set to false briefly when handling accepting contents with remainders to allow them to accept some of the contents without being marked as
+     * fully accounted for.
+     */
+    protected boolean decrementTargets = true;
 
     protected SplitInfo(int totalTargets) {
         this.toSplitAmong = totalTargets;
     }
 
+    /**
+     * Marks the given amount as being accounted for and "sent". Decrements {@link #getUnsent() how much we have left to send} and increments
+     * {@link #getTotalSent() how much we have sent}. If {@link #decrementTargets} is true, this also will reduce the number of targets to split among, and recalculate
+     * how much we can provide each target.
+     *
+     * @param amountNeeded Amount needed by the target and that we are accounting as having been sent to the target.
+     */
     public abstract void send(TYPE amountNeeded);
 
+    /**
+     * {@return the "share" each target should get when distributing in an even split}
+     */
     public abstract TYPE getShareAmount();
 
+    /**
+     * Gets the "share" including a potential remainder that targets should get when handling remainders. This is used for actually sending providing the split share to
+     * any targets that can accept more than we are able to offer in an even split. In general this number will either be equal to {@link #getShareAmount()} or greater
+     * than it by one while we still have an excess remainder.
+     *
+     * @return the "share" plus any potential remainder.
+     */
     public abstract TYPE getRemainderAmount();
 
+    /**
+     * {@return the amount of contents that has not been sent anywhere yet}
+     */
+    public abstract TYPE getUnsent();
+
+    /**
+     * {@return true if the value is equal to zero}
+     *
+     * @param value Value to check
+     */
+    public abstract boolean isZero(TYPE value);
+
+    /**
+     * {@return the total amount of contents that have been sent}
+     */
     public abstract TYPE getTotalSent();
 }
diff --git a/src/main/java/mekanism/common/lib/distribution/Target.java b/src/main/java/mekanism/common/lib/distribution/Target.java
@@ -61,8 +61,43 @@ public int getHandlerCount() {
      */
     public void sendRemainingSplit(SplitInfo<TYPE> splitInfo) {
         //If needed is not empty then we default it to the given calculated fair split amount of remaining energy
-        for (HandlerType<HANDLER, TYPE> recipient : needed) {
-            acceptAmount(recipient.handler(), splitInfo, splitInfo.getRemainderAmount());
+        if (!needed.isEmpty() && !splitInfo.isZero(splitInfo.getRemainderAmount())) {
+            Iterator<HandlerType<HANDLER, TYPE>> iterator = needed.iterator();
+            while (iterator.hasNext()) {
+                TYPE remainderAmount = splitInfo.getRemainderAmount();
+                if (splitInfo.isZero(remainderAmount)) {
+                    //We finished inserting everything we wanted to, we can just exit
+                    return;
+                }
+                HandlerType<HANDLER, TYPE> needInfo = iterator.next();
+                //Accept the remaining amount
+                TYPE amountNeeded = needInfo.amount();
+                if (amountNeeded.compareTo(remainderAmount) <= 0) {
+                    acceptAmount(needInfo.handler(), splitInfo, amountNeeded);
+                    //If the amount we needed was the less than or the same as our remaining amount
+                    // we can remove the value as it has now been sent
+                    iterator.remove();
+                } else {
+                    splitInfo.decrementTargets = false;
+                    acceptAmount(needInfo.handler(), splitInfo, remainderAmount);
+                    splitInfo.decrementTargets = true;
+                }
+            }
+            //TODO: If we remove buffers maybe we should evaluate not caring if we don't actually send the full excess remainder?
+            // Given ideally we wouldn't attempting to insert the excess remainder to handlers as a second call to the handler on the same tick
+            if (!splitInfo.isZero(splitInfo.getUnsent())) {
+                //If we still have some of a remainder after trying to evenly distribute the remainder just send it to the first target willing to accept it
+                // This might happen if one of the destinations was only able to accept part of the remaining amount, though in general that case will be
+                // covered by shifting the needed values
+                for (HandlerType<HANDLER, TYPE> recipient : needed) {
+                    TYPE remaining = splitInfo.getUnsent();
+                    if (splitInfo.isZero(remaining)) {
+                        //We finished, exit
+                        return;
+                    }
+                    acceptAmount(recipient.handler(), splitInfo, remaining);
+                }
+            }
         }
     }
 
@@ -96,14 +131,27 @@ public void sendRemainingSplit(SplitInfo<TYPE> splitInfo) {
      * @param splitInfo Information about current overall split.
      */
     public void sendPossible(EXTRA toSend, SplitInfo<TYPE> splitInfo) {
-        for (HANDLER entry : handlers) {
-            TYPE amountNeeded = simulate(entry, toSend);
-            if (amountNeeded.compareTo(splitInfo.getShareAmount()) <= 0) {
-                //Add the amount, in case something changed from simulation only mark actual sent amount
-                // in split info
-                acceptAmount(entry, splitInfo, amountNeeded);
-            } else {
-                needed.add(new HandlerType<>(entry, amountNeeded));
+        if (splitInfo.isZero(splitInfo.getShareAmount())) {
+            //We are all remainder, just calculate how much each can accept
+            for (HANDLER entry : handlers) {
+                TYPE amountNeeded = simulate(entry, toSend);
+                if (!splitInfo.isZero(amountNeeded)) {
+                    needed.add(new HandlerType<>(entry, amountNeeded));
+                }
+            }
+        } else {
+            for (HANDLER entry : handlers) {
+                TYPE amountNeeded = simulate(entry, toSend);
+                if (amountNeeded.compareTo(splitInfo.getShareAmount()) <= 0) {
+                    //Add the amount, in case something changed from simulation only mark actual sent amount
+                    // in split info
+                    if (!splitInfo.isZero(amountNeeded)) {
+                        //Note: We can skip actually running it if it doesn't need anything
+                        acceptAmount(entry, splitInfo, amountNeeded);
+                    }
+                } else {
+                    needed.add(new HandlerType<>(entry, amountNeeded));
+                }
             }
         }
     }
@@ -114,6 +162,9 @@ public void sendPossible(EXTRA toSend, SplitInfo<TYPE> splitInfo) {
      * @param splitInfo The new split to (re)check.
      */
     public void shiftNeeded(SplitInfo<TYPE> splitInfo) {
+        if (splitInfo.isZero(splitInfo.getShareAmount())) {
+            return;
+        }
         Iterator<HandlerType<HANDLER, TYPE>> iterator = needed.iterator();
         //Use an iterator rather than a copy of the keySet of the needed subMap
         // This allows for us to remove it once we find it without  having to
diff --git a/src/test/java/mekanism/common/lib/distribution/DistributionTest.java b/src/test/java/mekanism/common/lib/distribution/DistributionTest.java
diff --git a/src/test/java/mekanism/common/lib/distribution/handler/LyingAmountIntegerHandler.java b/src/test/java/mekanism/common/lib/distribution/handler/LyingAmountIntegerHandler.java
