Array Problems

[goobers]

Hi, I've got some code as below:

CODE

x = V[(opcode&0x0F00)>>8];
y = V[(opcode&0x00F0)>>4];

for (int yline=0;yline<(opcode&0x000F);yline++){

char data = memory[(indexReg+yline)];
for(int xpix=0;xpix<8;xpix++){
if ((data&(0x80>>xpix))==1){

if (screenBuf[x+xpix][y+yline]==1)V[0xF]=1;
screenBuf[x+xpix][y+yline]^=1;


}
}
}



I want the data in the screenBuf array to be either 1s or 0s, but for some reason when I dump out the values, they are all sorts of values, rarely 1s

Is there something simple that I'm missing here? I've been trying to sort this out for hours- Its the code that handles the screen drawing opcode for my Chip8 emulator, It is supposed to read in a byte from indexReg, extract all the bits and apply each one to a seperate x coordinate in screenBuf. It repeats this for each y line.

 

[Squidge]

If screenBuf is a local variable, are you sure it's initialized first before you start modifying it?

 

[goobers]

If screenBuf is a local variable, are you sure it's initialized first before you start modifying it?

yeah, i declared it as an unsigned char at the beginning of my Cpp file, I can post the whole thing if you like, but it's about 300 lines...

 

[skeezix]

One word: l2spaces

jeff

 

[goobers]

One word: l2spaces

jeff

:blink:

 

[skeezix]

typing with baby in lap

Not thats your bug, but use spacing to make things readable. It'll pay off in speed of dev and les bugs.

But baby prevents me from more than this right now

jeff

 

[goobers]

Willdo

This is getting a bit confusing I've pretty much got all of the rest of the emulator done except this... Maybe I'll rewrite the emulation for this opcode...

 

[Sphinxter]

If screenBuf is a local variable, are you sure it's initialized first before you start modifying it?

yeah, i declared it as an unsigned char at the beginning of my Cpp file, I can post the whole thing if you like, but it's about 300 lines...


If it's initialized to 0 this will never go off since it's not set to one until after being tested, but then why test? What's in V, you set the same 0xF to one over and over and over for the entire array but only if it wasn't initialized? Tough guessing the intent without significant data.

if (screenBuf[x+xpix][y+yline]==1)
V[0xF]=1;
screenBuf[x+xpix][y+yline]^=1;

 

[goobers]

If screenBuf is a local variable, are you sure it's initialized first before you start modifying it?

yeah, i declared it as an unsigned char at the beginning of my Cpp file, I can post the whole thing if you like, but it's about 300 lines...


If it's initialized to 0 this will never go off since it's not set to one until after being tested, but then why test? What's in V, you set the same 0xF to one over and over and over for the entire array but only if it wasn't initialized? Tough guessing the intent without significant data.

if (screenBuf[x+xpix][y+yline]==1)
V[0xF]=1;
screenBuf[x+xpix][y+yline]^=1;



no, V[0xF] (v[15]) is set to 1 if screenBuf[x+xpix][y+yline] is equal to 1

I was under the impression that:

CODE

if (screenBuf[x+xpix][y+yline]==1) V[0xF]=1;



is the same as:

CODE

if (screenBuf[x+xpix][y+yline]==1) {
V[0xF]=1;
}



after it's checked that, it XORs the value in screenBuf[x+xpix][y+yline] with 1:

CODE

screenBuf[x+xpix][y+yline]^=1;



I do hope I'm not talking crap, I'm pretty new to C++

 

[Squidge]

Need... comments... (and decent structure, as Jeff says

 

[paeryn]

This section isn't doing what you think (or it doesn't look like it to me.) Reformatted to aid readability:
CODE

char data = memory[ (indexReg+yline) ];
for( int xpix = 0; xpix < 8; xpix++) {
if( (data & (0x80 >> xpix) ) == 1) {
if (screenBuf[ x+xpix ][ y+yline ] == 1)
V[0xF] = 1;
screenBuf[ x+xpix ][ y+yline ] ^= 1;
}
}



When you check the sprite bits ((data & (0x80 >> xpix)) == 1) the only time this can ever be true is on the 8th pixel e.g. first pixel set: xpix = 0, data=0x85, therefore (0x85 & (0x80>>0) = 0x80 which does not equal 1 so the screen wouldn't be updated. I'd suggest that you either remove the == 1 or change it to != 0 to make the test a simple "is there a value".
I can't see anything in the code given that would alter screenBuf[x][y] to hold anything other than 0 or 1 as long as these were the only two values in the array to start with. Have you dumped the array just prior to calling this routine to make sure it's not getting corrupt elsewhere?

 

[goobers]

This section isn't doing what you think (or it doesn't look like it to me.) Reformatted to aid readability:
CODE

char data = memory[ (indexReg+yline) ];
for( int xpix = 0; xpix < 8; xpix++) {
if( (data & (0x80 >> xpix) ) == 1) {
if (screenBuf[ x+xpix ][ y+yline ] == 1)
V[0xF] = 1;
screenBuf[ x+xpix ][ y+yline ] ^= 1;
}
}

When you check the sprite bits ((data & (0x80 >> xpix)) == 1) the only time this can ever be true is on the 8th pixel e.g. first pixel set: xpix = 0, data=0x85, therefore (0x85 & (0x80>>0) = 0x80 which does not equal 1 so the screen wouldn't be updated. I'd suggest that you either remove the == 1 or change it to != 0 to make the test a simple "is there a value".
I can't see anything in the code given that would alter screenBuf[x][y] to hold anything other than 0 or 1 as long as these were the only two values in the array to start with. Have you dumped the array just prior to calling this routine to make sure it's not getting corrupt elsewhere?


Cheers, think i'll rewrite the whole section. Still can't explain why I'm getting an array filled with crazy values bigger than a byte... Meh.

 

[pcklee123]

Could it be something wrong in the printing code?

 

[paeryn]

Cheers, think i'll rewrite the whole section. Still can't explain why I'm getting an array filled with crazy values bigger than a byte... Meh.

If your printing code is showing values larger than a byte then it's reading more than a byte at a time.
If you're using printf (or something that uses printf-style formatting codes you need "%c" to read a byte.

 

[Dr_Ian]

if ((data&(0x80>>xpix))==1)

Do you have a byte and you want to check each bit to see if it's active? For example, if you had 0x96, which is 10010110, do you want to check in turn 1, 0, 0, 1, 0, 1, 1, 0 and only run the code on the 1s? That is what it seems like, in which case the line you gave should simply be:

if (data&(0x80>>xpix))

 

[goobers]

if ((data&(0x80>>xpix))==1)

Do you have a byte and you want to check each bit to see if it's active? For example, if you had 0x96, which is 10010110, do you want to check in turn 1, 0, 0, 1, 0, 1, 1, 0 and only run the code on the 1s? That is what it seems like, in which case the line you gave should simply be:

if (data&(0x80>>xpix))

yeah, that's exactly what i'm trying to do- Chip8 sprites are 8 pixels wide, and each line is stored in one byte. I'm trying to read each bit and XOR it with the screen array- if the screen array pixel is already 1, it sets V[0xF] to 1 and sets the pixel to 0.

 

[Dr_Ian]

Okay then the correction I gave should work.
By the way, do you want to set V[0xF] to 1 if any of the screen bits match the sprite bits.

 

[goobers]

EDIT: Scrub that, Yes, I do

 

[goobers]

hmm... it works now, but the array keeps getting filled with junk data from somewhere...

 

[Dr_Ian]

hmm... it works now, but the array keeps getting filled with junk data from somewhere...

Which array? 'memory' or 'screenBuf'?
In either case I do not think the cause is this function.