From e8c1e6ed8b36f7b0ed6fede88916eb1dfebb4f86 Mon Sep 17 00:00:00 2001
From: Unknown <marty.oehme@gmail.com>
Date: Fri, 19 Oct 2018 11:50:34 +0200
Subject: [PATCH] Refactor Engine Methods

Extract method checking for update necessity.
---
 lib/src/Engine.dart       | 33 ++++++++------
 test/src/engine_test.dart | 95 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/lib/src/Engine.dart b/lib/src/Engine.dart
index ad101ca..5653bb9 100644
--- a/lib/src/Engine.dart
+++ b/lib/src/Engine.dart
@@ -1,5 +1,4 @@
 import 'dart:html' as html;
-import 'dart:math';
 
 import 'package:rules_of_living/src/Simulation.dart';
 
@@ -47,31 +46,37 @@ class Engine {
   }
 
   void animFrame(num now) {
-    process(now);
+    int elapsed = _elapsed.elapsedMilliseconds;
+    _elapsed.reset();
+    process(elapsed, SAFETY_TIMEOUT, update: this.update, render: this.render);
     html.window.animationFrame.then(animFrame);
   }
 
-  void process(num now) {
-    _drawLag += _elapsed.elapsedMilliseconds;
-    _updateLag += _elapsed.elapsedMilliseconds;
-    _elapsed.reset();
+  void process(int elapsed, int timeOut, {Function update, Function render}) {
+    _drawLag += elapsed;
+    _updateLag += elapsed;
 
-    while (_updateLag >= _MS_PER_STEP) {
-      if (_elapsed.elapsedMilliseconds > SAFETY_TIMEOUT) {
-        // TODO stub - give warning etc when this occurs
-        print("ERROR STUCK IN UPDATE LOOP");
-        break;
-      }
-      if (running == true) update();
+    while (running == true &&
+        _shouldUpdate(_updateLag, elapsed, timeOut) == true) {
       _updateLag -= _MS_PER_STEP;
+      if (update == null) break;
+      update();
     }
 
     if (_drawLag >= _MS_PER_FRAME) {
-      render(_updateLag / _MS_PER_STEP);
       _drawLag = 0;
+      if (render == null) return;
+      render(_updateLag / _MS_PER_STEP);
     }
   }
 
+  bool _shouldUpdate(int updateLag, int elapsed, int timeOut) {
+    if (updateLag < _MS_PER_STEP) return false;
+    if (elapsed > timeOut) throw StackOverflowError;
+
+    return true;
+  }
+
   /// Update Engine Logic
   ///
   /// Updates the logic of the engine by one tick. Should usually not be called
diff --git a/test/src/engine_test.dart b/test/src/engine_test.dart
index bd184a4..f602069 100644
--- a/test/src/engine_test.dart
+++ b/test/src/engine_test.dart
@@ -1,15 +1,106 @@
 import 'dart:html' as html;
-import 'dart:math';
+import 'package:rules_of_living/src/Simulation.dart';
+import 'package:test/test.dart';
+import 'package:mockito/mockito.dart';
 
 @TestOn('browser')
 import 'package:rules_of_living/src/Engine.dart';
-import 'package:test/test.dart';
+
+class MockSimulation extends Mock implements Simulation {
+  int updateNum = 0;
+  bool hasChanges = false;
+
+  @override
+  Map<int, bool> update() {
+    updateNum++;
+    return hasChanges ? {1: true, 2: false} : {};
+  }
+}
 
 void main() {
   Engine sut;
   setUp(() {
     sut = Engine();
   });
+  group("process", () {
+    setUp(() => sut.running = true);
+    test("errors out if updating takes too long",
+        () => expect(() => sut.process(5000, 10), throwsA(StackOverflowError)));
+    test("does not update if not enough time elapsed to pass ms per step", () {
+      bool result = false;
+      sut.stepsPerSecond = 1000;
+
+      sut.process(999, 2000,
+          update: () => result = true, render: (double interp) => null);
+
+      expect(result, true);
+    });
+    test("updates only when the ms per step threshold is crossed", () {
+      int updateNum = 0;
+      sut.stepsPerSecond = 1;
+
+      sut.process(1001, 2000, update: () => updateNum++);
+      expect(updateNum, equals(1));
+    });
+    test("updates until updateLag has been resolved", () {
+      int updateNum = 0;
+      sut.stepsPerSecond = 1;
+
+      sut.process(2999, 5000, update: () => updateNum++);
+      expect(updateNum, equals(2));
+    });
+    test("works without passing in update or render function arguments", () {
+      sut.stepsPerSecond = 1000;
+      expect(() => sut.process(500, 5000), isNot(throwsA(anything)));
+    });
+  });
+  group("update", () {
+    MockSimulation mockSim;
+    setUp(() {
+      mockSim = MockSimulation();
+      sut.simulation = mockSim;
+    });
+    test("does not error out if no simulation variable is set", () {
+      sut.simulation = null;
+      expect(() => sut.update(), isNot(throwsA(anything)));
+    });
+    test("updates simulation one tick for every time it is called", () {
+      sut.update();
+      sut.update();
+      sut.update();
+      expect(mockSim.updateNum, equals(3));
+    });
+    test("sets running to false when simulation returns no changes", () {
+      sut.running = true;
+      sut.update();
+      expect(sut.running, equals(false));
+    });
+    test("keeps running when simulation returns changes", () {
+      sut.running = true;
+      mockSim.hasChanges = true;
+
+      sut.update();
+
+      expect(sut.running, equals(true));
+    });
+  });
+  group("step", () {
+    MockSimulation mockSim;
+    setUp(() {
+      mockSim = MockSimulation();
+      sut.simulation = mockSim;
+    });
+    test("advances the simulation by one update", () {
+      sut.step();
+      expect(mockSim.updateNum, equals(1));
+    });
+    test("turns off continuous engine updates", () {
+      sut.running = true;
+      sut.step();
+
+      expect(sut.running, equals(false));
+    });
+  });
   group("canvas", () {
     test("Engine can be instantiated without canvas", () {
       expect(sut, isNot(throwsNoSuchMethodError));