Regarding the p5.Vector functions random2D() and random3D() using Math.random()

[inaridarkfox4231]

Topic

The p5.Vector functions random2D() and random3D() now use Math.random() to get random numbers.

random3D

static random3D() {
  const angle = Math.random() * constants.TWO_PI;
  const vz = Math.random() * 2 - 1;
  const vzBase = Math.sqrt(1 - vz * vz);
  const vx = vzBase * Math.cos(angle);
  const vy = vzBase * Math.sin(angle);
  return new p5.Vector(vx, vy, vz);
}

But for example, the p5.js functions shuffle() and randomGaussian() use p5.js random().

shuffle

p5.prototype.shuffle = function(arr, bool) {
  const isView = ArrayBuffer && ArrayBuffer.isView && ArrayBuffer.isView(arr);
  arr = bool || isView ? arr : arr.slice();

  let rnd,
    tmp,
    idx = arr.length;
  while (idx > 1) {
    rnd = (this.random(0, 1) * idx) | 0;

    tmp = arr[--idx];
    arr[idx] = arr[rnd];
    arr[rnd] = tmp;
  }

  return arr;
};

The difference between random() and Math.random() is whether they are affected by randomSeed(). With random(), you can get the same set of values ​​if you use the same seed value.
For example, with the following sketch, if you get random3D() values ​​every frame, Math.random() won't work because the values ​​will be different each time.

(latest version)

function setup() {
  createCanvas(400, 400, WEBGL);
  noStroke();
}

function draw() {
  orbitControl();

  background(0);
  randomSeed(999);
  lights();
  fill("red");
  ambientMaterial("red");
  specularMaterial(64);

  for(let i=0; i<300; i++){
    const v = p5.Vector.random3D();
    translate(v.mult(100));
    sphere(4);
    translate(-v.x,-v.y,-v.z);
  }
}

In the following video, the modified random3D() is used in the second half. The same vector can be obtained because it is affected by randomSeed().

2023-05-16.18-47-26.mp4

There is a way to prefetch and use the same set of values, but it is more convenient to have it affected by randomSeed(). I think there is no problem because both methods can be used.

However, I am not very familiar with how to rewrite it, so I would appreciate it if someone could tell me...

static random3D() {

  const angle = p5.prototype.random() * constants.TWO_PI;
  const vz = p5.prototype.random() * 2 - 1;

  const vzBase = Math.sqrt(1 - vz * vz);
  const vx = vzBase * Math.cos(angle);
  const vy = vzBase * Math.sin(angle);
  return new p5.Vector(vx, vy, vz);
}

How should I rewrite Math.random()? random()? p5.random()? p5.prototype.random()? something else...

[davepagurek]

I'm not sure that it's possible to do this on the static vector methods, since the class is shared across potentially many p5 instances if used in instance mode.

For non-static methods, I think to do this, the p5.Vector class has to store a reference to the instance it was created on. Currently, rather than storing a reference to the instance, it stores a reference to the to/fromRadians methods of the instance:

p5.js/src/math/math.js

