How do you know if your code is structured well enough?

[Khaz]

One more tiny structural habit that just occured to me that I wonder if others do: I put an empty line following every branch instruction. I find it makes it easier to see how the program flows at a glance, and helps to not miss a label when renaming all my branches as per my last post.

Khaz (I think?)

Depends on your definition of "for SNESDev". I've tried to share my methods and any tools I come up with along the way, and I very much enjoy helping others with whatever problems they may have. The game I'm developing is explicitly for me, though. I may try to sell it, I may post it up for free, I have no idea yet... But I'm reserving all rights and being very paranoid about the source code and detailed plans (and the tiny few graphical assets I've actually put work into) right now, because I'm a very paranoid person by nature.

I might post up a demo ROM at some point in the nearish future, once I have the game engine complete and after I've taken the requisite copyright precautions just in case.

If/when the game is done and "published" in whatever way is appropriate I'll probably release the full source code too. Mostly because my collision calculation routine could easily be used to FINALLY make a proper Sonic port on SNES. Just don't hold your breath, that's a long way off...

I'd gladly post up a relevant piece of code just to show what it looks like, but I dunno about my vBlank routine, it's a mess at the moment.

[Drew Sebastino]

a proper Sonic port on SNES

I'm still amazed that this hasn't been done yet. Also, don't forget to add a debug mode.

[nicklausw]

Either you put space before your instructions, or you don't. Maybe that's just me.

[Myask]

One habit I like is putting a header at the top of each file/subroutine to briefly explain what it does and to keep all the relevant information in one place. For example, I'll list any prerequisites for calling it

Those're called the preconditions of a function; what must be true before it begins executing. The postconditions of a function are, similarly, what must be true after it finishes.

Unless you plan on other people needing to read your source code, I would say it's entirely up to you so long as you'll be able to understand it when you come back later.

One generally is another person by the time you "come back later".

[Drew Sebastino]

One generally is another person by the time you "come back later".

I went back to some code I made I hadn't touched for over a month and I read it just fine. Maybe I just structure everything really well or something. Then again, it looks like this:

Code: Select all

look_for_16x16_vram:
   cpy #$0080				;128, because there are 128 slots for sprites
   beq vram_finder_done			;no space left for sprite
   lda VramTable,y   
   beq slot_found			;there is space for another sprite
   iny
   bra look_for_16x16_vram		;look again if the space is already occupied

look_for_32x32_vram:
   cpy  #$0080						;128, because there are 128 slots for sprites
   beq  vram_finder_done				;no space left for sprite
   lda  VramTable,y					;upper lefthand corner of square
   bne  prepare_for_look_for_32x32_vram	;look again if the space is already occupied
   lda  VramTable+1,y					;upper righthand corner of square 
   bne  prepare_for_look_for_32x32_vram	;look again if the space is already occupied
   lda  VramTable+8,y					;lower lefthand corner of square
   bne  prepare_for_look_for_32x32_vram	;look again if the space is already occupied
   lda  VramTable+9,y					;lower righthand corner of square
   bne  prepare_for_look_for_32x32_vram	;look again if the space is already occupied
   bra  32x32_slot_found				;there is space for another sprite

[rainwarrior]

I'm a bit curious as to why you'd write:

Code: Select all

cpy #$0080 ; 128

instead of just:

Code: Select all

cpy #128

[Drew Sebastino]

I think it had something to do with the bits for the DMA controller, in that the number 128 isn't important, only that the high bit is set. I do things like that when I'm dealing with video hardware and stuff. I'm not too sure why I wrote the 128 side note though, because thinking in terms of numbers isn't very important here like I said.

[Myask]

Espozo: Because it's densely-commented (perhaps a little too-densely) and the labels/variable names are verbose.

rainwarrior: hmm. If I did it it'd be to imply to the reader that it's a 16-bit operation (even if it's the province of processor flags as to whether that's how it executes).

[Drew Sebastino]

Actually, never mind about me writing #$80 instead of 128. I though that was talking about something else, but it's really just talking about how many sprites there are, so I also don't have a clue.

Because it's densely-commented (perhaps a little too-densely)

It doesn't look as clustered in notepad for whatever reason.

the labels/variable names are verbose.

I don't see how you'd really use less. Anyway, what I wrote works just fine for me.

[Optiroc]

Personally I prefer being verbose with labels (as well as function names, properties and what-not in high level languages). I left short cryptic names behind when I left the C64 with Turbo Assembler (fitting the editor/assembler and the code produced on the same 64KB machine sounds like complete madness today but worked surprisingly well).

Espozo: Your "look_for_16x16_vram" routine (if not just an example of coding style) could use a quicker look-up algorithm. Looping through up to 128 entries to find an empty slot (or returning a failed allocation) will eat up your frame time quickly!

[Drew Sebastino]

Espozo: Your "look_for_16x16_vram" routine (if not just an example of coding style) could use a quicker look-up algorithm. Looping through up to 128 entries to find an empty slot (or returning a failed allocation) will eat up your frame time quickly!

This is actually my older code that I just came back to to fix. I had recently thought of the idea of having 2 128 entry tables, where one table is for 16x16 sized sprites, and the other is for 32x32 sized sprites, with each entry being 3 words instead of 4 because I don't need to say the size anymore, so it will look like this:

Table in Rom:

Sprite / Transfer Size = 0
BankNumber = 2
TileAdress = 4

Table in Ram:

VramAdress = 0 (I obviously don't know the vram address when in rom, and the size has been sorted)
BankNumber = 2
TileAdress = 4

The reason I'm doing this is because although there is a little extra processing to sort things into different tables, I don't need to check the sprite / transfer size for every sprite, and only need to look at the request counter for 16x16 sized sprites, and then move to 32x32 sized sprites which isn't any more in terms of processing to just check one big request counter like I had originally. However, the main advantage to this is that in the vram finder code, If I'm only doing 16x16, and then 32x32, I won't need to jump back to the beginning of the table, because I know it's filled out as much as it can be because it's originally all 16x16's, which can fit anywhere, unlike 32x32's which can only work if 4 spaces are clear. However, if I see myself running out of space in vram, I could have it to where all 32x32's go first, and 16x16's go second, but start at the beginning of the table unlike if 16x16's went first because there are plenty of spots where 16x16's fit and 32x32's don't, unlike the other way around. This would ensure that the 16x16's don't take a spot where a 32x32 could be when there's enough space for them that won't affect 32x32's like a spot that already has a 16x16 left from a previous frame.

[Drew Sebastino]

I think this is better

Code: Select all

VramAdress = 0
Size = 0
BankNumber = 2
TileAdress = 4



start_vram_finder:
  rep  #$30	; A=16, X/Y=16
  stz  32x32ColumnSkipperCounter
  ldx  #$0000
  ldy  #$0000
  cpx  32x32TileRequestCounter
  beq  16x16_vram_finder_start

look_for_32x32_vram:
  cpy  #$0080					;128, because there are 128 slots for sprites
  beq  vram_finder_done				;no space left for sprite
  lda  VramTable,y				;upper lefthand corner of square
  bne  prepare_for_look_for_32x32_vram		;look again if the space is already occupied
  lda  VramTable+1,y				;upper righthand corner of square   
  bne  prepare_for_look_for_32x32_vram		;look again if the space is already occupied
  lda  VramTable+8,y				;lower lefthand corner of square
  bne  prepare_for_look_for_32x32_vram		;look again if the space is already occupied
  lda  VramTable+9,y				;lower righthand corner of square
  bne  prepare_for_look_for_32x32_vram		;look again if the space is already occupied

32x32_slot_found:
  lda  #$01
  sta  VramTable,y		;say that four of the slots are now taken
  sta  VramTable+1,y
  sta  VramTable+8,y
  sta  VramTable+9,y
  sty  32x32TileRequestTable+VramAdress,x
  cpx  32x32TileRequestCounter
  beq  16x16_vram_finder_start
  inx
  inx
  inx
  inx
  bra  look_for_32x32_vram      

prepare_for_look_for_32x32_vram:
  inc  32x32ColumnSkipperCounter
  cmp  #$08
  beq  next_row
  iny
  iny
  bra  look_for_32x32_vram

next_row:
  tya				;If this is done right, this should skip every other row of tiles
  adc  #$08
  tay
  bra  look_for_32x32_vram


32x32_vram_finder_start:
  ldx  #$0000
  ldy  #$0000
  cpx  32x32TileRequestCounter
  beq  16x16_vram_finder_start

look_for_16x16_vram:
  cpy  #$0080			;128, because there are 128 slots for sprites
  beq  vram_finder_done		;no space left for sprite
  lda  VramTable,y   
  beq  slot_found		;there is space for another sprite
  iny
  bra  look_for_16x16_vram	;look again if the space is already occupied

16x16_slot_found:
  inc  VramTable,y		;say that one of the slots is now taken
  sty  16x16TileRequestTable+VramAdress,x
  cpx  16x16TileRequestCounter
  beq  vram_finder_done
  inx
  inx
  inx
  inx
  bra  look_for_16x16_vram

vram_finder_done:
  rts

One thing that still needs to be done is the figured out address for the 16x16 tiles needs to be converted into and address to work with 8x8 tiles, but I've been holding off on it because I don't even think I'll have it to where 1 entire byte represents a 16x16 area. If it's the fastest way, I'll keep it, but if it isn't, I'll change it. What are you doing, psychopathicteen?

[psycopathicteen]

I have it so each byte represents 4 16x16s or 1 32x32.

[Drew Sebastino]

I figured I'd do it too, as it takes less space and it's only one check for 32x32 sized sprites instead of four, but I ran into a problem: How do you translate it to where it's the tile number? Also, I guess you don't need to do anything special with 32x32 sprites every 8 of them (which is normally where the next row starts)?

Edit: Hey, I actually thought of one thing, if this makes a difference. When checking the byte for a spot out of the 4 that the sprite will fit, instead of doing this:

Code: Select all

  lda  VramTable,y
  bit  #$11000000
  bne  16x16_slot_found
  bit  #$00110000
  bne  16x16_slot_found
  bit  #$00001100
  bne  16x16_slot_found
  bit  #$00000011
  bne  16x16_slot_found

You could have each one have its own code that it jumps to, like this:

Code: Select all

  lda  VramTable,y
  bit  #$11000000
  bne  16x16_slot1_found
  bit  #$00110000
  bne  16x16_slot2_found
  bit  #$00001100
  bne  16x16_slot3_found
  bit  #$00000011
  bne  16x16_slot4_found

That way, you shouldn't have to worry about any of the bits in the byte, only the address of the byte.

[Drew Sebastino]

Please tell me there's a simpler way to doing it than what I just devised: (I'd only bothered with 32x32 sprites so far)

Code: Select all

  rep #$30	; A=16, X/Y=16
  tya
  and #$FFFC
  asl
  asl
  asl
  asl
  sta IDoNotKnowWhatToNameThisYet

  tya
  and #$0003
  asl
  asl
  adc IDoNotKnowWhatToNameThisYet
  sta 32x32TileRequestTable+VramAdress,x
  sep #$20	; A=8 

If I'm not mistaken, these numbers:

1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15

Would be converted into these numbers:

0, 4, 8, 16, 64, 68, 72, 76, 128, 132, 136, 140, 192, 196, 200, 204



Wait a minute, wouldn't it be a lot easier to use a lookup table? It would only have to be 64 bytes long and would save a lot of processing.