Clean and well-designed Tic-Tac-Toe game

Question

My design goal was to try and make a "clean" and well-designed Tic-Tac-Toe game but also to try and use concepts and programming constructs that I'm new to.

I'm planning to implement an "AI Player" class later on. I know that Tic-Tac-Toe is a solved game so there are more effective approaches for AI logic but I've never programmed an AI before so my idea is to at runtime generate a "database" of all possible gamestates, and then have the AI rank each "intermediate gamestate" by the amount of positive outcomes (that is where the AI wins) - negative outcomes when it decides where to place its marker. Would this be a good idea and would it work at all?

Both positive and negative feedback is welcome.

Game.cs

public partial class Game : Form
{
    #region Fields & Properties

    internal static VisualCell[,] vGrid;
    internal static Player[] players;

    #endregion

    #region Constructors

    public Game()
    {
        InitializeComponent();
    }

    #endregion

    #region Instanced Methods

    private void Game_Load(object sender, EventArgs e)
    {
        players = new Player[2];
        players[0] = new Player(); // Set to human player or AI player
        players[1] = new Player(); // Always human
        GameRound.Complete += ShowCompletionDialog;
        GenerateGridBtns();
        Task roundTask = Task.Factory.StartNew(() => new GameRound().Start());
    }

    private void ShowCompletionDialog(OutcomeType gameOutcome)
    {
        long pID = (long)gameOutcome + 1;
        string WonText = "Player " + pID + " won!";

        if (gameOutcome == OutcomeType.Draw)
        { //*HACK*: If the marker is empty it means that the passed player was a dummy and game was a draw
            WonText = "The game ended in a draw!";
        }

        DialogResult dialogResult = MessageBox.Show("Play again?", WonText, MessageBoxButtons.YesNo);

        if (dialogResult == DialogResult.Yes)
        {
            RestartGame();
            // TODO: Add a bunch of resets here...
        } else if (dialogResult == DialogResult.No)
        {
            //do something else
        }
    }

    private void GenerateGridBtns()
    {
        vGrid = new VisualCell[3, 3];

        for (int x = 0; x < 3; x++)
        {
            for (int y = 0; y < 3; y++)
            {
                vGrid[x, y] = new VisualCell() 
                { 
                    AutoSize = true, 
                    Location = new Point(x*110, y*110), 
                    Size = new Size(124, 124),
                    Enabled = false,
                    Font = new Font(Font.FontFamily, 40),
                    CellPos = new Tuple<int,int>(x, y)
                };
                Controls.Add(vGrid[x, y]);
            }
        }
    }

    private void RestartGame() 
    {
        for (int x = 0; x < 3; x++)
        {
            for (int y = 0; y < 3; y++)
            {
                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Text = String.Empty));
                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Enabled = false));
            }
        }
        var round = new GameRound();
        Task roundTask = Task.Factory.StartNew(() => round.Start());
    }

    #endregion 
}

VCell.cs

class VisualCell : Button
{
    #region Fields & Properties

    public Tuple<int, int> CellPos { get; set; }


    protected override bool ShowFocusCues
    {
        get
        {
            return false;
        }
    }

    #endregion

    #region Constructors

    public VisualCell(): base()
    {

        this.SetStyle(ControlStyles.Selectable, false);
    }

    #endregion
}

Cell.cs

class Cell
{
    #region Properties & Fields

    private Mark markType = Mark.Empty; /* Have to use old way of defining properties here unfortunately due to lack of support for,
                                           default values in auto-implemented properties. */
    public Mark MarkType { 
        get { return markType; } 
        set 
        {   //Only allow the type of the cell to change if it it's empty
            if (MarkType == Mark.Empty)
            {
                markType = value;
            }
        } 
    }

    #endregion

    #region Constructors

    public Cell()
    {
        MarkType = Mark.Empty;
    }

    #endregion
}

Grid.cs

enum OutcomeType
{
    None = -1, CrossWin, NoughtWin, Draw
}

class Grid
{
    const int MAX_CELLS = 3;

    #region Properties & Fields

    public Cell[,] cells { get; set; }
    public OutcomeType Outcome { get; private set; }

    #endregion

    #region Constructors

    public Grid()
    {
        Outcome = OutcomeType.None;
        cells = new Cell[MAX_CELLS, MAX_CELLS];

        for (int x = 0; x < MAX_CELLS; x++)
        {
            for (int y = 0; y < MAX_CELLS; y++)
            {
                cells[x, y] = new Cell();
            }
        }
    }

    #endregion

    #region Instanced Methods

    public bool CheckOutcome(Tuple<int, int> coords, Player player)
    {
        //Check for draws first
        if (cells.GetEmptyCells().Length == 0)
        {
            Outcome = OutcomeType.Draw;   
            return true;
        }

        //Now check for player wins
        var corners = new Tuple<int, int>[] { Tuple.Create(0,0), Tuple.Create(2,0), Tuple.Create(0,2), Tuple.Create(2,2) };
        var sides = new Tuple<int, int>[] { Tuple.Create(1,0), Tuple.Create(0,1), Tuple.Create(1,2), Tuple.Create(2,1) };
        var middle = new Tuple<int, int>(1,1);
        var checkDiagonals = false;

        //If the cell is at the corner or the middle we have to check for diagonal wins too
        if (corners.Any(e => e.Equals(coords)) || middle.Equals(coords)) { checkDiagonals = true; }

        if (player.PlayerWon(cells[coords.Item1, coords.Item2], this, checkDiagonals))
        {
            switch (player.marker)
            {
                case Mark.Cross:
                    Outcome = OutcomeType.CrossWin;
                    break;
                case Mark.Nought:
                    Outcome = OutcomeType.NoughtWin;
                    break;
            }

            return true;
        }

        //If execution reaches this point then no one has won, return false
        return false;
    }

   #endregion
}

Player.cs

class Player
{
    #region Fields & Properties

    private static int ctr;
    private static Game form;
    public Mark marker;
    public readonly int ID;
    public int Score { get; set; }

    #endregion 

    #region Constructors

    public Player()
    {
        ID = ctr++;

        if (ID == 0) { // X is the starting player
            marker = Mark.Cross; } 
        else { 
            marker = Mark.Nought; }
    }

    #endregion

    #region Instanced Methods

    public virtual Tuple<int, int> GetPlayerChoice(Grid grid, Player player) // Virtual because we want the option to override this for AIPlayer later.
    {
        var pMark = (player.ID == 0) ? "X" : "O";
        var choice = new Tuple<int,int>(-1, -1);
        var waitBtns = new List<Task>() { new Task(() => new SpinWait().SpinOnce()) };

        //Enable eligble buttons for player
        for (int x = 0; x < grid.cells.GetLength(0); x++)
        {
            for (int y = 0; y < grid.cells.GetLength(1); y++)
            {
                if (grid.cells[x, y].MarkType == Mark.Empty)
                {
                    Game.vGrid[x, y].Invoke(new Action(() => Game.vGrid[x, y].Enabled = true));                                
                    Game.vGrid[x, y].Click += new EventHandler(
                        (a,b) => 
                        {
                            var pos = new Tuple<int,int>((a as VisualCell).CellPos.Item1, (a as VisualCell).CellPos.Item2);
                            choice = Tuple.Create(pos.Item1, pos.Item2);
                            Game.vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => Game.vGrid[pos.Item1, pos.Item2].Text = pMark));
                            Game.vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => Game.vGrid[pos.Item1, pos.Item2].Enabled = false));
                            waitBtns.Add(new Task(() => new SpinWait().SpinOnce()));
                            waitBtns[waitBtns.FindIndex(t => t.Status == TaskStatus.Created)].Start();
                        });
                }
            }                
        }
        Task.WaitAny(waitBtns.ToArray());
        return choice;
    }

    public bool PlayerWon(Cell cell, Grid grid, bool checkDiagonals) 
    {
        if (cell.HorizontalRelatives(grid) == 2 || cell.VerticalRelatives(grid) == 2)
        {
            return true;
        }

        if (checkDiagonals)
        {
            if (cell.DiagonalRelatives(grid) == 2) { return true; } 
            else if (cell.DiagonalRelatives2(grid) == 2) { return true; }
        }

        return false;
    }

    #endregion 
}

Gameround.cs

class GameRound
{
    public delegate void RoundEndHandler(OutcomeType gameOutcome);

    #region Fields & Properties

    private static int ctr;
    private Grid grid;
    public readonly int ID;
    public static event RoundEndHandler Complete;
    public Player CurrentPlayer { get; private set; }

    #endregion

    #region Constructors

    public GameRound()
    {
        ID = ctr++;
        grid = new Grid();
    }

    #endregion

    #region Instanced Methods

    public void Start()
    {
        bool doLoop = true;

        while (doLoop)
        {
            for (int i = 0; i < Game.players.Length; i++)
            {
                CurrentPlayer = Game.players[i];
                var coord = Game.players[i].GetPlayerChoice(grid, CurrentPlayer);
                grid.cells[coord.Item1, coord.Item2].MarkType = CurrentPlayer.marker;

                //check if game is over
                if (grid.CheckOutcome(coord, CurrentPlayer))  
                {
                    doLoop = false;
                    break; 
                }
            }
        }

        Complete(grid.Outcome); 
    }

    #endregion
}

ExtensionMethods.cs

static class ExtensionMethods
{
    #region Cell Extension Methods

    public static int DiagonalRelatives(this Cell cell, Grid grid) 
    {
        int relatives = new int();

        for (int x = 0; x < 3; x++)
        {
            if (grid.cells[x, x].MarkType.Equals(cell.MarkType)) { relatives++; }
        }

        return relatives - 1;
    }

    public static int DiagonalRelatives2(this Cell cell, Grid grid)
    {
        int relatives = new int();

        for (int x = 0; x < 3; x++)
        {
            if (grid.cells[x, 2 - x].MarkType.Equals(cell.MarkType)) { relatives++; }
        }

        return relatives - 1;
    }

    public static int HorizontalRelatives(this Cell cell, Grid grid)
    {
        int relatives = new int();
        int rowNum = grid.cells.IndexOf(cell).Item2;

        for (int x = 0; x < 3; x++)
        {
            //Find row of cell
            if (grid.cells[x, rowNum].MarkType.Equals(cell.MarkType)) { relatives++; }
        }

        return relatives - 1;
    }
    public static int VerticalRelatives(this Cell cell, Grid grid)
    {
        int relatives = new int();
        int colNum = grid.cells.IndexOf(cell).Item1;

        for (int y = 0; y < 3; y++)
        {
            //Find row of cell
            if (grid.cells[colNum, y].MarkType.Equals(cell.MarkType)) { relatives++; }
        }

        return relatives - 1;
    }

    #endregion

    #region Cell[] Extension Methods

    public static Cell[] GetEmptyCells(this Cell[,] cells)
    {
        List<Cell> emptyCells = new List<Cell>();

        foreach (Cell cell in cells)
        {
            if (cell.MarkType == Mark.Empty)
            {
                emptyCells.Add(cell);
            }
        }

        return emptyCells.ToArray();
    }

    public static Tuple<int, int> IndexOf(this Cell[,] cells, Cell cell) 
    {
        for (int x = 0; x < cells.GetLength(0); x++)
        {
            for (int y = 0; y < cells.GetLength(1); y++)
            {
                if (cells[x, y].Equals(cell))
                {
                    return new Tuple<int,int>(x, y);
                }
            }
        }

        //If code reaches this point, then it didn't find anything, return -1
        return new Tuple<int,int>(-1, -1);
    }

    #endregion
}

Answer 1

One more set of comments.

---

You don't do a good job of invoking Dispose on objects whose classes implement IDisposable.

This includes your many Task instances, and your many VisualCell instances (there may be others I didn't notice).

I'd like to see more calls to Dispose, and/or using statements.

You can dispute this advice:

• Tasks: Do I need to dispose of Tasks?

• VisualCell instances: they have the same lifetime as your main form, and the application will end (with all resources reclaimed by the O/S) when your main form closes.

I mention it because it (disposing disposables) is IMO "good practice".

---

The code in GetPlayerChoice is IMO a horror show: masses of Invoke statements and Task instances with SpinWait instances.

You're installing vGrid[x, y].Click event handlers each time you call GetPlayerChoice (and I don't understand what happens to old, previous-installed Click event handlers).

The normal way to do this in a Windows Forms application would be:

• Don't use a Task
• Install a Click event handler once, in the Form (Game) class
• The event handler could call a method of GameRound, e.g. a method named void OnPlayerChoice(VisualCell chosenCell)
• Game would need a reference to the GameRound instance: therefore the GameRound instance would be member data of Game, instead of just a local variable in the NewGame method, so that Game could call the GameRound instance's OnPlayerChoice method
• OnPlayerChoice could return OutcomeType? in which case it wouldn't need a delegate to do a callback to the ShowCompletionDialog
• GameRound would need to remember which is the CurrentPlayer (as a property not just as a local variable); alternatively, Game could remember that, and pass it as a parameter. I still think that maybe GameRound should be merged into the Game class.

Because there are no Task instances, you don't need to use Invoke. The game is "event-driven" from the GUI, i.e. driven by the Click event handlers.

If one of the players is an AI then the Game, instead of waiting for the Click event, can call the AI player's "choose" method immediately after processing the human player's GUI click.

---

Alternatively it's theoretically possible, e.g. by defining abstract interfaces between 'View' (Form) and 'Controller' (GameRound) and 'Data Model' (Grid and Cells), to decouple the Game or GameRound from the Form more completely: so that the GameRound doesn't know whether it's attached to a Form, or to a Console, or is simply writing results to a File.

If you eventually did that, that might be another way to implement an AI: if the GameRound doesn't know where the player choices were coming from it might be easy to make some of the choices come from an AI instead of the GUI.

---

isn't the point of the managed garbage collector to dispose of objects that aren't used anymore for me?

Yes. The garbage collector might take a while to run, though. For example if you open a file and then let go of the open file without disposing it, the O/S file handle will remain open until the garbage collector reclaims the object; during that time, you can't reopen/reuse the file.

