0
\$\begingroup\$

Hello I need help to optimize this code. I have a radar and it gets the enemies from a list and adds an image to their location to track them. I use Update to remove the images and LateUpdate to add the images. Is there any way I can do this without costing performance. I have a list of all the Enemies. I have a pool of over 40 images to use as tracking for the radar. Enemy is always moving. Any ideas?

private void Update()
{
    if (statsView_Panel.activeInHierarchy)
    {
        if (showIndicators_Enemies)
        {
            if (!enemyListHolder.activeInHierarchy)
            {
                enemyListHolder.SetActive(true);
            }

            Remove_PositionIndicatorOnEnemies();
        }
    }
}

private void LateUpdate()
{
    if (showIndicators_Enemies)
    {
        if (!enemyListHolder.activeInHierarchy)
        {
            enemyListHolder.SetActive(true);
        }

        Add_PositionIndicatorOnEnemies();
    }
}

private void Add_PositionIndicatorOnEnemies()
{
    //  Do if ListHolder is Active
    if (enemyListHolder.activeInHierarchy)
    {
        //  Run if 1 or more Enemies
        if (EnemiesManager.Count > 0)
        {
            //  Check All Enemies
            for (int i = 0; i < EnemiesManager.Count; i++)
            {
                //  Check All Indicators
                for (int s = 0; s < enemyIndicList.Count; s++)
                {
                    //  Choose Indicator that is not active *****BUG: Will Add All Deactivated Indicators to Active Enemies*****
                    if (enemyIndicList[s].gameObject.activeInHierarchy == false)
                    {
                        //  Activate Indicator and Reposition
                        enemyIndicList[s].gameObject.SetActive(true);
                        Vector3 aLocatorPos;
                        aLocatorPos = theCam.WorldToScreenPoint(satManager.GetByIndex(i).transform.position);
                        aLocatorPos.z = 0;
                        enemyIndicList[s].transform.position = aLocatorPos;

                        //  Break from Loop
                        break;
                    }
                }
            }
        }
    }
}

private void Remove_PositionIndicatorOnEnemies()
{
    //  Do if ListHolder is Active
    if (enemyListHolder.activeInHierarchy)
    {
        //  Run if 1 or more Enemies
        if (EnemiesManager.Count > 0)
        {
            //  Check All Indicators
            for (int s = 0; s < enemyIndicList.Count; s++)
            {
                //  Choose Indicator that is active
                if (enemyIndicList[s].gameObject.activeInHierarchy == true)
                {
                    //  Deactivate Indicator
                    enemyIndicList[s].gameObject.SetActive(false);
                }
            }
        }
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Let's review the code from bottom to top.

Remove_PositionIndicatorOnEnemies

  • Having an underscore in a method name doesn't fit with the .NET naming guidelines.
  • By reverting the first if condition you can return early if enemyListHolder.activeInHierarchy is false which saves you one level of indentation.
  • The next if condition can be omitted because that check is done inside the loop condition.
  • Instead of using a for loop I would prefer a foreach loop and would restrict the items to only them where activeInHierarchy is true by a Where() clause.

Implementing the mentioned changes will lead to

private void RemovePositionIndicatorOnEnemies()
{
    if (!enemyListHolder.activeInHierarchy) { return; }

    foreach (var enemy in enemyIndicList.Where(e=> e.gameObject.activeInHierarchy))
    {
        enemy.gameObject.SetActive(false);
    }
}  

Add_PositionIndicatorOnEnemies

Here, the first three points from above apply as well.

Implementing the mentioned changes will lead to

private void Add_PositionIndicatorOnEnemies()
{
    if (!enemyListHolder.activeInHierarchy) {return; }

    for (int i = 0; i < EnemiesManager.Count; i++)
    {
        var enemy = enemyIndicList.FirstOrDefault(e=> !e.gameObject.activeInHierarchy);
        if (enemy != null)
        {
            enemy.gameObject.SetActive(true);
            Vector3 aLocatorPos = theCam.WorldToScreenPoint(satManager.GetByIndex(i).transform.position);
            aLocatorPos.z = 0;
            enemy.transform.position = aLocatorPos;
        }
    }
}  

LateUpdate

  • Again returning early will save one level of indentation.

Implementing the mentioned changes will lead to

private void LateUpdate()
{
    if (!showIndicators_Enemies) { return; }

    if (!enemyListHolder.activeInHierarchy)
    {
        enemyListHolder.SetActive(true);
    }

    Add_PositionIndicatorOnEnemies();
}  

Update

  • Here we can save two levels of indentation by reverting and combining the first two if conditions.

Implementing the mentioned changes will lead to

private void Update()
{
    if (!statsView_Panel.activeInHierarchy || !showIndicators_Enemies) { return; }

    if (!enemyListHolder.activeInHierarchy)
    {
        enemyListHolder.SetActive(true);
    }

    Remove_PositionIndicatorOnEnemies();
}
\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much, so what I took from this is try to save as many indentations as you can as its more calculations and use more Linq which is meant for the performance of Lists and Arrays. I will be studying more Linq! \$\endgroup\$
    – WokerHead
    Dec 22, 2020 at 15:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.