Merge pull request #4048 from CrypticGuy/master

bug in angleBetween() #3973

Author: stalgiag

src/math/p5.Vector.js

@@ -1181,18 +1181,22 @@ p5.Vector.prototype.rotate = function rotate(a) {
  * </code>
  * </div>
  */
+
 p5.Vector.prototype.angleBetween = function angleBetween(v) {
   const dotmagmag = this.dot(v) / (this.mag() * v.mag());
   // Mathematically speaking: the dotmagmag variable will be between -1 and 1
   // inclusive. Practically though it could be slightly outside this range due
   // to floating-point rounding issues. This can make Math.acos return NaN.
   //
   // Solution: we'll clamp the value to the -1,1 range
-  const angle = Math.acos(Math.min(1, Math.max(-1, dotmagmag)));
-  if (this.p5) return this.p5._fromRadians(angle);
+  let angle;
+  angle = Math.acos(Math.min(1, Math.max(-1, dotmagmag)));
+  angle = angle * Math.sign(this.cross(v).z || 1);
+  if (this.p5) {
+    angle = this.p5._fromRadians(angle);
+  }
   return angle;
 };
-
 /**
  * Linear interpolate the vector to another vector
  *

test/unit/math/p5.Vector.js

@@ -120,26 +120,29 @@ suite('p5.Vector', function() {
     test('should not trip on rounding issues in 2D space', function() {
       var v1 = myp5.createVector(-11, -20);
       var v2 = myp5.createVector(-5.5, -10);
-      expect(v1.angleBetween(v2)).to.be.closeTo(0, 0.00001);
+      var res = v1.angleBetween(v2);
+      //expect(Math.abs(v1.angleBetween(v2))).to.be.closeTo(0, 0.00001);
+      expect(res).to.be.closeTo(0, 0.00001);
 
       var v3 = myp5.createVector(-11, -20);
       var v4 = myp5.createVector(5.5, 10);
+      expect(Math.abs(v3.angleBetween(v4))).to.be.closeTo(180, 0.00001);
       expect(v3.angleBetween(v4)).to.be.closeTo(180, 0.00001);
     });
 
     test('should not trip on rounding issues in 3D space', function() {
       var v1 = myp5.createVector(1, 1.1, 1.2);
       var v2 = myp5.createVector(2, 2.2, 2.4);
 
-      var angle = v1.angleBetween(v2);
+      var angle = Math.abs(v1.angleBetween(v2));
       expect(angle).to.be.closeTo(0, 0.00001);
     });
 
     test('should return NaN for zero vector', function() {
       var v1 = myp5.createVector(0, 0, 0);
       var v2 = myp5.createVector(2, 3, 4);
 
-      expect(v1.angleBetween(v2)).to.be.NaN; // jshint ignore:line
+      expect(Math.abs(v1.angleBetween(v2))).to.be.NaN; // jshint ignore:line
       expect(v2.angleBetween(v1)).to.be.NaN; // jshint ignore:line
     });
   });
@@ -869,14 +872,15 @@ suite('p5.Vector', function() {
         v2 = new p5.Vector(2, 2, 0);
         res = v1.angleBetween(v2);
         expect(res).to.be.closeTo(Math.PI / 4, 0.01);
+        expect(v2.angleBetween(v1)).to.be.closeTo(-1 * Math.PI / 4, 0.01);
       });
     });
 
     suite('with [2,0,0] and [-2,0,0]', function() {
       test('should be 180 deg difference', function() {
         v1 = new p5.Vector(2, 0, 0);
         v2 = new p5.Vector(-2, 0, 0);
-        res = v1.angleBetween(v2);
+        res = Math.abs(v1.angleBetween(v2));
         expect(res).to.be.closeTo(Math.PI, 0.01);
       });
     });
@@ -885,14 +889,23 @@ suite('p5.Vector', function() {
       test('should be 135 deg difference', function() {
         v1 = new p5.Vector(2, 0, 0);
         v2 = new p5.Vector(-2, -2, 0);
-        res = v1.angleBetween(v2);
-        expect(res).to.be.closeTo(Math.PI / 2 + Math.PI / 4, 0.01);
+        expect(v1.angleBetween(v2)).to.be.closeTo(
+          -1 * (Math.PI / 2 + Math.PI / 4),
+          0.01
+        );
+        expect(v2.angleBetween(v1)).to.be.closeTo(
+          Math.PI / 2 + Math.PI / 4,
+          0.01
+        );
       });
 
       test('should be commutative', function() {
         v1 = new p5.Vector(2, 0, 0);
         v2 = new p5.Vector(-2, -2, 0);
-        expect(v1.angleBetween(v2)).to.be.closeTo(v2.angleBetween(v1), 0.01);
+        expect(Math.abs(v1.angleBetween(v2))).to.be.closeTo(
+          Math.abs(v2.angleBetween(v1)),
+          0.01
+        );
       });
     });
   });