Similarly some resources are in limited supply: you might only be allowed a few SQL connections, or SQL transaction, or GDI objects (brushes and fonts). "A few" might be hundreds, but if the garbage collector runs every 10 seconds or so that gives you a rate-limited throughput of 10 per second: so your code works fine until you put a real-world heavy load on it.

The garbage collector is there to reclaim memory. It knows what memory you have used. If the memory contains undisposed resources those resources will be properly disposed; but I see that as a second line of defence (in case you didn't dispose them explicitly).

Also about the old previous-installed event handlers for Click I also thought that would screw things up but having ran a dozen iterations of the game and seperate rounds without a problem, it doesn't seem to do so?

If I put a breakpoint in the event handler I can see that it's called multiple times: once the first time, twice the second time, etc. If it's called more than once (e.g. twice) then it sets the vButton text twice, disables it twice, adds two tasks to the waitBtns list. The event handlers also occupy memory and can't be garbage-collected: so if you ran it for long enough (much longer than you've run it) it would run out of memory.

Would you elaborate on what you mean by implementing abstract interfaces?

The simplest version I can think of wouldn't have interfaces.

Have a GameController with the following public API:

// Let players play
void PlayerChoose(Marker playerId, Tuple<int,int> position);
// Let players inspect the current board
// (optional, otherwise players can remember the state of play
// e.g. the Game Form has this state already duplicated in its buttons)
Cell[,] Cells { get; }
// Tell players when the board has changed
delegate void PlayedEventHandler(Marker playerId, Tuple<int,int> position);
event PlayedEventHandler Played;
// Tell players when the game is finished
delegate void FinishedEventHandler(OutcomeType outcome);
event FinishedEventHandler Finished;
// Let players start a new game
void Reset();

An all-human Form-based application can:

• Have a Game Form like yours
• Game contains (references) a GameController
• Game attaches to the Played event handler and updates its UI buttons accordingly
• Game attaches to its buttons' Clicked event handler and invokes the PlayerChoose method accordingly

A human-versus-AI application can do the same as the above, except:

• The Game Form is only used by one player
• There's another, AI class, which:
• Contains a reference to the GameController
• Attaches to the Played event handler
• Chooses its next move and invokes the PlayerChoose method, when the Played event handler tells it that the human has played.

See also:

• Model–view–presenter
• Other tic-tac-toe implementations in C# on this site (I don't know which of them are good)

Answer 2

To answer the "bonus question" that may not be the right algorithm for the AI: it assumes that the human player is playing randomly.

As a human player, I play to a) not-lose b) win. That's why most games (between experienced humans) end in a draw.

