I didn't think it was this because his speed is always no more than 1, from what I could see in his code.  But if that fixed it, awesome.  Nice job!

Yeah, I had it at 2 first but changed it to 1, so I still think it's strange that it won't work with BEQ and BNE. :S

This isn't exactly pretty, but it should work.

Seems like I still have some bugs with my paddles, how would I do
to check for a range of values in assembly? It's all
new to me, and I only know how to compare to one value
at a time, which I guess isn't very efficient. :/

Basically:

Routine:
LDA #$(lowest possible correct value)
BCC .End
LDA #$(highest possible correct value)
BCS .End
;;do stuff
.End:
RTS

Thanks!

Each paddle is 2 sprites, and they are stored in a variable like this:
player1 .rs 2
player2 .rs 2
So for each player I want to check the values player(sprite 1 y value) and player+4(sprite 2 y value)+8.
I tried with LDA player1+4+#$08, but obviously that didn't work.
All stuff that is easy in say C++ is really weird in assembly if you are not used to it.
Any good resource for learning that kind of essential stuff in assembly?

Add them all separately, then do the calculations. You can't find the difference between a memory location and a constant or two memory locations in one instruction. Load the width and constants, then do the memory additions. Remember (S)UB and Clear for addition.

Makes sense, I hadn't thought to store the offsets as data and refer to them.  And you're right, definitely not pretty as far as doing 4 INXs haha.  But, I think I can safely clean some of it up because I won't need to set tile number/attributes/x-pos every loop through NMI so I can move those out of that.  Thanks!

Having some difficulty understanding why this collision code doesn't work:

CheckPaddle1Collision:
  ;;if ball x < paddle1x
  LDA ballx
  CMP #PADDLE1X
  BCS CheckPaddle1CollisionDone
  ;;  if ball y > paddle y top
  LDA bally
  CMP paddle1ytop
  BCC CheckPaddle1CollisionDone
  ;;    if ball y < paddle y bottom
  LDA bally
  CMP paddle1ybot
  BCS CheckPaddle1CollisionDone
  ;;      bounce, ball now moving right
  LDA #$01
  STA ballright
  LDA #$00
  STA ballleft
CheckPaddle1CollisionDone:

Try doing one pixel more/less and see.  Sometimes that seems to work for me. (if this doesn't work, pretend I didn't suggest it.

CheckPaddle1Collision:
  ;;if ball x < paddle1x
  LDA ballx
  CLC
  ADC #$01
  CMP #PADDLE1X
  BCS CheckPaddle1CollisionDone
  ;;  if ball y > paddle y top
  LDA bally
  SEC
  SBC #$01
  CMP paddle1ytop
  BCC CheckPaddle1CollisionDone
  ;;    if ball y < paddle y bottom
  LDA bally
  CMP paddle1ybot
  BCS CheckPaddle1CollisionDone
  ;;      bounce, ball now moving right
  LDA #$01
  STA ballright
  LDA #$00
  STA ballleft
CheckPaddle1CollisionDone:

This is what I have at the moment, kinda buggy, but still my best attempt yet.<br>
Player1Collisions:<br>
    LDA player1 ; paddle1 y position<br>
    CLC<br>
    ADC #$10 ; 2 sprites for every paddle, so add 16<br>
    STA ballCollY ; bottom of paddle <br><br>
    
    LDA player1<br>
    CMP ballPos+1 ; ball y position<br>
    BCS .return<br><br>
    
    LDA ballCollY<br>
    CMP ballPos+1<br>
    BCC .return<br><br>

    LDA player1+3 ; paddle 1 x position<br>
    CMP ballPos ; ball x position<br>
    BCC .return<br><br>

    LDA  #$01<br>
    STA ballDirection+1 ; bD = up, bD+1 = down, bD+2 = left, bD+3 = right, 1 is off, 0 is on<br>
    STA ballDirection+2<br>
    LDA #$00<br>
    STA ballDirection<br>
    STA ballDirection+3<br><br>

.return:<br>
    RTS

Thanks guys! KHAN, I ended up tweaking your suggestion a bit and it worked. Due to the sprite's width I used 8 pixels instead of one. Also had to swap the additions with subtractions. Here's the final code which works perfectly:

CheckPaddle1Collision:
  ;;if ball x < paddle1x
  LDA ballx
  SEC
  SBC #$08
  CMP #PADDLE1X
  BCS CheckPaddle1CollisionDone
  ;;  if ball y > paddle y top
  LDA bally
  CLC
  ADC #$08
  CMP paddle1ytop
  BCC CheckPaddle1CollisionDone
  ;;    if ball y < paddle y bottom
  LDA bally
  SEC
  SBC #$08
  CMP paddle1ybot
  BCS CheckPaddle1CollisionDone
  ;;      bounce, ball now moving right
  LDA #$01
  STA ballright
  LDA #$00
  STA ballleft
CheckPaddle1CollisionDone:

Haha, that's great. I'm glad you understood it enough to make it work with your game.

Shit, thank you KHAN and muffins, I totally forgot to take the sprite width into account. >-<

Yeah it's definitely something to look out for. Different graphics systems use different graphics handles. The NES PPU appears to place the coordinate of the sprite in the middle, which is the easiest in my opinion. I've seen certain systems use a handle in the upper left hand corner of the graphic which is a nightmare to code for.

I thought the NES put the coordinates in the upper left corner. O_o
Hmm, maybe that is why it still is acting a little strange from time to time for me, wrong calculations.

Nope.  It is upper left.

My mistake!  I'm sure I would have figured it out soon enough when things started going haywire but I was thinking of it being in the middle all along. Probably would have come out in the wash in future, more complex collisions. I guess for pong it wasn't even noticeable

I've completed a functional pong game, and going through my code I can't help but feel like there's a more efficient way to code certain sections. For example, I am very much wanting for reusable functions that I can pass variables to, like in the case of reading controllers. When I press up, I want my paddle to go up. If there are two paddles, I want to pass the function a variable that identifies what controller is receiving the input, and therefore which paddle gets moved as a result. Is there a way to do this to avoid hard coding a lot of this stuff, or is what I'm asking for simply one of the reasons modern languages exist?

Here's an example of code specific for player 1 that cannot be reused for player 2 without copying the block of code and replacing all "player 1" variables with "player 2" variables:

MovePaddle1Up:
LDA buttons1
AND #%00001000
BEQ MovePaddle1UpDone ; Use BEQ because zero flag will be set if result of AND operation is false

;;if up button pressed, move paddle up
LDA paddle1ytop
SEC
SBC paddle1speed
STA paddle1ytop
LDA paddle1ybot
SEC
SBC paddle1speed
STA paddle1ybot

;; if paddle top > top wall
LDA paddle1ytop
CMP #TOPWALL
BCS MovePaddle1UpDone
LDA #TOPWALL
STA paddle1ytop ;set top tile y-coordinate to TOPWALL
CLC
ADC #$20 ;bottom of paddle is 32 pixels below TOPWALL, so have to keep it set there
STA paddle1ybot ;otherwise, bottom y-coordinate will decrease when up is pushed
MovePaddle1UpDone:

Why do we enable NMI, sprites, background etc every time we enter another NMI frame? isn't it set once and stays set i.e. the bits are always a 1?
How does this clean up the PPU section?

;;:Set starting game state
LDA #STATEPLAYING
STA gamestate



LDA #%10010000 ; enable NMI, sprites from Pattern Table 0, background from Pattern Table 1
STA $2000

LDA #%00011110 ; enable sprites, enable background, no clipping on left side
STA $2001

Forever:
JMP Forever ;jump back to Forever, infinite loop, waiting for NMI



NMI:
LDA #$00
STA $2003 ; set the low byte (00) of the RAM address
LDA #$02
STA $4014 ; set the high byte (02) of the RAM address, start the transfer

JSR DrawScore

;;This is the PPU clean up section, so rendering the next frame starts properly.
LDA #%10010000 ; enable NMI, sprites from Pattern Table 0, background from Pattern Table 1
STA $2000
LDA #%00011110 ; enable sprites, enable background, no clipping on left side
STA $2001
LDA #$00 ;;tell the ppu there is no background scrolling
STA $2005
STA $2005

No, you can turn NMI and everything off when needed by just writing to $2000,$2001 again.

Basically if you turn off the screen anywhere in your game, to update the nametable or anything like that, it will turn everything back on in the NMI section, after everything is redrawn (hopefully). So that's why he says it's a cleanup.

If you turn the screen back on anywhere else, you'd get lots of flickering.

Is this really only necessary with background updates and not sprites? So far it's only been a noticeable help with background updates.

You need to set the scroll after you write to the PPU's address ($2006) register I believe. Other than that I don't believe anything else needs done.

As far as sprites are concerned, I basically either move them off screen or reposition them between screens, depending on what I'm trying to do. You could probably turn sprites off and back on, but I don't really see a purpose in that.