Search Unity

Finding the Best "IF" Tree for Multiple Inputs and Conditions

Discussion in 'Scripting' started by Esmond0, Jan 9, 2019.

  1. Esmond0

    Esmond0

    Joined:
    Nov 22, 2017
    Posts:
    7
    I am working on a first person shooter where the guns have a secondary fire option and a semiautomatic option, and I am trying to figure out the best way to shape my IF tree in update to make it easier to understand and edit later. Have any of you had this issue before?

    I feel like it's a lot of lines of code to check a very simple question.

    Here's the update function so far:

    Code (CSharp):
    1.     void Update()
    2.     {
    3.         //Secondary Set first
    4.         if (Input.GetButton("Fire2"))
    5.         {
    6.             secondary = true;
    7.         }
    8.         else
    9.         {
    10.             secondary = false;
    11.         }
    12.         //Reload set second
    13.         if (Input.GetButton("Reload") && ammoInClip < clipSize)
    14.         {
    15.             Reload(ammoType, clipSize);
    16.             return;
    17.         }
    18.  
    19.         if (secondary == false)
    20.         {
    21.             if (semiautomatic == true)
    22.             {
    23.                 if (Input.GetButton("Fire1") && reloading == false)
    24.                 {
    25.                     if (burstFire == true)
    26.                     {
    27.                         StartCoroutine(BurstFire());
    28.                     }
    29.                     else
    30.                     {
    31.                         Shoot();
    32.                     }
    33.                 }
    34.             }
    35.             else
    36.             {
    37.                 if (Input.GetButtonDown("Fire1") && reloading == false)
    38.                 {
    39.                     if (burstFire == true)
    40.                     {
    41.                         StartCoroutine(BurstFire());
    42.                     }
    43.                     else
    44.                     {
    45.                         Shoot();
    46.                     }
    47.                 }
    48.             }
    49.         }
    50.         else if (secondary == true)
    51.         {
    52.             if (semiautomatic2 == true)
    53.             {
    54.                 if (Input.GetButton("Fire1") && reloading == false)
    55.                 {
    56.                     if (burstFire2 == true)
    57.                     {
    58.                         StartCoroutine(BurstFire2());
    59.                     }
    60.                     else
    61.                     {
    62.                         Shoot2();
    63.                     }
    64.                 }
    65.             }
    66.             else
    67.             {
    68.                 if (Input.GetButtonDown("Fire1") && reloading == false)
    69.                 {
    70.                     if (burstFire2 == true)
    71.                     {
    72.                         StartCoroutine(BurstFire2());
    73.                     }
    74.                     else
    75.                     {
    76.                         Shoot2();
    77.                     }
    78.                 }
    79.             }
    80.         }
    81.     }
     
  2. GroZZleR

    GroZZleR

    Joined:
    Feb 1, 2015
    Posts:
    2,205
    It looks more like the problem is you have your logic inverted, where the higher precedent actions are deeper in the chain. If the gun is reloading, for example, you don't need to evaluate any shooting logic at all but you have it as one of the very last checks. The biggest red flag to spot this yourself is how much logic you're duplicating throughout.

    A simple restructure seems to be fine:
    Code (csharp):
    1.  
    2. // bail if we're reloading
    3. if(reloading == true)
    4.    return;
    5.  
    6. // is the player firing?
    7. bool firing = false;
    8.  
    9. if(secondary == false)
    10.    firing = (semiautomatic == true) ? Input.GetButtonDown("Fire1") : Input.GetButton("Fire1");
    11. else
    12.    firing = (semiautomatic2 == true) ? Input.GetButtonDown("Fire1") : Input.GetButton("Fire1");
    13.  
    14. if(firing == false)
    15.    return;
    16.  
    17. // good to go
    18. if(burstFire == true)
    19. {
    20.    if(secondary == false)
    21.        StartCoroutine(BurstFire());
    22.    else
    23.        StartCoroutine(BurstFire2());
    24. }
    25. else
    26. {
    27.    if(secondary == false)
    28.        Shoot();
    29.    else
    30.        Shoot2();
    31. }
    32.  
     
    Esmond0 likes this.
  3. Esmond0

    Esmond0

    Joined:
    Nov 22, 2017
    Posts:
    7

    That is so much help! Thank you!
     
  4. Pudgy138

    Pudgy138

    Joined:
    Sep 10, 2018
    Posts:
    77
    Is there a better more efficient way than using IF statements, even as refactored as this is? Is this as performant as it can be or is there a better way? I know that's a sort of absurd question, but I'm always looking for better ways to do things like this.
     
  5. Esmond0

    Esmond0

    Joined:
    Nov 22, 2017
    Posts:
    7
    I know I could set up a different script for each type of weapon (one for semiautomatic burst fire, one for automatic burst, one for single automatic, etc), but I was hoping to have one script to just edit in the inspector (or with another script) to set up several different weapons.

    I could use switch statements (I read about them last night in my C# book), and then use an enum to setup each type of weapon. I'm not sure which is more performant though.
     
  6. GroZZleR

    GroZZleR

    Joined:
    Feb 1, 2015
    Posts:
    2,205
    I wouldn't worry too much about a couple of if statements in regards to performance. A single square root, like Vector3.Distance, takes more time than a thousand simple boolean if statements.

    A better solution would be as Esmond0 hinted at, having each weapon contain its own logic rather than having a "master" weapon class that handles every single situation as it's more adaptable for any future design changes.

    I would also personally move away from so many booleans and use enums to describe the modes as it's semantically clearer.

    Code (csharp):
    1.  
    2. enum FiringMode { Single, Burst, Automatic }
    3. enum WeaponMode { Primary, Secondary }
    4.  
     
    Esmond0 likes this.
unityunity