Lines 38 to 44 in 3b0d132

	if (this instanceof p5) {
	return new p5.Vector(
	this._fromRadians.bind(this),
	this._toRadians.bind(this),
	...arguments
	);
	} else {

If we instead just pass the p5 instance, maybe storing it internally as this._pInst, like p5.Renderer does, you could add a method like this:

_random() {
  if (this._pInst) {
    return this._pInst.random();
  } else {
    return Math.random();
  }
}

...and then use this._random() in methods. This doesn't fix p5.Vector.random3D(), but we could then add a new method that mutates a vector instance, maybe createVector().setRandom3D().

[inaridarkfox4231]

We can't use p5.js functions inside static functions... ok.

I tried making a patch. Should I pass the instance like this and get _toRadians() and _fromRadians() from it?

p5.prototype.createVector = function(x, y, z) {
  if (this instanceof p5) {
    return new p5.Vector(
      this,
      ...arguments
    );
  } else {
    return new p5.Vector(x, y, z);
  }
};

p5.Vector = function Vector() {
  let x, y, z;

  // This is how it comes in with createVector()
  // This check if the first argument is a function
  if (arguments[0] instanceof p5) {
    // In this case the vector have an associated p5 instance
    this.isPInst = true;
    this._pInst = arguments[0];
    this._fromRadians = this._pInst._fromRadians.bind(this._pInst);
    this._toRadians = this._pInst._toRadians.bind(this._pInst);
    x = arguments[1] || 0;
    y = arguments[2] || 0;
    z = arguments[3] || 0;
    // This is what we'll get with new p5.Vector()
  } else {
    x = arguments[0] || 0;
    y = arguments[1] || 0;
    z = arguments[2] || 0;
  }
  /**
   * The x component of the vector
   * @property x {Number}
   */
  this.x = x;
  /**
   * The y component of the vector
   * @property y {Number}
   */
  this.y = y;
  /**
   * The z component of the vector
   * @property z {Number}
   */
  this.z = z;
};

function setup() {
  createCanvas(400, 400);
  v=createVector(1,1,1);
  console.log(v._pInst);
  angleMode(DEGREES);
  console.log(v._fromRadians(1));
  console.log(v._toRadians(1));
}

output:

p5 {_setupDone: true, ...
57.29577951308232
0.017453292519943295

And _random(), setRandom2D(), setRandom3D() are prepared like this.

_random() {
  if (this._pInst) {
    return this._pInst.random();
  } else {
    return Math.random();
  }
}

setRandom2D () {
  this.set(this.fromAngle(this._random() * constants.TWO_PI));
  return this;
}

setRandom3D () {
  const angle = this._random() * constants.TWO_PI;
  this.z = this._random() * 2 - 1;
  const vzBase = Math.sqrt(1 - this.z * this.z);
  this.x = vzBase * Math.cos(angle);
  this.y = vzBase * Math.sin(angle);
  return this;
}

How about something like this...?

[davepagurek]

I think that looks good to me! @limzykenneth does this also make sense to you?

[inaridarkfox4231]

I'm sorry, random2D is better this way... (since it can't call fromAngle())

setRandom2D () {
  const angle = this._random() * constants.TWO_PI;
  this.x = Math.cos(angle);
  this.y = Math.sin(angle);
  this.z = 0;
  return this;
}

[inaridarkfox4231]

This one is better...sorry.

p5.Vector = class {
  // This is how it comes in with createVector()
  // This check if the first argument is p5 instance
  constructor(...args) {
    let x, y, z;
    if (args[0] instanceof p5) {
      this.isPInst = true;
      this._pInst = args[0];
      this._fromRadians = this._pInst._fromRadians.bind(this._pInst);
      this._toRadians = this._pInst._toRadians.bind(this._pInst);
      x = args[1] || 0;
      y = args[2] || 0;
      z = args[3] || 0;
      // This is what we'll get with new p5.Vector()
    } else {
      x = args[0] || 0;
      y = args[1] || 0;
      z = args[2] || 0;
    }
    /**
     * The x component of the vector
     * @property x {Number}
     */
    this.x = x;
    /**
     * The y component of the vector
     * @property y {Number}
     */
    this.y = y;
    /**
     * The z component of the vector
     * @property z {Number}
     */
    this.z = z;
  }
}

[inaridarkfox4231]

It's a small detail, but I thought it would be more consistent to use this.isPInst for the internal determination of _random().

_random() {
  if (this.isPInst) {
    return this._pInst.random();
  } else {
    return Math.random();
  }
}

[limzykenneth]

This has been done before for shuffle() a bit back here and here. Although for that case it was more straightforward than for p5.Vector since it is a p5 class method and can use this.random() directly.

To give a bit of context to this._pInst which used to exist on p5.Vector, that was removed (by me) in favour of direct reference to instance methods _fromRadians() and _toRadians() because if p5.Vector holds a reference to the p5 instance it belongs to, it actually creates a cyclic dependency. In most circumstances that's fine but there was a problem with the build and test workflow that breaks upon reaching a cyclic dependency (#5367, #5470).

So in this case, my recommendation is to deal with it in the same way as _fromRadians() and _toRadians() , by providing a reference to this.random() to p5.Vector instead of providing the whole p5 instance. Hope that made sense.

[inaridarkfox4231]

There must have been a good reason why you couldn't pass the p5 instance directly... Thanks for letting me know.

I thought of an implementation like this. First pass random() directly.

p5.prototype.createVector = function(x, y, z) {
  if (this instanceof p5) {
    return new p5.Vector(
      this._fromRadians.bind(this),
      this._toRadians.bind(this),
      this.random.bind(this), // random function.
      ...arguments
    );
  } else {
    return new p5.Vector(x, y, z);
  }
};

p5.Vector = class {
  // This is how it comes in with createVector()
  // This check if the first argument is a function
  constructor(...args) {
    let x, y, z;
    if (typeof args[0] === 'function') {
      this.isPInst = true;
      this._fromRadians = args[0];
      this._toRadians = args[1];
      this._random = args[2]; // random function.
      x = args[3] || 0;
      y = args[4] || 0;
      z = args[5] || 0;
      // This is what we'll get with new p5.Vector()
    } else {
      x = args[0] || 0;
      y = args[1] || 0;
      z = args[2] || 0;
    }
    /**
     * The x component of the vector
     * @property x {Number}
     */
    this.x = x;
    /**
     * The y component of the vector
     * @property y {Number}
     */
    this.y = y;
    /**
     * The z component of the vector
     * @property z {Number}
     */
    this.z = z;
  }
}

And use this if this.isPInst is true.

// setRandom2D
p5.Vector.prototype.setRandom2D = function(){
  const rdmForAngle = (this.isPInst ? this._random() : Math.random());
  const angle = rdmForAngle * TWO_PI;
  this.x = Math.cos(angle);
  this.y = Math.sin(angle);
  this.z = 0;
  return this;
}

// setRandom3D
p5.Vector.prototype.setRandom3D = function(){
  const rdmForAngle = (this.isPInst ? this._random() : Math.random());
  const rdmForZ = (this.isPInst ? this._random() : Math.random());
  const angle = rdmForAngle * TWO_PI;
  this.z = rdmForZ * 2 - 1;
  const vzBase = Math.sqrt(1 - this.z * this.z);
  this.x = vzBase * Math.cos(angle);
  this.y = vzBase * Math.sin(angle);
  return this;
}

How about this..?

[inaridarkfox4231]

And I have to fix the copy() and cross() as well...

copy () {
  if (this.isPInst) {
    return new p5.Vector(
      this._fromRadians,
      this._toRadians,
      this._random,
      this.x,
      this.y,
      this.z
    );
  } else {
    return new p5.Vector(this.x, this.y, this.z);
  }
}

cross(v) {
  const x = this.y * v.z - this.z * v.y;
  const y = this.z * v.x - this.x * v.z;
  const z = this.x * v.y - this.y * v.x;
  if (this.isPInst) {
    return new p5.Vector(this._fromRadians, this._toRadians, this._random, x, y, z);
  } else {
    return new p5.Vector(x, y, z);
  }
}

[inaridarkfox4231]

It's a small detail, but I feel like cross() should be written like this...

cross(v) {
  const x = this.y * v.z - this.z * v.y;
  const y = this.z * v.x - this.x * v.z;
  const z = this.x * v.y - this.y * v.x;
  const result = this.copy();
  result.set(x, y, z);
  return result;
}

I think it's better to do this so that we only have to change copy() when we have to increase the number of arguments.

That's what I thought, but add(), sub(), set() use the same logic, but if there is such a function, I feel like I can organize the processing.

function getValues(x, y, z){
  const values = {};
  if (x instanceof p5.Vector) {
    values.x = x.x || 0;
    values.y = x.y || 0;
    values.z = x.z || 0;
  } else if (Array.isArray(x)) {
    values.x = x[0] || 0;
    values.y = x[1] || 0;
    values.z = x[2] || 0;
  } else {
    values.x = x || 0;
    values.y = y || 0;
    values.z = z || 0;
  }
  return values;
}

Using this, for example set() can be written as:

set (x, y, z) {
  const values = getValues(x, y, z);
  this.x = values.x;
  this.y = values.y;
  this.z = values.z;
  return this;
}

And cross() can also be applied like cross(1, 1, 1) using this.

[inaridarkfox4231]

Alternatively, if you define it like this, you may be able to write mult() and div() more conveniently (in that case, set defaultValue to 1).

function getValidatedValue(value, defaultValue){
  if (value === undefined) {
    return defaultValue;
  } else if (Number.isFinite(value) && typeof value === 'number') {
    return value;
  } else {
    console.warn(
      'The value must be either undefined or a defined finite number.'
    );
  }
  return defaultValue;
}

function getValues(x, y, z, defaultValue = 0){
  const values = {};
  if (x instanceof p5.Vector) {
    values.x = getValidatedValue(x.x, defaultValue);
    values.y = getValidatedValue(x.y, defaultValue);
    values.z = getValidatedValue(x.z, defaultValue);
  } else if (Array.isArray(x)) {
    values.x = getValidatedValue(x[0], defaultValue);
    values.y = getValidatedValue(x[1], defaultValue);
    values.z = getValidatedValue(x[2], defaultValue);
  } else {
    values.x = getValidatedValue(x, defaultValue);
    values.y = getValidatedValue(y, defaultValue);
    values.z = getValidatedValue(z, defaultValue);
  }
  return values;
}

In the case of mult() and div(), unlike add() etc., if there is one argument, the remaining two values ​​are set to that value by default, but it can be handled by simply dividing it. It feels much easier than writing similar code for each of the three cases.

[limzykenneth]

@inaridarkfox4231 I think you can go ahead with filing a PR and we can work out the implementation details in the PR (much easier to review changes/suggestions in a PR as well 😉).

Any implementation details you are not sure to include or not you can mention in the PR post or comments itself.

Thanks.

[inaridarkfox4231]

Thank you for your reply! I would like to create a pull request. I'll put the rewrite idea on hold and just implement the function.
What I want to do is implement the function, so I'd like to focus on that...