Fixed frameskip with value != -1 running potentially nonstop and using an invalid internal frame count

Author: Bytekeeper

src/jmh/java/org/bk/ass/SimulatorBenchmark.java

@@ -2,7 +2,7 @@
 
 import org.bk.ass.sim.BWAPI4JAgentFactory;
 import org.bk.ass.sim.Simulator;
-import org.bk.ass.sim.Simulator.SimulatorBuilder;
+import org.bk.ass.sim.Simulator.Builder;
 import org.openbw.bwapi4j.test.BWDataProvider;
 import org.openbw.bwapi4j.type.UnitType;
 import org.openjdk.jmh.annotations.*;
@@ -20,8 +20,8 @@ public static class MyState {
 
     @Setup(Level.Invocation)
     public void setup() {
-      simulator = new SimulatorBuilder().build();
-      simulatorFS4 = new SimulatorBuilder().withFrameSkip(4).build();
+      simulator = new Builder().build();
+      simulatorFS4 = new Builder().withFrameSkip(4).build();
 
       for (int i = 0; i < 30; i++) {
         simulator.addAgentA(factory.of(UnitType.Zerg_Mutalisk));

src/main/java/org/bk/ass/sim/Simulator.java

@@ -123,10 +123,10 @@ public int simulate() {
    * Simulate the given number of frames. If negative, simulation will only stop if one party has no
    * agents left. If units decide to run away, this could be an endless loop - use with care!
    *
-   * @return the actual number of frames simulated, usually the given number of frames
+   * @return the number of frames left after simulating, usually 0
    */
   public int simulate(int frames) {
-    if (frames > 0) frames = (frames + frameSkip - 1) / frameSkip;
+    if (frames > 0) frames += Math.floorMod(frameSkip - frames, frameSkip);
     while (frames != 0 && !playerA.isEmpty() && !playerB.isEmpty()) {
       frames -= frameSkip;
       if (!step()) {
@@ -342,24 +342,24 @@ public IntEvaluation subtract(IntEvaluation other) {
     }
   }
 
-  public static final class SimulatorBuilder {
+  public static final class Builder {
     private Behavior playerABehavior = new RoleBasedBehavior();
     private Behavior playerBBehavior = new RoleBasedBehavior();
     private int frameSkip = 1;
 
-    public SimulatorBuilder() {}
+    public Builder() {}
 
-    public SimulatorBuilder withPlayerABehavior(Behavior playerABehavior) {
+    public Builder withPlayerABehavior(Behavior playerABehavior) {
       this.playerABehavior = playerABehavior;
       return this;
     }
 
-    public SimulatorBuilder withPlayerBBehavior(Behavior playerBBehavior) {
+    public Builder withPlayerBBehavior(Behavior playerBBehavior) {
       this.playerBBehavior = playerBBehavior;
       return this;
     }
 
-    public SimulatorBuilder withFrameSkip(int frameSkip) {
+    public Builder withFrameSkip(int frameSkip) {
       this.frameSkip = frameSkip;
       return this;
     }

src/test/java/org/bk/ass/sim/SimulatorTest.java

@@ -1,7 +1,7 @@
 package org.bk.ass.sim;
 
 import org.bk.ass.PositionOutOfBoundsException;
-import org.bk.ass.sim.Simulator.SimulatorBuilder;
+import org.bk.ass.sim.Simulator.Builder;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.openbw.bwapi4j.test.BWDataProvider;
@@ -13,7 +13,7 @@
 
 class SimulatorTest {
 
-  private Simulator simulator = new SimulatorBuilder().build();
+  private Simulator simulator = new Builder().build();
   private BWAPI4JAgentFactory factory = new BWAPI4JAgentFactory(null);
 
   @BeforeAll
@@ -108,7 +108,7 @@ void MMvsMM() {
   @Test
   void MMvsMM_FS4() {
     // GIVEN
-    simulator = new SimulatorBuilder().withFrameSkip(4).build();
+    simulator = new Builder().withFrameSkip(4).build();
     simulator.addAgentA(factory.of(UnitType.Terran_Marine));
     simulator.addAgentA(factory.of(UnitType.Terran_Marine));
     simulator.addAgentA(factory.of(UnitType.Terran_Marine));
@@ -657,7 +657,7 @@ void archonSplashShouldNotAffectOwnUnits() {
   @Test
   void goonShouldDieWhenRunningAwayFromScout() {
     // GIVEN
-    simulator = new SimulatorBuilder().withPlayerABehavior(new RetreatBehavior()).build();
+    simulator = new Builder().withPlayerABehavior(new RetreatBehavior()).build();
     simulator.addAgentA(factory.of(UnitType.Protoss_Dragoon).setX(500));
     simulator.addAgentB(factory.of(UnitType.Protoss_Scout).setX(495));
 
@@ -704,7 +704,7 @@ void shouldResetCollisions() {
   @Test
   void shouldNotAttackStasisedUnitsNorBeAttackedByThem() {
     // GIVEN
-    simulator = new SimulatorBuilder().build();
+    simulator = new Builder().build();
     simulator.addAgentA(factory.of(UnitType.Terran_Goliath).setX(500).setStasised(true));
     simulator.addAgentB(factory.of(UnitType.Terran_Wraith).setX(495));
 
@@ -719,7 +719,7 @@ void shouldNotAttackStasisedUnitsNorBeAttackedByThem() {
   @Test
   void shouldAttackLockeddownUnitsButDontBeAttackedByThem() {
     // GIVEN
-    simulator = new SimulatorBuilder().build();
+    simulator = new Builder().build();
     simulator.addAgentA(factory.of(UnitType.Terran_Goliath).setX(500).setLockeddown(true));
     simulator.addAgentB(factory.of(UnitType.Terran_Wraith).setX(495));
 
@@ -734,7 +734,7 @@ void shouldAttackLockeddownUnitsButDontBeAttackedByThem() {
   @Test
   void shouldNotRepairStasisedUnit() {
     // GIVEN
-    simulator = new SimulatorBuilder().build();
+    simulator = new Builder().build();
     simulator.addAgentA(
         factory.of(UnitType.Terran_Goliath).setX(500).setStasised(true).setHealth(97));
     simulator.addAgentA(factory.of(UnitType.Terran_SCV).setX(500));
@@ -749,7 +749,7 @@ void shouldNotRepairStasisedUnit() {
   @Test
   void shouldRepairLockeddownUnit() {
     // GIVEN
-    simulator = new SimulatorBuilder().build();
+    simulator = new Builder().build();
     simulator.addAgentA(
         factory.of(UnitType.Terran_Goliath).setX(500).setLockeddown(true).setHealth(97));
     simulator.addAgentA(factory.of(UnitType.Terran_SCV).setX(500));
@@ -897,7 +897,7 @@ void reaverVs12Lings() {
   @Test
   void lingsVsZealots_FS3() {
     // GIVEN
-    simulator = new SimulatorBuilder().withFrameSkip(3).build();
+    simulator = new Builder().withFrameSkip(3).build();
     simulator.addAgentA(factory.of(UnitType.Protoss_Zealot));
     simulator.addAgentA(factory.of(UnitType.Protoss_Zealot));
 
@@ -908,7 +908,7 @@ void lingsVsZealots_FS3() {
     simulator.addAgentB(factory.of(UnitType.Zerg_Zergling));
 
     // WHEN
-    simulator.simulate(-1);
+    simulator.simulate(250);
 
     // THEN
     assertThat(simulator.getAgentsA()).size().isOne();