WinAPI help

[vadalaz]

Hey guys, I've recently started learning WinAPI and I want to make a Tic Tac Toe game with it but I have a problem

I have a main window ("field") and 9 child windows ("tiles") and I want to make the tiles clickable so that a click calls the Beep() function. I also want to make each tile able to call Beep() only once (just like you can put a cross only once in the tile in tic tac toe). So I have this code:

#include <windows.h>

LPCWSTR FldClass = L"FieldClass";
LPCWSTR TileClass = L"TileClass";

LRESULT CALLBACK MainProc(HWND, UINT, WPARAM, LPARAM);
LRESULT CALLBACK TileProc(HWND, UINT, WPARAM, LPARAM);
void RegisterTile();

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int nShowCmd)
{
	// Registering the "field" class - the main window
	WNDCLASSEX Field;
	ZeroMemory(&Field, sizeof(Field));
	
	Field.cbSize = sizeof(Field);
	Field.hbrBackground = (HBRUSH)(COLOR_WINDOW);
	Field.hCursor = LoadCursor(NULL, IDC_ARROW);
	Field.hInstance = hInstance;
	Field.lpfnWndProc = MainProc;
	Field.lpszClassName = FldClass;
	Field.style = CS_VREDRAW | CS_HREDRAW;

	RegisterClassEx(&Field);

	// Creating the main window

	HWND FldWnd = CreateWindowEx(0, FldClass, L"Tic-Tac-Toe!", WS_OVERLAPPEDWINDOW,
		150, 150, 400, 420, 0, 0, hInstance, 0);
	ShowWindow(FldWnd, nShowCmd);
	UpdateWindow(FldWnd);

	// Message loop

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

	return msg.wParam;
}

// The Field Procedure

LRESULT CALLBACK MainProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) 
{
	switch(msg)
	{
	
	case WM_CREATE:
		HWND Tiles[9];
		RegisterTile();

		//creating the tiles

		Tiles[0] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE, 
			20, 20, 100, 100, hwnd, (HMENU) 1, 0, 0);
		Tiles[1] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			140, 20, 100, 100, hwnd, (HMENU) 2, 0, 0);
		Tiles[2] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			260, 20, 100, 100, hwnd, (HMENU) 3, 0, 0);
		Tiles[3] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			20, 140, 100, 100, hwnd, (HMENU) 4, 0, 0);
		Tiles[4] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			140, 140, 100, 100, hwnd, (HMENU) 5, 0, 0);
		Tiles[5] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			260, 140, 100, 100, hwnd, (HMENU) 6, 0, 0);
		Tiles[6] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			20, 260, 100, 100, hwnd, (HMENU) 7, 0, 0);
		Tiles[7] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			140, 260, 100, 100, hwnd, (HMENU) 8, 0, 0);
		Tiles[8] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
			260, 260, 100, 100, hwnd, (HMENU) 9, 0, 0);

		break;

	case WM_DESTROY:
		PostQuitMessage(0);
		break;
	
	case WM_CLOSE:
		DestroyWindow(hwnd);
		break;
	
	default:
		return DefWindowProc(hwnd, msg, wParam, lParam);
	}
	return 0;
}

// Function to register the tile class
void RegisterTile() 
{
	WNDCLASSEX Tile = {0};
	HBRUSH hBrush = CreateSolidBrush( RGB(100, 100, 100) );

	Tile.cbSize = sizeof(Tile);
	Tile.lpszClassName = TileClass;
	Tile.hbrBackground = hBrush;
	Tile.lpfnWndProc = TileProc;
	Tile.hCursor = LoadCursor(0, IDC_HAND);

	RegisterClassEx(&Tile);
}

//The tile procedure
LRESULT CALLBACK TileProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	static bool empty = true;
	switch(msg)
	{
		case WM_LBUTTONUP:
		{
			if (empty) 
			{
				Beep(220, 300);
				empty = false;
			}
			break;
		}
        }
	return DefWindowProc(hwnd, msg, wParam, lParam);
}

Unfortunately the 'empty' variable affects all the tiles so after clicking one of them for the first time, none of them work will call Beep() anymore. How do I associate the 'empty' variable and the tile window itself?

[thelamb]

First of all: Do you understand why there is only one beep and after that never again?

Secondly, let's think about your problem. You have an X number of objects(windows) and you want them to perform an action only once. So for each of these objects you can store a 'state' whether they have already performed the action or not.

Knowing this, what parameter/variable is unique for every object? This is a parameter that you can associate with the state variable.

In the end it comes down to storing the unique_object_identifier + state and searching for the unique_object_identifier in the TitleProc, to get the state and based on the state perform the action or not.

[vadalaz]

Thanks a lot, managed to get it working now like this:

// tile information holds the menu parameter and the state
struct TileInfo
{
	HMENU menu;
	bool empty;
};

TileInfo tInfo[9] = { 
{(HMENU)1, true}, {(HMENU)2, true}, {(HMENU)3, true}, 
{(HMENU)4, true}, {(HMENU)5, true}, {(HMENU)6, true}, 
{(HMENU)7, true}, {(HMENU)8, true}, {(HMENU)9, true}
};
//...
LRESULT CALLBACK TileProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	TileInfo current_tile;
	current_tile.menu = GetMenu(hwnd);

	switch(msg)
	{
		case WM_LBUTTONUP:
		{
			int index;
			for (int i = 0; i < 9; i++)
			{
				if (current_tile.menu == tInfo[i].menu)
				{
					index = i;
					current_tile.empty = tInfo[index].empty;
				}
			}

			if (current_tile.empty)
			{
				Beep(440, 400);
				tInfo[index].empty = false;
			}
			break;
		}
    }
	return DefWindowProc(hwnd, msg, wParam, lParam);
}

[thelamb]

Good job, you got my point.

But there are ways to improve your code. Firstly: you are basically writing C code, don't you want to use C++ constructs?

Secondly, 'hard-coding' the 1 though 9 values for HMENU should ring some alarm bells. What if you want to have more than 9 windows later? You'll need to modify the tInfo struct, and find the for( int i = 0 i < 9 .. ) loop again to change the 9. That works fine for this example, but my guess is that you don't want to stop coding after this and move on to bigger projects ;).

Can you make it dynamically fill the tInfo structure? And if you have nothing against C++ constructs, maybe use some kind of container to simplify the code?

Edit:
Also, the 'creating the tiles' portion can be improved. Think about doing it in some kind of loop, there are only a few parameters that are changing and they change in a fairly constant way, this will also make it much easier to update the tInfo structure, rather than writing the update code 9 times.

[vadalaz]

Haha surely I won't stop coding after this, this is just the beginning. :)

Anyways I added a global constant (const int sz = 9) two small loops to improve it:

// Initializing tile info
for (int i = 0; i < sz; i++)
{
	tInfo[i].empty = true;
	tInfo[i].menu = (HMENU)(i+1);
}
//...
//Creating the tiles

for (int i = 0; i < sz; i++)
{
	Tiles[i] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
	(20 + 120*(i%3)), (20 + 120*(i/3)), 100, 100, hwnd, (HMENU)(i+1), 0, 0);
}

And which parts of the code could I simplify with the C++ constructs? Should I use vectors instead of arrays here?

[thelamb]

Well done, I'm impressed that you straight got the use of % to do your 'creating the tiles loop' ;).

In C++ it is rarely necessary to work with arrays directly, it is very error-prone and mostly even confusing. So unless you're doing some low-level stuff where it is necessary(either because you don't have a choice or because the code is performance-critical) stick to C++ containers like vector etc.

It will make your code much more maintainable, as you don't have to make any assumption about the size of tInfo (vector takes care of allocating extra memory internally).

I do have one question about your last code snippet though:
Looking at just the 14 lines that you pasted, what can be a simple improvement to your code that both benefits speed and 'lines-of-code'?

[vadalaz]

Well if you mean I should do it in one loop instead of two, where should I place that loop then? Because atm the tInfo loop is in WinMain and the tiles creating loop is in the MainProc's WM_CREATE case... I tried putting them both to MainProc but then I was getting runtime errors

Should I put them both in WinMain and just erase the WM_CREATE case?

Edit: yeah I tried that and it worked :)

[thelamb]

Good, then that makes it time to move away from the dirty C-arrays and use proper C++ classes. Show us how far you get ;)

[vadalaz]

Sure, here's what I got now... it's actually tic-tac-toe now and it works.

#include <vector>
#include <windows.h>

// Global constants and forward declarations
const int sz = 9;
LPCWSTR FldClass = L"FieldClass";
LPCWSTR TileClass = L"TileClass";
LRESULT CALLBACK MainProc(HWND, UINT, WPARAM, LPARAM);
LRESULT CALLBACK TileProc(HWND, UINT, WPARAM, LPARAM);
void RegisterTile();
void DrawCross(HWND, RECT);
void DrawNought(HWND, RECT);
void CleanUp(std::vector<HWND>);

// Structure to hold information about the game status
struct GameInfo
{
	GameInfo() :crosses_turn(true) {};
	bool crosses_turn;
	bool crosses_win();
	bool noughts_win();
	bool draw();
};

// Structure to hold information about the tiles
struct TileInfo
{
	TileInfo() :empty(true), cross(false), nought(false) {};
	HMENU menu;
	bool empty;
	bool cross;
	bool nought;
};

std::vector<TileInfo>tInfo(sz);
GameInfo game_Info;

//ugly stuff here
bool GameInfo::crosses_win()
{
	return ( (tInfo[0].cross && tInfo[1].cross && tInfo[2].cross)
		|| (tInfo[3].cross && tInfo[4].cross && tInfo[5].cross)
		|| (tInfo[6].cross && tInfo[7].cross && tInfo[8].cross)
		
		|| (tInfo[0].cross && tInfo[3].cross && tInfo[6].cross)
		|| (tInfo[1].cross && tInfo[4].cross && tInfo[7].cross)
		|| (tInfo[2].cross && tInfo[5].cross && tInfo[8].cross)

		|| (tInfo[0].cross && tInfo[4].cross && tInfo[8].cross)
		|| (tInfo[6].cross && tInfo[4].cross && tInfo[2].cross));
}

bool GameInfo::noughts_win()
{
	return ( (tInfo[0].nought && tInfo[1].nought && tInfo[2].nought)
		|| (tInfo[3].nought && tInfo[4].nought && tInfo[5].nought)
		|| (tInfo[6].nought && tInfo[7].nought && tInfo[8].nought)
		
		|| (tInfo[0].nought && tInfo[3].nought && tInfo[6].nought)
		|| (tInfo[1].nought && tInfo[4].nought && tInfo[7].nought)
		|| (tInfo[2].nought && tInfo[5].nought && tInfo[8].nought)

		|| (tInfo[0].nought && tInfo[4].nought && tInfo[8].nought)
		|| (tInfo[6].nought && tInfo[4].nought && tInfo[2].nought));
}

bool GameInfo::draw()	
{
	return ( !(noughts_win()) && !(crosses_win()) && !tInfo[0].empty 
		&& !tInfo[1].empty && !tInfo[2].empty && !tInfo[3].empty 
		&& !tInfo[4].empty && !tInfo[5].empty && !tInfo[6].empty 
		&& !tInfo[7].empty && !tInfo[8].empty );
}

// Function to reset tiles info and the tile windows
void CleanUp(std::vector<HWND>TilesArray)
{
	for (int i = 0; i < sz; i++)
	{
		tInfo[i].cross = false;
		tInfo[i].nought = false;
		tInfo[i].empty = true;
		RECT rect;
		GetClientRect(TilesArray[i], &rect);
		InvalidateRect(TilesArray[i], &rect, TRUE);
	}
}

// The main function
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int nShowCmd)
{	
	std::vector<HWND>Tiles(sz);

	// Registering the "field" class - the main window
	WNDCLASSEX Field;
	ZeroMemory(&Field, sizeof(Field));
	
	Field.cbSize = sizeof(Field);
	Field.hbrBackground = (HBRUSH)(COLOR_WINDOW);
	Field.hCursor = LoadCursor(NULL, IDC_ARROW);
	Field.hInstance = hInstance;
	Field.lpfnWndProc = MainProc;
	Field.lpszClassName = FldClass;
	Field.style = CS_VREDRAW | CS_HREDRAW;

	RegisterClassEx(&Field);

	// Creating the main window

	HWND FldWnd = CreateWindowEx(0, FldClass, L"Tic-Tac-Toe", WS_OVERLAPPEDWINDOW,
		150, 150, 400, 420, 0, 0, hInstance, 0);
	ShowWindow(FldWnd, nShowCmd);
	UpdateWindow(FldWnd);

	// Initializing tile info and creating the tiles
	RegisterTile();

	for (int i = 0; i < sz; i++)
	{
		tInfo[i].menu = (HMENU)(i+1);
		Tiles[i] = CreateWindowEx(0, TileClass, NULL, WS_CHILD | WS_VISIBLE,
				(20 + 120*(i%3)), (20 + 120*(i/3)), 100, 100, FldWnd, (HMENU)(i+1), 0, 0);
	}

	// Message loop
	MSG msg;
	while ( GetMessage(&msg, FldWnd, 0, 0) > 0 )
	{
		TranslateMessage(&msg);
		DispatchMessage(&msg);
		if ( game_Info.noughts_win() )
		{
			MessageBox(FldWnd, L"Noughts won", L"Congratulations", MB_OK | MB_ICONINFORMATION);
			CleanUp(Tiles);
		}
		if ( game_Info.crosses_win() ) 
		{
			MessageBox(FldWnd, L"Crosses won", L"Congratulations", MB_OK | MB_ICONINFORMATION);
			CleanUp(Tiles);
		}
		if ( game_Info.draw() ) 
		{
			MessageBox(FldWnd, L"It's a tie!", L"Yay!", MB_OK | MB_ICONINFORMATION);
			CleanUp(Tiles);
		}
	}

	return msg.wParam;
}

// The Field Procedure

LRESULT CALLBACK MainProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) 
{
	switch(msg)
	{
	case WM_DESTROY:
		PostQuitMessage(0);
		break;
	
	case WM_CLOSE:
		DestroyWindow(hwnd);
		break;
	
	default:
		return DefWindowProc(hwnd, msg, wParam, lParam);
	}
	return 0;
}

// Function to register the tile class
void RegisterTile() 
{
	WNDCLASSEX Tile = {0};
	HBRUSH hBrush = CreateSolidBrush( RGB(100, 100, 100) );

	Tile.cbSize = sizeof(Tile);
	Tile.lpszClassName = TileClass;
	Tile.hbrBackground = hBrush;
	Tile.lpfnWndProc = TileProc;
	Tile.hCursor = LoadCursor(0, IDC_HAND);

	RegisterClassEx(&Tile);
}
// Function that draws a cross
void DrawCross(HWND hwnd, RECT tile_rect)
{
	HDC hDC = GetDC(hwnd);
	GetClientRect(hwnd, &tile_rect);
	HBRUSH hbrush = CreateSolidBrush( RGB(40, 40, 120) );
	FillRect(hDC, &tile_rect, hbrush);
	DeleteObject(hbrush);

	HPEN yellowpen = CreatePen(PS_SOLID, 5, RGB(250, 250, 20));
	SelectObject(hDC, yellowpen);
				
	POINT mainDiag[2];
	mainDiag[0].x = tile_rect.left + 15;
	mainDiag[0].y = tile_rect.top + 15;
	mainDiag[1].x = tile_rect.right - 15;
	mainDiag[1].y = tile_rect.bottom - 15;

	POINT minorDiag[2];
	minorDiag[0].x = tile_rect.right - 15;
	minorDiag[0].y = tile_rect.top + 15;
	minorDiag[1].x = tile_rect.left + 15;
	minorDiag[1].y = tile_rect.bottom - 15;

	Polyline(hDC, mainDiag, 2);
	Polyline(hDC, minorDiag, 2);

	DeleteObject(yellowpen);
	ReleaseDC(hwnd, hDC);
}

// Function that draws a nought
void DrawNought(HWND hwnd, RECT tile_rect)
{
	HDC hDC = GetDC(hwnd);
	GetClientRect(hwnd, &tile_rect);
	HBRUSH hbrush = CreateSolidBrush( RGB(120, 40, 40) );
	FillRect(hDC, &tile_rect, hbrush);
	DeleteObject(hbrush);

	HPEN yellowpen = CreatePen(PS_SOLID, 5, RGB(250, 250, 20));
	SelectObject(hDC, yellowpen);

	Arc(hDC, (tile_rect.left + 15), (tile_rect.top + 15), (tile_rect.right - 15), (tile_rect.bottom - 15), 0, 0, 0, 0);

	DeleteObject(yellowpen);
	ReleaseDC(hwnd, hDC);
}

//The tile procedure
LRESULT CALLBACK TileProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	TileInfo current_tile;
	current_tile.menu = GetMenu(hwnd);
	RECT tile_rect = {0};

	switch(msg)
	{
		case WM_LBUTTONUP:
		{
			int index;
			for (int i = 0; i < sz; i++)
			{
				if (current_tile.menu == tInfo[i].menu)
				{
					index = i;
					current_tile.empty = tInfo[index].empty;
				}
			}

			if (current_tile.empty)
			{
				if (game_Info.crosses_turn)
				{
					DrawCross(hwnd, tile_rect);
					tInfo[index].empty = false;
					tInfo[index].cross = true;
					game_Info.crosses_turn = false;
				}
				else
				{
					DrawNought(hwnd, tile_rect);
					tInfo[index].empty = false;
					tInfo[index].nought = true;
					game_Info.crosses_turn = true;
				}
			}
			break;
		}
	}
	return DefWindowProc(hwnd, msg, wParam, lParam);
}

Any comments? :)
I really hate the GameInfo functions, they're crying for a loop or something but it doesn't really seem like a loop is gonna help much in this particular task as there are only 9 windows anyway...

[thelamb]

Very well done, it's looking good :D.

The 'win checks' is a tricky one... I'm sure there is some standard way to do this but I've never had to do something simmilar. Maybe think of the problem differently:

Instead of checking the entire field every time, you could move the check to the function where you draw a new object, you know in which position the new object is drawn and the type of the object. So here you can check if the same object is anywhere near the new position, if it is check if there is a 3rd one in the correct position next to that.
The code might end up more unsexy but that's about the only thing I can come up with after my first coffee.

Atleast it will save you from calling all 3 win check functions, which shouldn't be necessary as crosses can only win when it's crosses turn, and you only have to check for a draw when all tiles are full.