Back to For Beginners

My first Win32 Game

For Beginners

Started by Robin04 August 06, 2014 03:16 PM

3 comments, last by Robin04 9 years, 8 months ago

Robin04

142

Author

August 06, 2014 03:16 PM

Hello Guys!

I'v just finished my first Win32 game in Visual Studio 2013 (C++). This a TicTacToe game that I'm very proud of. During the developement I tried to keep the code clean and well-structured. What do you think about it? Can you give me some advice how can I make it better?

Player.h


#ifndef PLAYER_H
#define PLAYER_H

class Player
{
public:
	Player();
	~Player();

	void AddScore(int score = 1);
	int GetScore();

private:
	int mScore;
};

#endif

Player.cpp


#include "Player.h"

Player::Player()
{
	mScore = 0;
}

Player::~Player() {}

void Player::AddScore(int score) 
{ 
	mScore += score;
}

int Player::GetScore()
{
	return mScore;
}

Field.h


#ifndef FIELD_H
#define FIELD_H

#include <Windows.h>

#define FC_X 0 //FieldContent is 'X'
#define FC_O 1 //FieldContent is 'O'
#define FC_EMPTY 3 //Field has no content yet

class Field
{
public:
	Field();
	~Field();

	int GetSign();
	void SetSign(int sign);
	void GetCornerPoints(POINT& p1, POINT& p2); //coordinates of the corners of the field (to check mouse click and draw signs)
	void SetCornerPoints(POINT p1, POINT p2);

private:
	int mSign;
	POINT mP1;
	POINT mP2;
};

#endif

Field.cpp


#include "Field.h"

Field::Field() {}
Field::~Field() {}

int Field::GetSign()
{
	return mSign;
}

void Field::SetSign(int sign)
{
	mSign = sign;
}

void Field::GetCornerPoints(POINT& p1, POINT& p2)
{
	p1 = mP1;
	p2 = mP2;
}

void Field::SetCornerPoints(POINT p1, POINT p2)
{
	mP1 = p1;
	mP2 = p2;
}

Board.h


#ifndef BOARD_H
#define BOARD_H

#include "Field.h"
#include "Player.h"
#include <string>
#include <sstream>
using namespace std;

class Board
{
public:
	Board();
	~Board();

	void DrawBoard(HDC hdc, Player player[]); //Function to draw the entire gameboard
	void SetFieldSign(int field_id, int sign);
	void ResetFields();
	int GetFieldSign(int field_id);
	int GetClickedFieldID(int click_point_x, int click_point_y);

private:
	Field Fields[9];
	HPEN pen;
	HBRUSH brush;
};

#endif

Board.cpp


#include "Board.h"

Board::Board()
{
	POINT p1;
	POINT p2;

	ResetFields();

	//Setting the corner points of the X and O signs to draw
	p1.x = 0;
	p1.y = 50;
	p2.x = 200;
	p2.y =  250;
	Fields[0].SetCornerPoints(p1, p2);

	p1.x = 200;
	p1.y = 50;
	p2.x = 400;
	p2.y = 250;
	Fields[1].SetCornerPoints(p1, p2);

	p1.x = 400;
	p1.y = 50;
	p2.x = 600;
	p2.y = 250;
	Fields[2].SetCornerPoints(p1, p2);

	p1.x = 0;
	p1.y = 250;
	p2.x = 200;
	p2.y = 450;
	Fields[3].SetCornerPoints(p1, p2);

	p1.x = 200;
	p1.y = 250;
	p2.x = 400;
	p2.y = 450;
	Fields[4].SetCornerPoints(p1, p2);

	p1.x = 400;
	p1.y = 250;
	p2.x = 600;
	p2.y = 450;
	Fields[5].SetCornerPoints(p1, p2);

	p1.x = 0;
	p1.y = 450;
	p2.x = 200;
	p2.y = 650;
	Fields[6].SetCornerPoints(p1, p2);

	p1.x = 200;
	p1.y = 450;
	p2.x = 400;
	p2.y = 650;
	Fields[7].SetCornerPoints(p1, p2);

	p1.x = 400;
	p1.y = 450;
	p2.x = 600;
	p2.y = 650;
	Fields[8].SetCornerPoints(p1, p2);
}

