Best way to make this code shorter?

Question

I'm working on dungeon generation and i ended up with a long code, I would like to shorten it if possible, but i'm a big noob, anyone got any ideas?

 if (roomScript.rCheck[0].room != null) {
             if (roomScript.connectionsAvailable[0] == true || roomScript.rCheck[0].connectionsAvailable[2] == true) {
                 roomScript.roomConnections[0].SetActive(false);
                 roomScript.connectionsEnabled[0] = true;
                 roomScript.connectionsConnected[0] = true;
                 roomScript.connectionsAvailable[0] = false;
 
                 roomScript.rCheck[0].roomScript.roomConnections[2].SetActive(false);
                 roomScript.rCheck[0].roomScript.connectionsEnabled[2] = true;
                 roomScript.rCheck[0].roomScript.connectionsConnected[2] = true;
                 roomScript.rCheck[0].roomScript.connectionsAvailable[2] = false;
             }
         }
 
         if (roomScript.rCheck[1].room != null) {
             if (roomScript.connectionsAvailable[1] == true || roomScript.rCheck[1].connectionsAvailable[3] == true) {
                 roomScript.roomConnections[1].SetActive(false);
                 roomScript.connectionsEnabled[1] = true;
                 roomScript.connectionsConnected[1] = true;
                 roomScript.connectionsAvailable[1] = false;
 
                 roomScript.rCheck[1].roomScript.roomConnections[3].SetActive(false);
                 roomScript.rCheck[1].roomScript.connectionsEnabled[3] = true;
                 roomScript.rCheck[1].roomScript.connectionsConnected[3] = true;
                 roomScript.rCheck[1].roomScript.connectionsAvailable[3] = false;
             }
         }
 
         if (roomScript.rCheck[2].room != null) {
             if (roomScript.connectionsAvailable[2] == true || roomScript.rCheck[2].connectionsAvailable[0] == true) {
                 roomScript.roomConnections[2].SetActive(false);
                 roomScript.connectionsEnabled[2] = true;
                 roomScript.connectionsConnected[2] = true;
                 roomScript.connectionsAvailable[2] = false;
 
                 roomScript.rCheck[2].roomScript.roomConnections[0].SetActive(false);
                 roomScript.rCheck[2].roomScript.connectionsEnabled[0] = true;
                 roomScript.rCheck[2].roomScript.connectionsConnected[0] = true;
                 roomScript.rCheck[2].roomScript.connectionsAvailable[0] = false;
             }
         }
 
         if (roomScript.rCheck[3].room != null) {
             if (roomScript.connectionsAvailable[3] == true || roomScript.rCheck[3].connectionsAvailable[1] == true) {
                 roomScript.roomConnections[3].SetActive(false);
                 roomScript.connectionsEnabled[3] = true;
                 roomScript.connectionsConnected[3] = true;
                 roomScript.connectionsAvailable[3] = false;
 
                 roomScript.rCheck[3].roomScript.roomConnections[1].SetActive(false);
                 roomScript.rCheck[3].roomScript.connectionsEnabled[1] = true;
                 roomScript.rCheck[3].roomScript.connectionsConnected[1] = true;
                 roomScript.rCheck[3].roomScript.connectionsAvailable[1] = false;
             }
         }

Answer 1

Answer by noamro · Dec 08, 2019 at 07:24 PM

Use a for loop :) instead of using a number use an integer

 for (int i = 0; i < roomScript.connectionsAvailable[].length; i++) 
 {
     roomScript.roomConnections[i].SetActive(false);
 }

Add comment · Show 1 · Share

Cornelis-de-Jager · Dec 08, 2019 at 11:17 PM 1

Share

Or even

 foreach (var rs in roomScript.connectionsAvailable){
      rs.SetActive(false);
 }

Answer 2

Answer by Bunny83 · Dec 08, 2019 at 11:34 PM

Specifically a for loop like this:

 for (int i = 0; i < 4; i++)
 {
     int i2 = (2+i) % 4;
     var rCheck = roomScript.rCheck[i];
     if (rCheck.room != null)
     {
         if (roomScript.connectionsAvailable[i] || rCheck.connectionsAvailable[i2])
         {
             roomScript.roomConnections[i].SetActive(false);
             roomScript.connectionsEnabled[i] = true;
             roomScript.connectionsConnected[i] = true;
             roomScript.connectionsAvailable[0] = false;
             
             rCheck.roomScript.roomConnections[i2].SetActive(false);
             rCheck.roomScript.connectionsEnabled[i2] = true;
             rCheck.roomScript.connectionsConnected[i2] = true;
             rCheck.roomScript.connectionsAvailable[i2] = false;
         }
     }
 }

This does the same as your current code.

Add comment · Show 2 · Share

Bunny83 · Dec 08, 2019 at 11:38 PM 0

Share

Note that "i2" is always 2 ahead of "i" but wraps around back to 0 when it reaches 4

 //i  | i2
 //-------
 //0  |  2
 //1  |  3
 //2  |  0
 //3  |  1

Since roomScript.rCheck[i] was used several times in your code I stored it in the local variable rCheck to simplify the lines where it was used.

Bunny83 · Dec 08, 2019 at 11:44 PM 2

Share

ps: If you really want to simplify your code you should use less individual arrays which logically belong together. If you have data that belongs together you might want to create a pure data class and group data together that belongs together. Your "roomConnection" seems to be an array of GameObjects. The other arrays (connectionsEnabled, connectionsConnected, connectionsAvailable) seems to belong to each connection so those 4 values should be grouped and then you would only have a single array and one instance of your data class for each connection. However to me most of these values (just based on the names and how you use them here) seems redundant anyways. Though we can't really suggest other approaches without knowing the exact usage and dependencies.

Best way to make this code shorter?

2 Replies

Your answer

Welcome to Unity Answers

Follow this Question

Related Questions