With the algorithm you suggested, there might be a move which results in 10 possible wins (if I play stupidly/randomly) and 1 loss (if I play cleverly). Because the win/loss ratio is high (10-to-1) you might pick it and (because I'm not stupid) I then play the correct move and win.

IMO the algorithm you want is to choose the move which allows you to prevent my winning no matter what I do, i.e. a move which allows you to guarantee you can't lose.

---

Mandatory code review:

I'm surprised that plays and vGrid are static inside Game. What if there's more than one Game instance?

It's difficult to assess the lifetime of a Game: why doesn't a Game include a Grid? Instead Game instantiates a GameRound as a local/temporary, which it passes to a local/temporary Task?

The Cell extension methods like int DiagonalRelatives(this Cell cell, Grid grid) might be more naturally ordinary methods of the Grid class.

What happens if the player plays on an already-marked cell? It looks like nothing happens, except that they lose their turn?

I suspect this could be a lot simpler/clearer (but I'm not tempted to rewrite it at the moment).

Answer 3

Okay I've tried my best at revamping my code and restructuring after the MVP design pattern. It's not a complete rewrite, and one could argue that it's not a "true" MVP implementation because I never implemented an interface for the model, it felt redudant for the primitive data that I was storing/accessing. Keep in mind, my interpretation of MVP is based on about an hour of reading about it and this is my first attempt at structuring my code after any "explicit" programming design pattern.

I've also taken some of your other ChrisW's suggestions and implemented them. I might not have implemented all of them, it's not cause I didn't like them or that I consider myself to know better, it's just that as a part of my learning process, first hand I always try to force myself to think up my own solutions to problems instead of just copy & pasting others solutions, others code should serve as inspiration I believe I'll learn to code better by that.

Overall, I want to thank ChrisW for his great feedback and his patience with me. I think the code looks cleaner now (But I'll await your feedback Chris) the only caveat with this solution is that implementing the AI Player will be a tad bit more difficult now.

Here's my new code(I've only listed the files that I have changed or added):

For easy access, I'll list roughly some of the changes that I've made:

• Removed static fields (as suggested by ChrisW)
• Not calling Dispose() on disposables but I have removed almost all of them now
• Removed a bunch of redundant code (IDs and some counters)
• GameRound class removed
• Added GameController class
• MVP design pattern implemented
• ... a bunch of other stuff I don't remember

GameForm.cs(formerly Game.cs)

public partial class GameForm : Form, IGameViewer
{
    #region Fields & Properties

    private VisualCell[,] vGrid = new VisualCell[3, 3];
    private IGamePresenter presenter;

    #endregion

    #region Constructors

    public GameForm()
    {
        InitializeComponent();
    }

    #endregion

    #region Instanced Methods

    private void Game_Load(object sender, EventArgs e)
    {
        presenter = new GameController(this);
        presenter.OnGameEnd += ShowCompletionDialog; // When IGamePresenter raises OnGameEnd event then the viewer is free to show completion dialog
    }

    private void ShowCompletionDialog(OutcomeType outcome)
    {
        string WonText = "Player " + (long)outcome + " won!";

        if (outcome.Equals(OutcomeType.Draw))
        { 
            WonText = "The game ended in a draw!";
        }

        DialogResult dialogResult = MessageBox.Show("Play again?", WonText, MessageBoxButtons.YesNo);

        if (dialogResult == DialogResult.Yes)
        {
          presenter.RestartGame(); // Viewer -> Presenter, restart the game
        } else if (dialogResult == DialogResult.No)
        {
            //do something else
        }
    }

    void IGameViewer.DisplayCell(Cell cell)
    {
        vGrid[cell.coords.Item1, cell.coords.Item2] = new VisualCell() 
        {
            AutoSize = true,
            Location = new Point(cell.coords.Item1 * 110, cell.coords.Item2 * 110),
            Size = new Size(124, 124),
            Font = new Font(Font.FontFamily, 40),
            CellPos = new Tuple<int, int>(cell.coords.Item1, cell.coords.Item2)
        };

        vGrid[cell.coords.Item1, cell.coords.Item2].Click += new EventHandler(
                    (a, b) =>
                    {
                        var pos = new Tuple<int, int>((a as VisualCell).CellPos.Item1, (a as VisualCell).CellPos.Item2);
                        presenter.PlayerChoice(pos); // Viewer -> Presenter, when btn is clicked call PlayerChoice();
                    });

        Controls.Add(vGrid[cell.coords.Item1, cell.coords.Item2]);
    }

    void IGameViewer.CellChanged(Cell cell) 
    { 
        vGrid[cell.coords.Item1, cell.coords.Item2].Text = (cell.MarkType.Equals(Mark.Cross)) ? "X" : "O";
        vGrid[cell.coords.Item1, cell.coords.Item2].Enabled = false;
    }

    void IGameViewer.ResetCell(Cell cell) 
    {
        vGrid[cell.coords.Item1, cell.coords.Item2].Text = String.Empty;
        vGrid[cell.coords.Item1, cell.coords.Item2].Enabled = true; 
    }

    #endregion 
}

GameController.cs

public delegate void GameEndHandler(OutcomeType outcome);

class GameController : IGamePresenter 
{
    #region Fields & Properties

    private IGameViewer viewer;
    private Player[] players = new Player[2];
    public event GameEndHandler OnGameEnd;
    public Player CurrentPlayer { get; private set; }

    private Grid grid;
    #endregion        

    #region Constructors

    public GameController(IGameViewer viewer) 
    { 
        this.viewer = viewer;
        grid = new Grid(viewer);
        players[0] = new Player(Mark.Cross); // Set to human or AI
        players[1] = new Player(Mark.Nought); // Always set to human
        CurrentPlayer = players[0];
    }

    #endregion

    #region Instanced Methods

    void IGamePresenter.PlayerChoice(Tuple<int,int> coords) 
    { 
        // Presenter -> Model, place marker at coords
        grid.cells[coords.Item1, coords.Item2].MarkType = CurrentPlayer.marker;

        // Model -> Presenter, if grid reaches an outcome end the game
        if(grid.CheckOutcome(coords, CurrentPlayer)) 
        {
            OnGameEnd(grid.Outcome);
        }

        CurrentPlayer = (CurrentPlayer.ID == 0) ? players[1] : players[0];
    }

    void IGamePresenter.RestartGame() 
    {
        for (int x = 0; x < grid.cells.GetLength(0); x++)
        {
            for (int y = 0; y < grid.cells.GetLength(1); y++)
            {
                grid.cells[x, y].Reset();
            }
        }
    }

    #endregion
}

Cell.cs

class Cell
{
    #region Properties & Fields

    private Mark markType = Mark.Empty; /* Have to use old way of defining properties here unfortunately due to lack of support for,
                                         * default values in auto-implemented properties. */

    private IGameViewer viewer;
    public readonly Tuple<int, int> coords;
    public Mark MarkType { 
        get { return markType; } 
        set 
        {
            // Only allow changes to cells without a mark 
            if (markType.Equals(Mark.Empty)) 
            {    
                markType = value;
                viewer.CellChanged(this); //Model -> Viewer update viewer to reflect change in cell marktype
            }      
        } 
    }

    #endregion

    #region Constructors

    public Cell(IGameViewer viewer, Tuple<int, int> coords)
    {
        this.viewer = viewer;
        this.coords = coords;
        viewer.DisplayCell(this);
    }

    #endregion

    #region Instanced Methods

    public void Reset() //This method is necessary because we can't use the property to
    {
        markType = Mark.Empty;
        viewer.ResetCell(this);
    }

    #endregion
}

public enum OutcomeType
{
    None = -1, CrossWin, NoughtWin, Draw
}

class Grid
{
    const int MAX_CELLS = 3;

    #region Properties & Fields

    public Cell[,] cells { get; set; }
    public OutcomeType Outcome { get; private set; }

    #endregion

    #region Constructors

    public Grid(IGameViewer viewer)
    {
        Outcome = OutcomeType.None;
        cells = new Cell[MAX_CELLS, MAX_CELLS];

        for (int x = 0; x < MAX_CELLS; x++)
        {
            for (int y = 0; y < MAX_CELLS; y++)
            {
                cells[x, y] = new Cell(viewer, new Tuple<int,int>(x, y));
            }
        }
    }

    #endregion

    #region Instanced Methods

    public bool CheckOutcome(Tuple<int, int> coords, Player player)
    {
        //Check for draws first
        if (cells.GetEmptyCells().Length == 0)
        {
            Outcome = OutcomeType.Draw;   
            return true;
        }

        //Now check for player wins
        var corners = new Tuple<int, int>[] { Tuple.Create(0,0), Tuple.Create(2,0), Tuple.Create(0,2), Tuple.Create(2,2) };
        var sides = new Tuple<int, int>[] { Tuple.Create(1,0), Tuple.Create(0,1), Tuple.Create(1,2), Tuple.Create(2,1) };
        var middle = new Tuple<int, int>(1,1);
        var checkDiagonals = false;

        //If the cell is at the corner or the middle we have to check for diagonal wins too
        if (corners.Any(e => e.Equals(coords)) || middle.Equals(coords)) { checkDiagonals = true; }

        if (player.PlayerWon(cells[coords.Item1, coords.Item2], this, checkDiagonals))
        {
            switch (player.marker)
            {
                case Mark.Cross:
                    Outcome = OutcomeType.CrossWin;
                    break;
                case Mark.Nought:
                    Outcome = OutcomeType.NoughtWin;
                    break;
            }

            return true;
        }

        //If execution reaches this point then no one has won, return false
        return false;
    }

   #endregion
}

Player.cs

class Player
{
    #region Fields & Properties

    private static int ctr;
    public Mark marker;
    public readonly int ID;
    public int Score { get; set; }

    #endregion 

    #region Constructors

    public Player(Mark marker)
    {
        ID = ctr++;
        this.marker = marker;
    }

    #endregion

    #region Instanced Methods

    public bool PlayerWon(Cell cell, Grid grid, bool checkDiagonals) 
    {
        if (cell.HorizontalRelatives(grid) == 2 || cell.VerticalRelatives(grid) == 2)
        {
            return true;
        }

        if (checkDiagonals)
        {
            if (cell.DiagonalRelatives(grid) == 2) { return true; } 
            else if (cell.DiagonalRelatives2(grid) == 2) { return true; }
        }

        return false;
    }

    #endregion 
}

IGameViewer.cs

/// <summary>
/// IGameViewer represents the viewing medium of the game, it could for example be WinForms, WPF or a Console application.
/// It receives data from the Model and Presenter and displays the data in a visual way for the enduser.
/// </summary>
interface IGameViewer
{
    //This is called from Model whenever a cell changes type
    void CellChanged(Cell cell);
    //Called whenever a new cell is created
    void DisplayCell(Cell cell);
    //Resets the cell to it's initial state, called when restarting the game
    void ResetCell(Cell cell);
}

IGamePresenter.cs

/// <summary>
/// GamePresenter interface will control the flow of the game, by manipulating the model(Cell/Grid) and 
/// formatting the data so it is presentable to the viewer(GameForm).
/// </summary>
interface IGamePresenter
{
    void RestartGame();
    void PlayerChoice(Tuple<int, int> coords);
    event GameEndHandler OnGameEnd;
}

Answer 4

I posted an edited version of some of your classes, below:

• I removed your comments so that my comments would be more apparent; all changes are commented.
• I commented-out instead of deleting some your lines of code, to make my deletions apparent

The major change in this version is that there are no longer any static member data.

If I refactor then I refactor in stages: this would be the end of my first round of refactoring; this round doesn't change any existing algorithms, just cleans up stray inter-class interactions (i.e. static data, and unused data).

Game.cs

public partial class Game : Form
{
    #region Fields & Properties

    // I don't like static data except where necessary.
    internal VisualCell[,] vGrid;
    internal Player[] players;

    #endregion

    #region Constructors

    public Game()
    {
        InitializeComponent();
    }

    #endregion

    #region Instanced Methods

    private void Game_Load(object sender, EventArgs e)
    {
        players = new Player[2];
        // Clearer to pass the correct Mark as a parameter.
        players[0] = new Player(Mark.Cross);
        players[1] = new Player(Mark.Nought);
        // Avoid static data
        //GameRound.Complete += ShowCompletionDialog;
        GenerateGridBtns();
        // Invoke new method.
        NewGameStart();
    }

    private void ShowCompletionDialog(OutcomeType gameOutcome)
    {
        long pID = (long)gameOutcome + 1;
        string WonText = "Player " + pID + " won!";

        if (gameOutcome == OutcomeType.Draw)
        {
            WonText = "The game ended in a draw!";
        }

        DialogResult dialogResult = MessageBox.Show("Play again?", WonText, MessageBoxButtons.YesNo);

        if (dialogResult == DialogResult.Yes)
        {
            RestartGame();
        }
        else if (dialogResult == DialogResult.No)
        {
            // What now?
        }
    }

    private void GenerateGridBtns()
    {
        vGrid = new VisualCell[3, 3];

        for (int x = 0; x < 3; x++)
        {
            for (int y = 0; y < 3; y++)
            {
                vGrid[x, y] = new VisualCell()
                {
                    AutoSize = true,
                    Location = new Point(x * 110, y * 110),
                    Size = new Size(124, 124),
                    Enabled = false,
                    Font = new Font(Font.FontFamily, 40),
                    CellPos = new Tuple<int, int>(x, y)
                };
                Controls.Add(vGrid[x, y]);
            }
        }
    }

    private void RestartGame()
    {
        for (int x = 0; x < 3; x++)
        {
            for (int y = 0; y < 3; y++)
            {
                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Text = String.Empty));
                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Enabled = false));
            }
        }
        // Invoke new method.
        NewGameStart();
    }

    // Make this a method so that it can be called from two places (start and restart)
    void NewGameStart()
    {
        // I don't see why GameRound and Game aren't the same class.
        // Because GameRound uses data owned by Game i.e. players and vGrid it would be
        // more convenient to move the GameRound methods into Game.
        // Or, construct mew players and new vGrid for each GameRound, as members of GameRound.
        // But keeping your existing class stucture (two classes) it's better to share data
        // by passing as parameters (e.g. into the constructor) than sharing static data.
        var round = new GameRound(ShowCompletionDialog, vGrid, players);
        Task roundTask = Task.Factory.StartNew(() => round.Start());
    }

    #endregion
}