Board::~Board()
{
	DeleteObject(pen);
	DeleteObject(brush);
}

void Board::DrawBoard(HDC hdc, Player player[])
{
	POINT PenWidth;
	PenWidth.x = 10;
	PenWidth.y = 10;

	POINT d_pt1; //draw point
	POINT d_pt2;

	LOGPEN lp;
	LOGBRUSH lb; //for the score lane at the top

	//Score Display
	lp.lopnStyle = BS_SOLID;
	lp.lopnWidth = PenWidth;

	lb.lbStyle = BS_SOLID;

	lb.lbColor = RGB(255, 0, 0); // red
	brush = CreateBrushIndirect(&lb);
	SelectObject(hdc, brush);
	Rectangle(hdc, 0, 0, 200, 50);

	lb.lbColor = RGB(160, 160, 160); //grey
	brush = CreateBrushIndirect(&lb);
	SelectObject(hdc, brush);
	Rectangle(hdc, 200, 0, 400, 50);

	lb.lbColor = RGB(0, 0, 255); //blue
	brush = CreateBrushIndirect(&lb);
	SelectObject(hdc, brush);
	Rectangle(hdc, 400, 0, 600, 50);

	lb.lbStyle = BS_NULL; //disable brush to draw the signs
	brush = CreateBrushIndirect(&lb);
	SelectObject(hdc, brush);

	//Putting the scores into a string
	//Source: http://www.cplusplus.com/articles/D9j2Nwbp/
	string Player1 = static_cast<ostringstream*>(&(ostringstream() << player[0].GetScore()))->str();
	string Player2 = static_cast<ostringstream*>(&(ostringstream() << player[1].GetScore()))->str();
	string Tie = static_cast<ostringstream*>(&(ostringstream() << player[2].GetScore()))->str();

	int x[3] = { 90, 290, 490 }; //x coordinates to range back the scores if they are more than 10 or 100
	if (player[0].GetScore() >= 10) x[0] = 75;
	if (player[2].GetScore() >= 10) x[1] = 275;
	if (player[1].GetScore() >= 10) x[2] = 470;

	if (player[0].GetScore() >= 100) x[0] = 65;
	if (player[2].GetScore() >= 100) x[1] = 265;
	if (player[1].GetScore() >= 100) x[2] = 460;

	HFONT hFont = CreateFont(40, 0, 0, 0, FW_DONTCARE, 0, 0, 0, DEFAULT_CHARSET, OUT_OUTLINE_PRECIS, CLIP_DEFAULT_PRECIS, CLEARTYPE_QUALITY, VARIABLE_PITCH, TEXT("Verdana"));
	SelectObject(hdc, hFont);

	SetBkColor(hdc, RGB(255, 0, 0)); //red
	TextOutA(hdc, x[0], 3, Player1.c_str(), Player1.size());

	SetBkColor(hdc, RGB(160, 160, 160)); //grey
	TextOutA(hdc, x[1], 3, Tie.c_str(), Tie.size());

	SetBkColor(hdc, RGB(0, 0, 255)); //blue
	TextOutA(hdc, x[2], 3, Player2.c_str(), Player2.size());

	
	//Drawing the signs 
	//Adding and substracting 30 from the coordinates because I don't want to draw the shapes right from the corners
	for (int i = 0; i < 9; i++)
	{
		if (Fields[i].GetSign() == FC_O)
		{
			lp.lopnColor = RGB(255, 0, 0); //red

			pen = CreatePenIndirect(&lp);
			SelectObject(hdc, pen);

			Fields[i].GetCornerPoints(d_pt1, d_pt2);
			d_pt1.x += 30;
			d_pt1.y += 30;
			d_pt2.x -= 30;
			d_pt2.y -= 30;
			Ellipse(hdc, d_pt1.x, d_pt1.y, d_pt2.x, d_pt2.y);
		}
		else if (Fields[i].GetSign() == FC_X)
		{
			lp.lopnColor = RGB(0, 0, 255); //blue

			pen = CreatePenIndirect(&lp);
			SelectObject(hdc, pen);

			Fields[i].GetCornerPoints(d_pt1, d_pt2);
			d_pt1.x += 30;
			d_pt1.y += 30;
			d_pt2.x -= 30;
			d_pt2.y -= 30;

			MoveToEx(hdc, d_pt1.x, d_pt1.y, 0); //one line of X
			LineTo(hdc, d_pt2.x, d_pt2.y);

			MoveToEx(hdc, d_pt1.x + 140, d_pt1.y, 0); //other line of X
			LineTo(hdc, d_pt2.x - 140 , d_pt2.y);
		}
	}

	//Drawing the lines of the board

	PenWidth.x = 5;
	PenWidth.y = 5;

	lp.lopnStyle = BS_SOLID;
	lp.lopnWidth = PenWidth;
	lp.lopnColor = RGB(0, 0, 0); //black

	pen = CreatePenIndirect(&lp);
	SelectObject(hdc, pen);

	//Vertical lines
	MoveToEx(hdc, 200, 0, 0);
	LineTo(hdc, 200, 650);

	MoveToEx(hdc, 400, 0, 0);
	LineTo(hdc, 400, 650);

	//Horizontal lines
	MoveToEx(hdc, 0, 50, 0);
	LineTo(hdc, 600, 50);

	MoveToEx(hdc, 0, 250, 0);
	LineTo(hdc, 600, 250);

	MoveToEx(hdc, 0, 450, 0);
	LineTo(hdc, 600, 450);
}

