No, it should look exactly like it was in my post.
Code: Select all
NMI:
PHA
STA temp;You're storing a new value into temp. (A)
LDA temp;You're loading the value you just stored from A back into A. Why? The value that's there just came from A.
TXA;You are putting the value of X into A, so the above line effectively does nothing because you never used the result.
PHA
STA temp;You're storing a new value into temp. (A) This effectively makes the first store to temp above do nothing, because its value effectively wasn't used in between being written and being overwritten.
LDA temp;You're loading the value you just stored from A back into A. Why? That value is already in A.
TYA;You are putting the value of X into A, so the above line effectively does nothing.
pha
LDA temp;This line effectively does nothing
NewAttribCheck:
LDA scroll;Because of this line.
You're loading the value of temp into A, but before it is used, you load the value of scroll into A.
For understanding: The value of A doesn't change after a store, so loading from the exact place you just stored after a load usually does nothing. (Unless you're relying on flag changes, and you are not here.)
Edit: Just as another warning, you should make sure the temp RAM for your NMI isn't used anywhere else. (It's not here, but still.) For a similar reason to why you need to push A, X and Y to the stack. If the NMI interrupts code that expects the temp RAM to be a certain value, and the NMI changes the temp RAM very bad things will probably happen.
You can safely read variables in your NMI, always be careful when something is written.
Another similar piece of code:
Code: Select all
DLs:
lda columnNumber
SEC
SBC #$01 ; go to next column
and #%11111111 ; only 256 columns of data, throw away top bit to wrap
sta columnNumber
JMP Con;This line effectively does nothing
Con:
You are jumping to Con, but the code could naturally travel there if that line wasn't there.
One of the problems is that you have several RTIs in your code. JSR Updating eventually ends up at some animation labels (FAni, etc) that jumps to done:
Code: Select all
Done:
lda #$00
sta NMIB+2
PLA
TAY
PLA
TAX
PLA
rti
Which pulls values off the stack that weren't pushed there (that I can see), and also returns with RTI instead of RTS.
Code: Select all
main:
JSR updating
jmp main
updating:
;code in between
rts
Anything you JSR to should end in RTS (unless you
really know what you're doing.) That is currently not true, at least for updating. One path of updating is updating->aniFrame->Vertical->Direction->DDIR->Gravity->F1->Jump->Delay->AnimationP1->Animation->scrolling->Pa2->Continue->p2->FAniP->FAni->Done->RTI
(That path may not be 100% possible since I didn't learn enough about your game state to be sure what are possible values. Still, you probably get the idea.)
You have another RTI under ReadUpDone that maybe should be an RTS, but I didn't take the time to study how your code gets there. (Or
if it gets there.) The reason it's suspicious is because it doesn't pull values from the stack, and the only interrupt start code you have
does push.
No matter how complicated the code in between updating: and the RTS is, updating still must end in RTS (unless you
really know what you're doing.) This is how you get back to the line beneath way back near the "main:" label. Anytime I create a new subroutine, I make a habit of typing its name, then the RTS underneath it, both before even adding the JSR to its name to my code. Then I write the code in between and will be safe. If I want to create another subroutine the subroutine I just created goes to, same process. label, rts, then jsr, then code under label.
I'd maybe recommend starting completely over rather than trying to band aid this code. It probably sounds scary, but I think with what you've learned you'll actually spend less time rewriting than you will chasing problems in this and you'll end up with better code as a result. I recommend writing code in smaller parts. It helps me read, but it also helps
you read.
I had to do a lot of work to find out what happened to updating. It could look something like this:
Code: Select all
main:
jsr waitVblank
jsr updating
jmp main
waitVblank:
;Code to wait for the frame start here
rts
updating:
jsr readbuttons
jsr updateplayer
rts
readbuttons:
;code to read buttons
rts
updateplayer:
jsr updateplayerstate
jsr updateplayeranimations
rts
updateplayerstate:
rts
updateplayeranimations:
rts
Basically, the more code you have in between the start of a subroutine's label and the RTS, the more paths you have to follow to find out what it's really doing. Dividing things allows you to create code you can verify all paths of without even scrolling in your text editor. Things get way less dense when you know the problem is in the 40 line updateplayeranimations rather than somewhere in 400 line updating.