Fix ULV recipe overclocking bug

ULV recipes erroneously overclocked to the highest possible energy tier,
needlessly increasing energy usage for recipes that reached maximum
speed at a lower tier of overclocking.

Duration is now properly considered to prevent this from happening.

Added a test case to AbstractRecipeLogicTest to verify the fix.

Also refactored AbstractRecipeLogic::calculateOverclock to accept a
Recipe instead of extracted values from a Recipe, and to calculate the
number of overclocks for non-ULV recipes without a loop.

Add some more documentation to related methods.

Author: Exaxxion

src/main/java/gregtech/api/capability/impl/AbstractRecipeLogic.java

@@ -279,8 +279,15 @@ protected static boolean areItemStacksEqual(ItemStack stackA, ItemStack stackB)
                 ItemStack.areItemStackTagsEqual(stackA, stackB));
     }
 
+    /**
+     * Attempts to start the specified recipe. A recipe will fail to start if there is insufficient energy,
+     * the output inventories are full, or the required ingredients are not present.
+     *
+     * @param recipe the recipe to start
+     * @return {@code true} if the recipe was started and inputs consumed, {@code false} otherwise.
+     */
     protected boolean setupAndConsumeRecipeInputs(Recipe recipe) {
-        int[] resultOverclock = calculateOverclock(recipe.getEUt(), recipe.getDuration());
+        int[] resultOverclock = calculateOverclock(recipe);
         int totalEUt = resultOverclock[0] * resultOverclock[1];
         IItemHandlerModifiable importInventory = getInputInventory();
         IItemHandlerModifiable exportInventory = getOutputInventory();
@@ -306,11 +313,32 @@ protected boolean setupAndConsumeRecipeInputs(Recipe recipe) {
         return recipe.matches(true, importInventory, importFluids);
     }
 
-    protected int[] calculateOverclock(int EUt, int duration) {
-        return calculateOverclock(EUt, this.overclockPolicy.getAsLong(), duration);
+    /**
+     * Performs overclocking with voltage using {@link #overclockPolicy} for the voltage.
+     * @see #calculateOverclock(Recipe, long)
+     */
+    protected int[] calculateOverclock(@NotNull Recipe recipe) {
+        return calculateOverclock(recipe, this.overclockPolicy.getAsLong());
     }
 
-    protected int[] calculateOverclock(int EUt, long voltage, int duration) {
+    /**
+     * Attempts to overclock a given recipe.
+     * <ul>
+     * <li>Recipes at or below 16 EU/t overclock by halving duration and quadrupling energy consumption, until the duration
+     * reaches a single game tick or the EU/t can no longer be increased.</li>
+     * <li>Recipes above 16 EU/t overclock by dividing duration by 2.8 and quadrupling energy consumption, until the duration
+     * reaches fewer than 3 game ticks or the EU/t can no longer be increased.</li>
+     * </ul>
+     * @param recipe the Recipe to overclock
+     * @param voltage the maximum EU/t to use for overclocking. This value must be positive.
+     * @return an {@code int[]} of length 2, where [0] is the computed EU/t and [1] the duration.
+     */
+    protected int[] calculateOverclock(@NotNull final Recipe recipe, final long voltage) {
+        assert(voltage >= 0);
+
+        int EUt = recipe.getEUt();
+        int duration = recipe.getDuration();
+
         if (!allowOverclocking) {
             return new int[] {EUt, duration};
         }
@@ -322,19 +350,22 @@ protected int[] calculateOverclock(int EUt, long voltage, int duration) {
             EUt = -EUt;
         if (EUt <= 16) {
             int multiplier = EUt <= 8 ? tier : tier - 1;
-            int resultEUt = EUt * (1 << multiplier) * (1 << multiplier);
-            int resultDuration = duration / (1 << multiplier);
-            return new int[]{negativeEU ? -resultEUt : resultEUt, resultDuration};
+            // Restrict the maximum number of overclocks to how many times the duration can be halved
+            int speedCap = (31 - Integer.numberOfLeadingZeros(duration));
+            if(multiplier > speedCap) multiplier = speedCap;
+            EUt *= (1 << 2 * multiplier);
+            duration /= (1 << multiplier);
         } else {
-            int resultEUt = EUt;
-            double resultDuration = duration;
-            //do not overclock further if duration is already too small
-            while (resultDuration >= 3 && resultEUt <= GTValues.V[tier - 1]) {
-                resultEUt *= 4;
-                resultDuration /= 2.8;
+            // Restrict the maximum number of overclocks to how many times the duration is divisible by 2.8
+            int speedCap = (int) (Math.log(duration) / Math.log(2.8));
+            int dt = tier - recipe.getBaseTier();
+            if(dt > speedCap) dt = speedCap;
+            if(dt > 0) {
+                EUt *= Math.pow(4, dt);
+                duration /= Math.pow(2.8, dt);
             }
-            return new int[]{negativeEU ? -resultEUt : resultEUt, (int) Math.ceil(resultDuration)};
         }
+        return new int[]{negativeEU ? -EUt : EUt, duration};
     }
 
     protected int getOverclockingTier(long voltage) {
@@ -356,7 +387,7 @@ public String[] getAvailableOverclockingTiers() {
     }
 
     protected void setupRecipe(Recipe recipe) {
-        int[] resultOverclock = calculateOverclock(recipe.getEUt(), recipe.getDuration());
+        int[] resultOverclock = calculateOverclock(recipe);
         this.progressTime = 1;
         setMaxProgress(resultOverclock[1]);
         this.recipeEUt = resultOverclock[0];
@@ -399,8 +430,13 @@ public List<Pair<ItemStack, Integer>> getChancedItemOutputs() {
         return chancedItemOutputs;
     }
 
+    /**
+     * <b>This is NOT for energy calculations. Use {@link #getOverclockingTier(long)} for energy.</b><br />
+     * Used to override the machine's tier for the purposes of determining chanced outputs.
+     * The default implementation simply returns the overclocking tier of the maximum voltage of the machine.
+     */
     protected int getMachineTierForRecipe(Recipe recipe) {
-        return GTUtility.getTierByVoltage(getMaxVoltage());
+        return getOverclockingTier(getMaxVoltage());
     }
 
     protected void completeRecipe() {

src/main/java/gregtech/api/capability/impl/RecipeLogicSteam.java

@@ -19,6 +19,7 @@
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.WorldServer;
 import net.minecraftforge.fluids.IFluidTank;
+import org.jetbrains.annotations.NotNull;
 
 public class RecipeLogicSteam extends AbstractRecipeLogic {
 
@@ -159,12 +160,12 @@ protected void completeRecipe() {
     }
 
     @Override
-    protected int[] calculateOverclock(int EUt, long voltage, int duration) {
+    protected int[] calculateOverclock(@NotNull final Recipe recipe) {
         if (!isHighPressure) {
             //disallow overclocking for low pressure bronze machines
-            return new int[]{EUt, duration};
+            return new int[]{recipe.getEUt(), recipe.getDuration()};
         }
-        return super.calculateOverclock(EUt, voltage, duration);
+        return super.calculateOverclock(recipe);
     }
 
     @Override

src/test/java/gregtech/api/capability/impl/AbstractRecipeLogicTest.java

@@ -129,4 +129,73 @@ protected long getMaxVoltage() {
 		assertTrue(AbstractRecipeLogic.areItemStacksEqual(arl.getOutputInventory().getStackInSlot(0),
 		                                                  new ItemStack(Blocks.STONE, 1)));
 	}
+
+	@Test
+	public void ulv_recipe_overclock_test() {
+
+		World world = DummyWorld.INSTANCE;
+
+		// Create an empty recipe map to work with
+		RecipeMap<SimpleRecipeBuilder> map = new RecipeMap<>("chemical_reactor_2",
+		                                                     0,
+		                                                     2,
+		                                                     0,
+		                                                     2,
+		                                                     0,
+		                                                     3,
+		                                                     0,
+		                                                     2,
+		                                                     new SimpleRecipeBuilder().EUt(30),
+		                                                     false);
+
+		MetaTileEntity at =
+			GregTechAPI.registerMetaTileEntity(198,
+			                                   new SimpleMachineMetaTileEntity(
+				                                   new ResourceLocation(GTValues.MODID, "chemical_reactor.uv"),
+				                                   map,
+				                                   Textures.CHEMICAL_REACTOR_OVERLAY,
+				                                   1));
+
+		MetaTileEntity atte = new MetaTileEntityHolder().setMetaTileEntity(at);
+		atte.getHolder().setWorld(world);
+
+		// Recipe that will reach maximum speed (1t) at ZPM
+		map.recipeBuilder()
+		   .inputs(new ItemStack(Blocks.COBBLESTONE))
+		   .outputs(new ItemStack(Blocks.STONE))
+		   .EUt(4).duration(128)
+		   .buildAndRegister();
+
+		// UV-tier machine
+		AbstractRecipeLogic arl = new AbstractRecipeLogic(atte, map) {
+			@Override
+			protected long getEnergyStored() {
+				return Long.MAX_VALUE;
+			}
+
+			@Override
+			protected long getEnergyCapacity() {
+				return Long.MAX_VALUE;
+			}
+
+			@Override
+			protected boolean drawEnergy(int recipeEUt) {
+				return true;
+			}
+
+			@Override
+			protected long getMaxVoltage() {
+				return GTValues.V[GTValues.UV];
+			}
+		};
+
+		arl.getInputInventory().insertItem(0, new ItemStack(Blocks.COBBLESTONE, 16), false);
+		Recipe recipe = arl.findRecipe(arl.getMaxVoltage(), arl.getInputInventory(), arl.getInputTank());
+		int[] oc = arl.calculateOverclock(recipe);
+
+		// Expect seven overclocks to a half-amp of ZPM and capped 1t duration, instead of previous behavior where
+		// the energy consumption increased as though an eighth overclock was performed
+		assertEquals(GTValues.V[GTValues.ZPM] / 2, oc[0]);
+		assertEquals(1, oc[1]);
+	}
 }