Trig based functions in sfml, angling of sprites

Question

void moveCircle(float dt, int radius) { //so center of screen will be rotation point
 setPosition(400+cos(_angle)*radius, 400 - sin(_angle)*radius); 
 _angle += (2*pi)*dt;
 if (_angle >= (2 * pi)) {
 _angle -= (2 * pi);
 }
 this->rotate(-2*dt*180);
 }
sf::Vector2f rotateCoordinate(sf::Vector2f mouseCoords, float angle) {
 return sf::Vector2f((mouseCoords.x*cos(angle) + mouseCoords.y*sin(angle)), (mouseCoords.y*cos(angle) - mouseCoords.x*sin(angle)));
 }
void angleShipToMouse(sf::RenderWindow* window) {
 if (sf::Mouse::isButtonPressed(sf::Mouse::Button::Left)) {
 sf::Vector2f mousePosition(sf::Mouse::getPosition(*window));
 std::cout << mousePosition.x << ", " << mousePosition.y << std::endl; 
 if (mousePosition.x > _windowWidth || mousePosition.x < 0 || mousePosition.y<0 || mousePosition.y>_windowHeight) {
 return;
 }
 //position of mouse relative to ship in 0 rotation reference frame(absolute)
 sf::Vector2f mouseRelativeShip(mousePosition.x - getX(), (mousePosition.y - getY())); 
 //adjusting coordinates to any rotation the sprite has to account for change in orientation of axis. 
 mouseRelativeShip = rotateCoordinate(mouseRelativeShip, _angle*(pi/180)); 
 float angle = atan2(mouseRelativeShip.y, mouseRelativeShip.x) *(180 / pi); //atan2 produces negative angles QUADS 1&2, positive in QUADS 3&4
 this->rotate(angle);
 _angle = this->getRotation(); 
 }
 }

So these are different fns obviously the first one is unrelated really to the other 2 - just want your opinion on how its done since i didnt look at any examples of others before doing it. I'm wondering if my angleShipToMouse is good code because I honestly got really confused on whether I wanted the local coordinates of the object to show y+ as being "above" the sprite or to just keep it as y- and how this would affect the rotation, in fact it took a long time of just tinkering to get it to work and I'm unsure specifically why this setup works with y- pointing up. I understand trig but the whole axis issue and the fact that rotation is measured clockwise here when it's normally anti-clockwise, really messed up my brain, lol.

Ship

Gets angled when you press left mouse click in said direction

The circle one just moves in a circle with the space ship angled in a way that looks like its travelling around it.

• 200_success: It's hard to review this code without more context. What does this->rotate(angle) do? What does this->getRotation() do? What is _angle for? You only ever write to it, but I don't see how you're using it. A diagram and/or screenshot would help as well.
• Giancarlo Gatti: Ok thanks that's true. posted some revisements. it's a simple program lol cus i havent used trig that much yet. _angle just represents the object's current rotation from when its initially loaded as that's been rotated 0 degrees.

Answer 1

Single Responsibility

Overall, I see one really big thing that I think needs to be addressed before anything else can be addressed. Namely, your class is doing way too many things. A class should have one single purpose. It should avoid relying on other classes as much as reasonably possible. This is known as loose coupling. A class that figures out positions and angles of a ship should not have to know anything about windows or mouse buttons. As such, I'd change the prototype for void angleShipToMouse to something like this:

void angleShipToMouse(const sf::Vector2f& mousePosition)

I'd then remove the logic that gets the mouse button state and checks whether a button is pressed and whether the position is within the window bounds and put those somewhere else in your program.

Do as few transformations of data as necessary

Several places in your code you're converting between degrees and radians. There are several problems with doing this:

1. It becomes difficult to figure out or remember which units a variable is in, particularly if it's named something generic like angle or rotation.
2. It's inefficient in both the space the program takes up and the time it takes to execute. It might not matter here, but maybe it will later.
3. It can introduce inaccuracies. Again, they may not be noticeable in this code, but they could be multiplied later.

In general, all code other than code which displays values to users is probably going to want to operate in radians. All math functions in the standard library use radians. You should probably be using radians internally and only converting to degrees for display.

Also, when you do want to convert between degrees and radians, you should write a named function to do it and call that function rather than always writing the values inline. It's easy to mistype one of the values and someone not familiar with the transformation might not immediately understand it looking at the code. If you had functions named degreesToRadians() and radiansToDegrees(), it becomes immediately obvious to anyone reading the code what you're doing.

Avoid magic numbers

In moveCircle() you have the value 400 twice in the first line. What does it represent? I'm guessing that the window or view this is in is 800x800 pixels, so (400, 400) is the center? If so, you should at least have a constant defined for that. I recommend something like:

const sf::Vector2f kWindowCenter(400, 400);

Or better yet, pass in the point you want to rotate around! What happens if you want to let the user resize the window (or view)? What if you want to rotate around a different point?