void Board::SetFieldSign(int field_id, int sign)
{
	if (sign == FC_O)		Fields[field_id].SetSign(sign);
	if (sign == FC_X)		Fields[field_id].SetSign(sign);
	if (sign == FC_EMPTY)	Fields[field_id].SetSign(sign);
}

void Board::ResetFields()
{
	for (int i = 0; i < 9; i++) Fields[i].SetSign(FC_EMPTY);
}

int Board::GetFieldSign(int field_id)
{
	return Fields[field_id].GetSign();
}

int Board::GetClickedFieldID(int click_point_x, int click_point_y)
{
	POINT p1;
	POINT p2;
	for (int i = 0; i < 9; i++)
	{
		Fields[i].GetCornerPoints(p1, p2);
		if (click_point_x > p1.x && click_point_x < p2.x &&
			click_point_y > p1.y && click_point_y < p2.y)
		{
			return i;
		}
	}
	return -1;
}

Game.h


#ifndef GAME_H
#define GAME_H

#include "Board.h"
#include <Windows.h>
#include "resource.h"

class Game
{
public:
	Game();
	~Game();

	int CheckWin(Board gb);
	void SwitchActivePlayer(HCURSOR& cursorO, HCURSOR& cursorX);
	int GetActivePlayer();
	void IncrementStep();
	void ResetSteps();
	int GetSteps();

private:
	int mStep; //number of steps made by players
	int mActivePlayer; //variable to indicate which player has to move
};

#endif

Game.cpp


#include "Game.h"

Game::Game()
{
	mStep = 0;
	mActivePlayer = 0;
}
Game::~Game() {}

