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...

[inaridarkfox4231]

Sorry, I can't maintain my motivation, so I would like to close this issue.
The reason is that I don't really feel the need for the proposed function, and that I've been so busy with work lately that I'm mentally exhausted.
There's an ongoing pull request for orbitControl(), but it's a feature I'd really like to get through. So I would like to concentrate on that.
Thank you very much for your reply.

[limzykenneth]

@inaridarkfox4231 That is totally fine, we really appreaciate any effort you put in to contributing to p5.js in any way. Do work at your own pace.

I would like to keep this issue open for now as I think it would be good to have and someone else can pick up the implementation. If you like you can unsubscribe from this issue with the button on the sidebar to the right so you don't receive notifications for it.

[inaridarkfox4231]

I got it. thank you very much.

[Qianqianye]

Thank you all for working on this issue. I am inviting the current Math stewards to this discussion @limzykenneth, @ericnlchen, @ChihYungChang, @bsubbaraman, @albertomancia, @JazerUCSB, @tedkmburu, @perminder-17, @Obi-Engine10, @jeanetteandrews. Would love to hear what y'all think. Thanks!