GameRound.cs

class GameRound
{
    public delegate void RoundEndHandler(OutcomeType gameOutcome);

    #region Fields & Properties
    // Could call these Id and IdCounter
    // or delete them because they're not used anywhere.
    //private static int ctr;
    //public readonly int ID;
    private Grid grid;
    // Doesn't have to be public static. Avoid static unless necessary.
    private event RoundEndHandler Complete;
    VisualCell[,] vGrid;
    Player[] players;
    // CurrentPlayer can be a local variable, not a public property.
    //public Player CurrentPlayer { get; private set; }

    #endregion

    #region Constructors

    // Pass the RoundEndHandler etc. as parameters instead of using public static.
    public GameRound(
        RoundEndHandler complete,
        VisualCell[,] vGrid,
        Player[] players)
    {
        // ID isn't used anywhere.
        //ID = ctr++;
        grid = new Grid();
        // Initialize new instance data.
        Complete = complete;
        this.vGrid = vGrid;
        this.players = players;
    }

    #endregion

    #region Instanced Methods

    public void Start()
    {
        // Use 'return' to escape from a nested loop.
        //bool doLoop = true;
        //while (doLoop)
        while (true)
        {
            // Acts on own instance data i.e. this.players not static Game.players.
            for (int i = 0; i < players.Length; i++)
            {
                // CurrentPlayer can be a local variable, not a public property.
                Player CurrentPlayer = players[i];
                // Pass vGrid as a parameter.
                var coord = players[i].GetPlayerChoice(grid, CurrentPlayer, vGrid);
                grid.cells[coord.Item1, coord.Item2].MarkType = CurrentPlayer.marker;

                //check if game is over
                OutcomeType? outcome = grid.CheckOutcome(coord, CurrentPlayer);
                if (outcome.HasValue)
                {
                    // Use 'return' to escape from a nested loop.
                    Complete(outcome.Value);
                    return;
                    //doLoop = false;
                    //break;
                }
            }
        }

        //Complete(grid.Outcome);
    }

    #endregion
}

Player.cs

class Player
{
    #region Fields & Properties

    // Never used.
    //private static Game form;
    // Clearer to pass the correct Mark as a parameter.
    //private static int ctr;
    //public readonly int ID;
    public Mark marker;
    // Never used.
    //public int Score { get; set; }

    #endregion

    #region Constructors

    public Player(Mark marker)
    {
        // Clearer to pass the correct Mark as a parameter.
        //ID = ctr++;

        //if (ID == 0)
        //{ // X is the starting player
        //    marker = Mark.Cross;
        //}
        //else
        //{
        //    marker = Mark.Nought;
        //}
        this.marker = marker;
    }

    #endregion

    #region Instanced Methods

    // vGrid is passed by parameter, not access static Game.vGrid
    public virtual Tuple<int, int> GetPlayerChoice(Grid grid, Player player, VisualCell[,] vGrid)
    {
        // Player's marker is a public property: might as well use that instead of 'ID'.
        //var pMark = (player.ID == 0) ? "X" : "O";
        var pMark = (player.marker == Mark.Cross) ? "X" : "O";
        var choice = new Tuple<int, int>(-1, -1);
        var waitBtns = new List<Task>() { new Task(() => new SpinWait().SpinOnce()) };

        //Enable eligble buttons for player
        for (int x = 0; x < grid.cells.GetLength(0); x++)
        {
            for (int y = 0; y < grid.cells.GetLength(1); y++)
            {
                if (grid.cells[x, y].MarkType == Mark.Empty)
                {
                    vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Enabled = true));
                    vGrid[x, y].Click += new EventHandler(
                        (a, b) =>
                        {
                            var pos = new Tuple<int, int>((a as VisualCell).CellPos.Item1, (a as VisualCell).CellPos.Item2);
                            choice = Tuple.Create(pos.Item1, pos.Item2);
                            vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => vGrid[pos.Item1, pos.Item2].Text = pMark));
                            vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => vGrid[pos.Item1, pos.Item2].Enabled = false));
                            waitBtns.Add(new Task(() => new SpinWait().SpinOnce()));
                            waitBtns[waitBtns.FindIndex(t => t.Status == TaskStatus.Created)].Start();
                        });
                }
            }
        }
        Task.WaitAny(waitBtns.ToArray());
        return choice;
    }

    public bool PlayerWon(Cell cell, Grid grid, bool checkDiagonals)
    {
        if (cell.HorizontalRelatives(grid) == 2 || cell.VerticalRelatives(grid) == 2)
        {
            return true;
        }

        if (checkDiagonals)
        {
            if (cell.DiagonalRelatives(grid) == 2) { return true; }
            else if (cell.DiagonalRelatives2(grid) == 2) { return true; }
        }

        return false;
    }

    #endregion
}

Grid.cs

class Grid
{
    const int MAX_CELLS = 3;

    #region Properties & Fields

    public Cell[,] cells { get; set; }
    // Instead of storing Outcome in the grid, can return it as a return code.
    //public OutcomeType Outcome { get; private set; }

    #endregion

    #region Constructors

    public Grid()
    {
        //Outcome = OutcomeType.None;
        cells = new Cell[MAX_CELLS, MAX_CELLS];

        for (int x = 0; x < MAX_CELLS; x++)
        {
            for (int y = 0; y < MAX_CELLS; y++)
            {
                cells[x, y] = new Cell();
            }
        }
    }

    #endregion

    #region Instanced Methods

    // Instead of storing Outcome in the grid, can return it as a return code.
    public OutcomeType? CheckOutcome(Tuple<int, int> coords, Player player)
    {
        //Check for draws first
        if (cells.GetEmptyCells().Length == 0)
        {
            //Outcome = OutcomeType.Draw;
            //return true;
            return OutcomeType.Draw;
        }

        var corners = new Tuple<int, int>[] { Tuple.Create(0, 0), Tuple.Create(2, 0), Tuple.Create(0, 2), Tuple.Create(2, 2) };
        var sides = new Tuple<int, int>[] { Tuple.Create(1, 0), Tuple.Create(0, 1), Tuple.Create(1, 2), Tuple.Create(2, 1) };
        var middle = new Tuple<int, int>(1, 1);
        var checkDiagonals = false;

        if (corners.Any(e => e.Equals(coords)) || middle.Equals(coords)) { checkDiagonals = true; }

        if (player.PlayerWon(cells[coords.Item1, coords.Item2], this, checkDiagonals))
        {
            switch (player.marker)
            {
                case Mark.Cross:
                    //Outcome = OutcomeType.CrossWin;
                    //break;
                    return OutcomeType.CrossWin;
                case Mark.Nought:
                    //Outcome = OutcomeType.NoughtWin;
                    //break;
                    return OutcomeType.NoughtWin;
                default:
                    throw new NotImplementedException();
            }

            //return true;
        }

        //If execution reaches this point then no one has won, return null
        return null;
    }

    #endregion
}