Collision Cleanup

First: You have a commercial rom in that zip. (even though it's slightly hacked, it's not good to put on this forum)

Other forum members: Please check that my info on $2002, and .rs is correct. Otherwise, correct me. I don't want to be "the blind leading the blind."

67726e: Get ready for a BOOK! (Make sure to read all the comments I put in the code blocks too, there's more info there.)

In any case: It may help you to write a flowchart of how your code is running. Something much more readable than the actual code, but still gets the idea across. This post is going to help you get your code working first. There are also some bad practices, but I'll get to them later.

Starting from your main game loop (The Forever Label):

Forever:
Check if player can move.
If not, jmp to forever.

(all buttons do nothing except for the dpad, so I'll skip 'em)

Read Up.
If pressed, subtract one from player's y position unless he is outside the top of the maze.

Read Down
If pressed, add one to player's y position unless he is outside the bottom of the maze.

(Here's where it gets weird)

Read Left.
Start collision checking (Here? Why? If left is not pressed, the collision detection routine will never be run since the BEQ ReadLeftDone above it skips it. So...the player can move freely through any wall as long as left isn't pressed)

I am now going to break away from my flowchart, because there are actual coding mistakes here. Even assuming the collision detection was in the right place, it wouldn't work.




That's a problem. You can't access the first two rows, and the first column of your CollisionTable. This can be fixed by subtracting those values so that #$18, #$27 (the coordinates where your playfield starts) becomes #$00, #$00 (so you can look up the right values in the collision array.)

The quickfix is something like this:




Another good exercise (like making a flowchart) is thinking through which values are possible on a given routine. (As I did above) It helped me realize some parts of your data were inaccessible.

Moving on.



The quick fix for this?



Finally, the .rs command doesn't work like you think it does. (It may not work like I think it does either, since I don't use it. Another forum member will cover my behind if I'm wrong) .rs reserves a number of bytes, it doesn't set a variable to the number you give you it.



This does NOT set LIVES to 3, it reserves 3 bytes where the .rsset counter currently is.



That said, the final thing I did to your code to make collision detection work completely (At least while traveling left, since I didn't move your collision detection routine from where it is. Also note it ONLY checks the player's top left point, so parts of him can still pass through blocks. But it DOES work for just that point.) was this:



There are obviously problems with the other .rs uses, but I couldn't really fix them without completely breaking your code.

Here's what I believe happens.



So the fix I did above just made sure XBLOCK, YBLOCK and COLLISIONBLOCK all referred to different RAM locations. I couldn't fix your NMI_COUNTER and ALLOW_MOVE being the same RAM location without putting more thought into it than I want to right now. (Sorry, this is a LONG post, and this is actually the last part I've written. This has been like three hours or so of writing... I may make another post with a fix for this too when I'm less tired.)

It is up to you to figure out how to make collision detection work for the other directions. But now I will cover some other things in your code, that either confused me, or just aren't as well done as they could be.

I could be wrong on this, so correct me if I'm wrong other forum members.



Later, something similar.



It happens at least one other time. Interestingly enough you don't do it before writing to the scroll register ($2005, which uses the same latch), but you don't need to. But... it doesn't hurt to keep them there, really (just uses some extra bytes and cycles at startup). I say again, if I'm wrong about the above please let me know other forum people, since I'd need to do some stuff in my own games. And we might want to clarify the wiki if I'm right, since I can see where he got the idea.





Loading a value will change the zero bit. So...



Of course you'll still need to cmp/cpy if you have more than two possibilities. But if you only have two, and one of them is zero, this is fine.

One other thing. One of the things people hate about NESASM is you have to specify zero page RAM locations to get any gain from them. You do this by putting < before a zero page RAM location at each use. Otherwise, it's slower than it should be, and uses 1 more byte than it should. (the same as regular RAM)



Yes... every time you use a zero page RAM location. I didn't do that on any of the fixed code other than this, but I totally should've.

Another subtle optimization tip



Loop optimization is a very good skill to have when working on more complex games.



TAX is always faster than storing and loading the accumulator, and assuming COLLISIONBLOCK (which is RAM) isn't used for anything else (it's not), then... you can use it for something else. Essentially it's wasted RAM, and the code is slower/uses more bytes than it could.




Similar to something above, loading a value changes the zero status bit.



Edit: OH MY GOODNESS, I FORGOT THIS WHICH IS ACTUALLY IMPORTANT!



Which... brings me to a similar point in another location.



Actually while I'm still here, one more loop optimization thing (This uses my revised code, but the fix holds true for what you originally had):



So to make it not clear during the loop:



But you can certainly feel free to always CLC in a loop. And of course ALWAYS do it when you're not %100 sure of the status of it before an addition somewhere in your code. Most of these later tips are just to get you to think more about the code you're writing, and to think through how to make things faster, but you can ignore almost everything after I finished getting the code working. (EXCEPT THE JSR THING!)

Edit: I've been writing this for hours now, might as well keep going. A good idea to get used to subroutines might be writing subroutines for each joypad button. Because the way you have it now, all code that needs to check for a button needs to be placed in between those code blocks, and that will get messy and hard to read FAST.

If you had subroutines for the joypad buttons you could do stuff like this:



You could make your collision detection a subroutine so you could use it for the player and all monsters in the maze. (Or at least use it for all the directions without copying and pasting the whole thing). Try it out, if you don't understand how to make a subroutine, or how to make one work, ask! I'll help! (And I'll try to write less than this)

That concludes the book. I'll edit this if I'm corrected about .rs/$2002 (or anything else, just so someone doesn't have to read later in the thread to find out I'm wrong.