-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Description
Problem
- Incorrect handling of 1D rotations: In 2.x, rotating a 1D vector produces incorrect results. For example, suppose a vector
vis created withlet v = createVector(1). In 1D space, only two angles are valid:0andPIradians. Although rotating byPIis valid and should produce[-1], the operation in 2.x leavesvunchanged. Supporting 1D rotations would require fixing this bug, and would require us to add error messages for angles other than0orPIradians /180degrees (unless we want to allow invalid input). - Inconsistent handling of 3D rotations: Another issue is that
rotate()is not designed for rotations in 3D (or higher), but there’s no reason why it wouldn’t support 3D rotations. In fact, the standalonerotate()function does support 3D rotations, which means the current lack of 3D support in the vector version hurts predictability and consistency, in addition to limiting utility.
Proposed solution
Rotations in 1D space are likely confusing and of limited value. So, the simplest option is as follows:
- Maintain support for 2D vectors.
- Add support for 3D vectors.
- Add error messages for vectors of other dimensions.
- Update compatibility README (see second point under "Breaking changes")
Breaking changes in principle, not in fact
This solution requires us to consider two breaking changes that are unlikely to cause actual disruption:
-
Breaking change to "1D" case (necessary, unlikely to break code in practice): Disallowing 1D rotations would break 1.x code like
createVector(1).rotate(HALF_PI), which would interpret the vector as[1, 0, 0]and would correctly turn it into[0, 1, 0](actually it returns something extremely close to that, due to floating point issues). But this appears to be a necessary change, as that code doesn’t make sense in the context of 2.x. A 1D vector cannot be rotated byHALF_PI. Also, this case appears unlikely to occur in practice. -
Breaking change to static method (unlikely to cause any disruption in practice): To extend the method to support 3D vectors, an optional
axisparameter can be added, as in the standalonerotate()function. This requires a breaking change in principle, since theaxisparameter would be indistinguishable from thetargetparameter in the static version of the method (it wouldn’t be possible to distinguish it based on parameter position or type). However, it appears that this feature is not used, so this would not be a breaking change in practice. The compatibility README could document the change just to be safe.
Disruption assessment: 0%
I performed the Google search site:https://editor.p5js.org/ "p5.Vector.rotate" and found only three sketches using the static version of the rotate() method, none of which used a target parameter. For context, I was easily able to find 50 sketches using the heading() method using a similar search, and I stopped searching once I reached that total. This strongly suggests that the target parameter of the static rotate() method is not actually used in practice.
Indicating support
If you're interested in helping to move the current issue along, you can add a comment to indicate whether you support the proposed solution.
Blocking issues (foundational issues for all of p5.Vector)
You could also add a comment on the following issues to indicate whether you support the proposals they contain, along with a brief rationale:
-
#8153: In order to carry out the proposed solution for
rotate(), we need to be able to reliably distinguish 2D and 3D vectors from vectors of other dimensions, including 1D vectors. Consensus on #8153 would clarify the meaning of 1D, 2D, and 3D. For example, in 1.x, 1D meant$y=0$ and$z=0$ . According to the proposal in #8153, 1D in 2.x would mean the vector has exactly one defined element (this would resolve an inconsistency with the current implementations of.x,.y,.zandtoString()). -
#8155: Consensus on a user-facing API for checking dimension size would enable a simpler, more stable implementation of
rotate(), as it would provide a reliable way to check the dimension size of the input. Adding a comment on the current leading proposal (.shape), either in support of the proposal or against it, would be helpful. The exact format of this API depends on the outcome of an ongoing review of getter/setter patterns in p5, so any comments can focus on the "shape" concept.
Metadata
Metadata
Assignees
Type
Projects
Status