Code review for jakub

codeHelp.cpp

////////////////////////////////////////////////////////////////////////////////
/// Here's a quick review of your code.
/// I didnt dive too much into breaking down your logic.
/// The main focus of my comments was making your code more readable.
/// In professional enviroments, readability is everything!
/// All of my comments use a "///" instead of "//"
////////////////////////////////////////////////////////////////////////////////

//***********************************************
//when hits the space key   /// ADD SPACE BETWEEN "//" and rest of comment.
void Player::Jump()
{
	if (jumpState == 0)     /// USE ENUMS INSTEAD OF JUMP STATE. ITS MUCH LESS HACKY.
                        /// ALSO WHAT DO THE MOVE STATES MEAN? WHAT ARE THE POSSIBE STATES.
                        /// I SHOULDNT HAVE TO READ YOUR CODE TO KNOW WHAT THE POSSIBLE STATES ARE.
	{                       /// ADDITIONALLY, WHY EVEN USE JUMP STATES? CAN VELY TELL YOU YOUR STATE?
		StartJump(38);
	}
}

void Player::StartJump(int y)
{
	this->jumpSpeed = y;     /// WHY "this->" here. KEEP YOUR CODE CONSISTENT!
	velY = -jumpSpeed;       /// BUT NOT HERE?

	setSpriteID(2);

	jumpState = 1;
}

void Player::resetJump()    /// WHY ARE SOME FUNCTIONS CAPITALIZED AND OTHERS NOT?
{
	jumpState = 0;
	velY = 0;
	gravity = 3;    /// WHY WOULD GRAVITY EVER CHANGE? GRAVITY IS NORMALLY A CONSTANT
}


//***********************************************
//Update position Y
void Player::UpdatePosY(int moveSpeed)
{
	//collision state
	bool collL, collR;

	if (moveSpeed > 0)
	{
		if (!collL && !collR)
		{
            /// TRY NOT TO EXCEED 80 CHARACTERS PER LINE!
			if (posY > (Config::getFM()->getH() / 2) - getHitBoxY() && Config::getSM()->GetGame()->getMoveMap() && Config::getSM()->GetGame()->getPosY() > 0)
			{
				//moveMap
				Config::getSM()->GetGame()->MoveMap(0, moveSpeed);
			}
				//move Player Y
			else posY += moveSpeed;
		}
		//if hit reset velY
		else velY = 0;
	}
	else if (moveSpeed < 0)
	{
		if (!collL && !collR)
		{
            /// THIS IS PAINFUL TO READ. BREAK IT UP AND USE GOOD VARIABLE NAMES TO MAKES IT MORE READABLE.
			if (posY < (Config::getFM()->getH() / 2) - getHitBoxY() && Config::getSM()->GetGame()->getMoveMap())
			{
				//moveMap
				Config::getSM()->GetGame()->MoveMap(0, moveSpeed);
			}
				//move Player Y
			else posY += moveSpeed;
		}
		//if hit reset velY
		else velY = 0;
	}
}


//***********************************************
//handle gravity  /// BREAK UP YOUR COMMENTS TOO INTO MULTI-LINE COMMENTS!
//This method doesn't work fine. If player is moving and there is a hole 32x32 somewhere on the map, he ignors this hole (does not fall down)
void Player::jumpPhysics()
{
	//collisionState
	bool collisionBot;
	if (jumpState == 1)
	{
		velY *= 0.986;
	}
	if (!Config::keySpace && velY < -6.5)   /// SO MANY MAGIC NUMBERS! 6.5, 7, 1.5
	{
		if (gravity > 7)
		{
			gravity /= 1.5;
		}
		setSpriteID(1);
		velY = -6.5;
	}
	if (!collisionBot)
	{
		velY += gravity
		jumpState = 2;
		setSpriteID(1);
	}
	else if(jumpState == 2)
	{
		resetJump();
	}
	UpdatePosY(velY/3.2);
}


//***********************************************
//So I wrote something like this. This works fine, but as you can see and as it is, this code is terrible.
void Player::PlayerPhysics()
{
	//collisionState
	bool collisionBot;
	if (jumpState == 1)
	{
		velY *= 0.986;
	}
	if (!Config::keySpace && velY < -6.5)
	{
		if (gravity > 7)
		{
			gravity /= 1.5;
		}
		setSpriteID(1);
		velY = -6.5;
	}
	if (!collisionBot)
	{
		gravity *= 1.04f;

		if (gravity > jumpSpeed)
		{
			gravity = jumpSpeed;
		}

		UpdatePosY((int)gravity);

		jumpState = 2;

		if (gravity >= 15)
		{
			setSpriteID(1);
		}
	}
	else if(jumpState == 2)
	{
		resetJump();
	}
	UpdatePosY(velY/3.2);
}