Tic Tac Toe - C# - Having trouble completing checks

Question

So here's the deal, I'm trying to compare images in a local resource for X and O, and if they're the same in the winCondition array, to message the user they won.

The problem I'm finding is that b1.Image isn't giving me the proper comparison. It's comparing this:

+  b1.Image  {System.Drawing.Bitmap}  System.Drawing.Image  {System.Drawing.Bitmap}

when I want it to compare the image names instead.

One of these problems is that when turnNumber = 5, it says I won, when I haven't. I believe this is due to the b1.Image problem. Once I click another square, it checks for the win again.

I want to disable the buttons once a game is completed, however I don't know how to do this.

Would it be as simple as:

   foreach (Button btnEnabled in buttonArray)
   {
       btnEnabled.Enabled = false;
   }

Here is the code, and again, thank you for any help. I have been struggling with this for a few days.

namespace BGreinAssignment2
{
    public partial class frmTicTacToe : Form
    {
        //Global variables
        private bool player1Turn = false;
        private bool player2Turn = true;

        private int[,] winCondition = 
        {
            {0,1,2}, //Horizontal top
            {3,4,5}, //Horizontal Middle
            {6,7,8}, //Horizontal Bottom
            {0,3,6}, //Vertical Left
            {1,4,7}, //Vertical Middle
            {2,5,8}, //Vertical Right
            {0,4,8}, //Diagonal Top Left to Bottom Right or Vice-Versa
            {2,4,6}  //Diagonal Top Right to Bottom Left or Vice-Versa
        };
        private Button[] buttonArray;
        private int turnNumber = 0;

        public frmTicTacToe()
        {
            InitializeComponent();
        }

        //Creates the button array for checks and sets X to go first.
        private void frmTicTacToe_Load(object sender, EventArgs e)
        {
            //Creates button array for checking if image is there/check for beginning of game
            buttonArray = new Button[9] {btnTopLeft, btnTopMid, btnTopRight, btnMidLeft, btnMid, btnMidRight, btnBotLeft, btnBotMid, btnBotRight};

            //Sets player 1 to go first to satisfy the "X always goes first"
            player1Turn = true;
            player2Turn = false;
        }

        /// <summary>
        /// Checks the buttons if the images don't create a win condition through the winCheck method,
        /// displays message box to user with "Draw!" break is included so it doesn't say "Draw!" for each button it checks.
        /// </summary>
        private void drawCheck()
        {
            foreach (Button checkDraw in buttonArray)
            {
                if (checkDraw.Image != null)
                {
                    MessageBox.Show("Draw!");
                    break;
                }
            }
        }

        /// <summary>
        /// Checks the win condition to see if the images are the same. If they are, it will show a message box with the winner.
        /// </summary>
        /// <param name="btnChecks">Creates an array to check the button images</param>
        /// <returns>If there is a winner, returns true and shows message box</returns>
        private bool winCheck(Button[] btnChecks)
        {
            bool win = false;
            for (int i = 0; i < 8; i++)
            {
                int a = winCondition[i, 0], b = winCondition[i, 1], c = winCondition[i, 2];

                Button b1 = btnChecks[a], b2 = btnChecks[b], b3 = btnChecks[c];

                if (b1.Image == null || b2.Image == null || b3.Image == null)
                {
                    continue;
                }

                if (b1.Image == b2.Image && b2.Image == b3.Image)
                {
                    win = true;
                    MessageBox.Show("Game over. " + b1.Image + " Wins!");
                }
            }
            return win;
        }

        //If player chooses top left square
        private void btnTopLeft_Click(object sender, EventArgs e)
        {
            if (btnTopLeft.Image == null)
            {
                if (player1Turn == true)
                {
                    if (turnNumber == 0)
                    {
                        btnTopLeft.Image = BGreinAssignment2.Properties.Resources.tic_tac_toe_X;
                        player1Turn = false;
                        player2Turn = true;
                        turnNumber++;
                    }
                    else
                    {
                        btnTopLeft.Image = BGreinAssignment2.Properties.Resources.tic_tac_toe_X;
                        player1Turn = false;
                        player2Turn = true;
                        turnNumber++;
                        if (turnNumber >= 5)
                        {
                            winCheck(buttonArray);
                        }
                        if (turnNumber == 9)
                        {
                            drawCheck();
                        }
                    }
                }
                else
                {
                    btnTopLeft.Image = BGreinAssignment2.Properties.Resources.tic_tac_toe_O;
                    player1Turn = true;
                    player2Turn = false;
                    turnNumber++;
                    if (turnNumber <= 5)
                    {
                        winCheck(buttonArray);
                    }
                    if (turnNumber == 9)
                    {
                        drawCheck();
                    }
                }
            }
            else
            {
                MessageBox.Show("This space has already been selected");
            }
        }

        //Excluded rest of code (just button clicks, repeats of same for 9 squares)

        /// <summary>
        /// Resets the game for the player.
        /// </summary>
        /// <param name="sender"></param>
        /// <param name="e"></param>
        private void btnPlayAgain_Click(object sender, EventArgs e)
        {
            //For each button, set image to null then reset turn counter and set turn to player 1.
            foreach (Button btnSpaces in buttonArray)
            {
                btnSpaces.Image = null;
            }
            turnNumber = 0;
            player1Turn = true;
            player2Turn = false;
        }
    }
}

Answer 1

As you discovered, comparing Images directly is not the answer. I haven't tested this, but I suspect it's because each Image could have very different properties, despite being loaded from the same resource. For example, if one button was only 20-pixels square, and another was 50, the Height and Width properties would be different. See the documentation for some other properties.

A better way to handle this, as @John3136 said, is to split the model away from the drawing. This could be as simple as using the Tag property on each button to store either an "O" or an "X". You'd check everything the same as you do now, except you'd be comparing b1.Tag == b2.Tag && b2.Tag == b3.Tag. A better, but more complicated way, would be to have an array of something to represent your game board. As with any programming problem, there's many possible ways to do it - char[9] would be the simplest, although string[] or bool?[] could work, creating an enum and storing an array of that, or an array of arrays of any of the above. (Also List<> instead of an array, eventually). In either case, you'd have to update your model at the same time as you update the image, but you're then doing a much simpler comparison to test for victory.

---

You're entirely correct about disabling the buttons - but here's a question for you. What happens if someone clicks on a button that the other player has already clicked? Sure, you can alert the user, but it still looks like a valid choice. Why not disable it as soon as it's clicked?

---

I'm also going to go a bit further afield and give you a tip for the "//Excluded rest of code (just button clicks, repeats of same for 9 squares)" line. If you're going to repeat the same code more than twice (and some people would say more than once), you're better off making a function which takes whatever will differentiate it as an argument. For example, you could have

    //If player chooses top left square
    private void btnTopLeft_Click(object sender, EventArgs e)
    {
        DoClick(btnTopLeft);
    }

    //If player chooses top center square
    private void ptnTopCenter_CLick(object sender, EventArgs e)
    {
        DoClick(btnTopCenter);
    }

    private void DoClick(Button button)
    {
      // Manipulate button appropriately
    }

This isn't doing anything you don't already know how to do - you're already calling other functions, and passing buttons around in arguments (see winCheck()). It's just a way of thinking which will reduce the amount of code you need to write, and thus the places you need to fix any given bug.