Click here to Skip to main content
15,891,840 members
Please Sign up or sign in to vote.
5.00/5 (1 vote)
See more:
Hi. Im a novice to MFC programming. And I was just writing some simple tic tac toe program. My codes are shown below. It only draws X in a square where "l button down" messages occur. But mysteriously it fails to draw Xs in all the squares. If I click in all the squares, it only draws Xs for like 3 or 4 times. After that it stops working even though I click on a square. No errors. No beep message beep sounds. It just stops. But I cant understand where the error is.

Heres my code in header file.

C++
<blockquote class="quote"><div class="op">Quote:</div>

class MainWindow : public CFrameWnd
{
public:
	MainWindow()
	{
		Create(NULL, L"WINDOW");
		
	}




protected:
	afx_msg void OnPaint();

	afx_msg void OnLButtonDown(UINT, CPoint);

private:
	void DrawGrid(CDC*);

	void DrawO(CDC*, int);  //Ignore this

	void DrawX(CDC*, int);


	DECLARE_MESSAGE_MAP()
};



class App : public CWinApp
{
public:
	BOOL InitInstance()
	{
		m_pMainWnd = new MainWindow;
		m_pMainWnd->ShowWindow(m_nCmdShow);
		m_pMainWnd->UpdateWindow();

		CRect rec(0, 0, 300, 300);
		m_pMainWnd->CalcWindowRect(rec);
		m_pMainWnd->SetWindowPos(NULL, 0, 0, rec.Width(), rec.Height(), SWP_NOZORDER | SWP_NOMOVE | SWP_NOREDRAW);
		
		return TRUE;
	}
};</blockquote>


And here is the cpp file.

C++
#include "stdafx.h"
#include "MFCApplication15.h"

#define O 1
#define X 2


CRect sqs[9] =
{
	{ 0, 0, 100, 100 },
	{ 100, 0, 200, 100 },
	{ 200, 0, 300, 100 },

	{ 0, 100, 100, 200 },
	{ 100, 100, 200, 200 },
	{ 200, 100, 300, 200 },

	{ 0, 200, 100, 300 },
	{ 100, 200, 200, 300 },
	{ 200, 200, 300, 300 }
};

int vals[9]{0, 0, 0, 0, 0, 0, 0, 0, 0};

void MainWindow::OnPaint()
{
	CClientDC dc(this);
	DrawGrid(&dc);

	for (int i{}; i < 9; i++)
		if (vals[i] == X)
			DrawX(&dc, i);
}

void MainWindow::OnLButtonDown(UINT flags, CPoint cp)
{
	for (int i{}; i < 9; i++)
	{
		if (PtInRect(sqs[i], cp))
			vals[i] = X;
	}
}

void MainWindow::DrawGrid(CDC* pDC)
{
	CPen *pen = new CPen(PS_SOLID, 5, RGB(0, 0, 0));
	pDC->SelectObject(pen);

	pDC->MoveTo(0, 100);
	pDC->LineTo(300, 100);
	pDC->MoveTo(0, 200);
	pDC->LineTo(300, 200);

	pDC->MoveTo(100, 0);
	pDC->LineTo(100, 300);
	pDC->MoveTo(200, 0);
	pDC->LineTo(200, 300);
}


void MainWindow::DrawO(CDC* pDC, int pos)  //Ignore this
{
	CPen* pen = new CPen(PS_SOLID, 3, RGB(255, 0, 0));

	pDC->SelectObject(pen);

	pDC->Ellipse(sqs[pos]);
}

void MainWindow::DrawX(CDC* pDC, int pos)
{
	CPen* pen = new CPen(PS_SOLID, 3, RGB(0, 0, 255));
	pDC->SelectObject(pen);

	pDC->MoveTo(sqs[pos].left, sqs[pos].top);
	pDC->LineTo(sqs[pos].right, sqs[pos].bottom);
	pDC->MoveTo(sqs[pos].right, sqs[pos].top);
	pDC->LineTo(sqs[pos].left, sqs[pos].bottom);
}

App myApp;

BEGIN_MESSAGE_MAP(MainWindow, CFrameWnd)
	ON_WM_PAINT()
	ON_WM_LBUTTONDOWN()
END_MESSAGE_MAP()


What's the wrong with my code ? Why it stops the drawing after like 4 clicks ?
Thanks in advance.
Posted
Comments
Leo Chapiro 6-Jan-15 10:08am    
In this code "for (int i{}; i < 9; i++)": what do you mean bei i{}? Why not for (i=0; i < 9; i++)?
M­­ar­­­­k 6-Jan-15 10:38am    
i{} means i = 0
CHill60 6-Jan-15 10:44am    
Not a construct I've come across - i = 0 is far easier to read
Richard MacCutchan 6-Jan-15 11:06am    
It is now. Go and look at the latest updates.
M­­ar­­­­k 6-Jan-15 10:45am    
But it works.
I mean if you use i{} instead of i=0 withing a for loop, it considers i as 0. isnt it ?

Thank you all for helps. I solved the problem. It was the pen.

C++
void MainWindow::DrawX(CDC* pDC, int pos)
{
	/*CPen* pen = new CPen(PS_SOLID, 3, RGB(0, 0, 255));
	pDC->SelectObject(pen);*/

	pDC->SelectObject(pn);

	pDC->MoveTo(sqs[pos].left, sqs[pos].top);
	pDC->LineTo(sqs[pos].right, sqs[pos].bottom);
	pDC->MoveTo(sqs[pos].right, sqs[pos].top);
	pDC->LineTo(sqs[pos].left, sqs[pos].bottom);
}


In this, you have to create the pen outside the function and select it inside the function. (as i have commented the the first two lines)
No idea why creating a pen inside a function creates such problems though. I reckon it must be something with memory consumption.
 
Share this answer
 
Comments
Leo Chapiro 7-Jan-15 4:11am    
Good job! :) +5
You should call InvalidateRect[^] in your OnLButtonDown function after setting all the squares. Windows only get redrawn in response to a WM_PAINT message, so whenever you change something in your application that affects the display you need to force a WM_PAINT to be sent, and this is the simple way to do it.
 
Share this answer
 
Comments
M­­ar­­­­k 6-Jan-15 11:18am    
you mean sqs ??
Invalidate all the rectangles in sqs[] array ? Like this ?

void MainWindow::OnLButtonDown(UINT flags, CPoint cp)
{
for (int i=0; i < 9; i++)
{
if (PtInRect(sqs[i], cp))
vals[i] = X;
}

for (int i{}; i < 9; i++)
InvalidateRect(sqs[i], FALSE);
}
Richard MacCutchan 6-Jan-15 11:33am    
No, you must invalidate the main window to force a WM_PAINT message to be generated. At a basic level the call would be InvalidateRect(NULL), to indicate that the entire window needs to be repainted. Go to the link I gave you and read the documentation, it's an important feature to understand.
M­­ar­­­­k 7-Jan-15 3:28am    
:( It didnt solve the problem.
Richard MacCutchan 7-Jan-15 6:34am    
Well that does not help us to help you. Edit your original post with the updated code, and explain exactly what does or does not happen.
I've noticed that you create each time a new GDI objeects on the heap without to get it free like this:

C#
void MainWindow::DrawX(CDC* pDC, int pos)
{
    CPen* pen = new CPen(PS_SOLID, 3, RGB(0, 0, 255));


That is wrong! Try instead to work with objects on the stack:

C#
void MainWindow::DrawX(CDC* pDC, int pos)
{
    CPen pen(PS_SOLID, 3, RGB(0, 0, 255));


Take a look at this article aa well: Attaching and Detaching Objects[^]
 
Share this answer
 
Comments
M­­ar­­­­k 7-Jan-15 3:47am    
thanks. :) it was the problem and Ive solved it. :)
i guess your program is conuming GDI resources without freeing it.
i do not understand well c++, but i do plain c.

pens, brushes, bitmaps ,... are GDI resources, you have to do two things when done with them.

1 ) restore back old gdi resource
2) and then free it.

example of code:

C++
hPen=CreatePen(...)  //create pen
hOldPen=SelectObject(hdc,hPen) // select new pen, and save oldPen
doDraw(hdc) // do you drawings
SelectObject(hdc,OldPen) // restoreback old pen
DeleteObject(hdc,hPen)   // delete the pen
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900