int Game::CheckWin(Board gb)
{
		//First vertical column
		if (gb.GetFieldSign(0) == gb.GetFieldSign(3) && gb.GetFieldSign(3) == gb.GetFieldSign(6))
		{
			if (gb.GetFieldSign(0) == FC_X)
				return 1;

			if (gb.GetFieldSign(0) == FC_O)
				return 0;
		}
		//Second vertical column
		if (gb.GetFieldSign(1) == gb.GetFieldSign(4) && gb.GetFieldSign(4) == gb.GetFieldSign(7))
		{
			if (gb.GetFieldSign(1) == FC_X)
				return 1;

			if (gb.GetFieldSign(1) == FC_O)
				return 0;
		}
		//Third vertical column
		if (gb.GetFieldSign(2) == gb.GetFieldSign(5) && gb.GetFieldSign(5) == gb.GetFieldSign(8))
		{
			if (gb.GetFieldSign(2) == FC_X)
				return 1;

			if (gb.GetFieldSign(2) == FC_O)
				return 0;
		}
		//First row
		if (gb.GetFieldSign(0) == gb.GetFieldSign(1) && gb.GetFieldSign(1) == gb.GetFieldSign(2))
		{
			if (gb.GetFieldSign(0) == FC_X)
				return 1;

			if (gb.GetFieldSign(0) == FC_O)
				return 0;
		}
		//Second row
		if (gb.GetFieldSign(3) == gb.GetFieldSign(4) && gb.GetFieldSign(4) == gb.GetFieldSign(5))
		{
			if (gb.GetFieldSign(3) == FC_X)
				return 1;

			if (gb.GetFieldSign(3) == FC_O)
				return 0;
		}
		//Third row
		if (gb.GetFieldSign(6) == gb.GetFieldSign(7) && gb.GetFieldSign(7) == gb.GetFieldSign(8))
		{
			if (gb.GetFieldSign(6) == FC_X)
				return 1;

			if (gb.GetFieldSign(6) == FC_O)
				return 0;
		}
		//top left - bottom right diagonal
		if (gb.GetFieldSign(0) == gb.GetFieldSign(4) && gb.GetFieldSign(4) == gb.GetFieldSign(8))
		{
			if (gb.GetFieldSign(0) == FC_X)
				return 1;

			if (gb.GetFieldSign(0) == FC_O)
				return 0;
		}
		//top right - bottom left diagonal
		if (gb.GetFieldSign(2) == gb.GetFieldSign(4) && gb.GetFieldSign(4) == gb.GetFieldSign(6))
		{
			if (gb.GetFieldSign(2) == FC_X)
				return 1;

			if (gb.GetFieldSign(2) == FC_O)
				return 0;
		}
	
	 if (mStep == 9) //if all field are full and no winner yet 
		return 2; //Tie
	 else
		return -2; //No winner yet
}

void Game::SwitchActivePlayer(HCURSOR& cursorO, HCURSOR& cursorX)
{
	if (mActivePlayer == 0)
	{
		mActivePlayer = 1;
		SetCursor(cursorX);
	}
		
	else
	{
		mActivePlayer = 0;	
		SetCursor(cursorO);
	}
}

int Game::GetActivePlayer()
{
	return mActivePlayer;
}

void Game::IncrementStep()
{
	mStep++;
}

void Game::ResetSteps()
{
	mStep = 0;
}

int Game::GetSteps()
{
	return mStep;
}

resource.h


//{{NO_DEPENDENCIES}}
// Microsoft Visual C++ generated include file.
// Used by TicTacToe.rc
//
#define IDC_CURSOR1                     101
#define IDC_CURSOR2                     102

// Next default values for new objects
// 
#ifdef APSTUDIO_INVOKED
#ifndef APSTUDIO_READONLY_SYMBOLS
#define _APS_NEXT_RESOURCE_VALUE        103
#define _APS_NEXT_COMMAND_VALUE         40001
#define _APS_NEXT_CONTROL_VALUE         1001
#define _APS_NEXT_SYMED_VALUE           101
#endif
#endif

WinMain.cpp


#include <Windows.h>
#include "Board.h"
#include "Field.h"
#include "Game.h"
#include "Player.h"
#include "resource.h"
//#define DEBUG //uncomment this, if you want to use the console window to write out variables or whatever you want

HWND ghMainWnd		= 0;
HINSTANCE ghAppInst = 0;
HCURSOR hCursorO;
HCURSOR hCursorX;

Board gGameBoard;
Player gPlayer[3]; //The third player[2] is a "ghost player". Only used to store the tie scores
Game gGame;

