Switch (selectedAction) :-(

[Jonathan_Borchardt]

I feel like switching on the object type moves logic that belongs in the action into the GridSystemVisual class.

Does anyone have a cleaner way that keeps the tile color, attack range, and attack targets logic inside the individual actions?

Maybe:

BaseAction could expose overridable GetPositionsAndGridVisualTypes()

something like:
GetPositionsAndGridVisualTypes() returns GridPositionAndGridVisualType[] with whatever logic the action code deems reasonable.

where:

interface GridPositionAndGridVisualType {
  GridPosition gridPosition;
  GridVisualType gridVisualType;
}

e.g.

[ // psudo
  { pos: {0,0}, type: type.red }, // in range
  { pos: {1,0}, type: type.red }, // in range
  { pos: {2,0}, type: type.red }, // in range
  { pos: {0,1}, type: type.red }, // in range
  { pos: {1,1}, type: type.darkRed }, // enemy
  { pos: {2,1}, type: type.red }, // in range
  { pos: {0,2}, type: type.red }, // in range
  { pos: {1,2}, type: type.green}, // oh a third color option for some wonky action
  { pos: {2,2}, type: type.red }, // in range
]

then UpdateGridVisual() just passes this to ShowGridPositionList()

Any thoughts?

[bixarrio]

I am personally not a big fan of switch statements, so I implemented this like this:

I combined the ShowGridPositionRange and ShowGridPositionRangeSquare methods into one method by adding a circular boolean parameter

private void ShowGridPositionRange(GridPosition gridPosition, int range, GridVisualType gridVisualType, bool circular)
{
    var gridPositions = new List<GridPosition>();
    for (var z = -range; z <= range; z++)
    {
        for (var x = -range; x <= range; x++)
        {
            var offsetGridPosition = new GridPosition(x, z);
            var testGridPosition = gridPosition + offsetGridPosition;
            if (!LevelGrid.Instance.IsValidGridPosition(testGridPosition)) continue;

            if (circular) // if we want a circular range, trim the corners
            {
                // This creates a 'manhatten' circle.
                //could also use Mathf.Sqrt((x * x) + (z * z))
                var testDistance = Mathf.Abs(x) + Mathf.Abs(z);
                if (testDistance > range) continue;
            }

            gridPositions.Add(testGridPosition);
        }
    }

    ShowGridPositions(gridPositions, gridVisualType);
}

Then I created an interface

public interface IGridVisualRange
{
    bool RangeIsCircular();
    int GetMaxDistance();
    GridVisualType GetRangeGridVisualType();
}

which I only implemented on ShootAction, SwordAction and InteractAction (I left out all the other code in here for brevity)

public class ShootAction : BaseAction, IGridVisualRange
{
    // These values can be [SerializeField] instead of hard-coded
    public bool RangeIsCircular() => true;
    public int GetMaxDistance() => _maxShootDistance;
    public GridVisualType GetRangeGridVisualType() => GridVisualType.RedSoft;
}

Note I moved the GridVisualType enum out of GridSystemVisual but that is not necessary

Then, lastly, I changed the code where the switch was

private void UpdateGridVisual()
{
    HideAllGridPositions();

    var selectedUnit = UnitActionSystem.Instance.GetSelectedUnit();
    var selectedAction = UnitActionSystem.Instance.GetSelectedAction();

    // This checks if the selectedAction implements the IGridVisualRange interface
    if (selectedAction is IGridVisualRange rangedAction)
    {
        // It does, so show the range
        ShowGridPositionRange(selectedUnit.GetGridPosition(), rangedAction.GetMaxDistance(), rangedAction.GetRangeGridVisualType(), rangedAction.RangeIsCircular());
    }

    ShowGridPositions(selectedAction.GetValidActionGridPositions(), selectedAction.GetGridVisualType());
}

With this it is simple for me to add range to an action without too much fuss. I only need to implement the interface

Note I have changed the code slightly from my own implementation, so there may be a typo somewhere

---

Edit: Oh, I left out that I added a method to BaseAction to define the GridVisualType. This is for all actions

public abstract class BaseAction : MonoBehaviour
{
    public abstract GridVisualType GetGridVisualType();
}

public class ShootAction : BaseAction, IGridVisualRange
{
    // This return value can actually be a [SerializeField] instead of hard-coded
    public override GridVisualType GetGridVisualType() => GridVisualType.Red;
}

[Jonathan_Borchardt]

Thanks for your thoughts!
I still find it odd that the Action and the grid visual both need to calculate cells.
First action needs to know if the action can be performed, the the grid visual needs to calculate what cells to color color.
There is a separation of concerns issue here (One could disagree with the other).
It seems that keeping the knowledge of what cells are valid for an action could be solely defined in an action, and the grid visual would just know how to draw it.
I will eventually write it up in real code and share.
Thanks again for your code example!

[bixarrio]

If you want the range calculation to be in the action, too, it’s not difficult to adapt the code I posted:

Update the interface to now include a function that returns the range grid positions

public interface IGridVisualRange
{
    bool RangeIsCircular();
    int GetMaxDistance();
    GridVisualType GetRangeGridVisualType();
    // Add this function that should return all valid positions in range
    List<GridPosition> GetRangeGridPositions(GridPosition gridPosition);
}

We will be calculating the grid positions for each action that has a range. To avoid a lot of code duplication, we will make a new RangedBaseAction base class and implement the interface

public abstract class RangedBaseAction : BaseAction, IGridVisualRange
{
    public virtual bool RangeIsCircular() => false;
    public abstract int GetMaxDistance();
    public abstract GridVisualType GetRangeGridVisualType();
    // The new function that calculates the valid grid positions in range
    public virtual List<GridPosition> GetRangeGridPositions(GridPosition gridPosition)
    {
        var range = GetMaxDistance();
        var gridPositions = new List<GridPosition>();
        for (var z = -range; z <= range; z++)
        {
            for (var x = -range; x <= range; x++)
            {
                var offsetGridPosition = new GridPosition(x, z);
                var testGridPosition = gridPosition + offsetGridPosition;
                if (!LevelGrid.Instance.IsValidGridPosition(testGridPosition)) continue;

                if (RangeIsCircular())
                {
                    var testDistance = Mathf.Abs(x) + Mathf.Abs(z);
                    if (testDistance > range) continue;
                }

                gridPositions.Add(testGridPosition);
            }
        }

        // return the grid positions
        return gridPositions;
    }
}

This GetRangeGridPositions(GridPosition gridPosition) is the same code that I simply moved from GridSystemVisual and modified it a little to get the details from within the action

Now we will update ShootAction, SwordAction and InteractAction to inherit from RangedBaseAction instead of BaseAction. This doesn’t affect any of the other, existing code because these actions are still BaseAction classes (RangedBaseAction is derived from BaseAction)

public class ShootAction : RangedBaseAction
{
    public override bool RangeIsCircular() => true;
    public override int GetMaxDistance() => _maxShootDistance;
    public override GridVisualType GetRangeGridVisualType() => GridVisualType.RedSoft;
}

Lastly, we need to update GridSystemVisual to now use this change instead

private void UpdateGridVisual()
{
    HideAllGridPositions();

    var selectedUnit = UnitActionSystem.Instance.GetSelectedUnit();
    var selectedAction = UnitActionSystem.Instance.GetSelectedAction();

    // This checks if the selectedAction implements the IGridVisualRange interface
    if (selectedAction is IGridVisualRange rangedAction)
    {
        // It is, so get the positions from the action
        var rangedPositions = rangedAction.GetRangeGridPositions(selectedUnit.GetGridPosition());
        // and display them
        ShowGridPositions(rangedPositions, rangedAction.GetRangeGridVisualType());
    }

    ShowGridPositions(selectedAction.GetValidActionGridPositions(), selectedAction.GetGridVisualType());
}

Now the action is responsible for calculating the valid grid positions in range, and GridSystemVisual is only responsible for displaying it.