How to compress if-'input.key'-statements?

[Haugen]

I am having ‘Ifception’ issues

I am writing my own movement-script for a ridgidbody, and it’s getting to be a mess, with so many if-statements, inside if-statements, inside if-statements, etc. This also results in have motherload of redundant code.

The Code, (simplified):

// Is a key being pressed or held down
if(Input.anyKey || Input.GetKeyDown(KeyCode.LeftShift) || Input.GetKeyDown(KeyCode.Space)) //is a key being pressed?
{
	// Is the D-key being pressed and are we moving less than maximum allowed speed?
	if(Input.GetKey(KeyCode.D) && rigidbody.velocity.x < MaxForwardSpeed)
	{
		// Are we touching a surface beneath us and are we forbidden to move while in mid-air?
		if (!AllowMovementInAir && IsTouchingGround)
		{
			// Add positive movement to the X-axis
			rigidbody.velocity = rigidbody.velocity + new Vector3(Acceleration, 0,0);
		}
	}
}

There are Multiple If-s at each level after the first. I’d love to get it all neat and ordered to improve readability and not having to have the line:

rigidbody.velocity = rigidbody.velocity + new Vector3(Acceleration, 0,0);

written out 14 times in different if-statements with small variations

thanks for any and all input!

-frank

[KellyThomas]

One common strategy is to refactor the conditions into functions. With appropriate naming this can make intent much more clear.

note that with the following I’m just guessing at your code’s intent so the this example will not mirror your functionality, but is an example of an alternative style.

bool isControlPressed() {
    return Input.anyKey;
}
 
bool isMovementAllowed() {
    return !AllowMovementInAir && IsTouchingGround;
}

bool isPlayerAtLessThanMaxHorizontalSpeed() {
    return Mathf.Abs(rigidbody.velocity.x) < MaxHorizontalSpeed;
}

bool isRightMovementKeyPressed() {
    return Input.GetKey(KeyCode.D);
}

bool isLeftMovementKeyPressed() {
    return Input.GetKey(KeyCode.A);
}

void ApplyAcceleration(Vector3 accelerationVector) {
    rigidbody.velocity = rigidbody.velocity + accelerationVector;
}
 
//then in your FixedUpdate():
 
if (isControlPressed() && isMovementAllowed()) {
    if (isPlayerAtLessThanMaxHorizontalSpeed()) {
        if(isRightMovementKeyPressed()) {
            ApplyAcceleration(new Vector3(Acceleration, 0.0f, 0.0f));
        }
        if (isLeftMovementKeyPressed()) {
            ApplyAcceleration(new Vector3(-Acceleration, 0.0f, 0.0f));
        }
    }
}

One other tip I will offer is to specify intent with your comments, any programmer can read what the code does but we have trouble guessing why. e.g. “!AllowMovementInAir && IsTouchingGround”, why is the player only able to move if they are both on the ground and not allowed to move in the air? It looks like it is meant to be “IsTouchingGround || AllowMovementInAir” i.e. are they either on the ground or allowed to move in air. It’s very hard for us to know when the comments describe code rather than intent.

[tanoshimi]

Agree with @KiraSensei - what’s the purpose of the first line (testing for any keypress), if all the subsequent statements test for specific keypresses anyway?

You could combine (!AllowMovementInAir && IsTouchingGround) into a single Boolean outside the if statements, like this: Bool canMove = (!AllowMovementInAir && IsTouchingGround);

Then, you can reduce each test to a single line as follows:

if(Input.GetKey(KeyCode.D) && rigidbody.velocity.x < MaxForwardSpeed && canMove) {...}
...