LRESULT CALLBACK
WndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	HDC hdc = 0;
	PAINTSTRUCT ps;

	switch (msg)
	{
	case WM_LBUTTONDOWN:
#ifdef DEBUG
		printf("ClickID: %i", gGameBoard.GetClickedFieldID(LOWORD(lParam), HIWORD(lParam)));
		printf("\n");
		printf("ActivePlayer: %i", gGame.GetActivePlayer());
		printf("\n");
		for (int i = 0; i < 9; i++)
		{
			printf("Field[%i]: %i", i, gGameBoard.GetFieldSign(i));
			printf("\n");
		}
#endif
		int field_id;
		field_id = gGameBoard.GetClickedFieldID(LOWORD(lParam), HIWORD(lParam));

		if (gGameBoard.GetFieldSign(field_id) == FC_EMPTY)
		{
			if (gGame.GetActivePlayer() == 0)
				gGameBoard.SetFieldSign(field_id, FC_O);

			if (gGame.GetActivePlayer() == 1)
				gGameBoard.SetFieldSign(field_id, FC_X);
			
			gGame.IncrementStep();
			gGame.SwitchActivePlayer(hCursorO, hCursorX);
			InvalidateRect(hWnd, 0, true);

			if (gGame.CheckWin(gGameBoard) != -2)
			{
				switch (gGame.CheckWin(gGameBoard))
				{
					case 0:
					{
						MessageBox(hWnd, L"Red player wins!", L"Winner", MB_OK);
						break;
					}
					case 1:
					{
						MessageBox(hWnd, L"Blue player wins!", L"Winner", MB_OK);
						break;
					}
					case 2:
					{
						MessageBox(hWnd, L"Tie round!", L"Winner", MB_OK);
						break;
					}
				}

				gPlayer[gGame.CheckWin(gGameBoard)].AddScore();
				gGameBoard.ResetFields();
				gGame.ResetSteps();
				InvalidateRect(hWnd, 0, true);
			}
		}
		return 0;

	case WM_PAINT:
		
#ifdef DEBUG
		AllocConsole();
		freopen("CONIN$", "r", stdin);
		freopen("CONOUT$", "w", stdout);
		freopen("CONOUT$", "w", stderr);

		printf("Player[0]: %i\n", gPlayer[0].GetScore());
		printf("Player[1]: %i\n", gPlayer[1].GetScore());
		printf("Player[2]: %i\n", gPlayer[2].GetScore());
#endif

		hdc = BeginPaint(hWnd, &ps);
		gGameBoard.DrawBoard(hdc, gPlayer);
		EndPaint(hWnd, &ps);
		ShowCursor(true);

		return 0;


	case WM_KEYDOWN:
		if (wParam == VK_ESCAPE)
			DestroyWindow(ghMainWnd);
		return 0;

	case WM_DESTROY:
		PostQuitMessage(0);
		return 0;
	}
	return DefWindowProc(hWnd, msg, wParam, lParam);
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrev, PSTR cmdLine, int showCmd)
{
	ghAppInst = hInstance;
	hCursorO = LoadCursor(ghAppInst, MAKEINTRESOURCE(IDC_CURSOR2));
	hCursorX = LoadCursor(ghAppInst, MAKEINTRESOURCE(IDC_CURSOR1));

	WNDCLASS wc;
	wc.style			= CS_HREDRAW | CS_VREDRAW;
	wc.lpfnWndProc		= WndProc;
	wc.cbClsExtra		= 0;
	wc.cbWndExtra		= 0;
	wc.hInstance		= ghAppInst;
	wc.hIcon			= LoadIcon(0, IDI_APPLICATION);
	wc.hCursor			= LoadCursor(ghAppInst, IDC_ICON);
	wc.hbrBackground	= (HBRUSH)GetStockObject(WHITE_BRUSH);
	wc.lpszMenuName		= 0;
	wc.lpszClassName	= L"TicTacToe";

	RegisterClass(&wc);

	ghMainWnd = CreateWindow(wc.lpszClassName, L"Tic Tac Toe", WS_OVERLAPPED | WS_MINIMIZEBOX | WS_SYSMENU, 412, 107, 615, 685, 0, 0, ghAppInst, 0);

	if (!ghMainWnd)
	{
		MessageBox(0, L"The window cannot be created. The application is closing.", L"Error", MB_OK | MB_ICONERROR);
		return 0;
	}

	ShowWindow(ghMainWnd, showCmd);
	UpdateWindow(ghMainWnd);

	MSG msg;
	ZeroMemory(&msg, sizeof(MSG));

	while (GetMessage(&msg, 0, 0, 0))
	{
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}

	return (int)msg.wParam;
}

