HighScore, getting the users name

[McCain]

I'm witing a highscore table for a game I'm making, and I've run in to a problem... Whenever I press a key the game quits... I get this error message: "First-chance exception in Game.exe: 0xC0000005: Access Violation." I have no idea what this means... void HighScores()
{
bool bHSRunning = true;
bool bDown = true;
bool bChanged = false;
char strTemp[2];
char strName[50];
char strABC[30]("abcdefghijklmnopqrstuvwxyzåäö");
char str123[11]("0123456789");
int iKey = 0;

SetTextColor(g_hdc, RGB(255, 0, 0));

while(bHSRunning)
{
if(GetAsyncKeyState(VK_ESCAPE))
{
bHSRunning = false;
CleanUpHighScores();
g_iMenuState = 4;
}
for(int i = 0; i < 95; i++)
{
if(!bDown)
{
if(HIWORD(GetAsyncKeyState(i)))
{
if(i >= 48 && i <= 57)
{
strncpy(strTemp, str123 + (i - 48), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 256;
}
if(i >= 65 && i <= 90)
{
strncpy(strTemp, strABC + (i - 65), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 256;
}
if(i == 8) //8 == BackSpace
{
chardel(strName, strlen(strName));
TextOut(g_hdc, 100, 100, strName, strlen(strName));
iKey = i;
bDown = true;
i = 256;
}
}
}
if(bDown)
{
if(!HIWORD(GetAsyncKeyState(iKey)))
{
bDown = false;
}
}
}
if(GetAsyncKeyState(VK_SHIFT) || GetAsyncKeyState(VK_LSHIFT) || GetAsyncKeyState(VK_RSHIFT))
{
strupr(strTemp);
}
if(bChanged)
{
strcat(strName, strTemp);
TextOut(g_hdc, 100, 100, strName, strlen(strName));
bChanged = false;
}
}
} It's a win32 app written in vc++6.0

[CornedBee]

This means you access memory that doesn't belong to you. Try to narrow the error down as much as possible (placing outputs in the code or better, if you know how, do a debug run).

[CornedBee]

BTW the code looks extremly inefficient to me. Maybe you should restructure it.

[McCain]

The error seems to be when the program gets to the line: "strcat(strName, strTemp);" (It's near the bottom)BTW the code looks extremly inefficient to me. Maybe you should restructure it. I'd love to, but I know nothing about how to optimize code...

[McCain]

BTW This code works just fine in a win32 console app (using cout instead of TextOut and calling it void main() instead of void HighScore())...

[McCain]

Ok, this code works: void HighScores(HWND hwnd)
{
bool bHSRunning = true;
bool bDown = true;
bool bChanged = false;
char strTemp[2];
char strName[50];
char strABC[30]("abcdefghijklmnopqrstuvwxyzåäö");
char str123[11]("0123456789");
int iKey = 0;

GetClientRect(hwnd, &g_rcClient);
g_hdc = GetDC(hwnd);

SetTextColor(g_hdc, RGB(255, 0, 0));
SetBkColor(g_hdc, RGB(0, 0, 0));
SetTextColor(g_hdcBuffer, RGB(255, 0, 0));
SetBkColor(g_hdcBuffer, RGB(0, 0, 0));

memset(strName, NULL, sizeof(strName));
memset(strTemp, NULL, sizeof(strTemp));

while(bHSRunning)
{
if(GetAsyncKeyState(VK_ESCAPE))
{
bHSRunning = false;
CleanUpHighScores();
g_iMenuState = 4;
}
for(int i = 0; i < 95; i++)
{
if(!bDown)
{
if(HIWORD(GetAsyncKeyState(i)))
{
if(i >= 48 && i <= 57)
{
strncpy(strTemp, str123 + (i - 48), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 256;
}
if(i >= 65 && i <= 90)
{
strncpy(strTemp, strABC + (i - 65), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 256;
}
if(i == 8) //8 == BackSpace
{
chardel(strName, strlen(strName));
FillRect(g_hdcBuffer, &g_rcClient, (HBRUSH)GetStockObject(BLACK_BRUSH));
TextOut(g_hdcBuffer, 100, 100, strName, strlen(strName));
BitBlt(g_hdc, 0, 0, g_rcClient.right, g_rcClient.bottom, g_hdcBuffer, 0, 0, SRCCOPY);
iKey = i;
bDown = true;
i = 256;
}
if(i == 32) //32 == Space
{
strcpy(strTemp, " ");
bChanged = true;
iKey = i;
bDown = true;
i = 256;
}
}
}
if(bDown)
{
if(!HIWORD(GetAsyncKeyState(iKey)))
{
bDown = false;
}
}
}
if(GetAsyncKeyState(VK_SHIFT) || GetAsyncKeyState(VK_LSHIFT) || GetAsyncKeyState(VK_RSHIFT))
{
strupr(strTemp);
}
if(bChanged)
{
strcat(strName, strTemp);
TextOut(g_hdc, 100, 100, strName, strlen(strName));
bChanged = false;
}
}
}But I still don't know why I needed to fill the strings with NULL or why it worked in the console and not in a regular win app. I've allso added support for the space bar. Now I just want to know how to make this code more efficient and how to add support for holding the backspace key down to erase more than one letter.

[CornedBee]

The reason is quite simple: your use of strcat. strcat searches for the end of the string and adds the characters there. The end of the string is marked by '\0', so if you don't initialize the buffer to all 0 it will search past the end of the buffer and then try to write there, resulting in the access violation.

But I don't see why you couldn't use strcpy instead of strcat here.

About the inefficiency: first, is there any specific reason why you're keeping track of the buttons yourself instead of letting the message loop handle it?

Second: instead of one for-loop and several ifs use several for-loops. This removes many unnecessary - because impossible - if-statements.

[McCain]

But why didn't I need to initialize the string when I ran the code in console mode?is there any specific reason why you're keeping track of the buttons yourself instead of letting the message loop handle it?I don't understand. I'm doing it the only way I could think of... If there is a better way, please let me know. How would I go about letting the message loop handle it?
I'll remove (most of) the ifs and post the updated code soon.

[McCain]

I think this code should be more optimized...void HighScores(HWND hwnd)
{
bool bHSRunning = true;
bool bDown = true;
bool bChanged = false;
char strTemp[2];
char strName[50];
char strABC[30]("abcdefghijklmnopqrstuvwxyzåäö");
char str123[11]("0123456789");
int iKey = 0;
int i = 0;

GetClientRect(hwnd, &g_rcClient);
g_hdc = GetDC(hwnd);

SetTextColor(g_hdc, RGB(255, 0, 0));
SetBkColor(g_hdc, RGB(0, 0, 0));
SetTextColor(g_hdcBuffer, RGB(255, 0, 0));
SetBkColor(g_hdcBuffer, RGB(0, 0, 0));

memset(strName, NULL, sizeof(strName));
memset(strTemp, NULL, sizeof(strTemp));

while(bHSRunning)
{
if(GetAsyncKeyState(VK_RETURN))
{
bHSRunning = false;
CleanUpHighScores();
g_iMenuState = 4;
}
if(!bDown)
{
for(i = 65; i < 91; i++)
{
if(HIWORD(GetAsyncKeyState(i)))
{

strncpy(strTemp, strABC + (i - 65), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 92;
}
}
}
if(!bDown)
{
for(i = 48; i < 58; i++)
{
if(HIWORD(GetAsyncKeyState(i)))
{
strncpy(strTemp, str123 + (i - 48), 1);
bChanged = true;
iKey = i;
bDown = true;
i = 60;
}
}
}
if(!bDown)
{
if(HIWORD(GetAsyncKeyState(8))) //8 == BackSpace
{
chardel(strName, strlen(strName));
FillRect(g_hdcBuffer, &g_rcClient, (HBRUSH)GetStockObject(BLACK_BRUSH));
TextOut(g_hdcBuffer, 100, 100, strName, strlen(strName));
BitBlt(g_hdc, 0, 0, g_rcClient.right, g_rcClient.bottom, g_hdcBuffer, 0, 0, SRCCOPY);
iKey = 8;
bDown = true;
//i = 256;
}
if(HIWORD(GetAsyncKeyState(32))) //32 == Space
{
strcpy(strTemp, " ");
bChanged = true;
iKey = 32;
bDown = true;
//i = 256;
}
}
if(bDown)
{
if(!HIWORD(GetAsyncKeyState(iKey)))
{
bDown = false;
}
}
if(GetAsyncKeyState(VK_SHIFT) || GetAsyncKeyState(VK_LSHIFT) || GetAsyncKeyState(VK_RSHIFT))
{
strupr(strTemp);
}
if(bChanged)
{
strcat(strName, strTemp);
TextOut(g_hdc, 100, 100, strName, strlen(strName));
bChanged = false;
}
}
}

[CornedBee]

Yes, that's what I meant.

As for why it worked in console: the content of uninitialized memory is random. You might have been lucky.