Can the Unit update code here be optimised?

[BadNeighbour]

Apologies if I’m asking a dumb question here, or it’s been asked before.

I’ve been going through all the GridSystem lectures again to try to get my head around how the grid stuff works, and I think I’m getting there. However I noticed the code being placed in the Unit script’s update function in this lecture:

 GridPosition newGridPosition = LevelGrid.Instance.GetGridPosition(transform.position);
        if (newGridPosition != gridPosition)
        {
            // Unit changed position
            GridPosition oldGridPosition = gridPosition;
            gridPosition = newGridPosition;

            LevelGrid.Instance.UnitMovedGridPosition(this, oldGridPosition, newGridPosition);
        }
    }

Unless I’m misunderstanding something, this is checking every unit on the map, every single frame, to see whether or not it has moved GridPosition.

Would it be possible to only check this if the unit is moving (or better yet, some kind of UnitActive check in case I add a way for a unit to be moved by an enemy/ally ability even when it’s not its own turn), and would that provide a performance benefit? Or would I merely be replacing checking for movement every frame with checking for activity every frame, and not accomplishing anything?

[Brian_Trotter]

Absolutely. Since we’re only ever moving one character at a time, you could do something like this:

  public void BeginMoving()
    {
        StartCoroutine(UpdatePosition());
    }

    public void StopMoving()
    {
        moving = false;
    }
    
    private bool moving;
    private IEnumerator UpdatePosition()
    {
        moving = true;
        while (moving)
        {
            GridPosition newGridPosition = LevelGrid.Instance.GetGridPosition(transform.position);
            if (newGridPosition != gridPosition)
            {
                // Unit changed Grid Position
                GridPosition oldGridPosition = gridPosition;
                gridPosition = newGridPosition;

                LevelGrid.Instance.UnitMovedGridPosition(this, oldGridPosition, newGridPosition);
            }
            yield return null;
        }
    }

Then when MoveAction starts moving, have it call it’s Unit’s BeginMoving() method and when it’s done, call StopMoving()

[BadNeighbour]

Thanks, I managed to get that working without much trouble at all.

So by the looks of things, you can use Coroutines and IEnumerators to avoid putting code in Update().. Am I right in thinking this is good practice? My instinct is to ask “Does this code need to be in Update?” going forward, just as the course teaches us to ask “Does this variable need to be public?”

I may try to do the same thing with the move action itself, since there’s a whole lot of code inside Update() for that and it’s presumably checking whether or not the unit is active every frame.

[Brian_Trotter]

@BadNeighbour

So by the looks of things, you can use Coroutines and IEnumerators to avoid putting code in Update(). . Am I right in thinking this is good practice?

Some would say yes, some would say no. I would say depends. A turn based strategy I wrote a couple of years ago used virtually no Update loops (exceptions were in one-offs like projectiles, where the lifetime of the projectile would entail updating). Beneath The Mountain by Brian K. Trotter