jpetrie

13,220

August 06, 2014 05:52 PM

Here are some comments:

You are missing some opportunities for const-correctness. For example, your accessor methods (like GetScore) could be declared "int GetScore() const;" since they do not require mutating the invoking object.
You should use constructor initializer lists; instead of setting mScore inside the body of the constructor, do "Player::Player() : mScore(0) { }" This is a good habit to get into because it is faster for some types (and certainly never slower) and required for some other types (such as references).
Use an enumeration for the field content instead of #define macros; in general, enumerations or const variables are preferable for constants. Macros should be used sparingly.
Use consistent naming. If "sign" in GetSign/SetSign refers to the field content (as I suspect it does just from looking at the header), adjust the names so "sign" and the "field content" enumeration are in sync (i.e., use one or the other).
You don't need to provide empty constructors/destructors, they will be generated for you by default (unless you do something to disable that, which are you not in these cases).
A common convention for "output parameters" (the results of GetCornerPoints) is to pass those parameters as pointers. That way, it is clearer at the call site that the parameters are potentially-modified (GetCornerPoints(&p1, &p2)). It also allows you to make some output parameters optional, in the case where the caller may not care about that particular parameter (and thus can pass a null pointer). Using non-const reference parameters is often a bad idea, because it can result in the violation of the principle of least-surprise.
Prior to C++11, "larger" non-POD types (possibly including point types, though not your specific point type) should be passed by const-reference, rather than by value (as in SetCornerPoints). This avoided extra copies. In C++11, move semantics alter this guidance: you can pass by value if you're just going to store, as you do in SetCornerPoints, since the compiler can use a move there. Otherwise, you may still want to consider passing by const-reference.
The corner points of the field are something you could mandate be provided by the constructor of Field; it's probably not a good idea to allow them to be mutated after the field is constructed, since that makes it easy to create nonsense boards. APIs should be easy to use in the correct fashion and hard to use in the incorrect fashion.
The field constructor should at least initialize the field to a good state (set the initial size, zero-out the points if you're not going to require them as constructor parameters).
DO NOT put "using namespace std;" in header files. This pollutes the global namespace for ever more, since you cannot undo a using declaration. Fully-qualify items in headers, or put the appropriate using directives in a tighter scope if necessary.
A tic-tac-toe board is a 2D grid. Consider altering the interface of your board to refer to cells by an row and column index, since that's more natural. You can still internally use a linear array of nine fields by doing some simple math on the row/column indices.
Try to avoid "dumb pass through" APIs when possible. An example of this is Board's GetFieldSign/SetFieldSign methods, which just forward to the field APIs to do the actual work. This technique can create a lot of boilerplate code that's not easy to maintain. Consider instead a design where the board can simply get a reference to a field at a given row and column: Field & GetField(std::size_t row, std::size_t column). Then you can leverage the API that is already present on the field class.
In a project this small, it's not too big of a deal, but in the future consider how you might separate game logic from rendering (your board class conflates them now). Keeping independent, single responsibilities allows for better maintainability and reusability.
Similarly, in a project this small it's not a big deal, but in larger projects consider looking at how you can hide implementation details against things like Windows and GDI. I don't mean stuffing things in the private access section of a class, I mean removing the header references to Windows and GDI entirely (thus preventing your clients from acquiring the build-time dependency of such a header. Things like the "private implementation" or "pimpl" pattern might be worth looking into once your projects get larger.
Use forward declarations when appropriate to avoid unnecessary header dependencies. You can, for example, forward-declare "class Player" in the Board header, and then you can avoid the player header include there (moving it to the .cpp file, where it's better because it won't cause as many files to recompile if it changes).
You can avoid the call to ResetFields in the board constructor if you instead ensure that a field default-constructs itself into a sane state (as above).
A tic-tac-toe board is a grid. You can do the initial computation of all the grid points in a loop, rather than nine individual manual sets.
The naming convention of the board member variables does not match that of the field member variables (no "m" prefix on the former). Consistency is important.
The board drawing function creates new GDI objects every call; this will eventually exhaust GDI handle resources and you'll crash. You should either create/destroy them every draw call, or create them once (in the board constructor), since you're also trying to destroy them in the destructor (which only actually cleans up the last pen and brush.
Your local member variable naming convention is also inconsistent.
There are lots of magic numbers in your drawing method; consider using named constants instead.
This: static_cast<ostringstream*>(&(ostringstream() << player[0].GetScore()))->str(); is horrible. Use std::to_string in C++11, or create a single stringstream on the stack and re-use it to do (stream << score; stream.str()) (or better yet, make it a utility function).
Think about how you can do many of the drawing steps -- the board lines, for example -- in fewer lines of code using loops and other ways to exploit the fact that you have a grid. It will make for fewer magic numbers and fewer places to adjust code when you want to change the overall look of the board.
The if statements in SetFieldSign are unnecessary. Just do: Fields[field_id].SetSign(sign); Check sign for validity prior to that if desired (if you switch to using an enumeration for the sign, it would be clearer).
Consider returning a field pointer out of GetClickedFieldID, so the user doesn't have to go re-lookup the field.
In your Game class, you pass the entire board by value to CheckWin. Pass it by constant reference.
"CheckWin" is a cumbersome name. Consider something like "CheckForWin" instead.
You can increase maintainability by factoring out entire-column and entire-row checking into utility functions (especially easy once you change the board API to do row/column queries). This reduces duplicate code.
You return magic numbers from CheckWin. Use an enumeration to indicate result state.
In SwitchActivePlayer, pass the objects by const reference. Consider a design that does not require passing seemingly-unrelated parameters to a function that sounds like it should just toggle internal state. What you have now produces a bad abstraction.
It's unclear to me why you need the "ghost player."
It does not make sense to call AllocConsole inside WM_PAINT. Move it to an initialization call site.

SeanMiddleditch

17,596

August 06, 2014 10:14 PM

Prior to C++11, "larger" types (like points) should be passed by const-reference, rather than by value (as in SetCornerPoints). This avoided extra copies. In C++11, move semantics alter this guidance: you can pass by value if you're just going to store, as you do in SetCornerPoints, since the compiler can use a move there. Otherwise, you may still want to consider passing by const-reference.

For what is otherwise just POD, move semantics will have no affect on performance. Moving at its absolute fastest is just copying bits which is the same as the copy operation does for POD. The const reference may or may not actually be necessary here even in C++98 since a point is so small (two ints) and with some ABIs using the const reference even in C++98 would be a deoptimization.

Move semantics play a role when you have side effects with the copy like with containers (since you're allocating and then copying all the contained elements every time in copy the container).

Sean Middleditch – Game Systems Engineer – Join my team!

jpetrie

13,220

August 06, 2014 10:48 PM

For what is otherwise just POD, move semantics will have no affect on performance. Moving at its absolute fastest is just copying bits which is the same as the copy operation does for POD. The const reference may or may not actually be necessary here even in C++98 since a point is so small (two ints) and with some ABIs using the const reference even in C++98 would be a deoptimization.

Yeah, good point (I updated that point to try to de-emphasize points specifically). In particular it's noteworthy that on Apple's later iOS platforms, the entire GLKit API passes 4x4 matrices around by value because they are plain data and the hardware/toolchain is capable enough to do the fast thing -- knowing something is passed by value also means you can know it cannot be aliased elsewhere, which can help optimization.

(But points aren't the only thing the OP is passing around by value in the above code.)

Robin04

142

Author

August 08, 2014 10:30 AM

Dear Josh Petrie!

Thank you very much for your comment. I learnt a lot from it. I very appreciate that you spent your free time by studying my code. Now I'm trying to improve the code and add some new features like basic AI (playing with the computer). :)

My first Win32 Game

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

My first Win32 Game

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines