Memory corruption under heavy load

Memory corruption under heavy load
by HamsterMan on 2009-07-23 (#49149)

Hello there everyone

Okay, just a heads up. I haven't been coding for the nes for very long so I'm pretty clueless about most the technical stuff about it.

A while back I started on my first NES game, which I put together using NBasic and Nesasm. I modified Nespaint and the famitracker sound driver to work with the project. Everything was working smoothly until I encountered a problem with the background tiles and random variables changing by themselfes whenever there is a lot happening in the game at once. I have already put all the $2006 $2007 writes in a buffer that is written during the VBlank NMI and all the sprites are drawn to a shadow OAM. I have tried just about everything to get the variables not to get corrupted, like I tried turning off the rendering and waiting for the VBlank flag in the main game loop instead of in the NMI, and that didn't help either.

Here is the most importent sections of the program.

Code:

//set buffer value
set_buffer_value:
   if next_buffer_index > 250 return
   set [background_draw_buffer next_buffer_index] first_value
   inc next_buffer_index
   set [background_draw_buffer next_buffer_index] second_value
   inc next_buffer_index
   set [background_draw_buffer next_buffer_index] ppu_value
   inc next_buffer_index

   return

//ingame loop
ingame_loop:
   //update joysticks
   gosub joystick1
   gosub joystick2

   //handle units
   gosub handle_units
   after_unit_handle:

   //handle level blocks
   gosub handle_blocks
   after_block_handle:

   //set the can draw flag
   set waiting_for_vblank_flag 1
   set can_draw_flag 1

   //wait for the vblank flag
   wait_for_vblank_loop:
      if waiting_for_vblank_flag = 1 goto wait_for_vblank_loop

   //gosub handle_sound
   goto ingame_loop

//ingame nmi loop
ingame_nmi_loop:
   //check if can draw flag is true
   if can_draw_flag = 0 goto cant_draw_yet
   set can_draw_flag 0

   //set the ppu to not render anything
   set $2001 0

   //set sprite dma
   set SPRITE_DMA 5

   //draw the buffer
   if next_buffer_index < 3 goto stop_buffer_loop

   set x 0
   draw_buffer_loop:
      set $2006 [background_draw_buffer x]
      inc x
      set $2006 [background_draw_buffer x]
      inc x
      set $2007 [background_draw_buffer x]
      inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2007 [background_draw_buffer x]
      //inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2007 [background_draw_buffer x]
      //inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2006 [background_draw_buffer x]
      //inc x
      //set $2007 [background_draw_buffer x]
      //inc x
      if x < next_buffer_index goto draw_buffer_loop

      set next_buffer_index 0
   stop_buffer_loop:

   //set scroll
   set $2005 0
   set $2005 248

   //clear the waiting for vblank flag
   set waiting_for_vblank_flag 0

   //set the ppu to render everything
   set $2000 %10010000
   set $2001 level_ppu

   //can't draw yet label
   cant_draw_yet:

   //rti
   asmline rti

I'd be most grateful if someone had any idea what could be causing this. I'm just about ready to give up on this project by now.

You can download the NES file plus source at http://hamsterworld.no-ip.info/mygame.rar. If you test it, try blowing up as many blocks as quickly as possible. Also, I haven't tried troubleshooting the game in any other emulator than nestopia.

by Kasumi on 2009-07-24 (#49154)

I admit I don't know anything about nbasic, so I cannot make as much sense of your code as I would like to. Therefore the "solution" that follows is simply a guess based on the problems you're describing. Sorry if it's wrong, and I've wasted your time.

Find three RAM locations you aren't using and name them nmia, nmix, and nmiy.
And directly after "ingame_nmi_loop:" add the following

Code:
sta nmia
stx nmix
sty nmiy

And directly before the rti that ends "ingame_nmi_loop:" (asmline rti)? add the following.

Code:
lda nmia
ldx nmix
ldy nmiy

(If you cannot use the mnemonics from nesasm, it looks like the equivalents in NBasic are "set nmia a", "set nmix x", "set nmiy y" and "set a nmia", "set x nmix", "set y nmiy" respectively)

In case my guess about the NBasic commands is wrong, and you cannot use nesasm mnemonics, what you are doing is storing the accumulator, the x register and the y register in separate RAM locations at the VERY beginning of your NMI routine. Then you are loading them back at the VERY end.

Your new nmi routine should look like this (or instead of the nesasm mnemonics, you have NBasic equivalents. Also, you need to name some locations in RAM these things):
Code:
//ingame nmi loop
ingame_nmi_loop:
sta nmia
stx nmix
sty nmiy
//check if can draw flag is true
if can_draw_flag = 0 goto cant_draw_yet
set can_draw_flag 0

//set the ppu to not render anything
set $2001 0

//set sprite dma
set SPRITE_DMA 5

//draw the buffer
if next_buffer_index < 3 goto stop_buffer_loop

set x 0
draw_buffer_loop:
set $2006 [background_draw_buffer x]
inc x
set $2006 [background_draw_buffer x]
inc x
set $2007 [background_draw_buffer x]
inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2007 [background_draw_buffer x]
//inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2007 [background_draw_buffer x]
//inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2006 [background_draw_buffer x]
//inc x
//set $2007 [background_draw_buffer x]
//inc x
if x < next_buffer_index goto draw_buffer_loop

set next_buffer_index 0
stop_buffer_loop:

//set scroll
set $2005 0
set $2005 248

//clear the waiting for vblank flag
set waiting_for_vblank_flag 0

//set the ppu to render everything
set $2000 %10010000
set $2001 level_ppu

//can't draw yet label
cant_draw_yet:
lda nmia
ldx nmix
ldy nmiy
//rti
asmline rti

The reason I am guessing this will fix what is happening to you is because the problem only occurs when a lot is happening on screen. If your main game loop takes less than a single frame to complete, the NMI routine will be run while you are waiting for it with this:

Code:
wait_for_vblank_loop:
if waiting_for_vblank_flag = 1 goto wait_for_vblank_loop

But if a lot is happening on screen, and your game loop takes longer than a single frame to complete, the NMI routine could be run in the middle of your game loop. (It's triggered every vblank. If a vblank occurs in the middle of your game loop, it will run then) Since your NMI routine changes the values stored in the X register and the accumulator, when it returns from the NMI routine to the middle of your game loop, the values that were there before the NMI was triggered are no longer there. This fix stores what was there before in RAM, so that after these values are changed by your NMI routine, they can be restored when the NMI returns to your game loop.

If I am wrong about this, I apologize for wasting everyone's time with my guess.

One other thing: I was only able to actually play your game once in Nintendulator. (A very accurate NES emulator) The rest of the time I kept falling down a hole right at the start, and no button presses made anything happen afterwords. I don't know why it worked once. There may be other things wrong with your code.

by Banshaku on 2009-07-24 (#49155)

What Kasumi wanted to do is to preserve and restore the registers. Usually you will do that with the stack. Here's what I found in the nbasic documentation:

-------------------------------------------
Push and Pop

There are times when you may wish to store data on the processor stack. For instance, saving and restoring the values of registers during an interrupt. For important cases like this, you can use the push and pop keywords to store or retrieve data on the stack. The nbasic implementation will only let you directly push or pop the values of registers. Note that, because gosub and return also manipulate the stack, you should always be certain to pop within the same function as a corresponding push. Due to the nature of stacks, if you push several registers, you should pop them in the reverse order. Here is an example of proper stack usage during an interrupt.

IRQ:
push a
push x
push y
gosub irq_handler
pop y
pop x
pop a
resume
-------------------------------------------

A NMI is an interupt so the same apply. I will test your game once I have a chance, just game back from work.

edit:

Another comment, since you're already in the NMI and the screen will be blanked, you don't need to disable it during the event (set $2001 = 0). This could be one of your problem but I'm just speculating. You should separate your screen update per frame in the main loop but since you have a flag, you seems to already doing this.

edit2:

Since famitracker was never ported to nesasm I was surprised to see my port inside your source code. So someone actually tried to use it, didn't know

Regarding the sound fx code I included ages ago, this was mostly a proof of concept and was not doing much. Once my current megaman 9 demo is working fine and made a module for sound that is easier to use than this, I could always port back the code for nesasm if there is anybody is interested.

Edit3:

Tested more, cannot hear any sound. Cannot reproduce the bug (I think) yet. The wall outside were corrupted once. One thing thought is the game doesn't work on all emulator since the player appears at different place based on the emulator. this mean it may not work on a real nes.

The more there is lava, the more you need to update the name table because of the animation you want to do. I would suggest that you use another mapper that uses chr-ram instead. This way, you would need to only update 4 tiles in the pattern table and it would be a lot smoother. that could be one of your issues at the moment. Will investigate more.

by Banshaku on 2009-07-24 (#49157)

Double post, tired of editing original message

First comment: you shouldn't give up on your game, it has potential. I usually don't like 1 screen game but this one seems simple but interesting.

Now for the real comment:

I would recommend that you separate you code per module. This will make it easier to read. For example, there is some .bas module for header, footer etc. You should do the same and have some for the data, random generator, menu, main loop etc. You don't need to separate into that much small but try to separate per logic group. This will help someone that will read your code to focus only on what could be the cause of your issue. Now I have to browse a 70k file. It's hard to search in it.

As for the bug, I don't think is only happen when there is too much things on the screen. I think it happen on a specific event. I didn't have that much and all of a sudden, the top menu was corrupted. Try to search on which event it happens. Since your now your game well, write down the list of all event and test them one by one. If it doesn't happen, do combination tests of events.

Animating the background by updating the name table when the full screen could be animated is a big no-no. You want to animate the lava block. The lava blocks takes 4 tiles. Either you bank switch the data or you use chr-ram and write the tile for the animation. I removed the comment for the music and the more the animation, the more it slow down. It shouldn't happen for what is happening on the screen. I recommend to:

- Convert your rom to chr-ram
- Load your chr data at init time
- Instead of animating all the converted lava map part in the name table, just update the 4 tiles in the pattern table

This little modification will boost the performance to your game. It will make quite a difference. For testing the performance, just disable the animation loop and see what happen. For now I don't know your code enough to do it (and don't know nbasic!).

Don't give up, I'm sure you will figure it out. I will do some test on the code if I can find the time.

by HamsterMan on 2009-07-24 (#49165)

I thank you both very much for your feedback

Kasumi: Your suggestion helped me partially solve the strange problem the game was experiencing. After I put in the code to store and load the registers in the NMI the unit variables didn't seem to randomly get corrupted anymore.

The secondary problem with different background tiles changing seems to have dissapeared after I made the $2006 $2007 write buffer properly clear itself in the beginning of the ingame loop.

This is the updated code

Code:
//ingame loop
ingame_loop:
//clear background buffer
set temp_value 0
clear_buffer_loop:
   set [background_draw_buffer temp_value] 0
   inc temp_value
if temp_value <> + next_buffer_index 9 branchto clear_buffer_loop
set next_buffer_index 0

//update joysticks
gosub joystick1
gosub joystick2

//handle units
gosub handle_units
after_unit_handle:

//handle level blocks
gosub handle_blocks
after_block_handle:

//update counter variables
inc counter
if counter > 7 set counter 1
inc even_odd
if even_odd = 2 set even_odd 0
inc cycles

//set the can draw flag
set waiting_for_vblank_flag 1
set can_draw_flag 1

//wait for the vblank flag
wait_for_vblank_loop:
   if waiting_for_vblank_flag = 1 goto wait_for_vblank_loop

//gosub handle_sound
goto ingame_loop

//ingame nmi loop
ingame_nmi_loop:
//store the registers
asm
sta nmia
stx nmix
sty nmiy
endasm

//check if can draw flag is true
if can_draw_flag = 0 goto cant_draw_yet
set can_draw_flag 0

//set sprite dma
set SPRITE_DMA 5

//draw the buffer
if next_buffer_index < 3 goto stop_buffer_loop

set x 0
draw_buffer_loop:
   set $2006 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2007 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2007 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2007 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2006 [background_draw_buffer x]
   inc x
   set $2007 [background_draw_buffer x]
   inc x
   if x < next_buffer_index goto draw_buffer_loop
stop_buffer_loop:

//clear the waiting for vblank flag
set waiting_for_vblank_flag 0

//set scroll
set $2005 0
set $2005 248

//set the ppu to render everything
set $2000 %10010000
set $2001 level_ppu

//can't draw yet label
cant_draw_yet:

//handle music and sound fx
gosub handle_SoundFx
gosub ft_music_play

//rti
asm
lda nmia
ldx nmix
ldy nmiy
rti
endasm

If you are wondering why I use 4 $2007 writes instead of one, it is because the lava will update one column over the screen each cycle, which is at max 12 16 * 16 tiles ie 48 writes and if there is only one write between each if sentence then apparently it goes outside the vblank and makes the screen flicker.

Banshaku:

The lava itself doesn't seem to be particularly demanding, even when the screen is full, when compared with having 12 units on the level at once. This is probably due to inefficient coding and I should properly clean up the unit handler section.

Good suggestion about seperating the main section into logical modules. Most of the game's code still resides in main.bas because I have had to move the prg bank switching commands around a bit to avoid having bank overflow errors, but I'm going to split the code up into more logical sections soon.

I have also noticed that the music slowed down quite a bit when a lot was happening in the game at once, but putting the commands updating the sound/music in the NMI seems to have fixed it. Now the game can slow down all it wants and the music plays on just fine.

As I'm pretty new to this I'm not really sure how to update the lava using the chr-ram method. Would this mean that all the lava on the screen would always use the same tiles?

I have put the updated source and NES file in the same link as above. The music currently in the game is just a short unfinished test tune one of my friends made.

by Kasumi on 2009-07-24 (#49173)

Banshaku: I forgot about using the stack to store those registers. My game is going to use the stack quite a lot, so I store my registers in RAM. I am also interested in your nesasm port of the famitracker driver, if you could port it after your megaman 9 demo is done.

HamsterMan: All that you have done makes the game work much better in Nintendulator. I actually never even thought to tell you that music didn't work in Nintendulator in your first version, but whatever you have done has made it work. A small note about the music: It isn't bad until the constant arpeggios. Then it's really annoying. I know it's a test song, but I thought I should say that. This game is looking interesting, and I'd like to see where you take it.

by Banshaku on 2009-07-24 (#49188)

HamsterMan wrote:
The lava itself doesn't seem to be particularly demanding, even when the screen is full, when compared with having 12 units on the level at once. This is probably due to inefficient coding and I should properly clean up the unit handler section.

It may not be demanding for the CPU but it is quite demanding for the PPU vblank. You can see the screen is refreshing from left to right, which shouldn't happen. With CHR-RAM animation, you would never see that at all.

As for coding, it's may not be because of your code specifically. I checked the assembler file made by nbasic and... shiver.. The code made is quite messy. So that may not help too. Once your game is working, try to see how you can improve the basic code to make it smaller. Maybe smaller is not appropriate, but how to optimize it to make it faster. Size is not really the issue. I checked the nbasic doc and I saw they had a few optimization trick. You can check if you can improve your code that way.

Then once you find the methods that are too intensive, if the updated basic code doesn't do anything then you can always improve the speed by pure assembly code.

HamsterMan wrote:
I have also noticed that the music slowed down quite a bit when a lot was happening in the game at once, but putting the commands updating the sound/music in the NMI seems to have fixed it. Now the game can slow down all it wants and the music plays on just fine.

Yes, if you put the music code in the NMI, music will not be affected when the logic slows the game since it will always play every NMI. But be aware that it will take one part of your vblank code windows, which is already very small and very busy because of the current way the screen is updated.

HamsterMan wrote:
As I'm pretty new to this I'm not really sure how to update the lava using the chr-ram method. Would this mean that all the lava on the screen would always use the same tiles?

It would requires a few changes that seems difficult but it's not actually. First you need to change the header to set the CHR-BANK to 0 since you will use ram.

Then you need to write the content located in bank 4 (chr-data) manually in the PPU ram since it will not be loaded automatically like chr-rom. This may be what you will find difficult at the beginning.

Once this is done, the game will work like if you had CHR-ROM except for the fact that you can update the PPU RAM in real time. This mean that yes, you will only uses 4 tiles for the lava, they will never change. What would change is the 4 tiles chr data inside the PPU RAM. If done properly, this will make your game looks a lot better and you won't see the screen refresh anymore like it is.

This is something you can do at a later stage. For now, try to separate your code by module. Later, focus on improving performance unless this issue cause a bug in your game.

Kasumi wrote:
Banshaku: I forgot about using the stack to store those registers. My game is going to use the stack quite a lot, so I store my registers in RAM. I am also interested in your nesasm port of the famitracker driver, if you could port it after your megaman 9 demo is done.

For the stack, I guess you don't really have to use it if you still have enough memory in the zero page. Just a reflex to say to use the stack

As for the driver, I think I may have ported the 2.29 to nesasm, which uses VRC6. But right now there is no real code for making sound fx so it's only useful for people that want to play music with nesasm without including an nsf file and want to make the sfx part themselves since they now can disable channels on demand. Still better than nsf include since you can put your code anywhere.

Last year (?) I was quite motivated while I was a beginner and wanted to port the code to all possible assembler but since the interest was not there and I seemed to be the only want that really want to use famitraker and not make my own sound engine, I just gave up to notify people about it and just used it in my own projects. I can always convert the code for the last one (3.0) if anybody needs it for nesasm, at the least. It's not a big task to convert actually.

Since the last versionm you can disable the channel (there is a function in it) but the way is done is not to my liking. You have to disable the channel one by one, you cannot just put a mask to disable in 1 shot all the channels you want. I can see the reason from the code thought. Since the driver can support the normal sound chip, VRC6 or MMC5, it makes the code more generic for all channels, at the cost of convenience of 1 variable. When using another chip, all channels would not fit in 1 variables. What I did in version 2.29, I had one for the normal chip and one for the VRC6.

Jsr is thinking about maybe implementing SFX with the driver. How it would work is that you could use some of the instrument as sound fx. When it will be available? For now, there is no time line for it but the idea is interesting.

For the music that was not playing, it's because the code was commented out in the previous sample. I figured it out while checking the code.

by HamsterMan on 2009-07-29 (#49300)

Well, I'm just about finished with this game by now. I would have added more features but I had barely any more space in the prg banks to work with. All in all, making a nes game certainly was an interesting experience and when I get some more free time on my hands I'm going to try making another one, or maybe a snes game.

Anyway, here it is

http://hamsterworld.no-ip.info/software ... capade.nes
http://hamsterworld.no-ip.info/software ... source.rar

There is an infinite amount of levels, and the goal is to simply collect as much gold as possible. The levels get somewhat harder the further you get.

Oh, and please mention if you discover any bugs.

by Banshaku on 2009-07-30 (#49320)

I did a quick test of it and it's interesting to see the final result of the game. Looks nice.

I'm just surprised to hear that my first sound fx that I tested on the nes was used "has-is" but it's ok I mostly made them to test the channels to see if the song was affected by pitch change and other things. I think I took some idea from one of Memblers sample if I remember well for the data structure. It seems they had more value than I thought

If you want to make other ones someday, I could explain how to edit the test data with the current code base for the driver. It's not very user friendly since it was mostly a proof of concept. The next updated driver, I will put more thought into it.

HamsterMan wrote:
Oh, and please mention if you discover any bugs.

Didn't play that much yet but in some case, there some collision glitch. For example let say that you want to go left and there is a block at the top, none in the middle and one at the bottom. In some case, the player didn't want to go left and was blocked, even thought it looked like it could have passed there. By moving up and down a little bit, after while, the character could pass.

I will try to test it more tonight if I can. Good job for your first nes game.