Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Bug Why is this enemy script killing my processing?

Discussion in 'Scripting' started by jleven22, Mar 27, 2023.

  1. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    I have a script which I've spent a long time on figuring out the logic for, and now that it's at this point it's destroying my processing.

    Basically this is an enemy that finds other nearby enemies and heals them. So it's looking for enemies within a certain distance and interacting with them.

    Can anyone help me figure out what the issue is? I'm guessing it's in my Update call, just not sure why.

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class EnemySeaBishop : MonoBehaviour
    6. {
    7.     // Start is called before the first frame update
    8.  
    9.     public Transform target1, target2, target3, target0;
    10.     public Collider2D hitCollider;
    11.     public Animator anim;
    12.     public float distanceToAddEnemy, distanceToHeal, waitTimeToHeal;
    13.     public int amountToHeal;
    14.     private float waitCountdown, timeToDisappear;
    15.     private int positionToPick;
    16.     public GameObject enemyHealFX;
    17.     private bool isTransitioning;
    18.  
    19.     public List<GameObject> enemies = new List<GameObject>();
    20.  
    21.  
    22.     void Start()
    23.     {
    24.  
    25.         transform.position = target0.transform.position;
    26.  
    27.         foreach (GameObject enemy in GameObject.FindGameObjectsWithTag("Enemy"))
    28.         {
    29.  
    30.             float distanceRange = Vector2.Distance(enemy.transform.position, transform.position);
    31.  
    32.             if (distanceRange < distanceToAddEnemy)
    33.             {
    34.  
    35.                 if (enemy.Equals(this.gameObject))
    36.  
    37.                     continue;
    38.  
    39.                 enemies.Add(enemy);
    40.             }
    41.         }
    42.     }
    43.  
    44.     // Update is called once per frame
    45.     void Update()
    46.     {
    47.         if (!isTransitioning)
    48.         {
    49.  
    50.  
    51.             foreach (GameObject target in enemies)
    52.             {
    53.                 float distanceToEnemy = Vector2.Distance(target.transform.position, transform.position);
    54.  
    55.                 if (waitCountdown > 0)
    56.                 {
    57.                     anim.SetTrigger("Idle");
    58.  
    59.                     waitCountdown -= Time.deltaTime;
    60.                 }
    61.  
    62.  
    63.                 if (distanceToEnemy < distanceToHeal)
    64.                 {
    65.  
    66.                     if (waitCountdown <= 0f)
    67.                     {
    68.  
    69.                         if (target.GetComponent<EnemyController>().health < target.GetComponent<EnemyController>().maxHealth)
    70.                         {
    71.  
    72.                             anim.SetTrigger("Heal");
    73.  
    74.                             int newHealth = target.GetComponent<EnemyController>().health + amountToHeal;
    75.  
    76.                             GameObject healIcon = Instantiate(enemyHealFX, target.GetComponent<EnemyController>().transform.position, Quaternion.Euler(0, 0, 0));
    77.                             healIcon.transform.position = new Vector3(target.GetComponent<EnemyController>().transform.position.x, target.GetComponent<EnemyController>().transform.position.y + 1, 0f);
    78.                             healIcon.transform.SetParent(target.transform);
    79.  
    80.  
    81.                             GameObject selfHealIcon = Instantiate(enemyHealFX, transform.position, Quaternion.Euler(0, 0, 0));
    82.                             selfHealIcon.transform.position = new Vector3(transform.position.x, transform.position.y + 1, 0f);
    83.  
    84.  
    85.  
    86.                             target.GetComponent<EnemyController>().health = newHealth;
    87.  
    88.                         }
    89.  
    90.                         waitCountdown = waitTimeToHeal;
    91.  
    92.                     }
    93.  
    94.                 }
    95.             }
    96.  
    97.             if (timeToDisappear > 0)
    98.             {
    99.                 timeToDisappear -= Time.deltaTime;
    100.             }
    101.  
    102.             if (timeToDisappear <= 0 && waitCountdown <= 0)
    103.             {
    104.                 StartCoroutine(DisappearAppear());
    105.             }
    106.         }
    107.     }
    108.  
    109.  
    110.     void SetRandomTime()
    111.     {
    112.         timeToDisappear = Random.Range(6, 12);
    113.     }
    114.  
    115.     private IEnumerator DisappearAppear()
    116.     {
    117.         //Disappear
    118.         isTransitioning = true;
    119.         anim.SetTrigger("Disappear");
    120.         hitCollider.enabled = false;
    121.         yield return new WaitForSeconds(8f);
    122.  
    123.         //Change Position
    124.         positionToPick = Random.Range(0, 3);
    125.  
    126.         if (positionToPick == 0)
    127.         {
    128.             transform.position = target0.transform.position;
    129.         }
    130.         if (positionToPick == 1)
    131.         {
    132.             transform.position = target1.transform.position;
    133.         }
    134.         if (positionToPick == 2)
    135.         {
    136.             transform.position = target2.transform.position;
    137.         }
    138.         if (positionToPick == 3)
    139.         {
    140.             transform.position = target3.transform.position;
    141.         }
    142.  
    143.         //Appear
    144.         anim.SetTrigger("Appear");
    145.         yield return new WaitForSeconds(1f);
    146.         hitCollider.enabled = true;
    147.         isTransitioning = false;
    148.         SetRandomTime();
    149.     }
    150.  
    151.  
    152.     private void OnTriggerEnter2D(Collider2D other)
    153.     {
    154.         if (other.tag == "Player Bullet")
    155.         {
    156.             StartCoroutine(DisappearAppear());
    157.             timeToDisappear = 0;
    158.             waitCountdown = 0;
    159.         }
    160.     }
    161. }
    162.  
    163.  
     
  2. Brathnann

    Brathnann

    Joined:
    Aug 12, 2014
    Posts:
    7,186
    Have you run the profiler? Otherwise, remember that loops that aren't set to yield will fully run through. Not sure how much your loop takes to process, but this is why the profiler is important.
     
  3. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,520
    What on does this mean?

    Does the program exit / crash?

    Does Unity lock up?

    Does it go too slow? Drop frames?

    Do you get bugs? What bugs?

    How to report your problem productively in the Unity3D forums:

    http://plbm.com/?p=220

    This is the bare minimum of information to report:

    - what you want
    - what you tried
    - what you expected to happen
    - what actually happened, especially any errors you see
    - links to documentation you used to cross-check your work (CRITICAL!!!)

    You may edit your post above.
     
  4. John_Leorid

    John_Leorid

    Joined:
    Nov 5, 2012
    Posts:
    646
    I've refactored the code in a way I would write it, optimized a few loops on the way. I can't really see anything that would have a big impact on performance here.

    The only reason why this could be problematic is when "waitTimeToHeal" is set to "0" (or a small value, something below "0.2" maybe) in the inspector. Then it would execute every frame, creating lots of GameObjects (which I guess destroy themselfes after some time).

    Everything else seems fine to me. I've added a debug log at the start that tells you how many enemies are found, but I doubt that the number is high enough to be considered a performance problem.

    Code (CSharp):
    1. using System.Collections;
    2. using System.Collections.Generic;
    3. using UnityEngine;
    4.  
    5. public class EnemySeaBishop : MonoBehaviour
    6. {
    7.     public Transform target1, target2, target3, target0;
    8.     public Collider2D hitCollider;
    9.     public Animator anim;
    10.     public float distanceToAddEnemy, distanceToHeal, waitTimeToHeal;
    11.     public int amountToHeal;
    12.     private float waitCountdown, timeToDisappear;
    13.     private int positionToPick;
    14.     public GameObject enemyHealFX;
    15.     private bool isTransitioning;
    16.  
    17.     [SerializeField] List<GameObject> _enemies = new(); // why was this public? can use "new()"
    18.     private List<Transform> _targetPositions;
    19.  
    20.     void Start()
    21.     {
    22.         transform.position = target0.transform.position;
    23.  
    24.         _targetPositions = new List<Transform>()
    25.         {
    26.             target0,
    27.             target1,
    28.             target2,
    29.             target3,
    30.         };
    31.  
    32.         foreach (GameObject enemy in GameObject.FindGameObjectsWithTag("Enemy"))
    33.         {
    34.             float distanceRange = Vector2.Distance(enemy.transform.position, transform.position);
    35.  
    36.             // ignore out of range enemies
    37.             if (distanceRange >= distanceToAddEnemy) continue;
    38.             // ignore self
    39.             if (enemy == this.gameObject) continue;
    40.  
    41.             // add all valid enemies to enemies list
    42.             _enemies.Add(enemy);
    43.         }
    44.         // just to see how many of those were actually added
    45.         Debug.Log($"{_enemies.Count} Enemies added to healing list", gameObject);
    46.     }
    47.  
    48.     void Update()
    49.     {
    50.         HandleHealing();
    51.  
    52.         HandleTeleporting();
    53.     }
    54.     void HandleHealing()
    55.     {
    56.         // no healing during transition
    57.         if (isTransitioning) return;
    58.  
    59.         if (waitCountdown > 0)
    60.         {
    61.             anim.SetTrigger("Idle");
    62.  
    63.             waitCountdown -= Time.deltaTime;
    64.  
    65.             // we still have to wait before we can heal
    66.             return;
    67.         }
    68.  
    69.         foreach (GameObject target in _enemies)
    70.         {
    71.             float distanceToEnemy = Vector2.Distance(target.transform.position, transform.position);
    72.             // cache EnemyController because we need it often. That's easier to read and better for performance
    73.             EnemyController targetController = target.GetComponent<EnemyController>();
    74.  
    75.             // ignore enemy as he is too far away
    76.             if (distanceToEnemy >= distanceToHeal) continue;
    77.  
    78.             // ignore enemy as he has full health
    79.             if (targetController.health >= targetController.maxHealth) continue;
    80.  
    81.             anim.SetTrigger("Heal");
    82.  
    83.             // spawn heal icons over enemy and self
    84.             Vector3 healIconPos = targetController.transform.position;
    85.             healIconPos.y += 1;
    86.             healIconPos.z = 0;
    87.             Quaternion healIconRot = Quaternion.identity; // same as Quaternion.Euler(0,0,0)
    88.  
    89.             GameObject healIcon = Instantiate(enemyHealFX, healIconPos, healIconRot);
    90.             healIcon.transform.SetParent(target.transform);
    91.  
    92.             Vector3 selfHealIconPos = transform.position;
    93.             selfHealIconPos.y += 1;
    94.             selfHealIconPos.z = 0;
    95.             Quaternion selfHealIconRot = Quaternion.identity;
    96.  
    97.             GameObject selfHealIcon = Instantiate(enemyHealFX, selfHealIconPos, selfHealIconRot);
    98.  
    99.             int newHealth = targetController.health + amountToHeal;
    100.             targetController.health = newHealth;
    101.  
    102.             // heal one enemy, then exit the loop and wait to
    103.             // heal the next one (this was, what the original code was doing)
    104.             waitCountdown = waitTimeToHeal;
    105.             break;
    106.         }
    107.     }
    108.  
    109.     void HandleTeleporting()
    110.     {
    111.         // no teleporting during a teleport
    112.         if (isTransitioning) return;
    113.  
    114.         if (timeToDisappear > 0)
    115.         {
    116.             timeToDisappear -= Time.deltaTime;
    117.         }
    118.  
    119.         if (timeToDisappear <= 0 && waitCountdown <= 0)
    120.         {
    121.             StartCoroutine(DisappearAppear());
    122.             // I would set the random time here because it's easier to understand and
    123.             // because changes in the coroutine are now less problematic
    124.             timeToDisappear = Random.Range(6, 12);
    125.         }
    126.     }
    127.  
    128.     private IEnumerator DisappearAppear()
    129.     {
    130.         //Disappear
    131.         isTransitioning = true;
    132.         anim.SetTrigger("Disappear");
    133.         hitCollider.enabled = false;
    134.         yield return new WaitForSeconds(8f);
    135.  
    136.         //Change Position
    137.         // Note: when using integer Random.Range, the max is exclusive, so
    138.         //         using "(0, 3)" here, will only return numbers 0, 1, 2
    139.         //         -> therefore we use "(0, 4)" instead
    140.         positionToPick = Random.Range(0, 4);
    141.         transform.position = _targetPositions[positionToPick].position;
    142.  
    143.         //Appear
    144.         anim.SetTrigger("Appear");
    145.         yield return new WaitForSeconds(1f);
    146.         hitCollider.enabled = true;
    147.         isTransitioning = false;
    148.     }
    149.  
    150.  
    151.     private void OnTriggerEnter2D(Collider2D other)
    152.     {
    153.         if (other.tag == "Player Bullet")
    154.         {
    155.             hitCollider.enabled = false;
    156.             timeToDisappear = 0;
    157.             waitCountdown = 0;
    158.         }
    159.     }
    160. }
     
    Last edited: Mar 28, 2023
  5. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    960
    At a quick glance I can see quite a few simple optimizations that could be made but honestly unless you are running this script a lot (as in thousands of times every frame) then I don't think they'd likely add up to much. As Kurt and brathnann said, you should run the profiler first and then if you still can't figure out what is wrong, at least provide us with some more information about what is going wrong and what the profiler says so that we can help narrow it down for you.
     
    jleven22 likes this.
  6. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    This is awesome! I'm going to give it a shot and see if it helps.

    In the meantime I'll try and get a screenshot up from my profiler to show what I'm up against.
     
  7. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    Thanks Kurt, I apologize for not providing more detail. I think I was just too frustrated to provide enough explanation at the time...

    I'll try and get a screenshot of the profiler and some updated info today.
     
  8. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    After giving this a shot, I came up with these two errors on the "new()"

    Assets\EnemySeaBishopAlt.cs(19,54): error CS8124: Tuple must contain at least two elements.

    Assets\EnemySeaBishopAlt.cs(19,55): error CS1526: A new expression requires (), [], or {} after type


    I'll be honest, I'm a bit out of my depth with what you wrote up here (that's why my original is so rudimentary, aka I am dumb).

    Btw I'm posting a screenshot of the profiler below.
     
  9. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,992
    HannibalL pretty much did 2 simple things. If you don't understand that code you can't probably understand the ideas:

    1) if you can't heal yet because of the delay, skip the healing-loop (your code checks every enemy even when it's not ready).
    2) once you heal someone, quit the loop (your code looks at everyone else even though it can't heal them).

    Your code doesn't seem to have some major time-killing problem, and worrying about small speed problems here and there -- esp. when you're not done -- is silly. But making those changes makes the code easier to read (I think). For example, I thought it healed everyone in range and had to trace through to see that it didn't.

    It could be waitTimeToHeal=0, as HannibelL mentioned. The problem would be even worse -- it would blast out a heal prefab on every nearby enemy every frame (if they were also somehow all damaged). But you'd clearly see that and be able to pause and check that every enemy had 20 heal-prefabs stacked on it.

    But there's one more common problem: often it's not where you think it is. You may have written this, gotten the slowdown, and assumed it's the problem. But before that you made a few small changes and didn't test them, and those are the real problem.
     
    jleven22 likes this.
  10. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421



    Here's what I'm looking at. After some research, I'm not sure I understand what "LogStringToConsole" is but it appears to be the culprit of some frame rate loss.
     
  11. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    This is a good thought. I'm going to check into these things.
    You can see the ms lost in the screenshot in my last post.
     
  12. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    960
    So first off, I would drill down into the EnemyActions.Update() and find the exact function call that is giving you the issue. To get that kind of detail you'll want to enable deep profiling. I'd suggest enabling this feature and then leave the profiler turned off until you reach the relevant portion of your game or code and then turn it on. Deep profiling absolutely tanks performance so doing it this way will help speed up the process of getting to where you actually want to profile.

    All of that being said, it sounds like your culprit is Debug.Log or something similar. It is an extremely super-duper slow function that should pretty much only be used for testing or for occasional confirmation of events and definitely shouldn't be something you spam multiple times per frame. Normally this will get compiled out for a final build, but you know... why hurt yourself in the dev build if you don't have to?

    The big thing to look at there is the amount of garbage being generated. 140KB per frame is absolutely massive! Generally with Unity anything over 0 is a bad idea but sometimes you just can't help it. That is likely due to what I mentioned above, the use of Debug.Log or something similar which generates garbage due to its use of string manipulation. Even worse, it actually is writing that stuff to a log file which incurs the cost of physical Input/Output on your drive.

    EDIT: Just took a closer look at it and notice the debug log ABOVE the highlighted update method. Yeah, definitely you need to get rid of that for exactly the reasons I mentioned. While you're at it though, still drill into the hightlighted method and see what is going on in there. Usually if you HAVE to generate gabrage it shouldn't be more than a few dozen to a few hundred bytes per frame. 140K, 101K and even 1.1K are way too much. Keep in mind that some of this might actually be due to the use of GetComponent() which actually only happens in the dev build and not in final builds so if you see garbage from that then it's okay to ignore it.
     
    Last edited: Mar 28, 2023
  13. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    I'll be honest, I'm so close to running a final build test that it might be worth it to review performance after building it out.
    At this stage I'm just doing bug fixes and optimization, so this might be a good next step for me just to see how things are affected. Thoughts?
     
    Sluggy likes this.
  14. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    960
    Well, generally things will always run better in release mode. One thing to keep in mind is that profiling in debug mode is mostly only useful for the most egregious of performance issues (like what you seem to have here). For more general-purpose optimizations or spot optimizations you should always profile on release builds as it will give far far more accurate information.
     
    jleven22 likes this.
  15. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    Ok this is good advice.

    This will be my second update on this game in two years, so the optimization and testing isn't something I'm super familiar with. I really appreciate the tip.
     
  16. John_Leorid

    John_Leorid

    Joined:
    Nov 5, 2012
    Posts:
    646
    Seems like it's using an old version of C#, which Unity-Version are you using?
    "A new expression requires (), [], or {} after type" -> this isn't required anymore since Unity v2021.1 I think?
    But yea, I think it's the line where I wrote
    [SerializeField] List<GameObject> _enemies = new();

    instead of
    [SerializeField] List<GameObject> _enemies = new List<GameObject>();

    Changing it should solve the compile errors.

    And about your Profiler Screenshot - a quick google search told me that it might be something writing to the editor log that doesn't show up in the Console-Window. You can view the Editor-Log by clicking on the three dots in your Console-Window, then on "Open Editor Log" - scrolling down should reveal the messages that are spammed here.

    Could be a lot of things but maybe, maybe you have turned off "Warnings" (yellow icon in console window) and this line:
    anim.SetTrigger("Idle");

    spamms something like "Parameter (Trigger) not set in Animator" for each enemy every frame to the console as a Warning. 10ms still seems like a lot to me but who knows, maybe that code is unoptimized, as it's basically just a bug you should fix (IF it's this line and not another one).

    Making many assumptions here - but definitely check your console & the editor log file. And if that doesn't help, you can turn on "Deep Profiling" in your Profiler to see the line that's causing the issue. (you also have to turn on stack trace in the profiler to really track it down I think)

    Let me know if that helped. ^^
     
    Last edited: Apr 20, 2023
  17. Skiriki

    Skiriki

    Joined:
    Aug 30, 2013
    Posts:
    68
    Unity's Debug.Log isn't compiled out by default in release builds. You'd have to wrap it in your own [Conditional("DEBUG")] attributed methode for that (ideally in a DLL so you retain the "click to jump to call site" functionality of debug logs.
     
    Sluggy likes this.
  18. Sluggy

    Sluggy

    Joined:
    Nov 27, 2012
    Posts:
    960
    Ah-ha, good call. Been so long since I used the vanilla version that I'd forgotten!
     
  19. jleven22

    jleven22

    Joined:
    Mar 26, 2019
    Posts:
    421
    The deep profiling is a real help!

    So I think I fixed this immediate issue. Seems like using SetTrigger() for the animator in update was becoming a problem, so I just whipped up this:

    Code (CSharp):
    1.                     if (!anim.GetCurrentAnimatorStateInfo(0).IsName("Idle"))
    2.                     {
    3.                         anim.SetTrigger("Idle");
    4.                     }
    I don't know why it's such a pull. Now, digging deeper into the rest of performance, it seems like anything involving the animator is a major contributor to the ms count, which seems confusing considering it's an expected function of Unity. I'm not doing anything too intensive. But for instance something as simple as SetFloat() in update for an animator appears to be pulling harder than I want it to.

    That said, there are hundreds of enemies using the same parameters in update at once. My point is they're just not doing anything too crazy to push up to 20ms I wouldn't think. It's literally 8 bit 2D.
     
  20. John_Leorid

    John_Leorid

    Joined:
    Nov 5, 2012
    Posts:
    646
    What does the ConsoleWindow / EditorLog look like? What's written there?

    And I'd still be interested in the UnityVersion you are using.