Code works, but could this be cleaner?

Unity Engine
,
1

Hi there,

I got my code to work. It basically is for a 2D topdown ship game where wind changes speed. When you are in the wind you go faster, when you are against the wind you go slower, and when you are neither with or against the wind you go a regular speed.

I’m happy with how it runs, but it is a crap ton of if statements and I am still fairly new to game development. If anyone had a suggestion for how I could improve this code in the future, that would be awesome. Also if it looks great the way it is, I’d appreciate knowing that as well.

Here it is:

void FixedUpdate()
    {
        RaycastHit2D hit = Physics2D.Raycast(transform.position, -transform.up);

        //WIND SPEED CHANGE
        if (hit.collider.name == "North")
        {       
            if (windDirection.currentWind == "N") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
            {
                accelerationPower = 15f;
            }
            else if (windDirection.currentWind == "S") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
            {
                accelerationPower = 5f;
            }
            else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
            {
                accelerationPower = 10f;
            }

        }
        else if (hit.collider.name == "South")
        {
            if (windDirection.currentWind == "S") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
            {
                accelerationPower = 15f;
            }
            else if (windDirection.currentWind == "N") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
            {
                accelerationPower = 5f;
            }
            else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
            {
                accelerationPower = 10f;
            }
        }
        else if (hit.collider.name == "East")
        {
            if (windDirection.currentWind == "E") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
            {
                accelerationPower = 15f;
            }
            else if (windDirection.currentWind == "W") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
            {
                accelerationPower = 5f;
            }
            else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
            {
                accelerationPower = 10f;
            }
        }
        else if (hit.collider.name == "West")
        {
            if (windDirection.currentWind == "W") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
            {
                accelerationPower = 15f;
            }
            else if (windDirection.currentWind == "E") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
            {
                accelerationPower = 5f;
            }
            else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
            {
                accelerationPower = 10f;
            }
        }

}
1 Like
2

There’s probably some polymorphic way to make this even simpler, but you could get rid of some of that duplicated code with something like this:

void FixedUpdate()
{
    switch (hit.collider.name)
    {
    case "North":
        accelerationPower = AccelerationPower(windDirection.currentWind, "N", "S");
        break;
      
    case "South":
        accelerationPower = AccelerationPower(windDirection.currentWind, "S", "N");
        break;
      
    case "East":
        accelerationPower = AccelerationPower(windDirection.currentWind, "E", "W");
        break;
      
    case "West":
        accelerationPower = AccelerationPower(windDirection.currentWind, "W", "E");
        break;
  
    default:
        throw new Exception("Unexpected collider name.");
    }
}

float AccelerationPower(string currentWind, string withWind, string againstWind)
{
    if (currentWind == withWind)
    {
        return 15;
    }
  
    if (currentWind == againstWind)
    {
        return 5;
    }
  
    return 10;
}

I would also be inclined to replace all the single-letter strings that represent wind directions with an enum, too.

2 Likes
3

If you change the wind directions to an int for windDirection.currentWind (north = 0, south = 1, east = 2, west =3), you can do something like this (untested):

private float[] accPowN = { 15f, 5f, 10f, 10f };
private float[] accPowS = { 5f, 15f, 10f, 10f };
private float[] accPowE = { 10f, 10f, 15f, 5f };
private float[] accPowW = { 10f, 10f, 5f, 15f };

void FixedUpdate()
{
   RaycastHit2D hit = Physics2D.Raycast(transform.position, -transform.up);

   if (hit.collider.name == "North")
      accelerationPower = accPowN[windDirection.currentWind];

   else if (hit.collider.name == "South")
      accelerationPower = accPowS[windDirection.currentWind];

   else if (hit.collider.name == "East")
      accelerationPower = accPowE[windDirection.currentWind];

   else if (hit.collider.name == "West")
      accelerationPower = accPowW[windDirection.currentWind];
}
4

Stevenmiller I really like switch statements and I used for the script that changes the wind randomly. I should have thought more about using them for this script too. I understand your script very easily and it looks much better.

Thorham3011 That is super interesting and I never thought of that! I like it a lot and it incredibly shorter than my version.

5

What are the north/east/south/west colliders you are hitting? A part of your ship, or?

You probably don’t need the raycast, you just need the direction of the wind and the direction of your ship, and then you can calculate the angle between those and get the speed from that, but I’m not quite clear on what those colliders are.

6

What I did was I created 4 children of the virtual camera, and placed them as N, E, S, W. I did it this way, so that it would follow the player, but not rotate with the player. I probably could have put them on the map, but I was toying with having the inbetween compass direction likes NE and NW, and figured it would be best this way.

I did not look up the best way to do it, and I figured if my way was not the most efficient, it would still help me learn a lot more about game development. You are probably right that your way is better

7

If your ship’s facing is decided by it’s transform’s direction, you could encode the wind as a Vector2 (the direction of the wind), and then do this:

// assume you already have Vector2 windDirection
Vector2 shipFacing = ship.transform.forward;

float angle = Vector2.Angle(shipFacing, windDirection);

float angle_t = Mathf.InverseLerp(0f, 180f, angle);

accelerationPower = Mathf.Lerp(15f, 5f, angle_t);

Lerp(a, b, t) gives you a if t is <= 0, and b if t >= 1, and a number between those if t is between them. If you’re more maths inclined, Lerp(a, b, t) is equal to a + (b - a) * t.

InverseLerp(a, b, v) is the inverse - it gives you the value t such that Lerp(a, b, t) == v.

So what happens in my code above is that angle_t is 0 if the angle between the ship and the wind is 0, and 1 if the angle is 180, and then it moves linearly between those two values. That t-value is fed into Lerp(15f, 5f, t), so you get an acceleration of 15 when the angle is 0, and an acceleration of 5 is the angle is 180.

This means that you get a smoother transition than what you currently have, that may or may not be what you actually want.

1 Like
8

Hmmm… seems as if AccelerationPower could figure out the opposite direction itself. You could maybe get rid of the switch with a single call like AccelerationPower(hit.collider.name);. Not helpful for this, but if other places also need wind force it would probably save time and trouble