Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[p5.js 2.0] Vector refactor n-dimentional and Matrix class extraction #7376

Open
wants to merge 25 commits into
base: dev-2.0
Choose a base branch
from

Conversation

holomorfo
Copy link

Resolves #6765

@davepagurek @limzykenneth
Changes:

I have added new inline documentation for the new methods. Here I share the analysis of the pending methods to update and some questions/challenges I'm facing:

Vector methods

  • constructor
  • § (get) values - DONE
  • § (set) values -> Check dimentions? YES
  • (get) x
  • getValue
  • setValue
  • § (get) y
  • (get) z
  • (get) w
  • § (set) x
  • (set) y
  • § (set) z
  • § (set) w
  • toString
    Comment from Ken regarding serializtion DONE Pending more general serialization.
  • set
    Need to set dimension also? Yes, DONE
  • сору
  • add
  • rem
    Only 3D for now
  • sub
  • mult
  • div
    Updated working for scalar, vector and array, question about different dimension vectors, how should it be handled? should we only divide what is possible and leave the rest unchanged? For example, what to do in this case [1,2,3]/[8,9] should it be: [1/8,2/9,3] or in the other case [8,9]/[1,2,3] =? [8/1, 9/2] ? I've not seen this implementation or inference in any other mathematics library or in general. @limzykenneth @davepagurek
  • mag
  • magSq
  • dot
  • cross Needs determinant Only 2D for now
  • dist
  • normalize
  • limit
  • setMag
    why is it call setMag?
    maybe because it does it in place
  • heading
    Check _fromRadians
  • setHeading
    Only for 2D
  • rotate
    Only for 2D
  • angleBetween needs cross to be nD only 3D for now
  • lerp Problem with inference of last value being amount for ND Only 3d for Now
  • slerp Problem with inference of last value being amount for ND Only 3d for Now
  • reflect
  • array
    In theory this should return only this.values however, this breaks some functionality on the test " p5.RendererGL > color interpolation > geometry with stroke colors use their colors" because of the default always returning a 3 elements array, will retain ad 3D for now but will re-visit
  • equals
  • clampToZero
  • _clampToZero

Static methods

  • fromAngle - only 2D
  • fromAngles - only 3D
  • random2D
  • random3D
  • сору
  • add
  • rem
    Needs module ND Only 3D for now
  • sub
  • mult
  • rotate
    needs rotate Nd Only 3D for now
  • div
    See comments in the non static method.
  • dot
  • cross - Only 3D for now
  • dist
  • lerp - Only 3D for now
  • slerp - Only 3D for now
  • mag
  • magSq
  • normalize
  • limit
  • setMag
  • heading
  • angleBetween - Only 3D for now
  • reflect
  • array - Only 3D for now, see above
  • equals

PR Checklist

@holomorfo
Copy link
Author

Hi @limzykenneth and @davepagurek this is a refactor of PR #7277 which I turned into a draft to avoid confusion. In this one, the idea is to have most of the changes ready for merging and do incremental updates as needed. I have a question regarding the comment:

implement the multiplication as a single nd multiplication function.
Will this be the vector to matrix multiplication? Since in the MatrixGL implementation it is fixed in only 3x3 or 4x4 matrices, so nd vector vs 3 or 4 matrices will not be compatible, what do you think @limzykenneth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant