gbadev.org forum archive

This is a read-only mirror of the content originally found on forum.gbadev.org (now offline), salvaged from Wayback machine copies. A new forum can be found here.

Beginners > What is going on?

#134581 - biubid_boy - Sat Jul 14, 2007 9:53 am

I'm creating a set of functions to automatically set up things like loading sprites and backgrounds. It should make programming easier in the future and it tests my knowledge in those areas. But one problem I have is in my load_background function. There seems to be something wrong. Here is my code: (note: It's not finished yet, so there isn't any support for 8bpp etc.)

background.c
Code:
 void load_bg_4bpp(tiledbg *bg, int bg_w, int bg_h, u16 *tiles, u16 tile_size, u16* map, u16 map_size, u16 *pal)
{
    if(bg_w == 256 && bg_h == 256) {c_sbb--;} //<--PROBLEM!
   
    memcpy(BGPaletteMem, pal, 512);
    memcpy(SCREEN_BASE_BLOCK(c_sbb), map, map_size);
    memcpy(MAP_BASE_BLOCK(c_cbb), tiles, tile_size);

    ...


main.c
Code:
 
load_bg_4bpp(&grass, 256, 256, grassTiles, grassTilesLen, grassMap, grassMapLen, grassPal);


The problem is, the first expression never evaluates true! I've tried everything I can think of and I can't get it to work. Am I missing something obvious here?
Please help,
Biubid_boy.

#134582 - keldon - Sat Jul 14, 2007 9:57 am

How can you be sure that it never evaluates to true?

#134583 - biubid_boy - Sat Jul 14, 2007 10:13 am

Because if I take out the if (just "c_sbb--") everything works perfectly. Unfortunately I need this for the function to work correctly.

#134595 - biubid_boy - Sat Jul 14, 2007 2:46 pm

Ok, I really don't know what is going on. Here is my entire code (rearranged in a failed at attempt at making it work).
Code:
void load_bg_4bpp(tiledbg *bg, int bg_w, int bg_h, u16 *tiles, u16 tile_size, u16* map, u16 map_size, u16 *pal)
{   
    memcpy(BGPaletteMem, pal, 512);
    memcpy((u16*)SCREEN_BASE_BLOCK(c_sbb), map, map_size);
    memcpy((u16*)MAP_BASE_BLOCK(c_cbb), tiles, tile_size);
   
    bg->bpp      = bg_bpp;
    bg->bg_num   = bg_num;
    bg->map_size = map_sizes[bg_w>>8][bg_h>>8];
    bg->cbb      = c_cbb;
    bg->sbb      = c_sbb;
   
    get_bg_cnt(bg_num) = BG_CONTROL(bg->bg_num, bg->cbb, bg->mosaic, bg->bpp, bg->sbb, bg->wrap, bg->map_size);
   
    if (map_sizes[bg_w>>8][bg_h>>8] == 0) {c_sbb--;}
    else if(map_sizes[bg_w>>8][bg_h>>8] == 3) {c_sbb -= 4;}
    //else {c_sbb-=2;} <- WTF?
   
    c_cbb++;
    bg_num++;
}

If I uncomment the else statement, my map suddenly becomes garbage, even when the 'else' code is after 'map-making' code. As far as I can see, as long as I don't call the function twice, that shouldn't matter at all. And yet it screws my maps! :(
What is going on?
Biubid_boy.

#134955 - Miked0801 - Wed Jul 18, 2007 9:42 pm

A few comments:

Your variable names are so sparce that I cannot interperate their meaning. Use better names. c_cbb and s_sbb in particular are causing me problems. sbb == screen_base_block?
Also, you haven't included your globals anywhere. the above vars are not declared anywhere so I am not sure of their type which may matter.

map_size and map_sizes are so close in name that I am having trouble telling them appart when glancing as well.

Asserts may also help when debugging your if statement. Are you sure that map_sizes[][] returns a value within your expected range each time? I'm guessing you want a value from 0-3, but again I am guessing as there are no comments the variables are poorly named.

Too many globals being modified in here therefore there are too many points of failure that are possible. Are you sure that the globals being modified here aren't causing havoc elsewhere which is causing your corruption?

Are you reading off the end of your map_sizes[][] array? I'd drop the >>8 vars into a temp to double check ranges and perhaps assert those values as well just in case.

Are you sure that the initial memcpy functions are receiving the correct sizes? If the sizes are bad (negative or too big) you will get buffer overflows which will cause random behavior - like random code changes causing stuff to work or not (like you describe.)

Anyways, put some asserts on you input, try to use 0 globals, rename your vars to english, and add a comment or two. You'll get more exact advice.

#134986 - Cearn - Thu Jul 19, 2007 1:00 am

I'm guessing:
Code:
// --- Parameters ---
bg_w        BG width in pixels
bg_h        BG height in pixels
tiles       Source tile data
tile_size   sizeof(tiles)
map         Source map data
map_size    sizeof(map)

// ---Globals ---
map_sizes   REG_BGxCNT size-bits, based on BG width and height.
c_cbb       Char-Base Block reference counter
c_sbb       Screen-Base Block reference counter.


c_cbb counts up, but c_sbb counts down. The latter should probably be updated before you copy into the screenblocks, not afterwards.

For example:, say c_sbb=31 and you load a 512x512 map into it. The second memcpy will copy into SBB 31 ... and 32,33 and 34, none of which exist. Afterwards, c_sbb will decrease by four to make 27, even though 28, 29 and 30 are still unused.

Another example: start with c_sbb=31 again and a 256x256 map. This time only SBB 31 is filled, the reference counter is decremented to 30 and everything seems fine. Now add a second map, say 512x512 in size. This fills 4 screen-blocks, starting at 30. In other words, this will overwrite the previously loaded map.

For decreasing reference counters: Decrement Before (Full-Descending stack). Start with 32 (not 31).
For increasing reference counters: Increment After (Empty-Ascending stack). Start with 0.

There is also a possibility of overlap between char-blocks if you're not careful. Make sure your tile-size is never over 0x4000.

Another possibility: I take it map_sizes is intended to get the REG_BGxCNT size-bits (bits 13&14) from the BG-dimensions, and looks something like this:
Code:
const u8 mapsizes[2][2]
{
   { 0, 1 },    // 256x256 and 512x256
   { 2, 3 }     // 256x512 and 512x512
};

If so, you're not grabbing the right bits. For a 256x256 map, you're retrieving element [256>>0][256>>8] = [1][1], and not [0][0]. You're off by one in both directions. Shift by 9 instead of 8. Or if you want to be really 1337, use ((bg_h>>8)&2) | (bg_w>>9)&1) :P. Or forgo the whole thing and use #defines for the sizes. Whatever you choose, hide it behind a function, because it will be mystifying to anyone else.

#134992 - Miked0801 - Thu Jul 19, 2007 1:36 am

Thanks Cearn. At least you appear to be somewhat familiar with the base code to at least guess at its functionality. I was shooting in the dark hoping to get something as the code in question is really sparce. :)