quinton-ashley / p5play

JavaScript game engine that uses q5.js/p5.js for graphics and Box2D for physics.
https://p5play.org
GNU Affero General Public License v3.0
667 stars 184 forks source link

`sprite.rotateTo` and `sprite.rotate` issues #307

Closed quinton-ashley closed 8 months ago

quinton-ashley commented 8 months ago

Recently on Discord @ coding398 has pointed out some issues with the sprite.rotateTo and sprite.rotate functions not working as documented. Their behavior is also currently a bit ambiguously defined which is part of the issue.

The sprite.rotateTo function was meant to rotate a sprite to an angle by the shortest angular distance to that angle and at a given rotation speed. However, the problem is that a positive rotation speed can be specified when rotating the shortest angular distance requires a negative rotation speed. Currently this erroneously causes the sprite to rotationally teleport to the destination rotation almost immediately.

Here's an example:

sprite.rotation = 0;

// single line syntax
sprite.rotateTo(200, 2);

// alternative syntax
sprite.rotationSpeed = 2;
sprite.rotateTo(200);

Here's a few ways to solve this:

Option 1. Use the user's rotationSpeed of 2 to rotate the sprite 200 degrees. To rotate the shortest angular distance users would have to do sprite.rotateTo(200, -2); or sprite.rotateTo(-160, -2);. This would be a more straight forward way of doing it and more intuitive just from looking at the code.

Option 2. Adjust the sign of the rotation speed, making it -2, to achieve the shortest angular distance of rotation, which would be -160 degrees. In the documentation the speed param to this function would be labelled an absolute amount of rotation. This is perhaps what most users want to happen in typical use cases, especially when rotating the sprite to face a position, for example sprite.rotateTo(mouse)

Option 3. Do option 1 for rotating to an angle but do option 2 for rotating a sprite to a position.

I'm interested in hearing the community's thoughts on this and how this issue with rotateTo should be fixed.

Additionally, the sprite.rotate function is meant to provide users an easy way to rotate the sprite by an angle at a rotation speed.

There's a similar issue here. How should a rotation of -30 degrees at a speed of -2 be done? Should the angle to rotate by be absolute or should the rotation speed be absolute?

sprite.rotate(-30, 2);
// or
sprite.rotate(30, -2);
// or
sprite.rotate(-30, -2);

I think if either the rotation amount, rotation speed, or both are negative, then the sprite should rotate with a negative rotationSpeed. No wrong answers! I would document it like this though sprite.rotate(-30, -2);

quinton-ashley commented 8 months ago

My current thought on this is that rotateTo should just do option 1, as it's just more straightforward and readable. Then perhaps there should be another function rotateMinDistTo or something like that which would implement option 2.

quinton-ashley commented 8 months ago

Fixed in v3.19.11

https://github.com/quinton-ashley/p5play/releases/tag/3.19.11