Code works, but could this be cleaner?

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:

Code (CSharp):

1. void FixedUpdate()
2.     {
3.         RaycastHit2D hit = Physics2D.Raycast(transform.position, -transform.up);
4.  
5.         //WIND SPEED CHANGE
6.         if (hit.collider.name == "North")
7.         {      
8.             if (windDirection.currentWind == "N") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
9.             {
10.                 accelerationPower = 15f;
11.             }
12.             else if (windDirection.currentWind == "S") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
13.             {
14.                 accelerationPower = 5f;
15.             }
16.             else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
17.             {
18.                 accelerationPower = 10f;
19.             }
20.  
21.         }
22.         else if (hit.collider.name == "South")
23.         {
24.             if (windDirection.currentWind == "S") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
25.             {
26.                 accelerationPower = 15f;
27.             }
28.             else if (windDirection.currentWind == "N") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
29.             {
30.                 accelerationPower = 5f;
31.             }
32.             else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
33.             {
34.                 accelerationPower = 10f;
35.             }
36.         }
37.         else if (hit.collider.name == "East")
38.         {
39.             if (windDirection.currentWind == "E") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
40.             {
41.                 accelerationPower = 15f;
42.             }
43.             else if (windDirection.currentWind == "W") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
44.             {
45.                 accelerationPower = 5f;
46.             }
47.             else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
48.             {
49.                 accelerationPower = 10f;
50.             }
51.         }
52.         else if (hit.collider.name == "West")
53.         {
54.             if (windDirection.currentWind == "W") //WHEN MOVING WITH WIND, SPEED BECOMES 15F
55.             {
56.                 accelerationPower = 15f;
57.             }
58.             else if (windDirection.currentWind == "E") //WHEN MOVING AGAINST WIND, SPEED BECOMES 5F
59.             {
60.                 accelerationPower = 5f;
61.             }
62.             else //WHEN NEITHER WITH OR AGAINST WIND, SPEED IS 10F
63.             {
64.                 accelerationPower = 10f;
65.             }
66.         }
67.  
68. }

 

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:

Code (CSharp):

1. void FixedUpdate()
2. {
3.     switch (hit.collider.name)
4.     {
5.     case "North":
6.         accelerationPower = AccelerationPower(windDirection.currentWind, "N", "S");
7.         break;
8.      
9.     case "South":
10.         accelerationPower = AccelerationPower(windDirection.currentWind, "S", "N");
11.         break;
12.      
13.     case "East":
14.         accelerationPower = AccelerationPower(windDirection.currentWind, "E", "W");
15.         break;
16.      
17.     case "West":
18.         accelerationPower = AccelerationPower(windDirection.currentWind, "W", "E");
19.         break;
20.  
21.     default:
22.         throw new Exception("Unexpected collider name.");
23.     }
24. }
25.  
26. float AccelerationPower(string currentWind, string withWind, string againstWind)
27. {
28.     if (currentWind == withWind)
29.     {
30.         return 15;
31.     }
32.  
33.     if (currentWind == againstWind)
34.     {
35.         return 5;
36.     }
37.  
38.     return 10;
39. }
40.  

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

enum

, too.

 

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):

Code (csharp):

1. private float[] accPowN = { 15f, 5f, 10f, 10f };
2. private float[] accPowS = { 5f, 15f, 10f, 10f };
3. private float[] accPowE = { 10f, 10f, 15f, 5f };
4. private float[] accPowW = { 10f, 10f, 5f, 15f };
5.  
6. void FixedUpdate()
7. {
8.    RaycastHit2D hit = Physics2D.Raycast(transform.position, -transform.up);
9.  
10.    if (hit.collider.name == "North")
11.       accelerationPower = accPowN[windDirection.currentWind];
12.  
13.    else if (hit.collider.name == "South")
14.       accelerationPower = accPowS[windDirection.currentWind];
15.  
16.    else if (hit.collider.name == "East")
17.       accelerationPower = accPowE[windDirection.currentWind];
18.  
19.    else if (hit.collider.name == "West")
20.       accelerationPower = accPowW[windDirection.currentWind];
21. }

 

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.

 

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.

 

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

 

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:

Code (csharp):

1.  
2. // assume you already have Vector2 windDirection
3. Vector2 shipFacing = ship.transform.forward;
4.  
5. float angle = Vector2.Angle(shipFacing, windDirection);
6.  
7. float angle_t = Mathf.InverseLerp(0f, 180f, angle);
8.  
9. accelerationPower = Mathf.Lerp(15f, 5f, angle_t);
10.  

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.

 

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