Evaluate value nested in foreach and if methods in chess

Question

I'm programming a game of chess in Unity3D and I'm trying to write the method to determine whether or not the king is in check by figuring out which king is the current player-to-move's king and then comparing the kings position against the list of threatened squares from the opponent.

I've tried using IndexOf, Intersect(), and a few other things, but I can't seem to get it to match properly. If I insert Debug.Log or Console.Write within the if() statement, the king's position returns correctly--but it doesn't seem to evaluate as true even when threatenedSquares has the king's position in it.

Current code EDITED WITH threatenedSquares returning a list of values via the debugger:

        public bool IsKingThreatened()
    {
        Vector2Int kingPosition;
        GameObject[] kings;
        kings = GameObject.FindGameObjectsWithTag("King");

        foreach(GameObject king in kings)
        {
            if (currentPlayer.pieces.Contains(king))
            {
                kingPosition = GridForPiece(king);
                Debug.Log("King position: " + kingPosition);
                Debug.Log("Player: " + currentPlayer.name);
                foreach(Vector2Int p in currentPlayer.threatenedSquares)
                {
                    Debug.Log(p);
                }

                if (currentPlayer.threatenedSquares.Contains(kingPosition))
                {
                    Debug.Log("King at " + kingPosition + "is threatened");
                    return true;
                }
            }
        }
        return false;
    }

Output:

Debug.Log: King position:  (Correct coordinates)
Debug.Log: Player: (active player)

kingThreatened == false;

currentPlayer.pieces is a List<GameObject> and threatenedSquares is a List<Vector2Int>

EDIT: cleaned it up a bit as per @clarkitech's point; however it still always returns false:

    public bool IsKingThreatened()
    {
        Vector2Int kingPosition;
        GameObject[] kings;
        kings = GameObject.FindGameObjectsWithTag("King");

        foreach (GameObject king in kings)
        {
            if (currentPlayer.pieces.Contains(king))
            {
                kingPosition = GridForPiece(king);

                return threatenedSquares.Contains(kingPosition);
            }
        }
        return false;
    }

Answer 1

You're always returning false. Here's a simplified view of the problem area.

    foreach (GameObject king in kings)
    {
        if (currentPlayer.pieces.Contains(king))
        {
            // your logic with no exit condition
        }
    }
    return false;

Once the loop completes, it will always return false unless you instead return kingThreatened

I would make this a little more direct, but I'm guessing a little bit about the properties on your game pieces.

public bool isKingThreatened => currentPlayer.pieces
    .Where(x => x.HasTag("king"))
    .Where(x => threatenedSquares.Contains(GridForPiece))
    .Any();

The pieces owned by currentPlayer that have tag "king" and where that piece's location is within threatenedSquares collection.

Answer 2

Clarkitect is correct about the problem with your logic, but the root of the problem is arguably that your code is complicated enough to make it easy to make that kind of a mistake in the first place.

I'd simplify things like so:

public bool IsKingThreatened()
{
    GameObject[] kings = GameObject.FindGameObjectsWithTag("King");
    return kings
        .Where(king => currentPlayer.pieces.Contains(king))
        .Select(GridForPiece)
        .Any(kingPosition => threatenedSquares.Contains(kingPosition));
}

Or perhaps:

public bool IsKingThreatened()
{
    GameObject currentKing = GameObject.FindGameObjectsWithTag("King")
        .Single(king => currentPlayer.pieces.Contains(king));
    Vector2Int kingPosition = GridForPiece(kingPosition);
    return threatenedSquares.Contains(kingPosition);
}