So, I'm trying to read controller data. I have written a little function thing to handle all this, but uhh... I get problems when I try to program responses for buttons other than the directional buttons.

This is what I've got:

The buttons on the NES controller are read one a time, in that order. And then the button* sub-routines are all basically the same, like this:

button*:
        ; check if button * is pressed
  lda controller       ; load button state into register A
  and #1               ; bitwise AND with 1
  beq callbackReturn   ; if the AND was true, branch to callbackReturn
                             ; callbackReturn jumps to an "rts" command
                             ; in other words, it's equivalent to a return in C
  
  ; do whatever

  rts                  ; return

Now, whenever that ; do whatever (besides the four directional buttons) involves the loading of a non-immediate value (ie- from a label or from a specific register) or the storing of a value, it sort of freezes the game (or at least prevents the working input from happening).

Umm.. your method looks overcomplicated but before i can figure out what might be wrong i need some more details.

So.. you say that controller is register-mapped to address $4016. I assume that each button is a 'bit' (how is the signal stored) 8 buttons = 8-bits?

Otherwise i don't see anything inherently wrong. You don't touch the stack, and you have a simple branch..

Actually.. how far is the branch (in memory distance)? Relative addressing is only 8-bit IIRC.

It may be overly complicated, but I don't currently know of any better ways to do it. I've seen some examples of a loop that reads all 8 buttons, but I have no idea how it works, so I'm staying away from it for now.

As far as I understand, yes that's how it works. It seems to only read one bit at a time, each time that register is read. I've tried bitwise ANDing it, having only read it once, using an 8 bit binary number, but it doesn't work, so it has to be read 8 times.

I'm new to assembly, so let me know if this was not what you meant. It's not very deep, it's within the "main loop", so it only goes 2 branches deep when running.

gameLoop:
  ; check if in VBlank

  ; draw sprite

  jsr readController
  jmp gameLoop

I can't recall the 6502 ISA off the top of my head but why not something like this

That's what I was talking about by only reading the controller once and then ANDing it with a single binary number. Unfortunately, it does not work.

If it doesn't work then maybe you should read the documents and open up your debugger. Test the simplest of cases.. track the stack. check out what happens when you hit that instruction. It is quite possibly you are overwriting your own instructions or perhaps your stack is screwed up.

ah 6502, takes me back, like Goalie Ca said, it may be your branch distance.
I think that you can branch back 127 bytes and forward 128.

To keep jumps small it may be better doing it like this :-

   bne dontreturn
   rts
dontreturn: ...

That's JR (jump relative), not JSR (jump to subroutine). JSR takes an absolute, 16bit, address.

I think the problem is most likely that you've misunderstood the NES specs, but I don't know them off hand. Maybe I can help more at lunchtime...

eh, bne is a branch which is 8 bit, jump is 16 bit yes, but the potential problem is that there is a "beq callbackReturn", callbackReturn isn't listed and could easily be beyond 128 bytes away from the branch.

I didn't spot the BEQ. I was looking at the JSRs.

Besides that, your code seems to be compliant to the docs here. Does it work if you change it to the code below?

button*:
        ; check if button * is pressed
  lda controller       ; load button state into register A
  and #1               ; bitwise AND with 1
  bne .noReturnNow  ; if the AND was true, avoid the rts that comes next

  rts
.noReturnNow
  
  ; do whatever

  rts                  ; return

I'm not sure if it's a universal trait of 6502 assemblers, but the 65SC02 assembler I used to use treated labels with a dot at the start of their name as local labels, i.e. they're accessible only between one real label (such as buttonA) and the next (presumably buttonB). So it's safe to use them in macros, and have a different .noReturnNow for each of your button functions.

Also, if you want to save a cycle, try this:

button*:
        ; check if button * is pressed
  lda controller       ; load button state into register A
  lsr               ; shift lowest bit into the carry register
  bcs .noReturnNow  ; if the AND was true, branch to callbackReturn
                             ; callbackReturn jumps to an "rts" command
                             ; in other words, it's equivalent to a return in C

  rts
.noReturnNow
  
  ; do whatever

  rts                  ; return

It's not very helpful here, but if you had an 8bit byte and wanted to do different things depending on the status of individual bits then often the fastest way to decompose it is to shift each bit in turn into carry.